[ADD] Real estate: Initial setup for module - #1369
Conversation
|
|
YassinWalid
left a comment
YassinWalid
left a comment
There was a problem hiding this comment.
Thanks for the impressive work. I have done an initial review, and left some comments in the PR and other general ones below. Please check them out, especially the coding guidelines and how to format the name/id of python fields and methods, xml record names and ids, as well as the model names themselves
Instead of date and datetime, you should use fields.Date and fields.Datetime
For the naming convention of records' name/id, name of models, fields, the order of methods for a model, and much more, check the coding guidelines for useful tips/rules.
| date_availability = fields.Date( | ||
| "Availability date", | ||
| copy=False, | ||
| default=date.today() + relativedelta(months=3), |
There was a problem hiding this comment.
| default=date.today() + relativedelta(months=3), | |
| default=fields.Date.today() + relativedelta(months=3), |
you should use odoo's fields.Date.today() instead of date.today()
You could also check out fields.Date.add(), which might eliminate the need for relativedelta
| _order = "id desc" | ||
|
|
||
| active = fields.Boolean(default=True) | ||
| name = fields.Char("Title", required=True, translate=True) |
There was a problem hiding this comment.
| name = fields.Char("Title", required=True, translate=True) | |
| name = fields.Char("Title", required=True) |
This is not needed, as it is translated by default
| @api.depends("offer_ids") | ||
| def _compute_selling_price(self): | ||
| for record in self: | ||
| accepted_offer = record.offer_ids.filtered(lambda o: o.status == "accepted") | ||
| record.selling_price = accepted_offer.price if accepted_offer else 0.0 |
There was a problem hiding this comment.
This will probably work, but whenever you add a new offer for example, it will be called, which might be unnecessary if this offer will is be accepted. I believe doing this in the action for accepting the offer works best performance wise.
| @api.onchange("offer_ids") | ||
| def _onchange_offer(self): | ||
| for record in self: | ||
| if record.state == "new" and len(record.offer_ids) > 0: | ||
| record.state = "offer received" |
There was a problem hiding this comment.
Same thing here, we could do that when creating a new offer as well
| def sold_action_btn(self): | ||
| for record in self: | ||
| if record.state == "cancelled": | ||
| raise exceptions.UserError(_("Cancelled properties cannot be sold")) | ||
|
|
||
| record.state = "sold" | ||
|
|
||
| def cancelled_action_btn(self): | ||
| for record in self: | ||
| if record.state == "sold": | ||
| raise exceptions.UserError(_("Sold properties cannot be cancelled")) | ||
| record.state = "cancelled" |
There was a problem hiding this comment.
| def sold_action_btn(self): | |
| for record in self: | |
| if record.state == "cancelled": | |
| raise exceptions.UserError(_("Cancelled properties cannot be sold")) | |
| record.state = "sold" | |
| def cancelled_action_btn(self): | |
| for record in self: | |
| if record.state == "sold": | |
| raise exceptions.UserError(_("Sold properties cannot be cancelled")) | |
| record.state = "cancelled" | |
| def action_sold(self): | |
| for record in self: | |
| if record.state == "cancelled": | |
| raise exceptions.UserError(_("Cancelled properties cannot be sold")) | |
| record.state = "sold" | |
| def action_cancel(self): | |
| for record in self: | |
| if record.state == "sold": | |
| raise exceptions.UserError(_("Sold properties cannot be cancelled")) | |
| record.state = "cancelled" |
According to the coding guidelines, the name of action methods should be prefixed with action_
There was a problem hiding this comment.
for the name, it should be similar to the id but with dots instead of underscores. Also, we don't need to specify the module name in the id column, since it is there by default.
| action="estate_settings_property_tag_action" /> | ||
| </menuitem> | ||
| </menuitem> | ||
| </odoo> No newline at end of file |
There was a problem hiding this comment.
| </odoo> | |
| </odoo> | |
Don't forget to add the empty line at the end of every file, since this allows better code tracability (git blame), in case someone decides to add a line after your last line
| @@ -0,0 +1,46 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <!-- the root elements of the data file --> | |||
There was a problem hiding this comment.
| <!-- the root elements of the data file --> |
This comment in the documentation was for explanation purposes. You don't need to put it in the files
| <field name="arch" type="xml"> | ||
| <search string="Filter string"> | ||
| <field name="name" /> | ||
| <filter string="Group by name" name="name" |
There was a problem hiding this comment.
why is this needed? Didn't we make the name field unique anyway?
| <button name="sold_action_btn" type="object" string="Property sold" | ||
| invisible="state == 'sold'" /> | ||
| <button name="cancelled_action_btn" type="object" string="Property cancelled" | ||
| invisible="state == 'sold'" /> |
There was a problem hiding this comment.
Do we want to show the action buttons even when the property is cancelled? in my opinion, trying to sell/cancel an already cancelled property does not make sense (this is still just an opinion, if you think otherwise let me know)
3 participants
Hello,
Happy to make my first PR in odoo.