[ADD] Estate Module (Python Framework Training) #1068
Conversation
|
|
barracudapps
left a comment
barracudapps
left a comment
There was a problem hiding this comment.
Some comments here and there. Can you also please review your PR title and description following our guidelines (https://www.odoo.com/documentation/19.0/contributing/development/git_guidelines.html)?
A clear commit message is really important to describe what you did and why you did it.
OmarKhalil2001
changed the title
OmarKhalil2001
force-pushed
the
omkha/first_pr_tutorial
branch
2 times, most recently
from
1911748 to
162d90a
Compare
OmarKhalil2001
force-pushed
the
omkha/first_pr_tutorial
branch
from
162d90a to
bf9de48
Compare
…e Module. Improved UI to show all the details about each building
barracudapps
left a comment
barracudapps
left a comment
There was a problem hiding this comment.
Thanks for the feature. Here are some comments.
Avoid as much redundancy as possible, ease the searching and respect our naming conventions.
Let's take buildings_model.py as an example. The file is in the models folder of the estate module a better naming would have been building.py (or maybe estate_building.py as it allow a quick search on the module name and building.py might exist in many modules with different meanings).
Moreover, as it is a model the name should not be plural. Indeed, this is the template to create one building (multiple times but it's still 1 item).
Last thing here: please follow the instructions as if the task (tutorial) was written by a PO). You can of course challenge everything but I would've gone with a property instead of a building.
Also, can you please change the PR message and tell a bit more about what the added code is doing and why you added these lines?
| "category": "Tutorials", | ||
| "version": "1.1", | ||
| "application": True, | ||
| "data": ["security/ir.model.access.csv", "views/views.xml", "views/menus.xml"], |
There was a problem hiding this comment.
Please split your data files (same comment for as many lists as possible):
| "data": ["security/ir.model.access.csv", "views/views.xml", "views/menus.xml"], | |
| "data": [ | |
| "security/ir.model.access.csv", | |
| "views/views.xml", | |
| "views/menus.xml", | |
| ], |
If someone has to add a file, it will be way easier and keep a clear overview (with git history) of the changes.
| "version": "1.1", | ||
| "application": True, | ||
| "data": ["security/ir.model.access.csv", "views/views.xml", "views/menus.xml"], | ||
| "author": "OMKHA", |
There was a problem hiding this comment.
| "author": "OMKHA", | |
| "author": "Odoo", |
As you are developing for Odoo, the company is the author of the file.
| "name": "real estate", | ||
| "description": "real estate renting management system", |
There was a problem hiding this comment.
I would have capitalized these lines
| from odoo import models, fields | ||
|
|
||
|
|
||
| class building_type_model(models.Model): |
There was a problem hiding this comment.
The class name should match the file name (for most cases) and follow the PascalCase convention.
As PEP8 says (https://peps.python.org/pep-0008/#class-names):
Class names should normally use the CapWords convention
| class building_type_model(models.Model): | |
| class BuildingType(models.Model): |
| description = fields.Text() | ||
| value = fields.Integer(copy=False) | ||
| availability_date = fields.Date( | ||
| default=fields.Date.today() + timedelta(days=90), copy=False |
There was a problem hiding this comment.
Use a lambda function or call a compute method because this will fix the default date to the server starting date
| access_first_model,access_first_model,model_estate_buildings,base.group_user,1,1,1,1 | ||
| access_building_type_model,access_building_type_model,model_estate_building_type,base.group_user,1,1,1,1 | ||
| access_building_tags_model,access_building_tags_model,model_estate_building_tags,base.group_user,1,1,1,1 | ||
| access_offers_model,access_offers_model,model_estate_offers,base.group_user,1,1,1,1 No newline at end of file |
There was a problem hiding this comment.
Don't forget Unix conventions on text files' structure based on a line definition (cf. https://peps.python.org/pep-0008/ and https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_206)
Check all XML files too
| <field name="name"/> | ||
| <field name="value"/> | ||
| <separator/> | ||
| <filter string="Available" name="Available" domain="['|', ('state', '=', 'new' ), ('state', '=', 'offer received')]"/> |
There was a problem hiding this comment.
Maybe try something like [('state', 'in', ['new', 'offer_received'])]
3 participants
[ADD] Implementation for Estate Module of Python Framework
Description: