[ADD] estate: created a new module, model defination, added security … #323

padi-odoo · 2025-02-05T13:47:49Z

…file, created views, actions

& menuitems, created custom list view.

…file, created views, actions & menuitems, created custom list view.

robodoo · 2025-02-05T13:47:53Z

mobe-odoo

Hello @padi-odoo ,
I've added some suggestions. Please have a look at them.
Some general instructions:

Please make sure to add a new line at the end of every file.
Please ensure the entire module follows coding guidelines.

mobe-odoo · 2025-02-06T06:01:35Z

estate/__manifest__.py

Why there is no version in manifest file?
Check the proper sequencing for fields in manifest file.

Included version and proper sequencing is done

mobe-odoo · 2025-02-06T06:05:30Z

estate/models/estate_property.py

@@ -0,0 +1,32 @@
+from datetime import datetime, timedelta, date
+from dateutil.relativedelta import relativedelta


Why you have imported this if not used?
Please make sure to import only the required libraries.

Removed extra imports from the file

mobe-odoo · 2025-02-06T06:06:19Z

estate/views/custom_listview.xml

Why there is need of new file for adding views?
Can't we add the views in its appropriate file (estate_property_views.xml)?

Separated files for menuitems and views, actions as suggested

mobe-odoo · 2025-02-06T06:09:24Z

estate/models/estate_property.py

+    _name = "estate.property"
+    _description = "Estate Property Model"
+
+    name = fields.Char(string="Your name", required=True, default="Unknown")


The string for the fields should be appropriate. Also what's the need of adding "default" parameter here?

Modified the string and suggested changes are done

mobe-odoo · 2025-02-06T06:12:53Z

estate/models/estate_property.py

+    _description = "Estate Property Model"
+
+    name = fields.Char(string="Your name", required=True, default="Unknown")
+    active = fields.Boolean(default=False)


What is the purpose of active field? and why you have passed 'False' in default?

Active field is used for filtering purpose between currently active and inactive records, default = False sets by default record as inactive

mobe-odoo · 2025-02-06T06:13:13Z

estate/models/estate_property.py

+
+    name = fields.Char(string="Your name", required=True, default="Unknown")
+    active = fields.Boolean(default=False)
+    state = fields.Selection(selection = [('New', 'New'), ('Offer Received', 'Offer Received'), ('Offer Accepted', 'Offer Accepted'), ('Sold', 'Sold'), ('Cancelled', 'Cancelled')], default="New")


Why there is no string for this field?

Added string with appropriate value

mobe-odoo · 2025-02-06T06:14:10Z

estate/models/estate_property.py

+    postcode = fields.Char()
+    date_availability = fields.Date(copy=False, default=fields.Date.today() + timedelta(days=+90))
+    expected_price = fields.Float(required=True)
+    selling_price = fields.Float(readonly=True, copy=False) 


Could you explain the purpose of "copy", "required", "readonly" parameters?

Copy decides the whether field's value is duplicated or not when the record is duplicated.
Required decides whether a field must have a value before saving the record.
Readonly prevents the alteration by giving access to read the data only

mobe-odoo · 2025-02-06T06:14:47Z

estate/views/estate_property_views.xml

+            <menuitem id="test_model_menu_action" action="estate_property_action" />
+        </menuitem>
+    </menuitem>
+


Suggested change

Remove this extra line.

Suggested Changes are done.

mobe-odoo · 2025-02-06T06:15:24Z

estate/models/estate_property.py

+from odoo import models, fields
+
+class EstateProperty(models.Model):
+


Suggested change

Remove this extra line

Suggested changes are done

…ed custome form view & search view, seperated files for menuitems and views, actions.

…many, One2many.

mobe-odoo

Hello @padi-odoo ,

Great work for now!
I've added some comments and suggestions, please go through it and if you have any query feel free to reach me out.

Thanks

mobe-odoo · 2025-02-10T07:19:20Z

estate/__manifest__.py

Make a habit to add description so others will know what is the purpose of this module.

Added description in manifest.py file

mobe-odoo · 2025-02-10T07:20:35Z

estate/models/__init__.py

Import should be in alphabetical order.

Rearranged imports in alphabetical order

mobe-odoo · 2025-02-10T07:22:17Z

estate/models/estate_property.py

@@ -0,0 +1,44 @@
+from datetime import timedelta
+
+from odoo import models, fields


The import should follow alphabetical order.
Refer doc: https://www.odoo.com/documentation/18.0/contributing/development/coding_guidelines.html#imports

mobe-odoo · 2025-02-10T07:23:38Z

estate/models/estate_property.py

+        ('west', 'West')
+        ], string="Garden Orientation", help="Orientation of the garden"
+    )   
+    property_type = fields.Many2one(comodel_name="estate.property.type", string="Property Type")


No need of writing 'comodel_name', check other occurrences as well.

Suggested change

property_type = fields.Many2one(comodel_name="estate.property.type", string="Property Type")

property_type = fields.Many2one("estate.property.type", string="Property Type")

Suggested changes are implemented

mobe-odoo · 2025-02-10T07:25:04Z

estate/models/estate_property.py

+    offer_ids = fields.One2many(comodel_name="estate.property.offer", inverse_name="property_id", string="Offers"
+    )


Suggested change

offer_ids = fields.One2many(comodel_name="estate.property.offer", inverse_name="property_id", string="Offers"

)

offer_ids = fields.One2many("estate.property.offer", "property_id", string="Offers")

Suggested changes are done

mobe-odoo · 2025-02-10T07:26:15Z

estate/models/estate_property_offer.py

+    status = fields.Selection([
+        ('accepted', 'Accepted'), 
+        ('refused', 'Refused')
+        ], string="Status" ,copy=False
+    )


Suggested change

status = fields.Selection([

('accepted', 'Accepted'),

('refused', 'Refused')

], string="Status" ,copy=False

)

status = fields.Selection([

('accepted', 'Accepted'),

('refused', 'Refused')

], string="Status", copy=False)

Refer codebase to see proper syntax for fields. Also check other occurences as well.

Corrected the syntax according to codebase

mobe-odoo · 2025-02-10T07:27:01Z

estate/views/estate_property_type_views.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="utf-8"?>
+<odoo>
+


Suggested change

Remove extra lines

Suggested changes are done

mobe-odoo · 2025-02-10T07:28:42Z

estate/views/estate_property_views.xml

+        <field name="model">estate.property</field>
+        <field name="arch" type="xml">
+            <list string="Properties">
+                <field name="name" width="20%" />


Suggested change

<field name="name" width="20%" />

<field name="name" width="20%"/>

check other occurrences as well.

Suggested changes are done

… methods and constraints using SQL and Python

mobe-odoo

Hello @padi-odoo ,

Please see the review comments.

Thanks!

mobe-odoo · 2025-02-11T09:28:36Z

estate/models/estate_property_offer.py

+    @api.depends('create_date', 'validity')
+    def _compute_date_deadline(self):
+        for record in self:
+            create_date = record.create_date.date() if record.create_date else fields.Date.today()


Do we really need this variable for create date??

Optimized code by removing unnecessary variable creation(create_date) as suggested.

mobe-odoo · 2025-02-11T09:29:22Z

estate/models/estate_property_offer.py

+            existing_accepted_offer = self.search([
+                ('property_id', '=', record.property_id.id),
+                ('status', '=', 'accepted')


We just need to check if there is an existing offer 'accepted' so can't we just add limit to it? Refer codebase for it.

Added limit attribute to self.search method to stop searching after we get 1 record instead of running complete for loop.

mobe-odoo · 2025-02-11T09:31:49Z

estate/models/estate_property_offer.py

+
+            record.status = 'accepted'
+            record.property_id.buyer_id = record.partner_id.id
+            record.property_id.selling_price = record.price


What impact should be there in property on accepting any offer?

The buyer_id field of the property should be updated with the buyer (partner_id) from the accepted offer.

The selling_price should be set to the price of the accepted offer if constraints are satisfied.

Offer's status should be set to 'accepted'.

mobe-odoo · 2025-02-11T09:32:34Z

estate/models/estate_property_offer.py

+            record.property_id.buyer_id = record.partner_id.id
+            record.property_id.selling_price = record.price
+
+    def action_refuse(self):


What if the property is already sold or cancelled, will there be any use for this button then??

If the property is already sold or cancelled, it would be logical to invisible the "Accept" and "Refuse" buttons, since the property status is final and no further offers can be accepted or refused.

mobe-odoo · 2025-02-11T09:34:57Z

estate/models/estate_property.py

+            else:
+                record.state = 'cancelled'
+
+    @api.constrains('selling_price')


Could you please verify the place of constrains method? Please follow coding guidelines

What is the difference between SQL and python constrains??

SQL constraints are applied directly at the database level and ensuring data correctness before the data is stored in the database.

Python constraints are defined within the Odoo models and applied by the logic we write in python functions, not directly by the database.

Corrected the place of constraint method.

…widget for state representaion,model & manual ordering and conditional buttons visibility

[ADD] estate: created a new module, model defination, added security …

f766db8

…file, created views, actions & menuitems, created custom list view.

mobe-odoo reviewed Feb 6, 2025

View reviewed changes

padi-odoo added 2 commits February 6, 2025 19:09

[IMP] estate: Performed changes suggested on previous PR draft, creat…

f7535c2

…ed custome form view & search view, seperated files for menuitems and views, actions.

[IMP] estate: Performed relations between models like Many2one, Many2…

4f0b885

…many, One2many.

padi-odoo force-pushed the 18.0-training-padi branch from 2f8890a to 4f0b885 Compare February 7, 2025 13:46

mobe-odoo reviewed Feb 10, 2025

View reviewed changes

[IMP] estate: Implemented computed fields, onchanges, actions, button…

504a7f9

… methods and constraints using SQL and Python

mobe-odoo reviewed Feb 11, 2025

View reviewed changes

[IMP] esate: Implemented inline list view for properties, status bar …

29fd54c

…widget for state representaion,model & manual ordering and conditional buttons visibility

		@@ -0,0 +1,32 @@
		from datetime import datetime, timedelta, date
		from dateutil.relativedelta import relativedelta

		from odoo import models, fields

		class EstateProperty(models.Model):

		@@ -0,0 +1,44 @@
		from datetime import timedelta

		from odoo import models, fields

	property_type = fields.Many2one(comodel_name="estate.property.type", string="Property Type")
	property_type = fields.Many2one("estate.property.type", string="Property Type")

		offer_ids = fields.One2many(comodel_name="estate.property.offer", inverse_name="property_id", string="Offers"
		)

	offer_ids = fields.One2many(comodel_name="estate.property.offer", inverse_name="property_id", string="Offers"
	)
	offer_ids = fields.One2many("estate.property.offer", "property_id", string="Offers")

		@@ -0,0 +1,10 @@
		<?xml version="1.0" encoding="utf-8"?>
		<odoo>

	<field name="name" width="20%" />
	<field name="name" width="20%"/>

[ADD] estate: created a new module, model defination, added security … #323

Are you sure you want to change the base?

[ADD] estate: created a new module, model defination, added security … #323

Conversation

padi-odoo commented Feb 5, 2025

robodoo commented Feb 5, 2025

mobe-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

padi-odoo Feb 6, 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

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

mobe-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

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

mobe-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

padi-odoo Feb 6, 2025 •

edited

Loading