[ADD] module: add new module real estate (Onboarding)#1238
[ADD] module: add new module real estate (Onboarding)#1238
Conversation
|
|
yogietama
force-pushed
the
19.0-onboarding-yomet
branch
from
2dd0080 to
5d0a3ce
Compare
yogietama
force-pushed
the
19.0-onboarding-yomet
branch
from
5d0a3ce to
aeda88e
Compare
|
Hello there ! Good work already, just added a few comments. Also it seems your first commit has the wrong author set (most probably because your config was not properly set before pushing, might want to check into this as a general git exercise) |
delcourtfl
left a comment
delcourtfl
left a comment
There was a problem hiding this comment.
Hello there ! Good work already, just a few comments.
(Might also need to check runbot style CI for some additional issues)
| _check_expected_price = models.Constraint( | ||
| 'CHECK(expected_price > 0)', | ||
| 'The expected price must be strictly positive', | ||
| ) |
There was a problem hiding this comment.
Always try to keep the fields declaration before the constraints (see https://www.odoo.com/documentation/19.0/contributing/development/coding_guidelines.html#symbols-and-conventions for the model attribute order)
| # if offer percentage lower then 90% | ||
| if float_compare(offer_percentage, | ||
| 0.9, precision_digits=2) == -1: | ||
| raise ValidationError(f"{m1}. {m2}") |
There was a problem hiding this comment.
I believe it's better to keep the error message in the error call (or just beside it, unless it repeats) to avoid having to look back and forth in the code to understand the issue.
| raise ValidationError(f"{m1}. {m2}") | |
| raise ValidationError( | |
| "The selling price cannot be lower than 90% of the expected price\n" | |
| "You must reduce the expected price if you want to accept this offer" | |
| ) |
| if record.selling_price > 0: | ||
| offer_percentage = record.selling_price / record.expected_price | ||
| # if offer percentage lower then 90% | ||
| if float_compare(offer_percentage, | ||
| 0.9, precision_digits=2) == -1: | ||
| raise ValidationError(f"{m1}. {m2}") |
There was a problem hiding this comment.
I believe in practice it would be better to keep the multiplication instead of the division to avoid x / 0 issue (expected_price is not checked here). Also try using the float_is_zero to ensure there is no issue with floats (and you can format function calls in a cleaner way):
| if record.selling_price > 0: | |
| offer_percentage = record.selling_price / record.expected_price | |
| # if offer percentage lower then 90% | |
| if float_compare(offer_percentage, | |
| 0.9, precision_digits=2) == -1: | |
| raise ValidationError(f"{m1}. {m2}") | |
| if float_is_zero(record.selling_price, precision_digits=2): | |
| continue | |
| min_price = 0.9 * property.expected_price | |
| if float_compare(record.selling_price, min_price, precision_digits=2) == -1: | |
| raise ValidationError("...") |
| def action_accepted(self): | ||
| for record in self: | ||
| if record.property_id.selling_price == 0: | ||
| record.status = "accepted" |
There was a problem hiding this comment.
Check if it doesn't already have an accepted offer ?
| else: | ||
| return |
| raise ValidationError( | ||
| "You're not eligible to refused an accepted offer") |
There was a problem hiding this comment.
I prefer this notation but I think both are okay in the codebase
| raise ValidationError( | |
| "You're not eligible to refused an accepted offer") | |
| raise ValidationError( | |
| "You're not eligible to refused an accepted offer" | |
| ) |
| 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 No newline at end of file |
There was a problem hiding this comment.
New line needed here 👀 (it's the case in all odoo files)
| 'summary': 'Track leads and close opportunities', | ||
| 'website': 'https://www.odoo.com/app/estate', | ||
| 'depends': [ | ||
| 'base' |
There was a problem hiding this comment.
Nitpicking, add what is needed to easily add new elements later on:
| 'base' | |
| 'base', |
This commit is here to introduce the testing framework of Odoo. Try running the tests using `--test-tags :TestEstateProperty`. Doc: https://www.odoo.com/documentation/18.0/developer/reference/backend/testing.html?highlight=tests#invocation The tests were made such that the first one should work but the second one should fail. Your job is to ensure both tests pass in the end. You should update the behaviour of the appropriate models. If you want, you can also add a small test of your own to get a feel for it. Chapter 15: The final word
yogietama
force-pushed
the
19.0-onboarding-yomet
branch
from
0bcf5fe to
feb7edb
Compare
4 participants
Create Real Estate Module for onboarding program