[ADD] estate: defines a new app for estate properties #792

A-Yehia19 · 2025-05-20T09:57:52Z

No description provided.

robodoo · 2025-05-20T09:57:54Z

barracudapps

Nice!
Some small improvements though you can apply.
🚀

barracudapps · 2025-05-21T08:15:48Z

estate/__manifest__.py

+    'license': 'LGPL-3',
+    'application': True,
+    'installable': True,
+    'depends': ['base', 'web'],


Suggested change

'depends': ['base', 'web'],

'depends': ['base'],

Your module does not require anything coming from the web module so far

barracudapps · 2025-05-21T08:20:14Z

estate/views/estate_property_views.xml

+        <field name="arch" type="xml">
+            <list string="property list">
+                <field name="name" />
+                <field name="date_availability" width="400" />


Suggested change

<field name="date_availability" width="400" />

<field name="date_availability"/>

Avoid hardcoding styles when it's possible as per screen sizes compatibility.
Use classes instead and check for existing classes in other views and/or stylesheets. As Odoo uses Bootstrap, you can also make responsive UI based on BS styles

barracudapps · 2025-05-21T08:21:48Z

estate/views/estate_property_views.xml

+        <field name="res_model">estate.property</field>
+        <field name="view_mode">list,form</field>
+    </record>
+</odoo>


Suggested change

</odoo>

</odoo>

As per UNIX compatibility concern, please always add a newline at the end of your files.
Other Python conventions we follow at Odoo can be found on PEP8

…ome new attributes added the tags and property types, buyer, amd seller models and linked them in the property model. also make view of offers list

barracudapps · 2025-05-21T12:24:50Z

@A-Yehia19
Can you please review the name of your PR and your commits titles/messages ?
Everything you need to know is here

barracudapps · 2025-05-21T12:28:56Z

estate/models/estate_property_offer.py

+    _description = 'Estate Model Offer'
+
+    price = fields.Float()
+    status = fields.Selection(copy=False, selection=[('Accepted', 'Accepted'), ('Refused', 'Refused')])


Suggested change

status = fields.Selection(copy=False, selection=[('Accepted', 'Accepted'), ('Refused', 'Refused')])

status = fields.Selection(copy=False, selection=[('accepted', 'Accepted'), ('refused', 'Refused')])

Prefer using snake_case keys.
We basically try to fit PEP8 standards

barracudapps · 2025-05-21T12:31:45Z

estate/views/estate_property_views.xml

+            <form string="property form">
+                <sheet>
+                    <group>
+                        <field name="name" style="font-size:50px;" />


Suggested change

<field name="name" style="font-size:50px;" />

<field name="name" />

Avoid hardcoding styles. Prefer using Bootstrap or check for an existing class in CSS files that does what you want to apply (or add the style you need)

add the `total_area` and the `best_price` as computed attributes in the property modules and finish thier configuration added `validity` and `date_deadline` in the offer model as inverse computed attributes and configure them [FIX] styling issues replaced harcooded font style with bootstrap classes, and make the code follow sake_case naming

…ractions ad constrains in the `selling_price, expected_price` attributes in the property model to be positive and the `name, property_type_id` to be unique together added consrains in the `price` attribute in the offer model to also be positive #[FIX] some styling issues removed unused imports

barracudapps · 2025-05-22T13:00:08Z

estate/models/estate_property.py

+    def _check_date_end(self):
+        for record in self:
+            if record.state == 'offer_accepted' and float_compare(record.selling_price, 0.9 * record.expected_price, 2):
+                raise exceptions.UserError(message='the seeling price must be atleast 90% of the expected price.')


Suggested change

raise exceptions.UserError(message='the seeling price must be atleast 90% of the expected price.')

