[ADD] estate:Create first Estate module #731

adak-ahmed · 2025-04-22T13:58:51Z

Adding manifest and fields.

robodoo · 2025-04-22T13:58:55Z

SergeBayet

Some remarks. :)

SergeBayet · 2025-04-22T14:01:30Z

Estate/__manifest__.py

+    'application':True,
+
+
+}


Missing a blank line at the end of the file.

SergeBayet · 2025-04-22T14:01:39Z

Estate/models/__init__.py

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


SergeBayet · 2025-04-22T14:02:20Z

Estate/models/estate.py

+
+from odoo import fields, models
+class Estate(models.Model):
+    _name="estate"


blank spaces around equal signs.

SergeBayet

Some remarks 👍

SergeBayet · 2025-04-23T13:16:25Z

realestate/__manifest__.py

+
+

There are plenty of unnecessary blank lines in your files.

SergeBayet · 2025-04-23T13:16:37Z

realestate/models/realestate.py

+
+
+
+
+
+
+


Like here :)
Remove these lines here and elsewhere in your code.

SergeBayet · 2025-04-23T13:17:55Z

realestate/views/estate_view.xml

+    </menuitem>
+
+
+</odoo>


And here a blank line is missing at the end of file

SergeBayet

Some remarks! Keep going! :)

SergeBayet · 2025-04-24T09:25:23Z

realestate/models/realestate.py

+            self.garden_orientation = "north"
+        else:
+            self.garden_area = 0
+            self.garden_orientation = ""


Suggested change

self.garden_orientation = ""

self.garden_orientation = False

SergeBayet · 2025-04-24T09:27:31Z

realestate/models/realestate.py

+            record.total_area = record.living_area + record.garden_area
+
+    @api.depends("offer_ids")
+    def _max_offer_price(self):


the method should start with _compute_...

SergeBayet · 2025-04-24T09:27:51Z

realestate/models/realestate.py

+        for record in self:
+            record.total_area = record.living_area + record.garden_area
+
+    @api.depends("offer_ids")


Depends only on offer_ids, not offer_ids.price, which means the method won’t be triggered when an offer’s price changes, unless a new offer is added/removed.

SergeBayet · 2025-04-24T09:28:18Z

realestate/models/realestate.py

+
+    @api.depends("offer_ids")
+    def _max_offer_price(self):
+        self.best_price = max(self.offer_ids.mapped("price"), default=0)


This sets self.best_price assuming self is a single record, but compute methods must support recordsets.
You should loop through self.

SergeBayet · 2025-04-24T09:29:39Z

realestate/models/offer.py

@@ -8,3 +9,18 @@ class Offer(models.Model):
    status = fields.Selection([("accepted", "Accepted"), ("refused", "Refused")])
    buyer_id = fields.Many2one("buyer")
    property_id = fields.Many2one("realestate")
+    validity = fields.Integer()
+    deadline = fields.Date(compute="_compute_deadline", inverse="_inverse_deadline")


It's a good habit to add the suffix _date on a Date field.

SergeBayet · 2025-04-24T12:00:00Z

@naja628 And a last one for you.

SergeBayet · 2025-04-25T09:47:34Z

realestate/models/offer.py

+
+    def set_offer_accepted(self):
+        self.ensure_one()
+        record = self


SergeBayet · 2025-04-25T09:48:09Z

realestate/models/offer.py

+            if record.property_id.state == "sold":
+                raise UserError("This property is already sold")
+            record.status = "accepted"
+            record.property_id.selling_price = record.price


You can use 'mappedto avoid repeatingrecord.property_id`

naja628

Hello :)

[Sorry I see Serge also did a 2nd pass on your code too -- I guess you will have 2 2nd reviews]

I left some comments on your code

I see you haven't reacted to Serge's previous suggestions yet -- could you do that too please?

naja628 · 2025-04-25T09:44:27Z

realestate/models/__init__.py

+from . import realestate
+from . import owner
+from . import types
+from . import tags
+from . import seller
+from . import buyer
+from . import offer


nitpick: by convention, we prefix model names with the name of the module

Suggested change

from . import realestate

from . import owner

from . import types

from . import tags

from . import seller

from . import buyer

from . import offer

from . import realestate_property

from . import realesate_owner

....

naja628 · 2025-04-25T09:45:17Z

realestate/__manifest__.py

+        "security/ir.model.access.csv",
+        "views/estate_view.xml",
+        "views/owner_view.xml",
+        "views/type_view.xml",
+        "views/tag_view.xml",
+        "views/seller_view.xml",
+        "views/buyer_view.xml",
+        "views/offer_view.xml",


similarly the files could be named realestate_owner_view.xml ...

naja628 · 2025-04-25T09:45:45Z

realestate/models/buyer.py

+
+
+class Buyer(models.Model):
+    _name = "buyer"


Suggested change

_name = "buyer"

_name = "realestate_buyer"

(etc...)

naja628 · 2025-04-25T11:40:27Z

realestate/models/realestate.py

+    status = fields.Selection(related="offer_ids.status")
+    price = fields.Float(related="offer_ids.price")
+    validity = fields.Integer(related="offer_ids.validity")
+    deadline = fields.Date(related="offer_ids.deadline")


You cannot created related fields that pass through an x2many (you can look at the warning in the Doc)

What are you trying to accomplish here? What do you think the value should be?

naja628 · 2025-04-25T11:42:34Z

realestate/views/buyer_view.xml

+    </menuitem>
+
+
+</odoo>


Some of your files are missing the eol character at the end of the last line (hence the red signs in the github ui) -- could you configure your editor so it doesn't do that please?

naja628 · 2025-04-25T11:44:36Z

realestate/models/realestate.py

+    _sql_constraints = [
+        (
+            "check_expected_price",
+            "CHECK(expected_price> 0)",


nitpick:

Suggested change

"CHECK(expected_price> 0)",

"CHECK(expected_price > 0)",

(more places)

naja628 · 2025-04-25T11:49:11Z

realestate/models/realestate.py

+        for record in self:
+            if record.selling_price < 0.9 * record.expected_price:
+                raise ValidationError(
+                    "Selling price must be at least 90'%' of the expected price"


why the quotes around the %

try wrap user facing text in a call to _ so it can be translated

naja628 · 2025-04-25T12:38:47Z

realestate/views/estate_view.xml

+
+

nitpick:

Suggested change

I like the idea of separating things but 2 blank lines is a bit overkill ^^

naja628 · 2025-04-25T13:26:06Z

realestate/views/estate_view.xml

+            <search>
+                <field name="postcode" />
+                <field name='name' />
+                <filter name='is_garden' string='With Garden' domain="[('garden','=', 'True')]" />


nitpick: do you think is_garden is a good name for what this represents?

SergeBayet · 2025-04-30T09:18:13Z

@adak-ahmed You can click on the button "Ready for review" and try to make runbot happy. :) The commit titles don't follow the guidelines.

Chapter 3

2ec5183

SergeBayet suggested changes Apr 22, 2025

View reviewed changes

[ADD] Realestate: Adding list,form,and search views

ee36135

SergeBayet suggested changes Apr 23, 2025

View reviewed changes

adak-ahmed added 2 commits April 23, 2025 17:33

[ADD] estate: Considering relation between models

6fa1a8a

[ADD] estate: Computed Fields and Onchanges

5bc7a96

SergeBayet suggested changes Apr 24, 2025

View reviewed changes

[ADD]: estate Adding constraints and actions

8b38906

SergeBayet suggested changes Apr 25, 2025

View reviewed changes

naja628 reviewed Apr 25, 2025

View reviewed changes

adak-ahmed added 3 commits April 29, 2025 16:33

[ADD]: estate Adding stat buttons and options to views

4489954

[IMP]: estate Inherting CRUD operations

7140196

[ADD] Realestate: Invoice Creation

e09e56c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADD] estate:Create first Estate module #731

[ADD] estate:Create first Estate module #731

adak-ahmed 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 Apr 23, 2025

SergeBayet Apr 23, 2025

SergeBayet Apr 23, 2025

SergeBayet left a comment

SergeBayet Apr 24, 2025

SergeBayet Apr 24, 2025

SergeBayet Apr 24, 2025

SergeBayet Apr 24, 2025

SergeBayet Apr 24, 2025

SergeBayet commented Apr 24, 2025

SergeBayet Apr 25, 2025

SergeBayet Apr 25, 2025

naja628 left a comment •

edited

Loading

naja628 Apr 25, 2025

naja628 Apr 25, 2025

naja628 Apr 25, 2025

naja628 Apr 25, 2025

naja628 Apr 25, 2025

naja628 Apr 25, 2025

naja628 Apr 25, 2025

naja628 Apr 25, 2025

naja628 Apr 25, 2025

SergeBayet commented Apr 30, 2025

		</menuitem>


		</odoo>

		</menuitem>


		</odoo>

[ADD] estate:Create first Estate module #731

Are you sure you want to change the base?

[ADD] estate:Create first Estate module #731

Conversation

adak-ahmed 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

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

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

SergeBayet commented Apr 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naja628 left a comment • edited Loading

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

SergeBayet commented Apr 30, 2025

naja628 left a comment •

edited

Loading