Technical Training - alnav by Kermut572 · Pull Request #1413 · odoo/tutorials

Kermut572 · 2026-09-15T09:51:19Z

Creation of the required manifest and init files for the real estate module.

robodoo · 2026-09-15T09:51:23Z

msho-odoo

Great work!

Left you a couple of comments.
And for the commit message, it usually should align with if this commit is merged it will .... so you can write something like [ADD] estate: add estate property module or whatever message you think is better :)
refer to the Git Guidelines

msho-odoo · 2026-09-15T11:49:02Z

+    'data': [],
+    # data files containing optionally loaded demonstration data
+    'demo': [],
+}


Please add a new line at the end of files :)
Also always refer to your runbot to address the styling errors ans the warnings

msho-odoo · 2026-09-15T11:49:42Z

+    # data files always loaded at installation
+    'data': [],
+    # data files containing optionally loaded demonstration data
+    'demo': [],


I guess no need to add them if they are not used (unless it gave you warnings on the log?)

msho-odoo · 2026-09-15T11:50:25Z

+    'name': "Real Estate",
+    'version': '1.0',
+    'depends': ['base'],
+    'author': "alnav",


usually author is Odoo S.A. :)

msho-odoo

Nice work, left a couple of tiny comments

When you try to fix some code after reviewing, there is no need to create a new commit just for the fix, it makes the history unnecessarily crowded, try using git commit --amend instead of git commit, this command lets you edit the last commit and also pushes your current changes to it without making a new one :)

msho-odoo · 2026-09-15T12:15:55Z

+    bedrooms = fields.Integer("Number of Bedrooms")
+    living_area = fields.Integer("Living Area")
+    facades = fields.Integer("Number of Facades")
+    garage = fields.Boolean("Has a Garage")


good practice for boolean fields to be is_something or has_something so when someone reads the name in the middle of the code know it's a boolean, so you might make it has_garden and has_garage

msho-odoo · 2026-09-15T12:32:43Z

+    garage = fields.Boolean("Has a Garage")
+    garden = fields.Boolean("Has a Garden")
+    garden_area = fields.Integer("Garden Area")
+    garden_orientation = fields.Selection(string="Garden Orientation", selection=[('north', "North"), ("south", "South"), ("east", "East"), ("west", "West")])


Suggested change

garden_orientation = fields.Selection(string="Garden Orientation", selection=[('north', "North"), ("south", "South"), ("east", "East"), ("west", "West")])

garden_orientation = fields.Selection(

string="Garden Orientation",

selection=[

("north", "North"),

("south", "South"),

("east", "East"),

("west", "West"),

]

)

this is a better and conventional styling at odoo for bracketed objects if you would say :)
nitpick regarding the quotes: some teams might stick to single quotes being only on technical strings (the ones that the user don't see) and double quotes are for the strings that the user can see)
so for example the selection can be ('east', "East") instead of ('east', 'East') but some teams don't do this, they just stick to their own convention which is all single quotes (if possible) or all double quotes so you can pick a convention and stick to it for now :)

msho-odoo · 2026-09-15T14:36:15Z

@Kermut572 just forgot to tell you, you can mention me here when you think the PR is ready for another review, typically, after each chapter :)

Kermut572 · 2026-09-15T14:45:43Z

@Kermut572 just forgot to tell you, you can mention me here when you think the PR is ready for another review, typically, after each chapter :)

Alright I'll keep that in mind, thanks!

Kermut572 · 2026-09-15T14:55:56Z

@msho-odoo just finished chapter 5, PR is ready for review :)

msho-odoo

Nice work 💯
left you some comments

nitpick about the commit message, I would make it add instead of added just to align with guidelines because you would say This commit will add property model actions... not This commit will added property model actions but it's not a big thing, well done.
Also have a look at the runbot, see why ci/tutorials is giving you a warning ant try to solve it.
Keep up the good work 🔥

msho-odoo · 2026-09-15T15:05:22Z

+    descritpion = fields.Text("Property Description")
+    postcode = fields.Char("Postcode")
+
+    date_availability = fields.Date("Available Date", copy=False, default=fields.Date.add(fields.Date.today(), months=3))


Using default=fields.Date.add(fields.Date.today(), months=3) makes this only get valued first time it's imported and it's stuck there, you need to use lambda function instead, you will find multiple examples in the codebase, it's good practice to search for previous code there as well :)

msho-odoo · 2026-09-15T15:06:51Z

+<?xml version="1.0"?>
+<odoo>
+    <menuitem id="estate_menu_root" name="Real Estate">
+        <menuitem id="estate_menu_first_level" name="Advertisements">


It's better to give your ids a more descriptive name so that it's are understood when it's read somewhere in the code without needing to go back to its definition

msho-odoo · 2026-09-15T15:07:30Z

+    "application": True,
+    "data": [
+        "security/ir.model.access.csv",
+


why the extra line :)

In the account_accountant enterprise module they have something like this
`'data/account_accountant_tour.xml',
'data/ir_config_parameters.xml',

'security/ir.model.access.csv',
'security/account_accountant_security.xml',

'views/account_account_views.xml',
'views/account_fiscal_year_view.xml',
...`

I thought this was the standard way to format it, but looking at the other modules it seems to be more of an exception than a rule so I'll change it :D

okaay, maybe some teams stick to that if you saw it somewhere :)
but I think mostly teams don't :)

Kermut572 · 2026-09-16T11:11:26Z

@msho-odoo chapter 6 ready for review :)

msho-odoo

Nice work 👍
Just for the commit message we use [ADD] tag when adding a brand new module in the commit but adding some fields, views or logic/feature we use [IMP] which you will be using most of the time as we don't add a new module everyday at odoo :) but we do a lot of improvements
I also left you some comments but they are not big, just some nitpicks.
Good job 🔥

msho-odoo · 2026-09-16T11:47:41Z

@@ -0,0 +1,50 @@
+from datetime import datetime
+


No need to leave empty lines after each import line.
If you must, you can leave a line between the external import lines and the odoo import line, but that's just a nitpick from me

msho-odoo · 2026-09-16T11:51:06Z

+<?xml version="1.0"?>
+<odoo>
+    <menuitem id="estate_main_menu_button" name="Real_Estate">
+        <menuitem id="estate_app_top_bar_property_model_button" name="Advertisements">


I wouldn't add _button at the end of the id of the menuitem, it could get misunderstood with a real action button, I would stick to the naming guidelines here

msho-odoo · 2026-09-16T11:54:14Z

+                <field name="facades"/>
+                <separator/>
+                <filter string="Available" name="active" domain="[('active', '=', True)]"/>
+                <filter string="New" name="active" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>


Usually, we write the name attribute before the string :)
Your domain is correct, but I think this is more readable:

Suggested change

<filter string="New" name="active" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>

<filter name="active" string="New" domain="[('state', 'in', ('new', 'offer_received'))]"/>

msho-odoo · 2026-09-16T11:58:12Z

+        </field>
+    </record>
+
+    <record id="estate_property_model_list_view" model="ir.ui.view">


Suggested change

<record id="estate_property_model_list_view" model="ir.ui.view">

<record id="estate_property_view_list" model="ir.ui.view">

just to align with the naming guidelines
You can have a quick check for the id names in the files in general

msho-odoo · 2026-09-16T11:59:43Z

+        <field name="arch" type="xml">
+            <form string="Property">
+                <sheet>
+                    <field name="name" style="font-size:20pt;"/>


in-line styling is not encouraged, you can use the tag <h1> </h1> instead for this case or the ready to use classes (you can check some views in the codebase if you like)

[ADD] Real estate module base files

f28308d

Kermut572 changed the title ~~[ADD] Real estate module base files~~ Technical Training - alnav Sep 15, 2026

msho-odoo requested changes Sep 15, 2026

View reviewed changes

msho-odoo marked this pull request as ready for review September 15, 2026 12:03

[FIX] estate: fixed manifest file and added newlines

cdd3405

msho-odoo requested changes Sep 15, 2026

View reviewed changes

[ADD] estate: created Property model

c5c3d74

Kermut572 force-pushed the 19.0-tutorials-alnav branch 2 times, most recently from 4982cd4 to c5c3d74 Compare September 15, 2026 12:52

[ADD] estate: added access rights to Property model

5e35e5e

Kermut572 force-pushed the 19.0-tutorials-alnav branch from 6f2f43f to 5e35e5e Compare September 15, 2026 13:28

msho-odoo requested changes Sep 15, 2026

View reviewed changes

Kermut572 force-pushed the 19.0-tutorials-alnav branch from 5bc9a6a to f2182a6 Compare September 16, 2026 07:52

[ADD] estate: added Property model action and menus

8142e1b

Kermut572 force-pushed the 19.0-tutorials-alnav branch from f2182a6 to 8142e1b Compare September 16, 2026 07:58

[ADD] add custom views for search, list and form views

2dde396

msho-odoo requested changes Sep 16, 2026

View reviewed changes

	<filter string="New" name="active" domain="['\|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>
	<filter name="active" string="New" domain="[('state', 'in', ('new', 'offer_received'))]"/>

	<record id="estate_property_model_list_view" model="ir.ui.view">
	<record id="estate_property_view_list" model="ir.ui.view">

Uh oh!

Conversation

Kermut572 commented Sep 15, 2026

Uh oh!

robodoo commented Sep 15, 2026

Uh oh!

msho-odoo left a comment

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!

msho-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

msho-odoo Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msho-odoo Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msho-odoo commented Sep 15, 2026

Uh oh!

Kermut572 commented Sep 15, 2026

Uh oh!

Kermut572 commented Sep 15, 2026

Uh oh!

msho-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Kermut572 commented Sep 16, 2026

Uh oh!

msho-odoo left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

msho-odoo Sep 15, 2026 •

edited

Loading

msho-odoo Sep 15, 2026 •

edited

Loading