-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[ADD] estate: add real estate module (framework 101 tutorial) #1077
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 19.0
Are you sure you want to change the base?
Conversation
924f90b to
0af92ad
Compare
cab625c to
d8993d0
Compare
ushyme
left a comment
There was a problem hiding this 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.
2fb7576 to
9dd3005
Compare
Framework 101 tutorial - add estate module - add estate_account module for integration with invoicing
440b49e to
6b2e660
Compare
ushyme
left a comment
There was a problem hiding this 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"]) |
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
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 ?
| 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() |
There was a problem hiding this comment.
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

No description provided.