[ADD] estate: adding new estate module #727

amry-odoo · 2025-04-22T13:09:00Z

Add manifest and dependencies...

robodoo · 2025-04-22T13:09:11Z

SergeBayet

Great first PR! Some remarks though 👍

SergeBayet · 2025-04-22T13:25:21Z

estate/__init__.py

@@ -0,0 +1 @@
+from . import models


Missing blank line at the end of your file.

SergeBayet · 2025-04-22T13:25:29Z

estate/__manifest__.py

+    'depends': [
+        'base',
+    ]
+}


SergeBayet · 2025-04-22T13:25:52Z

estate/models/estate_property.py

+    garden_area = fields.Integer()
+    garden_orientation = fields.Selection(
+        string='Orientation',
+        selection=[('north','North'), ('south','South'), ('east', 'East'), ('west', 'West')]


Suggested change

selection=[('north','North'), ('south','South'), ('east', 'East'), ('west', 'West')]

selection=[('north', 'North'), ('south', 'South'), ('east', 'East'), ('west', 'West')]

Add manifest and dependencies...

SergeBayet

You're probably need a linter in your IDE to avoid these kind of errors. See below.

estate/models/estate_property.py

SergeBayet

Ok for me. Just a little typo :)

estate/views/estate_property_views.xml

SergeBayet

Good! 👍

SergeBayet · 2025-04-24T11:51:43Z

@naja628 For you :)

naja628

Hello :) -- Good work !

You have one commit left whose title doesn't conform with the guidelines.

I left some more comments on your code

naja628 · 2025-04-25T07:59:59Z

estate/models/estate_property.py

+    name = fields.Char(required=True)
+    description = fields.Text()
+    postcode = fields.Char()
+    date_availability = fields.Date(copy=False, default=du.add(date.today(), months=3))


Here date.today() will be evaled once when the whole class is evaled (i.e. everytime you launch odoo i think).
When it needs to be re-evaled everytime you create new records you can pass a lambda or function to the default kwarg instead

estate/models/estate_property.py

estate/models/estate_property_offer.py

estate/views/estate_property_views.xml

estate/views/estate_menus.xml

Added a functional module to create new listings following all the chapter 5 guidelines.

Chapter 6 views: search, list, form done

Repo now is up to date with chapter 7

Added an onchange field and some compute fields, following Chapter 8 rules. Also refactored different views to separate them

Added actions as mentioned in chapter 9 did some refactoring to follow latest comments

Added constraints to follow chapter 10 Refactored to fix a bug in chapter 9

Made some adjustments to follow chapter 11 guidelines

Repo now represents edits made to follow chapter 12 guidelines, inherited view and class created and CRUD modifications were made

estate_account linking module created, now follows chapter 13 guidelines

naja628

Hello -- good work :)

I left some comments

naja628 · 2025-04-30T14:51:45Z

estate/models/estate_property.py

+        "res.users", name="Salesperson", default=lambda self: self.env.user
+    )
+    tag_ids = fields.Many2many("estate.property.tag", string="Tags")
+    offers_ids = fields.One2many(


nitpick:

Suggested change

offers_ids = fields.One2many(

offer_ids = fields.One2many(

prefer thing_ids to things_ids for x2m fields

naja628 · 2025-04-30T14:54:07Z

estate/models/estate_property.py

+            if single_property.offers_ids:
+                single_property.best_price = max(
+                    single_property.offers_ids.mapped("price")
+                )
+            else:
+                single_property.best_price = 0


tip: max accepts a default kwarg for these kinds of cases

naja628 · 2025-04-30T14:54:42Z

estate/models/estate_property.py

+    def action_property_cancel(self):
+        for single_property in self:
+            if single_property.state == "sold":
+                raise UserError("Sold properties cannot be cancelled!")


Try to wrap user facing strings in a call to _ so they can be translated

naja628 · 2025-04-30T14:57:01Z

estate/models/estate_property.py

+            if not float_utils.float_is_zero(
+                single_property.selling_price, precision_rounding=0.1
+            ):
+                if single_property.selling_price < (
+                    0.9 * single_property.expected_price
+                ):
+                    raise ValidationError(
+                        "Selling price cannot be lower than 90%% of Expected price"
+                    )


nitpick: It's ok to prefer keeping your lines short but here it hurts readability a bit I think

naja628 · 2025-04-30T14:59:22Z

estate/models/estate_property_offer.py

+                self.property_id.state = "offer-accepted"
+                self.property_id.selling_price = self.price
+                self.property_id.buyer_id = self.partner_id


Here you could use a call to write to avoid repeating self.property_id and leave the orm a chance to make better optimizations

naja628 · 2025-04-30T15:10:22Z

estate/models/estate_property_offer.py

+    def action_offer_accept(self):
+        for offer in self:
+            offer.status = "accepted"
+            offer._action_check_offers()


why split the logic in a separate function here?

naja628 · 2025-04-30T15:13:25Z

estate/models/estate_property_offer.py

+
+    @api.depends("status")
+    def _action_check_offers(self):
+        if self.status == "accepted":


It looks like you're using this if more or less to check whether we came from action_offer_accept -- this is a bit weird

naja628 · 2025-04-30T15:15:20Z

estate/models/estate_property_offer.py

+    def create(self, vals):
+        for val in vals:


Suggested change

def create(self, vals):

for val in vals:

def create(self, vals_list):

for vals in vals_list:

naja628 · 2025-04-30T15:15:58Z

estate/models/estate_property_offer.py

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


here price may not always be in the vals. You could use .get

naja628 · 2025-04-30T15:26:10Z

estate/views/res_users_views.xml

+              </xpath>
+        </field>
+    </record>
+</odoo>


missing EOL character (editor config)

Guidelines follow chapter 14

SergeBayet suggested changes Apr 22, 2025

View reviewed changes

[ADD] estate: adding new estate module

0b23021

Add manifest and dependencies...

amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from fd89ef7 to 0b23021 Compare April 22, 2025 13:28

SergeBayet suggested changes Apr 22, 2025

View reviewed changes

estate/models/estate_property.py Outdated Show resolved Hide resolved

amry-odoo marked this pull request as ready for review April 23, 2025 11:26

amry-odoo requested a review from SergeBayet April 23, 2025 13:33

SergeBayet approved these changes Apr 23, 2025

View reviewed changes

estate/views/estate_property_views.xml Outdated Show resolved Hide resolved

SergeBayet approved these changes Apr 24, 2025

View reviewed changes

naja628 reviewed Apr 25, 2025

View reviewed changes

amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from d06d33d to 313ec7d Compare April 25, 2025 10:29

amry-odoo added 5 commits April 25, 2025 12:30

[ADD] estate: Added first estate module

d4fd942

Added a functional module to create new listings following all the chapter 5 guidelines.

[IMP] estate: Added different views

ff2ed32

Chapter 6 views: search, list, form done

[IMP] estate: Property Types, tags, offers added

74d2b8e

Repo now is up to date with chapter 7

[IMP] estate: Added computed fields

43a27fe

Added an onchange field and some compute fields, following Chapter 8 rules. Also refactored different views to separate them

[IMP] estate: Added actions, some refactoring

3a9a5a2

Added actions as mentioned in chapter 9 did some refactoring to follow latest comments

amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 313ec7d to 3a9a5a2 Compare April 25, 2025 10:30

amry-odoo added 2 commits April 28, 2025 11:24

[IMP] estate: Added constraints

2390818

Added constraints to follow chapter 10 Refactored to fix a bug in chapter 9

[ADD] estate: added views, action buttons and widgets

6316118

Made some adjustments to follow chapter 11 guidelines

amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 3108bd3 to 6316118 Compare April 30, 2025 08:13

[ADD] estate: Added an inherited module

6f9cc29

Repo now represents edits made to follow chapter 12 guidelines, inherited view and class created and CRUD modifications were made

amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 1688c2c to 6f9cc29 Compare April 30, 2025 09:58

[ADD] estate: Created Link module between account and estate

3c56016

estate_account linking module created, now follows chapter 13 guidelines

amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 99c4242 to 3c56016 Compare April 30, 2025 14:49

naja628 reviewed Apr 30, 2025

View reviewed changes

[IMP] estate: Added Kanban View

203b4b9

Guidelines follow chapter 14

amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 0a1d15b to 203b4b9 Compare April 30, 2025 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADD] estate: adding new estate module #727

[ADD] estate: adding new estate module #727

amry-odoo commented Apr 22, 2025

robodoo commented Apr 22, 2025

SergeBayet left a comment

SergeBayet Apr 22, 2025

SergeBayet Apr 22, 2025

SergeBayet Apr 22, 2025

SergeBayet left a comment

SergeBayet left a comment

SergeBayet left a comment

SergeBayet commented Apr 24, 2025

naja628 left a comment

naja628 Apr 25, 2025

naja628 left a comment

naja628 Apr 30, 2025

naja628 Apr 30, 2025

naja628 Apr 30, 2025

naja628 Apr 30, 2025

naja628 Apr 30, 2025

naja628 Apr 30, 2025

naja628 Apr 30, 2025

naja628 Apr 30, 2025

naja628 Apr 30, 2025

naja628 Apr 30, 2025

	selection=[('north','North'), ('south','South'), ('east', 'East'), ('west', 'West')]
	selection=[('north', 'North'), ('south', 'South'), ('east', 'East'), ('west', 'West')]

[ADD] estate: adding new estate module #727

Are you sure you want to change the base?

[ADD] estate: adding new estate module #727

Conversation

amry-odoo commented Apr 22, 2025

robodoo commented Apr 22, 2025

SergeBayet 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

SergeBayet left a comment

Choose a reason for hiding this comment

SergeBayet left a comment

Choose a reason for hiding this comment

SergeBayet left a comment

Choose a reason for hiding this comment

SergeBayet commented Apr 24, 2025

naja628 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naja628 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