WIMAR - Onboarding #1005

wimar-odoo · 2025-10-22T14:10:53Z

No description provided.

robodoo · 2025-10-22T14:10:56Z

thcl-odoo

First set of comments :) Overall, it's fine!

Here are a few general comments:

Try to be consistent with the use of single quotes ' and ". If you start using one of them, be consistent throughout :)
Be careful with commit conventions (you can check the documentation here: Documentation )

For example [ADD] Chapter 1: Init and manifest => [ADD] estate: app creation
[ADD] Chapter 3: Models and Basic Fields => [IMP] estate: add models & basic fields etc.

estate/models/__init__.py

estate/models/estate_property.py

estate/__init__.py

estate/__manifest__.py

.gitignore

Create empty __init__.py and __manifest__.py with module name, set as application.

Created the Estate Property model and added it to the __init__.py file. Defined the model’s attributes and fields.

Created access rights granting full permissions to base.group_user and added them to the __manifest__.py file.

Created the first action and menu, and added them to the __manifest__.py file. Linked the action to the menu.

- Added custom views: List, Form, and Search - Fixed empty line at EOF to comply with PEP8

- Added property types such as House, Apartment, etc., with corresponding Menus, Actions, and Views - Added property tags such as Renovated, etc., with corresponding Menus, Actions, and Views - Added an Offers tab on properties to display all offers related to a specific property

- Added computed, inverse, and onchange fields to improve user convenience and automation: total_area, best_price, validity, date_deadline, and garden

- Added buttons to sell or cancel a property, and to accept or refuse an offer

- Added SQL constraints to ensure positive prices and unique names - Added Python constraints to enforce offers at least 90% of the expected price

thcl-odoo

New batch of comments but looking good so far :)

thcl-odoo · 2025-10-27T08:37:34Z

estate/models/estate_property.py

+        for record in self:
+            if record.state == "cancelled":
+                raise UserError("Cancelled properties cannot be sold.")
+            record.state = "sold"
+        return True


It's better to make the error message translatable :)

raise UserError(_("Cancelled properties cannot be sold."))

You could also do something like this

Suggested change

for record in self:

if record.state == "cancelled":

raise UserError("Cancelled properties cannot be sold.")

record.state = "sold"

return True

if self.filtered(lambda x: x.state == "cancelled"):

raise UserError(_("Cancelled properties cannot be sold."))

self.state = "sold"

return True

estate/models/estate_property.py

thcl-odoo · 2025-10-27T08:39:25Z

estate/models/estate_property_offer.py

+        selection=[("accepted", "Accepted"), ("refused", "Refused")],
+        copy=False,
+    )
+    partner_id = fields.Many2one("res.partner", string="Buyer",  required=True)


linter cries about this space 😅

Suggested change

partner_id = fields.Many2one("res.partner", string="Buyer", required=True)

partner_id = fields.Many2one("res.partner", string="Buyer", required=True)

estate/models/estate_property_offer.py

thcl-odoo · 2025-10-27T08:48:15Z

estate/security/ir.model.access.csv

+access_estate_property_user,access_estate_property_user,model_estate_property,base.group_user,1,1,1,1
+access_estate_property_type_user,access_estate_property_type_user,model_estate_property_type,base.group_user,1,1,1,1
+access_estate_property_tag_user,access_estate_property_tag_user,model_estate_property_tag,base.group_user,1,1,1,1
+access_estate_property_offer_user,access_estate_property_offer_user,model_estate_property_offer,base.group_user,1,1,1,1


missing line break 😄

- Automatic and manual ordering - Decorations and invisible attributes - Stat button with action - Domain filters

- Override create method with ondelete constraint - Inherit res.users to display properties on user form

thcl-odoo

new batch of comments 😄

this commit [IMP] estate: Chapter 13 - Other modules could/should be sth like [ADD] estate_account: Chapter 13 - Other modules

thcl-odoo · 2025-10-29T07:42:59Z

estate/models/estate_property.py

+    _positive_expected_price = models.Constraint(
+        "CHECK(expected_price > 0)",
+        "The expected price must be strictly positive.",
+    )


just a small comment like that but nothing mandatory: I feel like it's more readable to have the constraints just under the fields, so they are not "hidden" between all those functions :)

but it's just a personal preference

thcl-odoo · 2025-10-29T07:45:22Z

estate/models/estate_property_offer.py

+    @api.model
+    def create(self, vals_list):
+        for vals in vals_list:
+            if vals["price"] <= self.env["estate.property"].browse(vals["property_id"]).best_price:


