Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Odoo tutorial - SERU branch #626

Open
wants to merge 4 commits into
base: 18.0
Choose a base branch
from
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
1 change: 1 addition & 0 deletions awesome_clicker/__manifest__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-
# some not so great changes to make a first commit
Copy link

Choose a reason for hiding this comment

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

remove that

{
'name': "Awesome Clicker",

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
Copy link

Choose a reason for hiding this comment

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

Commit message:
if you use [ADD], which is the right thing to do in this case, you already say that you add a module, so no need to make it your commit title. Explain what the module does in few words, beginning by a verb.
"[ADD] estate: handle real estate properties and salesflow"
Then in description, you can describe what features and capabilities the module have
and even list all the models used to handle the business cases. What do what and how they interact.
Basically, when reading commit message, you don't need to read the code to know how the module works. Or reading commit message will help you understand the code.
Better reading 10 lines of real words than 250 lines of code.
The goal of the commit message is mainly to help future debugging or refactoring or functionality rework, when you try to find why a certain line has been added to the code base, you read the commit message to get the context in which the commit content has been added. Knowledge is power. Better too much info than not enough :)

19 changes: 19 additions & 0 deletions estate/__manifest__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"name": "Real Estate",
"category": "Real Estate",
Copy link

Choose a reason for hiding this comment

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

missing verison and description

"application": True,
"installable": True,
"depends":[
"base",
"web"
],
"data": [
"security/ir.model.access.csv",
"views/estate_menus.xml",
"views/estate_property_views.xml",
"views/estate_property_offer_views.xml",
"views/estate_property_type_views.xml",
"views/estate_property_tag_views.xml",
"views/res_users_views.xml",
]
}
Copy link

Choose a reason for hiding this comment

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

check for all end of file missing empty lines

5 changes: 5 additions & 0 deletions estate/models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from . import estate_property
from . import estate_property_type
from . import estate_property_tag
from . import estate_property_offer
from . import res_users
110 changes: 110 additions & 0 deletions estate/models/estate_property.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
from odoo import api, fields, models, exceptions, tools


class Property(models.Model):
_name = "estate.property"
_description = "Real Estate property"
_order = "sequence, id desc"

sequence = fields.Integer("Sequence", default=1)
name = fields.Char("Title", required=True)
description = fields.Text("Description")
postcode = fields.Char("Postcode")
date_availability = fields.Date(
"Available Date", copy=False, default=fields.Date.add(fields.Date.today(), months=3))
expected_price = fields.Float("Expected price", required=True)
selling_price = fields.Float("Selling price", readonly=True, copy=False)
bedrooms = fields.Integer("Bedrooms", default=2)
living_area = fields.Integer("Living area (sqm)")
facades = fields.Integer("Facades")
Copy link

Choose a reason for hiding this comment

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

usually when a field is basically a counter, you postfix with '_count'
because 'facade' only is not specific enough

garage = fields.Boolean("Garage")
garden = fields.Boolean("Garden")
garden_area = fields.Integer("Garden area (sqm)")
garden_orientation = fields.Selection([
("north", "North"),
("south", "South"),
("east", "East"),
("west", "West")
], string="Garden Orientation")
active = fields.Boolean("Active", default=True)
Copy link

Choose a reason for hiding this comment

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

try to sort fields in alphabetical order
keeping active and name as first (as we usually place them at the top)

state = fields.Selection([
("new", "New"),
("offer_received", "Offer Received"),
("offer_accepted", "Offer Accepted"),
("sold", "Sold"),
("canceled", "Canceled")
], string="Status", required=True, copy=False, default="new")

_sql_constraints = [
("check_expected_price", "CHECK(expected_price > 0)",
"Expected price must be strictly positive."),
("check_selling_price", "CHECK(selling_price >= 0)",
"Selling Price must be positive.")
]
Comment on lines +38 to +43
Copy link

Choose a reason for hiding this comment

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

put this after field definition


