-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[ADD] estate : adding new module in tutorials #216
base: 18.0
Are you sure you want to change the base?
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.
You are on the right track
- Configure your IDE to get ride of EOF issues. "Insert Final Newline" in VScode.
- Remove all
# -*- coding: utf-8 -*-
, they are not required anymore (since version 16.0). - Keep your python imports in alphabetical order to prevent duplication (ref: estate_property.py)
estate/__manifest__.py
Outdated
'summary': "Realistic business advertisements", | ||
'application': True, | ||
'installable': True, | ||
'auto_install': False, |
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.
This is already the default (ref: doc)
estate/models/estate_property.py
Outdated
|
||
|
||
|
||
|
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.
two blank lines before class declaration
estate/models/estate_property.py
Outdated
@api.onchange('garden') | ||
def _onchange_garden(self): | ||
self.garden_area = 10 if self.garden else False | ||
self.garden_orientation = 'n' if self.garden else False |
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.
Does not seem to match your selection field declaration
estate/models/estate_property.py
Outdated
|
||
@api.onchange('garden') | ||
def _onchange_garden(self): | ||
self.garden_area = 10 if self.garden else False |
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.
self.garden_area = 10 if self.garden else False | |
self.garden_area = 10 if self.garden else 0 |
|
||
def action_accept(self): | ||
for record in self: | ||
try: |
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.
Why is the try catch necessary ? Either you fix the issue or add a comment to explain why it might behave unexpectedly
record.property_id.action_process_accept(self) | ||
record.status = 'accepted' | ||
except UserError as e: | ||
raise e |
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.
Only raise builtin odoo errors for optimal UX
raise e | |
raise UserError("error message") |
@AbdelrahmanFawzy1 The CI tool is still red, please try to fix the issues |
Co-authored-by: Killian Frappart <[email protected]>
Co-authored-by: Killian Frappart <[email protected]>
Co-authored-by: Killian Frappart <[email protected]>
b5524df
to
e4e3326
Compare
raise UserError("Cancelled Items cannot be sold.") | ||
self.state = "sold" | ||
|
||
def action_set_sold(self): |
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.
no loop in action coming from a form view
def _compute_date_deadline(self): | ||
for record in self: | ||
if not record.create_date: | ||
record.create_date = fields.Date.today() |
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.
You should probably refrain from writing on super fields such as id, create_date, etc...
raise UserError("this property has already an accepted offer!!") | ||
self.state = "offer_accepted" | ||
self.selling_price = offer.price | ||
self.partner_id = offer.partner_id |
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.
missing return
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.
Why do I need it? I am just setting self attributes?
estate/models/estate_property.py
Outdated
|
||
|
||
@api.ondelete(at_uninstall=False) | ||
def prevent_delete(self): |
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.
naming convention
def prevent_delete(self): | |
def _unlink_property(self): |
("check_price", "CHECK(price >= 0)", "Offer prices MUST be postive."), | ||
] | ||
|
||
def action_accept(self): |
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.
no loop in form action
@@ -0,0 +1,30 @@ | |||
from odoo import models, Command | |||
|
|||
class InheritedEstateProperty(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.
class InheritedEstateProperty(models.Model): | |
class EstateProperty(models.Model): |
@@ -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 |
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.
This access rule exists already in the estate module
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.
Doesn't each module have to own its own access rules?! Is it because we inherit from the estate module, then its access rules apply here as well?
this.todoItemsNr =0; | ||
useAutofocus("todo_input"); | ||
} | ||
addTodo(inp) { |
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.
JS convention
addTodo(inp) { | |
addTodo(event ) { |
|
||
} | ||
inp.target.value = ''; | ||
this.render(); |
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.
You should probably not need to call this method explicitly
this.render(); |
|
||
@api.model_create_multi | ||
def create(self, vals): | ||
property_record = self.env['estate.property'].browse(vals['property_id']) |
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.
property_record = self.env['estate.property'].browse(vals['property_id']) | |
property_record = self.env['estate.property'].browse(vals['property_id']).exists() |
existing_offers = self.search([ | ||
('property_id', '=', vals['property_id']) | ||
]) | ||
if any(offer.price >= vals['price'] for offer in existing_offers): |
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 price comparison should ba a part of the search query above for optimal performance
No description provided.