Bug 11278 - Adjusting bulkmarcimport.pl for customization routine and verbose printing
Summary: Adjusting bulkmarcimport.pl for customization routine and verbose printing
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Whiteboard:
Keywords: rel_3_16_candidate
Depends on:
Blocks:
 
Reported: 2013-11-21 12:01 UTC by Marcel de Rooy
Modified: 2014-12-07 20:07 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
One test record in marcxml (2.77 KB, text/xml)
2013-11-21 12:34 UTC, Marcel de Rooy
Details
Bug 11278: Adjusting bulkmarcimport.pl for customization routine and verbose printing (3.14 KB, patch)
2013-11-21 12:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11278: [EXAMPLE: DO_NOT_PUSH] Example LocalChanges.pm (1.39 KB, patch)
2013-11-21 12:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11278: Adjusting bulkmarcimport.pl for customization routine and verbose printing (3.14 KB, patch)
2013-11-25 13:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11278: Followup for customize command line parameter (3.67 KB, patch)
2013-11-25 13:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11278: Adjusting bulkmarcimport.pl for customization routine and verbose printing (3.16 KB, patch)
2014-01-27 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11278: Followup for customize command line parameter (3.69 KB, patch)
2014-01-27 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11278: (follow-up) Return an exit value (1) if the module is not found. (914 bytes, patch)
2014-01-27 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 11278: Adjusting bulkmarcimport.pl for customization routine and verbose printing (3.23 KB, patch)
2014-03-11 11:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 11278: Followup for customize command line parameter (3.77 KB, patch)
2014-03-11 11:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 11278: Followup for customize command line parameter (3.77 KB, patch)
2014-03-11 11:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 11278: Adjusting bulkmarcimport.pl for customization routine and verbose printing (3.23 KB, patch)
2014-03-11 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 11278: Followup for customize command line parameter (3.77 KB, patch)
2014-03-11 11:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 11278: (follow-up) Return an exit value (1) if the module is not found. (1.22 KB, patch)
2014-03-11 11:37 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 Marcel de Rooy 2013-11-21 12:01:58 UTC
The forthcoming patch changes bulkmarcimport.pl in two ways:

1) For the verbose option, verbose level 2 now means print the formatted marcxml for the records. Verbose 1 just prints dots and counts-per-hundred-records.

2) The new code for customization allows you to add a LocalChanges.pm in the misc/migration_tools folder. If this module exists, it is 'required' and a routine customize is called for each record. This makes it possible to add local adjustments to your marc record before they are processed in bulkmarcimport. I will attach a second example patch with such a module just to show the idea; this patch does not need signoff, etc.
Comment 1 Marcel de Rooy 2013-11-21 12:34:00 UTC
Created attachment 23061 [details]
One test record in marcxml
Comment 2 Marcel de Rooy 2013-11-21 12:38:17 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2013-11-21 12:38:20 UTC
Created attachment 23063 [details] [review]
Bug 11278: [EXAMPLE: DO_NOT_PUSH] Example LocalChanges.pm

This patch adds a LocalChanges.pm to migration_tools.
To illustrate the concept of record customization during bulkmarcimport.
Comment 4 Marcel de Rooy 2013-11-25 13:52:09 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2013-11-25 13:52:14 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2014-01-27 11:57:00 UTC
Hi Marcel,
It does not seem to work with the default value for the custom parameter:

% perl misc/migration_tools/bulkmarcimport.pl -file /tmp/zztest01.xml -v=2 -b -m XML -t | more
TESTING MODE ONLY
    DOING NOTHING
===============
Characteristic MARC flavour: UNIMARC
LDR 00000uam a22000001i 4500
001     734
008     870805
041    _aeng
044    _cNL
070    _a86307796X
084    _a14
245 00 _aEmerging hierarchies : processes of stratification and early state formation in the Indonesian archipelago : prehistory and the ethnographic present
       _cdoor Ina Erna Slamet-Velsink
260    _a[S.l.
       _bs.n.]
       _c1986
300    _aviii, 310 p., 20 pl.
       _bill., maps
       _c22 cm
500    _aDoctoral diss. RU Leiden
500    _aBibliogr.: p. 278-296
500    _aIncludes index
650  4 _aarchaeology
650  4 _astates
650  4 _anation building
650  4 _acultural and social anthropology
651  4 _aindonesia
700 1  _aSlamet-Velsink, Ina Erna
852    _aCentrale Bibliotheek (CB)
       _cN 87-911
       _p14000000001457
       _zBeschikbaar
