-
Notifications
You must be signed in to change notification settings - Fork 2.6k
QUCOL Onboarding #993
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
base: 19.0
Are you sure you want to change the base?
QUCOL Onboarding #993
Changes from all commits
326711a
04fcfac
a14ca42
792ff40
aa31e98
23db79f
5563be0
42d5841
19b9db7
1af32bc
4601f83
4462ec9
a3bdf2f
48d5ad6
2faaaee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| from . import models |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| { | ||
| 'name': 'Estate', | ||
| 'depends': [ | ||
| 'base' | ||
| ], | ||
| 'data': [ | ||
| 'security/ir.model.access.csv', | ||
|
|
||
| 'views/estate_property_views.xml', | ||
| 'views/estate_property_offer_views.xml', | ||
| 'views/estate_property_type_views.xml', | ||
| 'views/estate_property_tag_views.xml', | ||
| 'views/estate_res_user_views.xml', | ||
| 'views/estate_menu_views.xml', | ||
| ], | ||
| 'application': True, | ||
| 'author': 'Odoo S.A.', | ||
| 'license': 'LGPL-3' | ||
| } | ||
| 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_user | ||||||||||||||||||||||
|
Comment on lines
+1
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be sorted alphabetically, that's a python convention:
Suggested change
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,105 @@ | ||||||||||
| from odoo import api, fields, models, _ | ||||||||||
| from odoo.exceptions import UserError, ValidationError | ||||||||||
| from odoo.tools import float_compare | ||||||||||
|
|
||||||||||
| DEFAULT_GARDEN_AREA = 10 | ||||||||||
| DEFAULT_GARDEN_ORIENTATION = "north" | ||||||||||
|
|
||||||||||
|
Comment on lines
+5
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's the right way to write constants but I am not sure to understand why it is required. They are used only in _update_garden_area_and_orientation, can they be defined directly in that method? |
||||||||||
|
|
||||||||||
| class PropertyModel(models.Model): | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need of Model inside the class name of a model, but I agree that it could have been. It is an Odoo convention. |
||||||||||
| _name = "estate.property" | ||||||||||
| _description = "Estate Property model" | ||||||||||
| _order = "id desc" | ||||||||||
|
|
||||||||||
| name = fields.Char("Title", required=True) | ||||||||||
| description = fields.Text() | ||||||||||
| postcode = fields.Char() | ||||||||||
| date_availability = fields.Date(default=fields.Date.add(fields.Date.today(), months=3), copy=False) | ||||||||||
| expected_price = fields.Float(required=True) | ||||||||||
| best_offer = fields.Float(compute="_compute_highest_price") | ||||||||||
| selling_price = fields.Float(readonly=True, copy=False) | ||||||||||
| bedrooms = fields.Integer(default=2) | ||||||||||
| living_area = fields.Integer("Living Area (sqm)") | ||||||||||
| facades = fields.Integer() | ||||||||||
| garage = fields.Boolean() | ||||||||||
| garden = fields.Boolean() | ||||||||||
| garden_area = fields.Integer("Garden Area (sqm)") | ||||||||||
| garden_orientation = fields.Selection( | ||||||||||
| selection=[('north', 'North'), ('south', 'South'), ('east', 'East'), ('west', 'West')] | ||||||||||
| ) | ||||||||||
| total_living_area = fields.Integer("Total Area (sqm)", compute="_compute_total_area") | ||||||||||
|
|
||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need of that blank line 😃. |
||||||||||
| active = fields.Boolean(default=True) | ||||||||||
| state = fields.Selection( | ||||||||||
| selection=[ | ||||||||||
| ("new", "New"), | ||||||||||
| ("received", "Offer Received"), | ||||||||||
| ("accepted", "Offer Accepted"), | ||||||||||
| ("sold", "Sold"), | ||||||||||
| ("cancelled", "Cancelled") | ||||||||||
| ], | ||||||||||
| string="Status", | ||||||||||
| required=True, | ||||||||||
| copy=False, | ||||||||||
| default="new" | ||||||||||
| ) | ||||||||||
| property_type_id = fields.Many2one("estate.property.type") | ||||||||||
| buyer = fields.Many2one("res.partner", copy=False) | ||||||||||
| salesperson = fields.Many2one("res.users", default=lambda self: self.env.user) | ||||||||||
|
Comment on lines
+47
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should contain _id in the name as they are relationship fields.
Suggested change
|
||||||||||
| tag_ids = fields.Many2many("estate.property.tag") | ||||||||||
| offer_ids = fields.One2many("estate.property.offer", "property_id") | ||||||||||
|
|
||||||||||
| _check_positive_expected_price = models.Constraint( | ||||||||||
| "CHECK(expected_price >= 0)", | ||||||||||
| "The expected price must be positive." | ||||||||||
| ) | ||||||||||
| _check_positive_selling_price = models.Constraint( | ||||||||||
| "CHECK(selling_price >= 0)", | ||||||||||
| "The selling price must be positive" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| @api.depends("living_area", "garden_area") | ||||||||||
| def _compute_total_area(self): | ||||||||||
| for record in self: | ||||||||||
| record.total_living_area = record.living_area + record.garden_area | ||||||||||
|
|
||||||||||
| @api.depends("offer_ids") | ||||||||||
| def _compute_highest_price(self): | ||||||||||
| for record in self: | ||||||||||
| record.best_offer = max(record.offer_ids.mapped("price")) if record.offer_ids else 0 | ||||||||||
|
|
||||||||||
| @api.onchange("garden") | ||||||||||
| def _update_garden_area_and_orientation(self): | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||||||||||
| if self.garden: | ||||||||||
| self.garden_area = DEFAULT_GARDEN_AREA | ||||||||||
| self.garden_orientation = DEFAULT_GARDEN_ORIENTATION | ||||||||||
| else: | ||||||||||
| self.garden_area = 0 | ||||||||||
| self.garden_orientation = None | ||||||||||
|
|
||||||||||
| @api.constrains("selling_price", "expected_price") | ||||||||||
| def _check_selling_price(self): | ||||||||||
| for record in self: | ||||||||||
| if record.selling_price and float_compare(record.selling_price, record.expected_price * .9, 0) == -1: | ||||||||||
| raise ValidationError(r"The selling price cannot be lower than 90% of the expected price.") | ||||||||||
|
|
||||||||||
| @api.ondelete(at_uninstall=False) | ||||||||||
| def _unlink_if_new_or_cancelled(self): | ||||||||||
| if any(record.state not in ('new', 'cancelled') for record in self): | ||||||||||
| raise UserError(_("Only 'New' and 'Cancelled' properties can be deleted.")) | ||||||||||
|
|
||||||||||
| def action_mark_as_sold(self): | ||||||||||
| self.ensure_one() | ||||||||||
| if self.state == "cancelled": | ||||||||||
| raise UserError(_("A cancelled property cannot be set as sold.")) | ||||||||||
| if not any(offer.status == "accepted" for offer in self.offer_ids): | ||||||||||
| raise UserError(_("A property must have an accepted offer to be marked as sold.")) | ||||||||||
| self.state = "sold" | ||||||||||
| return True | ||||||||||
|
|
||||||||||
| def action_mark_as_cancelled(self): | ||||||||||
| self.ensure_one() | ||||||||||
| if self.state == "sold": | ||||||||||
| raise UserError(_("A sold property cannot be set as cancelled.")) | ||||||||||
| self.state = "cancelled" | ||||||||||
| return True | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,65 @@ | ||||||||||||||||
| from odoo import api, fields, models, _ | ||||||||||||||||
| from odoo.exceptions import UserError | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| class PropertyOfferModel(models.Model): | ||||||||||||||||
| _name = "estate.property.offer" | ||||||||||||||||
| _description = "Estate Property Offer model" | ||||||||||||||||
| _order = "price desc" | ||||||||||||||||
|
|
||||||||||||||||
| price = fields.Float() | ||||||||||||||||
| status = fields.Selection( | ||||||||||||||||
| selection=[ | ||||||||||||||||
| ("accepted", "Accepted"), | ||||||||||||||||
| ("refused", "Refused") | ||||||||||||||||
| ], | ||||||||||||||||
| copy=False | ||||||||||||||||
| ) | ||||||||||||||||
| partner_id = fields.Many2one("res.partner", required=True) | ||||||||||||||||
| property_id = fields.Many2one("estate.property", required=True) | ||||||||||||||||
| property_type_id = fields.Many2one(related="property_id.property_type_id", store=True) | ||||||||||||||||
| validity = fields.Integer(default=7) | ||||||||||||||||
| date_deadline = fields.Date(compute="_compute_deadline", inverse="_inverse_deadline") | ||||||||||||||||
|
|
||||||||||||||||
| _check_price = models.Constraint( | ||||||||||||||||
| "CHECK(price >= 0)", | ||||||||||||||||
| "The price of the offer must be positive." | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| @api.depends("validity") | ||||||||||||||||
| def _compute_deadline(self): | ||||||||||||||||
| for record in self: | ||||||||||||||||
| record.date_deadline = fields.Date.add((record.create_date or fields.Datetime.now()), days=record.validity) | ||||||||||||||||
|
|
||||||||||||||||
| def _inverse_deadline(self): | ||||||||||||||||
| for record in self: | ||||||||||||||||
| record.validity = (record.date_deadline - fields.Date.to_date(record.create_date)).days if record.date_deadline else record.validity | ||||||||||||||||
|
|
||||||||||||||||
| @api.model | ||||||||||||||||
| def create(self, vals_list: list[dict]): | ||||||||||||||||
| for val in vals_list: | ||||||||||||||||
| estate_property = self.env["estate.property"].browse(val["property_id"]) | ||||||||||||||||
|
Comment on lines
+40
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You shouldn't do sql request inside a loop 😃. It will do a lot of requests to your db as the create method can be used to create multiple records. Get first all the needed ids and then make a request with all of them with_prefetch(), which will allow you to get records by their id with browse inside the loop without doing any requests to the data base. Here also you can see that I used EstateProperties instead of estate_properties, that's an Odoo convention. When a variable represent a model we have to use the same convention than for the name of a python class. Sorry for that one, I have been a little hard on that one 😅. Just remember no SQL queries inside loops.
Suggested change
|
||||||||||||||||
| if estate_property.state == "sold": | ||||||||||||||||
| raise UserError(_("Cannot create a new offer for a sold property.")) | ||||||||||||||||
| if any(offer.price > val["price"] for offer in estate_property.offer_ids): | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
| raise UserError(_("Cannot create a new offer with a lower price than an existing offer.")) | ||||||||||||||||
| if estate_property.state == 'new': | ||||||||||||||||
| estate_property.state = 'received' | ||||||||||||||||
| return super().create(vals_list) | ||||||||||||||||
|
|
||||||||||||||||
| def accept_offer(self): | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as for the other action methods, the name should start by action 😃. |
||||||||||||||||
| self.ensure_one() | ||||||||||||||||
| self.status = "accepted" | ||||||||||||||||
| self.property_id.selling_price = self.price | ||||||||||||||||
| self.property_id.buyer = self.partner_id | ||||||||||||||||
| self.property_id.state = "accepted" | ||||||||||||||||
| self.refuse_all_other_offers() | ||||||||||||||||
|
|
||||||||||||||||
| def refuse_all_other_offers(self): | ||||||||||||||||
| for offer in self.property_id.offer_ids: | ||||||||||||||||
| if offer != self: | ||||||||||||||||
| offer.status = "refused" | ||||||||||||||||
|
|
||||||||||||||||
| def refuse_offer(self): | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||||||||||||||||
| self.ensure_one() | ||||||||||||||||
| self.status = "refused" | ||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| from odoo import fields, models | ||
|
|
||
|
|
||
| class PropertyTagModel(models.Model): | ||
| _name = "estate.property.tag" | ||
| _description = "Estate Property Tag model" | ||
| _order = "name" | ||
|
|
||
| name = fields.Char(required=True) | ||
| color = fields.Integer() | ||
|
|
||
| _check_tag_uniqueness = models.Constraint( | ||
| "UNIQUE(name)", | ||
| "Each tag should have a unique name." | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| from odoo import api, fields, models | ||
|
|
||
|
|
||
| class PropertyTypeModel(models.Model): | ||
| _name = "estate.property.type" | ||
| _description = "Estate Property Type model" | ||
| _order = "sequence" | ||
|
|
||
| name = fields.Char(required=True) | ||
| property_ids = fields.One2many("estate.property", "property_type_id") | ||
| offer_ids = fields.One2many("estate.property.offer", "property_type_id") | ||
| offer_count = fields.Integer("Offer Count", compute="_compute_offer_count") | ||
| sequence = fields.Integer("Sequence") | ||
|
|
||
| _check_type_uniqueness = models.Constraint( | ||
| "UNIQUE(name)", | ||
| "Each type should have a unique name." | ||
| ) | ||
|
|
||
| @api.depends("offer_ids") | ||
| def _compute_offer_count(self): | ||
| for record in self: | ||
| record.offer_count = len(record.offer_ids) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| from odoo import fields, models | ||
|
|
||
|
|
||
| class ResUser(models.Model): | ||
| _inherit = "res.users" | ||
|
|
||
| property_ids = fields.One2many( | ||
| "estate.property", "salesperson" | ||
| , domain=['|', ('state', '=', 'new'), ('state', '=', 'received')] | ||
| ) |
| 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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| from . import test_estate_property |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| from odoo.tests.common import TransactionCase | ||
| from odoo.exceptions import UserError | ||
| from odoo.tests import tagged, Form | ||
|
|
||
|
|
||
| ADMINISTRATOR_ID = 3 | ||
|
|
||
|
|
||
| @tagged("post_install", "-at_install") | ||
| class EstateTestCase(TransactionCase): | ||
|
|
||
| @classmethod | ||
| def setUpClass(cls): | ||
| super().setUpClass() | ||
| cls.new_property = cls.env["estate.property"].create([{ | ||
| "name": "New Property", | ||
| "state": "new", | ||
| "expected_price": 1000.0 | ||
| }]) | ||
| cls.received_property = cls.env["estate.property"].create([{ | ||
| "name": "Received offer Property", | ||
| "state": "received", | ||
| "expected_price": 2000.0 | ||
| }]) | ||
| cls.accepted_property = cls.env["estate.property"].create([{ | ||
| "name": "Accepeted offer Property", | ||
| "state": "accepted", | ||
| "expected_price": 3000.0 | ||
| }]) | ||
| cls.create_sold_property() | ||
| cls.cancelled_property = cls.env["estate.property"].create([{ | ||
| "name": "Cancelled Property", | ||
| "state": "cancelled", | ||
| "expected_price": 5000.0 | ||
| }]) | ||
| cls.env["estate.property.offer"].create([ | ||
| { | ||
| "partner_id": ADMINISTRATOR_ID, | ||
| "property_id": cls.accepted_property.id, | ||
| "status": "accepted", | ||
| "price": 3000.0 | ||
| }, | ||
| { | ||
| "partner_id": ADMINISTRATOR_ID, | ||
| "property_id": cls.received_property.id, | ||
| "status": "refused", | ||
| "price": 1999.0 | ||
| }, | ||
| { | ||
| "partner_id": ADMINISTRATOR_ID, | ||
| "property_id": cls.received_property.id, | ||
| "price": 2000.0 | ||
| } | ||
| ]) | ||
|
|
||
| @classmethod | ||
| def create_sold_property(cls): | ||
| cls.sold_property = cls.env["estate.property"].create([{ | ||
| "name": "Sold Property", | ||
| "state": "new", | ||
| "expected_price": 4000.0 | ||
| }]) | ||
| cls.env["estate.property.offer"].create([{ | ||
| "partner_id": ADMINISTRATOR_ID, | ||
| "property_id": cls.sold_property.id, | ||
| "status": "accepted", | ||
| "price": 4000.0 | ||
| }]) | ||
| cls.sold_property.state = "sold" | ||
|
|
||
| def test_offer_for_sold_property(self): | ||
| offer_for_sold_property = { | ||
| "partner_id": ADMINISTRATOR_ID, | ||
| "property_id": self.sold_property.id | ||
| } | ||
| correct_offers = [ | ||
| {"partner_id": ADMINISTRATOR_ID, "property_id": self.new_property.id, "price": 1000}, | ||
| {"partner_id": ADMINISTRATOR_ID, "property_id": self.received_property.id, "price": 2000}, | ||
| {"partner_id": ADMINISTRATOR_ID, "property_id": self.accepted_property.id, "price": 3000}, | ||
| {"partner_id": ADMINISTRATOR_ID, "property_id": self.cancelled_property.id, "price": 5000}, | ||
| ] | ||
| with self.assertRaises(UserError, msg="Creating an offer for a sold property should raise a UserError but it did not."): | ||
| self.env["estate.property.offer"].create([offer_for_sold_property]) | ||
| for offer in correct_offers: | ||
| try: | ||
| self.env["estate.property.offer"].create([offer]) | ||
| except UserError: | ||
| self.fail() | ||
|
|
||
| def test_sell_property_with_no_accepted_offer(self): | ||
| non_sellable_properties = [ | ||
| self.new_property, | ||
| self.received_property | ||
| ] | ||
| for property in non_sellable_properties: | ||
| with self.assertRaises(UserError): | ||
| property.action_mark_as_sold() | ||
| self.accepted_property.action_mark_as_sold() | ||
|
|
||
| def test_sold_state_correctly_set(self): | ||
| self.accepted_property.action_mark_as_sold() | ||
| self.assertEqual(self.accepted_property.state, "sold", "When a property is sold, its state should be set to 'sold'.") | ||
|
|
||
| def test_garden_area_orientation_reset(self): | ||
| property_form = Form(self.env["estate.property"]) | ||
| self.assertFalse(property_form.garden) | ||
| self.assertEqual(property_form.garden_area, 0) | ||
| self.assertFalse(property_form.garden_orientation) | ||
| property_form.garden = True | ||
| self.assertTrue(property_form.garden) | ||
| self.assertEqual(property_form.garden_area, 10) | ||
| self.assertEqual(property_form.garden_orientation, "north") | ||
| property_form.garden = False | ||
| self.assertFalse(property_form.garden) | ||
| self.assertEqual(property_form.garden_area, 0) | ||
| self.assertFalse(property_form.garden_orientation) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| <odoo> | ||
| <menuitem id="estate_menu_root" name="Real Estate"> | ||
| <menuitem id="estate_advertisements_menu" name="Advertisements"> | ||
| <menuitem id="estate_model_menu_action" action="estate_property_action"/> | ||
| </menuitem> | ||
| <menuitem id="estate_settings_menu" name="Settings"> | ||
| <menuitem id="estate_property_types_menu" action="estate_property_type_action"/> | ||
| <menuitem id="estate_property_tags_menu" action="estate_property_tag_action"/> | ||
| </menuitem> | ||
| </menuitem> | ||
| </odoo> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need of this blank line. Do not forget that a lot of developers are working on the project and we must avoid unnecessary changes.