trcaz - Tutorial PR #944
trcaz - Tutorial PR #944
Conversation
|
|
|
Update commit messages according to |
| <field name="arch" type="xml"> | ||
| <form string="Property form test string?"> | ||
| <sheet> | ||
| <div class="oe_title"> |
There was a problem hiding this comment.
Why is everything inside an oe_title div 😄
There was a problem hiding this comment.
Don't ask me, I did what I found in the code base example 😅
estate/models/estate_property.py
Outdated
| for record in self: | ||
| if record.state == "sold": | ||
| raise exceptions.UserError("A sold property cannot be cancelled.") | ||
| else: | ||
| record.state = "cancelled" | ||
| return True |
There was a problem hiding this comment.
I'm just writing this to have a discussion please comment your thoughts about it.
What this method mainly does is cancelling properties, so the happy path of the method should end with record.state = 'cancelled'. Hiding the happy path under else might make the method less readable (at least for longer methods than this one).
What do you think?
There was a problem hiding this comment.
Now that I got Ruff working properly, the code has changed, I applied the style warnings and this code has change, let me know what you think.
| @api.constrains("price") | ||
| def _check_price_2(self): | ||
| for record in self: | ||
| if float_compare(record.price, 0.90*record.property_id.expected_price, precision_digits=2) == -1: |
There was a problem hiding this comment.
| if float_compare(record.price, 0.90*record.property_id.expected_price, precision_digits=2) == -1: | |
| if float_compare(record.price, 0.9 * record.property_id.expected_price, precision_digits=2) == -1: |
Also what if record.property_id is None seems like you would get an error have you thought about it?
There was a problem hiding this comment.
(This constraint doesn't seem to work for me anyway 😅)
Can a property offer have no property id? Isn't it populated from the one2many automatically? When creating an offer from the property form?
There was a problem hiding this comment.
Yes as you said it should always be filled, so no problem here as long as property_id is a required field.
Let's check the constraint if it's still not working
| @@ -79,16 +65,22 @@ def _onchange_garden(self): | |||
| self.garden_area = None | |||
| self.garden_orientation = None | |||
|
|
|||
| @api.ondelete(at_uninstall=False) | |||
| def _prevent_deletion_unless_new_or_cancelled(self): | |||
There was a problem hiding this comment.
| def _prevent_deletion_unless_new_or_cancelled(self): | |
| def _unlink_prevent_deletion_unless_new_or_cancelled(self): |
couldn't find a rule for that in the guidelines but most ondelete methods start with unlink
| record.status = "accepted" | ||
| record.property_id.buyer_id = record.partner_id | ||
| record.property_id.state = 'offer_accepted' | ||
| record.property_id.selling_price = record.price | ||
|
|
||
| @api.model_create_multi | ||
| def create(self, vals_list): | ||
| for record in vals_list: |
There was a problem hiding this comment.
| for record in vals_list: | |
| for vals in vals_list: |
These are not records yet so the name can be confusing
| property = self.env['estate.property'].browse(record['property_id']) | ||
| property.state = 'offer_received' |
There was a problem hiding this comment.
Here you're executing browse in every iteration.
Instead you can browse for the ids before the loop and even do everything without the loop.
| class ResUsers(models.Model): | ||
| _inherit = 'res.users' | ||
|
|
||
| property_ids = fields.One2many("estate.property", "salesperson_id", string="Properties") |
There was a problem hiding this comment.
Properties might be the default string here. But double check
trcazier
force-pushed
the
19.0-tutorial-trcaz
branch
from
d65dba8 to
c8d8a50
Compare
…fix search group by & menu names
trcazier
force-pushed
the
19.0-tutorial-trcaz
branch
from
c8d8a50 to
4d3532f
Compare
No description provided.