Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ __pycache__/
# C extensions
*.so

# IDE
.vscode/
ruff.toml

# Distribution / packaging
Comment on lines +9 to 13
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't be part of your diff 😄

.Python
build/
Expand Down
1 change: 1 addition & 0 deletions estate/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from . import models
16 changes: 16 additions & 0 deletions estate/__manifest__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"name": "Real Estate",
"application": True,
"depends": ["base"],
"author": "Simon Dumoulin",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As an Odoo employee, you can't sign your work in your own name. The author of this module is "Odoo S.A." 😄

"license": "LGPL-3",
"data": [
"security/ir.model.access.csv",
"views/estate_property_views.xml",
"views/estate_property_type_views.xml",
"views/estate_property_tag_views.xml",
"views/estate_property_offer_views.xml",
"views/estate_menus.xml",
"views/res_users_views.xml",
],
}
7 changes: 7 additions & 0 deletions estate/models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from . import (
estate_property,
estate_property_offer,
estate_property_tag,
estate_property_type,
res_users,
)
Comment on lines +1 to +7
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I agree it looks better but we write the from . import x on each line except maybe for test files 😄

Suggested change
from . import (
estate_property,
estate_property_offer,
estate_property_tag,
estate_property_type,
res_users,
)
from . import estate_property
from . import estate_property_offer
from . import estate_property_tag
from . import estate_property_type
from . import res_users

123 changes: 123 additions & 0 deletions estate/models/estate_property.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
from dateutil.relativedelta import relativedelta

from odoo import api, fields, models
from odoo.exceptions import UserError, ValidationError
from odoo.tools.float_utils import float_compare, float_is_zero
Comment on lines +1 to +5
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice ordering



class EstateProperty(models.Model):
_name = "estate.property"
_description = "Estate Property"
_order = "id desc"
Comment on lines +8 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice naming


name = fields.Char(string="Title", required=True)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

And nice indentation, spacing, decision making on when to oneline or multiline a field. I already sense this review is going to go very well 👍

description = fields.Text()
postcode = fields.Char()
date_availability = fields.Date(
string="Available From",
copy=False,
default=fields.Date.today() + relativedelta(days=3),
)
expected_price = fields.Float(required=True)
selling_price = fields.Float(readonly=True, copy=False)
bedrooms = fields.Integer(default=2)
living_area = fields.Integer(string="Living Area (sqm)")
facades = fields.Integer()
garages = fields.Boolean()
garden = fields.Boolean()
garden_area = fields.Integer(string="Garden Area (sqm)")
garden_orientation = fields.Selection(
selection=[
("north", "North"),
("south", "South"),
("east", "East"),
("west", "West"),
]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The trailing coma so that people can add attributes to the field without modifying your lines 👍

Suggested change
]
],

)
active = fields.Boolean(default=True)
state = fields.Selection(
readonly=False,
selection=[
("new", "New"),
("offer_received", "Offer Received"),
("offer_accepted", "Offer Accepted"),
("sold", "Sold"),
("canceled", "Canceled"),
],
string="Status",
default="new",
required=True,
)
type_id = fields.Many2one("estate.property.type", string="Property Type")
buyer_id = fields.Many2one("res.partner", string="Buyer", copy=False, readonly=True)
salesperson_id = fields.Many2one(
"res.users", string="Salesman", default=lambda self: self.env.user
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
"res.users", string="Salesman", default=lambda self: self.env.user
"res.users", string="Salesman", default=lambda self: self.env.user,

)
tag_ids = fields.Many2many("estate.property.tag", string="Tags")
offer_ids = fields.One2many("estate.property.offer", "property_id", string="Offers")
total_area = fields.Integer(compute="_compute_total_area")
best_price = fields.Float(compute="_compute_best_price")

_check_expected_price = models.Constraint(
"CHECK(expected_price > 0)",
"A property expected price should be strictly positive.",
)

_check_selling_price = models.Constraint(
"CHECK(selling_price >= 0)", "A property selling price should be positive."
)

@api.depends("living_area", "garden_area")
def _compute_total_area(self):
for record in self:
record.total_area = record.living_area + record.garden_area

