Skip to content
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

[ADD] created estate app #351

Open
wants to merge 14 commits into
base: 18.0
Choose a base branch
from
Open

Conversation

duau-odoo
Copy link

No description provided.

@robodoo
Copy link

robodoo commented Feb 17, 2025

Pull request status dashboard

Copy link

@rugo-odoo rugo-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things.

bedrooms = fields.Integer("Bedrooms", default=2)
living_area = fields.Integer("Living Area (sqm)")
facades = fields.Integer("Facades")
garage = fields.Boolean("Garage")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, the label will take the name of the field and capitalize it. So in this case it will be, by default, Garage. So we don't need to manually add it.

Comment on lines 20 to 25
garden_orientation = fields.Selection(
[("north", "North"),
("south", "South"),
("east", "East"),
("west", "West")],
"Garden Orientation")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
garden_orientation = fields.Selection(
[("north", "North"),
("south", "South"),
("east", "East"),
("west", "West")],
"Garden Orientation")
garden_orientation = fields.Selection([
("north", "North"),
("south", "South"),
("east", "East"),
("west", "West"),
], "Garden Orientation")

The advantage of doing it like this is that if we need to add a value, we won't break git history.

Comment on lines 75 to 82
<filter name="available" string="Available" domain="[
'|',
('status', '=', 'new'),
('status', '=', 'offer_received')]"
/>
<group expand="1" string="Group By">
<filter string="Postcode" name="postcode" context="{'group_by':'postcode'}"/>
</group>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some extra indents in here.

Copy link

@hupo-odoo hupo-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! It's impressive what you've done in 2 days 😄

Comment on lines 105 to 107
'|',
('status', '=', 'new'),
('status', '=', 'offer_received')]"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'|',
('status', '=', 'new'),
('status', '=', 'offer_received')]"
('status', 'in', ('new','offer_received'))


def accept_offer(self):
for record in self:
if record.property_id.offer_ids.mapped("status").count("accepted") >= 1:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if record.property_id.offer_ids.mapped("status").count("accepted") >= 1:
if offer.property_id.offer_ids.filtered(lambda o: o.status == 'accepted'):

using filtered is cleaner

record.date_deadline = fields.Date.add(fields.Date.today(), days=record.validity)

def accept_offer(self):
for record in self:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for record in self:
for offer in self:

record is too vague and when used in huge compute functions, you sometime forgot what self exactly is

Comment on lines 21 to 25
for record in self:
if record.create_date:
record.date_deadline = fields.Date.add(record.create_date, days=record.validity)
else:
record.date_deadline = fields.Date.add(fields.Date.today(), days=record.validity)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for record in self:
if record.create_date:
record.date_deadline = fields.Date.add(record.create_date, days=record.validity)
else:
record.date_deadline = fields.Date.add(fields.Date.today(), days=record.validity)
for offer in self:
create_date = offer.create_date or fields.Date.today()
record.date_deadline = fields.Date.add(create_date, days=record.validity)

a bit more concise 😄

Comment on lines 49 to 53
for record in self:
if record.offer_ids:
record.best_offer = max(record.offer_ids.mapped("price"))
else:
record.best_offer = 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for record in self:
if record.offer_ids:
record.best_offer = max(record.offer_ids.mapped("price"))
else:
record.best_offer = 0
for property in self:
property.best_offer = max(property.offer_ids.mapped("price")) if property.offer_ids else 0.0

or you could even do simpler:

property.best_offer = max(property.offer_ids.mapped("price") + [0.0])

for simple if else, better use one line

Also, best_offer is a Float, so it's best practice to assign the value 0.0 directly instead of 0

Comment on lines 65 to 68
for record in self:
if record.status == "sold":
raise exceptions.UserError("Sold properties cannot be cancelled")
record.status = "cancelled"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for record in self:
if record.status == "sold":
raise exceptions.UserError("Sold properties cannot be cancelled")
record.status = "cancelled"
if self.filtered(lambda property: property.status == "sold"):
raise exceptions.UserError("Sold properties cannot be cancelled")
self.status = "cancelled"

here you don't even need to loop on the records manually 😄
same comment applies for function below

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants