romc - Technical Training #208

romaincarlier4 · 2024-12-16T13:09:00Z

No description provided.

robodoo · 2024-12-16T13:09:03Z

kfr-odoo

LGTM :)

Configure your IDE to get ride of EOF issues. "Insert Final Newline" in VScode.
Remove all # -*- coding: utf-8 -*-, they are not required anymore (since version 16.0).

kfr-odoo · 2024-12-18T14:06:59Z

estate/models/__init__.py

@@ -0,0 +1 @@
+from . import estate_property, estate_property_type, estate_property_tag, estate_property_offer


We usually split imports, one by line. Does not change much but it is good to stay consistent with the standard codebase

kfr-odoo · 2024-12-18T14:14:29Z

estate/models/estate_property.py

+    @api.depends("garden_area", "living_area")
+    def _compute_total_area(self):
+        for record in self:
+            record.total_area = record.garden_area + record.living_area
+
+    best_price = fields.Float(compute="_compute_best_price")


Methods after fields

Suggested change

@api.depends("garden_area", "living_area")

def _compute_total_area(self):

for record in self:

record.total_area = record.garden_area + record.living_area

best_price = fields.Float(compute="_compute_best_price")

best_price = fields.Float(compute="_compute_best_price")

@api.depends("garden_area", "living_area")

def _compute_total_area(self):

for record in self:

record.total_area = record.garden_area + record.living_area

kfr-odoo · 2024-12-18T14:16:03Z

estate/models/estate_property.py

+    def _compute_best_price(self):
+        for record in self:
+            prices = record.offer_ids.mapped("price")
+            record.best_price = max(prices) if len(prices) else 0


Nitpicking, max() default param syntax is better than a ternary

Suggested change

record.best_price = max(prices) if len(prices) else 0

record.best_price = max(prices, default=0)

kfr-odoo · 2024-12-18T14:17:14Z

estate/models/estate_property_offer.py

+    @api.depends("create_date", "validity")
+    def _compute_date_deadline(self):
+        for record in self:
+            create_date = record.create_date if record.create_date else date.today()


Nitpicking: shorter syntax

Suggested change

create_date = record.create_date if record.create_date else date.today()

create_date = record.create_date or date.today()

kfr-odoo · 2024-12-19T12:58:57Z

estate/models/estate_property_offer.py

+        other_records = self.search(
+            [("id", "!=", str(self.id)), ("property_id", "=", self.property_id.id)]
+        )
+        for rec in other_records:
+            if rec.status == "accepted":
+                raise UserError("Cannot have more than one accepted offer.")


Suggested change

other_records = self.search(

[("id", "!=", str(self.id)), ("property_id", "=", self.property_id.id)]

)

for rec in other_records:

if rec.status == "accepted":

raise UserError("Cannot have more than one accepted offer.")

accepted_offers = self.search(

[("id", "!=", str(self.id)), ("property_id", "=", self.property_id.id), ("status", "=" "accepted")]

)

if accepted_offers:

raise UserError("Cannot have more than one accepted offer.")

kfr-odoo · 2024-12-19T13:00:44Z

estate/models/estate_property_offer.py

+    def action_refuse_offer(self):
+        self.ensure_one()
+        old_status = self.status
+        self.status = "refused"
+        if old_status == "accepted":
+            self.property_id.buyer_id = ""
+            self.property_id.selling_price = 0
+            self.property_id.state = "offer_received"
+        return True


Suggested change

def action_refuse_offer(self):

self.ensure_one()

old_status = self.status

self.status = "refused"

if old_status == "accepted":

self.property_id.buyer_id = ""

self.property_id.selling_price = 0

self.property_id.state = "offer_received"

return True

def action_refuse_offer(self):

self.ensure_one()

if self.status == "accepted":

self.property_id.buyer_id = ""

self.property_id.selling_price = 0

self.property_id.state = "offer_received"

self.status = "refused"

return True

kfr-odoo · 2024-12-20T13:37:42Z

estate/models/estate_property.py

+        if any(
+            record.state in ("offer_received", "offer_accepted", "sold")
+            for record in self
+        ):


There is a builtin for that

Suggested change

if any(

record.state in ("offer_received", "offer_accepted", "sold")

for record in self

):

if self.filtered(lambda property: property.state in ("offer_received", "offer_accepted", "sold")):

kfr-odoo · 2024-12-20T13:40:21Z

estate/models/estate_property_offer.py

+            self.env["estate.property"].browse(
+                vals["property_id"]
+            ).state = "offer_received"


browse() is unreliable and should always be suffixed by exists()
The point of this method is to create a recordset with some ids inside but it does not verify their existence

Suggested change

self.env["estate.property"].browse(

vals["property_id"]

).state = "offer_received"

self.env["estate.property"].browse(

vals["property_id"]

).exists().state = "offer_received"

It could be mentioned in the tutorial ?

"Yes indeed, I'll make sure to mention it in the new tutorial" -ANV

kfr-odoo · 2024-12-20T13:46:36Z

estate/models/estate_property_offer.py

+            self.env["estate.property"].browse(
+                vals["property_id"]
+            ).state = "offer_received"
+            if any(record.price > vals["price"] for record in all_offers):


You should only check offers for a given property right ? Not all offers of all the properties in the db

kfr-odoo · 2024-12-23T13:20:11Z

awesome_owl/static/src/counter/counter.js

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


It is surprising that this props may not exists. The best approach would be to figure out why it is not provided sometime, fix the issue and remove this if check.
If it is inevitable, here is an alternative syntax that you may (or not) want to use

