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

vval - technical onboarding #623

Open
wants to merge 2 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 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 :)

14 changes: 14 additions & 0 deletions estate/__manifest__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
'name': 'Real Estate',
'application': True,
'depends': ['base'],
'data': [
'security/ir.model.access.csv',
'views/estate_property_offer_views.xml',
'views/estate_property_tag_views.xml',
'views/estate_property_type_views.xml',
'views/estate_property_views.xml',
'views/estate_menus.xml',
'views/res_users_views.xml',
],
}
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_offer
from . import estate_property_tag
from . import estate_property_type
from . import res_users
125 changes: 125 additions & 0 deletions estate/models/estate_property.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
from odoo import api, exceptions, fields, models, tools

class EstateProperty(models.Model):
_name = 'estate.property'
_description = 'Real estate management'

active = fields.Boolean('Is Active?', default=True)
name = fields.Char('Title', required=True)
property_type_id = fields.Many2one('estate.property.type', string='Property Type')
property_tag_ids = fields.Many2many('estate.property.tag', string='Property Tags')

postcode = fields.Char('Postcode')
date_availability = fields.Date('Available From',
copy=False,
default=fields.Date.add(fields.Date.today(), months=3)
)
expected_price = fields.Float('Expected Price', required=True)
best_price = fields.Float('Best Offer', compute='_compute_best_price', default=0)
selling_price = fields.Float('Selling Price', readonly=True, copy=False)

bedrooms = fields.Integer('Bedrooms', default=2)
description = fields.Text('Description')
facades = fields.Integer('Facades')
garage = fields.Boolean('Garage')
garden = fields.Boolean('Garden')
garden_area = fields.Integer('Garden Area (sqm)')
garden_orientation = fields.Selection([
('north', 'North'),
('east', 'East'),
('south', 'South'),
('west', 'West')
], string='Garden Orientation'
)
living_area = fields.Integer('Living Area (sqm)')
total_area = fields.Integer('Total Area (sqm)', compute='_compute_total_area')

buyer_id = fields.Many2one('res.partner', string='Buyer', readonly=True, copy=False)
salesperson_id = fields.Many2one('res.users',
string='Salesperson',
default=lambda self: self.env.user
)

offer_ids = fields.One2many('estate.property.offer', 'property_id', string='Offers')
state = fields.Selection([
('new', 'New'),
('offer_received', 'Offer Received'),
('offer_accepted', 'Offer Accepted'),
('sold', 'Sold'),
('canceled', 'Canceled'),
], string='Status', copy=False, required=True, readonly=True,
default='new'
)

_order = 'id desc'
_sql_constraints = [
('expected_price_strictly_positive', 'CHECK(expected_price > 0)',
'The expected price should be stricly positive.'),
('selling_price_positive', 'CHECK(selling_price >= 0)',
'The selling price should be positive.')
]

@api.constrains('selling_price')
def _check_selling_price(self):
for record in self:
if record.offer_ids and fields.Float.compare(
record.selling_price,
record.expected_price * 0.9,
precision_digits=1
) < 0:
Comment on lines +65 to +69
Copy link

Choose a reason for hiding this comment

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

make a variable for the compare. not sure you need to specify precision_digit name.

is_selling_price_to_low = fields.Float.compare(record.selling_price, record.expected_price * 0.9, 1) < 0

I don't find much examples of fields.float.compare in the code base. Maybe better use odoo.tools 'float_compare' function.
also, we prefer using explicit names rather than 'record' which is too general.
here you work on properties so use for property in self.

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

@api.depends('offer_ids')
def _default_state(self):
return 'offer_accepted' if self.offer_ids else 'new'

@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):
for record in self:
if record.offer_ids:
for offer in record.offer_ids:
record.best_price = max(record.best_price, offer.price)
Comment on lines +86 to +87
Copy link

Choose a reason for hiding this comment

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

get the best price among the offers, then assign the max value. you can also compact the code using ternary assignation
record.best_price = max(offer.price for offer in record.offer_ids) if record.offer_ids else 0

BUT, even if related records are fetched by the ORM for all the recordset when calling related record on a record of the recordset.. maybe here it could be nice to make a read_group with aggregate max.
to goal is to make only one SQL request for all the records. then loop with the result.

else:
record.best_price = 0

@api.onchange('expected_price')
def _onchange_expected_price(self):
self._check_selling_price()
Comment on lines +91 to +93
Copy link

Choose a reason for hiding this comment

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

we don't call constrain method in onchange. Otherwize each time the user is modifying his data form, he will get error firing. Let the user work, then, let the contrain do their job once the user save the record.
constrain (_check) methods are called during write.


def print_stuff(self):
print("Hello, I handled a conflict")

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

def action_mark_canceled(self):
for record in self:
if record.state == 'sold':
raise exceptions.UserError('You cannot mark a sold property as canceled')
Comment on lines +105 to +106
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, yu can use property._origin to get the old value.

else:
record.state = 'canceled'
return True