@api.depends("offer_ids.price")
def _compute_best_price(self):
best = max(self.offer_ids.mapped("price")) if self.offer_ids else 0
for record in self:
record.best_price = best
Comment on lines +77 to +79
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Becareful here that it's going to behave weirdly when self containts multiple records as you are setting on each property the best price to the value of the best offer amongst ALL the offers of ALL the properties.

Suggested change
best = max(self.offer_ids.mapped("price")) if self.offer_ids else 0
for record in self:
record.best_price = best
for record in self:
record.best_price = max(record.offer_ids.mapped("price")) if record.offer_ids else 0


@api.constrains("selling_price", "expected_price")
def _check_selling_price(self):
for record in self:
if not float_is_zero(record.selling_price, precision_digits=2):
if (
float_compare(
record.selling_price,
0.9 * record.expected_price,
precision_digits=2,
)
== -1
):
Comment on lines +85 to +92
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just a tiny bit more readable imo. Up to you 👍

Suggested change
if (
float_compare(
record.selling_price,
0.9 * record.expected_price,
precision_digits=2,
)
== -1
):
if float_compare(
record.selling_price,
0.9 * record.expected_price,
precision_digits=2,
) < 0:

raise ValidationError(
"The selling price must be at least 90% of the expected price"
)
Comment on lines +93 to +95
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Any string that will be shown to the user needs to be translatable as we have clients all around the world. For that we use the odoo._ method which marks a string as requiring translations for our translators.

Suggested change
raise ValidationError(
"The selling price must be at least 90% of the expected price"
)
# Needs `from odoo import _`
raise ValidationError(_("The selling price must be at least 90% of the expected price"))


@api.onchange("garden")
def _onchange_garden(self):
if self.garden:
self.garden_area = 10
self.garden_orientation = "north"
else:
self.garden_area = 0
self.garden_orientation = None

@api.ondelete(at_uninstall=False)
def _unlink_except_new_or_canceled(self):
if any(record.state == "new" or record.state == "canceled" for record in self):
raise UserError("Can't delete a new or canceled property.")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

UserError -> translations


def action_set_sold(self):
for record in self:
if record.state == "canceled":
raise UserError("Canceled properties cannont be sold.")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Translation

record.state = "sold"
return True

def action_set_canceled(self):
for record in self:
if record.state == "sold":
raise UserError("Sold properties cannont be canceled.")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Translation

record.state = "canceled"
return True
82 changes: 82 additions & 0 deletions estate/models/estate_property_offer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
from dateutil.relativedelta import relativedelta

from odoo import api, fields, models
from odoo.exceptions import UserError


class EstatePropertyOffer(models.Model):
_name = "estate.property.offer"
_description = "Estate Property Offer"
_order = "price desc"

price = fields.Float()
status = fields.Selection(
selection=[("accepted", "Accepted"), ("refused", "Refused")], copy=False
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
selection=[("accepted", "Accepted"), ("refused", "Refused")], copy=False
selection=[("accepted", "Accepted"), ("refused", "Refused")], copy=False,

)
partner_id = fields.Many2one("res.partner", required=True)
property_id = fields.Many2one("estate.property", required=True)
validity = fields.Integer(default=7, string="Validity (days)")
date_deadline = fields.Date(
compute="_compute_date_deadline",
inverse="_inverse_date_deadline",
string="Deadline",
)
property_type_id = fields.Many2one(
string="Property Type", related="property_id.type_id", store=True
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
string="Property Type", related="property_id.type_id", store=True
string="Property Type", related="property_id.type_id", store=True,

)

_check_price = models.Constraint(
"CHECK(price > 0)", "An offer price should be strictly positive."
)

@api.depends("create_date", "validity")
def _compute_date_deadline(self):
for record in self:
base_date = (
fields.Date.to_date(record.create_date)
if record.create_date
else fields.Date.today()
)
record.date_deadline = base_date + relativedelta(days=record.validity)
Comment on lines +35 to +40
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why not just:

Suggested change
base_date = (
fields.Date.to_date(record.create_date)
if record.create_date
else fields.Date.today()
)
record.date_deadline = base_date + relativedelta(days=record.validity)
record.date_deadline = (record.create_date or fields.Date.today()) + relativedelta(days=record.validity)


