[ADD] estate: a new module to manage estate properties #163

malm-odoo · 2024-10-21T12:25:31Z

Module managing estate properties allowing addition of properties alongside their details (type, area, etc.). Keeps track of the offers for each property, prices for each offer, and the buyer.

aboo-odoo · 2024-10-21T12:26:42Z

Ping

Lucky you I'm your reviewer :D

aboo-odoo

Very good work, mainly styling comments 👍

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 on your branch for inspiration.

Lastly, have all your .idea diffs in the PRs, could you make those disappear ? Thanks a lot.

Once again, good work ! Keep it up 🥇

aboo-odoo · 2024-10-22T12:05:06Z

estate/models/estate_property.py

+from odoo import models, fields
+from dateutil.relativedelta import relativedelta


Regarding imports, we try to order them as followed:

Python imports

Other imports

Odoo imports

Suggested change

from odoo import models, fields

from dateutil.relativedelta import relativedelta

from dateutil.relativedelta import relativedelta

from odoo import fields, models

aboo-odoo · 2024-10-22T12:05:15Z

estate/models/estate_property.py

+
+
+class EstateProperty(models.Model):
+    _name = "estate_property"


Suggested change

_name = "estate_property"

_name = "estate.property"

aboo-odoo · 2024-10-22T12:06:47Z

estate/models/estate_property.py

+    _name = "estate_property"
+    _description = "Create real estate properties and keep track of status"
+
+    name = fields.Char(required=True)


Comment that also applies to all other fields: also add the string parameter so that the field can get translated to other languages 👍

aboo-odoo · 2024-10-22T12:08:05Z

estate/models/estate_property.py

+    garden = fields.Boolean()
+    garden_area = fields.Integer()
+    garden_orientation = fields.Selection(
+        string='Type',


Quote convention is

'' for strings the users do not see

"" for strings the users see

Suggested change

string='Type',

string="Type",

aboo-odoo · 2024-10-22T12:09:26Z

estate/models/estate_property.py

+    active = fields.Boolean('Active', default=True)
+    state = fields.Selection(
+        string='State',
+        selection=[('new', 'New'), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'), ('sold', 'Sold'), ('cancelled', "Cancelled")]


Since the selection is a bit long here, it would be nice to indent it to make it easier to read and manage in the future

Suggested change

selection=[('new', 'New'), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'), ('sold', 'Sold'), ('cancelled', "Cancelled")]

selection=[

('new', "New"),

('offer_received', "Offer Received"),

('offer_accepted', "Offer Accepted"),

('sold', "Sold"),

('cancelled', "Cancelled"),

]

aboo-odoo · 2024-10-22T12:12:37Z

estate/views/estate_property_views.xml

+        </field>
+    </record>
+
+    <record id="estate_property_list_view_search" model="ir.ui.view">


Suggested change

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

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

aboo-odoo · 2024-10-22T12:13:35Z

estate/views/estate_property_views.xml

+                <filter string="Available" name="state" domain="['|',
+                                                                ('state', '=', 'new'),
+                                                                ('state', '=', 'offer_received')]"/>


Easier to read

Suggested change

<filter string="Available" name="state" domain="['|',

('state', '=', 'new'),

('state', '=', 'offer_received')]"/>

<filter string="Available"

name="state"

domain="[

'|',

('state', '=', 'new'),

('state', '=', 'offer_received'),

]"/>

aboo-odoo · 2024-10-22T12:14:28Z

estate/views/estate_property_views.xml

+                <filter string="Available" name="state" domain="['|',
+                                                                ('state', '=', 'new'),
+                                                                ('state', '=', 'offer_received')]"/>
+                <group expand="1" string="Group By">


Not needed, try without and you'll see that the group_by section shows

aboo-odoo · 2024-10-22T12:15:13Z

estate/views/estate_property_views.xml

+                        <group>
+                            <field name="postcode"/>
+                        </group>
+                        <group>
+                            <field name="expected_price"/>
+                        </group>
+                        <group>
+                            <field name="date_availability"/>
+                        </group>
+                        <group>
+                            <field name="selling_price"/>
+                        </group>


No need to create a different group for each

Suggested change

<group>

<field name="postcode"/>

</group>

