Skip to content
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

[17.0][MIG] maintenance_plan: Migration to 17.0 #414

Merged
merged 112 commits into from
Aug 8, 2024

Conversation

FernandoRomera
Copy link

No description provided.

grindtildeath and others added 30 commits July 23, 2024 14:44
… equipment maintenance team is not filled

[ADD] icon.png
[UPD] Update maintenance_plan.pot
Currently translated at 100.0% (30 of 30 strings)

Translation: maintenance-12.0/maintenance-12.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-12-0/maintenance-12-0-maintenance_plan/es/

[UPD] README.rst
[ADD] instructions and notes to Plan

- Add menus for maintenance plans.
- Set active button on maintenance plans.

improve navigation to maintenance plans.

[ADD] post-migration script
[UPD] Update maintenance_plan.pot

[UPD] README.rst

maintenance_plan 12.0.3.0.0

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: maintenance-12.0/maintenance-12.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-12-0/maintenance-12-0-maintenance_plan/
[UPD] Update maintenance_plan.pot

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: maintenance-12.0/maintenance-12.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-12-0/maintenance-12-0-maintenance_plan/
Currently translated at 98.4% (63 of 64 strings)

Translation: maintenance-12.0/maintenance-12.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-12-0/maintenance-12-0-maintenance_plan/es/
[UPD] Update maintenance_plan.pot

[UPD] README.rst

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: maintenance-13.0/maintenance-13.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-13-0/maintenance-13-0-maintenance_plan/
[UPD] Update maintenance_plan.pot

maintenance_plan 13.0.1.1.0

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: maintenance-13.0/maintenance-13.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-13-0/maintenance-13-0-maintenance_plan/

pre-commit update
Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: maintenance-13.0/maintenance-13.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-13-0/maintenance-13-0-maintenance_plan/
[UPD] Update maintenance_plan.pot

[UPD] README.rst

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: maintenance-13.0/maintenance-13.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-13-0/maintenance-13-0-maintenance_plan/
[UPD] Update maintenance_plan.pot

[UPD] README.rst
Currently translated at 100.0% (69 of 69 strings)

Translation: maintenance-14.0/maintenance-14.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-14-0/maintenance-14-0-maintenance_plan/it/
* Use latest request_date both for done and todo searches.
* When there is a latest done apply interval

Co-authored-by: Lois Rilo <[email protected]>
Co-authored-by: LoisRForgeFlow <[email protected]>

maintenance_plan 14.0.1.0.1
Currently translated at 100.0% (69 of 69 strings)

Translation: maintenance-14.0/maintenance-14.0-maintenance_plan
Translate-URL: https://translation.odoo-community.org/projects/maintenance-14-0/maintenance-14-0-maintenance_plan/it/
* only consider request after the start maintenance date.
* todo request ordered asc.
* add more tests.

maintenance_plan 14.0.1.0.2
@FernandoRomera
Copy link
Author

@pedrobaeza @astirpe @etobella
Can you merge this PR, please?

@pedrobaeza
Copy link
Member

A PSC or maintainer should confirm, like @etobella

"author": "Camptocamp SA, ForgeFlow, Odoo Community Association (OCA)",
"license": "AGPL-3",
"category": "Maintenance",
"website": "https://github.com/OCA/maintenance",
"images": [],
"depends": ["base_maintenance"],
"depends": ["base_maintenance", "maintenance"],
Copy link
Member

Choose a reason for hiding this comment

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

I think maintenance is not required (base_maintenance depends on it)

https://github.com/OCA/maintenance/blob/17.0/base_maintenance/__manifest__.py#L12

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, my mistake.

Copy link
Author

Choose a reason for hiding this comment

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

@etobella
Done.

@@ -23,17 +21,17 @@ def post_init_hook(cr, registry):

for equipment in equipments:
request = equipment.maintenance_ids.filtered(
lambda r: r.maintenance_type == "preventive"
lambda r, equipment=equipment: r.maintenance_type == "preventive"
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Author

Choose a reason for hiding this comment

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

B023 Function definition does not bind loop variable `equipment

https://docs.astral.sh/ruff/rules/function-uses-loop-variable/`

Copy link
Member

Choose a reason for hiding this comment

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

I see, it is a new requirement from ruff 👍

@FernandoRomera FernandoRomera force-pushed the 17.0-mig-maintenance_plan branch from f86fb60 to b4bb3bc Compare August 8, 2024 06:09
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@etobella
Copy link
Member

etobella commented Aug 8, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 17.0-ocabot-merge-pr-414-by-etobella-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit aec528a into OCA:17.0 Aug 8, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 34b68d6. Thanks a lot for contributing to OCA. ❤️

@victoralmau
Copy link
Member

Analyzing the maintenance module in v17 to add the migration script to OpenUpgrade I see that what is indicated here #414 (comment) is not exactly correct, because there is a recurrence field (recurring_maintenance and extra fields).

When creating now a request of preventive type and set recurrence (7 days for example), when the status is changed to one defined as “completed” (done field) a new request will be created (copy) with the corresponding date (schedule_date) https://github.com/odoo/odoo/blob/17.0/addons/maintenance/models/maintenance.py#L337 and in the new one we can maintain that recurrence, change it, etc (the corresponding activity is also added).

Having clarified the above I am not sure that this module is necessary. I comment this because the approach of the migration script will be different (define the corresponding data to use the odoo recurrence or use this module).

@victoralmau
Copy link
Member

What do you think about my previous comment (OpenUpgrade scripts will depend on it because maybe this module is not necessary)? Ping @carlos-lopez-tecnativa

Copy link

@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

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

What do you think about my previous comment (OpenUpgrade scripts will depend on it because maybe this module is not necessary)? Ping @carlos-lopez-tecnativa

Thanks for the ping, @victoralmau. I tested this module and have some observations related to its migration.

Regarding recurrency, I believe this module provides a functionality that Odoo does not currently have—the ability to create maintenance requests in advance based on the Planning Horizon. In Odoo, you can only create the next maintenance request when the current one moves to a stage marked as "Done." However, for preventive maintenance in some contexts, it's necessary to generate all future maintenance requests in advance.

I think this module is still valuable but could benefit from some enhancements. For instance, it would be helpful if all maintenance requests related to the recurrency were visible within the maintenance.request form view, similar to how related tasks are displayed in project.task.

What do you think about this approach?

image

Comment on lines +8 to +11
<field name="context">{
'default_maintenance_plan_id': id,
}</field>
<field name="domain">[('maintenance_plan_id', '=', id)]</field>

Choose a reason for hiding this comment

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

image

Comment on lines +121 to +135
def name_get(self):
result = []
for plan in self:
result.append(
(
plan.id,
plan.name
or _(
"Unnamed %(kind)s plan (%(eqpmt)s)",
kind=plan.maintenance_kind_id.name or "",
eqpmt=plan.equipment_id.name,
),
)
)
return result

Choose a reason for hiding this comment

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

Not blocking, but this can be replaced by _compute_display_name according to the migration guidelines.
https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-17.0

@carlos-lopez-tecnativa
Copy link

Please let me know the progress on this module's evolution. I plan to start the migration of the maintenance_plan_employee module, which depends on this one. @pedrobaeza

@pedrobaeza
Copy link
Member

@carlos-lopez-tecnativa do the PR improving this module for not getting blocked.

@victoralmau
Copy link
Member

My conclusions in summary after discussing it with my colleague @carlos-lopez-tecnativa

1- Keep the OCA module

Pros:

Cons:

  • We are not taking advantage of the approach (fields) of recurrence in requests, causing confusion for whoever uses them and/or when they discover it.
  • We are adding code (and the module itself) maintenance_plan that core has already contemplated otherwise.
  • It is necessary to make change in Openupgrade [17.0][OU-ADD] maintenance: Migration scripts OpenUpgrade#4654

2- Do NOT use the OCA module

Pros:

  • No inconvenience for users who did not know it until now.
  • You can create recurrences (plans) from the first request.
  • Not all future requests are created at once, the next one will be created when the previous one is finished (more flexibility in the change of recurrence along the “plan”).
  • Removes code to maintain (maintenance_plan).

Cons:

  • Not all plan requests are created (only for users who already used it previously, in which case an OCA module could be created that by means of a button on the requests allows to create all future requests avoiding/confirming that on completion it will not create the next one).
  • Migration scripts in OpenUpgrade to adapt plans to linked stock requests and corresponding recurrence fields. Possible discussion if future uninitiated requests need to be deleted (to avoid confusion) (as they will be created when the next one is finalized).

Having clarified all this, I am of the opinion that the maintenance_plan module should be removed (having analyzed this before migrating it) and focus it towards core usage, applying the corresponding changes in Openupgrade at OCA/OpenUpgrade#4654.

Ping @pedrobaeza

@pedrobaeza
Copy link
Member

OK then on stop using this module. Let's do the OU thing, and we can drop in this module a deprecated message in the README.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.