property_type_id = fields.Many2one(
"estate.property.type", string="Property Type")
salesman_id = fields.Many2one(
"res.users", string="Salesman", default=lambda self: self.env.user)
buyer_id = fields.Many2one("res.partner", string="Buyer", copy=False)
tag_ids = fields.Many2many("estate.property.tag", string="Tags")
offer_ids = fields.One2many(
"estate.property.offer", "property_id", string="Offers")

total_area = fields.Float("Total area (sqm)", compute="_compute_area")
best_price = fields.Float("Best price", compute="_compute_best_price")

Copy link

Choose a reason for hiding this comment

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

you can know remove those conflict commits

@api.depends("living_area", "garden_area")
def _compute_area(self):
for record in self:
Copy link

Choose a reason for hiding this comment

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

we try to be more explicit. record is too generic.
Here we are manipulating properties, so name it : for property in self.
apply this logic everywhere

record.total_area = record.living_area + record.garden_area

@api.depends("offer_ids.price")
def _compute_best_price(self):
for record in self:
record.best_price = max([0] + record.offer_ids.mapped('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 = ""
Comment on lines +69 to +74
Copy link

Choose a reason for hiding this comment

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

use ternary assignment.
self.garden will be cached anyway. so testing it twice is not a big deal
You will gain some lines


def action_set_sold(self):
for record in self:
if record.state != "canceled":
record.state = "sold"
else:
raise exceptions.UserError(
"Canceled properties cannot be sold.")

def action_set_canceled(self):
for record in self:
if record.state != "sold":
record.state = "canceled"
else:
raise exceptions.UserError(
"Sold properties cannot be canceled.")
Comment on lines +76 to +90
Copy link

Choose a reason for hiding this comment

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

this should be done in constrain.
Otherwise, if setting the property as "canceled" by RPC or by any other means possible, it won't raise an error. If we want to keep data consistency, rules likes that should be general and not only when calling actions.
In _check (constrain) methods, you can use property._origin to get the old value.


def hello_seru(self):
print("I'm a conflict in your branch")
def handle_conflict(self):
print("Hello! I handled a conflict.")

@api.constrains("expected_price", "selling_price")
def _check_price(self):
for record in self:
if record.selling_price and record.expected_price and not tools.float_utils.float_is_zero(record.selling_price, precision_digits=2):
Copy link

Choose a reason for hiding this comment

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

if line is too long, split line for each condition

if record.selling_price < 0.9 * record.expected_price:
Copy link

Choose a reason for hiding this comment

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

as you compare float, could be cool to use float_compare (from odoo tools)

raise exceptions.ValidationError(
"The selling price cannot be lower than 90% of the expected price.")

@api.ondelete(at_uninstall=False)
def _not_delete_if_not_new_or_canceled(self):
Copy link

Choose a reason for hiding this comment

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

on delete methods are usually named _unlink_except_something

for record in self:
if record.state not in ["new", "canceled"]:
raise exceptions.UserError(
"You cannot delete a property that is not new.")
65 changes: 65 additions & 0 deletions estate/models/estate_property_offer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
from odoo import api, models, fields, exceptions


Copy link

Choose a reason for hiding this comment

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

2 empty lines before class
apply everywhere

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

price = fields.Float("Price")
status = fields.Selection([
("accepted", "Accepted"),
("refused", "Refused")],
string="Status", copy=False)
partner_id = fields.Many2one("res.partner", string="Partner", required=True)
property_id = fields.Many2one("estate.property", string="Property", required=True)
validity = fields.Integer("Validity (days)", default=7)
date_deadline = fields.Date("Deadline", compute="_compute_date_deadline", inverse="_inverse_date_deadline")
property_type_id = fields.Many2one("estate.property.type", string="Property Type", related="property_id.property_type_id")

_sql_constraints = [
("check_price", "CHECK(price > 0)", "Offer price must be positive.")
]

@api.depends("validity")
def _compute_date_deadline(self):
for record in self:
record.date_deadline = fields.Date.add(
record.create_day if hasattr(record, "create_day") else fields.Date.today(),
days=record.validity)

def _inverse_date_deadline(self):
for record in self:
create_date = record.create_day if hasattr(record, "create_day") else fields.Date.today()
record.validity = (record.date_deadline - create_date).days

def action_accept_offer(self):
for record in self:
if record.property_id.state not in ["offer_accepted", "sold"]:
Copy link

Choose a reason for hiding this comment

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

do the oposite. try to minimise indentations.
so the the opposite condition is met, raise, then no need the else, as either it raise, either you can continue.

# if tools.float_utils.float_compare(record.price, 0.9 * record.property_id.expected_price, precision_rounding=2) == -1:
# i don't know why the above didn't work (i tried for expected price 100 and offer price 89 and it still accepted the offer
# so i wrote the below instead, which is not a good practice)
if record.price < 0.9 * record.property_id.expected_price:
raise exceptions.ValidationError("The selling price cannot be lower than 90% of the expected price.")
else:
record.status = "accepted"
Copy link

Choose a reason for hiding this comment

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

what happens to all the offers on the properties ? you should also refuse all the other offers, right ?
You can do that by making status a computed field depending on property_id.offer_ids.status. (I think.. unless circular dependancy will fire..) or in write method. (because it should be applied even if an offer is accepted via other means than this action.

record.property_id.state = "offer_accepted"
record.property_id.buyer_id = record.partner_id
record.property_id.selling_price = record.price
else:
raise exceptions.UserError("Only one offer can be accepted.")

def action_refuse_offer(self):
for record in self:
Copy link

Choose a reason for hiding this comment

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

note that when actions are called only on single record, (or when it's absolutely necessary to only have one record in self, you can use self.ensure_one()
(not really the case here but still interesting to know :) )

record.status = "refused"
Comment on lines +53 to +54
Copy link

Choose a reason for hiding this comment

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

self.status = "refused"
to write on all records at the same time


@api.model_create_multi
def create(self, vals):
Copy link

Choose a reason for hiding this comment

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

create method should be place before actions. Check the guidelines to sort your code properly

for val in vals:
property = self.env["estate.property"].browse(val["property_id"])
Copy link

Choose a reason for hiding this comment

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

will crash if property_id not in vals (I know it's a required field but let super handle i, we don't want to make it crash before it checks for required fields and raise a proper error for that.

if val["price"] < property.best_price:
raise exceptions.UserError(f"Price must be at least ${property.best_price}.")
Comment on lines +59 to +61
Copy link

Choose a reason for hiding this comment

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

looks like a constrain to me.
use _check method instead


property.state = "offer_received"
Copy link

Choose a reason for hiding this comment

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

but anyway, I would do that after the offer create. So you can just do

offers = super()
offers.property_id.state = 'received'

it will raise if not property is set on one the offer to create.
and you can write on all properties in batch as it's always to same value


return super().create(vals)
12 changes: 12 additions & 0 deletions estate/models/estate_property_tag.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from odoo import models, fields

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

name = fields.Char(required=True)
color = fields.Integer("Color")
_sql_constraints = [
Copy link

Choose a reason for hiding this comment

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

add an empty line to separated fields definition from the rest

("name_unique", "UNIQUE(name)", "The property tag name must be unique.")
]
36 changes: 36 additions & 0 deletions estate/models/estate_property_type.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
from odoo import api, fields, models

class PropertyType(models.Model):
_name = "estate.property.type"
_description = "Real estate property type"
_order = "sequence, name"

name = fields.Char(required = True)
sequence = fields.Integer("Sequence", default=1)
sql_constraints = [
('name_unique', 'UNIQUE(name)', 'The property type name must be unique.')
]
Comment on lines +10 to +12
Copy link

Choose a reason for hiding this comment

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

put this after fields definition


property_ids = fields.One2many("estate.property", "property_type_id", string = "Properties")
offer_ids = fields.One2many("estate.property.offer", "property_type_id", string="Offers")
offer_count = fields.Integer(compute="_compute_offers", string="Offers count")

@api.depends("offer_ids")
def _compute_offers(self):
for record in self:
record.offer_count = len(record.offer_ids)

class PropertyTypeLine(models.Model):
_name = "estate.property.type.line"
_description = "Real estate property type line"

model_id = fields.Many2one("estate.property.type")
Copy link

Choose a reason for hiding this comment

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

model_id ?
I guess you mean property_type_id ?

name = fields.Char("Title")
expected_price = fields.Float("Expected Price")
state = fields.Selection([
("new", "New"),
("offer_received", "Offer Received"),
("offer_accepted", "Offer Accepted"),
("sold", "Sold"),
("canceled", "Canceled")
], string="Status", required=True, copy=False, default="new")
11 changes: 11 additions & 0 deletions estate/models/res_users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from odoo import fields, models

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

property_ids = fields.One2many(
"estate.property",
"salesman_id",
string="Properties",
domain=[('state', 'in', ['new', 'offer_received'])]
)
6 changes: 6 additions & 0 deletions estate/security/ir.model.access.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
estate.access_estate_property,access_estate_property,estate.model_estate_property,base.group_user,1,1,1,1
access_estate_property_type,access_estate_property_type,estate.model_estate_property_type,base.group_user,1,1,1,1
access_estate_property_tag,access_estate_property_tag,estate.model_estate_property_tag,base.group_user,1,1,1,1
access_estate_property_offer,access_estate_property_offer,estate.model_estate_property_offer,base.group_user,1,1,1,1
estate.access_estate_property_type_line,access_estate_property_type_line,estate.model_estate_property_type_line,base.group_user,1,1,1,1
14 changes: 14 additions & 0 deletions estate/views/estate_menus.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?xml version="1.0"?>
<odoo>
<menuitem id="estate_menu_root" name="Real Estate">
<menuitem id="estate_first_level_menu" name="Advertisements">
<menuitem id="estate_property_model_menu_action" action="estate_property_action" />
</menuitem>
<menuitem id="estate_settings" name="Settings">
<menuitem id="estate_property_type_model_menu_action"
action="estate_property_type_action" />
<menuitem id="estate_property_tag_model_menu_action"
action="estate_property_tag_action" />
</menuitem>
</menuitem>
Comment on lines +3 to +13
Copy link

Choose a reason for hiding this comment

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

for the exercise: make one line per menuitems, not using encapsulation
but using parent_id instead.
this will help you when adding menuitems on bridge modules or modules depending on the module where you want to put a new menu item

</odoo>
46 changes: 46 additions & 0 deletions estate/views/estate_property_offer_views.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?xml version="1.1" encoding="UTF-8"?>
<odoo>
<record id="estate_property_offer_view_form" model="ir.ui.view">
<field name="name">estate.property.offer.form</field>
Copy link

Choose a reason for hiding this comment

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

estate.property.offer.view.form
apply this for every view

<field name="model">estate.property.offer</field>
<field name="arch" type="xml">
<form>
<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="Offers" editable="bottom" decoration-success="status == 'accepted'"
decoration-danger="status == 'refused'">
<field name="price" />
<field name="partner_id" />
<field name="validity" />
<field name="date_deadline" />
<button name="action_accept_offer" title="accept" type="object" icon="fa-check"
invisible="status in ['refused', 'accepted']" />
<button name="action_refuse_offer" title="refuse" type="object" icon="fa-times"
invisible="status in ['refused', 'accepted']" />
<field name="status" column_invisible="1" />
</list>
</field>
</record>

<record id="estate_property_offer_action" model="ir.actions.act_window">
<field name="name">Offers</field>
<field name="res_model">estate.property.offer</field>
<field name="view_mode">list,form</field>
<field name="domain">[('property_type_id', '=', context.get('active_id'))]</field>
</record>

</odoo>
19 changes: 19 additions & 0 deletions estate/views/estate_property_tag_views.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?xml version="1.0"?>
<odoo>
<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>

<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="Tags" editable="bottom">
<field name="name" />
<field name="color" widget="color" />
</list>
</field>
</record>
</odoo>
Loading