Bug 19164 - Allow MARC modification templates to be used in staged MARC imports
Summary: Allow MARC modification templates to be used in staged MARC imports
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Bibliographic record staging/import (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jon Knight
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-22 15:43 UTC by Jon Knight
Modified: 2019-05-30 11:59 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
This enhancement allows applying MARC modification templates to batches of records during the import/staging process from the staff client
Version(s) released in:


Attachments
First cut at some changes to misc/stage_file.pl script to provide for MARC modification template support. Not ready for QA yet. (3.44 KB, patch)
2017-08-22 15:58 UTC, Jon Knight
Details | Diff | Splinter Review
Test MARC record (1.10 KB, application/octet-stream)
2017-10-18 10:19 UTC, Jon Knight
Details
Bug 19164 - Allow MARC modification templates to be used in staged MARC imports (2.51 KB, patch)
2017-10-18 10:24 UTC, Jon Knight
Details | Diff | Splinter Review
Patch for misc/stage_file.pl script to provide for MARC modification template support. (5.52 KB, patch)
2017-10-18 13:34 UTC, Jon Knight
Details | Diff | Splinter Review
Record not visible in staged file after running misc/stage-file.pl (46.18 KB, image/png)
2017-10-21 03:51 UTC, Alex Buckley
Details
Bug_19164: remove use strict/warn lines (were commented out anyway) (692 bytes, patch)
2017-10-23 16:31 UTC, Jon Knight
Details | Diff | Splinter Review
marc_file (9.75 KB, text/plain)
2017-12-21 14:40 UTC, Koha Team Lyon 3
Details
Bug_19164: remove use strict/warn lines (were commented out anyway) (752 bytes, patch)
2019-01-17 22:15 UTC, Mackey Johnstone
Details | Diff | Splinter Review
Bug 19164: Add MARC modification template support to misc/stage-file.pl. (5.45 KB, patch)
2019-02-11 16:05 UTC, Jon Knight
Details | Diff | Splinter Review
Bug 19164: Add MARC modification template support to misc/stage-file.pl. (5.57 KB, patch)
2019-02-11 17:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19164: (QA follow-up) Minor whitespace cleanup (3.78 KB, patch)
2019-02-11 17:46 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Knight 2017-08-22 15:43:20 UTC
In Bug 18690 Katrin Fischer suggested that it would be useful to have MARC Modification Templates used in stage MARC imports in a similar way to that enhancement's inclusion in bulkmarcimport.pl
Comment 1 Jon Knight 2017-08-22 15:58:37 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.
Comment 2 Katrin Fischer 2017-08-22 17:12:23 UTC
Woot!
Comment 3 Katrin Fischer 2017-08-22 17:43:43 UTC
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?
Comment 4 Jon Knight 2017-08-23 09:06:56 UTC
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.
Comment 5 Jon Knight 2017-08-23 09:08:13 UTC
Ah, just realised I was referencing the wrong Bug ID for the other bug - is actually 18389. Too many browser tabs open!
Comment 6 Jon Knight 2017-10-18 10:19:35 UTC
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.
Comment 7 Jon Knight 2017-10-18 10:24:56 UTC
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:
Comment 8 Jon Knight 2017-10-18 10:34:00 UTC
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.
Comment 9 Hugo Agud 2017-10-18 12:56:15 UTC
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
Comment 10 Jon Knight 2017-10-18 13:34:02 UTC
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.
Comment 11 Jon Knight 2017-10-18 13:37:43 UTC
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.
Comment 12 Alex Buckley 2017-10-21 03:51:34 UTC
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 13 Katrin Fischer 2017-10-22 10:13:05 UTC
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;
Comment 14 Jon Knight 2017-10-23 16:16:24 UTC
(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.
Comment 15 Jon Knight 2017-10-23 16:31:55 UTC
Created attachment 68425 [details] [review]
Bug_19164: remove use strict/warn lines (were commented out anyway)
Comment 16 Koha Team Lyon 3 2017-12-21 11:04:56 UTC
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.
Comment 17 Jon Knight 2017-12-21 12:43:31 UTC
OK, I'll take a look at this in a day or so.  Assigned back to me.
Comment 18 Jon Knight 2017-12-21 12:52:54 UTC
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?
Comment 19 Koha Team Lyon 3 2017-12-21 14:38:33 UTC
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..
Comment 20 Koha Team Lyon 3 2017-12-21 14:40:20 UTC
Created attachment 70033 [details]
marc_file
Comment 21 Koha Team Lyon 3 2018-02-23 16:01:00 UTC
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
Comment 22 Jon Knight 2018-03-07 18:43:20 UTC
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?
Comment 23 Koha Team Lyon 3 2018-03-15 16:24:26 UTC
Hello, 
I have tried on a 16.11.16 and it works.
I'm going to try it on a master
Comment 24 Jon Knight 2018-03-16 14:23:34 UTC
Great thank you - I'll change the status to needs sign off in that case!
Comment 25 Mackey Johnstone 2019-01-17 22:15:15 UTC
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 26 Martin Renvoize 2019-02-11 09:23:36 UTC
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.
Comment 27 Jon Knight 2019-02-11 16:05:48 UTC
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.
Comment 28 Jon Knight 2019-02-11 16:08:30 UTC
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.
Comment 29 Martin Renvoize 2019-02-11 17:46:44 UTC
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>
Comment 30 Martin Renvoize 2019-02-11 17:46:47 UTC
Created attachment 84988 [details] [review]
Bug 19164: (QA follow-up) Minor whitespace cleanup
Comment 31 Martin Renvoize 2019-02-11 17:49:13 UTC
I've taken  Mackey Johnstone as a signoff and added the line appropriately so I can move on to QA.
Comment 32 Martin Renvoize 2019-02-11 17:49:44 UTC
Works as expected, a nice simple but very helpful patch. Thanks for bearing with it so long Jon.

Passing QA
Comment 33 Nick Clemens 2019-02-19 16:03:08 UTC
Awesome work all!

Pushed to master for 19.05
Comment 34 Martin Renvoize 2019-02-26 09:09:39 UTC
Enhancement, will not be backported to 18.11.x series.