JEDEL Onboarding #998

jedel-odoo · 2025-10-20T11:49:02Z

No description provided.

robodoo · 2025-10-20T11:49:05Z

Mathilde411

Small comments :)

estate/models/__init__.py

estate/security/ir.model.access.csv

estate/__init__.py

estate/__manifest__.py

Mathilde411

Good work here !
I see that your runbot is red because of style, so you may fix that.
Also be mindful about the naming of files/xmlids: https://www.odoo.com/documentation/19.0/contributing/development/coding_guidelines.html#xml-ids-and-naming

estate/models/estate_property.py

Mathilde411 · 2025-10-22T08:15:07Z

estate/models/estate_property.py

+    @api.constrains('expected_price', 'selling_price')
+    def _check_selling_price(self):
+        for record in self:
+            if not float_is_zero(record.selling_price, precision_digits=3):
+                if float_compare(record.selling_price, record.expected_price*0.9, precision_digits=3) < 0:
+                    raise UserError(r"The selling price must be at least 90% of the expected price !")


Be careful about model attribute order: https://www.odoo.com/documentation/19.0/contributing/development/coding_guidelines.html#symbols-and-conventions

I don't understand the change I have to make

estate/models/estate_property_offer.py

Mathilde411 · 2025-10-22T08:17:48Z

estate/security/ir.model.access.csv

+estate.access_estate_property,access_estate_property,model_estate_property,base.group_user,1,1,1,1
+estate.access_estate_property_type,access_estate_property_type,model_estate_property_type,base.group_user,1,1,1,1
+estate.access_estate_property_tag,access_estate_property_tag,model_estate_property_tag,base.group_user,1,1,1,1
+estate.access_estate_property_offer,access_estate_property_offer,model_estate_property_offer,base.group_user,1,1,1,1


Missing newline

Mathilde411 · 2025-10-22T08:20:07Z

estate/views/estate_property_views.xml

+                <field name="bedrooms"/>
+                <field name="living_area" string="Living Area (sqm)"/>
+                <field name="facades"/>
+                <filter string="Available" name="available" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>


Suggested change

<filter string="Available" name="available" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>

<filter string="Available" name="available" domain="[('state', 'in', ('new', 'offer_received'))]"/>

Simpler way when comparing the same variable :)

Mathilde411

Very good work !
Still a few things to fix left from yesterday's pass :)

Mathilde411 · 2025-10-23T09:44:36Z

estate/models/estate_property.py

+
+class EstateProperty(models.Model):
+    _name = "estate.property"
+    _description = "Property for the Real Estate app"


Suggested change

_description = "Property for the Real Estate app"

_description = "Estate Property"

_description contains a short human-readable name for the model

estate/models/estate_property.py

Mathilde411 · 2025-10-23T09:48:58Z

estate/models/estate_property.py

+    @api.constrains('expected_price', 'selling_price')
+    def _check_selling_price(self):
+        for record in self:
+            if not float_is_zero(record.selling_price, precision_digits=3):
+                if float_compare(record.selling_price, record.expected_price*0.9, precision_digits=3) < 0:
+                    raise UserError(r"The selling price must be at least 90% of the expected price !")


estate/models/estate_property_offer.py

Mathilde411 · 2025-10-23T09:50:38Z

estate/models/estate_property_offer.py

+            if len(offers) > 0:
+                if val['price'] < max(offers.mapped('price')):
+                    raise UserError("You cannot create an offer with a lower amount than an existing offer !")


Suggested change

if len(offers) > 0:

if val['price'] < max(offers.mapped('price')):

raise UserError("You cannot create an offer with a lower amount than an existing offer !")

if offers and val['price'] < max(offers.mapped('price')):

raise UserError("You cannot create an offer with a lower amount than an existing offer !")

Cleaner that way :)If you have an empty recordset, it will automatically be cast to False

Mathilde411

Very good overall.
Could you just squash so that you have 1 commit per chapter ?

Mathilde411 · 2025-10-24T08:58:22Z

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+    "files.insertFinalNewline": true
+}


Ironically, this is missing the newline :x

Mathilde411 · 2025-10-24T09:21:50Z

estate/models/estate_property.py

+    @api.ondelete(at_uninstall=False)
+    def _unlike_if_stats_new_or_cancelled(self):
+        for record in self:
+            if record.state in ('new', 'cancelled'):
+                raise UserError("You cannot delete a new or cancelled property !")


Suggested change

@api.ondelete(at_uninstall=False)

def _unlike_if_stats_new_or_cancelled(self):

for record in self:

if record.state in ('new', 'cancelled'):

raise UserError("You cannot delete a new or cancelled property !")

@api.ondelete(at_uninstall=False)

def _unlink_if_stats_new_or_cancelled(self):

for record in self:

if record.state in ('new', 'cancelled'):

raise UserError("You cannot delete a new or cancelled property !")

Spelling.
Also method ordering ! This is one of the CRUD methods, it should go higher :)

Mathilde411 · 2025-10-24T09:24:29Z

estate/models/estate_property_offer.py

+        for offer in self:
+            offer.status = "refused"


Suggested change

for offer in self:

offer.status = "refused"

self.status = "refused"

return True

Assigning attributes on recordsets works !

Mathilde411 · 2025-10-24T09:38:39Z

estate/models/estate_property_offer.py

+    @api.model_create_multi
+    def create(self, vals_list):
+        for val in vals_list:
+            if self.env['estate.property'].browse(val['property_id']).state == 'sold':
+                raise UserError("You cannot create an offer for a sold property !")
+            offers = self.env['estate.property.offer'].search([('property_id', '=', val['property_id'])])
+            if offers and val['price'] < max(offers.mapped('price')):
+                raise UserError("You cannot create an offer with a lower amount than an existing offer !")
+        offers = super().create(vals_list)
+        offers.property_id.state = 'offer_received'
+        return offers


