[ADD] estate : adding new module in tutorials by AbdelrahmanFawzy1 · Pull Request #216 · odoo/tutorials

AbdelrahmanFawzy1 · 2024-12-17T08:10:41Z

No description provided.

robodoo · 2024-12-17T08:10:44Z

kfr-odoo

You are on the right track

Configure your IDE to get ride of EOF issues. "Insert Final Newline" in VScode.
Remove all # -*- coding: utf-8 -*-, they are not required anymore (since version 16.0).
Keep your python imports in alphabetical order to prevent duplication (ref: estate_property.py)

kfr-odoo · 2024-12-17T14:18:39Z

estate/__manifest__.py

+    'summary': "Realistic business advertisements",
+    'application': True,
+    'installable': True,
+    'auto_install': False,


This is already the default (ref: doc)

kfr-odoo · 2024-12-19T13:05:46Z

estate/models/estate_property.py

+
+
+
+


two blank lines before class declaration

Suggested change

kfr-odoo · 2024-12-19T13:06:53Z

estate/models/estate_property.py

+    @api.onchange('garden')
+    def _onchange_garden(self):
+        self.garden_area = 10 if self.garden else False
+        self.garden_orientation = 'n' if self.garden else False


Does not seem to match your selection field declaration

kfr-odoo · 2024-12-19T13:07:05Z

estate/models/estate_property.py

+
+    @api.onchange('garden')
+    def _onchange_garden(self):
+        self.garden_area = 10 if self.garden else False


Suggested change

self.garden_area = 10 if self.garden else False

self.garden_area = 10 if self.garden else 0

estate/models/estate_property.py

kfr-odoo · 2024-12-19T13:11:04Z

estate/models/estate_property_offer.py

+
+    def action_accept(self):
+        for record in self:
+            try:


Why is the try catch necessary ? Either you fix the issue or add a comment to explain why it might behave unexpectedly

kfr-odoo · 2024-12-19T13:11:39Z

estate/models/estate_property_offer.py

+                record.property_id.action_process_accept(self)
+                record.status = 'accepted'
+            except UserError as e:
+                raise e


Only raise builtin odoo errors for optimal UX

Suggested change

raise e

raise UserError("error message")

estate/models/estate_property_offer.py

kfr-odoo · 2024-12-19T13:14:28Z

@AbdelrahmanFawzy1 The CI tool is still red, please try to fix the issues

Co-authored-by: Killian Frappart <kfr@odoo.com>

kfr-odoo · 2024-12-20T14:14:59Z

estate/models/estate_property.py

+            raise UserError("Cancelled Items cannot be sold.")
+        self.state = "sold"
+
+    def action_set_sold(self):


no loop in action coming from a form view

kfr-odoo · 2024-12-20T14:16:20Z

estate/models/estate_property_offer.py

+    def _compute_date_deadline(self):
+        for record in self:
+            if not record.create_date:
+                record.create_date = fields.Date.today()


You should probably refrain from writing on super fields such as id, create_date, etc...

kfr-odoo · 2024-12-23T14:51:37Z

estate/models/estate_property.py

+            raise UserError("this property has already an accepted offer!!")
+        self.state = "offer_accepted"
+        self.selling_price = offer.price
+        self.partner_id = offer.partner_id


missing return

Why do I need it? I am just setting self attributes?

kfr-odoo · 2024-12-23T14:52:47Z

estate/models/estate_property.py

+
+
+    @api.ondelete(at_uninstall=False)
+    def prevent_delete(self):


naming convention

Suggested change

def prevent_delete(self):

def _unlink_property(self):

kfr-odoo · 2024-12-23T14:53:12Z

estate/models/estate_property_offer.py

+        ("check_price", "CHECK(price >= 0)", "Offer prices MUST be postive."),
+    ]
+
+    def action_accept(self):


no loop in form action

kfr-odoo · 2024-12-23T14:58:44Z

estate_account/models/estate_property.py

@@ -0,0 +1,30 @@
+from odoo import models, Command
+
+class InheritedEstateProperty(models.Model):


Suggested change

class InheritedEstateProperty(models.Model):

class EstateProperty(models.Model):

kfr-odoo · 2024-12-23T14:59:41Z

estate_account/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


This access rule exists already in the estate module

Doesn't each module have to own its own access rules?! Is it because we inherit from the estate module, then its access rules apply here as well?

kfr-odoo · 2024-12-27T15:08:30Z

awesome_owl/static/src/components/TodoList/todolist.js

+        this.todoItemsNr =0;
+        useAutofocus("todo_input");
+    }
+    addTodo(inp) {


JS convention

Suggested change

addTodo(inp) {

addTodo(event ) {

kfr-odoo · 2024-12-27T15:10:08Z

awesome_owl/static/src/components/TodoList/todolist.js

+
+            }
+            inp.target.value = '';
+            this.render();


You should probably not need to call this method explicitly

Suggested change

this.render();

kfr-odoo · 2024-12-27T15:13:11Z

estate/models/estate_property_offer.py

+
+    @api.model_create_multi
+    def create(self, vals):
+        property_record = self.env['estate.property'].browse(vals['property_id'])


Suggested change

property_record = self.env['estate.property'].browse(vals['property_id'])

property_record = self.env['estate.property'].browse(vals['property_id']).exists()

kfr-odoo · 2024-12-27T15:14:17Z

estate/models/estate_property_offer.py

+        existing_offers = self.search([
+            ('property_id', '=', vals['property_id'])
+        ])
+        if any(offer.price >= vals['price'] for offer in existing_offers):


The price comparison should ba a part of the search query above for optimal performance

kfr-odoo requested changes Dec 17, 2024

View reviewed changes

kfr-odoo reviewed Dec 19, 2024

View reviewed changes

AbdelrahmanFawzy1 and others added 15 commits December 20, 2024 10:45

[ADD] Create new module for Chapter 1

c097d69

[IMP] estate: add ORM table fields

95b57eb

[IMP] estate: security permissions added

1c040ea

[IMP] estate: views added

cc2e471

[IMP] estate: More views Added

2c0d253

[IMP] estate: Models Relationships

4792d6a

[IMP] Add computed fields

fc1c6e2

[IMP] Actions Added

09a8058

[IMP] Constraints Added

5f8c979

[IMP] fix

927b96d

[IMP] add license field

32dd96b

[CLN] Clean up estate_property.py formatting

9eef81a

Co-authored-by: Killian Frappart <kfr@odoo.com>

[FIX] Update estate_property.py for issue resolution

46e1e6e

Co-authored-by: Killian Frappart <kfr@odoo.com>

[IMP] Update estate_property_offer.py for improvements

b289ebc

Co-authored-by: Killian Frappart <kfr@odoo.com>

[IMP] style files using black

e4e3326

AbdelrahmanFawzy1 force-pushed the 18.0-training-abdh branch from b5524df to e4e3326 Compare December 20, 2024 09:48

kfr-odoo reviewed Dec 20, 2024

View reviewed changes

AbdelrahmanFawzy1 added 4 commits December 23, 2024 07:01

[IMP] Added Sprinkles

7d44f47

[IMP] Inheritance added

4e1d2a7

[IMP] Invoicing Linked

77004d5

[IMP] Kanban View added

2da5257

kfr-odoo reviewed Dec 23, 2024

View reviewed changes

AbdelrahmanFawzy1 added 3 commits December 24, 2024 09:26

[IMP] Issues Fix

ac82719

[IMP] Chapter 1: Web Framework

0445d11

[IMP] Chapter 2 first parts

44a16fb

kfr-odoo approved these changes Dec 27, 2024

View reviewed changes

	self.garden_area = 10 if self.garden else False
	self.garden_area = 10 if self.garden else 0

		@@ -0,0 +1,30 @@
		from odoo import models, Command

		class InheritedEstateProperty(models.Model):

	class InheritedEstateProperty(models.Model):
	class EstateProperty(models.Model):

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

	property_record = self.env['estate.property'].browse(vals['property_id'])
	property_record = self.env['estate.property'].browse(vals['property_id']).exists()

Conversation

AbdelrahmanFawzy1 commented Dec 17, 2024

Uh oh!

robodoo commented Dec 17, 2024

Uh oh!

kfr-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kfr-odoo commented Dec 19, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants