Skip to content

Conversation

@robal-odoo
Copy link

No description provided.

@robodoo
Copy link

robodoo commented Dec 16, 2025

Pull request status dashboard

@robal-odoo robal-odoo force-pushed the 19.0-estate-tutorial-robal branch from 924f90b to 0af92ad Compare December 16, 2025 12:59
@robal-odoo robal-odoo changed the title Estate tutorial (robal) [ADD] estate: add real estate module (framework 101 tutorial) Dec 16, 2025
@robal-odoo robal-odoo force-pushed the 19.0-estate-tutorial-robal branch from cab625c to d8993d0 Compare December 16, 2025 16:12
@robal-odoo robal-odoo marked this pull request as ready for review December 19, 2025 08:21
Copy link

@ushyme ushyme left a comment

Choose a reason for hiding this comment

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

Good work so far. I just have a few suggestions.

@robal-odoo robal-odoo force-pushed the 19.0-estate-tutorial-robal branch from 2fb7576 to 9dd3005 Compare December 19, 2025 12:44
Framework 101 tutorial
- add estate module
- add estate_account module for integration with invoicing
@robal-odoo robal-odoo force-pushed the 19.0-estate-tutorial-robal branch from 440b49e to 6b2e660 Compare December 19, 2025 14:12
Copy link

@ushyme ushyme left a comment

Choose a reason for hiding this comment

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

Looks good 👍🏾; Just a few remarks.

@api.model
def create(self, vals):
for val in vals:
estate_property = self.env["estate.property"].browse(val["property_id"])
Copy link

Choose a reason for hiding this comment

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

If vals contains offers for multiple properties, this will trigger multiple singleton browses.
Extract all property_ids from vals first, browse them in one go, and then iterate.

Comment on lines 50 to 60
def action_accept(self) -> bool:
# TODO double check validation
for record in self:
if record.property_id.offer_ids.filtered(lambda r: r.status == "accepted"):
raise UserError(record.env._("Cannot accept this offer because another offer has already been accepted for the property."))

record.status = "accepted"
record.property_id.selling_price = record.price
record.property_id.buyer_id = record.partner_id
record.property_id.state = "offer_accepted"
return True
Copy link

Choose a reason for hiding this comment

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

what if the property is already sold or cancelled ?

Comment on lines 7 to 21
def action_set_sold(self) -> bool:
percent_description = "6 percent of selling price"
admin_fee_description = "Administrative fees"
# Note: does not work if fiscal localization is not set
for record in self:
account_move = {
"partner_id": record.buyer_id.id,
"move_type": "out_invoice",
"invoice_line_ids": [
Command.create({"name": percent_description, "quantity": 1, "price_unit": 0.06 * record.selling_price}),
Command.create({"name": admin_fee_description, "quantity": 1, "price_unit": 100.0}),
],
}
self.env["account.move"].create(account_move)
return super().action_set_sold()
Copy link

Choose a reason for hiding this comment

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

Currently, you are calling self.env["account.move"].create(account_move) inside the loop. This triggers an INSERT and potentially multiple compute/inverse calls per record.

Also, what if the buyer_id is not set ?

- Stop user from accepting offers if the linked property is sold or cancelled
- Move the call to "create" outside the loop
- Extract the calls to "browse" outside the loop
- Stop properties from being marked as sold if there isn't an accepted offer
- Leverage the validation logic from the parent model
- That validation already ensures there is an accepted offer, which means that buyer_id is set
…accept, refuse)

- Cannot change the status of an offer if it's already set
- Cannot accept/refuse offers on a property that's cancelled or sold
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