Skip to content

Conversation

@ybenbrai
Copy link

No description provided.

@robodoo
Copy link

robodoo commented Nov 18, 2025

Pull request status dashboard

@tskv tskv closed this Nov 18, 2025
@tskv tskv deleted the 19.0-tutorial-yoben branch November 18, 2025 13:06
@tskv tskv restored the 19.0-tutorial-yoben branch November 18, 2025 13:06
@ybenbrai ybenbrai reopened this Nov 18, 2025
@ybenbrai ybenbrai self-assigned this Nov 18, 2025
Copy link
Contributor

@clbr-odoo clbr-odoo left a comment

Choose a reason for hiding this comment

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

Good job so far, just a few comments !

Your commit title are not fully correct: usually we put the [ADD] tag on the module init (where the manifest is). Following commits will be [IMP](provements) to the now existing module.
See git guidelines for more details.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in a folder named security by convention, not data.

from odoo import fields, models


class Property(models.Model):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
class Property(models.Model):
class EstateProperty(models.Model):

There is no big incidence here but we generally put the name of the file in CamelCase here.
It's usually good to be specific. Property is super generic and could be existing in other modules.


class Property(models.Model):
_name = "estate.property"
_description = "This is a estate property"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_description = "This is a estate property"
_description = "Estate property"

class Property(models.Model):
_name = "estate.property"
_description = "This is a estate property"
_order = "sequence"
Copy link
Contributor

Choose a reason for hiding this comment

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

That _order will define how the search will return you the records.
You don't have a sequence field on your estate.property model, leave it empty to keep the default (that order by ID) for now.

_description = "This is a estate property"
_order = "sequence"

name = fields.Char('Name', required=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name = fields.Char('Name', required=True)
name = fields.Char(string='Name', required=True)

Note that what you did is totally equivalent to this ☝️ . Some reviewers prefer that we name all parameters explicitly.
Also note that by default, Odoo will use the variable name to create a label if not provided.
In this case it will create "Name", so you don't really need to specify it.

Copy link
Author

Choose a reason for hiding this comment

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

Note that what you did is totally equivalent to this ☝️ . Some reviewers prefer that we name all parameters explicitly. Also note that by default, Odoo will use the variable name to create a label if not provided. In this case it will create "Name", so you don't really need to specify it.

okay noted !, i changed it to Title in local now to match the screenshots

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. You don't need to have an __init__ file in your view folder as it does not contain Python files.
  2. By convention, that folder should not be view but views. Please double-check the coding guidelines for reference. Also don't hesitate to start to explore Odoo codebase and copy how it's done 🙂

action="estate_property_action"
parent="estate_menu_advertisements"/>

</odoo> No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing EOL at the end of the file.

Comment on lines 13 to 14
'data/ir.model.access.csv',
'view/estate_property_views.xml',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'data/ir.model.access.csv',
'view/estate_property_views.xml',
'security/ir.model.access.csv',
'views/estate_property_views.xml',

'summary': 'Track Real Estate',
'website': 'https://www.odoo.com/app/realestate',
'author': 'Odoo S.A.',
'license': 'LGPL-3',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'license': 'LGPL-3',
'application': True,
'license': 'LGPL-3',

You probably want to have application=True for this module. See the manifest reference.

<field name="view_mode">tree,form</field>
</record>

<menuitem id="estate_menu_root" name="Real Estate"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that you can also nest the module to avoid defining the parent, like this:

<menuitem ...>
    <menuitem .../>
</menuitem/>

youness benbraitit (yoben) added 5 commits November 19, 2025 10:30
added more fields to the database table with needed requirements
added access rights
	fixed the issues on linting
@ybenbrai ybenbrai force-pushed the 19.0-tutorial-yoben branch from 06e1b31 to 8a7f9f0 Compare November 19, 2025 09:30
youness benbraitit (yoben) added 5 commits November 19, 2025 11:26
added improvement to the properties
added search / filter / form / header
	fixed the issue for the styles and the tutorial syntax
	added tags types.. and accesses and infos in the form
@ybenbrai ybenbrai requested a review from clbr-odoo November 21, 2025 10:38
youness benbraitit (yoben) added 4 commits November 21, 2025 11:48
	Chapter 8: Computed Fields And Onchanges
	count the oriontation depend of criteria using action
	added requirement of chapter 9
Copy link
Contributor

@clbr-odoo clbr-odoo left a comment

Choose a reason for hiding this comment

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

Don't forget to fix all missing EOL, keep up 👍

access_estate_property,access.estate.property,model_estate_property,base.group_user,1,1,1,1
access_estate_property_type,access.estate.property.type,model_estate_property_type,base.group_user,1,1,1,1
access_estate_property_tag,access.estate.property.tag,model_estate_property_tag,base.group_user,1,1,1,1
access_estate_property_offer,access.estate.property.offer,model_estate_property_offer,base.group_user,1,1,1,1 No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

EOL missing at the end of the file.

</list>
</field>
</record>
</odoo> No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

EOL

<menuitem id="estate_property_tag_menu_action" name="Property Tags" action="estate_property_tag_action" />
</menuitem>
</menuitem>
</odoo> No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

EOL

<field name="res_model">estate.property.offer</field>
<field name="view_mode">tree,form</field>
</record>
</odoo> No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

EOL

<field name="view_mode">list,form</field>
</record>

</odoo> No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

EOL

Comment on lines +64 to +67
@api.depends('living_area', 'garden_area')
def _compute_total_area(self):
for rec in self:
rec.total_area = (rec.living_area or 0) + (rec.garden_area or 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This compute is defined twice (see line 76).

self.garden_orientation = False

def action_sold(self):
if "cancelled" in self.mapped("state"):
Copy link
Contributor

Choose a reason for hiding this comment

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

It works but it's not super readable. I'd do:

Suggested change
if "cancelled" in self.mapped("state"):
if any([prop.state == "cancelled" for prop in self]):

def action_sold(self):
if "cancelled" in self.mapped("state"):
raise UserError("Canceled property cannot be sold !")
return self.write({"state": "sold"})
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return self.write({"state": "sold"})
self.state = 'sold'
return True

write doesn't return anything, and this notation also works, even if the recordset has many records.

@api.depends('validity')
def _compute_date_deadline(self):
for offer in self:
if offer._origin.validity:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if offer._origin.validity:
if offer.validity:

validity will always be set, worst case to 0 so this if...else can be avoided.

Comment on lines +36 to +38
offer.property_id.selling_price = offer.price
offer.property_id.buyer_id = offer.partner_id.id
offer.property_id.state = 'offer_accepted'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
offer.property_id.selling_price = offer.price
offer.property_id.buyer_id = offer.partner_id.id
offer.property_id.state = 'offer_accepted'
offer.property_id.write({
'selling_price': offer.price,
'buyer_id': offer.partner_id.id,
'state': 'offer_accepted',
})

It's better to do those three "write" in one :)

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