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] estate: a new module to manage estate properties #164

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

Conversation

ouou-odoo
Copy link

@ouou-odoo ouou-odoo commented Oct 21, 2024

We should add an estate module for real estates companies, here is a small proof of work module.

Features :

  • Creating properties, types of properties
  • Handling customers offers
  • Sorting your properties by tags or by type
  • Generating invoices when properties are sold
  • and much more...

@aboo-odoo
Copy link

Ping

Lucky you I'm your reviewer :D

@aboo-odoo
Copy link

Ah ouais, ça delete mon commit comme ça ... :peppa_sad:

Copy link

@aboo-odoo aboo-odoo left a comment

Choose a reason for hiding this comment

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

Very good work keep going 🏅

I've been extra picky and nitpicked a lot but honestly most of this is just styling stuff.

Two last comments I can make so far are:

  • Could you rename your PR to something like [ADD] estate: a new module to manage estate properties and embellish a bit your description
  • Mind that commit messages (and PR title) must be as followed [TAG] module(s): short description. The body of your commit message is as important as the title. I advice you to build that body use the following three sections:
  1. Problem
  2. Objective
  3. Solution
    and end your commit message with a reference to the task your assigned to (here there is none so you can put task-xxxxxx)
    Check the commit message I pushed before on your branch for inspiration.

Once again, extremely good work. Keep it up 🥇

Comment on lines 6 to 13
'data': ['data/ir.model.access.csv',
'views/estate_property_views.xml',
'views/estate_menus.xml',
'views/estate_property_type_views.xml',
],

Choose a reason for hiding this comment

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

Sightly nicer indentation IMO

Suggested change
'data': ['data/ir.model.access.csv',
'views/estate_property_views.xml',
'views/estate_menus.xml',
'views/estate_property_type_views.xml',
],
'data': [
'data/ir.model.access.csv',
'views/estate_property_views.xml',
'views/estate_menus.xml',
'views/estate_property_type_views.xml',
],

access_estate_prop,access_estate_prop,model_estate_property,base.group_user,1,1,1,1
access_estate_prop_type,access_estate_prop_type,model_estate_property_type,base.group_user,1,1,1,1
access_estate_prop_offer,access_estate_prop_offer,model_estate_property_offer,base.group_user,1,1,1,1
access_estate_prop_tag,access_estate_prop_tag,model_estate_property_tag,base.group_user,1,1,1,1

Choose a reason for hiding this comment

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

Even though runbot does not tag it red, it's better to always add a line break at the end of a file. This makes further modification and git diff smaller later on when a line is added

@@ -0,0 +1 @@
from . import estate_property, estate_property_type, estate_property_tag, estate_property_offer

Choose a reason for hiding this comment

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

Same reason as previously, it helps reduce the diff later and ease reviewing process if someone modifies the lines later on

Suggested change
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

from odoo.tools import float_compare
from .helper import format_selection

from odoo.fields import Many2many, One2many

Choose a reason for hiding this comment

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

Not needed, just use fields.Many2many directly since it is already imported

Comment on lines 1 to 6
from odoo import api, fields, models
from dateutil.relativedelta import relativedelta

from odoo.exceptions import UserError, ValidationError
from odoo.tools import float_compare
from .helper import format_selection

Choose a reason for hiding this comment

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

Regarding imports, we try to order them as followed

  1. Python imports
  2. Other imports
  3. Odoo imports
Suggested change
from odoo import api, fields, models
from dateutil.relativedelta import relativedelta
from odoo.exceptions import UserError, ValidationError
from odoo.tools import float_compare
from .helper import format_selection
from dateutil.relativedelta import relativedelta
from odoo import api, fields, models
from odoo.exceptions import UserError, ValidationError
from odoo.tools import float_compare
from .helper import format_selection

@@ -0,0 +1,111 @@
<odoo>
<data>
<record id="property_view_list" model="ir.ui.view">

Choose a reason for hiding this comment

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

name

</field>
</record>

<record id="property_view_form" model="ir.ui.view">

Choose a reason for hiding this comment

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

name

</field>
</record>

<record id="property_view_search" model="ir.ui.view">

Choose a reason for hiding this comment

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

name

<field name="facades"/>
<filter name="Available properties"
domain="['|', ('state', '=', 'new'), ('state', '=', 'offer received')]"/>
<filter name="Postal code" domain="[]" context="{'group_by': 'postcode'}"/>

Choose a reason for hiding this comment

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

Suggested change
<filter name="Postal code" domain="[]" context="{'group_by': 'postcode'}"/>
<filter name="Postal code" context="{'group_by': 'postcode'}"/>

</field>
</record>

<record id="action_property" model="ir.actions.act_window">

Choose a reason for hiding this comment

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

name

@ouou-odoo ouou-odoo changed the title Avancée [ADD] estate: a new module to manage estate properties and embellish a bit your description Oct 23, 2024
@ouou-odoo ouou-odoo changed the title [ADD] estate: a new module to manage estate properties and embellish a bit your description [ADD] estate: a new module to manage estate properties Oct 24, 2024
Features :

- Creating properties, types of properties
- Handling customers offers
- Sorting your properties by tags or by type
- Generating invoices when properties are sold and much more...
estate_account/models/estate_property.py Show resolved Hide resolved
Command.create({'name': 'Administrative fees', 'price_unit': fees, 'quantity': 1}),
]

journal = self.env['account.journal'].search([('type', '=', 'sale')])

Choose a reason for hiding this comment

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

This could return more than one journal, add limit=1 to make sure


def action_sold(self):
if not (self.buyer and self.selling_price):
raise UserError('Customer or selling price was not set.')

Choose a reason for hiding this comment

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

I noticed your error messages are not getting translation

Add _() -> from odoo import _ to make them translatable

Problem:

The errors were not automatically translated.

Solution:

Imported _ from odoo and added _ to each error.
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.

2 participants