Server framework 101#1339
Conversation
|
|
ziriraha
left a comment
ziriraha
left a comment
There was a problem hiding this comment.
looks good, one thing to keep in mind is that you should add a new link at the end of each file. i recommend setting up your editor to add it for you.
Co-authored-by: Ziri <45337531+ziriraha@users.noreply.github.com>
LucasDaniOV
force-pushed
the
19.0-onboarding-luoud
branch
from
7b557ac to
c06fdb3
Compare
thank you |
| msg_0 = "Property is already sold" | ||
| raise exceptions.UserError(msg_0) | ||
| property.state = "sold" | ||
| return True |
There was a problem hiding this comment.
fyi (i dont know if its explained in the tutorial) actions can return action objects so you can say things like: open this popup, reload the page, redirect to here
| """ | ||
| Relational fields | ||
| """ |
There was a problem hiding this comment.
I personally dont like these separation comments, some people add them but then others when changing the code they put everything wherever they want and they become useless
nitpick: Also I prefer # instead of """ for multiline comments
| msg = "Selling price cannot be lower than ninety percent of expected price" | ||
| raise exceptions.ValidationError(msg) |
There was a problem hiding this comment.
| msg = "Selling price cannot be lower than ninety percent of expected price" | |
| raise exceptions.ValidationError(msg) | |
| raise exceptions.ValidationError(self.env._("Selling price cannot be lower than ninety percent of expected price")) |
You can inline it and you should make it translatable
Another nitpick: but i prefer if you import from odoo.exceptions import ValidationError instead of using it this way
| @api.model | ||
| def create(self, vals_list): | ||
| for vals in vals_list: | ||
| property = self.env["estate.property"].browse(vals["property_id"]) |
There was a problem hiding this comment.
this is going to explode query count, you should do a browse outside the for loop and the use that recordset to browse it (again) for the correct id, something like:
properties = self.env["estate.property"].browse([vals["property_id"] for vals in vals_list])
for vals in vals_list:
property = properties.browse(vals["property_id"])The first call saves them on cache, the second one just retrieves the one needed
| </list> | ||
| </field> | ||
| </record> | ||
| </odoo> No newline at end of file |
| "installable": True, | ||
| "application": True, | ||
| "data": [ | ||
| "security/ir.model.access.csv", | ||
| ], |
There was a problem hiding this comment.
| "installable": True, | |
| "application": True, | |
| "data": [ | |
| "security/ir.model.access.csv", | |
| ], |
not really as its a module that will connect both apps (not an app itself), in any case we would put auto_install: True so it automatically installs when you have estate and account apps.
| @@ -0,0 +1,11 @@ | |||
| { | |||
| "name": "Real Estate Account", | |||
| "depends": ["base_setup", "estate", "account"], | |||
There was a problem hiding this comment.
| "depends": ["base_setup", "estate", "account"], | |
| "depends": ["estate", "account"], |
base_setup is implied from the other modules
| @@ -0,0 +1,17 @@ | |||
| { | |||
| "name": "Real Estate", | |||
| "depends": ["base_setup"], | |||
There was a problem hiding this comment.
| "depends": ["base_setup"], | |
| "depends": ["base"], |
You dont need base_setup here
3 participants
No description provided.