luleg - Technical Training - Creation of the Real Estate Module#1176
luleg - Technical Training - Creation of the Real Estate Module#1176
Conversation
AlessandroLupo
left a comment
There was a problem hiding this comment.
Nice work! 👍
Ideally, we should have a single commit per tutorial chapter. But I see that you introduced some code in advance (for example, EstateTagCategory is introduced in chapter 7, right?). Is there a specific reason for this? As a git exercise, you could try to move some of this content to the right commit when you reach the correct chapter.
It is very good that you fixed the styling error. You could try to do it in the already existing commits instead of adding a new one.
Depending on your confidence with git, this may be boring or may be very interesting. Feel free to ask for help if you need :)
|
Thank you for the feedbacks! I kinda just go and find what I need for what I want to do and do it ^^' Well, about moving parts of the code... it would probably be pretty annoying, I know a bit of git but in no way in hell do I already know how to do that in particular (moving code around commits) (especially now that it's pushed and assuming I don't force push, which I would love not to do). |
|
Good job 👍 I see your runbot is red, please try to fix the problems and make it green. It's just a styling error and a problem about an invisible element, feel free to ask for help :) |
8347447 to
0fedb0f
Compare
… notebook to the form with more infos.
…y2one, addition of property offer and completion of the property page
0fedb0f to
adf087f
Compare
Automatic calculation of validity/deadline, total area/living area/garden area, and best price
29b5cc1 to
48de967
Compare
c98800a to
4d76898
Compare
AlessandroLupo
left a comment
There was a problem hiding this comment.
Nice work with the currency conversion part. Just a few minor comments 👍
| property_type_id = fields.Many2one("estate.property.type", string="Property Type", related="property_id.property_type_id") | ||
|
|
||
| # Beginning of the deadline part | ||
| # Deadline Part |
There was a problem hiding this comment.
It is a good practice to leave comments on your code. But consider cleaning them a bit. In this case, you have "deadline part", then a bit after you have "beginning of the deadline part" and then "end of the deadline part". In most of the cases, commenting only the beginning of a block is enough :)
estate/security/ir.model.access.csv
Outdated
| access_estate_property_offer,access_estate_property_offer,model_estate_property_offer,base.group_user,1,1,1,1 | ||
| access_estate_tag,access_estate_tag,model_estate_tag,base.group_user,1,1,1,1 | ||
| access_estate_tag_category,access_estate_tag_category,model_estate_tag_category,base.group_user,1,1,1,1 No newline at end of file | ||
| access_estate_tag,access_estate_tag,model_estate_tag,base.group_user,1,1,1,1 No newline at end of file |
There was a problem hiding this comment.
Empty line is missing at the end of file ;)
| <field name="translated_price" options="{'currency_field': 'property_currency_id'}" /> | ||
| <field name="price" options="{'currency_field': 'currency_id'}" /> |
There was a problem hiding this comment.
I found a bit confusing that both "Price" and "Original price" are shown here. Also, "price" is shown without currency symbol. Maybe consider showing only the original price?
There was a problem hiding this comment.
What's shown on top is the best offer in the same currency as the expected sell price, so, for commercials to be able to know which offer is the best one without having to do the conversion themselves, I need to show the translated price next to the offered price.
You could say that you don't need it since the offers are ordered related to the translated price, but then how do you know how much less would be the second best offer?
There was a problem hiding this comment.
Oh okay now I see your point. You could use something like "price in the local currency" just to make it clearer, but that's up to you 👍
3db0868 to
bb74433
Compare
AlessandroLupo
left a comment
There was a problem hiding this comment.
Good job. You're not the first one having a problem with the one2many domain in ResUsers, see my comment :D
| @api.ondelete(at_uninstall=False) | ||
| def _unlink_except_if_advanced_stage(self): | ||
| for property in self: | ||
| if not property.stage in ["new", "cancelled"]: |
There was a problem hiding this comment.
or if property.state not in [....]
|
|
||
| # Domain doesn't work | ||
| property_ids = fields.One2many("estate.property", "seller_id", string="Properties List", | ||
| domain="[('available_from', '<=', 'today'), ('stage', '!=', 'cancelled'), ('stage', '!=', 'sold')]") |
There was a problem hiding this comment.
Domain doesn't work because it is supposed to be a list, not a string :D
Also consider doing ('stage', 'not in', ('cancelled', 'sold'))
| <xpath expr="//notebook" position="inside"> | ||
| <page string="Real Estate Properties"> | ||
| <!-- The filter doesn't work, I'm too sick for this. To fix later --> | ||
| <field name="property_ids" readonly="1" domain="[('available_from', '>=', 'today'), ('stage', '!=', 'cancelled'), ('stage', '!=', 'sold')]"/> |
There was a problem hiding this comment.
I think you won't need the domain here anymore after having fixed the one in ResUsers

Here is my progress on the Technical training
Don't hesitate to give me feedbacks about what I could improve.