Skip to content

18.0 tutorial nire #816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: 18.0
Choose a base branch
from
Open

18.0 tutorial nire #816

wants to merge 18 commits into from

Conversation

nire-odoo
Copy link

No description provided.

@robodoo
Copy link

robodoo commented Jun 18, 2025

Pull request status dashboard

Copy link

@bso-odoo bso-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a few cosmetic comments.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try to always include the return character at the end of the last line in every file.
This way, when a new line is added later, the diff will not impact this previously last line.

@@ -6,6 +6,9 @@
# 'version': '18.0',
'author': 'Nicolas Renard',
'data': [
'views/estate_property_views.xml',
'views/estate_menus.xml',
'security/ir.model.access.csv'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In arrays, lists, sets, dictionaries, objects... we include a , comma after the last entry too.
This way, if a new entry is added, the line will not be part of the diff.

Copy link

@bso-odoo bso-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a few comments:

Comment on lines 0 to 1
from . import models No newline at end of file
from . import models

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's take this opportunity to train using git: try to apply such changes directly within the commit where the line was introduced.
You can use the git rebase -i command to rewrite the history on your local branch, then you'll need to use git push --force-with-lease to send sush changes to the repo. (Try not to use git push --force to avoid accidents when you'll work with other people on a common branch)


name = fields.Char("Name",required=True)


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny... this line shows empty lines at the end... but somehow says that it does not end with a return. >_<

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there are spaces on it.

access_estate_property_type,estate_property_type,model_estate_property_type,base.group_user,1,1,1,1
access_estate_property_tag,estate_property_tag,model_estate_property_tag,base.group_user,1,1,1,1
access_estate_property_offer,estate_property_offer,model_estate_property_offer,base.group_user,1,1,1,1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return on last line also on XML and CSV.

@nire-odoo nire-odoo force-pushed the 18.0-tutorial-nire branch 2 times, most recently from c09b8c2 to 43cf3f3 Compare June 19, 2025 07:53
Copy link

@bso-odoo bso-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a few comments.
You can also try to adapt your commit messages in order to match the commit guidelines.

Comment on lines 15 to 37
living_area = fields.Integer("Living area")
living_area = fields.Integer("Living area (sqm)")
facades = fields.Integer("Facades")
garage = fields.Boolean("Garage")
garden = fields.Boolean("Garden")
garden_area = fields.Integer("Garden area")
garden_orientation = fields.Selection(
string='Garden orientation',
selection=[('north', 'North'), ('south', 'South'), ('east', 'East'), ('west', 'West')],
selection=[('north', "North"), ('south', "South"), ('east', "East"), ('west', "West")],
help="This Type is used to tell the garden orientation for a property"
)
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', 'Canceled')],
selection=[('new', "New"), ('offer_received', "Offer received"), ('offer_accepted', "Offer Accepted"), ('sold', "Sold"), ('cancelled', "Canceled")],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to do such changes in the commit where they initially appeared.


name = fields.Char("Name",required=True)


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there are spaces on it.

Copy link

@bso-odoo bso-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some comments.
Note that you still have some files that do not have a return on their last line.

<field name="name">estate.tag.list.view</field>
<field name="model">estate.property.tag</field>
<field name="arch" type="xml">
<list string="Tests" editable='top'>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use " around attribute values in XML

def _unlink_except_new_or_cancelled(self):
for property in self:
if property.state == 'new' or property.state == 'cancelled':
raise UserError("Can't delete a property that is new or cancelled!")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those strings are not implicitly translated, you have to explicitly call _ (from odoo import _)

@nire-odoo nire-odoo force-pushed the 18.0-tutorial-nire branch 3 times, most recently from 745271b to c49701b Compare June 26, 2025 08:12
@nire-odoo nire-odoo force-pushed the 18.0-tutorial-nire branch from c49701b to e0c81e2 Compare June 26, 2025 08:27
Comment on lines +18 to +21
onWillUpdateProps(() => {
this.chart.destroy()
this.renderChart()
})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, this.props still contains the old props. (Therefore the name "on WILL update")
onWillUpdateProps((props) => {...}) receives the new props as parameter.
You most likely want the render to use the most recent ones, instead of the old ones.

@nire-odoo nire-odoo force-pushed the 18.0-tutorial-nire branch from 9b9bc86 to d2bc9b0 Compare June 30, 2025 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants