Technical Onboarding - Server Framework 101#1284
Conversation
|
|
sidum-odoo
changed the title
sidum-odoo
force-pushed
the
19.0-technical-onboarding-sidum
branch
6 times, most recently
from
d9590a4 to
7c8b8e2
Compare
|
@Megaaaaaa ready for review |
Chapters 1 to 15 of the Server Framework 101 Tutorial, which is about building a Real Estate Management App within Odoo. The main module "estate" allows property management with Kanban, List and Form views. Types and tags and offers can be added/assigned to a property. The flow is managed through automatic state changes when offers are received/accepted. Additionnaly, the module "estate_account" allows automatic invoice creation when a property is sold.
sidum-odoo
force-pushed
the
19.0-technical-onboarding-sidum
branch
from
7c8b8e2 to
f46f1a1
Compare
Megaaaaaa
left a comment
Megaaaaaa
left a comment
There was a problem hiding this comment.
Hello 👋
I must say I'm impressed, this is a really really good PR 👍
There is very few things to say and very few reccurring comments. Only the trailing commas and the translations. Otherwise it's rather comment about technical stuff than about guielines. Tat's the kind of thing you want to have on your PRs later. That's nice 👍
So, please keep in mind that a review is not the utlimate solution, it's only someone seeing things he would have done differently and offering an alternative. You're always free to answer with an other alternative or even say you disagree and bring your arguments. For stuff that's not just styling or conventions, don't tak the wrong habit of just blindly applying comments, you spent time developping the thing, everything you've tried doesn't show up in the diff, only the final one. You probably have very interesting things to say to the reviewer about the decisions you made.
What I would suggest you do and continue to do while you're not the most comfortable with odoo's structure is to not apply all the changes at once. For example, changing multiple compute methods at once might lead you to a lot of errors if you don't check that the new implementation doesn't break anything before working on another one. Splitting the review in multiple steps and trying to run your db in between those can save you a lot of time in the long run.
Also a quick tip for the methodology when applying reviews changes: What I like to do it put a reaction like a thumbsup on a message once I have made the change locally. Then when everything is marked with something, I push, go through my diff and mark as resolved the things that are now outdated so nothing is left forgotten.
If you have any question, don't hesitate to ask!
| from . import ( | ||
| estate_property, | ||
| estate_property_offer, | ||
| estate_property_tag, | ||
| estate_property_type, | ||
| res_users, | ||
| ) |
There was a problem hiding this comment.
I agree it looks better but we write the from . import x on each line except maybe for test files 😄
| from . import ( | |
| estate_property, | |
| estate_property_offer, | |
| estate_property_tag, | |
| estate_property_type, | |
| res_users, | |
| ) | |
| from . import estate_property | |
| from . import estate_property_offer | |
| from . import estate_property_tag | |
| from . import estate_property_type | |
| from . import res_users |
| from dateutil.relativedelta import relativedelta | ||
|
|
||
| from odoo import api, fields, models | ||
| from odoo.exceptions import UserError, ValidationError | ||
| from odoo.tools.float_utils import float_compare, float_is_zero |
| class EstateProperty(models.Model): | ||
| _name = "estate.property" | ||
| _description = "Estate Property" | ||
| _order = "id desc" |
| _description = "Estate Property" | ||
| _order = "id desc" | ||
|
|
||
| name = fields.Char(string="Title", required=True) |
There was a problem hiding this comment.
And nice indentation, spacing, decision making on when to oneline or multiline a field. I already sense this review is going to go very well 👍
| ("south", "South"), | ||
| ("east", "East"), | ||
| ("west", "West"), | ||
| ] |
There was a problem hiding this comment.
The trailing coma so that people can add attributes to the field without modifying your lines 👍
| ] | |
| ], |
| property_ids = fields.One2many( | ||
| "estate.property", | ||
| "salesperson_id", | ||
| domain="['|',('state', '=', 'new'),('state', '=', 'offer_received')]", |
There was a problem hiding this comment.
No space around operators but definitely a space after commas 😄
| domain="['|',('state', '=', 'new'),('state', '=', 'offer_received')]", | |
| domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]", |
| <list> | ||
| <field name="name"/> | ||
| <field name="expected_price"/> | ||
| <field name ="state"/> |
There was a problem hiding this comment.
You did that to check if I was paying attention right ? Well I AM ! 😄
| <field name ="state"/> | |
| <field name="state"/> |
| "name": "Real Estate", | ||
| "application": True, | ||
| "depends": ["base"], | ||
| "author": "Simon Dumoulin", |
There was a problem hiding this comment.
As an Odoo employee, you can't sign your work in your own name. The author of this module is "Odoo S.A." 😄
| # IDE | ||
| .vscode/ | ||
| ruff.toml | ||
|
|
||
| # Distribution / packaging |
| { | ||
| "name": "Property Invoicing", | ||
| "depends": ["estate", "account"], | ||
| "author": "Simon Dumoulin", |
3 participants
No description provided.