Skip to content

[ADD] estate: Chapters 1-11 #873

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

Open
wants to merge 9 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
16 changes: 16 additions & 0 deletions estate/__manifest__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
'name': "estate",
'depends': ['base'],
'application': True,
'license': 'LGPL-3',
'data': ['security/ir.model.access.csv',
'views/estate_property_views.xml',
'views/estate_property_tag_views.xml',
'views/estate_property_offer_views.xml',
'views/estate_property_type_views.xml',
'views/estate_property_menus.xml',
'views/inherited_model_view.xml'
]


}
1 change: 1 addition & 0 deletions estate/models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from . import estate_property, estate_property_type, estate_property_tag, estate_property_offer, inherited_model
102 changes: 102 additions & 0 deletions estate/models/estate_property.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
from odoo import api, fields, models
from odoo.exceptions import UserError, ValidationError
from odoo.tools import float_compare, float_is_zero

Choose a reason for hiding this comment

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

The runbot ci/style probably says that but we always have two blank lines between the imports and the models

Suggested change


class TestModel(models.Model):

Choose a reason for hiding this comment

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

The class name of a model is always its _name attribute but without the dots and all first letters of words in uppercase (almost camelCase but first word is capitalized too)

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

_name = "estate_property"

Choose a reason for hiding this comment

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

The _name attribute of the model is always formated as words separated by dots so:

Suggested change
_name = "estate_property"
_name = "estate.property"

_description = "Estate Property model"
_order = "id desc"

name = fields.Char(required=True)
description = fields.Text()
postcode = fields.Char()
date_availability = fields.Date(copy=False, default=fields.Date.add(fields.Date.today(), months=3))
expected_price = fields.Float(required=True) #
selling_price = fields.Float(readonly=True, copy=False) #
bedrooms = fields.Integer(default=2)
living_area = fields.Integer()
facades = fields.Integer() #
garage = fields.Boolean()
garden = fields.Boolean()
garden_area = fields.Integer()
garden_orientation = fields.Selection(
string='Garden Orientation',
selection=[('North', 'North'), ('South', 'South'), ('East', 'East'), ('West', 'West')])
active = fields.Boolean('Active', default=True)

state = fields.Selection(
string='Property Status',
selection=[('New', 'New'), ('Offer Received', 'Offer Received'), ('Offer Accepted', 'Offer Accepted'), ('Sold', 'Sold'), ('Cancelled', 'Cancelled')],
required=True,
default="New")
property_type_id = fields.Many2one("estate_property_type", string="Property Type")
salesperson = fields.Many2one('res.users', string='Salesman', default=lambda self: self.env.user)
buyer = fields.Many2one('res.partner', string='Buyer', copy=False)
tag_ids = fields.Many2many("estate_property_tag", string="Property Tags")
offer_ids = fields.One2many(comodel_name="estate_property_offer", inverse_name="property_id")
total_area = fields.Integer(compute="_compute_total_area")
best_price = fields.Float(compute="_compute_best_offer")

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

@api.depends('offer_ids')
def _compute_best_offer(self):
for property in self:
if len(property.offer_ids) > 0:
property.best_price = max(property.offer_ids.mapped('price'))
else:
property.best_price = 0.0

@api.onchange("garden")
def _onchange_garden(self):
if self.garden:
self.garden_orientation = "North"

Choose a reason for hiding this comment

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

Since you will have changed the technical value of the garden_orientation field, you will to check for == 'north'

Suggested change
self.garden_orientation = "North"
self.garden_orientation = 'north'

Other occurences of this lower in the diff 👍

self.garden_area = 10
else:
self.garden_area = 0
self.garden_orientation = ''

Choose a reason for hiding this comment

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

It'd be better to set it as False for readability (doesn't change anything nor to the execution to the result)

Suggested change
self.garden_orientation = ''
self.garden_orientation = False


def sell(self):
for record in self:
if not record.state == 'Offer Accepted':
raise UserError("You cannot sell a property for which there is no accepted offer!")

else:
record.state = "Sold"
return True

def cancel(self):
for record in self:
if record.state == 'Sold':
raise UserError("Sold properties cannot be cancelled!")

else:
record.state = "Cancelled"
return True

_sql_constraints = [
('strictly_positive_expected_price',
'CHECK(expected_price > 0)',
'The expected price should be strictly positive'),

('positive_seling_price',
'CHECK(selling_price >= 0)',
'The selling price should be positive'),

]

@api.constrains('expected_price', 'selling_price')
def filter_bad_offers(self):
for property in self:
if not float_is_zero(property.selling_price, precision_digits=6) and float_compare(property.selling_price, 0.9 * property.expected_price, precision_digits=6) < 0:
raise ValidationError('The selling price is below the 90% threshold of the expected price')

Choose a reason for hiding this comment

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

Depends on the team you will join but we tend to always wrap user messages in between douoble quotes. At least in R&D Accounting. But whatever the team you will join, you will be asked to be consistent with your use of single and double quotes so as you used doubles quotes for all teh other errors before this one, I'd suggest you do the same here 👍

