|
Description
Jon Knight
2017-08-22 15:43:20 UTC
Created attachment 66351 [details] [review] First cut at some changes to misc/stage_file.pl script to provide for MARC modification template support. Not ready for QA yet. Woot! I like using the name as it's easier to remember and harder to confuse. Also easier to set it up the same on multiple installations. Maybe we could just add a warning when a non-unique template name is used? Yes, I'm just doing the same thing in bug 18690 now. If folk like what I do there, I'll pop the same code in here too. Ah, just realised I was referencing the wrong Bug ID for the other bug - is actually 18389. Too many browser tabs open! Created attachment 68239 [details] Test MARC record This is a test MARC record (shamelessly stolen from Bug 19069) that can be used to test that MARC modification templates are applied using the batch misc/stage_file.pl script. Created attachment 68240 [details] [review] Bug 19164 - Allow MARC modification templates to be used in staged MARC imports Patch checkes for duplicate MARC modificiation templates and if only one is found that matches the name given on the command line, submits the batch staging of the input files using that template. Test plan: Test plan: 1) In the staff web client got to "Home › Tools › MARC modification templates" and create a MARC Modification Template called "Test Bug 19164". Set the action to be "Copy field 999$l to 998$l using RegEx s/NASH/FLASH/ if 999$l matches NASH" 2) Download the test MARC record from this bug into a file called marc1.mrc into your koha dev user's home directory. 3) Run misc/stage_file.pl with the command line: misc/stage_file.pl --file ~/marc1.mrc --add-items --marcmodtemplate "Test Bug 19164" --item-action replace --comment "testing MARC mod templates on batch staging" 4) In the staff client, go to Home › Tools › Manage staged MARC records and check that the file is visible and in status "Staged" 5) Click on the filename to bring up details of the staged batch, and then click on "Basic Christianity. Stott, John R. W. (0802811892)". Note that there is a 998$l field with the value "FLASH" that has been created using the MARC Modification Template. Applying: First cut at some changes to misc/stage_file.pl script to provide for MARC modification template support. Not ready for QA yet. Using index info to reconstruct a base tree... M misc/stage_file.pl Falling back to patching base and 3-way merge... Auto-merging misc/stage_file.pl CONFLICT (content): Merge conflict in misc/stage_file.pl Failed to merge in the changes. Patch failed at 0001 First cut at some changes to misc/stage_file.pl script to provide for MARC modification template support. Not ready for QA yet. The copy of the patch that failed is found in: /home/vagrant/kohaclone/.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/First-cut-at-some-changes-to-miscstagefilepl-scrip-x5L2A_.patch Created attachment 68249 [details] [review] Patch for misc/stage_file.pl script to provide for MARC modification template support. https://bugs.koha-community.org/show_bug.cgi?id=19164 Bug 19164 - Allow MARC modification templates to be used in staged MARC imports Patch checks for duplicate MARC modificiation templates and if only one is found that matches the name given on the command line, submits the batch staging of the input files using that template. Test plan: 1) In the staff web client got to "Home › Tools › MARC modification templates" and create a MARC Modification Template called "Test Bug 19164". Set the action to be "Copy field 999$l to 998$l using RegEx s/NASH/FLASH/ if 999$l matches NASH" 2) Download the test MARC record from this bug into a file called marc1.mrc into your koha dev user's home directory. 3) Run misc/stage_file.pl with the command line: misc/stage_file.pl --file ~/marc1.mrc --add-items --marcmodtemplate "Test Bug 19164" --item-action replace --comment "testing MARC mod templates on batch staging" 4) In the staff client, go to Home › Tools › Manage staged MARC records and check that the file is visible and in status "Staged" 5) Click on the filename to bring up details of the staged batch, and then click on "Basic Christianity. Stott, John R. W. (0802811892)". Note that there is a 998$l field with the value "FLASH" that has been created using the MARC Modification Template. Rebased the pair of patches (something must have changed in master since I did the first one that was already in my branch) into a single new patch and obsoleted the two original patches. Now applies cleanly from scratch in my kohadevbox using git bz apply. Created attachment 68341 [details]
Record not visible in staged file after running misc/stage-file.pl
Hi Jon
After running the misc/stage-file.pl with the parameters as set in your test plan I have found that instead of displaying the "Basic Christianity. Stott, John R. W. (0802811892)" record a record link 'null' is displayed (see screenshot) and it only contains MARC fields: 000, 001 003, 005 and 008
I copied the contents of your marc file across into my kohadevbox and pasted it into new file with vi and made sure the formatting matched your marc file so I am assuming that this issue may be in the misc/stage-file.pl script.
Comment on attachment 68249 [details] [review] Patch for misc/stage_file.pl script to provide for MARC modification template support. Review of attachment 68249 [details] [review]: ----------------------------------------------------------------- Hi Jon, failing QA because of Alex's comment - can you please take a look? Also spotted a tiny something, see below. ::: misc/stage_file.pl @@ +19,5 @@ > # with this program; if not, write to the Free Software Foundation, Inc., > # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > > +#use strict; > +#use warnings; You can delete the commented lines when using Modern:Perl; (In reply to Alex Buckley from comment #12) > Created attachment 68341 [details] > Record not visible in staged file after running misc/stage-file.pl > > Hi Jon > > After running the misc/stage-file.pl with the parameters as set in your test > plan I have found that instead of displaying the "Basic Christianity. Stott, > John R. W. (0802811892)" record a record link 'null' is displayed (see > screenshot) and it only contains MARC fields: 000, 001 003, 005 and 008 > > I copied the contents of your marc file across into my kohadevbox and pasted > it into new file with vi and made sure the formatting matched your marc file > so I am assuming that this issue may be in the misc/stage-file.pl script. Hmm, I've just tried on a fresh branch on an updated kohadevbox and I can't reproduce this. Could you add your copy of the marc file to this case so that I can see if vi has done anything odd to it? When I transferred it on my kohadevbox I just used vagrant's NFS share, so it never went near a text editor. Created attachment 68425 [details] [review] Bug_19164: remove use strict/warn lines (were commented out anyway) Hello, just for information, we apply it on the top of a koha 16.11.12 and test it with our own marc file and our own model. We launch this command : stage_file.pl --encoding UTF-8 --file export_201710031044.mrc --add-items --marcmodtemplate "test" It works as expected, the modification model is apllied but there's several warnings : ERROR occurred in SetUTF8Flag Field 020 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. ERROR occurred in SetUTF8Flag Field 245 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. ERROR occurred in SetUTF8Flag Field 020 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. ERROR occurred in SetUTF8Flag Field 245 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. ERROR occurred in SetUTF8Flag Field 020 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. ERROR occurred in SetUTF8Flag Field 245 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. Use of uninitialized value in join or string at /usr/share/perl5/MARC/Field.pm line 696. ERROR occurred in SetUTF8Flag Field 020 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. ERROR occurred in SetUTF8Flag Field 245 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. Use of uninitialized value in join or string at /usr/share/perl5/MARC/Field.pm line 696. ERROR occurred in SetUTF8Flag Field 020 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. ERROR occurred in SetUTF8Flag Field 245 must have at least one subfield at /home/koha/src/C4/Charset.pm line 145. OK, I'll take a look at this in a day or so. Assigned back to me. I don't suppose you could upload a copy of "export_201710031044.mrc" so that I can make sure that I'm playing with the same file. Also are you using MARC 21 or UNIMARC? Hello, yes I will upload the file. We work in Unimarc but the file is in marc21. Actually, we want to use template for changing key fields from marc21 fields in unimarc fields. It's only temporary files for acquisitions. I think you're on something.. the warnings are perhaps only because our installation is in unimarc.. Created attachment 70033 [details]
marc_file
Hello, if you have time to work on it before Marseille hackfest (begins on 12th March), I'm ok to test it on a master during the hackfest. Sonia Been doing some more playing with this. On my kohadevbox, applying this patch to the latest master, making a MARC modification rule of: Copy field 960$q to 998$q and then applying it to the export_201710031044.mrc file doesn't give me any errors on the command line, and appears to copy the 960$q to 998$q in the staged records as expected. Then I tried going to the 16.11.x branch in git and applying the patch on top of that. The patches apply OK, but the import fails (test1.mrc is my copy of the uploaded MARC file): vagrant@kohadevbox:kohaclone(16.11.x)$ git checkout -b bug_19164_16_11 Switched to a new branch 'bug_19164_16_11' vagrant@kohadevbox:kohaclone(bug_19164_16_11)$ git bz apply 19164 Bug 19164 - Allow MARC modification templates to be used in staged MARC imports 68249 - Patch for misc/stage_file.pl script to provide for MARC modification template support. 68425 - Bug_19164: remove use strict/warn lines (were commented out anyway) Apply? [(y)es, (n)o, (i)nteractive] y Applying: Patch for misc/stage_file.pl script to provide for MARC modification template support. Applying: Bug_19164: remove use strict/warn lines (were commented out anyway) vagrant@kohadevbox:kohaclone(bug_19164_16_11)$ misc/stage_file.pl --file ~/test.mrc --add-items --marcmodtemplate "Test Bug 19164" --item-action replace --comment "testing MARC mod templates on batch staging" ... staging MARC records -- please wait Not a HASH reference at /home/vagrant/kohaclone/C4/Biblio.pm line 2786. Hmm, so it looks like something is broken for using this patch with the latest 16.11.x. The question now is, as this is an enhancement (ie adding a new feature to Koha), do we worry about that too much if its working in the master train? Hello, I have tried on a 16.11.16 and it works. I'm going to try it on a master Great thank you - I'll change the status to needs sign off in that case! Created attachment 84167 [details] [review] Bug_19164: remove use strict/warn lines (were commented out anyway) Signed-off-by: Mackey Johnstone <mackeyfj@gmail.com> Comment on attachment 68249 [details] [review] Patch for misc/stage_file.pl script to provide for MARC modification template support. Review of attachment 68249 [details] [review]: ----------------------------------------------------------------- I don't believe this patch as a whole is signed off yet, but I've done an initial QA run on it anyway. Please see the code comments. ::: misc/stage_file.pl @@ +74,5 @@ > + if($this_template->{'name'} eq $marc_mod_template) { > + $marc_mod_template_id = $this_template->{'template_id'}; > + last; > + } > + if($this_template->{'name'} eq $marc_mod_template) { This block can never be reached as the code stands... @@ +149,5 @@ > + BatchStageMarcRecords( > + $record_type, $params->{encoding}, > + $marc_records, $params->{input_file}, > + $params->{'marc_mod_template_id'}, > + $params->{batch_comment}, '', Minor whitespace error introductions in the above four lines. Created attachment 84982 [details] [review] Bug 19164: Add MARC modification template support to misc/stage-file.pl. https://bugs.koha-community.org/show_bug.cgi?id=19164 Bug 19164 - Allow MARC modification templates to be used in staged MARC imports Patch checks for duplicate MARC modificiation templates and if only one is found that matches the name given on the command line, submits the batch staging of the input files using that template. Test plan: 1) In the staff web client got to "Home › Tools › MARC modification templates" and create a MARC Modification Template called "Test Bug 19164". Set the action to be "Copy field 999$l to 998$l using RegEx s/NASH/FLASH/ if 999$l matches NASH" 2) Download the test MARC record from this bug into a file called marc1.mrc into your koha dev user's home directory. 3) Run misc/stage_file.pl with the command line: misc/stage_file.pl --file ~/marc1.mrc --add-items --marcmodtemplate "Test Bug 19164" --item-action replace --comment "testing MARC mod templates on batch staging" 4) In the staff client, go to Home › Tools › Manage staged MARC records and check that the file is visible and in status "Staged" 5) Click on the filename to bring up details of the staged batch, and then click on "Basic Christianity. Stott, John R. W. (0802811892)". Note that there is a 998$l field with the value "FLASH" that has been created using the MARC Modification Template. I've rebased the patches so that the changes are all in one patch file now, and fixed the unreachable code. Run on my kohadevbox against master and it seems to work OK with the test marc record I uploaded originally. I also ran `qa -c 1 -v 2` on the new commit and it seems to pass the tests there. Created attachment 84987 [details] [review] Bug 19164: Add MARC modification template support to misc/stage-file.pl. https://bugs.koha-community.org/show_bug.cgi?id=19164 Bug 19164 - Allow MARC modification templates to be used in staged MARC imports Patch checks for duplicate MARC modificiation templates and if only one is found that matches the name given on the command line, submits the batch staging of the input files using that template. Test plan: 1) In the staff web client got to "Home › Tools › MARC modification templates" and create a MARC Modification Template called "Test Bug 19164". Set the action to be "Copy field 999$l to 998$l using RegEx s/NASH/FLASH/ if 999$l matches NASH" 2) Download the test MARC record from this bug into a file called marc1.mrc into your koha dev user's home directory. 3) Run misc/stage_file.pl with the command line: misc/stage_file.pl --file ~/marc1.mrc --add-items --marcmodtemplate "Test Bug 19164" --item-action replace --comment "testing MARC mod templates on batch staging" 4) In the staff client, go to Home › Tools › Manage staged MARC records and check that the file is visible and in status "Staged" 5) Click on the filename to bring up details of the staged batch, and then click on "Basic Christianity. Stott, John R. W. (0802811892)". Note that there is a 998$l field with the value "FLASH" that has been created using the MARC Modification Template. Signed-off-by: Mackey Johnstone <mackeyfj@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 84988 [details] [review] Bug 19164: (QA follow-up) Minor whitespace cleanup I've taken Mackey Johnstone as a signoff and added the line appropriately so I can move on to QA. Works as expected, a nice simple but very helpful patch. Thanks for bearing with it so long Jon. Passing QA Awesome work all! Pushed to master for 19.05 Enhancement, will not be backported to 18.11.x series. Are there any plans to add this functionality to the web interface? (In reply to Simon Hohl from comment #35) > Are there any plans to add this functionality to the web interface? Not by me. |