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
Created attachment 55543 [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
Needs unit tests - wanted comments on approach first.
Sorry, patch no longer applies. Please rebase: 55543 - Bug 15869 - change framework on overlay Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 15869 - change framework on overlay .git/rebase-apply/patch:17: trailing whitespace. # optional callback to monitor status warning: 1 line adds whitespace errors. Using index info to reconstruct a base tree... M C4/ImportBatch.pm M koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt M tools/manage-marc-import.pl .git/rebase-apply/patch:17: trailing whitespace. # optional callback to monitor status warning: 1 line adds whitespace errors. Falling back to patching base and 3-way merge... Auto-merging tools/manage-marc-import.pl CONFLICT (content): Merge conflict in tools/manage-marc-import.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt Auto-merging C4/ImportBatch.pm CONFLICT (content): Merge conflict in C4/ImportBatch.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 15869 - change framework on overlay The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-15869---change-framework-on-overlay-K6lG8D.patch
Created attachment 70800 [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
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.
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.
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
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 :-)
+1, this would be handy!
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
Created attachment 140858 [details] [review] Bug 15869: Unit test
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>
Created attachment 142258 [details] [review] Bug 15869: Unit test Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
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?
Created attachment 143033 [details] [review] Bug 15869: (follow-up) Don't mock undef return value
Created attachment 145271 [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> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 145272 [details] [review] Bug 15869: Unit test Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 145273 [details] [review] Bug 15869: (follow-up) Don't mock undef return value Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 145274 [details] [review] Bug 15869: (QA follow-up) Add some padding around the pulldowns Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 145406 [details] [review] Bug 15869: (QA follow-up) Don't pass placeholder framework '_USE_ORIG' This param in the dropdown is meant to signify no change, distinct from "" when means to set to default. In rebasing this condition was lost, this patch restores undef of the variable to avoid setting framework to an invalid value To test: 1 - Stage a record with a match (export and import a record) 2 - Attempt to import and overlay with option 'When replacing records use this framework:' 'Keep original framework' 3 - It fails, logs note 'value too long for field frameworkcode' 4 - Apply this patch 5 - Try again, it succeeds - framework is not changed
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work, thanks everyone! Pushed to 22.11.x for the next release.
Doesn't apply cleanly to 22.05.x, no backport. If needed please rebase.
*** Bug 24843 has been marked as a duplicate of this bug. ***
I'm pretty sure that $overlay_framework = undef if $overlay_framework = '_USE_ORIG'; needs to be $overlay_framework = undef if $overlay_framework == '_USE_ORIG'; The import fails until this is changed.
(In reply to David Roberts from comment #25) > I'm pretty sure that > > $overlay_framework = undef if $overlay_framework = '_USE_ORIG'; > > needs to be > > $overlay_framework = undef if $overlay_framework == '_USE_ORIG'; > > The import fails until this is changed. I believe this was fixed on another bug report.