[ADD] estate: new module to manage estate #822

taki-odoo · 2025-06-18T13:18:16Z

No description provided.

robodoo · 2025-06-18T13:18:20Z

aboo-odoo

First review for you! Don't hesitate to ask if you have question, our goal is teach you as much as possible before joining a team

aboo-odoo · 2025-06-24T07:47:19Z

estate/__init__.py

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


Always add an EOF character so that the next modification of the file made by someone else would not include this line

This comment applies several times in other files

aboo-odoo · 2025-06-24T07:48:01Z

estate/__manifest__.py

@@ -0,0 +1,14 @@
+{
+    'name': 'Estate',


In python, for quoting strings we try to follow the following convention:

single quote around technical terms
double quotes around display terms
=> ('north', "North")

Let's try to follow this during this tutorial, even though you'll find many counter examples in the existing code.

aboo-odoo · 2025-06-24T07:48:51Z

estate/models/estate_property.py

+    name = fields.Char(string="Title", required=True)
+    description = fields.Text()
+    postcode = fields.Char()
+    date_availability = fields.Date(copy=False, default=lambda self: date.today() + timedelta(days=90))


90 days is not quite 3 months 😄

aboo-odoo · 2025-06-24T07:49:55Z

estate/models/estate_property.py

+    property_type = fields.Many2one("estate.property.type", string="Property Type")
+    property_salesman = fields.Many2one("res.users", string="Salesman", default=lambda self: self.env.user)
+    property_buyer = fields.Many2one("res.partner", string="Buyer");
+    property_tags = fields.Many2many("estate.property.tag", string="Tags")
+    property_offers = fields.One2many("estate.property.offer", "property_id", string="Offers")


For relational fields, the convention is to suffix them by _id _ids to easily recognize them. And there is no need to prefix them by estate_

aboo-odoo · 2025-06-24T07:50:14Z

estate/models/estate_property.py

+from datetime import date, timedelta
+
+class EstateProperty(models.Model):
+    _name = "estate.property"


Suggested change

_name = "estate.property"

_name = 'estate.property'

aboo-odoo · 2025-06-24T08:01:17Z

estate/views/estate_menus.xml

+    <menuitem id="Estate_menu_root" name="Estate"/>
+
+    <menuitem id="Estate_menu_Advertissements" name="Advertissement" parent="Estate_menu_root"/>
+    <menuitem id="Estate_menu_Properties" name="Property" parent="Estate_menu_Advertissements" action="estate_property_view"/>
+    <menuitem id="Estate_menu_Properties_offer" name="Offers" parent="Estate_menu_Advertissements" action="estate_property_offer_view"/>
+
+    <menuitem id="Estate_menu_settings" name="Settings" parent="Estate_menu_root" sequence="20"/>
+    <menuitem id="Estate_menu_Properties_type" name="Property Type" parent="Estate_menu_settings" action="estate_property_type_view"/>
+    <menuitem id="Estate_menu_Properties_tags" name="Property Tags" parent="Estate_menu_settings" action="estate_property_tag_view"/>


What you did works, but placing them in the file means you can do this

Suggested change

<menuitem id="Estate_menu_root" name="Estate"/>

<menuitem id="Estate_menu_Advertissements" name="Advertissement" parent="Estate_menu_root"/>

<menuitem id="Estate_menu_Properties" name="Property" parent="Estate_menu_Advertissements" action="estate_property_view"/>

<menuitem id="Estate_menu_Properties_offer" name="Offers" parent="Estate_menu_Advertissements" action="estate_property_offer_view"/>

<menuitem id="Estate_menu_settings" name="Settings" parent="Estate_menu_root" sequence="20"/>

<menuitem id="Estate_menu_Properties_type" name="Property Type" parent="Estate_menu_settings" action="estate_property_type_view"/>

<menuitem id="Estate_menu_Properties_tags" name="Property Tags" parent="Estate_menu_settings" action="estate_property_tag_view"/>

<menuitem id="estate_menu_root" name="Estate"/>

<menuitem id="estate_menu_Advertissements" name="Advertissement"/>

<menuitem id="estate_menu_Properties" name="Property" action="estate_property_view"/>

<menuitem id="estate_menu_Properties_offer" name="Offers" action="estate_property_offer_view"/>

</menuitem>

<menuitem id="estate_menu_settings" name="Settings"/>

<menuitem id="estate_menu_Properties_type" name="Property Type" action="estate_property_type_view"/>

<menuitem id="estate_menu_Properties_tags" name="Property Tags" action="estate_property_tag_view"/>

</menuitem>

</menuitem>

which is more readable and easier to manage.

Also, no upper case for ids in general.

aboo-odoo · 2025-06-24T08:02:07Z

estate/views/estate_property_views.xml

+                <field name="facades"/>
+                <separator/>
+                <filter string="available" name="available" domain="[('available', '=', False)]"/>
+                <group expand="1" string="Group By">


What does expand 1 do in this case ? And why did you add the filter in a group ?

aboo-odoo · 2025-06-24T08:24:35Z

estate/views/estate_property_tag_views.xml

+            </form>
+        </field>
+    </record>
+    <record id="estate_property_tag_view" model="ir.actions.act_window">


Convention for action id is <model_name>_action

aboo-odoo · 2025-06-24T08:25:01Z

estate/views/estate_property_offer_views.xml

+            </form>
+        </field>
+    </record>
+        <record id="estate_property_offer_view_tree" model="ir.ui.view">


Suggested change

<record id="estate_property_offer_view_tree" model="ir.ui.view">

<record id="estate_property_offer_view_list" model="ir.ui.view">

tree is the old naming for list

aboo-odoo · 2025-06-24T08:27:20Z

estate/views/estate_property_views.xml

+            <form string="Estate Property">
+            </form>


Empty Form ?

aboo-odoo · 2025-06-25T07:44:17Z

Hey, could you do the fix and push your latest change at some point today so that I can review your extra work ? Thanks

aboo-odoo · 2025-06-26T09:00:21Z

git add <files> - to add changes to your future commit
git commit - to commit the change
git fetch origin 18.0 - to fetch the latest change on the main branch
git rebase 18.0 -i - to get the lastest change on your branch (may trigger conflicts) and squash all your commits into one
git push --force-with-lease dev - to push on your branch

Utils:

git stash - to store current change in a stack
git stash pop - to add back the earliest change in the stack

git status - to help you overall

aboo-odoo

Hey! Final review on the python part, it focuses on functionality, not style anymore 😄

By Monday, try to get your runbot green 🟢 and fix the comments from last time and this review please 👍

aboo-odoo · 2025-06-27T14:30:20Z

estate/models/estate_property.py

+            if record.offer_ids:
+                record.best_offer = max(record.offer_ids.mapped("price"))
+            else:
+                record.best_offer = 0.0


This can be simplified into

Suggested change

if record.offer_ids:

record.best_offer = max(record.offer_ids.mapped("price"))

else:

record.best_offer = 0.0

record.best_offer = max(record.offer_ids.mapped("price"), default=0.0)

aboo-odoo · 2025-06-27T14:31:43Z

estate/models/estate_property.py

+            if record.state != 'sold':
+                record.state = 'cancelled'
+            else:
+                raise UserError("You cannot cancel a sold property.")


Most strings are automatically made translatable: field names, module and model names, etc. But this is not the case for error messages. To make them translatable, use the _() method like so

Suggested change

raise UserError("You cannot cancel a sold property.")

raise UserError(_("You cannot cancel a sold property."))

aboo-odoo · 2025-06-27T14:32:45Z

estate/models/estate_property.py

+            if float_compare(record.selling_price, min_acceptable_price, precision_digits=2) < 0:
+                raise ValidationError("The selling price cannot be lower than 90% of the expected price.")
+
+    def unlink(self):


This is called upon the module deletion, to avoid triggering the error when the module is deleted by a user, add the api.ondelete(at_uninstall=False)

aboo-odoo · 2025-06-27T14:37:09Z

estate/models/estate_property_offer.py

+    @api.depends("validity")
+    def _compute_deadline(self):
+        for record in self:
+            record.deadline = date.today() + timedelta(days=record.validity)


Currently, your method does this:

Each time validity is updated, the deadline will be today + validity.

However, we'd prefer to have:

Each time validity is updated, the deadline is the day we created the announce + validity.

Meaning that currently, if I create an estate on the 7th of March with a validity of 7, the deadline is the 14th of March. On the 10th of March, I want to extend the validity to 10 days. I would expect to have the deadline on the 17th but I'll actually have the 20th.

aboo-odoo · 2025-06-27T14:37:50Z

estate/models/estate_property_offer.py

+    def _inverse_deadline(self):
+        for record in self:
+            if record.deadline:
+                record.validity = (record.deadline - date.today()).days


This will also need adapting with regards to the comment above

aboo-odoo · 2025-06-27T14:40:58Z

estate/models/estate_property_type.py

+    @api.depends("property_id.offer_ids")
+    def _compute_offer_count(self):
+        for record in self:
+            record.offer_counts = sum(len(prop.offer_ids) for prop in record.property_id)


What you did is correct, but relational fields allow for some simplifaction like

Suggested change

record.offer_counts = sum(len(prop.offer_ids) for prop in record.property_id)

record.offer_counts = len(record.property_id.offer_ids)

aboo-odoo · 2025-06-27T14:41:38Z

estate/views/estate_property_type_views.xml

+                    <header>
+						<button type="action"


This a lot of indent 👀

Suggested change

<header>

<button type="action"

<header>

<button type="action"

aboo-odoo · 2025-06-27T14:42:36Z

estate_account/models/estate_property.py

+    _inherit = 'estate.property'
+
+    def sold_property_button(self):
+        res = super().sold_property_button()


Your sold_property_button in estate does not return anything. Thus, this one should not either

Suggested change

res = super().sold_property_button()

super().sold_property_button()

aboo-odoo · 2025-06-27T14:42:44Z

estate_account/models/estate_property.py

+                ],
+            })
+
+        return res


Suggested change

return res

aboo-odoo · 2025-06-27T14:44:19Z

estate/models/estate_property_offer.py

+    def create(self, vals_list):
+        for vals in vals_list:
+            property_id = vals.get('property_id')
+            price = vals.get('price')


.get defaults to False when not found which will trigger an error later when you'll compare a float to a bool

Suggested change

price = vals.get('price')

price = vals.get('price', default=0.0)

aboo-odoo · 2025-07-01T12:02:10Z

Now that your onboarding is done, could you close the PR ? 😄

aboo-odoo reviewed Jun 24, 2025

View reviewed changes

taki-odoo force-pushed the 18.0-tutorials-taki branch 2 times, most recently from be996c2 to ba0ce5c Compare June 26, 2025 08:59

[ADD] estate: new module to manage estate

6626196

taki-odoo force-pushed the 18.0-tutorials-taki branch from ba0ce5c to 6626196 Compare June 26, 2025 15:26

[ADD] estate: new module to manage estate

2622484

aboo-odoo reviewed Jun 27, 2025

View reviewed changes

taki-odoo added 2 commits June 30, 2025 11:54

[ADD] estate modules

0aa9bda

[ADD] estate modules

c1ce697

taki-odoo closed this Jul 1, 2025

-    <menuitem id="Estate_menu_root" name="Estate"/>
-    <menuitem id="Estate_menu_Advertissements" name="Advertissement" parent="Estate_menu_root"/>
-    <menuitem id="Estate_menu_Properties" name="Property" parent="Estate_menu_Advertissements" action="estate_property_view"/>
-    <menuitem id="Estate_menu_Properties_offer" name="Offers" parent="Estate_menu_Advertissements" action="estate_property_offer_view"/>
-    <menuitem id="Estate_menu_settings" name="Settings" parent="Estate_menu_root" sequence="20"/>
-    <menuitem id="Estate_menu_Properties_type" name="Property Type" parent="Estate_menu_settings" action="estate_property_type_view"/>
-    <menuitem id="Estate_menu_Properties_tags" name="Property Tags" parent="Estate_menu_settings" action="estate_property_tag_view"/>
+    <menuitem id="estate_menu_root" name="Estate"/>
+        <menuitem id="estate_menu_Advertissements" name="Advertissement"/>
+            <menuitem id="estate_menu_Properties" name="Property" action="estate_property_view"/>
+            <menuitem id="estate_menu_Properties_offer" name="Offers" action="estate_property_offer_view"/>
+        </menuitem>
+        <menuitem id="estate_menu_settings" name="Settings"/>
+            <menuitem id="estate_menu_Properties_type" name="Property Type" action="estate_property_type_view"/>
+            <menuitem id="estate_menu_Properties_tags" name="Property Tags" action="estate_property_tag_view"/>
+        </menuitem>
+    </menuitem>

	<record id="estate_property_offer_view_tree" model="ir.ui.view">
	<record id="estate_property_offer_view_list" model="ir.ui.view">

	raise UserError("You cannot cancel a sold property.")
	raise UserError(_("You cannot cancel a sold property."))

	record.offer_counts = sum(len(prop.offer_ids) for prop in record.property_id)
	record.offer_counts = len(record.property_id.offer_ids)

	res = super().sold_property_button()
	super().sold_property_button()

	price = vals.get('price')
	price = vals.get('price', default=0.0)

[ADD] estate: new module to manage estate #822

[ADD] estate: new module to manage estate #822

Uh oh!

Conversation

taki-odoo commented Jun 18, 2025

Uh oh!

robodoo commented Jun 18, 2025

Uh oh!

aboo-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

Uh oh!

aboo-odoo commented Jun 25, 2025

Uh oh!

aboo-odoo commented Jun 26, 2025

Uh oh!

aboo-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

Uh oh!

aboo-odoo commented Jul 1, 2025

Uh oh!

Uh oh!