raise UserError(self.env._('the selling price must be at least 90% of the expected price.')

As Séna would say:

In Odoo, translatable strings in "data"-type content are automatically exported for translation. However, other user-facing strings, Odoo cannot automatically know whether they are translatable. To ensure these strings are exported for translation, you need to explicitly mark them. This is achieved by wrapping the literal string with the self.env._() function. The _() function signals to Odoo's translation system that the string should be included in the translation catalog.

More info here: https://www.odoo.com/documentation/18.0/developer/howtos/translations.html#explicit-exports

You can check this line here:
https://github.com/odoo/odoo/blob/05cff3b7d866f6bc95c4b32f343ae14a4da946f2/addons/account/models/account_move.py#L2075

barracudapps · 2025-05-22T13:02:38Z

estate/models/estate_property.py

+from odoo.tools.float_utils import float_compare



Suggested change

from odoo.tools.float_utils import float_compare

from odoo.tools.float_utils import float_compare

from odoo.exceptions import UserError

Import exceptions as they may be useful for future changes on your file

add colors to records according to its state, and hide some buttons if according to the step the property is in. added default filter to show available properties

barracudapps · 2025-05-28T14:48:20Z

Hi mate!
Can you please explain why you made this PR in the description ?

@A-Yehia19 Can you please review the name of your PR and your commits titles/messages ? Everything you need to know is here

barracudapps · 2025-05-28T14:51:06Z

As you can see on the runbot, you have some changes requested:
• https://runbot.odoo.com/runbot/build/81254345 (issue in your XML)
• https://runbot.odoo.com/runbot/build/81254346 (commit titles not following our conventions)

barracudapps

Nice! A lot of traps you handled correctly!
Some nitpicking comments though

barracudapps · 2025-05-28T14:56:54Z

estate/models/estate_property.py

+            best = 0
+            for offer in record.offer_ids:
+                best = max(best, offer.price)
+            record.best_price = best


You can try to optimize this by mapping results with something like record.offer_ids.mapped('price')

barracudapps · 2025-05-28T14:58:23Z

estate/models/estate_property_offer.py

+            if record.validity:
+                record.date_deadline = fields.Date.today() + timedelta(days=record.validity)
+            else:
+                record.date_deadline = fields.Date.today()


Suggested change

if record.validity:

record.date_deadline = fields.Date.today() + timedelta(days=record.validity)

else:

record.date_deadline = fields.Date.today()

record.date_deadline = fields.Date.today()

if record.validity:

record.date_deadline += timedelta(days=record.validity)

Maybe easier to understand

barracudapps · 2025-05-28T15:00:45Z

estate/models/estate_property_offer.py

+    @api.model
+    def create(self, vals):
+        if vals['price'] <= self.env['estate.property'].browse(vals['property_id']).best_price:
+            raise UserError(self.env._('The offer price must be greater than the best offer.'))
+
+        if self.env['estate.property'].browse(vals['property_id']).state == 'new':
+            self.env['estate.property'].browse(vals['property_id']).state = 'offer_received'
+        return super(TestModel, self).create(vals)


Are you sure this works properly ?
model_create_multi should be used here as per
https://github.com/odoo/odoo/blob/15fd3f62769b137cb9a0625f47b333424b851a27/odoo/api.py#L491-L509

barracudapps · 2025-05-28T15:04:41Z

estate/views/estate_property_views.xml

+                decoration-muted="state == 'sold'">
+                <field name="name" />
+                <field name="property_type_id" />
+                <field name="date_availability" width="200vw" optional="true" />


Suggested change

<field name="date_availability" width="200vw" optional="true" />

<field name="date_availability" optional="true" />

Avoid hardcoding styles. Usually, hardcoding styles is for reports to be printed and event the reports mainly work using classes.
You can use all existing classes + BootStrap ones

barracudapps · 2025-05-28T15:06:16Z

estate_account/__manifest__.py

+    'license': 'LGPL-3',
+    'application': True,
+    'installable': True,
+    'depends': ['base', 'estate', 'account'],


Suggested change

'depends': ['base', 'estate', 'account'],

'depends': ['estate', 'account'],

This is redundant as your estate module already relies on base module

[ADD] add estate app with empty view

189fd21

[ADD] added views of adding, listing properties and search filters

8bb1c21

barracudapps suggested changes May 21, 2025

View reviewed changes

[ADD] added 'offers' and 'other info' tabs in the property form and s…

5e798d2

…ome new attributes added the tags and property types, buyer, amd seller models and linked them in the property model. also make view of offers list

barracudapps reviewed May 21, 2025

View reviewed changes

A-Yehia19 changed the title ~~[ADD] add estate app with empty view~~ [ADD] computed modules May 21, 2025

barracudapps reviewed May 22, 2025

View reviewed changes

A-Yehia19 changed the title ~~[ADD] computed modules~~ [ADD] estate: defines a new app for estate properties May 22, 2025

A-Yehia19 added 4 commits May 22, 2025 17:07

[ADD] interactive UI effects to the app

dc6f9ec

add colors to records according to its state, and hide some buttons if according to the step the property is in. added default filter to show available properties

[ADD] action button between property typers and offer

d19a32a

[ADD] chapter 12 & 13 done

e6be488

[ADD] kanban view chapter 14

9599785

barracudapps suggested changes May 28, 2025

View reviewed changes

	<field name="date_availability" width="400" />
	<field name="date_availability"/>

	status = fields.Selection(copy=False, selection=[('Accepted', 'Accepted'), ('Refused', 'Refused')])
	status = fields.Selection(copy=False, selection=[('accepted', 'Accepted'), ('refused', 'Refused')])

	<field name="name" style="font-size:50px;" />
	<field name="name" />

	raise exceptions.UserError(message='the seeling price must be atleast 90% of the expected price.')
	raise UserError(self.env._('the selling price must be at least 90% of the expected price.')

	<field name="date_availability" width="200vw" optional="true" />
	<field name="date_availability" optional="true" />

	'depends': ['base', 'estate', 'account'],
	'depends': ['estate', 'account'],

[ADD] estate: defines a new app for estate properties #792

Are you sure you want to change the base?

[ADD] estate: defines a new app for estate properties #792

Uh oh!

Conversation

A-Yehia19 commented May 20, 2025

Uh oh!

robodoo commented May 20, 2025

Uh oh!

barracudapps 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!

barracudapps commented May 21, 2025

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!

barracudapps commented May 28, 2025

Uh oh!

barracudapps commented May 28, 2025

Uh oh!

barracudapps 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!

Uh oh!