def action_mark_sold(self):
for record in self:
if record.state == 'canceled':
raise exceptions.UserError('You cannot mark a canceled property as sold')
Comment on lines +113 to +114
Copy link

Choose a reason for hiding this comment

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

same here. use the same check method than for the case above.

else:
record.state = 'sold'
return True

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

class EstatePropertyOffer(models.Model):
_name = 'estate.property.offer'
_description = 'Estate property offer'

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

_order = 'price desc'
_sql_constraints = [
('offer_price_strictly_positive', 'CHECK(price > 0)',
'Offer price should be stricly positive.')
]

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

def _inverse_date_deadline(self):
for record in self:
record.validity = (record.date_deadline - record.create_date.date()).days

def action_accept_offer(self):
for record in self:
match record.property_id.state:
case 'new' | 'offer_received':
for offer in record.property_id.offer_ids:
offer.status = 'refused'
record.property_id.buyer_id = record.partner_id
record.property_id.selling_price = record.price
record.property_id.state = 'offer_accepted'
record.status = 'accepted'
case 'offer_accepted' | 'sold':
raise exceptions.UserError('An offer has already been accepted')
case 'canceled':
raise exceptions.UserError('This sale has been canceled')
return True

def action_refuse_offer(self):
for record in self:
match record.status:
case 'accepted':
raise exceptions.UserError('You cannot refuse an accepted offer')
case False:
record.status = 'refused'
return True

@api.model_create_multi
def create(self, vals_list):
for vals in vals_list:
self.env['estate.property'].browse(vals['property_id']).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.

will crash if 'property_id' not in vals.
also, I would do that after the create.
so you can use the created record to access their properties

if vals['price'] < self.env['estate.property'].browse(vals['property_id']).best_price:
raise exceptions.ValidationError(
'The offer´s price must be higher than the current best offer'
)
Comment on lines +73 to +76
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

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

class EstatePropertyTag(models.Model):
_name = 'estate.property.tag'
_description = 'Estate property tag'

active = fields.Boolean('Is Active?', default=True)
name = fields.Char('Name', required=True)
color = fields.Integer('Color', default=0)

_order = 'name'
_sql_constraints = [
('tag_name_unique', 'UNIQUE(name)',
'Tag name should be unique.')
]
23 changes: 23 additions & 0 deletions estate/models/estate_property_type.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
from odoo import api, fields, models

class EstatePropertyType(models.Model):
_name = 'estate.property.type'
_description = 'Estate property type'

active = fields.Boolean('Is Active?', default=True)
name = fields.Char('Name', required=True)
offer_ids = fields.One2many('estate.property.offer', 'property_type_id', string ='Offers')
offer_count = fields.Integer('Offer count', compute='_compute_offer_count')
property_ids = fields.One2many('estate.property', 'property_type_id', string='Properties')
sequence = fields.Integer('Sequence', default=1)

_order = 'sequence, name'
_sql_constraints = [
('type_name_unique', 'UNIQUE(name)',
'Type name should be unique.')
]

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

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

property_ids = fields.One2many('estate.property', 'salesperson_id',
string='Properties', domain=['|',
('state', '=', 'new'),
('state', '=', 'offer_received'),
])
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
estate.access_estate_property,access_estate_property,estate.model_estate_property,base.group_user,1,1,1,1
estate.access_estate_property_type,access_estate_property_type,estate.model_estate_property_type,base.group_user,1,1,1,1
estate.access_estate_property_tag,access_estate_property_tag,estate.model_estate_property_tag,base.group_user,1,1,1,1
estate.access_estate_property_offer,access_estate_property_offer,estate.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_menu_root" name="Real Estate">
<menuitem id="estate_menu_advertisements" name="Advertisements">
<menuitem id="estate_action_properties" action="action_estate_properties"/>
</menuitem>
<menuitem id="estate_menu_settings" name="Settings">
<menuitem id="estate_action_property_types" action="action_estate_property_types"/>
<menuitem id="estate_action_property_tags" action="action_estate_property_tags"/>
</menuitem>
</menuitem>
</odoo>
43 changes: 43 additions & 0 deletions estate/views/estate_property_offer_views.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?xml version="1.0"?>
<odoo>
<record id="action_estate_property_offer" 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>

<record id="estate_property_offer_view_list" 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 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" type="object" icon="fa-check" invisible="status"/>
<button name="action_refuse_offer" type="object" icon="fa-times" invisible="status"/>
<field name="status" column_invisible="true"/>
</list>
</field>
</record>

<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>
<group>
<field name="price"/>
<field name="partner_id"/>
<field name="validity"/>
<field name="date_deadline"/>
<field name="status"/>
</group>
</form>
</field>
</record>
</odoo>
18 changes: 18 additions & 0 deletions estate/views/estate_property_tag_views.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0"?>
<odoo>
<record id="action_estate_property_tags" 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_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>
<h1><field name="name"/></h1>
</form>
</field>
</record>
</odoo>
Loading