Skip to content

[ADD] estate: adding new estate module #727

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

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

Conversation

amry-odoo
Copy link

Add manifest and dependencies...

@robodoo
Copy link

robodoo commented Apr 22, 2025

Pull request status dashboard

Copy link

@SergeBayet SergeBayet left a comment

Choose a reason for hiding this comment

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

Great first PR! Some remarks though 👍

@@ -0,0 +1 @@
from . import models

Choose a reason for hiding this comment

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

Missing blank line at the end of your file.

'depends': [
'base',
]
}

Choose a reason for hiding this comment

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

Same here.

garden_area = fields.Integer()
garden_orientation = fields.Selection(
string='Orientation',
selection=[('north','North'), ('south','South'), ('east', 'East'), ('west', 'West')]

Choose a reason for hiding this comment

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

Suggested change
selection=[('north','North'), ('south','South'), ('east', 'East'), ('west', 'West')]
selection=[('north', 'North'), ('south', 'South'), ('east', 'East'), ('west', 'West')]

Add manifest and dependencies...
@amry-odoo amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from fd89ef7 to 0b23021 Compare April 22, 2025 13:28
Copy link

@SergeBayet SergeBayet left a comment

Choose a reason for hiding this comment

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

You're probably need a linter in your IDE to avoid these kind of errors. See below.

@amry-odoo amry-odoo marked this pull request as ready for review April 23, 2025 11:26
@amry-odoo amry-odoo requested a review from SergeBayet April 23, 2025 13:33
Copy link

@SergeBayet SergeBayet left a comment

Choose a reason for hiding this comment

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

Ok for me. Just a little typo :)

Copy link

@SergeBayet SergeBayet left a comment

Choose a reason for hiding this comment

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

Good! 👍

@SergeBayet
Copy link

@naja628 For you :)

Copy link

@naja628 naja628 left a comment

Choose a reason for hiding this comment

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

Hello :) -- Good work !

You have one commit left whose title doesn't conform with the guidelines.

I left some more comments on your code

name = fields.Char(required=True)
description = fields.Text()
postcode = fields.Char()
date_availability = fields.Date(copy=False, default=du.add(date.today(), months=3))
Copy link

Choose a reason for hiding this comment

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

Here date.today() will be evaled once when the whole class is evaled (i.e. everytime you launch odoo i think).
When it needs to be re-evaled everytime you create new records you can pass a lambda or function to the default kwarg instead

@amry-odoo amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from d06d33d to 313ec7d Compare April 25, 2025 10:29
Added a functional module to create new listings following all the chapter 5 guidelines.
Chapter 6 views: search, list, form done
Repo now is up to date with chapter 7
Added an onchange field and some compute fields, following Chapter 8 rules. Also refactored different views to separate them
Added actions as mentioned in chapter 9
did some refactoring to follow latest comments
@amry-odoo amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 313ec7d to 3a9a5a2 Compare April 25, 2025 10:30
Added constraints to follow chapter 10
Refactored to fix a bug in chapter 9
Made some adjustments to follow chapter 11 guidelines
@amry-odoo amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 3108bd3 to 6316118 Compare April 30, 2025 08:13
Repo now represents edits made to follow chapter 12 guidelines, inherited view and class created and CRUD modifications were made
@amry-odoo amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 1688c2c to 6f9cc29 Compare April 30, 2025 09:58
estate_account linking module created, now follows chapter 13 guidelines
@amry-odoo amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 99c4242 to 3c56016 Compare April 30, 2025 14:49
Copy link

@naja628 naja628 left a comment

Choose a reason for hiding this comment

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

Hello -- good work :)

I left some comments

def create(self, vals):
for val in vals:
single_property = self.env["estate.property"].browse(val["property_id"])
if val["price"] < single_property.best_price:
Copy link

Choose a reason for hiding this comment

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

here price may not always be in the vals. You could use .get

Guidelines follow chapter 14
@amry-odoo amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 0a1d15b to 203b4b9 Compare April 30, 2025 15:40
@amry-odoo amry-odoo force-pushed the 18.0-tutorial-estate-amry branch from 45f1d4e to bbd1d95 Compare May 2, 2025 10:58
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