-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[ADD] Design an Estate module #1125
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?
Conversation
This module will be the basis of the Estate App
artn-odoo
left a comment
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.
Nice job so far !
All comments are small styling nitpicks but your code won't pass the styling tests if they are not fixed.
We also try to always add an empty line at the end of every file. If using vscode, the Insert Final Newline option can be activated to do it automatically every time you save your file.
- selling_price will be set after the sale, and should not be duplicated when created a new, not sold, property. - date_availability should not be copied, because a dynamic default value will be added
model, menus, views, access
artn-odoo
left a comment
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.
Nice one, as usual just a few nitpicks
estate/models/estate_property.py
Outdated
| date_availability = fields.Date('Availability Date', copy=False, | ||
| default=fields.Date.add(fields.Date.today(), months=3)) |
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.
If the line is split because it's too long, every parameter should be on it's own line
| date_availability = fields.Date('Availability Date', copy=False, | |
| default=fields.Date.add(fields.Date.today(), months=3)) | |
| date_availability = fields.Date( | |
| 'Availability Date', | |
| copy=False, | |
| default=fields.Date.add(fields.Date.today(), months=3) | |
| ) |
And the closing bracket's indentation should match the opening one
estate/models/estate_property.py
Outdated
| garden_orientation = fields.Selection( | ||
| string='Garden Orientation', | ||
| selection=[ | ||
| ('north', 'North'), ('south', 'South'), | ||
| ('east', 'East'), ('west', 'West') | ||
| ]) |
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.
Try to match the opening bracket's indentation
| garden_orientation = fields.Selection( | |
| string='Garden Orientation', | |
| selection=[ | |
| ('north', 'North'), ('south', 'South'), | |
| ('east', 'East'), ('west', 'West') | |
| ]) | |
| garden_orientation = fields.Selection( | |
| string='Garden Orientation', | |
| selection=[ | |
| ('north', 'North'), | |
| ('south', 'South'), | |
| ('east', 'East'), | |
| ('west', 'West') | |
| ] | |
| ) |
Also, it would be better to have each selection proposition on it's own line
| selection=[ | ||
| ('new', 'New'), ('offer_received', 'Offer Received'), | ||
| ('offer_accepted', 'Offer Accepted'), | ||
| ('sold', 'Sold'), ('cancelled', 'Cancelled') ], | ||
| default='new', | ||
| required=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.
Same as above
| selection=[ | |
| ('new', 'New'), ('offer_received', 'Offer Received'), | |
| ('offer_accepted', 'Offer Accepted'), | |
| ('sold', 'Sold'), ('cancelled', 'Cancelled') ], | |
| default='new', | |
| required=True | |
| ) | |
| selection=[ | |
| ('new', 'New'), | |
| ('offer_received', 'Offer Received'), | |
| ('offer_accepted', 'Offer Accepted'), | |
| ('sold', 'Sold'), | |
| ('cancelled', 'Cancelled') | |
| ], | |
| default='new', | |
| required=True | |
| ) |

No description provided.