Bug 15869 - Change framework on overlay
Summary: Change framework on overlay
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 5 votes (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-19 19:44 UTC by Nicole C. Engard
Modified: 2022-11-02 19:34 UTC (History)
10 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:


Attachments
Bug 15869 - change framework on overlay (4.31 KB, patch)
2016-09-13 15:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15869 - change framework on overlay (3.93 KB, patch)
2018-01-22 16:02 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15869: Change framework on overlay (8.12 KB, patch)
2018-07-16 17:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15869: Change framework on overlay (8.96 KB, patch)
2022-09-22 19:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15869: Unit test (2.64 KB, patch)
2022-09-22 19:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15869: Change framework on overlay (9.02 KB, patch)
2022-10-20 14:46 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 15869: Unit test (2.70 KB, patch)
2022-10-20 14:46 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 15869: (follow-up) Don't mock undef return value (1.00 KB, patch)
2022-11-02 19:34 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2016-02-19 19:44:09 UTC
When you stage files for import you can choose how to handle matches.  One option is to replace the existing record.  The problem is that many libraries use this option to load a fuller record over a quick or fast record and the framework doesn't change.  So you can choose the framework before importing but that framework only applies to new records, it doesn't update records that are being replaced or overlaid.  We should fix that to update the framework for all items, not just new ones.

Nicole
Comment 1 Nick Clemens 2016-09-13 15:28:35 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2016-09-13 15:29:47 UTC
Needs unit tests - wanted comments on approach first.
Comment 3 Katrin Fischer 2017-10-07 22:05:08 UTC Comment hidden (obsolete)
Comment 4 Nick Clemens 2018-01-22 16:02:16 UTC Comment hidden (obsolete)
Comment 5 Mark Tompsett 2018-01-31 02:13:47 UTC
I don't think this is Academy, because line 567 of C4/ImportBatch.pm in master is probably the wrong condition. I think it should be >= 1, not ==1. Read lines 251/252 of tools/manage-marc-import.pl where 4 parameters are passed. This means adding a parameter in the middle is messed up. If the code is cleaned up, then perhaps, but I'm marking this Failed QA, because it reads awful and probably explains why imports are flaky sometimes. Also, it fails to meet the "must have tests" coding guidelines.
Comment 6 emjhet 2018-05-30 20:13:14 UTC
This is an issue with overlaying any record, not just staged records. 

I should be able to select that all Imported records are imported in a specific Framework, whether imported singularly or as a batch, and whether overlaid onto a brief order record or as the first bibliographic content. 

Indeed, libraries should be able to select any of their Frameworks as THEIR default Framework, instead of having to use the Koha Default.
Comment 7 Nick Clemens 2018-07-16 17:22:23 UTC
Created attachment 77018 [details] [review]
Bug 15869: Change framework on overlay

This patch allows for selection of framework to use when overlaying
records - by default it is set to keep the initial framework

To test:
 1 - Create some records using one framework
 2 - Export the records
 3 - Edit the records to add fields not in original framework
 4 - Stage records using a rule that will find matches
 5 - Import
 6 - Note records contain new fields on display, but they are lost on edit
 7 - Apply patch
 8 - Stage records again
 9 - Select a framework that contains the new fields on import
10 - Import records
11 - Note records now use selected framework and are displayed/edited
correctly
Comment 8 Nick Clemens 2018-07-16 17:24:38 UTC
Patches updated just to see.

I think the condition was right (it is checking if there are two variables left in the parameters) but agree it is ugly - changed to take a hash

It still needs unit tests, feel free to help :-)
Comment 9 Andrew Fuerste-Henry 2021-09-08 20:21:46 UTC
+1, this would be handy!
Comment 10 Nick Clemens 2022-09-22 19:15:57 UTC
Created attachment 140857 [details] [review]
Bug 15869: Change framework on overlay

This patch allows for selection of framework to use when overlaying
records - by default it is set to keep the initial framework

To test:
 1 - Create some records using one framework
 2 - Export the records
 3 - Edit the records to add fields not in original framework
 4 - Stage records using a rule that will find matches
 5 - Import
 6 - Note records contain new fields on display, but they are lost on edit
 7 - Apply patch
 8 - Stage records again
 9 - Select a framework that contains the new fields on import
10 - Import records
11 - Note records now use selected framework and are displayed/edited
correctly
Comment 11 Nick Clemens 2022-09-22 19:16:02 UTC
Created attachment 140858 [details] [review]
Bug 15869: Unit test
Comment 12 ByWater Sandboxes 2022-10-20 14:46:51 UTC
Created attachment 142257 [details] [review]
Bug 15869: Change framework on overlay

This patch allows for selection of framework to use when overlaying
records - by default it is set to keep the initial framework

To test:
 1 - Create some records using one framework
 2 - Export the records
 3 - Edit the records to add fields not in original framework
 4 - Stage records using a rule that will find matches
 5 - Import
 6 - Note records contain new fields on display, but they are lost on edit
 7 - Apply patch
 8 - Stage records again
 9 - Select a framework that contains the new fields on import
10 - Import records
11 - Note records now use selected framework and are displayed/edited
correctly

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 13 ByWater Sandboxes 2022-10-20 14:46:57 UTC
Created attachment 142258 [details] [review]
Bug 15869: Unit test

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 14 Katrin Fischer 2022-10-28 19:33:49 UTC
There is a new warning in the output of the unit test with this patch set applied:

t/db_dependent/ImportBatch.t .. 9/19 Use of uninitialized value $item_result in string eq at /kohadevbox/koha/C4/ImportBatch.pm line 672.
Use of uninitialized value $item_result in string eq at /kohadevbox/koha/C4/ImportBatch.pm line 672.

Can you please check?
Comment 15 Nick Clemens 2022-11-02 19:34:08 UTC
Created attachment 143033 [details] [review]
Bug 15869: (follow-up) Don't mock undef return value