-
Notifications
You must be signed in to change notification settings - Fork 2.7k
MALER Onboarding #995
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?
MALER Onboarding #995
Conversation
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.
Thanks for your work. Except for the missing blank lines, the pep8 is respected and that's good 😃.
estate/models/__init__.py
Outdated
| @@ -0,0 +1 @@ | |||
| from . import estate_property No newline at end of file | |||
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.
A blank line is missing at the end of your file. You have to put it at the end of all of your files 😃.
estate/models/estate_property.py
Outdated
| garden_orientation = fields.Selection( | ||
| string='Orientation', | ||
| selection=[('north','North'), ('east','East'), ('south','South'), ('west','West')], | ||
| ) No newline at end of file |
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.
The blank line is also missing.
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 | |||
| access_estate_property,access_estate_property,model_estate_property,base.group_user,1,1,1,1 No newline at end of file | |||
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.
The blank line is also missing here too 😃.
estate/__init__.py
Outdated
| @@ -0,0 +1 @@ | |||
| from . import models No newline at end of file | |||
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.
Here too
estate/__manifest__.py
Outdated
| 'data': [ | ||
| 'security/ir.model.access.csv', | ||
| ], | ||
| } No newline at end of file |
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.
Here too 😃
| @@ -0,0 +1,22 @@ | |||
| from odoo import models, fields | |||
|
|
|||
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.
A blank line is missing here. You should have two blank lines above a class. It is a python convention coming from the pip8.
147498a to
5108d34
Compare
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.
Hi @MatteoLeroy 👋
I did a new review of your code. Here are comments about code optimization and python conventions.
Thanks for you work 😃.
| from . import estate_property | ||
| from . import estate_property_type | ||
| from . import estate_property_tag | ||
| from . import estate_property_offer | ||
| from . import inherited_user |
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.
Should be sorted alphabetically 😃.
| from odoo import api, models, fields | ||
| from odoo.exceptions import UserError | ||
| import datetime | ||
| from dateutil.relativedelta import relativedelta | ||
| from odoo.tools.float_utils import float_compare |
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.
Same here.
You must also place built-in imports before external imports and external imports before internal imports. A built-in import is when you import code that comes with Python by default (such as datetime). An external import occurs when you import code from outside your project (such as dateutil). And an internal import occurs when you import a code that is present in your project, but in another file.
| from odoo import api, models, fields | |
| from odoo.exceptions import UserError | |
| import datetime | |
| from dateutil.relativedelta import relativedelta | |
| from odoo.tools.float_utils import float_compare | |
| import datetime | |
| from dateutil.relativedelta import relativedelta | |
| from odoo import api, fields, models | |
| from odoo.exceptions import UserError | |
| from odoo.tools.float_utils import float_compare |
| for record in self: | ||
| if record.state == 'sold': | ||
| raise UserError('A sold property cannot be cancelled') | ||
| else: | ||
| record.state = 'cancelled' | ||
| 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.
Here, your action is used on a form. That means that you are updating only one record. So you do not need to iterate self and you can add a self.ensure_one() to make sure that your self contains only one record, if it is not the case it triggered an error, and so it protects the db. You can apply that to all method called by a button in a form.
| for record in self: | |
| if record.state == 'sold': | |
| raise UserError('A sold property cannot be cancelled') | |
| else: | |
| record.state = 'cancelled' | |
| return True | |
| self.ensure_one() | |
| if self.state == 'sold': | |
| raise UserError('A sold property cannot be cancelled') | |
| record.state = 'cancelled' | |
| return True |
| @api.ondelete(at_uninstall=False) | ||
| def unlink_property(self): | ||
| for record in self: | ||
| if record.state != 'new' and record.state != 'canceled': |
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.
Do not hesitate to use the in operator when you accept several values for a variable.
| if record.state != 'new' and record.state != 'canceled': | |
| if record.state in ['new', 'canceled']: |
| @@ -0,0 +1,67 @@ | |||
| from odoo import api, models, fields | |||
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.
Should be sorted alphabetically.
| from odoo import api, models, fields | |
| from odoo import api, fields, models |
| </list> | ||
| </field> | ||
| </record> | ||
| </odoo> No newline at end of file |
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.
Same here
| </list> | ||
| </field> | ||
| </record> | ||
| </odoo> No newline at end of file |
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.
Same here.
| </kanban> | ||
| </field> | ||
| </record> | ||
| </odoo> No newline at end of file |
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.
Same here.
| </notebook> | ||
| </field> | ||
| </record> | ||
| </odoo> No newline at end of file |
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.
Same here.
| from odoo import fields, models | ||
|
|
||
|
|
||
| class InheritedUser(models.Model): |
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.
The class and file should be named ResUser and res_user.py, respectively.

No description provided.