-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Estate Beginning #948
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?
Estate Beginning #948
Conversation
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.
Nice job, I had some comments mostly about formatting. You can think about formatting your code automatically using Ruff.
Please update the commit messages according to https://www.odoo.com/documentation/19.0/contributing/development/git_guidelines.html#commit-message-structure |
…ffers, to avoid wrong inputs [FIX] Estate: Fixed business logic error that assumed we always had multiple offers
…ake everything feel more fluid
…the states of the property. Also added properties in the view of users
from . import ( | ||
estate_property, | ||
estate_property_type, | ||
estate_property_tag, | ||
estate_property_offer, | ||
) |
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.
from . import ( | |
estate_property, | |
estate_property_type, | |
estate_property_tag, | |
estate_property_offer, | |
) | |
from . import estate_property, | |
from . import estate_property_type, | |
from . import estate_property_tag, | |
from . import estate_property_offer, |
I think this is the way to do it because these files might depend on each other in the future
estate/models/estate_property.py
Outdated
@@ -1,22 +1,74 @@ | |||
from odoo import fields, models | |||
from datetime import date, timedelta | |||
from odoo import fields, models, api |
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.
from odoo import fields, models, api | |
from odoo import api, fields, models |
runbot usually complains that these imports should be in alphabetical order
estate_property_type, | ||
estate_property_tag, | ||
estate_property_offer, | ||
inherited_users, |
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.
The inherited models should have the same file name. - res_users.py
"views/estate_property_tag_views.xml", | ||
"views/estate_property_offer_views.xml", | ||
"views/menus.xml", | ||
"views/base_inherited_users_views.xml", |
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.
Should have the same name everywhere res_users_views.xml
You should add inherit
while naming the records
default=lambda self: fields.Date.today() + timedelta(days=90), | ||
) | ||
expected_price = fields.Float(required=True) | ||
expected_price = fields.Float(required=True, string="Expected Price") |
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.
Already the default string
expected_price = fields.Float(required=True, string="Expected Price") | |
expected_price = fields.Float(required=True) |
<data> | ||
|
||
<record id="res_users_view_form" model="ir.ui.view"> | ||
<field name="name">res.users.view.form.inherit.gamification</field> | ||
<field name="model">res.users</field> | ||
<field name="inherit_id" ref="base.view_users_form"/> | ||
<field name="arch" type="xml"> | ||
<xpath expr="//notebook" position="inside"> | ||
<page string="Real Estate Properties"> | ||
<group> | ||
<field name="property_ids"/> | ||
</group> | ||
</page> | ||
</xpath> | ||
</field> | ||
</record> | ||
|
||
</data> |
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.
should be indented
<field name="selling_price"/> | ||
</div> | ||
<div> | ||
<field name="tags_ids" widget="many2many_tags" options="{'color_field': 'color'}"/> |
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 missed this above I guess but the field name shouldn't be plural_plural here _ids is enough for us to understand it contains multiple records so, tag_ids is a better name
# "data": [ | ||
# "security/ir.model.access.csv", | ||
# "views/estate_property_views.xml", | ||
# "views/estate_property_type_views.xml", | ||
# "views/estate_property_tag_views.xml", | ||
# "views/estate_property_offer_views.xml", | ||
# "views/menus.xml", | ||
# "views/base_inherited_users_views.xml", | ||
# ], |
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.
Delete if not used
for record in self: | ||
values = {} | ||
values["partner_id"] = record.buyer_partner_id.id | ||
values["move_type"] = "out_invoice" | ||
values["invoice_line_ids"] = [ | ||
Command.create( | ||
{ | ||
"name": record.name, | ||
"quantity": 1, | ||
"price_unit": record.selling_price * (6 / 100), | ||
} | ||
), | ||
Command.create( | ||
{ | ||
"name": "Administrative fees", | ||
"quantity": 1, | ||
"price_unit": 100, | ||
} | ||
), | ||
] |
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: | |
values = {} | |
values["partner_id"] = record.buyer_partner_id.id | |
values["move_type"] = "out_invoice" | |
values["invoice_line_ids"] = [ | |
Command.create( | |
{ | |
"name": record.name, | |
"quantity": 1, | |
"price_unit": record.selling_price * (6 / 100), | |
} | |
), | |
Command.create( | |
{ | |
"name": "Administrative fees", | |
"quantity": 1, | |
"price_unit": 100, | |
} | |
), | |
] | |
values = { | |
"partner_id": record.something | |
"move_type": ..., | |
"invoice_line_ids": ..., | |
for record in self} |
), | ||
] | ||
|
||
self.env["account.move"].create(values) |
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.
don't create it in the loop, collect all values and create them after the loop
No description provided.