def _inverse_date_deadline(self):
for record in self:
base_date = (
fields.Date.to_date(record.create_date)
if record.create_date
else fields.Date.today()
)
delta = record.date_deadline - base_date
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same here just go for create_date or today, makes it much cleaner 👍

record.validity = delta.days

@api.model
def create(self, vals_list):
for vals in vals_list:
prop = self.env["estate.property"].browse(vals["property_id"])
best_price = prop.best_price
if vals["price"] < best_price:
raise UserError(
"You can't create an offer with a lower amount than an existing offer."
)
prop.state = "offer_received"
return super().create(vals_list)
Comment on lines +53 to +62
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ok so multiple important things here:

The first one that got me started is that you do a .browse() inside a for loop. Methods that read the database like .search(), .browse(), etc have a very high impact on performance. Not because of the fetching itself but more because of the entire process around it so if you can do your fetch in a single request BEFORE looping, you gain a ton of performances.

Then, you have a UserError so you need to mark it as translatable.

And finally, don't be shy with variable names. Use full words so that it's very clear what everything is even a thousand lines after a declaration. No need to scroll.

Suggested change
def create(self, vals_list):
for vals in vals_list:
prop = self.env["estate.property"].browse(vals["property_id"])
best_price = prop.best_price
if vals["price"] < best_price:
raise UserError(
"You can't create an offer with a lower amount than an existing offer."
)
prop.state = "offer_received"
return super().create(vals_list)
def create(self, vals_list):
properties = self.env["estate.property"].browse([vals["property_id"] for vals in vals_list]) # Might need a `if vals.get("property_id")
properties_by_id = {prop.id: prop for prop in properties}
for vals in vals_list:
property = properties_by_id.get(vals["property_id"])
best_price = property.best_price
if vals["price"] < best_price:
raise UserError(_("You can't create an offer with a lower amount than an existing offer."))
property.state = "offer_received"
return super().create(vals_list)


def action_set_accepted(self):
states = self.property_id.offer_ids.mapped("status")
for record in self:
if "accepted" in states:
raise UserError("Une offre a déjà été acceptée.")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

UserError so must be translatable AND you cannot write in French. Every single word you write in the code must be in english (if odoo controlled, some api's we call have their own naming convention, don't alias those 😄)

Suggested change
raise UserError("Une offre a déjà été acceptée.")
raise UserError(_("An offer has already been accepted."))

record.status = "accepted"
record.property_id.buyer_id = record.partner_id
record.property_id.selling_price = record.price
record.property_id.state = "offer_accepted"
return True

def action_set_refused(self):
for record in self:
previous_status = record.status
record.status = "refused"
if previous_status == "accepted":
record.property_id.buyer_id = None
record.property_id.selling_price = 0
Comment on lines +79 to +81
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Even if the previous state was not accepted, now that it's refused, you want no buyer_id and no selling_price, no ?

Suggested change
if previous_status == "accepted":
record.property_id.buyer_id = None
record.property_id.selling_price = 0
record.property_id.buyer_id = None
record.property_id.selling_price = 0

return True
14 changes: 14 additions & 0 deletions estate/models/estate_property_tag.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from odoo import fields, models


class EstatePropertyTag(models.Model):
_name = "estate.property.tag"
_description = "Estate Property Tag"
_order = "name"

name = fields.Char(required=True)
color = fields.Integer()

_check_unique_name = models.Constraint(
"UNIQUE(name)", "A property tag name should be unique."
)
25 changes: 25 additions & 0 deletions estate/models/estate_property_type.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
from odoo import api, fields, models


class EstatePropertyType(models.Model):
_name = "estate.property.type"
_description = "Estate Property Type"
_order = "sequence,name"

name = fields.Char(required=True)
property_ids = fields.One2many("estate.property", "type_id", string="Properties")
sequence = fields.Integer()
offer_ids = fields.One2many(
"estate.property.offer", "property_type_id", string="Offers"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
"estate.property.offer", "property_type_id", string="Offers"
"estate.property.offer", "property_type_id", string="Offers",

)
offer_count = fields.Integer(compute="_compute_offer_count")

_check_unique_name = models.Constraint(
"UNIQUE(name)", "A property tag name should be unique."
)

@api.depends("offer_ids")
def _compute_offer_count(self):
for record in self:
record.offer_count = len(record.offer_ids)
return True
12 changes: 12 additions & 0 deletions estate/models/res_users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from odoo import fields, models


class ResUsers(models.Model):
_inherit = "res.users"

property_ids = fields.One2many(
"estate.property",
"salesperson_id",
domain="['|',('state', '=', 'new'),('state', '=', 'offer_received')]",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No space around operators but definitely a space after commas 😄

Suggested change
domain="['|',('state', '=', 'new'),('state', '=', 'offer_received')]",
domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]",

string="Real Estate Properties",
)
5 changes: 5 additions & 0 deletions estate/security/ir.model.access.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
id,name,model_id/id,group_id/id,perm_read,perm_write,perm_create,perm_unlink
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
12 changes: 12 additions & 0 deletions estate/views/estate_menus.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0"?>
<odoo>
<menuitem id="estate_property_menu_root" name="Real Estate">
<menuitem id="estate_property_menu" name="Advertisements">
<menuitem id="estate_property_menu_action" action="estate_property_action"/>
</menuitem>
<menuitem id="estate_settings_menu" name="Settings">
<menuitem id="estate_property_type_menu_action" action="estate_property_type_action"/>
<menuitem id="estate_property_tag_menu_action" action="estate_property_tag_action"/>
</menuitem>
</menuitem>
</odoo>
42 changes: 42 additions & 0 deletions estate/views/estate_property_offer_views.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?xml version="1.0"?>
<odoo>
<record id="estate_property_offer_view_form" model="ir.ui.view">
<field name="name">estate.property.offer.form</field>
<field name="model">estate.property.offer</field>
<field name="arch" type="xml">
<form string="Property Type">
<sheet>
<group>
<field name="price"/>
<field name="partner_id"/>
<field name="validity"/>
<field name="date_deadline"/>
<field name="status"/>
</group>
</sheet>
</form>
</field>
</record>

<record id="estate_property_offer_view_tree" model="ir.ui.view">
<field name="name">estate.property.offer.list</field>
<field name="model">estate.property.offer</field>
<field name="arch" type="xml">
<list string="Offer" editable="top" decoration-danger="status == 'refused'" decoration-success="status == 'accepted'">
<field name="price"/>
<field name="partner_id"/>
<field name="validity"/>
<field name="date_deadline"/>
<button name="action_set_accepted" string="Accept" type="object" icon="fa-check" invisible="status"/>
<button name="action_set_refused" string="Refuse" type="object" icon="fa-times" invisible="status"/>
</list>
</field>
</record>

<record id="estate_property_offer_action" model="ir.actions.act_window">
<field name="name">Property Offers</field>
<field name="res_model">estate.property.offer</field>
<field name="view_mode">list,form</field>
<field name="domain">[('property_type_id', '=', active_id)]</field>
</record>
</odoo>
35 changes: 35 additions & 0 deletions estate/views/estate_property_tag_views.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?xml version="1.0"?>
<odoo>
<record id="estate_property_tag_view_form" model="ir.ui.view">
<field name="name">estate.property.tag.form</field>
<field name="model">estate.property.tag</field>
<field name="arch" type="xml">
<form string="Property Tag">
<sheet>
<div class="oe_title">
<h1><field name="name"/></h1>
</div>
<group>
<field name="color" widget="color_picker"/>
</group>
</sheet>
</form>
</field>
</record>

<record id="estate_property_tag_view_tree" model="ir.ui.view">
<field name="name">estate.property.tag.list</field>
<field name="model">estate.property.tag</field>
<field name="arch" type="xml">
<list string="Tag" editable="top">
<field name="name"/>
</list>
</field>
</record>

<record id="estate_property_tag_action" model="ir.actions.act_window">
<field name="name">Property Tags</field>
<field name="res_model">estate.property.tag</field>
<field name="view_mode">list,form</field>
</record>
</odoo>
Loading