Suggested change
raise ValidationError('The selling price is below the 90% threshold of the expected price')
raise ValidationError("The selling price is below the 90% threshold of the expected price")

return True

@api.ondelete(at_uninstall=False)
def _unlink_if_offer_not_new_not_cancelled(self):
if any(property.state not in ["New", "Cancelled"] for property in self):
raise UserError("Can't delete an offer which is not cancelled or new!")
106 changes: 106 additions & 0 deletions estate/models/estate_property_offer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
from odoo import models, fields, api
from datetime import datetime
from odoo.exceptions import UserError
from odoo.tools import float_compare
Comment on lines +1 to +4

Choose a reason for hiding this comment

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

Here is an instance of two blocks of imports. You have external ones and odoo ones so your imports should look like this:

Suggested change
from odoo import models, fields, api
from datetime import datetime
from odoo.exceptions import UserError
from odoo.tools import float_compare
from datetime import datetime
from odoo import api, fields, models
from odoo.exceptions import UserError
from odoo.tools import float_compare


Choose a reason for hiding this comment

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

missing one blank space


class EstatePropertyOffer(models.Model):
_name = "estate_property_offer"

Choose a reason for hiding this comment

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

Suggested change
_name = "estate_property_offer"
_name = "estate.property.offer"

_description = "Estate Property Offer model"
_order = "price desc"

Choose a reason for hiding this comment

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

too many blank spaces, more of this lower in the diff 👍

name = fields.Char(required=True, default="Offer")
price = fields.Float()
state = fields.Selection(
string='State',
selection=[('Accepted', 'Accepted'), ('Refused', 'Refused')],
copy=False)
partner_id = fields.Many2one('res.partner', string='Partner', required=True)
property_id = fields.Many2one('estate_property', required=True, ondelete='cascade')
date_deadline = fields.Date(compute='_compute_deadline', inverse='_compute_validity')
validity = fields.Integer(default=7)

property_state = fields.Selection(
related='property_id.state',
readonly=True,
)

property_type_id = fields.Many2one(
related='property_id.property_type_id',
)

@api.depends('validity')
def _compute_deadline(self):
for property in self:
if not property.create_date:
property.date_deadline = fields.Date.add(fields.Date.today(), days=property.validity)

else:
property.date_deadline = fields.Date.add(property.create_date, days=property.validity)

def _compute_validity(self):
for property in self:
if not property.create_date:

property.validity = (property.date_deadline - fields.Date.today()).days

else:
property.validity = (datetime.combine(property.date_deadline, datetime.min.time()) - property.create_date).days

def action_confirm(self):
for record in self:
if record.property_id.state not in ["Cancelled", "Sold", "Offer Accepted"]:
record.state = 'Accepted'
record.property_id.buyer = record.partner_id
record.property_id.selling_price = record.price
record.property_id.state = "Offer Accepted"
elif record.property_id.state == "Offer Accepted":
if record.state != "Accepted":
raise UserError("Only one offer can be accepted!")

else:

Choose a reason for hiding this comment

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

The else works great but in a project like odoo with hundreds of devs, the quicker it is to understand what you did, the best so we tend to make everything as explicit as possible. In this case, you could replace your else by the last value(s) possible of the preperty' state field to make it a tiny bit more readable 👍

Suggested change
else:
elif record.property_id.state == "New":

raise UserError("Property not available!")

return True

def action_reject(self):
for record in self:
if record.property_id.state not in ["Cancelled", "Sold"]:
if record.state == 'Accepted':
record.property_id.state = "Offer Received"
record.property_id.buyer = ''
record.property_id.selling_price = 0.0
record.state = 'Refused'

else:

Choose a reason for hiding this comment

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

Same here. To clarify a bit, I don't think anyone will ever ask you to do it so if you don't it's fine. There are places where it's really better and some not so much. Just keep in mind you spend 99% of your time in code that is not yours so it's always better when it's easy to understand 👍

raise UserError("Property not available!")

return True

_sql_constraints = [
('strictly_positive_offer_price',
'CHECK(price > 0)',
'The offer price should be strictly positive')

]

@api.model_create_multi
def create(self, vals_list):
property_ids = set()
for val in vals_list:
property_ids.add(val['property_id'])

property_objects = self.env["estate_property"].browse(list(property_ids))

property_best_price = {prop.id: prop.best_price for prop in property_objects}

for val in vals_list:
if float_compare(property_best_price[val['property_id']], val['price'], precision_digits=6) > 0:
raise UserError(f"Offer must be higher than {property_best_price[val['property_id']]}!")

offers = super().create(vals_list)

for offer in offers:
offer.property_id.state = 'Offer Received'

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

Choose a reason for hiding this comment

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

As this file is shorter, i'll just leave the list of the things i saw here and let you look after what it means in terms changes to be made 😄

  • Import order guidelines
  • Module technical name naming convention
  • Blank spaces
  • Missing EOL
  • Field attribute operator spacing