852    _aCentrale Bibliotheek (CB)
       _cN 96-317
       _p14000002218786
       _zBeschikbaar

1 MARC records done in 0.0246779918670654 seconds


There is no 005 created.

Marked as Failed QA.
Comment 7 Marcel de Rooy 2014-01-27 12:38:37 UTC
(In reply to Jonathan Druart from comment #6)
> Hi Marcel,
> It does not seem to work with the default value for the custom parameter:
> 
> % perl misc/migration_tools/bulkmarcimport.pl -file /tmp/zztest01.xml -v=2
> -b -m XML -t | more

The default value will only be used if you supply the custom flag. Something like bulkmarcimport.pl -file ... -cust ...
In your example the -custom parameter is not passed, so bulkmarcimport does not look at the customization routines (just normal behavior).

Does this need a better explanation in the POD ?
Comment 8 Jonathan Druart 2014-01-27 13:47:18 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2014-01-27 13:47:41 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2014-01-27 13:47:50 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2014-01-27 13:49:08 UTC
(In reply to M. de Rooy from comment #7)
> In your example the -custom parameter is not passed, so bulkmarcimport does
> not look at the customization routines (just normal behavior).

Yes sorry, I have just stupidly followed the test plan.
Comment 12 Marcel de Rooy 2014-01-27 13:57:27 UTC
(In reply to Jonathan Druart from comment #11)
Thanks for testing and follow-up.
Comment 13 Martin Renvoize 2014-03-11 11:27:18 UTC Comment hidden (obsolete)
Comment 14 Martin Renvoize 2014-03-11 11:27:35 UTC Comment hidden (obsolete)
Comment 15 Martin Renvoize 2014-03-11 11:33:59 UTC Comment hidden (obsolete)
Comment 16 Martin Renvoize 2014-03-11 11:37:06 UTC
Created attachment 26099 [details] [review]
[PASSED QA] Bug 11278: Adjusting bulkmarcimport.pl for customization routine and verbose printing

This patch makes two adjustments:
[1] For the verbose option, verbose level 2 now means print the formatted
marcxml per record.
[2] If a module LocalChanges.pm is found in misc/migration_tools, the routine
customize in this module is called for each marc record. This allows you to
make local changes to these marc records before importing them.

Test plan:
[1] Test the verbose option: do no use -v on the command line, use -v 1 and
-v 2 on the command line. (Do not yet copy LocalChanges.pm in the folder.)
You may used the attached example file on Bugzilla:
perl misc/migration_tools/bulkmarcimport.pl -file zztest01.xml -v 2 -b -m XML -t | more
Note the option t for test; no records will be imported.
[2] Copy LocalChanges.pm in the migration_tools folder. You may use the
example provided on Bugzilla (in a patch). If you use the example module,
check the contents of 001, 005 and 590 fields. (The verbose 2 option allows
you to easily check that.)

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2014-03-11 11:37:22 UTC
Created attachment 26100 [details] [review]
[PASSED QA] Bug 11278: Followup for customize command line parameter

The initial patch for this bug did not include a specific command line
option for customization. If a module LocalChanges.pm existed, it would
be used without asking.
This patch adds a command line option enabling the customization option
and offering the extra possibility of using another module name. If no file
name is passed, we default to LocalChanges.
Without the -custom option, behavior is as it was.
Also some POD lines are added to document the feature.

Test plan:
[1] Make a LocalChanges.pm in migration_tools. Verify that it is not used,
    if you do not enable the -cust parameter.
[2] Run the script again with -cust. Verify that it is called now.
[3] Copy LocalChanges.pm to Whatever.pm. Make some change. Run with
    -cust Whatever and verify that the new module is used.
[4] Copy Whatever.pm to another dir, make some change. Run with -cust and the
    full name. Verify that the latest change was used.
[5] Run without any option. Check the pod documentation.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2014-03-11 11:37:47 UTC
Created attachment 26101 [details] [review]
[PASSED QA] Bug 11278: (follow-up) Return an exit value (1) if the module is not found.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize 2014-03-11 11:45:25 UTC
Does exactly what it says on the tin, and passes QA scripts too.

I'm happy that the code all looks sane and reasonable, and have tested some larger imports with & without the given example filter module.

I would love to see some of the bulkmarkimport features added to the stage_file.pl and commit_file.pl workflow for this action; but this is for another day.
Comment 20 Galen Charlton 2014-05-05 01:00:22 UTC
Pushed to master.  Thanks, Marcel!