<group>

<field name="expected_price"/>

</group>

<group>

<field name="date_availability"/>

</group>

<group>

<field name="selling_price"/>

</group>

<group>

<field name="postcode"/>

<field name="expected_price"/>

<field name="date_availability"/>

<field name="selling_price"/>

</group>

aboo-odoo · 2024-10-22T12:15:23Z

estate/views/estate_property_views.xml

+            </form>
+        </field>
+    </record>
+</odoo>


aboo-odoo · 2024-10-22T13:26:16Z

estate/models/estate_property.py

+    name = fields.Char(required=True)
+    description = fields.Text()
+    postcode = fields.Char()
+    date_availability = fields.Date(default=fields.Datetime.today() + relativedelta(months=3), copy=False)


The default value here won't work, the reason being the, currently, it is compute once when you start your server and then the compute default value will remain unchanged until you next stop-start your server. That means that, currently, if you start a server today that will run forever, the default value will be 22/01 forever.

To solve the issue you need to use a lambda function

Problem --------- Users may want to cancel several houses and flats at once, currently this is not possible; they have to do it one by one. Objective --------- Add a way to cancel several estates in one go. Solution --------- Add a server action in the demo data so that it is available for new users. Old users can create the same server action on their database. task-123456

Problem --------- Users may want to see the total area after adding the garden area and the living area, as well as the best offer after adding multiple offers to the property. They also might want to either enter a validity number of days or a deadline, and the other should be computed automatically. Objective --------- Add a field (total area) dependant on two fields (garden and living area). Add another field which is the maximum of all offer prices. Add a relation between the validity date and the deadline fields (after creating them). Solution --------- Add a computed field for the total area depending on garden and living area fields. Add a computed field for the best price depending on the offer prices, computing the maximum of them all. Add an computed field for the date deadline and add both a compute and inverse function for it. task-xxxxxx

…de their actions. Problem --------- Users may want to modify the property status through buttons, and see their actions reflected in the data. Same applies to the offer status. Objective --------- Add a sold and cancel button for the property, modify the status based on their activity. Add accept and refuse buttons for the offers, modify the status based on their activity. Reflect the actions of offers on the property data. Solution --------- Add a sold and cancel button in the header part of the XML file, alongside their functions applied on press. Add a accept and refuse button in the list view of the offers. Based on which is clicked, modify the selling price and the buyer of the property as well as the status of the offer. task-xxxxxx

Problem --------- Users may enter incorrect data like a negative expected or offer price. Also, they may create multiple property types and tags of the same name. The selling price of a property shouldn't be less than 90% of the expected price. Objective --------- Add constraints to the field boxes (expected, selling, and offer price). The property tags and types should have constraints too. Add constraints to prevent selling prices from being lower than 90% of the expected price. Solution --------- Added an SQL constraint checking the expected, selling, and offer price to be positive. Added an SQL constraint to check the uniqueness of the property tags and types. Added a Python constraint checking the selling price to be not lower than 90% of the expected price.

…ties in the user form

…nk with invoicing

aboo-odoo

Could you update the PR title and add a lil description ? Thanks 🏅

Also, I have all your .idea stuff here, could you remove them from the diff ?

aboo-odoo · 2024-10-25T09:15:26Z

estate/models/__init__.py

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


Better indent

Suggested change

from . import estate_property, estate_property_type, estate_property_tag, estate_property_offer, users

from . import estate_property

from . import estate_property_type

from . import estate_property_tag

from . import estate_property_offer

from . import users

aboo-odoo · 2024-10-25T09:16:17Z

estate/models/estate_property.py

+    property_type_id = fields.Many2one('estate.property.types', string="Property Type")
+    salesman_id = fields.Many2one('res.users', string="Salesman", default=lambda self: self.env.user)
+    buyer_id = fields.Many2one('res.partner', string="Buyer")
+    tag_ids = fields.Many2many('estate.property.tag', string="Property Tags")
+    offer_ids = fields.One2many('estate.property.offer', 'property_id', string="Offers")
+    total_area = fields.Float(compute="_compute_total_area", string="Total Area")
+    best_price = fields.Float(compute="_compute_best_price", string="Best Price")


Those are missing the string option, useful for translation 👍

