-
Notifications
You must be signed in to change notification settings - Fork 3.1k
daloe - Technical Training #1276
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?
Changes from all commits
574e7cf
3877d67
13d6847
720922e
506fa92
1e5c4b1
470c7f5
0cd6225
48326be
c75989a
a817933
a9c7840
fceea68
77cd2ae
7968990
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,18 @@ | ||
| { | ||
| 'name': "Estate", | ||
| 'depends': ['base'], | ||
| 'application': True, | ||
| 'installable': True, | ||
| "author": "daloe", | ||
| "license": "LGPL-3", | ||
| "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_menus.xml', | ||
| 'views/res_users_views.xml' | ||
| ], | ||
| } | ||
| 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 | ||||||||||||||||||||||
|
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. Order your python import alphabetically 👍
Suggested change
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,196 @@ | ||||||||||||||||||||||
| from dateutil.relativedelta import relativedelta | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| from odoo import api, fields, models | ||||||||||||||||||||||
| from odoo.exceptions import UserError, ValidationError | ||||||||||||||||||||||
| from odoo.tools import float_compare | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| class EstateProperty(models.Model): | ||||||||||||||||||||||
| _name = "estate.property" | ||||||||||||||||||||||
| _description = "technical training estate property 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. So the
Suggested change
|
||||||||||||||||||||||
| _order = "id desc" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # RELATIONAL FIELDS | ||||||||||||||||||||||
|
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. We try to have as little comments in the code as possible so you can put it as a help for structure while you dev but they shouldn't be in your final version ready for merge. The only exceptions are very technical things that are not explicit enoug hfrom the code itself or things that might look wrong but are actually the intended behavior.
Suggested change
|
||||||||||||||||||||||
| estate_property_type_id = fields.Many2one( | ||||||||||||||||||||||
| "estate.property.type", | ||||||||||||||||||||||
| ondelete="set null", | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| estate_property_tag_ids = fields.Many2many( | ||||||||||||||||||||||
| "estate.property.tag", | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
Comment on lines
+18
to
+22
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. I see you like to put a lot of space in your code which is good but for fields declaration, you'll notice that they are very often a compact list of one line declarations 😅 Yo ucan keep doing that for methods and stuff but don't put blank spaces in between your fields and try to oneline those that don't have to much in the declaration like this one for example.
Suggested change
|
||||||||||||||||||||||
| buyer_id = fields.Many2one( | ||||||||||||||||||||||
| "res.partner", | ||||||||||||||||||||||
| ondelete="set null", | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
Comment on lines
+23
to
+27
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. I would oneline this one too |
||||||||||||||||||||||
| seller_id = fields.Many2one( | ||||||||||||||||||||||
| "res.users", | ||||||||||||||||||||||
| ondelete="set null", | ||||||||||||||||||||||
| default=lambda self: self.env.user.id, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| estate_property_offer_ids = fields.One2many( | ||||||||||||||||||||||
| "estate.property.offer", | ||||||||||||||||||||||
| "estate_property_id", | ||||||||||||||||||||||
| copy=False, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # BASIC FIELDS | ||||||||||||||||||||||
|
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
|
||||||||||||||||||||||
| name = fields.Char( | ||||||||||||||||||||||
| "Estate Property", | ||||||||||||||||||||||
| required=True, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| active = fields.Boolean( | ||||||||||||||||||||||
| default=True, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
Comment on lines
+41
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. oneline these ones |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| status = fields.Selection( | ||||||||||||||||||||||
| [["new", "New"], ["offer_received", "Offer Received"], ["offer_accepted", "Offer Accepted"], ["sold", "Sold"], ["cancelled", "Cancelled"]], | ||||||||||||||||||||||
|
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. Doesn't change much but we tend to use a list of tuples for selection declarations and when they get to three or four or more elements, we multiline the list for clarity and an easier modification in the future. One very important thing in R&D at odoo is to try to keep the commit history as clean as possible so anytime you multiline something, you allow someone to modify a smaller part of the code to get the same result keeping a clearer history in the end. A lot of our styling guidelines revolves around that.
Suggested change
|
||||||||||||||||||||||
| default="new", | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| description = fields.Text( | ||||||||||||||||||||||
| "Description" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| postcode = fields.Char( | ||||||||||||||||||||||
| "Postcode" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
Comment on lines
+55
to
+61
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. oneline these one, you get the point. Just try to keep a smaller diff if it doesn't provide too much of a benefit to have a bigger one 😄 |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| date_availability = fields.Date( | ||||||||||||||||||||||
| "Available From", | ||||||||||||||||||||||
| copy=False, | ||||||||||||||||||||||
| default=fields.Date.today() + relativedelta(months=+3), | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| expected_price = fields.Float( | ||||||||||||||||||||||
| "Expected Price", | ||||||||||||||||||||||
| required=True, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| selling_price = fields.Float( | ||||||||||||||||||||||
| "Selling Price", | ||||||||||||||||||||||
| copy=False, | ||||||||||||||||||||||
| readonly=True, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| bedrooms = fields.Integer( | ||||||||||||||||||||||
| "Bedrooms", | ||||||||||||||||||||||
| default=2, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| living_area = fields.Integer( | ||||||||||||||||||||||
| "Living Area (sqm)" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| facades = fields.Integer( | ||||||||||||||||||||||
| "Facades" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| garage = fields.Boolean( | ||||||||||||||||||||||
| "Garage", | ||||||||||||||||||||||
| default=False, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| garden = fields.Boolean( | ||||||||||||||||||||||
| "Garden", | ||||||||||||||||||||||
| default=False, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| garden_area = fields.Integer( | ||||||||||||||||||||||
| "Garden Area" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| garden_orientation = fields.Selection( | ||||||||||||||||||||||
| [["north", "North"], ["east", "East"], ["south", "South"], ["west", "West"]], | ||||||||||||||||||||||
|
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. List of tuples 👍 Even though it's a selection field, I would keep this one in a single line because I don't expect a new orientation to be added to the selection 😄 |
||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # COMPUTED FIELDS | ||||||||||||||||||||||
|
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
|
||||||||||||||||||||||
| total_area = fields.Integer(compute="_compute_total_area") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| best_price = fields.Float("Best Offer", compute="_compute_best_price") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # SQL CONSTRAINTS & INDEXES | ||||||||||||||||||||||
|
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
|
||||||||||||||||||||||
| _check_expected_price_positive = models.Constraint( | ||||||||||||||||||||||
| "CHECK(expected_price > 0)", | ||||||||||||||||||||||
| "expected_price <= 0" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| _check_selling_price_positive = models.Constraint( | ||||||||||||||||||||||
| "CHECK(selling_price is null or selling_price > 0)", | ||||||||||||||||||||||
| "selling_price <= 0" | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # COMPUTE & INVERSE & SEARCH | ||||||||||||||||||||||
|
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
What I like though is that you nicely ordered fields -> constrains -> methods without having a bit of each everywhere. This is how we tend to structure of models declaration and it's nice that you've already doing it intuitively 👍 |
||||||||||||||||||||||
| @api.depends("living_area") | ||||||||||||||||||||||
| def _compute_total_area(self): | ||||||||||||||||||||||
| for estate in self: | ||||||||||||||||||||||
| null_safe_garden_area = 0 | ||||||||||||||||||||||
| if estate.garden_area: | ||||||||||||||||||||||
| null_safe_garden_area = estate.garden_area | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| estate.total_area = estate.living_area + null_safe_garden_area | ||||||||||||||||||||||
|
Comment on lines
+129
to
+135
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. This is a bit strange looking to me. Probably because of the name of the variable that can be actually wrong if there is a garden_area and therefore the value is not null. If I'm not mistaken, since
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @api.depends("estate_property_offer_ids") | ||||||||||||||||||||||
| def _compute_best_price(self): | ||||||||||||||||||||||
| for estate in self: | ||||||||||||||||||||||
| if len(estate.estate_property_offer_ids) > 0: | ||||||||||||||||||||||
| estate.best_price = max(self.estate_property_offer_ids.mapped("price")) | ||||||||||||||||||||||
| else: | ||||||||||||||||||||||
| estate.best_price = 0 | ||||||||||||||||||||||
|
Comment on lines
+140
to
+143
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. Mapped doesn't raise on an empty recordset so you don't even need an
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # PYTHON CONSTRAINS & ONCHANGE | ||||||||||||||||||||||
|
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
|
||||||||||||||||||||||
| @api.constrains('selling_price') | ||||||||||||||||||||||
| def _check_ninety_percent_of_expected(self): | ||||||||||||||||||||||
| for estate in self: | ||||||||||||||||||||||
| if estate.selling_price > 0: # restrict checks to only when offer acceptance happens | ||||||||||||||||||||||
| if float_compare(estate.selling_price, 0.9 * estate.expected_price, 2) < 0: | ||||||||||||||||||||||
| raise ValidationError("Offer is <90% of expected") | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @api.onchange("garden") | ||||||||||||||||||||||
| def _onchange_garden_orientation(self): | ||||||||||||||||||||||
| if self.garden: | ||||||||||||||||||||||
| self.garden_area = 10 | ||||||||||||||||||||||
| self.garden_orientation = "north" | ||||||||||||||||||||||
| else: | ||||||||||||||||||||||
| self.garden_area = None | ||||||||||||||||||||||
| self.garden_orientation = None | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # ORM OVERRIDES | ||||||||||||||||||||||
|
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
|
||||||||||||||||||||||
| @api.ondelete(at_uninstall=False) | ||||||||||||||||||||||
| def _unlink_if_new_cancelled(self): | ||||||||||||||||||||||
| for user in self: | ||||||||||||||||||||||
| if user.status not in ("new", "cancelled"): | ||||||||||||||||||||||
| error = self.env._("Cannot delete active listing") | ||||||||||||||||||||||
| raise UserError(error) | ||||||||||||||||||||||
|
Comment on lines
+167
to
+168
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. Don't spend the extra step putting it in a variable 👍
Suggested change
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 on the two next methods |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # ACTIONS | ||||||||||||||||||||||
|
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
|
||||||||||||||||||||||
| def action_set_sold(self): | ||||||||||||||||||||||
| self.ensure_one() # this button should only exist on a form | ||||||||||||||||||||||
|
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 reason for half the
Suggested change
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 on the next method |
||||||||||||||||||||||
| if not self.active: | ||||||||||||||||||||||
| error = self.env._("Cannot set inactive/cancelled listing to sold") | ||||||||||||||||||||||
| raise UserError(error) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self.status = "sold" | ||||||||||||||||||||||
| return True | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def action_set_cancelled(self): | ||||||||||||||||||||||
| self.ensure_one() # this button should only exist on a form | ||||||||||||||||||||||
| if self.status == "sold": | ||||||||||||||||||||||
| error = self.env._("Cannot set sold listing to cancelled") | ||||||||||||||||||||||
| raise UserError(error) | ||||||||||||||||||||||
| self.status = "cancelled" | ||||||||||||||||||||||
| return True | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def action_uncancel(self): | ||||||||||||||||||||||
| self.ensure_one() | ||||||||||||||||||||||
| self.status = "new" | ||||||||||||||||||||||
| return True | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # BUSINESS LOGIC | ||||||||||||||||||||||
|
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
|
||||||||||||||||||||||
| def set_offer_received(self): | ||||||||||||||||||||||
| for estate in self: | ||||||||||||||||||||||
| estate.status = "offer_received" | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,137 @@ | ||||||||||||||||||
| from datetime import timedelta | ||||||||||||||||||
|
|
||||||||||||||||||
| from odoo.exceptions import UserError | ||||||||||||||||||
| from odoo import models, fields, api | ||||||||||||||||||
|
Comment on lines
+1
to
+4
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. Keep this piece of documentation close, it's very (very very very) useful 😄
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class EstatePropertyOffer(models.Model): | ||||||||||||||||||
| _name = "estate.property.offer" | ||||||||||||||||||
| _description = "tech training estate property tag" | ||||||||||||||||||
|
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. At least the uppercases then any name you want 👍
Suggested change
|
||||||||||||||||||
| _order = "price desc" | ||||||||||||||||||
|
|
||||||||||||||||||
| # RELATIONAL FIELDS | ||||||||||||||||||
|
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
|
||||||||||||||||||
| estate_property_id = fields.Many2one( | ||||||||||||||||||
| "estate.property", | ||||||||||||||||||
| ondelete="cascade", | ||||||||||||||||||
| required=True, | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
|
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. Again a lot of blank spaces that we will just not keep in a final version before merge 😄 |
||||||||||||||||||
| property_type_id = fields.Many2one( | ||||||||||||||||||
| related="estate_property_id.estate_property_type_id", | ||||||||||||||||||
| store=True, | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| buyer_id = fields.Many2one( | ||||||||||||||||||
| "res.partner", | ||||||||||||||||||
| string="Partner", | ||||||||||||||||||
| ondelete="cascade", | ||||||||||||||||||
| default=lambda self: self.env.user.partner_id.id, | ||||||||||||||||||
| required=True, | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| # BASIC FIELDS | ||||||||||||||||||
|
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
|
||||||||||||||||||
| price = fields.Float( | ||||||||||||||||||
| string="Price", | ||||||||||||||||||
| required=True, | ||||||||||||||||||
| aggregator="max", | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| status = fields.Selection( | ||||||||||||||||||
| [["new", "New"], ["refused", "Refused"], ["accepted", "Accepted"]], | ||||||||||||||||||
|
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. List of tuples and this one could be multilines I feel, up to you really 👍 |
||||||||||||||||||
| default="new", | ||||||||||||||||||
| copy=False, | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| # COMPUTED FIELDS | ||||||||||||||||||
|
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
|
||||||||||||||||||
| validity = fields.Integer(default=7) | ||||||||||||||||||
|
|
||||||||||||||||||
| date_deadline = fields.Date(string="Deadline", compute="_compute_date_deadline", inverse="_inverse_date_deadline") | ||||||||||||||||||
|
|
||||||||||||||||||
| # SQL CONSTRAINTS & INDEXES | ||||||||||||||||||
|
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
|
||||||||||||||||||
| _check_positive_offer = models.Constraint( | ||||||||||||||||||
| "CHECK(price > 0)", | ||||||||||||||||||
| "Offer price must be greater than 0" | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| # COMPUTE & INVERSE & SEARCH | ||||||||||||||||||
|
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
|
||||||||||||||||||
| @api.depends("validity") | ||||||||||||||||||
| def _compute_date_deadline(self): | ||||||||||||||||||
| for estate in self: | ||||||||||||||||||
| if estate.create_date: | ||||||||||||||||||
| estate.date_deadline = estate.create_date + timedelta(days=estate.validity) | ||||||||||||||||||
| else: | ||||||||||||||||||
| estate.date_deadline = fields.Date.today() + timedelta(days=estate.validity) | ||||||||||||||||||
|
Comment on lines
+60
to
+63
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. Personnal preference but since you don't expect an elif here, I would just :
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| def _inverse_date_deadline(self): | ||||||||||||||||||
| for estate in self: | ||||||||||||||||||
| new_validity = (estate.date_deadline - fields.Date.today()).days | ||||||||||||||||||
| if new_validity < 0: | ||||||||||||||||||
| raise UserError("validity < 0") | ||||||||||||||||||
|
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. Any user error and even more generally any string that will be displayed to the user needs to be a translatable sentence 👍
Suggested change
|
||||||||||||||||||
| estate.validity = (estate.date_deadline - fields.Date.today()).days | ||||||||||||||||||
|
|
||||||||||||||||||
| # ORM OVERRIDES | ||||||||||||||||||
|
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
|
||||||||||||||||||
| @api.model | ||||||||||||||||||
| def create(self, vals): | ||||||||||||||||||
| for new_offer in vals: | ||||||||||||||||||
| current_property_offers_price_asc = self.env["estate.property.offer"].search( | ||||||||||||||||||
| [('estate_property_id', '=', new_offer.get("estate_property_id"))] | ||||||||||||||||||
| ).sorted("price asc") | ||||||||||||||||||
|
Comment on lines
+76
to
+78
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. Have you tried to only get the maximum current offer with current_lowest_offer = self.env["estate.property.offer"].search(
[
('estate_property_id', '=', new_offer.get("estate_property_id"))
], order="price asc", limit=1,
)Could be helpful here 👀 |
||||||||||||||||||
|
|
||||||||||||||||||
| estate_property = self.env["estate.property"].browse(new_offer.get("estate_property_id")) | ||||||||||||||||||
| if not current_property_offers_price_asc or len(current_property_offers_price_asc) == 0: | ||||||||||||||||||
| if estate_property.status == "new": | ||||||||||||||||||
| estate_property.set_offer_received() | ||||||||||||||||||
| return super().create(vals) | ||||||||||||||||||
|
|
||||||||||||||||||
| if new_offer.get("price") < current_property_offers_price_asc[0].price: | ||||||||||||||||||
| raise UserError("submitted offer < lowest current offer") | ||||||||||||||||||
|
|
||||||||||||||||||
| if estate_property.status == "new": | ||||||||||||||||||
| estate_property.set_offer_received() | ||||||||||||||||||
| return super().create(vals) | ||||||||||||||||||
|
|
||||||||||||||||||
| @api.model | ||||||||||||||||||
| def write(self, vals): | ||||||||||||||||||
| updated_price = vals.get("price") | ||||||||||||||||||
| if not updated_price: | ||||||||||||||||||
| return super().write(vals) # just skip, since logic only applies if there's updated price | ||||||||||||||||||
|
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
|
||||||||||||||||||
|
|
||||||||||||||||||
| current_property_offers_price_asc = self.estate_property_id.estate_property_offer_ids.sorted("price asc") | ||||||||||||||||||
|
|
||||||||||||||||||
| if self == current_property_offers_price_asc[0]: | ||||||||||||||||||
| return super().write(vals) | ||||||||||||||||||
|
|
||||||||||||||||||
| if updated_price < current_property_offers_price_asc[0].price: | ||||||||||||||||||
| raise UserError("submitted offer < lowest current offer") | ||||||||||||||||||
|
|
||||||||||||||||||
| return super().write(vals) | ||||||||||||||||||
|
|
||||||||||||||||||
| # ACTIONS | ||||||||||||||||||
|
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
|
||||||||||||||||||
| def action_accept_offer(self): | ||||||||||||||||||
| self.ensure_one() | ||||||||||||||||||
| for offer in self: | ||||||||||||||||||
| if fields.Date.today() > offer.date_deadline: | ||||||||||||||||||
| raise UserError("offer expired") | ||||||||||||||||||
|
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. UserError -> Translatable |
||||||||||||||||||
|
|
||||||||||||||||||
| estate = offer.estate_property_id | ||||||||||||||||||
| for recur_offer in estate.estate_property_offer_ids: | ||||||||||||||||||
| if recur_offer.status == "accepted": | ||||||||||||||||||
| # more foolproof than just checking estate_property.status | ||||||||||||||||||
|
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. Not sure this one would be kept or not but I would at least reformulate it 🤔
Suggested change
or even if you it more professional
Suggested change
|
||||||||||||||||||
| raise UserError("another offer already accepted") | ||||||||||||||||||
|
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. Translatable |
||||||||||||||||||
|
|
||||||||||||||||||
| estate.buyer_id = offer.buyer_id | ||||||||||||||||||
| estate.selling_price = offer.price | ||||||||||||||||||
| estate.status = "offer_accepted" | ||||||||||||||||||
|
|
||||||||||||||||||
| offer.status = "accepted" | ||||||||||||||||||
| return True | ||||||||||||||||||
|
|
||||||||||||||||||
| def action_refuse_offer(self): | ||||||||||||||||||
| self.ensure_one() | ||||||||||||||||||
| self.status = "refused" | ||||||||||||||||||
|
|
||||||||||||||||||
| estate = self.estate_property_id | ||||||||||||||||||
| estate.buyer_id = None | ||||||||||||||||||
| estate.selling_price = None | ||||||||||||||||||
| estate.status = "offer_received" | ||||||||||||||||||
| return True | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from odoo import models, fields | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class EstatePropertyTag(models.Model): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _name = 'estate.property.tag' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _description = 'tech training estate property tag' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _order = 'name asc' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # RELATIONAL FIELDS | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| estate_property_ids = fields.Many2many( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "estate.property", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # BASIC FIELDS | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name = fields.Char( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Tag", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| color = fields.Integer( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string="Color", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default=0, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # SQL CONSTRAINTS & INDEXES | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _check_unique_name = models.UniqueIndex( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "(UPPER(name))", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Tag should be unique" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+9
to
+29
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. This file is a nice place for a full example 😄
Suggested change
It'll look more like this in 99.9% of the files 👍 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
As an odoo employee, you can't sign your work in your own name, the author of this module is "Odoo S.A." 😄