-
-
Notifications
You must be signed in to change notification settings - Fork 543
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
[16.0][FIX] server_action_mass_edit: Don't do onchange #984
base: 16.0
Are you sure you want to change the base?
[16.0][FIX] server_action_mass_edit: Don't do onchange #984
Conversation
This change was introduced unnoticed in the migration to 16.0 in OCA#544, but it has several problems: - The onchange is only executed for one record, not several, and this module is for mass actions. - Not all the users want to play onchanges, being for one record or several. Let's remove it for being consistent. An ongoing discussion for supporting onchanges through `onchange_helper` is meanwhile active.
@grindtildeath could you review this one, as you worked a lot on onchange topic ? thanks ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what this override is meant to do, but it is not working anyway 🙄
Maybe @ramiadavid can give the reason of the change? Even if the code seems weird indeed. |
Actually, I don't remember the reason for this, from the git history I understand that the change is mine, but I have no memory of having done this myself, it seems strange to me that I have made these changes in a migration because it is not even a module that let's use a lot. |
Done there : #544 And particularly there: 617be5f#diff-8d785e151e9ad076da0aff7904315bc3ca9dc70ad1fca741b35e07beced4198fR75 |
Hi, I tried and couldn't open the wizard Here is the traceback
|
When I remove these lines, it works. I guess they should be removed too, dont you think? server-ux/server_action_mass_edit/wizard/mass_editing_wizard.py Lines 91 to 94 in 5c82c07
|
This change was introduced unnoticed in the migration to 16.0 in #544, but it has several problems:
Let's remove it for being consistent. An ongoing discussion for supporting onchanges through
onchange_helper
is meanwhile active.@Tecnativa