Conversation
6449371 to
077ffd7
Compare
lost-odoo
left a comment
There was a problem hiding this comment.
Hello, Good Job! I have already made a first pass. Feel free to ask if you have any questions 😄
estate/models/estate_property.py
Outdated
| from dateutil.relativedelta import relativedelta | ||
|
|
||
| class RecurringPlan(models.Model): | ||
| _name = "estate.property" |
There was a problem hiding this comment.
We typically use single quotes for strings, and reserve double quotes for user-facing text.
| _name = "estate.property" | |
| _name = 'estate.property' |
I'll let you modify it at other places
estate/models/estate_property.py
Outdated
| name = fields.Char(required=True) | ||
| description = fields.Text() | ||
| postcode = fields.Char() | ||
| date_availability = fields.Date('Date Available', copy=False, default=(fields.Date.today() + relativedelta(months=3))) |
There was a problem hiding this comment.
You should use a lambda here; otherwise, this value will be evaluated at server start and remain static.
estate/security/ir.model.access.csv
Outdated
| @@ -0,0 +1,2 @@ | |||
| id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink | |||
| estate.access_estate_property,access_estate_property,estate.model_estate_property,base.group_user,1,1,1,1 No newline at end of file | |||
estate/security/ir.model.access.csv
Outdated
| @@ -0,0 +1,2 @@ | |||
| id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink | |||
| estate.access_estate_property,access_estate_property,estate.model_estate_property,base.group_user,1,1,1,1 No newline at end of file | |||
There was a problem hiding this comment.
| estate.access_estate_property,access_estate_property,estate.model_estate_property,base.group_user,1,1,1,1 | |
| access.estate.property.user,access_estate_property_user,estate.model_estate_property,base.group_user,1,1,1,1 |
estate/views/estate_menus.xml
Outdated
| <menuitem id="estate_model_menu_action" action="estate_property_action"/> | ||
| </menuitem> | ||
| </menuitem> | ||
| </odoo> No newline at end of file |
estate/views/estate_menus.xml
Outdated
| <?xml version="1.0" encoding="utf-8"?> | ||
| <odoo> | ||
| <menuitem id="estate_property_menu_root" name="Estate Properties"> | ||
| <menuitem id="properties" name="Properties"> |
There was a problem hiding this comment.
| <menuitem id="properties" name="Properties"> | |
| <menuitem id="estate_properties_menu" name="Properties"> |
estate/views/estate_menus.xml
Outdated
| <odoo> | ||
| <menuitem id="estate_property_menu_root" name="Estate Properties"> | ||
| <menuitem id="properties" name="Properties"> | ||
| <menuitem id="estate_model_menu_action" action="estate_property_action"/> |
There was a problem hiding this comment.
| <menuitem id="estate_model_menu_action" action="estate_property_action"/> | |
| <menuitem id="estate_property_action" action="estate_property_action"/> |
dd1d706 to
e9e2d4c
Compare
lost-odoo
left a comment
There was a problem hiding this comment.
Hello, Good job !
I have made my last review for the ORM training. Don't forget to fix those comments and then resolve them on github 😄
| name = fields.Char(required=True) | ||
| class EstateProperty(models.Model): | ||
| _name = 'estate.property' | ||
| _description = 'A specific property' |
There was a problem hiding this comment.
Double quotes here as it is a string that will be shown to the user
| def action_sold(self): | ||
| for record in self: | ||
| if record.state == 'cancelled': | ||
| raise exceptions.UserError('A cancelled listing cannot be sold') |
There was a problem hiding this comment.
You should import UserError directly instead of using exceptions.UserError
| def action_sold(self): | ||
| for record in self: | ||
| if record.state == 'cancelled': | ||
| raise exceptions.UserError('A cancelled listing cannot be sold') |
There was a problem hiding this comment.
Also it needs to be translated using self.env._("A cancelled listing cannot be sold.")
Do not forget about double quotes also 😄
| raise exceptions.ValidationError( | ||
| 'The selling price must be at least 90%% of the expected price' | ||
| ) | ||
|
|
There was a problem hiding this comment.
Same here and for every other exceptions ^^
| if float_compare( | ||
| record.selling_price, record.expected_price * 0.9, 2 | ||
| ) == -1 and not float_is_zero(record.selling_price, 2): |
There was a problem hiding this comment.
if float_compare(record.selling_price, record.expected_price * 0.9, 2) == -1
and not float_is_zero(record.selling_price, 2):
This is easier to read IMO 😄
| @api.model_create_multi | ||
| def create(self, vals_list): | ||
| for val in vals_list: | ||
| property_for_offer = self.env['estate.property'].browse(val['property_id']) |
There was a problem hiding this comment.
Avoid doing a browse in a loop because that way you make a lot of sql queries. You can browse all of them beforehand then use the records in the loop.
|
|
||
| class EstatePropertyType(models.Model): | ||
| _name = 'estate.property.tag' | ||
| _description = 'A property tag' |
There was a problem hiding this comment.
The Description is not really a description I know it is confusing haha. It should be the Display String of the Model.
So here for instance : "Property Tag" and for estate.property.offer: "Property Offer".
Don't forget to change it for all models ^^
| 'estate.property', | ||
| 'salesperson_id', | ||
| string='Property', | ||
| domain="['|', ('state', '=', 'New'), ('state', '=', 'Offer Received')]", |
There was a problem hiding this comment.
The domain should not be in a string. You can directly pass the list otherwise it should not work.
| <odoo> | ||
| </odoo> |
| # Pyre type checker | ||
| .pyre/ | ||
|
|
||
| # Linting | ||
| pyproject.toml |

No description provided.