Skip to content

Conversation

@roole-odoo
Copy link

[roole] chapter 2

Finished Chapter 2 for roole.

Changed the manifest to test the 'git commit' env
@robodoo
Copy link

robodoo commented Nov 18, 2025

Pull request status dashboard

Chapter 3 finished.
- the ir.model.access.csv have been created
- read, write, create and unlink permissions have been given to the group base.group_user.
Fixing manifest with license
- an action have been loaded in the system.
- Property view created.
- view Menu added.
- Extra attributes have been added.
Copy link
Contributor

@clbr-odoo clbr-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good job, just some nitpicks to report 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prevent from appending _model on every model. Since your model is estate.property it should be estate_property.py

- Search + filters added
- Enhanced form view
- Added ugly column in the view
- review remarks applied
- implemented the chapter 7
- Added need file to views
- Fixed xmls
Implemented the chapter 8
- correction of the estate_property style PEP8
- Chapter 9, button implementation
Copy link
Contributor

@clbr-odoo clbr-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good job, keep up !

Comment on lines +41 to +42
for offers in record.property_id.offer_ids:
offers.status = "refused"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can actually do this since you're setting all records of the recordset to the same value.
record.property_id.offer_ids.status = "refused"

And take care of the plural misused:

Suggested change
for offers in record.property_id.offer_ids:
offers.status = "refused"
for offer in record.property_id.offer_ids:
offer.status = "refused"

Comment on lines +44 to +46
record.property_id.state = "offer_accepted"
record.property_id.selling_price = record.price
record.property_id.buyer_id = record.partner_id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
record.property_id.state = "offer_accepted"
record.property_id.selling_price = record.price
record.property_id.buyer_id = record.partner_id
record.property_id.write({
"state": "offer_accepted",
"selling_price": record.price,
"buyer_id": record.partner_id,
})

You can also do this in one call. Tbh the ORM will most likely process all the write in one batch anyway but still, it's a good practice to do this.


@api.depends('validity')
def _compute_deadline(self):
for record in self:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for record in self:
for estate in self:

We more and more try to avoid generic "record" naming in those cases to make it more readable.
This way even when reading a snippet we know what kind of records we're looping on.

@api.depends("offer_ids.price")
def _compute_best_price(self):
for record in self:
if len(record.offer_ids) > 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if len(record.offer_ids) > 0:
if record.offer_ids:

an empty recordset will return a Falsy value when evaluated like this, no need to check the lenght.

def _compute_best_price(self):
for record in self:
if len(record.offer_ids) > 0:
record.best_price = max(record.offer_ids.mapped("price"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can avoid the if_else and do a one liner like this if you want 😉

Suggested change
record.best_price = max(record.offer_ids.mapped("price"))
record.best_price = max(record.offer_ids.mapped("price"), default=0.0)

raise UserError("Canceled property can't be sold.")
else:
record.state = "sold"
return True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That return should not be in the else clause but at the very end of the method.
But tbh you don't need the else at all.

<field name="living_area"/>
<field name="facades"/>
<separator/>
<filter string="Available" name="active" domain="['|', ('state', '=', 'new' ),('state', '=', 'offer received' )]"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<filter string="Available" name="active" domain="['|', ('state', '=', 'new' ),('state', '=', 'offer received' )]"/>
<filter string="Available" name="active" domain="[('state', 'in', ('new', 'offer received'))]"/>

<field name="facades"/>
<separator/>
<filter string="Available" name="active" domain="['|', ('state', '=', 'new' ),('state', '=', 'offer received' )]"/>
<filter string="Postco²de" name="postcode" context="{'group_by':'postcode'}"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo !

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid pushing your personal config 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants