We had the need to filter out certain record based on marc-fields when exporting, so added a --marc_conditions options where filter conditions can be passed to the export script. For example: --marc_conditions="942a=!1".
Created attachment 73384 [details] [review] Bug 20486: Add --marc_conditions option to export_records.pl Add --marc_conditions option to export_records.pl script, for excluding records based on conditions applied to MARC-record data. How to test: 1) Run tests in t/Exporter/Record.t 2) All tests should pass Sponsored-by: Gothenburg University Library
Oops, condition in first comment should be: --marc_conditions="942a!=1" (not =!)
Created attachment 73497 [details] [review] Bug 20486: Add --marc_conditions option to export_records.pl Add --marc_conditions option to export_records.pl script, for excluding records based on conditions applied to MARC-record data. How to test: 1) Run tests in t/Exporter/Record.t 2) All tests should pass Sponsored-by: Gothenburg University Library Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Created attachment 73999 [details] [review] Bug 20486: Add --marc_conditions option to export_records.pl Add --marc_conditions option to export_records.pl script, for excluding records based on conditions applied to MARC-record data. How to test: 1) Run tests in t/Exporter/Record.t 2) All tests should pass Sponsored-by: Gothenburg University Library
Fix a problem where _get_record_for_export returns undef both on error and if not matching condition, so impossible to catch the error, now logging all errors inside of _get_record_for_export instead.
Comment on attachment 73999 [details] [review] Bug 20486: Add --marc_conditions option to export_records.pl Review of attachment 73999 [details] [review]: ----------------------------------------------------------------- Same question as on bug 20465: this adds functionality to the CLI script that is not available in the export tool in the GUI. Do we want them to diverge like that? Requesting feedback. Also: QA scripts shows a POD issue: FAIL misc/export_records.pl OK critic OK forbidden patterns OK git manipulation FAIL pod Apparent command =back not preceded by blank line *** ERROR: =over on line 263 without closing =back (at head1) in file misc/export_records.pl ::: Koha/Exporter/Record.pm @@ +77,5 @@ > + return undef unless all { $op->($_, $match_value) } @target_values; > + } > + } > + } > + return undef if $no_target && $operator ne '!='; # TODO: ?? Gives me an uneasy feeling. What was the thought here?
Created attachment 74853 [details] [review] Bug 20486: Add --marc_conditions option to export_records.pl Add --marc_conditions option to export_records.pl script, for excluding records based on conditions applied to MARC-record data. How to test: 1) Run tests in t/Exporter/Record.t 2) All tests should pass Sponsored-by: Gothenburg University Library
I fixed the pod issue. The reason for the "# TODO: ??" was that not completely sure how ta handle the case where marc target cannot be found for different operators, but think it's handled correctly so removed this. Unfortunately we do not have the resources to port this functionality to the web-UI as well as we do not use it. This was initially just supposed to be a hard coded hack for our local needs but I generalized the matching functionality so other targets that the ones we use can be used for filtering in case someone else needed this.
Created attachment 75110 [details] [review] Bug 20486: Add --marc_conditions option to export_records.pl Add --marc_conditions option to export_records.pl script, for excluding records based on conditions applied to MARC-record data. How to test: 1) Run tests in t/Exporter/Record.t 2) All tests should pass Sponsored-by: Gothenburg University Library
Rebased against Koha master.
Created attachment 79266 [details] [review] Bug 20486: Add --marc_conditions option to export_records.pl Add --marc_conditions option to export_records.pl script, for excluding records based on conditions applied to MARC-record data. How to test: 1) Run tests in t/Exporter/Record.t 2) All tests should pass Sponsored-by: Gothenburg University Library Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 80318 [details] [review] Bug 20486: Add --marc_conditions option to export_records.pl Add --marc_conditions option to export_records.pl script, for excluding records based on conditions applied to MARC-record data. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass Sponsored-by: Gothenburg University Library Signed-off-by: Maksim Sen <maksim.sen@inlibro.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
I have passed QA But the question about the divergent functionality between the CLI and the web interface remains unanswered. I will leave that in the RM hands to decide
I would like a second opinion here, I would like the scripts to have the same options, but don't consider it a blocker. Can another QA member (or more) weigh in
I would not consider it blocker either. Side note: return undef will raise a perlcritic warning: "return" statement with explicit "undef"
The ui part of this could be done as follow-up bug, no blocker for me. But the return explicit undef should be fixed, we want to pass perlcritic
Created attachment 81762 [details] [review] Bug 20486: Add --marc_conditions option to export_records.pl Add --marc_conditions option to export_records.pl script, for excluding records based on conditions applied to MARC-record data. How to test: 1) Run tests in t/db_dependent/Exporter/Record.t 2) All tests should pass Sponsored-by: Gothenburg University Library Signed-off-by: Maksim Sen <maksim.sen@inlibro.com> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Ok, fixed!
Awesome work all! Pushed to master for 18.11
Enhancement, will not be backported to 18.05.x series.
Hello, I have questions about this one : is it possible to add several filters? If it's the case, which is the separator between the several filters? Could we use whatever operator in the filter (= or > or < )? Thanks in advance! Sonia
(In reply to Koha Team Lyon 3 from comment #21) > Hello, > I have questions about this one : is it possible to add several filters? If > it's the case, which is the separator between the several filters? > Could we use whatever operator in the filter (= or > or < )? > > Thanks in advance! > > Sonia Bonjour Sonia, Reading the POD, it says: """ [...] with multiple conditions separated by commas (,). For example: --marc_conditions="035a!=(EXAMPLE)123,041a=swe". Multiple conditions are all required to match. """ The operators are <, >, = and !=