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

[FIX] mrp: avoid module warning when a group is removed #32887

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@dbo-odoo
Copy link
Contributor

commented Apr 23, 2019

Due to a weird combination of dependencies and settings, the behaviour
of the workorder checkbox in the MRP configuration wizard is a bit
dangerous:

  • if the group is enabled via another mechanism (e.g. a technical view)
    before accessing the wizard, the default is set to True and an invisible
    module_mrp_workorder field is enabled, promptly causing the
    installation of the module upon saving (although the user did not modify
    a single thing in the wizard)
  • if the module was installed through another mechanism (e.g. the apps
    menu) but the group was disabled, accessing the wizard will cause the
    group to get unchecked, promptly causing the removal of your module and
    all its data \o/

The second case is quite stressful for users, since accessing their
configuration wizard for the first time will immediately display a
warning that unchecking that option will uninstall a few modules (even
though they didn't actually uncheck anything).

This commit tries to mitigate this second case. The onchange behaviour
needs to adapt to the actual state of the module in database - we
cannot rely on the module_wrp_workorder field since there is no way to
know if it was modified by another onchange, for example. Based on the
actual module state, we split the onchange behaviour:

  • if mrp_workorder is not installed, then triggering the group field
    should be mirrored on the module field (enabling it should install the
    module, disabling it should cancel the module installation since it must
    have happened in the same transaction)
  • if mrp_workorder is installed, then disabling the group should not
    uninstall it and enabling the group should, well, enable the group and
    leave the module selection as it found it

@dbo-odoo dbo-odoo requested a review from sle-odoo Apr 23, 2019

@robodoo robodoo added the seen 🙂 label Apr 23, 2019

@dbo-odoo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

@sle-odoo
the quality dependency diamond conundrum we talked about

@dbo-odoo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

might be backported to v11 I think

@robodoo robodoo added the CI 🤖 label Apr 23, 2019

@dbo-odoo dbo-odoo force-pushed the odoo-dev:12.0-shitty-onchange-dbo branch 2 times, most recently from 593cb77 to 0882007 Apr 24, 2019

@robodoo robodoo removed the CI 🤖 label Apr 24, 2019

[FIX] mrp: avoid module warning when a group is removed
Due to a weird combination of dependencies and settings, the behaviour
of the workorder checkbox in the MRP configuration wizard is a bit
dangerous:
- if the group is enabled via another mechanism (e.g. a technical view)
before accessing the wizard, the default is set to True and an invisible
`module_mrp_workorder` field is enabled, promptly causing the
installation of the module upon saving (although the user did not modify
a single thing in the wizard)
- if the module was installed through another mechanism (e.g. the apps
menu) but the group was disabled, accessing the wizard will cause the
group to get unchecked, promptly causing the removal of your module and
all its data \o/

The second case is quite stressful for users, since accessing their
configuration wizard for the first time will immediately display a
warning that unchecking that option will uninstall a few modules (even
though they didn't actually uncheck anything).

This commit tries to mitigate this second case. The onchange behaviour
needs to adapt to the *actual* state of the module in database - we
cannot rely on the module_wrp_workorder field since there is no way to
know if it was modified by another onchange, for example. Based on the
actual module state, we split the onchange behaviour:
- if mrp_workorder *is not* installed, then triggering the group field
should be mirrored on the module field (enabling it should install the
module, disabling it should cancel the module installation since it must
have happened in the same transaction)
- if mrp_workorder *is* installed, then disabling the group should *not*
uninstall it and enabling the group should, well, enable the group and
leave the module selection as it found it

@dbo-odoo dbo-odoo force-pushed the odoo-dev:12.0-shitty-onchange-dbo branch from 0882007 to c128c10 Apr 24, 2019

@dbo-odoo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

already fixed at 6c70217

@dbo-odoo dbo-odoo closed this Apr 24, 2019

@robodoo robodoo added the closed 💔 label Apr 24, 2019

@dbo-odoo dbo-odoo deleted the odoo-dev:12.0-shitty-onchange-dbo branch Apr 24, 2019

@sle-odoo

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

noice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.