18.0 server tutorial frva #737

frva-odoo · 2025-04-23T07:31:25Z

No description provided.

robodoo · 2025-04-23T07:31:28Z

naja628

Hello :) good work !

I left some comments on the code.

The little red things at the end of your files in the github ui are probably because you're missing end of line characters at the end of the last line. Your editor can probably be configured so it doesn't do that.

naja628 · 2025-04-23T08:09:01Z

estate/__init__.py

@@ -0,0 +1,3 @@
+from . import models
+
+from odoo import api, SUPERUSER_ID


What are you trying to do with this line?

I pasted it from somewhere I found and forgot to see what it does ... I erased it for the next commit since it doesn't look mandatory

naja628 · 2025-04-23T08:22:43Z

estate/models/estate_property.py

+    name = fields.Char('Estate Name',required=True, translate=True)
+    description = fields.Text('Description')
+    postcode = fields.Char('Postcode')
+    date_availability = fields.Date('Date Availability', copy=False, default=fields.Date.add(fields.Date.today(),months=3))


Be careful for the default, if you do it this way fields.Date.today() will get evaled at the time where the class is evaled (so when odoo is started) instead of when we create a new record. default accepts lambda functions as well for this reason.

nitpick: for the label i think something like 'Date Available' or 'Availability Date' which is more natural-language-y would be better

naja628 · 2025-04-23T08:24:24Z

estate/views/estate_menus.xml

descriptive names please

naja628 · 2025-04-23T08:26:14Z

estate/models/estate_property.py

+    garden_area = fields.Integer('Garden Area')
+    garden_orientation = fields.Selection(string='Type', selection=[('north', 'North'), ('south', 'South'),('west', 'West'), ('east', 'East')])
+    active = fields.Boolean(default=True)
+    state = fields.Selection(required=True, copy=False, default='New', selection=[('New', 'New'), ('Offer Received', 'Offer Received'),('Offer Accepted', 'Offer Accepted'), ('Sold', 'Sold'), ('Cancelled', 'Cancelled')])


nitpick: usually we like the technical names of a selection to feel more code-like, so lower_snake_case

naja628 · 2025-04-24T13:29:29Z

estate/models/estate_property.py

+    name = fields.Char('Title', required=True, translate=True)
+    description = fields.Text('Description')
+    postcode = fields.Char('Postcode')
+    date_availability = fields.Date('Date Available', copy=False, default=lambda self: self._get_day_in_3_months())


Hello -- your answer to this #737 (comment) here is a bit redundant because you're both using a lambda and defining a fuction
You could either inline the functions return in the lambda or define the function above the fields and do default=_get_day_in_3_months.
By convention when we do the latter we usually name the function something like _default_<fieldname>

naja628 · 2025-04-24T13:32:04Z

@vava-odoo I'll let you take over here :)

vava-odoo

Good job! Just a few ( 👀 ) comments. Don't hesitate to ask if you have questions!

vava-odoo · 2025-04-28T10:46:06Z

estate/models/__init__.py

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


Convention is one file imported at a time

vava-odoo · 2025-04-28T10:46:41Z

estate/models/estate_property.py

+from odoo import fields, models
+from odoo import api
+from odoo import exceptions


But nor here 😇
Also, we usually import UserError and ValidationError direclty

Suggested change

from odoo import fields, models

from odoo import api

from odoo import exceptions

from odoo import api, fields, models

from odoo.exceptions import UserError, ValidationError

vava-odoo · 2025-04-28T10:50:09Z

estate/models/estate_property.py

+    garden_area = fields.Integer('Garden Area')
+    garden_orientation = fields.Selection(string='Orientation Type', selection=[('north', 'North'), ('south', 'South'), ('west', 'West'), ('east', 'East')])
+    active = fields.Boolean(default=True)
+    state = fields.Selection(required=True, copy=False, default='new', selection=[('new', 'New'), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'), ('sold', 'Sold'), ('cancelled', 'Cancelled')])


A bit long... Suggestion:

Suggested change

state = fields.Selection(required=True, copy=False, default='new', selection=[('new', 'New'), ('offer_received', 'Offer Received'), ('offer_accepted', 'Offer Accepted'), ('sold', 'Sold'), ('cancelled', 'Cancelled')])

state = fields.Selection(

required=True, copy=False, default='new',

selection=[

('new', 'New'),

('offer_received', 'Offer Received'),

('offer_accepted', 'Offer Accepted'),

('sold', 'Sold'),

('cancelled', 'Cancelled'),

],

)

vava-odoo · 2025-04-28T10:52:26Z

estate/models/estate_property.py

+         'The selling price must be positive.')
+    ]
+
+    @api.onchange("selling_price", "expected_price")


Why this onchange? You only want to constrain, don't you?

I thought it was necessary to be executed at each time the selling or expected price changes but I guess it's not then

vava-odoo · 2025-04-28T10:55:07Z

estate/models/estate_property.py

+
+    @api.depends("living_area", "garden_area")
+    def _compute_total_area(self):
+        self.total_area = self.living_area + self.garden_area


What if you edit several records at once?

vava-odoo · 2025-04-28T11:11:30Z

estate_account/models/estate_property.py

+            invoice_dictionary = {
+                "partner_id": record.buyer_id.id,
+                "move_type": "out_invoice",
+                "journal_id": self.env['account.journal'].search([("code", "=", "INV")]).id,


Why do you need a journal? What happen if there is none?
You could also add limit=1 to improve the perf

vava-odoo · 2025-04-28T11:13:21Z

estate_account/models/estate_property.py

+                ]
+            }
+            self.env["account.move"].create(invoice_dictionary)
+            super().action_sell_property()


You need to return super(), outside of the loop
Also, it would be interesting to call super first, to trigger the validation error if there is one before creating the invoice. It would be something like

Suggested change

super().action_sell_property()

res = super().action_sell_property()

for record in self:

...

return res

vava-odoo · 2025-04-28T11:13:55Z

estate_account/models/estate_property.py

+                    })
+                ]
+            }
+            self.env["account.move"].create(invoice_dictionary)


It would be best to batch create with a list of invoice values, outside of the loop on self.

vava-odoo · 2025-04-28T11:14:48Z

estate_account/__manifest__.py

+        'estate',
+        'account'
+    ],
+    'data': [],


Empty, can be dropped

vava-odoo · 2025-04-28T11:15:08Z

estate_account/security/ir.model.access.csv

Empty, not in manifest... Can be removed 🙂

vava-odoo · 2025-04-29T09:54:07Z

estate_account/models/estate_property.py

+        for val in invoice_values:
+            self.env["account.move"].create(val)


The purpose of my previous comment was to be able to create multiple invoices at once

Suggested change

for val in invoice_values:

self.env["account.move"].create(val)

self.env["account.move"].create(invoice_values)

naja628

Hello :) -- Good work !
I left some comments on the js tutorial

naja628 · 2025-05-02T13:58:57Z

awesome_owl/static/src/card/card.js

+    static template = "Card";
+    static props = {
+        title: String,
+        slots: Object


Suggested change

slots: Object

slots: Object,

Also: could you make the slots optional so that something like below doesn't give an error?

<Card title='...'/>

naja628 · 2025-05-02T13:59:32Z

awesome_owl/static/src/card/card.js

+    };
+
+    setup(){
+        this.state = useState( {open: true});


nitpick: inconsistent spacing

Suggested change

this.state = useState( {open: true});

this.state = useState({ open: true });

naja628 · 2025-05-02T14:09:58Z

awesome_owl/static/src/counter/counter.js

+        if (this.props.onChange){
+            this.props.onChange()
+        }


tip: js syntactic sugar 🌈 (subjective -- it's ok to not like/use it)

Suggested change

if (this.props.onChange){

this.props.onChange()

}

this.props.onChange?.();

naja628 · 2025-05-02T14:11:04Z

awesome_owl/static/src/counter/counter.js

+    increment(){
+        this.state.value++;
+        if (this.props.onChange){
+            this.props.onChange()


nitpick: inconsistent ; usage (do either always or never)

Suggested change

this.props.onChange()

this.props.onChange();

naja628 · 2025-05-02T14:14:44Z

awesome_owl/static/src/playground.xml

+                <Counter onChange.bind="incrementSum"/>
+            </Card>
+            <Card title="'boring title'">
+                Some text egjpojges


naja628 · 2025-05-02T14:59:17Z

awesome_owl/static/src/todolist/todolist.js

+         if (todoIdInArray >= 0) {
+             this.todos.splice(todoIdInArray, 1);
+         }


indentation

Suggested change

if (todoIdInArray >= 0) {

this.todos.splice(todoIdInArray, 1);

}

if (todoIdInArray >= 0) {

this.todos.splice(todoIdInArray, 1);

}

naja628 · 2025-05-02T15:00:09Z

awesome_owl/static/src/utils.js

+export function UseAutofocus(refString){
+    var myRef = useRef(refString);
+    onMounted(() => {
+        console.log(myRef.el)


left-over debug code

naja628 · 2025-05-02T15:04:44Z

awesome_owl/static/src/todolist/todolist.js

+    }
+
+    removeTodo(todoId){
+        var todoIdInArray = this.todos.findIndex((todo) => todo.id == todoId);


Suggested change

var todoIdInArray = this.todos.findIndex((todo) => todo.id == todoId);

const todoIdInArray = this.todos.findIndex((todo) => todo.id == todoId);

never use var; prefer const over let when possible

naja628 · 2025-05-02T15:05:35Z

awesome_owl/static/src/utils.js

+
+
+export function UseAutofocus(refString){
+    var myRef = useRef(refString);


Suggested change

var myRef = useRef(refString);

const myRef = useRef(refString);

also myRef could use a better name

naja628 · 2025-05-02T15:06:55Z

awesome_dashboard/static/src/dashboard.js

+            type: 'ir.actions.act_window',
+            name: 'All leads',
+            res_model: 'crm.lead',
+            views: [[false, 'list'],[false, 'form']],


Suggested change

views: [[false, 'list'],[false, 'form']],

views: [[false, 'list'], [false, 'form']],

or evn

Suggested change

views: [[false, 'list'],[false, 'form']],

views: [

[false, 'list'],

[false, 'form'],

],

…eported issues

naja628 reviewed Apr 23, 2025

View reviewed changes

naja628 reviewed Apr 24, 2025

View reviewed changes

vava-odoo reviewed Apr 28, 2025

View reviewed changes

frva-odoo added 12 commits April 28, 2025 13:44

[IMP] estate: End of chapter 4 commit

c12884e

[IMP] estate: End of Chapter 5

d7696ab

[IMP] estate: End of chapter 6

8c00a6e

[FIX] estate: Clean code and correction of reported issues

4c39311

[IMP] estate: End of chapter 7

6f18a4b

[IMP] estate: End of day 2

438818c

[IMP] estate: End of chapter 8

dbbf98e

[IMP] estate: End of chapter 9

d1cf2bc

[IMP] estate: End of chapter 11

ecd1179

[IMP] estate: End of chapter 12

d9ff057

[ADD] estate_account: end of tutorial chapter 13

10d9c53

[IMP] estate: End of tutorial chapter 14

0475bd5

frva-odoo force-pushed the 18.0-server-tutorial-frva branch from f9b9613 to 0475bd5 Compare April 28, 2025 11:47

[FIX] estate,estate_account: Correction of issues

dda9737

frva-odoo force-pushed the 18.0-server-tutorial-frva branch from 265d728 to dda9737 Compare April 29, 2025 06:49

vava-odoo reviewed Apr 29, 2025

View reviewed changes

frva-odoo added 7 commits April 29, 2025 16:13

[IMP] awesome_owl: component, subcomponent and props

4f9e253

[IMP] awesome_owl: markup, props validation and callback prop

89d84e7

[IMP] awesome_owl: todo, dynamic attributes and adding todos

7dd734d

[IMP] awesome_owl: focusing input, togglin and deleting todos

5c6eb35

[IMP] awesome_owl: generic cards and minimizing

3f63141

[IMP] awesome_dashboard: a new layout

ea5a464

[IMP] awesome_dashboard: Add some buttons for quick navigation (2)

d48b0ae

naja628 reviewed May 2, 2025

View reviewed changes

[IMP] awesome_dashboard: Add a dashboard item (3)

5a12840

[FIX] awesome_owl, awesome_dashboard, estate_account: correction of r…

1b8a7ab

…eported issues

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

18.0 server tutorial frva #737

18.0 server tutorial frva #737

frva-odoo commented Apr 23, 2025

robodoo commented Apr 23, 2025

naja628 left a comment •

edited

Loading

naja628 Apr 23, 2025

frva-odoo Apr 23, 2025 •

edited

Loading

naja628 Apr 23, 2025

naja628 Apr 23, 2025

naja628 Apr 23, 2025

naja628 Apr 24, 2025

naja628 commented Apr 24, 2025

vava-odoo left a comment •

edited

Loading

vava-odoo Apr 28, 2025

vava-odoo Apr 28, 2025

vava-odoo Apr 28, 2025

vava-odoo Apr 28, 2025

frva-odoo Apr 28, 2025

vava-odoo Apr 28, 2025

vava-odoo Apr 28, 2025

vava-odoo Apr 28, 2025

vava-odoo Apr 28, 2025

vava-odoo Apr 28, 2025

vava-odoo Apr 28, 2025

vava-odoo Apr 29, 2025

naja628 left a comment •

edited

Loading

naja628 May 2, 2025

naja628 May 2, 2025

naja628 May 2, 2025 •

edited

Loading

naja628 May 2, 2025

naja628 May 2, 2025

naja628 May 2, 2025

naja628 May 2, 2025

naja628 May 2, 2025

naja628 May 2, 2025

naja628 May 2, 2025

		@@ -0,0 +1,3 @@
		from . import models

		from odoo import api, SUPERUSER_ID

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

-            super().action_sell_property()
+            res = super().action_sell_property()
+            for record in self:
+                ...
+            return res

		for val in invoice_values:
		self.env["account.move"].create(val)

	for val in invoice_values:
	self.env["account.move"].create(val)
	self.env["account.move"].create(invoice_values)

	this.state = useState( {open: true});
	this.state = useState({ open: true });

	var todoIdInArray = this.todos.findIndex((todo) => todo.id == todoId);
	const todoIdInArray = this.todos.findIndex((todo) => todo.id == todoId);



		export function UseAutofocus(refString){
		var myRef = useRef(refString);

	var myRef = useRef(refString);
	const myRef = useRef(refString);

	views: [[false, 'list'],[false, 'form']],
	views: [[false, 'list'], [false, 'form']],

18.0 server tutorial frva #737

Are you sure you want to change the base?

18.0 server tutorial frva #737

Conversation

frva-odoo commented Apr 23, 2025

robodoo commented Apr 23, 2025

naja628 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frva-odoo Apr 23, 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 commented Apr 24, 2025

vava-odoo 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naja628 May 2, 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

naja628 left a comment •

edited

Loading

frva-odoo Apr 23, 2025 •

edited

Loading

vava-odoo left a comment •

edited

Loading

naja628 left a comment •

edited

Loading

naja628 May 2, 2025 •

edited

Loading