it's better to make the orm method outside of the loop.
For example, you could retrieve every "estate.property" outside of the loop and then filtered them in your loop

thcl-odoo · 2025-10-29T07:46:22Z

estate/models/estate_property_offer.py

+        for vals in vals_list:
+            if vals["price"] <= self.env["estate.property"].browse(vals["property_id"]).best_price:
+                raise UserError(_("The offer price must be strictly greater than the current best offer."))
+            self.env["estate.property"].browse(vals["property_id"]).state = "received"


same with this line, if you use an intermediate variable with your filtered property

thcl-odoo · 2025-10-29T07:49:33Z

estate/views/estate_property_views.xml

+                <field name="living_area" filter_domain="[('living_area', '>', self)]"/>
+                <field name="facades"/>
+                <separator/>
+                <filter string="Available" name="state" domain="['|',('state', '=', 'new'),('state', '=', 'received')]"/>


it could be

Suggested change

<filter string="Available" name="state" domain="['|',('state', '=', 'new'),('state', '=', 'received')]"/>

<filter string="Available" name="state" domain="[('state', 'in', ['new', 'received'])]"/>

- Create module to generate invoices in Invoicing

thcl-odoo

new batch of comments, wp :p

thcl-odoo · 2025-10-31T14:11:40Z

awesome_owl/static/src/counter/counter.xml

+
+    <t t-name="awesome_owl.counter">
+        <div>
+            Counter: <t t-esc="state.value"/>


Suggested change

Counter: <t t-esc="state.value"/>

Counter: <t t-out="state.value"/>

thcl-odoo · 2025-10-31T14:14:52Z

awesome_owl/static/src/counter/counter.js

+        if (this.props.onChange) {
+            this.props.onChange();
+        }


just for your information, it's maybe less readable in this case but in JS you can do something like this

Suggested change

if (this.props.onChange) {

this.props.onChange();

}

this.props.onChange?.();

thcl-odoo · 2025-10-31T14:16:44Z

awesome_owl/static/src/playground.scss

+.chapter_tuto {
+    border: solid; padding: 10px; border-radius: 5px; margin-bottom: 5px
+}


just for lisibility :)

Suggested change

.chapter_tuto {

border: solid; padding: 10px; border-radius: 5px; margin-bottom: 5px

}

.chapter_tuto {

border: solid;

padding: 10px;

border-radius: 5px;

margin-bottom: 5px

}

thcl-odoo · 2025-10-31T14:16:58Z

awesome_owl/static/src/playground.xml

+        <div class="chapter_tuto">
+            <Counter onChange.bind="incrementSum" t-props="{'buttonText': 'Custom Incr.'}"/>
+            <Counter onChange.bind="incrementSum"/>
+            <p>The sum id: <t t-esc="state.value"/></p>


Suggested change

<p>The sum id: <t t-esc="state.value"/></p>

<p>The sum id: <t t-out="state.value"/></p>

thcl-odoo

Oups I missed this one

thcl-odoo · 2025-10-31T14:34:40Z

awesome_owl/static/src/counter/counter.xml

+            <button class="btn btn-primary" t-on-click="increment" style="margin-left: 5px">
+                <t t-out="props.buttonText"/>
+            </button>


You can "inline" the t-out like this

Suggested change

<button class="btn btn-primary" t-on-click="increment" style="margin-left: 5px">

<t t-out="props.buttonText"/>

</button>

<button class="btn btn-primary" t-on-click="increment" style="margin-left: 5px" t-out="props.buttonText" />

thcl-odoo

Good job!

Lastest batch of comments

thcl-odoo · 2025-11-03T08:37:56Z

awesome_dashboard/static/src/dashboard/dashboard_configuration/dashboard_configuration.js

+    }
+
+    getItemDescription(item) {
+        return item.description || "Dashboard item";


Suggested change

return item.description || "Dashboard item";

return item.description || _t("Dashboard item");

thcl-odoo · 2025-11-03T08:38:24Z

awesome_dashboard/static/src/dashboard/dashboard_configuration/dashboard_configuration.xml

+                                t-on-change="() => this.toggleItem(item.backend_attribute)"
+                            />
+                            <label class="form-check-label" t-att-for="'item_' + item.backend_attribute">
+                                <t t-esc="getItemDescription(item)"/>


Suggested change

<t t-esc="getItemDescription(item)"/>

<t t-out="getItemDescription(item)"/>

thcl-odoo · 2025-11-03T08:39:59Z

awesome_dashboard/static/src/dashboard/numbercard/numbercard.js

+            type: Number,
+        }
+    }
+}


Missing EOL

thcl-odoo · 2025-11-03T08:45:28Z

awesome_dashboard/static/src/dashboard/dashboard_configuration/dashboard_configuration.xml

+                                class="form-check-input"
+                                t-att-id="'item_' + item.backend_attribute"
+                                t-att-checked="state.selectedItems[item.backend_attribute]"
+                                t-on-change="() => this.toggleItem(item.backend_attribute)"


No need to change, but another pattern could have been used like.
Sometimes, it can be useful so you've seen this at least once :)

Suggested change

t-on-change="() => this.toggleItem(item.backend_attribute)"

t-att-data-backend-attribute="item.backend_attribute"

t-on-change="toggleItem"

And then update your JS function like this

toggleItem(event) { const backendAttribute = event.currentTarget.getAttribute("backend-attribute") this.state.selectedItems[backendAttribute] = !this.state.selectedItems[backendAttribute]; }

thcl-odoo · 2025-11-03T08:46:07Z

awesome_dashboard/static/src/dashboard/numbercard/numbercard.xml

+<templates xml:space="preserve">
+
+    <t t-name="awesome_dashboard.NumberCard">
+        <p class="text-center"><t t-out="props.title"/></p>


could be simplified like this

Suggested change

<p class="text-center"><t t-out="props.title"/></p>

<p class="text-center" t-out="props.title"/>

thcl-odoo · 2025-11-03T08:56:04Z

awesome_dashboard/static/src/dashboard/dashboard.js

+        const visibleItems = {};
+        Object.entries(this.allItems).forEach(([itemId, item]) => {
+            if (!this.state.hiddenItems.includes(item.backend_attribute)) {
+                visibleItems[itemId] = item;
+            }
+        });
+        return visibleItems;


In general, when you want to "migrate" from an array to a single value (here a dict), .reduce(...) is the solution :)

(just for your information)

Suggested change

const visibleItems = {};

Object.entries(this.allItems).forEach(([itemId, item]) => {

if (!this.state.hiddenItems.includes(item.backend_attribute)) {

visibleItems[itemId] = item;

}

});

return visibleItems;

return Object.entries(this.allItems).reduce((acc, [itemId, item]) => {

if (!this.state.hiddenItems.includes(item.backend_attribute)) {

acc[itemId] = item;

}

return acc;

}, {});

thcl-odoo · 2025-11-03T08:57:10Z

awesome_dashboard/static/src/dashboard/dashboard.js

+            const config = {};
+            for (const item of Object.values(this.allItems)) {
+                config[item.backend_attribute] = !hiddenItems.includes(item.backend_attribute);
+            }


Same here for the reduce (but nothing mandatory)

Suggested change

const config = {};

for (const item of Object.values(this.allItems)) {

config[item.backend_attribute] = !hiddenItems.includes(item.backend_attribute);

}

const config = Object.values(this.allItems).reduce((acc, item) => {

acc[item.backend_attribute] = !hiddenItems.includes(item.backend_attribute);

return acc;

}, {});

thcl-odoo · 2025-11-03T08:58:11Z

awesome_dashboard/static/src/dashboard/dashboard.scss

+  margin: 5px;
+  padding: 10px;
+  color: black;
+}


missing eol

thcl-odoo · 2025-11-03T08:58:57Z

awesome_dashboard/static/src/dashboard/dashboard_item.xml

+
+    <t t-name="awesome_dashboard.dashboarditem">
+        <div class="item_card" t-att-style="`width: ${18 * props.size}rem;`">
+            <t t-slot="default"></t>


could be autoclosing tag

Suggested change

<t t-slot="default"></t>

<t t-slot="default" />

thcl-odoo · 2025-11-03T08:59:34Z

awesome_dashboard/static/src/dashboard/statistics_service.js

+    },
+};
+
+registry.category("services").add("awesome_dashboard.statistics", statisticsService);


missing EOL 😄

wimar-odoo requested a review from thcl-odoo October 22, 2025 14:10

thcl-odoo reviewed Oct 23, 2025

View reviewed changes

wimar-odoo added 6 commits October 23, 2025 14:52

[ADD] estate: Init and manifest

6e4093e

Create empty __init__.py and __manifest__.py with module name, set as application.

[IMP] estate: Models and Basic Fields

66a0801

Created the Estate Property model and added it to the __init__.py file. Defined the model’s attributes and fields.

[IMP] estate: Security - A Brief Introduction

82e151b

