Odoo tutorial - SERU branch #626
Conversation
|
|
estate/__manifest__.py
Outdated
| 'version': '0.1', | ||
| 'category': 'Real Estate' | ||
| 'category': 'Real Estate', | ||
| 'depends': ['base_setup'], |
There was a problem hiding this comment.
you will probably need base and web. (base setup is way to "low")
estate/__manifest__.py
Outdated
| 'depends':[ | ||
| 'base_setup' | ||
| ], |
There was a problem hiding this comment.
duplicate (it already there in the file)
| <field name="res_model">estate.property</field> | ||
| <field name="view_mode">list,form</field> | ||
| </record> | ||
| </odoo> |
There was a problem hiding this comment.
General remark: not a big deal but we expect 1 empty line at the end of each file
estate/views/estate_menus.xml
Outdated
| @@ -0,0 +1,9 @@ | |||
| <?xml version="1.0"?> | |||
| <odoo> | |||
|
|
|||
There was a problem hiding this comment.
keep your code minimal by removing useless lines
| <form> | ||
| <sheet> | ||
| <h1 style="margin-bottom: 30px"> | ||
| <field name="name" string="Title" placeholder="Title" /> |
There was a problem hiding this comment.
unless you want to specify another label than the one of the field, you don't need to specify the string.
By the way, we should always set a label for field in the model definition. (see my other comment for that)
| <notebook> | ||
| <page string="Description"> | ||
| <group> | ||
| <field name="description" string="Title" /> |
There was a problem hiding this comment.
string seems not really aligned with the field
| <field name="name" string="Title" /> | ||
| <field name="postcode" string="Postcode" /> |
| _name = "estate.property" | ||
| _description = "Real Estate property" | ||
|
|
||
| name = fields.Char(required=True) |
There was a problem hiding this comment.
Always set the label of the field. even if it's obvious and even if the orm will generate one for you using the field name.
postcode = fields.Char("Postcode") or postcode = fields.Char(string="Postcode")
-> better the first in this case, unless string is not the first param. (typically for relational fields)
| garden_area = fields.Integer() | ||
| garden_orientation = fields.Selection([("north", "North"), ("south", "South"), ("east", "East"), ("west", "West")]) | ||
| active = fields.Boolean(default=True) | ||
| state = fields.Selection([("new", "New"), ("offer received", "Offer Received"), ("offer accepted", "Offer Accepted"), ("sold", "Sold"), ("canceled", "Canceled")], required=True, copy=False, default="new") |
There was a problem hiding this comment.
| state = fields.Selection([("new", "New"), ("offer received", "Offer Received"), ("offer accepted", "Offer Accepted"), ("sold", "Sold"), ("canceled", "Canceled")], required=True, copy=False, default="new") | |
| state = fields.Selection([ | |
| ("new", "New"), | |
| ("offer_received", "Offer Received"), | |
| ("offer_accepted", "Offer Accepted"), | |
| ("sold", "Sold"), | |
| ("canceled", "Canceled") | |
| ], string="Status", required=True, copy=False, default="new") |
just a matter of coding style. to gain some lines without being unreadable :)
also, don't use white spaces for the technical name of your selection options.
Syozik
force-pushed
the
seru-branch
branch
3 times, most recently
from
75338dd to
bee76c5
Compare
Followed the tutorial task-xxxxx
Followed the tutorial task-xxxxx
There was a problem hiding this comment.
Hello ! A review that I would do for a 'normal' task. Try to apply every comments.
it's still possible that reviewer is wrong on some comments so keep critical spirit.
I didn't reviewed the architecture of the module as you followed what the tutorial what told you to do. But this is already some point we pay attention to.
In general, keep your code clean and as simple as possible, put and do things at the right place.
enjoy :)
| @@ -53,7 +53,15 @@ class Property(models.Model): | |||
|
|
|||
| total_area = fields.Float("Total area (sqm)", compute="_compute_area") | |||
| best_price = fields.Float("Best price", compute="_compute_best_price") | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
you can know remove those conflict commits
| @@ -1,4 +1,5 @@ | |||
| # -*- coding: utf-8 -*- | |||
| # some not so great changes to make a first commit | |||
| @@ -0,0 +1 @@ | |||
| from . import models | |||
There was a problem hiding this comment.
Commit message:
if you use [ADD], which is the right thing to do in this case, you already say that you add a module, so no need to make it your commit title. Explain what the module does in few words, beginning by a verb.
"[ADD] estate: handle real estate properties and salesflow"
Then in description, you can describe what features and capabilities the module have
and even list all the models used to handle the business cases. What do what and how they interact.
Basically, when reading commit message, you don't need to read the code to know how the module works. Or reading commit message will help you understand the code.
Better reading 10 lines of real words than 250 lines of code.
The goal of the commit message is mainly to help future debugging or refactoring or functionality rework, when you try to find why a certain line has been added to the code base, you read the commit message to get the context in which the commit content has been added. Knowledge is power. Better too much info than not enough :)
| @@ -0,0 +1,19 @@ | |||
| { | |||
| "name": "Real Estate", | |||
| "category": "Real Estate", | |||
| "views/estate_property_tag_views.xml", | ||
| "views/res_users_views.xml", | ||
| ] | ||
| } |
There was a problem hiding this comment.
check for all end of file missing empty lines
| <field name="view_mode">list,form</field> | ||
| </record> | ||
|
|
||
|
|
| <button class="oe_stat_button" icon="fa-money" type="action" | ||
| name="%(estate.estate_property_offer_action)d"> | ||
| <div> | ||
| <field name="offer_count" string="" /> |
| <field name="facades" /> | ||
| <separator /> | ||
| <filter name='available' string="Available" | ||
| domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]" /> |
| selling_price = fields.Float("Selling price", readonly=True, copy=False) | ||
| bedrooms = fields.Integer("Bedrooms", default=2) | ||
| living_area = fields.Integer("Living area (sqm)") | ||
| facades = fields.Integer("Facades") |
There was a problem hiding this comment.
usually when a field is basically a counter, you postfix with '_count'
because 'facade' only is not specific enough
| <field name="name">estate.property.list</field> | ||
| <field name="model">estate.property</field> | ||
| <field name='arch' type="xml"> | ||
| <list string="Properties" decoration-success="state == 'offer_received' or state == 'offer_accepted'" decoration-bf = "state == 'offer_accepted'" decoration-muted="state == 'sold'"> |
Created a new Estate module that allows us to add properties, their types and tags. We can monitor the status of the property. When property is sold another added module - estate account creates a new invoice.