Bug 14722 - Refactor the catalogue export tool (command-line tools/export.pl does not work anymore. Use misc/export_records.pl instead)
Summary: Refactor the catalogue export tool (command-line tools/export.pl does not wor...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 14682 (view as bug list)
Depends on: 14709 14710
Blocks: 15493 15923 16855 16859 16935 17622
  Show dependency treegraph
 
Reported: 2015-08-25 09:56 UTC by Jonathan Druart
Modified: 2017-06-14 22:17 UTC (History)
7 users (show)

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


Attachments
Bug 14722: Add Koha::Biblioitem[s] (2.87 KB, patch)
2015-08-25 10:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14722: Refactor the export tool (56.93 KB, patch)
2015-08-25 10:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14722: Add Koha::Biblioitem[s] (2.92 KB, patch)
2015-09-03 09:23 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 14722: Refactor the export tool (56.99 KB, patch)
2015-09-03 09:23 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 14722: rename the script (1.05 KB, patch)
2015-10-19 08:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14722: Add POD for some options (1.55 KB, patch)
2015-10-19 08:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14722: CSV is only available for biblio records (1.50 KB, patch)
2015-10-19 08:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 14722: Add Koha::Biblioitem[s] (2.99 KB, patch)
2015-10-20 22:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14722: Refactor the export tool (57.05 KB, patch)
2015-10-20 22:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14722: rename the script (1.12 KB, patch)
2015-10-20 22:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14722: Add POD for some options (1.61 KB, patch)
2015-10-20 22:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14722: CSV is only available for biblio records (1.57 KB, patch)
2015-10-20 22:27 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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. ***