Created access rights granting full permissions to base.group_user and added them to the __manifest__.py file.

[IMP] estate: Finally, Some UI to Play With

cd40928

Created the first action and menu, and added them to the __manifest__.py file. Linked the action to the menu.

[IMP] estate: Basic Views

9a9a4f3

- Added custom views: List, Form, and Search - Fixed empty line at EOF to comply with PEP8

[FIX] estate: Remove useless string parameter from model attributes

9a90bd7

wimar-odoo force-pushed the 19.0-onboarding-wimar branch from 8e27485 to bd3010f Compare October 23, 2025 12:57

[FIX] estate: adjustments after first review

db9ded5

wimar-odoo force-pushed the 19.0-onboarding-wimar branch from bd3010f to db9ded5 Compare October 23, 2025 13:15

wimar-odoo added 4 commits October 24, 2025 09:20

[IMP] estate: Chapter 8 - Computed, Inverse, and Onchange Fields

edaf208

- Added computed, inverse, and onchange fields to improve user convenience and automation: total_area, best_price, validity, date_deadline, and garden

[IMP] estate: Chapter 9 - Button Actions

1d073e0

- Added buttons to sell or cancel a property, and to accept or refuse an offer

[IMP] estate: Chapter 10 - SQL and Python Constraints

4353811

- Added SQL constraints to ensure positive prices and unique names - Added Python constraints to enforce offers at least 90% of the expected price

thcl-odoo reviewed Oct 27, 2025

View reviewed changes

wimar-odoo added 3 commits October 27, 2025 14:15

[IMP] estate: Chapter 11 - Sprinkles

cad2ef0

- Automatic and manual ordering - Decorations and invisible attributes - Stat button with action - Domain filters

[FIX] estate: PR review adjustments

fb58afd

[IMP] estate: Chapter 12 - Inheritance

a9e718c

- Override create method with ondelete constraint - Inherit res.users to display properties on user form

wimar-odoo force-pushed the 19.0-onboarding-wimar branch 2 times, most recently from e876115 to d87db09 Compare October 28, 2025 15:42

thcl-odoo reviewed Oct 29, 2025

View reviewed changes

wimar-odoo added 3 commits October 30, 2025 09:51

[ADD] estate_account: Chapter 13 - Other modules

9081d57

- Create module to generate invoices in Invoicing

[IMP] estate: Chapter 14 - Kanban view

f1448b2

[IMP] awesome_owl: Chapter 1 - Intro Owl Components

b48fa58

wimar-odoo force-pushed the 19.0-onboarding-wimar branch from 9ba22fe to b48fa58 Compare October 30, 2025 08:52

[FIX] estate: last review adjustments

bd25287

thcl-odoo reviewed Oct 31, 2025

View reviewed changes

wimar-odoo added 2 commits November 3, 2025 09:10

[IMP] awesome_dashboard: Chapter 2 - Build a dashboard

816de58

[FIX] awesome_owl: Last review adjustments

eefcaf9

thcl-odoo reviewed Nov 3, 2025

View reviewed changes

[FIX] awesome_dashboard: Last review adjustments

2918ab0

	partner_id = fields.Many2one("res.partner", string="Buyer", required=True)
	partner_id = fields.Many2one("res.partner", string="Buyer", required=True)

	<filter string="Available" name="state" domain="['\|',('state', '=', 'new'),('state', '=', 'received')]"/>
	<filter string="Available" name="state" domain="[('state', 'in', ['new', 'received'])]"/>

	Counter: <t t-esc="state.value"/>
	Counter: <t t-out="state.value"/>

	<p>The sum id: <t t-esc="state.value"/></p>
	<p>The sum id: <t t-out="state.value"/></p>

	return item.description \|\| "Dashboard item";
	return item.description \|\| _t("Dashboard item");

	<t t-esc="getItemDescription(item)"/>
	<t t-out="getItemDescription(item)"/>

	t-on-change="() => this.toggleItem(item.backend_attribute)"
	t-att-data-backend-attribute="item.backend_attribute"
	t-on-change="toggleItem"

	<p class="text-center"><t t-out="props.title"/></p>
	<p class="text-center" t-out="props.title"/>

WIMAR - Onboarding #1005

Are you sure you want to change the base?

WIMAR - Onboarding #1005

Uh oh!

Conversation

wimar-odoo commented Oct 22, 2025

Uh oh!

robodoo commented Oct 22, 2025

Uh oh!

thcl-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thcl-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thcl-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thcl-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thcl-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thcl-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment