[ADD] estate: added a new estate module #734

abeg-odoo · 2025-04-23T04:26:41Z

We have added a module as an application with a model a some basic properties like (name, description, expected price ...etc)
We have added security rules for read, write, create and delete actions on this model
We have added menu items and UI interaction to access a form view of the records we create and we also added attributes to some properties of the model like (copy, default, readonly, required)

We have added a module as an application with a model a some basic properties like (name, description, expected price ...etc) We have added security rules for read, write, create and delete actions on this model We have added menu items and UI interaction to access a form view of the records we create and we also added attributes to some properties of the model like (copy, default, readonly, required)

robodoo · 2025-04-23T04:26:44Z

Changed the individual definition of menuitems to nested menu items for better readability

Added linting with flake8 and fixed some linting warnings

SergeBayet

Some linting remarks. 👍

SergeBayet · 2025-04-23T10:53:34Z

estate/security/ir.model.access.csv

@@ -0,0 +1,2 @@
+id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
+estate.access_estate_property,access_estate_property,estate.model_estate_property,base.group_user,1,1,1,1


Missing blank line at the end of the file.

SergeBayet · 2025-04-23T10:53:58Z

estate/views/estate_property_views.xml

+                </p>
+            </field>
+        </record>
+


Unnecessary blank line here

SergeBayet · 2025-04-23T10:54:09Z

estate/views/estate_property_views.xml

+            </field>
+        </record>
+    </data>
+</odoo>


Blank line missing

fixed review warnings

SergeBayet

Seems good :) Just a little remark

SergeBayet · 2025-04-23T14:33:20Z

estate/views/estate_property_views.xml

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


Just a nitpick :)

Suggested change

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

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

SergeBayet · 2025-04-23T14:49:38Z

estate/views/estate_property_views.xml

@@ -79,7 +79,7 @@
                    <field name="bedrooms"/>
                    <field name="living_area"/>
                    <field name="facades"/>
-                    <filter string="Available" name="state" domain="['|',('state', '=', 'new'),('state', '=', 'offer_received')]"/>
+                    <filter string="Available" name="state" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>
                    <group expand="1" string="Group By">


Try to squash this commit with the previous one. No need to have a separated commit for this.
doc: https://www.git-tower.com/learn/git/faq/git-squash

In this commit we added form view with a well formated view of the created properties We also added a search view for searching by name, by selling price, etc .., added a filter by availability and a group by postcode

SergeBayet · 2025-04-24T09:22:41Z

estate/security/ir.model.access.csv

@@ -1,2 +1,5 @@
 id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
 estate.access_estate_property,access_estate_property,estate.model_estate_property,base.group_user,1,1,1,1
+estate.access_estate_property_type,access_estate_property_type,estate.model_estate_property_type,base.group_user,1,1,1,1
+estate.access_estate_property_tag,access_estate_property_tag,estate.model_estate_property_tag,base.group_user,1,1,1,1
+estate.access_estate_property_offer,access_estate_property_offer,estate.model_estate_property_offer,base.group_user,1,1,1,1


Missing a blank line EOF.

- Added Estate property type model and linked it with properties as a many2one field - Added buyer and salesperson fields to a property as many2one fields - Added tags to properties as many2many fields - Added offers model and linked it with properties as one2many field

SergeBayet · 2025-04-24T10:26:57Z

estate/models/estate_property_offer.py

@@ -0,0 +1,17 @@
+from odoo import models, fields
+


Missing a blank line. We should have 2 blank lines between imports and class definition. Check the other files. Thanks 👍

SergeBayet · 2025-04-24T10:27:31Z

estate/__manifest__.py

+{
+    'name': 'Estate',
+    'depends': ['base'],
+    'application': True,


Don't forget to add the license to avoid warnings in the log.

- Added total area as computed field, as the sum of living area and garden area - Added best price as computed field calculated from the linked offers - Added 2 fields in the offer model: date_deadline and validity and linked them with inverse updates - Added an onchange callback to the garden field to set/reset the values of garden_area and garden_orientation

SergeBayet · 2025-04-24T11:43:04Z

@naja628 You can take over :)

naja628

Hello -- good work :)
I left some nitpicks

naja628 · 2025-04-24T11:46:19Z

estate/__init__.py

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


nitpick: not really needed but ok

naja628 · 2025-04-24T12:01:53Z

estate/models/__init__.py

+from . import estate_property
+from . import estate_property_type, estate_property_tag, estate_property_offer
+
+assert estate_property
+assert estate_property_type
+assert estate_property_tag
+assert estate_property_offer


Suggested change

from . import estate_property

from . import estate_property_type, estate_property_tag, estate_property_offer

assert estate_property

assert estate_property_type

assert estate_property_tag