Isn't it the second parameter in all of these? The string=""?

aboo-odoo · 2024-10-25T09:17:04Z

estate/models/estate_property.py

+        for record in self:
+            if record.buyer_id and float_compare(record.selling_price, 0.9 * record.expected_price, 5) == -1:


Use of a meaningful variable name to ease the understanding of the method

Suggested change

for record in self:

if record.buyer_id and float_compare(record.selling_price, 0.9 * record.expected_price, 5) == -1:

for estate in self:

if estate.buyer_id and float_compare(record.selling_price, 0.9 * record.expected_price, 5) == -1:

aboo-odoo · 2024-10-25T09:17:33Z

estate/models/estate_property.py

+    def _compute_best_price(self):
+        for record in self:
+            prices = record.offer_ids.mapped('price')
+


No need of empty line here

aboo-odoo · 2024-10-25T09:18:00Z

estate/models/estate_property.py

+        for record in self:
+            prices = record.offer_ids.mapped('price')
+
+            record.best_price = max(prices) if len(prices) > 0 else 0


Lil python trick

Suggested change

record.best_price = max(prices) if len(prices) > 0 else 0

record.best_price = max(prices, default=0)

aboo-odoo · 2024-10-25T09:19:26Z

estate/models/estate_property_offer.py

+    partner_id = fields.Many2one('res.partner', string="Partner", required=True)
+    property_id = fields.Many2one('estate.property', string="Property", required=True)


String parameter

aboo-odoo · 2024-10-25T09:19:59Z

estate/models/estate_property_offer.py

+            record.date_deadline = record.create_date + timedelta(days=record.validity)
+
+    def _inverse_date_deadline(self):
+        for record in self:


No placing it everywhere but all you looped method need this change hehe

Suggested change

for record in self:

for offer in self:

aboo-odoo · 2024-10-25T09:20:56Z

estate/models/estate_property_type.py

+    offer_ids = One2many('estate.property.offer', 'property_type_id')
+    offer_count = fields.Integer(compute="_compute_offer_count", string="Offer Count")


aboo-odoo · 2024-10-25T09:23:53Z

estate_account/models/estate_property.py

+    def action_sold(self):
+        invoices = []
+        for record in self:
+            journal = self.env['account.journal'].search([('type', '=', 'sale')])


This could return several journal, add limit=1 to make sure you only get one 👍

aboo-odoo · 2024-10-25T09:24:40Z

estate_account/models/estate_property.py

+            invoices.append(invoice)
+
+        self.env['account.move'].create(invoices)


Very nice to think of perf like that, honestly very impressed

It could be even nicer by placing journal = self.env[].search outside of the loop, just before it so that you only search once

malm-odoo force-pushed the 18.0-fix-invoices-malm branch 2 times, most recently from c929665 to d55d464 Compare October 22, 2024 07:40

aboo-odoo force-pushed the 18.0-fix-invoices-malm branch from 6d1449f to 489b815 Compare October 22, 2024 09:22

aboo-odoo reviewed Oct 22, 2024

View reviewed changes

malm-odoo force-pushed the 18.0-fix-invoices-malm branch 2 times, most recently from 042bf07 to 37b90b6 Compare October 23, 2024 12:58

fdardenne force-pushed the 18.0 branch from b509c0d to b16e643 Compare October 24, 2024 12:03

malm-odoo force-pushed the 18.0-fix-invoices-malm branch 2 times, most recently from 6567d13 to e8020f0 Compare October 24, 2024 13:04

malm-odoo and others added 19 commits October 24, 2024 15:21

[ADD] chap1: Manifest

cd26d0b

[ADD] chap2: __init__.py

98dc91c

[ADD] chap2: Content to manifest

2430f51

[ADD] chap2: Manifest depends

8478e5f

[FIX] chap2: Manifest syntax

be2d317

[FIX] chap5: Changed dirs

760525b

[ADD] chap5: Added xml files

640bb1d

[ADD] chap5: License in manifest

9324b1e

[FIX] chap5: style

9ff6ca6

[ADD] chap6: view form, filters, search, and group by

dddd880

[ADD] estate: Add property types, a salesman, and a buyer

5539fb8

[ADD] estate: Added newline to fix style warnings

940160e

[ADD] estate: Added property tags and offers page

c49def8

[ADD] estate: Added newline to fix style

a896aae

[ADD] estate: Newline to fix style

63198aa

[ADD] estate: Added the sprinkles

b0c767f

malm-odoo force-pushed the 18.0-fix-invoices-malm branch from a8ffcf6 to b0c767f Compare October 24, 2024 13:23

malm-odoo added 3 commits October 24, 2024 17:38

[ADD] estate: Added oncreate method in offers module and users proper…

ae52a8a

…ties in the user form

[ADD] estate_account: Added a method inheriting the action_sold to li…

7d94274

…nk with invoicing

[ADD] estate: Added kanban view

7de8a5f

aboo-odoo reviewed Oct 25, 2024

View reviewed changes

[FIX] estate: Fixed views order in manifest

a23543e

aboo-odoo reviewed Oct 25, 2024

View reviewed changes

malm-odoo changed the title ~~test~~ [ADD] estate: a new module to manage estate properties Oct 25, 2024

malm-odoo added 2 commits October 25, 2024 13:09

[ADD] estate: Added .idea to gitignore

9fc9a00

[FIX] estate & estate_account: Fixed all the comments on PR

69f7e7f

malm-odoo force-pushed the 18.0-fix-invoices-malm branch from 70679b7 to 69f7e7f Compare October 25, 2024 11:10

[ADD] awesome_owl: Added counter, card, and todo list.

fb39311

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADD] estate: a new module to manage estate properties #163

[ADD] estate: a new module to manage estate properties #163

malm-odoo commented Oct 21, 2024 •

edited

Loading

aboo-odoo commented Oct 21, 2024

aboo-odoo left a comment

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo Oct 22, 2024

aboo-odoo left a comment

aboo-odoo Oct 25, 2024

aboo-odoo Oct 25, 2024

malm-odoo Oct 25, 2024

aboo-odoo Oct 25, 2024

aboo-odoo Oct 25, 2024

aboo-odoo Oct 25, 2024

aboo-odoo Oct 25, 2024

aboo-odoo Oct 25, 2024

aboo-odoo Oct 25, 2024

aboo-odoo Oct 25, 2024

aboo-odoo Oct 25, 2024 •

edited

Loading

		from odoo import models, fields
		from dateutil.relativedelta import relativedelta



		class EstateProperty(models.Model):
		_name = "estate_property"

-        selection=[('new', 'New'), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'), ('sold', 'Sold'), ('cancelled', "Cancelled")]
+        selection=[
+            ('new', "New"),
+            ('offer_received', "Offer Received"),
+            ('offer_accepted', "Offer Accepted"),
+            ('sold', "Sold"),
+            ('cancelled', "Cancelled"),
+        ]

	<record id="estate_property_list_view_search" model="ir.ui.view">
	<record id="estate_property_view_search" model="ir.ui.view">

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

-from . import estate_property, estate_property_type, estate_property_tag, estate_property_offer, users
+from . import estate_property
+from . import estate_property_type
+from . import estate_property_tag
+from . import estate_property_offer
+from . import users

		for record in self:
		if record.buyer_id and float_compare(record.selling_price, 0.9 * record.expected_price, 5) == -1:

	record.best_price = max(prices) if len(prices) > 0 else 0
	record.best_price = max(prices, default=0)

		partner_id = fields.Many2one('res.partner', string="Partner", required=True)
		property_id = fields.Many2one('estate.property', string="Property", required=True)

		offer_ids = One2many('estate.property.offer', 'property_type_id')
		offer_count = fields.Integer(compute="_compute_offer_count", string="Offer Count")

		invoices.append(invoice)

		self.env['account.move'].create(invoices)

[ADD] estate: a new module to manage estate properties #163

Are you sure you want to change the base?

[ADD] estate: a new module to manage estate properties #163

Conversation

malm-odoo commented Oct 21, 2024 • edited Loading

aboo-odoo commented Oct 21, 2024

aboo-odoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aboo-odoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aboo-odoo Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

malm-odoo commented Oct 21, 2024 •

edited

Loading

aboo-odoo Oct 25, 2024 •

edited

Loading