srkod - estate addon practice by sreedevk · Pull Request #1278 · odoo/tutorials

sreedevk · 2026-05-19T14:34:26Z

No description provided.

robodoo · 2026-05-19T14:34:29Z

Megaaaaaa

Hello 👋

Here's a first (official) review for you, really nice PR for a first, gj 👍

There's not much and even amongst those there quite a lot of things that repeat themselves so not a whole lot to say 😄

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 the time developping the thing, everything you've tried doesn't show up in the diff, only the last 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 a technical value inside a Selection field has a lot of impact and if you change everything at once, you might end up with a lot of errors when running the database. 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 or answered, 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!

Megaaaaaa · 2026-05-22T07:34:53Z

+    description = fields.Text()
+    postcode = fields.Char()
+    date_availability = fields.Date(
+        default=(fields.Date.today() + relativedelta(months=3)), copy=False


Just making sure the history gets preserved when someone else want to add a line 👍

Suggested change

default=(fields.Date.today() + relativedelta(months=3)), copy=False

default=(fields.Date.today() + relativedelta(months=3)), copy=False,

Megaaaaaa · 2026-05-22T07:35:03Z

+        default=(fields.Date.today() + relativedelta(months=3)), copy=False
+    )
+    seller_id = fields.Many2one(
+        "res.users", string="Sales Person", default=lambda self: self.env.user


Suggested change

"res.users", string="Sales Person", default=lambda self: self.env.user

"res.users", string="Sales Person", default=lambda self: self.env.user,

Megaaaaaa · 2026-05-22T07:36:24Z

+    _check_expected_price = models.Constraint(
+        "CHECK(expected_price > 0)", "Expected price should be positive."
+    )


Usually in a model definition we put first the "underscore fields" (_name, _desc, etc) then all the fields, then all the constrains and finally all the methods so this would go after all you fields declarations 👍

Megaaaaaa · 2026-05-22T07:36:37Z

+    _check_selling_price = models.Constraint(
+        "CHECK(selling_price > 0)", "Selling price must be positive."
+    )


And same here 👍

Megaaaaaa · 2026-05-22T08:05:12Z

+                        <group>
+                            <field 
+                                name="offer_ids"
+                                context="{'list_view_of': 'estate.estate_property_offer_list_form'}"


typo ?

Suggested change

context="{'list_view_of': 'estate.estate_property_offer_list_form'}"

context="{'list_view_ref': 'estate.estate_property_offer_list_form'}"

Megaaaaaa · 2026-05-22T08:07:18Z

+                <notebook>
+                    <page string="Properties">
+                        <group>
+                            <field name="property_ids" context="{'list_view_of': 'estate.estate_property_list_form'}" />


again ?

Suggested change

<field name="property_ids" context="{'list_view_of': 'estate.estate_property_list_form'}" />

<field name="property_ids" context="{'list_view_ref': 'estate.estate_property_list_form'}" />

Megaaaaaa · 2026-05-22T08:07:37Z

+    "author": "Sreedev Kodichath <srkod@odoo.com>",
+    "license": "LGPL-3",
+    "data": [
+        # datafiles


Don't keep the comments 😄

Suggested change

# datafiles

Megaaaaaa · 2026-05-22T08:07:46Z

+    "data": [
+        # datafiles
+        "security/ir.model.access.csv",
+        # views


Suggested change

# views

Megaaaaaa · 2026-05-22T08:08:13Z

+{
+    "name": "Estate",
+    "depends": ["base"],
+    "author": "Sreedev Kodichath <srkod@odoo.com>",


As an Odoo employee, you cannot sign any work in your own name, the author of this module is "Odoo S.A." 😉

Megaaaaaa · 2026-05-22T08:20:19Z

Also since you've already squashed your work into a single commit, you can spend the extra time to make it compliant to the git guidelines. We'll explain it next week anyway but you've done the hardest part, having a single commit 😄
If you do so, don't forget to update the PR title and message to remain the same as your commit's because those won't update automatically.
(and if you're not sure about the task-123, here it would be task-6229508, 6229508 being the id of your Server 101 task you can find in its url: https://www.odoo.com/odoo/project/26755/tasks/6229508)

[ADD] estate: added the module setup files, the estate_property model & basic default views [LINT] estate: ruff linting & formatting [FIX] estate: added all rights to estate_property model [ADD] estate: estate_property model field level restrictions & properties added [LINT] estate: ruff linting & formatting [ADD] estate: custom list view for estate_property [REF] estate: follow naming convention for view records [REM] estate: remove test model [ADD] estate: added search view with custom fields [ADD] estate: add filters for available properties & group by postcode [ADD] estate: add property type models & views [ADD] estate: property type relationship to property added [ADD] estate: buyer & seller fields added to estate_property [ADD] estate: property tag added [LINT] estate: linting & formatting [ADD] estate: estate property offer added [ADD] estate: best price & total area computed fields added [ADD] estate: property offer date_deadline & validity fields added [ADD] estate: property onchange method for garden based attributes [ADD] estate: offer refuse & accept operations added [ADD] estate: sold & cancelled buttons added to properties [ADD] estate: accepting offer now sets buyer & selling price of property [ADD] estate: property constraints [LINT] satisfy the linting and formatting overlords [FIX] estate: api.constrains was misspelt as api.constraint [FIX] estate: missing precision_digits in float_is_zero call [FIX] estate: inverse deadline computation type issue fixed [ADD] estate: property_ids view for property_type form [LINT] estate: sacrifice to the linter [ADD] estate: property status now displayed as a statusbar widget [ADD] estate: deterministic ordering of records [ADD] estate: manual sequencing of types enabled using a handle widget [ADD] estate: tag color fields added to property form & prevent type creation [ADD] estate: sold and cancel buttons now only show up when appropriate [ADD] estate: default available filter applied Update estate/models/estate_property.py Co-authored-by: Thomas THBE <65757639+Megaaaaaa@users.noreply.github.com> tmp

sreedevk requested a review from Megaaaaaa May 19, 2026 15:14

sreedevk changed the title ~~added base module~~ srkod - estate addon practice May 19, 2026

sreedevk force-pushed the 19.0-setting-up-real-estate-module-srkod branch 11 times, most recently from 9ee43b2 to edfc7fe Compare May 22, 2026 07:59

Megaaaaaa requested changes May 22, 2026

View reviewed changes

sreedevk force-pushed the 19.0-setting-up-real-estate-module-srkod branch from 2d80c99 to 151935a Compare May 22, 2026 18:15

sreedevk force-pushed the 19.0-setting-up-real-estate-module-srkod branch from 151935a to b4d66c5 Compare May 22, 2026 18:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

srkod - estate addon practice#1278

srkod - estate addon practice#1278
sreedevk wants to merge 1 commit into
odoo:19.0from
odoo-dev:19.0-setting-up-real-estate-module-srkod

sreedevk commented May 19, 2026

Uh oh!

robodoo commented May 19, 2026

Uh oh!

Megaaaaaa left a comment

Uh oh!

Uh oh!

Megaaaaaa May 22, 2026

Uh oh!

Megaaaaaa May 22, 2026

Uh oh!

Megaaaaaa May 22, 2026

Uh oh!

Megaaaaaa May 22, 2026

Uh oh!

Megaaaaaa May 22, 2026

Uh oh!

Megaaaaaa May 22, 2026

Uh oh!

Megaaaaaa May 22, 2026

Uh oh!

Megaaaaaa May 22, 2026

Uh oh!

Megaaaaaa May 22, 2026

Uh oh!

Megaaaaaa commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	default=(fields.Date.today() + relativedelta(months=3)), copy=False
	default=(fields.Date.today() + relativedelta(months=3)), copy=False,

	"res.users", string="Sales Person", default=lambda self: self.env.user
	"res.users", string="Sales Person", default=lambda self: self.env.user,

	context="{'list_view_of': 'estate.estate_property_offer_list_form'}"
	context="{'list_view_ref': 'estate.estate_property_offer_list_form'}"

	<field name="property_ids" context="{'list_view_of': 'estate.estate_property_list_form'}" />
	<field name="property_ids" context="{'list_view_ref': 'estate.estate_property_list_form'}" />

Conversation

sreedevk commented May 19, 2026

Uh oh!

robodoo commented May 19, 2026

Uh oh!

Megaaaaaa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Megaaaaaa commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants