Skip to content

[ADD] estate: Initial setup for module - #1369

Open
sdemeesterde wants to merge 21 commits into
odoo:19.0from
odoo-dev:19.0-realestate-samde
Open

[ADD] estate: Initial setup for module#1369
sdemeesterde wants to merge 21 commits into
odoo:19.0from
odoo-dev:19.0-realestate-samde

Conversation

@sdemeesterde

Copy link
Copy Markdown

Hello,

Happy to make my first PR in odoo.

@sdemeesterde
sdemeesterde requested a review from SaddemAmine July 20, 2026 13:36
@robodoo

robodoo commented Jul 20, 2026

Copy link
Copy Markdown

Pull request status dashboard

@sdemeesterde
sdemeesterde requested a review from YassinWalid July 22, 2026 07:19

@YassinWalid YassinWalid left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the impressive work. I have done an initial review, and left some comments in the PR and other general ones below. Please check them out, especially the coding guidelines and how to format the name/id of python fields and methods, xml record names and ids, as well as the model names themselves

Instead of date and datetime, you should use fields.Date and fields.Datetime

For the naming convention of records' name/id, name of models, fields, the order of methods for a model, and much more, check the coding guidelines for useful tips/rules.

Comment thread estate/models/estate_property.py Outdated
Comment thread estate/models/estate_property.py Outdated
Comment thread estate/models/estate_property.py Outdated
Comment thread estate/models/estate_property.py Outdated
Comment thread estate/models/estate_property.py Outdated
Comment thread estate/security/ir.model.access.csv
Comment thread estate/views/estate_menus.xml Outdated
Comment thread estate/views/estate_property_offers_list.xml Outdated
Comment thread estate/views/estate_property_tag_views.xml Outdated
Comment thread estate/views/estate_property_views.xml Outdated
@sdemeesterde
sdemeesterde force-pushed the 19.0-realestate-samde branch from 5142161 to 15fcd01 Compare July 30, 2026 11:07
@sdemeesterde

Copy link
Copy Markdown
Author

Thank you for the many comments ! I really appreciate all the feedback, and tried to fix all the issues.

I still have a warning from the ci about the translation, but should I care about it ?

@sdemeesterde
sdemeesterde requested a review from YassinWalid July 30, 2026 11:14
@sdemeesterde sdemeesterde self-assigned this Jul 30, 2026

@YassinWalid YassinWalid left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work. I left some final comments on the PR, as well as some general ones below.

For the commit message titles, they should be [TAG] <module_name>: title. So in your case, they should have "estate" instead of "Real Estate". Also, the commit that introduces estate_account should have the tag [ADD] not [IMP].

Regardig the warnings, they're because translations should be done as self.env._ instead of _ since 18.0

Comment thread estate/models/res_users.py
Comment thread estate/models/estate_property_offer.py Outdated
Comment thread estate/security/ir.model.access.csv Outdated
Comment thread estate/models/estate_property_offer.py Outdated
Comment thread estate/views/user_properties_views.xml Outdated
Comment thread estate_account/models/estate_property.py Outdated
Comment thread estate_account/security/ir.model.access.csv Outdated
@sdemeesterde sdemeesterde changed the title [ADD] Real estate: Initial setup for module [ADD] estate: Initial setup for module Jul 31, 2026
@sdemeesterde
sdemeesterde force-pushed the 19.0-realestate-samde branch from 15fcd01 to 4e2016e Compare July 31, 2026 11:26
@sdemeesterde

Copy link
Copy Markdown
Author

Thanks for the review, I implemented the necessary changes.

@sdemeesterde
sdemeesterde requested a review from YassinWalid July 31, 2026 12:39

@YassinWalid YassinWalid left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work.
One tiny further comment: For 528d7a9 the title should have "estate_account" instead of "estate", since this is the module you're adding.

@sdemeesterde
sdemeesterde force-pushed the 19.0-realestate-samde branch from 6a27940 to fc34842 Compare July 31, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants