-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
18.0 tutorial drod #178
base: 18.0
Are you sure you want to change the base?
18.0 tutorial drod #178
Conversation
c592c35
to
c61bcc4
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.
Very very nice work ! You thought about (almost) every corner cases, well done !
You can quickly work on the different comments I sent you and squash your commits in one (only yours this time 😄 ).
Well done !
@@ -0,0 +1 @@ | |||
from . import estate_property_infos, estate_property, estate_property_offer, res_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.
Better to give each its own line: clearer and later, if some files have been added, people can find more easily who was at the origin of a specific file with a git blame
|
||
|
||
class Estate_Property(models.Model): | ||
_name = "estate_property" |
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.
_name = "estate_property" | |
_name = "estate.property" |
We use _
only in specific cases, in a model name. It is better to use a .
in between words (you could think of it as the model "property" of the module "estate" -> estate.property
).
This is the same for all other model names
active = False | ||
|
||
name = fields.Char(required=True, string="Title") | ||
|
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.
Avoid unnecessary empty lines between fields declarations, it takes a lot of space and doesn't bring much clarity :D
) | ||
|
||
date_availability = fields.Date( | ||
default=(date.today() + relativedelta(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.
You could also use fields.Date.today() + relativedelta(months=3)
if you wanted, it would use one less import
|
||
color = fields.Integer(string="Colour") | ||
|
||
property_estate_ids = fields.Many2many( |
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 unnecessary, we just want the tags on the properties, but we don't really care to find properties from a tag (a search by tags on the properties list view will suffice if we want to find properties with a special tag
<group> | ||
<group> | ||
<field name="type_id" attributes="{'can_write': false}" | ||
options="{'no_create': true, 'no_open': 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.
Why those options and attribute ?
<field name="name">estate_property_kanban</field> | ||
<field name="model">estate_property</field> | ||
<field name="arch" type="xml"> | ||
<kanban class="o_kanban_mobile" default_group_by="type_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.
Don't forget your kanban view shouldn't allow drag and drop
'name': '[TUTO] Estate_Account', | ||
'summary': 'Module to invoice sold real estate properties', | ||
'depends': [ | ||
'base_setup', 'estate', 'account' |
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.
'base_setup' here is not needed, as it is already a parent of 'account' which this module depends on ;)
'base_setup', 'estate', 'account' | ||
], | ||
'license': "LGPL-3", | ||
'data': [ |
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 need to add empty list in the manifest
[ADD] awesome_owl: Add ugly-ass boxes (Chapter 3) [IMP] awesome_owl: Finish Chapter 4 I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript I hate JavaScript [IMP] awesome_owl: Finish Chapter 5 [IMP] awesome_owl: Finish Chapter 6
[IMP] awesome_owl: Finish Chapter 8 [IMP] awesome_owl: Finish Chapter 9 [IMP] awesome_owl: Finish Chapter 10
No description provided.