-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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] Create module 'estate' and add models defination,security files… #320
base: 18.0
Are you sure you want to change the base?
Conversation
…, views and menus
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.
Hello @matd-odoo ,
Great work so far!
I've added some suggestions, please have a look at them.
Some general instructions:
- Please make sure to add a new line at the end of every file.
- Please ensure the entire module follows coding guidelines.
Thanks!
estate/__manifest__.py
Outdated
# # data files containing optionally loaded demonstration data | ||
# 'demo': [ | ||
# 'demo/demo_data.xml', | ||
# ], |
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.
Please remove unnecessary lines.
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.
Changes are done.
estate/__manifest__.py
Outdated
# 'demo': [ | ||
# 'demo/demo_data.xml', | ||
# ], | ||
} |
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 new line at EOF. Please check other occurrences as well.
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.
Added new line at EOF.
@@ -0,0 +1,38 @@ | |||
from odoo import models, fields | |||
from datetime import timedelta |
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.
Order of import should be proper.
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.
Changes are done.
estate/models/estate_property.py
Outdated
|
||
class EstateProperty(models.Model): | ||
_name = "estate.property" | ||
_description = "Property warning!" |
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.
Description for model should be appropriate.
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.
Changes are done.
estate/models/estate_property.py
Outdated
_name = "estate.property" | ||
_description = "Property warning!" | ||
|
||
name = fields.Char('Name', required=True, default='Unknown') |
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 string for the fields should be appropriate. Also what's the need of adding "default" parameter here?
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.
Suggested changes are done.
"default" parameter is use for by default set "Unknown" value in case if user not add name.
estate/models/estate_property.py
Outdated
name = fields.Char('Name', required=True, default='Unknown') | ||
description = fields.Text() | ||
postcode = fields.Char('postcode', size=6) | ||
date_avaliability = fields.Date('Available date', default=(fields.Date.today() + timedelta(days=90)).strftime('%Y-%m-%d'), copy=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.
Could you explain the purpose of "copy", "required", "readonly" parameters?
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.
"copy" parameter is use for copy fields value. by default "copy=True", if do not want to copy value then set "copy=False".
"required" parameter is use for fields value are mandatory.
"readonly" parameter is use for only read value of fields, can't change or update values.
estate/models/estate_property.py
Outdated
('cancelled', "Cancelled") | ||
] | ||
) | ||
active = fields.Boolean(active=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.
What is the purpose of active field? and why you have passed 'False' in default?
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.
active field is use for filtering between currently active and non-active records, default=True use for non-active records.
estate/__manifest__.py
Outdated
It's provide real estate module | ||
""", | ||
'application': True, | ||
# data files always loaded at installation |
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.
# data files always loaded at installation |
is there any need for this comment here?
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, changes are done.
|
||
|
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.
unnecessary lines
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.
Suggested changes are done.
…custom list view, form view and search view.
…, views and menus