class EstatePropertyTag(models.Model):
_name = "estate_property_tag"
_description = "Estate Property Tag model"
_order = "name"
name = fields.Char(required=True)
color = fields.Integer()

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

Choose a reason for hiding this comment

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

This one i'll let you do by yourself with the comments i wrote before. There's a bunch of repeating stuff 😄



class EstatePropertyType(models.Model):
_name = "estate_property_type"
_description = "Estate Property Type model"
_order = "sequence,name"

Choose a reason for hiding this comment

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

Suggested change
_order = "sequence,name"
_order = "sequence, name"


name = fields.Char(required=True)
property_ids = fields.One2many(comodel_name="estate_property", inverse_name="property_type_id")
sequence = fields.Integer('Sequence', default=1)
offer_ids = fields.One2many(comodel_name="estate_property_offer", inverse_name="property_type_id")
offer_count = fields.Integer(compute="_compute_offer_count")

_sql_constraints = [
('is_property_type_unique', 'UNIQUE(name)', 'Property type name must be unique!')
]

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

Choose a reason for hiding this comment

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

And this one too is pretty straight forward with what i wrote before 😄



class InheritedModel(models.Model):
_inherit = "res.users"
_description = "Res Users Inherited model"
property_ids = fields.One2many(comodel_name="estate_property", inverse_name="salesperson", domain=[('state', 'in', ['New', '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
13 changes: 13 additions & 0 deletions estate/views/estate_property_menus.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0"?>
<odoo>
<menuitem id="test_menu_root" name="Real Estate">
<menuitem id="test_first_level_menu" name="Advertisements">
<menuitem id="test_model_menu_action" action="test_model_action"/>
</menuitem>
<menuitem id="estate_property_type_settings_menu" name="Settings">
<menuitem id="property_type_action" action="estate_property_type_properties"/>
<menuitem id="estate_property_tag_menu_tags" action="estate_property_tag_action"/>
</menuitem>

Choose a reason for hiding this comment

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

Useless blank line 👍

</menuitem>
</odoo>
61 changes: 61 additions & 0 deletions estate/views/estate_property_offer_views.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?xml version="1.0"?>

Choose a reason for hiding this comment

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

Lots of blank lines that shouldn't be here in this file. 😄

<odoo>


<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', '=', active_id)]</field>
</record>
Comment on lines +5 to +10

Choose a reason for hiding this comment

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

Suggested change
<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', '=', active_id)]</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', '=', active_id)]</field>
</record>



<record id="estate_property_offer_list_view" model="ir.ui.view">

<field name="name">List View</field>

Choose a reason for hiding this comment

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

Try to make it a bit more specific 👍

Suggested change
<field name="name">List View</field>
<field name="name">Offers List View</field>
or
<field name="name">Property Offers</field>
or even just
<field name="name">Offers</field>

<field name="model">estate_property_offer</field>
<field name="arch" type="xml">
<list string="Offer List" editable="bottom" decoration-success = "state == 'Accepted'" decoration-danger = "(state == 'Refused') or (state != 'Accepted' and property_state in ('Offer Accepted', 'Cancelled', 'Sold'))">

<field name="price" string = "Price"/>

Choose a reason for hiding this comment

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

operators spacing 😄

<field name="partner_id" string = "Partner"/>
<field name="validity" string = "Validity (days)"/>
<field name="date_deadline" string = "Deadline"/>

<button name="action_confirm" string="Accept" type="object" icon="fa-check" invisible="(state in ('Accepted','Refused')) or property_state in ('Cancelled', 'Offer Accepted', 'Sold')"/>
<button name="action_reject" string="Reject" type="object" icon="fa-times" invisible="(state in ('Accepted','Refused')) or property_state in ('Cancelled', 'Offer Accepted', 'Sold')"/>


</list>
</field>
</record>


<record id="estate_property_offer_form_view" model="ir.ui.view">
<field name="name">Form View</field>
<field name="model">estate_property_offer</field>
<field name="arch" type="xml">
<form string="Property Form">
<sheet>

<div class="estate_property_offer_oe_title">
<h1> <field name="name" nolabel="1"/> </h1>
</div>

<group>
<field name="price" string = "Price"/>
<field name="partner_id" string = "Partner"/>
<field name="validity" string = "Validity (days)"/>
<field name="date_deadline" string = "Deadline"/>
<field name="state" string = "State"/>
Comment on lines +46 to +50

Choose a reason for hiding this comment

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

always indent by gorups of 4 spaces ( ). A same level of indent should be aligned 😄


</group>

</sheet>
</form>
</field>
</record>


</odoo>

Choose a reason for hiding this comment

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

missing EOL

23 changes: 23 additions & 0 deletions estate/views/estate_property_tag_views.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?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_list_view" model="ir.ui.view">
<field name="name">Property Tags List View</field>
<field name="model">estate_property_tag</field>
<field name="arch" type="xml">
<list string="Tag List" editable = "bottom">
<field name="name" />
</list>
</field>
</record>


</odoo>
Loading