Suggested change

if (this.props.onChange) {

this.props.onChange();

this.props.onChange?.();

This is what is expected. The counter component can optionnally have a callback prop, but can also very well be a simple counter with no callback needed. I changed with the better '?' syntax.

kfr-odoo · 2024-12-23T14:23:35Z

estate/models/estate_property_offer.py

+                )
+
+            property_offers = offers_per_property[vals["property_id"]]
+            property_model = self.env["estate.property"].browse(vals["property_id"])


Naming conventions used ( not necessary to split in two lines )

Suggested change

property_model = self.env["estate.property"].browse(vals["property_id"])

EstateProperty = self.env["estate.property"]

property = EstateProperty.browse(vals["property_id"])

kfr-odoo · 2024-12-23T14:24:46Z

estate/tests/test_estate.py

+    @classmethod
+    def setUpClass(cls):
+
+        super(EstateTestCase, cls).setUpClass()


Suggested change

super(EstateTestCase, cls).setUpClass()

super().setUpClass()

kfr-odoo · 2024-12-27T14:06:55Z

awesome_dashboard/static/src/dashboard/dashboard.js

+import { DashboardItem } from "./dashboard_item";
+import { PieChart } from "../piechart/piechart";
+import { DashboardDialog } from "./dashboard_dialog";
+import { browser } from "@web/core/browser/browser";


Import organization could be improved

kfr-odoo reviewed Dec 17, 2024

View reviewed changes

kfr-odoo reviewed Dec 18, 2024

View reviewed changes

romaincarlier4 force-pushed the 18.0-training-romc branch from ce3c108 to f107512 Compare December 18, 2024 14:40

romaincarlier4 added 8 commits December 18, 2024 15:41

[ADD] estate: start new module for training

d98b856

[IMP] add fields for real estate table

6123ef7

[IMP] estate: add security access rules

7d89dad

[IMP] estate: add views and menu

42bf41b

[IMP] estate: add list, form and search views

973bf38

[IMP] estate: relations between models

720e52a

[IMP] estate: add compute and onchange fields

eb9b85a

[IMP] estate: add sold/cancel and accept/refuse buttons

66a5c8d

romaincarlier4 force-pushed the 18.0-training-romc branch from f107512 to 66a5c8d Compare December 18, 2024 14:41

romaincarlier4 added 4 commits December 18, 2024 15:52

[FIX] estate: fix bug for accepting offers

2120538

[IMP] estate: coding guidelines improvements

99ca07a

[IMP] estate: add SQL and python constraints

4f294be

[IMP] estate: add the sprinkles ✨

b0417e3

kfr-odoo reviewed Dec 19, 2024

View reviewed changes

[IMP] estate: add inheritance

63f631c

romaincarlier4 force-pushed the 18.0-training-romc branch from e236c0f to 63f631c Compare December 20, 2024 08:30

romaincarlier4 added 4 commits December 20, 2024 09:35

[IMP] estate: refactor code

9c873c0

[IMP] estate: link sold properties to invoices

d6db194

[IMP] estate: add kanban view

c745b80

[FIX] estate: fix kanban warning

b22d5d9

kfr-odoo reviewed Dec 20, 2024

View reviewed changes

romaincarlier4 added 3 commits December 20, 2024 17:14

[IMP] estate: refactor code

977f427

[ADD] awesome owl: chapter 1

dbcf657

[IMP] estate: add tests

f1c5751

kfr-odoo reviewed Dec 23, 2024

View reviewed changes

[ADD] awesome dashboard: chapter 2

6704364

romaincarlier4 added 4 commits December 26, 2024 10:55

[IMP] awesome owl: refactor code

2e5e8f1

[IMP] estate: follow coding guidelines

ccfc291

[IMP] estate: add demo data

e34aacb

[FIX] estate: fix demo data

488345e

kfr-odoo approved these changes Dec 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

romc - Technical Training #208

romc - Technical Training #208

romaincarlier4 commented Dec 16, 2024

robodoo commented Dec 16, 2024

kfr-odoo left a comment

kfr-odoo Dec 18, 2024

kfr-odoo Dec 18, 2024

kfr-odoo Dec 18, 2024

kfr-odoo Dec 18, 2024

kfr-odoo Dec 19, 2024

kfr-odoo Dec 19, 2024

kfr-odoo Dec 20, 2024

kfr-odoo Dec 20, 2024

romaincarlier4 Dec 20, 2024

kfr-odoo Dec 20, 2024

kfr-odoo Dec 20, 2024

kfr-odoo Dec 23, 2024

romaincarlier4 Dec 26, 2024

kfr-odoo Dec 23, 2024

kfr-odoo Dec 23, 2024

kfr-odoo Dec 27, 2024

		@@ -0,0 +1 @@
		from . import estate_property, estate_property_type, estate_property_tag, estate_property_offer

	record.best_price = max(prices) if len(prices) else 0
	record.best_price = max(prices, default=0)

	create_date = record.create_date if record.create_date else date.today()
	create_date = record.create_date or date.today()

	if (this.props.onChange) {
	this.props.onChange();
	this.props.onChange?.();

	property_model = self.env["estate.property"].browse(vals["property_id"])
	EstateProperty = self.env["estate.property"]
	property = EstateProperty.browse(vals["property_id"])

romc - Technical Training #208

Are you sure you want to change the base?

romc - Technical Training #208

Conversation

romaincarlier4 commented Dec 16, 2024

robodoo commented Dec 16, 2024

kfr-odoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment