Bug 9739 - more options for serialsUpdate.pl
Summary: more options for serialsUpdate.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)
Target Milestone: ---
Assignee: Fridolin SOMERS
QA Contact:
URL:
Whiteboard:
Keywords: rel_3_14_candidate
Depends on:
Blocks:
 
Reported: 2013-03-01 13:16 UTC by Fridolin SOMERS
Modified: 2014-12-07 20:02 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
Proposed patch (3.93 KB, patch)
2013-03-01 13:38 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Proposed patch (followup) (3.93 KB, patch)
2013-03-01 13:41 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Proposed patch (3.93 KB, patch)
2013-05-10 07:54 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Proposed patch (followup) (3.89 KB, patch)
2013-05-10 07:55 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9739: more options for serialsUpdate.pl (4.04 KB, patch)
2013-09-08 01:59 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9739: more options for serialsUpdate.pl (followup) (4.01 KB, patch)
2013-09-08 01:59 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 9739 - followup - adds missing comma (879 bytes, patch)
2013-09-08 02:02 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 9739: more options for serialsUpdate.pl (4.25 KB, patch)
2013-09-29 11:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9739: more options for serialsUpdate.pl (followup) (4.07 KB, patch)
2013-09-29 11:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 9739 - followup - adds missing comma (948 bytes, patch)
2013-09-29 11:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 9739: Follow-up - Improve help text a bit (2.02 KB, patch)
2013-09-29 13:34 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 Fridolin SOMERS 2013-03-01 13:16:05 UTC
Script serialsUpdate.pl must be set in crontab to run daily.
It checks serials that are late, modifies status and adds a note.

A library reported that this automatic note can not be changed nor disabled.
So I propose to add an option to defined note (keeping default one if not defined) and and option to disable this note.
Comment 1 Fridolin SOMERS 2013-03-01 13:38:27 UTC Comment hidden (obsolete)
Comment 2 Fridolin SOMERS 2013-03-01 13:41:07 UTC Comment hidden (obsolete)
Comment 3 mathieu saby 2013-05-01 10:39:33 UTC
Fridolyn, I suppose the 1st attachment is obsolete?

M. Saby
Comment 4 Fridolin SOMERS 2013-05-02 13:18:52 UTC
(In reply to comment #3)
> Fridolyn, I suppose the 1st attachment is obsolete?

No, 1st attachement is the main patch.
Comment 5 mathieu saby 2013-05-03 10:20:00 UTC
Your followup looks strange : it contains the same strings as the main patch.

Mathieu
Comment 6 Fridolin SOMERS 2013-05-10 07:54:59 UTC Comment hidden (obsolete)
Comment 7 Fridolin SOMERS 2013-05-10 07:55:16 UTC Comment hidden (obsolete)
Comment 8 Fridolin SOMERS 2013-05-10 07:56:09 UTC
(In reply to comment #5)
> Your followup looks strange : it contains the same strings as the main patch.
> 
> Mathieu

Indeed.
I recreated patches form my branch and reattached them.
Thanks.
Comment 9 Bernardo Gonzalez Kriegel 2013-09-08 01:59:30 UTC Comment hidden (obsolete)
Comment 10 Bernardo Gonzalez Kriegel 2013-09-08 01:59:45 UTC Comment hidden (obsolete)
Comment 11 Bernardo Gonzalez Kriegel 2013-09-08 02:02:47 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2013-09-29 11:09:29 UTC
I have found some smaller problems with this script that I wanted to note:

1) When I start the script without giving any parameters nothing happens. Maybe it would be helpful to print out the help in this case?

2) The script updates serial issues that are already late and claimed with the note. I think it shouldn't touch issues that are already marked late in the interface.

3) The script overwrites and deletes existing notes on the issues.

I think all this problems predate your patch, but I am quite worried about 2) and 3).

The existing notes can now be preserved using the --no-note option, but I think ideally the new notes should be amended to the existing one in general.

Still, this patch is an improvement, so it will pass.
Comment 13 Katrin Fischer 2013-09-29 11:14:57 UTC
Created attachment 21588 [details] [review]
[PASSED QA] Bug 9739: more options for serialsUpdate.pl

Script serialsUpdate.pl must be set in crontab to run daily.
It checks serials that are late, modifies status and adds a note.

This patch adds some options :
--note : defined note, a defaut one is used if not defined
--no-note : disable automatic note
--verbose : used to control output
This patch also implements confirmation option "-c" who whas present but not used.

Test plan :
Run script on a database with late serials :
- serialsUpdate.pl -h
=> you get help text in output
- serialsUpdate.pl --man
=> you get full help
- serialsUpdate.pl -v
=> you get output of changed serials but database has not changed
- serialsUpdate.pl -c -v
=> you get output of changed serials and database has changed, with note "Automatically set to late"
- serialsUpdate.pl -c
=> database has changed without anything in output
- serialsUpdate.pl -c --note "LATE"
=> database has changed, with note "LATE"
- serialsUpdate.pl -c --no-note
=> database has changed with no note

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works as described. No koha-qa errors.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
New options behave as described.
There are some existing problems with this script that I have noted
directly on the bug report.
Comment 14 Katrin Fischer 2013-09-29 11:15:10 UTC
Created attachment 21589 [details] [review]
[PASSED QA] Bug 9739: more options for serialsUpdate.pl (followup)

cosmetic changes :
perltidy
q{} for query
list of columns is query instead of SELECT *

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Your cosmetic changes missed a comma, fixed in a followup

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 15 Katrin Fischer 2013-09-29 11:15:21 UTC
Created attachment 21590 [details] [review]
[PASSED QA] Bug 9739 - followup - adds missing comma

Adds a missing comma in select statement.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 16 Katrin Fischer 2013-09-29 13:34:42 UTC
Created attachment 21592 [details] [review]
Bug 9739: Follow-up - Improve help text a bit

Changes only strings in the script: the help text and the
success message for updated serial issues.

Thx to Jared for proof reading!
Comment 17 Galen Charlton 2013-10-20 19:00:07 UTC
Pushed to master.  Thanks, Fridolyn!