Suggested change

@api.model_create_multi

def create(self, vals_list):

for val in vals_list:

if self.env['estate.property'].browse(val['property_id']).state == 'sold':

raise UserError("You cannot create an offer for a sold property !")

offers = self.env['estate.property.offer'].search([('property_id', '=', val['property_id'])])

if offers and val['price'] < max(offers.mapped('price')):

raise UserError("You cannot create an offer with a lower amount than an existing offer !")

offers = super().create(vals_list)

offers.property_id.state = 'offer_received'

return offers

@api.model_create_multi

def create(self, vals_list):

if self.env['estate.property'].search_count([('id', 'in', [val.get('property_id') for val in vals_list]), ('state', '=', 'sold')]): # If the number of corresponding object != 0...

raise UserError("You cannot create an offer for a sold property !")

for val in vals_list:

offers = self.env['estate.property.offer'].search([('property_id', '=', val['property_id'])])

if offers and val['price'] < max(offers.mapped('price')):

raise UserError("You cannot create an offer with a lower amount than an existing offer !")

offers = super().create(vals_list)

offers.property_id.state = 'offer_received'

return offers

Doing browse in a loop is a BIG nono. It means you will do 1 SQL request per element of the recordset, which is extremely bad for performance. Prefer doing checks like this ouside of the loop, with things like this
Also this is a CRUD method so it should go hisher in the ordering

Also you should use float compare here.
In reality you'd have a currency attached to your price, and you'd use that for comparison.

Mathilde411 · 2025-10-24T09:43:40Z

estate/tests/test_property.py

+        partner = self.env['res.partner'].create(
+            [{
+                'name': 'My Favorite Partner'
+            }]
+        )


Create this guy in the setUp. means the tests will run faster if you need a partner somewhere else in the tests

Mathilde411 · 2025-10-24T09:46:15Z

estate_account/models/estate_property.py

+                Command.create({
+                    'name': self.name,
+                    'quantity': 1,
+                    'price_unit': self.selling_price
+                }),
+                Command.create({
+                    'name': 'Taxes',
+                    'quantity': 1,
+                    'price_unit': self.selling_price * 0.06
+                }),
+                Command.create({
+                    'name': 'Administrative fees',
+                    'quantity': 1,
+                    'price_unit': 100.00
+                })


Just saw this but I think this is not what the exercise asks you to do.

[ADD] Estate:createthe app

f8e1bc0

Mathilde411 changed the title ~~[ADD] Estate:createthe app~~ JEDEL Onboarding Oct 20, 2025

Mathilde411 self-requested a review October 20, 2025 11:55

[IMP] estate : set application to True

c558c6b

Mathilde411 self-assigned this Oct 20, 2025

jedel-odoo added 2 commits October 20, 2025 15:14

[IMP] Estate : Chapter 3

cd52496

[IMP] estate : add security model

8cb8dc5

Mathilde411 requested changes Oct 20, 2025

View reviewed changes

jedel-odoo added 2 commits October 20, 2025 17:13

[IMP] estate : adding menus and fields

a0182ea

[IMP] estate : adding custom views (chapter 6)

a449045

Mathilde411 self-requested a review October 21, 2025 11:18

Mathilde411 removed their assignment Oct 21, 2025

jedel-odoo and others added 10 commits October 21, 2025 13:54

[IMP] estate : adding types, tags and offers (chapter 7)

ded2b66

[IMP] estate: Add notes.

286c828

[IMP] estate: adding computed fields and onchanges (chapter 8)

fa3f1ea

[IMP] estate: adding computed fields and onchanges (chapter8)

f9ed8a6

[IMP] estate: style corrections

2ff906f

[IMP] estate: tutorials corrections

67147af

[IMP] estate: tutorials corrections

a0a6f39

[IMP] estate: adding buttons (chapter 9

abc4cdf

[IMP] estate: corrections

1c19652

[IMP] estate: adding constraints (chapter 10)

c5e045e

Mathilde411 requested changes Oct 22, 2025

View reviewed changes

jedel-odoo added 6 commits October 22, 2025 13:45

[IMP] estate: adding sprinkels (chapter 11)

3ef6943

[IMP] estate: adding user inheritance (chapter 12)

6a76b10

[IMP] estate: style corrections

dd2110b

[ADD] estate_account: adding a child class

a4d520a

[IMP] estate, estate_account: style corrections

25c5923

[IMP] estate: kanban view

383a8d6

Mathilde411 requested changes Oct 23, 2025

View reviewed changes

jedel-odoo added 4 commits October 23, 2025 12:58

[IMP] estate: corrections

f19a562

[IMP] estate: corrections

1313f2b

[IMP] estate: adding some tests

47aa27d

[FIX] estate: corrections

92f8ce1

Mathilde411 requested changes Oct 24, 2025

View reviewed changes

	<filter string="Available" name="available" domain="['\|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>
	<filter string="Available" name="available" domain="[('state', 'in', ('new', 'offer_received'))]"/>

	_description = "Property for the Real Estate app"
	_description = "Estate Property"

@@ @@ -0,0 +1,3 @@ @@
+              {
+                  "files.insertFinalNewline": true
+              }

                
                    No newline at end of file

JEDEL Onboarding #998

Are you sure you want to change the base?

JEDEL Onboarding #998

Uh oh!

Conversation

jedel-odoo commented Oct 20, 2025

Uh oh!

robodoo commented Oct 20, 2025

Uh oh!

Mathilde411 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mathilde411 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mathilde411 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants