-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
17.0 estate azey #125
base: 17.0
Are you sure you want to change the base?
17.0 estate azey #125
Conversation
Ping |
705769b
to
0804a8b
Compare
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.
Good job so far, I've been extra picky so don't take things personally 🏅
- Rename your commit PR title to something like [ADD] estate: blablabla and also add a description
- Add the string parameter to the fields so that it can get translated, it also make it a bit more understandable
- I know this is a training but beware of spelling.
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.
- Mind the ID naming convention, the guidelines are here
b6ad338
to
62fe36d
Compare
[ADD] estate: adding first action [ADD] estate: ading mnus [ADD] estate: This is my first commit using guidlines In this one there my code for chapters from 1 to 6. [IMP] estate: Imroving style [IMP] estate: improving style'
62fe36d
to
75606fb
Compare
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.
Very good job! a few last things:
- Could you rename the PR title and add a description to your PR
- Squash all your commits into one
for record in self: | ||
record.total_area = record.living_area + record.garden_area |
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.
I know they use record
in the tutorial, but something more meaningful, like estate
will make your code much more readable in the long run
for record in self: | ||
record.best_price = max(record.offer_ids.mapped('price'), default=0) |
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.
same here
for record in self: | |
record.best_price = max(record.offer_ids.mapped('price'), default=0) | |
for estate in self: | |
estate.best_price = max(estate.offer_ids.mapped('price'), default=0) |
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.
Btw, I didn't know max
took a default value :o Thanks for showing me :D
|
||
def action_sold_property(self): | ||
if "canceled" in self.mapped("state"): | ||
raise UserError("Canceled properties cannot be sold.") |
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.
Like for the string parameter in the fields, we try to make all strings translatable. Usually, it is done automatically, like for the fields name. In exceptions, it is not done automatically yet. To set a manual string as translatable, you have to import _
-> from odoo import _
and use it as such
raise UserError("Canceled properties cannot be sold.") | |
raise UserError(_("Canceled properties cannot be sold.")) |
p.s: could you do it for all the other error messages as well
self.garden_orientation = False | ||
|
||
def action_sold_property(self): | ||
if "canceled" in self.mapped("state"): |
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.
This method may be called on several records (self
is multiple records). There are two ways to deal with this:
1 - either like you've done in the compute methods with a for estate in self:
loop.
2 - or, by adding self.ensure_one()
at the beginning of your method. This triggers an error if there are more or less than one record.
Usually, we tend to prefer the 1st option when possible to improve performances of the server.
raise UserError("Canceled properties cannot be sold.") | ||
return self.write({"state": "sold", "active": False}) | ||
|
||
def action_cancel_property(self): |
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.
Some for this method about the several records
} | ||
) | ||
|
||
def action_refused(self): |
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.
Same comment about multiple records
|
||
@api.constrains("price", "property_id.expected_price") | ||
def check_price(self): | ||
for record in self: |
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.
for record in self: | |
for estate in self: |
date = offer.create_date.date() if offer.create_date else fields.Date.today() | ||
offer.validity = (offer.date_deadline - date).days | ||
|
||
def action_accepted(self): |
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.
Also, shouldn't this method refuse all other offers if one has been accepted ?
|
||
class estatePropertyTag(models.Model): | ||
_name = "estate.property.tag" | ||
_description = "This module introduces property tags to the real estate model, allowing properties to be categorized with multiple descriptive tags like 'cozy' or 'renovated' using a many-to-many relationship." |
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.
_description = "This module introduces property tags to the real estate model, allowing properties to be categorized with multiple descriptive tags like 'cozy' or 'renovated' using a many-to-many relationship." | |
_description = """ | |
This module introduces property tags to the real estate model, allowing | |
properties to be categorized with multiple descriptive tags like 'cozy' | |
or 'renovated' using a many-to-many relationship. | |
""" |
|
||
@api.depends("offer_ids") | ||
def _compute_offers(self): | ||
for record in self: |
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.
for record in self: | |
for property_type in self: |
No description provided.