Bug 33744 - Plugins not working on duplicated MARC fields
Summary: Plugins not working on duplicated MARC fields
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Matt Blenkinsop
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-05-16 11:12 UTC by Matt Blenkinsop
Modified: 2023-12-28 20:47 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.04,22.11.10


Attachments
Bug 33744: Fix deprecated use of .data('events') (1.42 KB, patch)
2023-05-16 11:17 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 33744: Fix deprecated use of .data('events') (1.42 KB, patch)
2023-07-17 10:56 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 33744: Fix deprecated use of .data('events') (1.45 KB, patch)
2023-08-25 10:26 UTC, David Nind
Details | Diff | Splinter Review
Bug 33744: Fix cloning (sub)fields with framework plugins (1.68 KB, patch)
2023-09-01 09:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Blenkinsop 2023-05-16 11:12:35 UTC
AddEventHandlers in cataloging.js uses .data('events') which is now deprecated. This means that a duplicated field with the upload.pl plugin doesn't function as expected as the event handler doesn't pass to the duplicated field.

To reproduce:
1) In Administration > MARC frameworks, choose a framework and through the actions button click on MARC structure
2) Search for field 856
3) Click on edit subfields
4) Select subfield "u" using the tabs at the top
5) At the bottom, in the dropdown menu called "Plugin", select "upload.pl" (bottom of the list) and save
6) Navigate to Cataloging and create a new record using the framework you have just edited
7) Click on tab 8 and check you have field 856. Click on the Upload button and the pop up should open as expected allowing you to upload a file
8) Click on the "Repeat this tag" button for the 856 tag
9) Click on the Upload button and nothing will happen
10) Delete the cloned tag
11) On the original tag, click the "Repeat this subfield" button next to the "Upload" button.
12) Click the "Upload" button on the new subfield and the same issue will occur.
Comment 1 Marcel de Rooy 2023-05-16 11:14:20 UTC
Looks like this is addressed under bug 30975.
Comment 2 Marcel de Rooy 2023-05-16 11:14:37 UTC

*** This bug has been marked as a duplicate of bug 30975 ***
Comment 3 Matt Blenkinsop 2023-05-16 11:17:13 UTC
Created attachment 151270 [details] [review]
Bug 33744: Fix deprecated use of .data('events')

This patch removes a deprecated internal data structure no longer supported in jQuery. This means that AddEventHandlers now correctly passes events to cloned fields in the MARC record

Test plan:
1) Apply patch
2) Repeat steps in the original bug description
3) This time clicking on the Upload button for a cloned field should launch the uploads pop up.
Comment 4 Marcel de Rooy 2023-05-16 11:53:41 UTC
> This patch removes a deprecated internal data structure no longer supported in jQuery.

 // Note: This code depends on the jQuery data for events; this structure
 // is moved to _data as of jQuery 1.8.
-    var ev= $(oldcontrol).data('events');
+    var ev = $._data(oldcontrol, "events");

You are using an internal structure here. Where we really dont know how long it will be supported.
Comment 5 Marcel de Rooy 2023-05-16 11:54:52 UTC
Perhaps we can use it as a temporary workaround until 30975 gets in ?
Comment 6 Matt Blenkinsop 2023-07-17 10:55:26 UTC
30975 is still blocked so maybe Marcel is right and we should use this patch as a temporary fix - I'll rebase and re-submit
Comment 7 Matt Blenkinsop 2023-07-17 10:56:49 UTC
Created attachment 153538 [details] [review]
Bug 33744: Fix deprecated use of .data('events')

This patch removes a deprecated internal data structure no longer supported in jQuery. This means that AddEventHandlers now correctly passes events to cloned fields in the MARC record

Test plan:
1) Apply patch
2) Repeat steps in the original bug description
3) This time clicking on the Upload button for a cloned field should launch the uploads pop up.
Comment 8 David Nind 2023-08-25 10:26:27 UTC
Created attachment 154796 [details] [review]
Bug 33744: Fix deprecated use of .data('events')

This patch removes a deprecated internal data structure no longer supported in jQuery. This means that AddEventHandlers now correctly passes events to cloned fields in the MARC record

Test plan:
1) Apply patch
2) Repeat steps in the original bug description
3) This time clicking on the Upload button for a cloned field should launch the uploads pop up.

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2023-08-25 10:29:43 UTC
I've added some comments to bug 30975 (comment 45) on whether to go ahead with 30975 as is, or go ahead with this one as a temporary measure.

From the user perspective, maybe getting cloning of subfields with plugins/value builders should be the priority?
Comment 10 Marcel de Rooy 2023-09-01 09:11:21 UTC
Looking here
Comment 11 Marcel de Rooy 2023-09-01 09:28:20 UTC
Created attachment 155081 [details] [review]
Bug 33744: Fix cloning (sub)fields with framework plugins

This patch is meant as a temporary measure for fixing the problem.
Bug 30975 may resolve it further, but this is good for now.

Note: The original commit message referred to removing a deprecated internal
structure. But this patch actually still uses the structure which has been
moved to _data in jQuery now.

Test plan:
1) Apply patch
2) Repeat steps in the original bug description
3) This time clicking on the Upload button for a cloned field should launch the uploads pop up.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2023-09-01 09:29:14 UTC
(In reply to David Nind from comment #8)
> This patch removes a deprecated internal data structure no longer supported
> in jQuery. This means that AddEventHandlers now correctly passes events to
> cloned fields in the MARC record

Took the liberty to further clarify this statement in the commit message.
Comment 13 Tomás Cohen Arazi 2023-09-01 14:10:11 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 14 Fridolin Somers 2023-09-08 00:45:14 UTC
Pushed to 23.05.x for 23.05.04
Comment 15 Matt Blenkinsop 2023-09-08 09:47:50 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x