[18.0][MIG] server_action_mass_edit: Migration to 18.0 #951

trisdoan · 2024-10-10T02:37:20Z

Note

based on: [18.0][FIX] base_technical_features: fix incorrect replacement #980

pedrobaeza · 2024-10-10T06:27:11Z

/ocabot migration server_action_mass_edit

florenciafrigieri2 · 2024-10-16T13:44:28Z

@trisdoan Hi! I tested the module but it didn't worked.
I created a server action to change the salesperson in the SOs, but when selecting the action within sales an error came up (I was not able to even see the wizard).

This are the last lines of the error:
File "/mnt/data/odoo-addons-dir/server_action_mass_edit/wizard/mass_editing_wizard.py", line 93, in onchange
dynamic_fields["selection__" + line.field_id.name] = fields.Selection(
File "/opt/odoo/odoo/fields.py", line 2772, in init
self._selection = dict(selection) if isinstance(selection, list) else None
ValueError: dictionary update sequence element #0 has length 0; 2 is required

I leave here a print of the server action

trisdoan · 2024-10-18T08:45:55Z

@trisdoan Hi! I tested the module but it didn't worked. I created a server action to change the salesperson in the SOs, but when selecting the action within sales an error came up (I was not able to even see the wizard).

This are the last lines of the error: File "/mnt/data/odoo-addons-dir/server_action_mass_edit/wizard/mass_editing_wizard.py", line 93, in onchange dynamic_fields["selection__" + line.field_id.name] = fields.Selection( File "/opt/odoo/odoo/fields.py", line 2772, in init self._selection = dict(selection) if isinstance(selection, list) else None ValueError: dictionary update sequence element #0 has length 0; 2 is required

I leave here a print of the server action

Hi @florenciafrigieri2, thank you for the review

I just added a fix, could you please try again?

TDu

I don't see the error mentioned by @florenciafrigieri2 anymore, but I am not familiar with this module !
Code review looks good.

henrybackman

Couple of minor comments

henrybackman · 2024-10-29T14:45:46Z

server_action_mass_edit/tests/test_mass_editing.py

+
+@common.tagged("-at_install", "post_install")
+class TestMassEditing(common.TransactionCase):
+    def setUp(self):


Would setUpClass work for the common test setup to avoid running the setup separately for each test case?

henrybackman · 2024-10-29T14:51:26Z

server_action_mass_edit/tests/test_mass_editing.py

+    def test_wiz_fields_view_get(self):
+        """Test whether fields_view_get method returns arch.


Suggested change

def test_wiz_fields_view_get(self):

"""Test whether fields_view_get method returns arch.

def test_wiz_get_view(self):

"""Test whether get_view method returns arch.

henrybackman · 2024-10-29T14:52:34Z

server_action_mass_edit/wizard/mass_editing_wizard.py

+        return vals_list
+
+    def read(self, fields=None, load="_classic_read"):
+        """Without this call, dynamic fields build by fields_view_get()


Suggested change

"""Without this call, dynamic fields build by fields_view_get()

"""Without this call, dynamic fields build by get_view()

florenciafrigieri2

Functional test! Now is working ok!

OCA-git-bot · 2024-10-29T15:07:27Z

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). 🤖

stefsava · 2024-10-29T18:25:52Z

It seems like there is a regression.
I can't bulk insert product attributes, while I could do it in 17.0.

Before it asked me to add a line,
now it doesn't ask and there are multiple columns.

With 17.0

With 18.0 and this PR

legalsylvain

See @stefsava comment.

trisdoan · 2024-11-07T03:52:00Z

Hello @stefsava, I checked and it looks to me that the issue is not because of the module

I tried with different o2m fields in another model
For product attribute, after https://github.com/odoo/odoo/pull/160270, there is a list view created and it has attribute create='false'. I changed to create='true' (for testing only), here's result:

stefsava · 2024-11-07T15:56:15Z

Hello @trisdoan

your suggestion doesn't solve my use case.

If I change create='true' I can see "Add a row", but the choice doesn't lead to anything useful.

Instead I found a workaround that brings the behavior back to that of 17.0 that I think can help you understand the problem detected.

I discovered that 17.0 only has the view "product.template.attribute.line.form" while
18.0 has added "product.template.attribute.line.view.list", and for some reason that I don't understand the list view is chosen instead of the form view.

My workaround was simply to deactive the list view, although I fear it may introduce regressions elsewhere.

trisdoan · 2024-11-29T05:15:55Z

~~DRAFT: working on failed test~~

…avoid incompatibilities with data from those users.

Currently translated at 100.0% (43 of 43 strings) Translation: server-ux-16.0/server-ux-16.0-server_action_mass_edit Translate-URL: https://translation.odoo-community.org/projects/server-ux-16-0/server-ux-16-0-server_action_mass_edit/tr/

Currently translated at 100.0% (44 of 44 strings) Translation: server-ux-17.0/server-ux-17.0-server_action_mass_edit Translate-URL: https://translation.odoo-community.org/projects/server-ux-17-0/server-ux-17-0-server_action_mass_edit/it/

…y field

…TumbaoJu

…TumbaoJu

add pre-init-hook

trisdoan · 2024-12-09T05:01:56Z

Hi, I updated the code to include a pre-hook which handles the renaming (benefit those who use the addon in 14.0 and now migrate to 18.0)

PaulGoubert

Functionnal review OK

grindtildeath · 2024-12-17T15:17:24Z

Thanks for the mig.
While we are at it, can we please bring #963 in the new version? 🙏

cc @etobella @pedrobaeza @legalsylvain

pedrobaeza · 2024-12-17T15:19:46Z

As said already in the other PR, that should be a different module, not integrated in this one. And even more in these recent versions, where onchanges are rare, as most of them have been replaced by computed writable fields.

legalsylvain · 2024-12-18T07:34:09Z

As said already in the other PR, that should be a different module, not integrated in this one. And even more in these recent versions, where onchanges are rare, as most of them have been replaced by computed writable fields.

As @grindtildeath, I'm also in favor of the integration of #963 in V18.

The main critic about [16.0][FIX] mass_edit: Play onchanges before writing #963 was the addition of a new dependency in a stable and widely deployed module, that could generate for certain types of deployment. In Guideline to prohibit or allow the addition of dependencies on stable modules odoo-community.org#193, add dependency in a migration PR was a consensus.
in [16.0][FIX] mass_edit: Play onchanges before writing #963, most of people was considering the PR as a bugfix.
In the meantime, there is an unanswered question here : [16.0][FIX] mass_edit: Play onchanges before writing #963 (comment). If you could elaborate a real use case when it is risky, opinion could change.

Thanks !

pedrobaeza · 2024-12-18T07:42:39Z

The module is there for 10 versions, and now it's a critical problem to not play onchanges. Come on...

etobella · 2024-12-18T08:50:48Z

My 2 cents.

Odoo is deprecating the use of onchange. If you see numbers, on 15, we had 513 onchange and 1645 depends on odoo core code. On 18, we have 461 onchange and 2935 depends. I think we shouldn't add this code and leave it on another module, the cost is minimal and if we check numbers, odoo is removing onchange for depends because we can. I would recommend to remove onchange on most of the modules we create if possible.

rousseldenis · 2024-12-18T10:18:06Z

I'm in favor too in #963 integration.

The module is there for 10 versions, and now it's a critical problem to not play onchanges. Come on...

In fact, I think I see the behavior expected by @pedrobaeza.

This module was used to mass edit fields without touching any other value in the record or elsewhere. A kind of database editor. Stop me if I'm wrong.

BUT:

That behavior is not documented in README. So, people are expecting the changes to automatically change the other values like in any other Odoo flow.
Current users are maybe not aware that their changes do not impact other fields (the remaining onchanges ones).

So, I think saying a misbehavior present for several versions is not a bug is not correct. As said before, maybe nobody saw that problem...

@pedrobaeza So, you rely on compute methods code and not in onchanges one ? That's weird.

IMHO, we should improve modules behavior when we can. When Odoo remove all onchanges, we will remove it.

I would recommend to remove onchange on most of the modules we create if possible.

@etobella Of course, this should maybe added in contribution guidelines. Concerning the numbers, I would say if there is only 1 onchange, that code should be still applicable.

pedrobaeza · 2024-12-18T10:36:14Z

Yes, onchanges are designed for changes in UI during the manual manipulation of the record, relying also in view features like readonly or non existing fields for its behavior. Computed methods, on contrary, are designed to work in a view-agnostic mode, and are more confident. Executing onchanges in a wild mode doing mass editions can lead to unexpected behaviors.

And anyway, the same you have the module sale_crm as a glue module between sale and crm, you must have server_action_mass_edit_onchange for the same, and make it optional for people.

Please stop wasting the time of all the good contributors involved in this discussion while there's a solution that fits all, although you consider it a bug. The bug is solved installing the module in all your instances.

etobella · 2024-12-18T10:37:42Z

According to Odoo documentation:
https://www.odoo.com/documentation/16.0/developer/tutorials/getting_started/09_compute_onchange.html?highlight=onchange

Odoo is not recommending onchange. So, I do not recommend to add this functionality here because it might be a problem later.

rousseldenis · 2024-12-18T10:41:52Z

The bug is solved installing the module in all your instances.

Which one and where?

Please stop wasting the time of all the good contributors

I suppose I'm on the other side

here's a solution that fits all,

I thought @grindtildeath @simahawk @legalsylvain @mmequignon were not in that way. But maybe I'm wrong

pedrobaeza · 2024-12-18T10:43:59Z

Which one and where?

Seriously? You are the one saying that it's a bug to not play onchanges. So the bug is solved creating the glue module and installing it.

rousseldenis · 2024-12-18T10:52:46Z

Which one and where?

Seriously? You are the one saying that it's a bug to not play onchanges. So the bug is solved creating the glue module and installing it.

Ok, don't be harsh, I've just reread the comments and some people stil say (or never said the contrary) that this is a bug:

They were the basis for my questioning.

pedrobaeza · 2024-12-18T10:57:51Z

Why forcing those that are totally against this "bugfix" to have it, even only in the new version? If there's no other solution, I understand that this debate makes sense, but the simpler solution to avoid the debate is to have that extra module. We are not saying that is forbidden to have it on OCA. We are just asking to not integrate it in the base module.

legalsylvain · 2024-12-18T13:40:37Z

Odoo is deprecating the use of onchange.

No. I read the text of your link (https://www.odoo.com/documentation/16.0/developer/tutorials/getting_started/09_compute_onchange.html?highlight=onchange) and there is no section about deprecation or about "obsolete feature that should not be used in the futur". Just explanation about pro and cons. I remember clearly an answer of Odoo SA developper, during a recent Odoo experience, saying that both tools remain valid.

V12 : 504 onchanges / 638 depends
V13 : 572 onchanges / 752 depends
V14 : 479 onchanges / 1258 depends
V15 : 507 onchanges / 1625 depends
V16 : 447 onchanges / 2168 depends
V17 : 452 onchanges / 2703 depends
V18 : 459 onchanges / 2934 depends

I don't see any massive withdrawal of onchange to make it disappear. On the other hand, there is a more massive use of the depends function. Between V12 and V18, onchange : -9% ; depends : +360%

regards.

Ok, don't be harsh, I've just reread the comments and some people stil say (or never said the contrary) that this is a bug.

@rousseldenis : As mentioned in this long comment, Odoo SA developpers says it's a bug also ;-) (in fact Odoo prohibits mass edition on fields if there are onchange on it : see commit message and diff : odoo/odoo@14198a2#diff-4c25e9b1afc64d76fd27b33de1bc39a16b8b79fc254f21ab350e51b7deae9b02R40)

grindtildeath · 2024-12-18T19:22:43Z

I agree we should prefer compute to onchange but IMO this discussion is missing the point.

Basically, we went from "do not introduce this in a stable version" to "do not introduce that because I don't want it" ...

FWIW my patch is adding a bugfix (or a feature if you prefer) that is opt-in and won't change anything if someone does not want to use it. The only drawback being an added dependency on a module that is not in the same repository but is still in the OCA, to avoid repeating code.

But I guess I'm just a bad contributor wanting to waste the time of the good ones...

pedrobaeza · 2024-12-18T21:00:33Z

You are a good contributor, but stucked on imposing to some other good contributors something they don't want, like it's depending on other module. The solution for all is the extra module. Put if you want in the README of the base module that there's the glue module for the onchange case, but don't force people to install onchange_helper on the base module.

P.S.: I have never said to put it on the migration. I oppose to the dependency change in any version.

legalsylvain · 2024-12-18T21:53:41Z

The solution for all is the extra module. Put if you want in the README of the base module that there's the glue module for the onchange case, but don't force people to install onchange_helper on the base module.

Please Pedro. The main question is : "is it a mistake not to execute the onchange ?" If the answer is yes, propose to make an extra module, and write in the readme of the first module "There is a bug in this module, but you can fix it installing another module" doesn't makes sense.

For the time being,

there are more OCA contributors who think there's a mistake than who don't.
Odoo SA developers also think it's a mistake.

I asked a question 2 monthes ago ("Provide a use case where applying onchange would be dangerous?"). I need an answer to change my mind. (except speed of execution that depends on the number of items and because there may also be performance problems with api.depends)
When I explain in one sentence to my novice users what the mass editing wizard is for, I tell them that “it avoids the need to edit items one by one.”
When you edit one by one, the onchange is executed.

pedrobaeza · 2024-12-18T21:59:51Z

There are also a lot of persons not wanting to put this dependency. Why the people wanting to put it worth more than the one not wanting?

And more having a solution for all, which is the extra module.

trisdoan force-pushed the 18.0-mig-server_action_mass_edit branch from 29381ab to 0f93b99 Compare October 10, 2024 02:43

OCA-git-bot added this to the 18.0 milestone Oct 10, 2024

OCA-git-bot mentioned this pull request Oct 10, 2024

Migration to version 18.0 #948

Open

17 tasks

trisdoan mentioned this pull request Oct 10, 2024

[18.0][MIG] server_action_mass_edit: Migration to 18.0 camptocamp/server-ux#2

Closed

trisdoan force-pushed the 18.0-mig-server_action_mass_edit branch from 0f93b99 to 7399b98 Compare October 18, 2024 08:45

TDu approved these changes Oct 29, 2024

View reviewed changes

henrybackman approved these changes Oct 29, 2024

View reviewed changes

OCA-git-bot added the approved label Oct 29, 2024

florenciafrigieri2 approved these changes Oct 29, 2024

View reviewed changes

OCA-git-bot added the ready to merge label Oct 29, 2024

legalsylvain requested changes Oct 30, 2024

View reviewed changes

OCA-git-bot removed approved ready to merge labels Oct 30, 2024

trisdoan force-pushed the 18.0-mig-server_action_mass_edit branch 3 times, most recently from 87e31a4 to 6b8dca7 Compare November 29, 2024 05:11

trisdoan marked this pull request as draft November 29, 2024 05:15

trisdoan and others added 5 commits November 29, 2024 16:26

[FIX] base_technical_features: fix incorrect replacement

8b37531

[REF] rename 'mass_editing' module into 'server_action_mass_edit'

6102b99

[UPD] french translation

f2bbd7a

[IMP] server_action_mass_edit: Do not re-use demo and admin users to …

a164145

…avoid incompatibilities with data from those users.

Translated using Weblate (Turkish)

ac43fa4

Currently translated at 100.0% (43 of 43 strings) Translation: server-ux-16.0/server-ux-16.0-server_action_mass_edit Translate-URL: https://translation.odoo-community.org/projects/server-ux-16-0/server-ux-16-0-server_action_mass_edit/tr/

mymage and others added 7 commits December 9, 2024 11:56

Translated using Weblate (Italian)

b15f8b1

Currently translated at 100.0% (44 of 44 strings) Translation: server-ux-17.0/server-ux-17.0-server_action_mass_edit Translate-URL: https://translation.odoo-community.org/projects/server-ux-17-0/server-ux-17-0-server_action_mass_edit/it/

[FIX] server_action_mess_edit: Application error when discard a x2man…

c6b8454

…y field

[BOT] post-merge updates

e4beb3d

[IMP] Improve documentation during OCA Days 2024 Workshop thanks to @…

b5b7ec6

…TumbaoJu

[BOT] post-merge updates

efb20db

[IMP] server_action_mass_edit: pre-commit auto fixes

3e4468c

[MIG] server_action_mass_edit: Migration to 18.0

f88d10f

add pre-init-hook

trisdoan force-pushed the 18.0-mig-server_action_mass_edit branch from 6b8dca7 to f88d10f Compare December 9, 2024 04:58

trisdoan marked this pull request as ready for review December 9, 2024 04:58

PaulGoubert approved these changes Dec 17, 2024

View reviewed changes

etobella mentioned this pull request Dec 18, 2024

[16.0][FIX] mass_edit: Play onchanges before writing #963

Open

pedrobaeza mentioned this pull request Dec 18, 2024

Adding server_action_mass_edit v18 module #983

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[18.0][MIG] server_action_mass_edit: Migration to 18.0 #951

[18.0][MIG] server_action_mass_edit: Migration to 18.0 #951

trisdoan commented Oct 10, 2024 •

edited

Loading

pedrobaeza commented Oct 10, 2024

florenciafrigieri2 commented Oct 16, 2024

trisdoan commented Oct 18, 2024

TDu left a comment

henrybackman left a comment

henrybackman Oct 29, 2024

henrybackman Oct 29, 2024

henrybackman Oct 29, 2024

florenciafrigieri2 left a comment

OCA-git-bot commented Oct 29, 2024

stefsava commented Oct 29, 2024

legalsylvain left a comment

trisdoan commented Nov 7, 2024 •

edited

Loading

stefsava commented Nov 7, 2024

trisdoan commented Nov 29, 2024 •

edited

Loading

trisdoan commented Dec 9, 2024 •

edited

Loading

PaulGoubert left a comment

grindtildeath commented Dec 17, 2024

pedrobaeza commented Dec 17, 2024

legalsylvain commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

etobella commented Dec 18, 2024

rousseldenis commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

etobella commented Dec 18, 2024

rousseldenis commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

rousseldenis commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

legalsylvain commented Dec 18, 2024

grindtildeath commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

legalsylvain commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

		def test_wiz_fields_view_get(self):
		"""Test whether fields_view_get method returns arch.

	"""Without this call, dynamic fields build by fields_view_get()
	"""Without this call, dynamic fields build by get_view()

[18.0][MIG] server_action_mass_edit: Migration to 18.0 #951

Are you sure you want to change the base?

[18.0][MIG] server_action_mass_edit: Migration to 18.0 #951

Conversation

trisdoan commented Oct 10, 2024 • edited Loading

Note

pedrobaeza commented Oct 10, 2024

florenciafrigieri2 commented Oct 16, 2024

trisdoan commented Oct 18, 2024

TDu left a comment

Choose a reason for hiding this comment

henrybackman left a comment

Choose a reason for hiding this comment

henrybackman Oct 29, 2024

Choose a reason for hiding this comment

henrybackman Oct 29, 2024

Choose a reason for hiding this comment

henrybackman Oct 29, 2024

Choose a reason for hiding this comment

florenciafrigieri2 left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Oct 29, 2024

stefsava commented Oct 29, 2024

legalsylvain left a comment

Choose a reason for hiding this comment

trisdoan commented Nov 7, 2024 • edited Loading

stefsava commented Nov 7, 2024

trisdoan commented Nov 29, 2024 • edited Loading

trisdoan commented Dec 9, 2024 • edited Loading

PaulGoubert left a comment

Choose a reason for hiding this comment

grindtildeath commented Dec 17, 2024

pedrobaeza commented Dec 17, 2024

legalsylvain commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

etobella commented Dec 18, 2024

rousseldenis commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

etobella commented Dec 18, 2024

rousseldenis commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

rousseldenis commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

legalsylvain commented Dec 18, 2024

grindtildeath commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

legalsylvain commented Dec 18, 2024

pedrobaeza commented Dec 18, 2024

trisdoan commented Oct 10, 2024 •

edited

Loading

trisdoan commented Nov 7, 2024 •

edited

Loading

trisdoan commented Nov 29, 2024 •

edited

Loading

trisdoan commented Dec 9, 2024 •

edited

Loading