Skip to content

Conversation

@CILC98
Copy link

@CILC98 CILC98 commented Nov 29, 2025

No description provided.

@CILC98 CILC98 force-pushed the 18.0-mig-web_widget_remaining_days_exact_date branch 2 times, most recently from cb57ded to b17d68c Compare November 29, 2025 19:26
@CILC98 CILC98 force-pushed the 18.0-mig-web_widget_remaining_days_exact_date branch from b17d68c to 4c54e3f Compare November 30, 2025 00:30
Copy link
Contributor

@celm1990 celm1990 left a comment

Choose a reason for hiding this comment

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

Code review: I think it is necessary to split the commits. Please keep a single commit with only the changes related to the migration. If you want to improve this module, I think it is better to create a new PR with the new features, or at least put them in a separate commit and not in the migration commit.
What do you think, @CarlosRoca13 ?

@@ -0,0 +1,26 @@
# Copyright 2025 Tecnativa - Carlos Roca Trescloud - César León
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2025 Tecnativa - Carlos Roca Trescloud - César León
# Copyright 2025 Tecnativa - Carlos Roca
# Copyright 2025 Trescloud - César León
Comment on lines +20 to +26
raise models.ValidationError(
_(
"There is already a rule for the model '%s'."
" You cannot create two rules for the same model."
)
% record.res_model_id.name
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise models.ValidationError(
_(
"There is already a rule for the model '%s'."
" You cannot create two rules for the same model."
)
% record.res_model_id.name
)
raise models.ValidationError(
self.env._(
"There is already a rule for the model '%s'."
" You cannot create two rules for the same model.",
record.res_model_id.name
)
)
Comment on lines +140 to +151
"line_ids": [
(
0,
0,
{
"name": name,
"technical_name": technical_name,
"active": active,
},
)
for name, technical_name, active in view_types
],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"line_ids": [
(
0,
0,
{
"name": name,
"technical_name": technical_name,
"active": active,
},
)
for name, technical_name, active in view_types
],
"line_ids": [
Command.create(
{
"name": name,
"technical_name": technical_name,
"active": active,
},
)
for name, technical_name, active in view_types
],
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow the OCA guidelines. The order of fields, CRUD methods, etc. is described here:
https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#37models



@tagged("post_install", "-at_install")
class TestWidgetRemainingDaysExactDate(common.TransactionCase):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please inherit from BaseCommon

Suggested change
class TestWidgetRemainingDaysExactDate(common.TransactionCase):
class TestWidgetRemainingDaysExactDate(BaseCommon):
Comment on lines +7 to +9
@classmethod
def setUpClass(cls):
return super().setUpClass()
Copy link
Contributor

Choose a reason for hiding this comment

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

If this function does nothing, please remove it. Otherwise, implement the logic here to prepare the data for the test.

Suggested change
@classmethod
def setUpClass(cls):
return super().setUpClass()
Comment on lines +9 to +11
create="true"
edit="true"
delete="true"
Copy link
Contributor

Choose a reason for hiding this comment

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

These options are the default values, so it is not necessary to set them.

Suggested change
create="true"
edit="true"
delete="true"
@@ -0,0 +1,49 @@
/** @odoo-module **/
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/** @odoo-module **/
Comment on lines +30 to +32
active = fields.Boolean(
string="Active record",
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t see the point of having an active field in a TransientModel. Why do you need this field? I think it can be removed.

Suggested change
active = fields.Boolean(
string="Active record",
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants