ollet - Technical Training by ollet-odoo · Pull Request #1414 · odoo/tutorials

ollet-odoo · 2026-09-15T09:54:24Z

task-6573598

robodoo · 2026-09-15T09:54:37Z

task-6573598

msho-odoo

Good job 🔥

Just for some commit messages, we write it in a way that aligns with when this commit is merged it will ..... so for example estate: Basic security can be add access rules to estate_property or however you would like to describe your change :)
refer to Git Guidlines

I also left you some comments :)
feel free to ping me when you need another review, also always make sure your runbot is green ahead please :)

task-6573598

ollet-odoo · 2026-09-15T14:56:34Z

@msho-odoo Thanks for the review, i have corrected according to your remarks. I don't know if I should mark the conversation as resolved when I fix them or if you will do it when you do the review again

msho-odoo · 2026-09-15T15:13:13Z

@msho-odoo Thanks for the review, i have corrected according to your remarks. I don't know if I should mark the conversation as resolved when I fix them or if you will do it when you do the review again

Usually resolving the comment would depend on the team or the reviewer and what they prefer :)
but here if you're sure you've done the change, feel free to resolve it 👍

msho-odoo

Good work 👍
Just try to use git commit --amend instead of just git commit when fixing few stuff like spaces, names, quotes, etc. because the same commit still applies, no need to add the fixes to a new commit.
Also, left you a few changes.
Keep it up 🔥

msho-odoo · 2026-09-15T15:20:57Z

+    state = fields.Selection(default='new', required=True, copy=False,
+                             selection=[
+                                 ('new', "New"),
+                                 ('offer_received', "Offer Received"),
+                                 ('offer_accepted', "Offer Accepted"),
+                                 ('sold', "Sold"),
+                                 ('cancelled', "Cancelled")
+                             ]
+                            )


Suggested change

state = fields.Selection(default='new', required=True, copy=False,

selection=[

('new', "New"),

('offer_received', "Offer Received"),

('offer_accepted', "Offer Accepted"),

('sold', "Sold"),

('cancelled', "Cancelled")

]

)

state = fields.Selection(

default='new',

required=True,

copy=False,

selection=[

('new', "New"),

('offer_received', "Offer Received"),

('offer_accepted', "Offer Accepted"),

('sold', "Sold"),

('cancelled', "Cancelled")

]

)

msho-odoo · 2026-09-15T15:21:09Z

@@ -0,0 +1,2 @@
+id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
+estate.access_estate_property_user,access_estate_property,model_estate_property,base.group_user,1,1,1,0


EOF line :)

msho-odoo · 2026-09-15T15:23:37Z

@@ -0,0 +1,2 @@
+id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
+estate.access_estate_property_user,access_estate_property,model_estate_property,base.group_user,1,1,1,0


No need to add the estate. before the id, it's implicit since it's in the same module.
I believe you should give unlink access to the user, here you're setting the perm_unlink to 0

I was using the line suggested in the log as a base.
Is it a consistency rule to note have it in it's own module or just a hint you are giving me?

I am just giving you the reasoning why you don't need the <module_name>. before the id in this case but it's not a big thing

msho-odoo · 2026-09-15T15:24:41Z

+    'depends': ['base'],
+    'data': [
+        'security/ir.model.access.csv',
+        'data/estate_property_views.xml',


The views file and the menus file should be in views folder not data folder

ollet-odoo · 2026-09-16T07:59:34Z

robodoo rebase-merge

robodoo · 2026-09-16T07:59:36Z

Merge method set to rebase and merge, using the PR as merge commit message.

Add model, views, security and many2one

add model, views, security and links

msho-odoo

Thank you for the good work 🔥

Left you a couple of comments (some of them are nitpicks so, you can attend to the important one first :))

Also please ping me when you finish a chapter for a review so that changes are not too much to review and I know whether you missed something in the code or you didn't get to that chapter yet:)
but thanks, keep it up 👍

ollet-odoo · 2026-09-17T12:58:43Z

I have finished chapter 11 and the tests are passing if you could please review again.

msho-odoo

Thank you, Good work!
Left you small suggestions and nitpicks for chapter 11, no big thing

msho-odoo · 2026-09-17T13:57:24Z

+            for record in property_id.offer_ids:
+                if record.price > vals['price']:
+                    raise UserError("Cannot create an offer with a lower value than an existing one")


It's correct of course, you can also use max() and mapped(), you will do the same thing in one line and without for loops, if you like

msho-odoo · 2026-09-17T14:00:12Z

+class PropertyType(models.Model):
+    _name = "estate.property.type"
+    _description = "Property Type"
+    _order = "sequence asc"


I think the type also wants order by name

msho-odoo · 2026-09-17T14:08:54Z

+            <list editable="bottom"
+                  decoration-success="status == 'accepted'"
+                  decoration-danger="status == 'refused'"
+            >


We don't like a very long line but this one is fine, it makes it easier to read the file :)

Suggested change

<list editable="bottom"

decoration-success="status == 'accepted'"

decoration-danger="status == 'refused'"

>

<list editable="bottom" decoration-success="status == 'accepted'" decoration-danger="status == 'refused'">

msho-odoo · 2026-09-17T14:14:06Z

+                <field name="validity"/>
+                <field name="date_deadline"/>
+                <button name="action_accept" title="Accept" type="object" icon="fa-check"
+                        invisible="property_id_state in ('offer_accepted', 'sold', 'cancelled')"


surely it's correct, but usually teams avoid adding new fields to the database unless absolutely necessary. It's no big issue here, it's a related field already but thought to mention it
For example here, if you already don't show an offer for cancelled or sold properties, you can just make the invisible condition here be something like state != False

msho-odoo · 2026-09-17T14:24:46Z

+                    <button name="action_do_sold" type="object" class="oe_highlight" string="Sold"
+                            invisible="state in ('sold', 'cancelled', 'new')"/>
+                    <button name="action_do_cancel" type="object" string="Cancel"
+                            invisible="state in ('sold', 'cancelled',)"/>


nitpick

Suggested change

invisible="state in ('sold', 'cancelled',)"/>

invisible="state in ('sold', 'cancelled')"/>

msho-odoo · 2026-09-17T14:29:10Z

+                        <field name="property_type_id" widget="many2one" can_create="false"
+                               can_write="false"
+                               options="{'no_create': true}"


I believe yours is correct, here is also a more used way if you like

Suggested change

<field name="property_type_id" widget="many2one" can_create="false"

can_write="false"

options="{'no_create': true}"

<field name="property_type_id" widget="many2one" options="{'no_create': True, 'no_edit': True}"/>

[ADD] estate: Add real estate module scafolding

b592011

task-6573598

ollet-odoo force-pushed the 19.0-server-tuto-ollet branch from 189c49d to b592011 Compare September 15, 2026 11:32

ollet-odoo changed the title ~~[add] estate: Add real estate module scafolding~~ ollet - Technical Training Sep 15, 2026

[IMP] estate: Add estate.property model

9282f17

task-6573598

ollet-odoo force-pushed the 19.0-server-tuto-ollet branch from 072738c to acc1335 Compare September 15, 2026 13:26

msho-odoo requested changes Sep 15, 2026

View reviewed changes

ollet-odoo added 9 commits September 15, 2026 16:52

[IMP] estate: add first access rule to estate_property

339767a

task-6573598

[IMP] estate: add Menu, form and list view

433fc25

task-6573598

[IMP] estate: Improve Property model

7a55a96

[CLN] estate: Menu, form and list view

703a614

task-6573598

[IMP] estate: Add list view

5ec1f59

[FIX] estate Property set active by default

b365a35

[IMP] estate: Property do not duplicate state

2dd8a20

[IMP] estate: Property form

8badc9b

[CLN] estate: Correct review comments

18d1370

ollet-odoo force-pushed the 19.0-server-tuto-ollet branch from 37f6405 to 18d1370 Compare September 15, 2026 14:53

msho-odoo requested changes Sep 15, 2026

View reviewed changes

ollet-odoo force-pushed the 19.0-server-tuto-ollet branch 2 times, most recently from 8de9400 to c0879b8 Compare September 16, 2026 09:02

ollet-odoo added 6 commits September 16, 2026 21:32

[IMP] estate: display name and state in views

162db65

[IMP] estate: add search, filter and group by on property

0afb8b8

[IMP] estate: add Type for properties

7d4e7ce

Add model, views, security and many2one

[IMP] estate: add buyer and salesperson on Property

a835310

[IMP] estate: Add Property tags

4f8b1c9

add model, views, security and links

[IMP] estate: Add Property Offers

ea8305e

add model, views, security and links

ollet-odoo added 7 commits September 16, 2026 21:32

[IMP] estate: Add living_area on Property

c3845d7

[IMP] estate: Add best_price on Property

e7eabbf

[IMP] estate: add deadline and validity on property

dd0f613

[IMP] estate: set fields default for garden

b24f6c7

[IMP] estate: add cancel and sold button on property

c8db864

[IMP] estate: ability to refuse or refuse offers

774457f

[IMP] estate: add data validation constraints

cf79289

ollet-odoo force-pushed the 19.0-server-tuto-ollet branch from 5d2b876 to cf79289 Compare September 16, 2026 19:36

msho-odoo requested changes Sep 17, 2026

View reviewed changes

ollet-odoo added 3 commits September 17, 2026 11:27

[CLN] estate: attributes order

a7f50eb

[CLN] estate: use double quote everywhere

ef52fd9

[CLN] estate: fix field orders

991989f

ollet-odoo force-pushed the 19.0-server-tuto-ollet branch from e5e4802 to a4907f5 Compare September 17, 2026 11:13

[CLN] estate: Correct review remarks

de922c4

ollet-odoo force-pushed the 19.0-server-tuto-ollet branch 3 times, most recently from 1d537d9 to f7be358 Compare September 17, 2026 11:47

ollet-odoo added 2 commits September 17, 2026 13:47

[IMP] estate: Improve user interface

c0d2543

[IMP] estate: only allow unlinking new or cancelled properties

bf627c7

ollet-odoo requested a review from msho-odoo September 17, 2026 12:58

[IMP] estate: implement chapter8 inheritance

9f5e770

ollet-odoo force-pushed the 19.0-server-tuto-ollet branch from f7be358 to 9f5e770 Compare September 17, 2026 13:44

msho-odoo requested changes Sep 17, 2026

View reviewed changes

		@@ -0,0 +1,2 @@
		id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
		estate.access_estate_property_user,access_estate_property,model_estate_property,base.group_user,1,1,1,0 No newline at end of file

	invisible="state in ('sold', 'cancelled',)"/>
	invisible="state in ('sold', 'cancelled')"/>

Conversation

ollet-odoo commented Sep 15, 2026

Uh oh!

robodoo commented Sep 15, 2026

Uh oh!

msho-odoo left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ollet-odoo commented Sep 15, 2026

Uh oh!

msho-odoo 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!

Choose a reason for hiding this comment

Uh oh!

ollet-odoo commented Sep 16, 2026

Uh oh!

robodoo commented Sep 16, 2026

Uh oh!

msho-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ollet-odoo commented Sep 17, 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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

msho-odoo left a comment •

edited

Loading