assert estate_property_offer

from . import estate_property

from . import estate_property_type

...

nitpick: this syntax is fine but in odoo code, __init__ files usually look like this

naja628 · 2025-04-24T12:32:45Z

estate/models/estate_property.py

+
+    name = fields.Char(required=True)
+    description = fields.Text()
+    property_tags_ids = fields.Many2many("estate.property.tag")


nitpick:

Suggested change

property_tags_ids = fields.Many2many("estate.property.tag")

property_tag_ids = fields.Many2many("estate.property.tag", string="Property Tags")

for x2m the convention is usually to keep the base name in the singular (since _ids is already plural)

naja628 · 2025-04-24T12:48:43Z

estate/views/estate_property_views.xml

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


just for info this should be equivalent to

Suggested change

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

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

- Added 2 buttons: cancel and set a property as sold - Added 2 buttons for the offers, accept and refuse - Once an offer is accepted, the selling price and the Buyer are Set in the property info

- Added SQL constraints for : - property expected price must be strictly positive - property selling price must be positive - offer price must be strictly positive - property tag and type must be unique - Added python constraint for selling price must not be lower than 90% of expected price

- Added inline views to show properties of a type in a list (only shows title, expected price, status) - Added widget statusbar to have a better view of the status of the property - Added default list ordering and manual ordering with handle - Added conditional attributes like invisible and readonly to create dynamic forms that adapt to data context - Added aditable inline editing to the lists - Added default filters for search views with search_defaul_Availability and custom search logic for the surface with filter_domain - Added a stat button to quickly access all offers of a given property type

Fixed views imports in the data field in the manifest file

- Overrided CRUD methods for property deletion and offer creation logic (don't allow deletion of ongoing properties, and don't create offers lower than the existing ones) - Add properties field to res.user model and add a view to see properties from the user list view

- Added a new link module estate_account responsible for creating an invoice when the sold button is created - Overriden the action_set_sold method to add the creation of the invoice in it

Added kanban view grouped by property type for properties

naja628

Hello :) -- good work !

I left some comments

naja628 · 2025-04-29T13:15:07Z

estate/models/__init__.py

+
+assert estate_property
+assert estate_property_type
+assert estate_property_tag
+assert estate_property_offer
+assert res_users


naja628 · 2025-04-29T13:16:28Z

estate/models/estate_property.py

+from odoo import api, models, fields
+from odoo.exceptions import UserError, ValidationError
+from dateutil.relativedelta import relativedelta
+
+from odoo.tools.float_utils import float_compare, float_is_zero


Suggested change

from odoo import api, models, fields

from odoo.exceptions import UserError, ValidationError

from dateutil.relativedelta import relativedelta

from odoo.tools.float_utils import float_compare, float_is_zero

from dateutil.relativedelta import relativedelta

from odoo import api, models, fields

from odoo.exceptions import UserError, ValidationError

from odoo.tools.float_utils import float_compare, float_is_zero

nitpick: we like to do 2 groups of import for external and internal imports

naja628 · 2025-04-29T13:21:19Z

estate/models/estate_property.py

+    postcode = fields.Char()
+    date_availability = fields.Date(
+        copy=False,
+        default=_default_date_availability()


Suggested change

default=_default_date_availability()

default=_default_date_availability

Here calling the function means it will called only once instead of at every new record creation

naja628 · 2025-04-29T13:22:57Z

estate/models/estate_property.py

+    def action_set_sold(self):
+        for record in self:
+            if record.state == 'cancelled':
+                raise UserError("Canceled properties cannot be set as sold.")


try to wrap user facing string in a call to the _ function so the translation system sees them.

naja628 · 2025-04-29T13:23:37Z

estate/models/estate_property.py

+        return types.search([])  # this is equivalent to return self.env['estate.property.type'].search([])
+
+    def action_set_sold(self):
+        for record in self:


nitpick: prefer descriptive names for the loop variable when looping over recordsets

naja628 · 2025-04-29T14:00:27Z

estate/models/estate_property_offer.py

+            property_id = val.get('property_id')
+            price = val.get('price')


by default, get will return None if no 2nd argument is specified but you assume they exist later in the code.

Suggested change

property_id = val.get('property_id')

price = val.get('price')

property_id = val.get('property_id')

price = val.get('price')

ici, dans ce cas, qu'est ce que je devrais faire pour palier à ce problème
est ce que je dois mettre une valeur par défaut ou afficher une erreur quand le get return None

I think here, for price a fallback makes sense.
For property_id it's require=True (and has no default) so you will already get an error if it's not in the keys (in the super().create probably) -- but I think I'd still skip some of the logic when it's not defined (i mean with a condition for example)

naja628 · 2025-04-29T14:07:30Z

estate/views/estate_property_views.xml

+                    <field name="facades"/>
+                    <filter name="available" string="Available" domain="[('state', 'in', ['new', 'offer_received'])]"/>
+                    <group expand="1" string="Group By">
+                        <filter string="Postcode" name="postcode" context="{'group_by':'postcode'}"/>


nitpick:

Suggested change

<filter string="Postcode" name="postcode" context="{'group_by':'postcode'}"/>

<filter string="Postcode" name="postcode" context="{'group_by': 'postcode'}"/>

naja628 · 2025-04-29T14:07:52Z

estate/views/res_users_views.xml

+            </field>
+        </record>
+    </data>
+</odoo>


missing eol character (editor config)

naja628 · 2025-04-29T14:08:54Z

estate_account/__manifest__.py

+    'data': [
+    ],
+    'installable': True,
+    'application': False,


Suggested change

'application': False,

'application': False,

'auto_install': True,

brigde modules typically contain functionally that we want to automatically "appear" when the dependencies are all installed

naja628 · 2025-04-29T14:09:16Z

estate_account/__manifest__.py

Missing keys: 'author', etc

- Removed un necessary assert (for the unused imports in the __init__ files) - Renamed looping variables' names for better readability (property instead of record, and offer instead of record) - Added auto_install property to True in the estate_account link module (used to auto install dependencies when we install estate_account)

- Added Card and Counter components - Added markup html - Added props validation in Card

- Added todo list (with adding and deleting functionnality) - Added dynamic attributes for setting a todo item to Done - Added Focusing the input - Modified the Card component to accept a default slot as content instead of a simple text - Added toggle hide or show the content of the card

- Added dashboard layout - Added a statistics service for getting statistics using rpc call - Cached the statistics using memoize

naja628

Hello :) -- Good work again !
I left some comments

naja628 · 2025-05-05T13:52:00Z

awesome_owl/static/src/Card/card.js

+/** @odoo-module **/
+


These are no longer needed.
you can also remove them in the files they give you ^^

naja628 · 2025-05-05T13:53:39Z

awesome_owl/static/src/Card/card.js

+        title: String,
+        slots: {
+            type: Object,
+        }


nitpick:

Suggested change

}

},

naja628 · 2025-05-05T13:54:39Z

awesome_owl/static/src/Card/card.js

+    static props = {
+        title: String,
+        slots: {
+            type: Object,


here if you don't make it optional something like <Card .../> will give an error and you will have to do <Card ...></Card>

naja628 · 2025-05-05T13:55:37Z

awesome_owl/static/src/Card/card.js

+        this.state.isOpen = !this.state.isOpen;
+    }
+
+    static template = "awesome_owl.card";


niptpicks:

move to the top of the class

prefer PascalCase for template names

Suggested change

static template = "awesome_owl.card";

static template = "awesome_owl.Card";

naja628 · 2025-05-05T13:58:27Z

awesome_owl/static/src/Counter/counter.js

+/** @odoo-module **/
+


Suggested change

/** @odoo-module **/

naja628 · 2025-05-05T14:10:25Z

awesome_owl/static/src/playground.js

+    static components = { TodoList, Card, Counter };
+
+    html = markup("<div>some content</div>")
+    counters_sum = useState({ value: 0 });


nitpick: js-side use camelCase for variables

Suggested change

counters_sum = useState({ value: 0 });

countersSum = useState({ value: 0 });

naja628 · 2025-05-05T14:11:26Z

awesome_owl/static/src/playground.js

+    incrementSum() {
+        this.counters_sum.value++;
+    }


You're defining it correctly but you're not actually using it in your playground

naja628 · 2025-05-05T14:28:00Z

awesome_dashboard/static/src/dashboard.scss

+
+.o_dashboard {
+    background-color: $bg-gray;
+}


nitpick: missing eol character

naja628 · 2025-05-05T14:31:10Z

awesome_dashboard/static/src/dashboard.js


 class AwesomeDashboard extends Component {
    static template = "awesome_dashboard.AwesomeDashboard";
+    static components = {
+        Layout,
+        DashboardItem


nitpick:

Suggested change

DashboardItem

DashboardItem,

naja628 · 2025-05-05T14:33:54Z

awesome_owl/static/src/ToDo/todo_list.xml

+    <t t-name="awesome_owl.todo_list">
+        <div class="card d-inline-block m-2" style="width: 300px;">
+            <input t-ref="input" t-on-keyup="onInputEnter"/>
+            <t t-foreach="this.todos" t-as="i" t-key="i.id">


Suggested change

<t t-foreach="this.todos" t-as="i" t-key="i.id">

<t t-foreach="this.todos" t-as="todo" t-key="todo.id">

abeg-odoo added 2 commits April 23, 2025 06:31

[REF] estate: refactored some code

ede556e

Changed the individual definition of menuitems to nested menu items for better readability

[REF] estate: added flake8 linting

84e5068

Added linting with flake8 and fixed some linting warnings

abeg-odoo marked this pull request as ready for review April 23, 2025 10:44

SergeBayet suggested changes Apr 23, 2025

View reviewed changes

[REF] estate: fixed review warnings

ef955ea

fixed review warnings

SergeBayet reviewed Apr 23, 2025

View reviewed changes

SergeBayet suggested changes Apr 23, 2025

View reviewed changes

[IMP] estate: added form view and search view

7f831ed

In this commit we added form view with a well formated view of the created properties We also added a search view for searching by name, by selling price, etc .., added a filter by availability and a group by postcode

abeg-odoo force-pushed the 18.0-add-estate-module-abeg branch from 0cc4cde to 7f831ed Compare April 24, 2025 06:35

SergeBayet suggested changes Apr 24, 2025

View reviewed changes

abeg-odoo force-pushed the 18.0-add-estate-module-abeg branch from 3d88445 to 4c14c9b Compare April 24, 2025 09:56

SergeBayet suggested changes Apr 24, 2025

View reviewed changes

naja628 reviewed Apr 24, 2025

View reviewed changes

abeg-odoo added 3 commits April 24, 2025 16:33

[IMP] estate: chapter9 - added buttons

be5ca2f

- Added 2 buttons: cancel and set a property as sold - Added 2 buttons for the offers, accept and refuse - Once an offer is accepted, the selling price and the Buyer are Set in the property info

abeg-odoo force-pushed the 18.0-add-estate-module-abeg branch from 0fc52fe to 8826e2b Compare April 25, 2025 12:06

abeg-odoo added 4 commits April 25, 2025 14:42

[FIX] estate: fixed data import order

df29c68

Fixed views imports in the data field in the manifest file

[IMP] estate: chapter13 - invoice on sold property

4878ba5

- Added a new link module estate_account responsible for creating an invoice when the sold button is created - Overriden the action_set_sold method to add the creation of the invoice in it

[IMP] estate: chapter14 - Add kanban view

d270336

Added kanban view grouped by property type for properties

abeg-odoo force-pushed the 18.0-add-estate-module-abeg branch from 68304dd to d270336 Compare April 28, 2025 15:23

naja628 reviewed Apr 29, 2025

View reviewed changes

abeg-odoo force-pushed the 18.0-add-estate-module-abeg branch from c93ae8a to 47ebfcf Compare April 29, 2025 15:53

abeg-odoo force-pushed the 18.0-add-estate-module-abeg branch from 47ebfcf to 77d0bd8 Compare April 30, 2025 08:27

abeg-odoo added 3 commits April 30, 2025 10:58

[IMP] Web Framework Tuto - chapter 1

9c751ec

- Added Card and Counter components - Added markup html - Added props validation in Card

[IMP] Web Framework Tuto - chapter 2

2fdc85d

- Added dashboard layout - Added a statistics service for getting statistics using rpc call - Cached the statistics using memoize

naja628 reviewed May 5, 2025

View reviewed changes

		@@ -0,0 +1,2 @@
		id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
		estate.access_estate_property,access_estate_property,estate.model_estate_property,base.group_user,1,1,1,1

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

	property_tags_ids = fields.Many2many("estate.property.tag")
	property_tag_ids = fields.Many2many("estate.property.tag", string="Property Tags")

	default=_default_date_availability()
	default=_default_date_availability

		property_id = val.get('property_id')
		price = val.get('price')

	<filter string="Postcode" name="postcode" context="{'group_by':'postcode'}"/>
	<filter string="Postcode" name="postcode" context="{'group_by': 'postcode'}"/>

	'application': False,
	'application': False,
	'auto_install': True,

	static template = "awesome_owl.card";
	static template = "awesome_owl.Card";

	counters_sum = useState({ value: 0 });
	countersSum = useState({ value: 0 });

	<t t-foreach="this.todos" t-as="i" t-key="i.id">
	<t t-foreach="this.todos" t-as="todo" t-key="todo.id">

[ADD] estate: added a new estate module #734

Are you sure you want to change the base?

[ADD] estate: added a new estate module #734

Conversation

abeg-odoo commented Apr 23, 2025

robodoo commented Apr 23, 2025

SergeBayet 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

SergeBayet 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

SergeBayet commented Apr 24, 2025

naja628 left a comment • edited Loading

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

naja628 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

naja628 Apr 30, 2025 • edited Loading

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

naja628 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

naja628 left a comment •

edited

Loading

naja628 Apr 30, 2025 •

edited

Loading