Bug 26894 - Marc Modification Templates treat subfield 0 as no subfield set when moving fields
Summary: Marc Modification Templates treat subfield 0 as no subfield set when moving f...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
Depends on: 17510 26781
Blocks:
  Show dependency treegraph
 
Reported: 2020-11-02 11:26 UTC by Nick Clemens (kidclamp)
Modified: 2021-12-13 21:09 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.02,20.05.08
Circulation function:


Attachments
Bug 26894: Handle subfield 0 and delete empty fields (4.31 KB, patch)
2020-11-02 12:11 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 26894: Handle subfield 0 and delete empty fields (4.36 KB, patch)
2020-12-11 14:05 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 26894: Handle subfield 0 and delete empty fields (4.42 KB, patch)
2021-01-12 12:11 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2020-11-02 11:26:58 UTC
Similar to bug 26781 when moving subfield 0 we move the whole record. 

The tests from 17510 actually cover this, but I suspect it is still not the desired/expected behavior
Comment 1 Nick Clemens (kidclamp) 2020-11-02 12:11:36 UTC
Created attachment 112827 [details] [review]
Bug 26894: Handle subfield 0 and delete empty fields

This patch corrects handling of subfield 0 when copying/moving. Before these
patches copying from subfield 0 would copy the entire field.

We also correct an error that if moving a single subfield, we can leave behind empty
fields with no subfields. They should be deleted.

To test:
1 - Define a new MARC Modification template with actions:
        Delete field 100$0
        Add new field 100$0 with value Test
        Update existing or add new field 100$0 with value TestUpdated
        Move field 100$0 to 600$0
        Copy field 600$0 to 100$0
        Copy and replace field 245$0 to 700$0
2 - Define a new record like:
    LDR 00334nam a22001217a 4500
    003 ff
    005 20201102111604.0
    008 201102b        xxu||||| |||| 00| 0 eng d
    040 _ _ ‡cvsd
    100 1 _ ‡012345‡aKnuth, Donal Ervin‡d1938
    245 _ _ ‡012345‡aThe aty of computer programming‡cDonald E. Knuth
    650 _ 0 ‡042‡aComputer programming‡9462
3 - Modify this record using the template above
4 - Note that entier 245 is copied to 700, same for 600 field
5 - Apply patch
6 - Now only subfields 0 are copied
Comment 2 Fridolin Somers 2020-12-11 14:05:57 UTC
Created attachment 114343 [details] [review]
Bug 26894: Handle subfield 0 and delete empty fields

This patch corrects handling of subfield 0 when copying/moving. Before these
patches copying from subfield 0 would copy the entire field.

We also correct an error that if moving a single subfield, we can leave behind empty
fields with no subfields. They should be deleted.

To test:
1 - Define a new MARC Modification template with actions:
        Delete field 100$0
        Add new field 100$0 with value Test
        Update existing or add new field 100$0 with value TestUpdated
        Move field 100$0 to 600$0
        Copy field 600$0 to 100$0
        Copy and replace field 245$0 to 700$0
2 - Define a new record like:
    LDR 00334nam a22001217a 4500
    003 ff
    005 20201102111604.0
    008 201102b        xxu||||| |||| 00| 0 eng d
    040 _ _ ‡cvsd
    100 1 _ ‡012345‡aKnuth, Donal Ervin‡d1938
    245 _ _ ‡012345‡aThe aty of computer programming‡cDonald E. Knuth
    650 _ 0 ‡042‡aComputer programming‡9462
3 - Modify this record using the template above
4 - Note that entier 245 is copied to 700, same for 600 field
5 - Apply patch
6 - Now only subfields 0 are copied

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 3 Fridolin Somers 2020-12-11 14:11:37 UTC
Tested well.

Note that I was running manually misc/background_jobs_worker.pl, that must be restarted after applying patches.
Comment 4 Martin Renvoize (ashimema) 2021-01-12 12:11:08 UTC
Created attachment 115076 [details] [review]
Bug 26894: Handle subfield 0 and delete empty fields

This patch corrects handling of subfield 0 when copying/moving. Before these
patches copying from subfield 0 would copy the entire field.

We also correct an error that if moving a single subfield, we can leave behind empty
fields with no subfields. They should be deleted.

To test:
1 - Define a new MARC Modification template with actions:
        Delete field 100$0
        Add new field 100$0 with value Test
        Update existing or add new field 100$0 with value TestUpdated
        Move field 100$0 to 600$0
        Copy field 600$0 to 100$0
        Copy and replace field 245$0 to 700$0
2 - Define a new record like:
    LDR 00334nam a22001217a 4500
    003 ff
    005 20201102111604.0
    008 201102b        xxu||||| |||| 00| 0 eng d
    040 _ _ ‡cvsd
    100 1 _ ‡012345‡aKnuth, Donal Ervin‡d1938
    245 _ _ ‡012345‡aThe aty of computer programming‡cDonald E. Knuth
    650 _ 0 ‡042‡aComputer programming‡9462
3 - Modify this record using the template above
4 - Note that entier 245 is copied to 700, same for 600 field
5 - Apply patch
6 - Now only subfields 0 are copied

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize (ashimema) 2021-01-12 12:11:23 UTC
Good catch, Passing QA
Comment 6 Jonathan Druart 2021-01-12 15:48:08 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 7 Fridolin Somers 2021-01-18 14:31:24 UTC
Pushed to 20.11.x for 20.11.02
Comment 8 Andrew Fuerste-Henry 2021-01-22 16:52:58 UTC
Pushed to 20.05.x for 20.05.08
Comment 9 Victor Grousset/tuxayo 2021-01-22 21:12:43 UTC
Missing dependencies for 19.11.x, it shouldn't be affected, no backport.