Bug 14722

Summary: Refactor the catalogue export tool (command-line tools/export.pl does not work anymore. Use misc/export_records.pl instead)
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: ToolsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, gwilliams, josef.moravec, katrin.fischer, paul.poulain, sophie.meynieux, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 14709, 14710    
Bug Blocks: 13317, 15493, 15923, 16855, 16859, 16935, 17622    
Attachments: Bug 14722: Add Koha::Biblioitem[s]
Bug 14722: Refactor the export tool
Bug 14722: Add Koha::Biblioitem[s]
Bug 14722: Refactor the export tool
Bug 14722: rename the script
Bug 14722: Add POD for some options
Bug 14722: CSV is only available for biblio records
[PASSED QA] Bug 14722: Add Koha::Biblioitem[s]
[PASSED QA] Bug 14722: Refactor the export tool
[PASSED QA] Bug 14722: rename the script
[PASSED QA] Bug 14722: Add POD for some options
[PASSED QA] Bug 14722: CSV is only available for biblio records

Description Jonathan Druart 2015-08-25 09:56:26 UTC
The export tool (tools/export.pl) is badly designed and can be call from the command line.
The part of the code used when called from the command line should be moved to a specific script.

The common code must be moved to a package, with tests provided.
Comment 1 Jonathan Druart 2015-08-25 10:43:15 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-08-25 10:43:22 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2015-09-03 09:23:06 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2015-09-03 09:23:30 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2015-10-18 14:46:26 UTC
A few small things:

1) Could we rename the script from just export.pl to export_records.pl? I think this would make it easier to spot and be in line with the export_borrowers.pl script already in the same directory.

2) Running the script without any argument exports all your bibliographic records - this could create quite a big file for a bigger catalog. Could it be better to display the help and have a -c switch for confirming or similar?

3) Some of the options have no documentation. For example: start_accession and end_accession - is this the date_created from the record?

4) Are csv and auths incompatible? It seems that 
./misc/export.pl --filename=koha2.mrc --format=csv --record-type=auths
Creates a file with bib data.
Comment 6 Katrin Fischer 2015-10-18 14:48:21 UTC
Overall - big improvement. Please take a look at my comments soon!
Comment 7 Jonathan Druart 2015-10-19 08:29:06 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-10-19 08:29:11 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-10-19 08:29:14 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2015-10-19 08:31:27 UTC
(In reply to Katrin Fischer from comment #5)
> A few small things:
> 
> 1) Could we rename the script from just export.pl to export_records.pl? I
> think this would make it easier to spot and be in line with the
> export_borrowers.pl script already in the same directory.

Yes, agreed.
Feel free to squash patches 2 and 3.

> 2) Running the script without any argument exports all your bibliographic
> records - this could create quite a big file for a bigger catalog. Could it
> be better to display the help and have a -c switch for confirming or similar?

It's the current behavior. If we want to change it, it should be done in another bug report I think.

> 3) Some of the options have no documentation. For example: start_accession
> and end_accession - is this the date_created from the record?

I documented all the options in a new patch.

> 4) Are csv and auths incompatible? It seems that 
> ./misc/export.pl --filename=koha2.mrc --format=csv --record-type=auths
> Creates a file with bib data.

Exactly, it does not work.
I have added a safeguard in the script.
Comment 11 Katrin Fischer 2015-10-20 22:27:44 UTC
Created attachment 43673 [details] [review]
[PASSED QA] Bug 14722: Add Koha::Biblioitem[s]

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2015-10-20 22:27:48 UTC
Created attachment 43674 [details] [review]
[PASSED QA] Bug 14722: Refactor the export tool

Why a refactoring was need for this script?
The export tool (tools/export.pl) can be called from the command line
and some parts of code were unnecessarity complicated (just look at the
code, you will understand).

Worse still, the script does not provide the same options for both
interface. For instance you cannot export records given a range of
biblionumbers, authids, callnumbers, etc. from the commandline.

What does this patch?
1/ Important: The script tools/export.pl does not work anymore if called from
the command line (should be in the release notes).
2/ The code used to generated a file (csv, iso2709 or xml) has been moved to a new
module (Koha::Exporter::Record) and tests have been provided.
3/ No change is done on the web interface
4/ Some new options have been added to the commandline script
(misc/export.pl):
    - starting_authid
    - ending_authid
    - authtype
    - starting_biblionumber
    - ending_biblionumber
    - itemtype
    - starting_callnumber
    - ending_callnumber
    - start_accession
    - end_accession
5/ There is a change in the behavior if an error occurs:
Can't call method "as_usmarc" on an undefined value at Koha/Exporter/Record.pm line 114.
record (number 5530) is invalid and therefore not exported because its reopening generates warnings above at Koha/Exporter/Record.pm line 117.

Before this patch, they were not displayed (using the command line).

What does not do this patch?
It does not provide the 'clean', 'timestamp' and 'deleted_barcodes' options to
the web interface (same as before).

What about the perfs?
With a DB with ~800 biblios (MARC21)
Before: perl tools/export.pl 14.79s user 0.83s system 71% cpu 21.905 total
After:  perl misc/export.pl  17.19s user 0.84s system 75% cpu 24.018 total

With a DB with ~6400 biblios (UNIMARC)
Before: perl tools/export.pl 26.55s user 0.76s system 76% cpu 35.498 total
After:  perl misc/export.pl  26.78s user 0.84s system 80% cpu 34.494 total

How to test this patch?
Test plan:
A. Web interface:
1/ On the current master, export some records, biblios and authorities (with
the 3 differents exports) playing with the different filters (item type,
libraries, callnumber, accession date, don't export items, remove
non-local items, don't export fields, etc.).
2/ Apply this patch, export again the same records, and compare the
generated files. They must be identical!
3/ Confirm that the export features on the checkout list
(circ/circulation.pl) works as before this patch.

B. The command line
1/ On the current master, export some records, biblios and authorities (with
the 2 differents exports) playing with the different options (date,
deleted_barcodes, clean).
2/ Apply this patch, export again the same records, and compare the
generated files. They must be identical!

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2015-10-20 22:27:51 UTC
Created attachment 43675 [details] [review]
[PASSED QA] Bug 14722: rename the script

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2015-10-20 22:27:54 UTC
Created attachment 43676 [details] [review]
[PASSED QA] Bug 14722: Add POD for some options

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2015-10-20 22:27:58 UTC
Created attachment 43677 [details] [review]
[PASSED QA] Bug 14722: CSV is only available for biblio records

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Tomás Cohen Arazi 2015-10-27 20:07:54 UTC
Patches pushed to master.

Nice job Jonathan!
Comment 17 Fridolin Somers 2015-10-28 08:33:08 UTC
Is the script "misc/exportauth.pl" then obsolete ?
Comment 18 Paul Poulain 2015-10-28 09:39:24 UTC
Updating title and importance to have the release notes automatically filled.
Comment 19 Josef Moravec 2017-01-17 18:44:40 UTC
*** Bug 14682 has been marked as a duplicate of this bug. ***