Bug 9045 - Adding some parameters to export_borrowers.pl
Summary: Adding some parameters to export_borrowers.pl
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)
Assignee: Sophie MEYNIEUX
QA Contact: Katrin Fischer
URL:
Keywords:
: 8874 (view as bug list)
Depends on: 8376
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-09 17:07 UTC by Sophie MEYNIEUX
Modified: 2014-05-26 21:04 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 9045 : Adding --where parameter to filter what kind of borrowers to export and --separator to choose a specific caracter for patron export (2.61 KB, patch)
2012-11-09 17:16 UTC, Sophie MEYNIEUX
Details | Diff | Splinter Review
Bug 9045 : Adding --where parameter to filter what kind of borrowers to export and --separator to choose a specific caracter for patron export (2.59 KB, patch)
2012-11-09 17:21 UTC, Sophie MEYNIEUX
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9045 : Adding --where parameter to filter what kind of borrowers to export and --separator to choose a specific caracter for patron export (2.73 KB, patch)
2012-12-07 14:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9045 followup : Adding test if results has some records and usage example for --where clause (1.56 KB, patch)
2013-03-18 14:32 UTC, Sophie MEYNIEUX
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9045 - Adding some parameters to export_borrowers.pl (1.97 KB, patch)
2013-03-19 19:03 UTC, Owen Leonard
Details | Diff | Splinter Review
[PASSED QA] Bug 9045 : Adding --where parameter to filter what kind of borrowers to export and --separator to choose a specific caracter for patron export (2.83 KB, patch)
2013-03-21 09:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9045 - Adding some parameters to export_borrowers.pl (2.28 KB, patch)
2013-03-21 09:03 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 Sophie MEYNIEUX 2012-11-09 17:07:10 UTC
As suggested by Paul Poulain un Bug 8376, this patch adds a --where parameter to filter some kind of borrowers to export.
Also adding a --separator parameter, usefull when the recipient of the export ask for a specific file format.
Comment 1 Sophie MEYNIEUX 2012-11-09 17:16:30 UTC Comment hidden (obsolete)
Comment 2 Sophie MEYNIEUX 2012-11-09 17:21:11 UTC Comment hidden (obsolete)
Comment 3 Sophie MEYNIEUX 2012-11-09 17:21:50 UTC
Forgot a warn, sent a new patch
Comment 4 Kyle M Hall 2012-12-07 14:13:15 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2012-12-07 14:30:45 UTC
*** Bug 8874 has been marked as a duplicate of this bug. ***
Comment 6 Marcel de Rooy 2013-02-08 10:04:03 UTC
QA Comment:
Nice addition! Two points of attention: 

Looking for a cardnumber that does not exist produces: DBD::mysql::st fetchrow_array failed: fetch() without execute() at ./export_borrowers.pl line 112.
Please check if your query still has result records. Note that you should already do that on line 91 (first fetchrow). This may already give no results and a uninit warning in the log.

Documentational note: Using a condition on a character field requires using quotes like:  ./export_borrowers.pl -s @ -where "surname='De Rooy'"
I would suggest explaining this in the usage statement with a simple example (not using the previous one :)

Just for the record: The where clause coming from the command line is not checked, so could be misused (sql injection). Note that does not block this patch; 
more command-line scripts in misc contain similar constructs. 

Failed QA
Comment 7 Sophie MEYNIEUX 2013-03-18 14:32:09 UTC Comment hidden (obsolete)
Comment 8 Sophie MEYNIEUX 2013-03-18 14:34:41 UTC
Follow-up patch to answer Marcel De Rooy requirements.

If request has no results, script exits with a meaningful message
Comment 9 Owen Leonard 2013-03-19 19:03:39 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2013-03-21 09:03:13 UTC
Created attachment 16578 [details] [review]
[PASSED QA] Bug 9045 : Adding --where parameter to filter what kind of borrowers to export and --separator to choose a specific caracter for patron export

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Tested, works as advertised. Note, to use tab character as a separater,
do this: -s $'\t'

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comments on second patch.
Comment 11 Katrin Fischer 2013-03-21 09:03:36 UTC
Created attachment 16579 [details] [review]
[PASSED QA] Bug 9045 - Adding some parameters to export_borrowers.pl

Followup : Adding test if results has some records and usage example for
--where clause

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

I infer from the bug comments that a proper test for this patch is to
check how the script reacts if it returns no results with a --where
clause, and to verify that there is a good usage example for the --where
option. These are both true, so I think this passes.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests and QA script pass.

Tests done:
- tested -s option with \;
- tested -f surname -f borrowernumber to have multiple fields specified
- tested -w with "borrowernumber > 100" and similar with and without
  results - if no results a message is shown
- tested -h shows documentation for new options
Comment 12 Jared Camins-Esakov 2013-03-21 14:48:28 UTC
This patch has been pushed to master.