Bug 19454

Summary: Add the ability to filter on patron category for borrowers-force-messaging-defaults.pl
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Command-line UtilitiesAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: enhancement    
Priority: P5 - low CC: charles.farmer, jonathan.druart, katrin.fischer, nick, robin
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18197
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 19452    
Bug Blocks: 33204    
Attachments: Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl
Bug 19454: (Follow-up) Add a total line when updating
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl
Bug 19454: (Follow-up) Add a total line when updating
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl
Bug 19454: (Follow-up) Add a total line when updating
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl
Bug 19454: (Follow-up) Add a total line when updating
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl
Bug 19454: (Follow-up) Add a total line when updating
Bug 19454: (QA follow-up) Fix selection on categorycode
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl
Bug 19454: (Follow-up) Add a total line when updating
Bug 19454: (QA follow-up) Fix selection on categorycode
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl
Bug 19454: (Follow-up) Add a total line when updating
Bug 19454: (QA follow-up) Fix selection on categorycode

Description Marcel de Rooy 2017-10-12 12:06:37 UTC

    
Comment 1 Marcel de Rooy 2017-10-16 10:06:45 UTC
Created attachment 68176 [details] [review]
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl

Instead of updating patrons over all patron categories, it would be
helpful if we could filter on a specified category.

Test plan:
[1] Select two patrons A and B in say categories C1 and C2.
[2] Change the msg prefs for A and B away from defaults.
[3] Run borrowers-force-messaging-defaults.pl -doit -cat C1
    Verify that patron A changed and patron B did not.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2017-10-16 11:10:28 UTC
Created attachment 68177 [details] [review]
Bug 19454: (Follow-up) Add a total line when updating

Test plan:
[1] Run the script with -doit and -cat [some_category] and verify that
    the printed total is correct.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2017-10-17 12:31:56 UTC
Created attachment 68202 [details] [review]
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl

Instead of updating patrons over all patron categories, it would be
helpful if we could filter on a specified category.

Test plan:
[1] Select two patrons A and B in say categories C1 and C2.
[2] Change the msg prefs for A and B away from defaults.
[3] Run borrowers-force-messaging-defaults.pl -doit -cat C1
    Verify that patron A changed and patron B did not.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2017-10-17 12:32:00 UTC
Created attachment 68203 [details] [review]
Bug 19454: (Follow-up) Add a total line when updating

Test plan:
[1] Run the script with -doit and -cat [some_category] and verify that
    the printed total is correct.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2017-10-17 12:32:31 UTC
Rebased
Comment 6 Charles Farmer 2017-11-03 20:49:33 UTC
Can't really say if the problem is on my end or not, but I couldn't apply your patches:
> fatal: sha1 information is lacking or useless (misc/maintenance/borrowers-force-messaging-defaults.pl)
Comment 7 Marcel de Rooy 2017-11-06 07:46:18 UTC
(In reply to Charles Farmer from comment #6)
> Can't really say if the problem is on my end or not, but I couldn't apply
> your patches:
> > fatal: sha1 information is lacking or useless (misc/maintenance/borrowers-force-messaging-defaults.pl)

You need two dependencies. 19451 and 19452
Comment 8 Charles Farmer 2017-11-17 20:36:11 UTC
Created attachment 69205 [details] [review]
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl

Instead of updating patrons over all patron categories, it would be
helpful if we could filter on a specified category.

Test plan:
[1] Select two patrons A and B in say categories C1 and C2.
[2] Change the msg prefs for A and B away from defaults.
[3] Run borrowers-force-messaging-defaults.pl -doit -cat C1
    Verify that patron A changed and patron B did not.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 9 Charles Farmer 2017-11-17 20:36:14 UTC
Created attachment 69206 [details] [review]
Bug 19454: (Follow-up) Add a total line when updating

Test plan:
[1] Run the script with -doit and -cat [some_category] and verify that
    the printed total is correct.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 10 Marcel de Rooy 2017-11-20 07:06:07 UTC
(In reply to Charles Farmer from comment #9)
> Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Thx Charles
Comment 11 Marcel de Rooy 2018-02-15 13:27:08 UTC
Created attachment 71654 [details] [review]
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl

Instead of updating patrons over all patron categories, it would be
helpful if we could filter on a specified category.

Test plan:
[1] Select two patrons A and B in say categories C1 and C2.
[2] Change the msg prefs for A and B away from defaults.
[3] Run borrowers-force-messaging-defaults.pl -doit -cat C1
    Verify that patron A changed and patron B did not.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 12 Marcel de Rooy 2018-02-15 13:27:13 UTC
Created attachment 71655 [details] [review]
Bug 19454: (Follow-up) Add a total line when updating

Test plan:
[1] Run the script with -doit and -cat [some_category] and verify that
    the printed total is correct.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 13 Marcel de Rooy 2018-02-15 13:27:32 UTC
Simple rebase
Comment 14 Katrin Fischer 2018-03-12 14:44:07 UTC
Hi Marcel,

2 things I am afraid.

1) Question: I notice all other options are are -- and the new one is the only one - - is there a reason to use one or the other?

2) The example given doesn't work for me:

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ ./misc/maintenance/borrowers-force-messaging-defaults.pl --doit -category PT

Since: 0000-00-00
DBD::mysql::st execute failed: Column 'categorycode' in where clause is ambiguous [for Statement "SELECT DISTINCT bo.borrowernumber, bo.categorycode FROM borrowers bo
LEFT JOIN borrower_message_preferences mp USING (borrowernumber)
WHERE bo.dateenrolled >= ? AND categorycode = ?" with ParamValues: 0='0000-00-00', 1='PT'] at ./misc/maintenance/borrowers-force-messaging-defaults.pl line 62.
DBD::mysql::st fetchrow failed: fetch() without execute() [for Statement "SELECT DISTINCT bo.borrowernumber, bo.categorycode FROM borrowers bo
LEFT JOIN borrower_message_preferences mp USING (borrowernumber)
WHERE bo.dateenrolled >= ? AND categorycode = ?" with ParamValues: 0='0000-00-00', 1='PT'] at ./misc/maintenance/borrowers-force


My database is using the test date, I made sure there are patrons with category PT.
Comment 15 Marcel de Rooy 2018-03-14 12:30:44 UTC
(In reply to Katrin Fischer from comment #14)

Thx Katrin for QAing.

> 1) Question: I notice all other options are are -- and the new one is the
> only one - - is there a reason to use one or the other?

Good catch :) There is no real difference. I will add a minus for consistency. According to Getopt::Long we should one minus for single character options like -l and two minuses for long option names like --long. But if you define a 'category' option and you have nothing else starting with a 'c', -c and --c will work both.

> DBD::mysql::st execute failed: Column 'categorycode' in where clause is
> ambiguous 

Oops. Caused by a QA follow-up on one of the preceding reports that replaced a subquery by a join, followed by a rebase here. Will fix.
Comment 16 Marcel de Rooy 2018-03-14 12:31:09 UTC Comment hidden (obsolete)
Comment 17 Marcel de Rooy 2018-03-14 12:31:14 UTC Comment hidden (obsolete)
Comment 18 Marcel de Rooy 2018-03-14 12:31:18 UTC Comment hidden (obsolete)
Comment 19 Marcel de Rooy 2018-03-14 12:32:33 UTC
Created attachment 72852 [details] [review]
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl

Instead of updating patrons over all patron categories, it would be
helpful if we could filter on a specified category.

Test plan:
[1] Select two patrons A and B in say categories C1 and C2.
[2] Change the msg prefs for A and B away from defaults.
[3] Run borrowers-force-messaging-defaults.pl -doit -cat C1
    Verify that patron A changed and patron B did not.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Amended: Replace -category by --category. (marcelr 20180314)
Comment 20 Marcel de Rooy 2018-03-14 12:32:38 UTC
Created attachment 72853 [details] [review]
Bug 19454: (Follow-up) Add a total line when updating

Test plan:
[1] Run the script with -doit and -cat [some_category] and verify that
    the printed total is correct.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 21 Marcel de Rooy 2018-03-14 12:32:42 UTC
Created attachment 72854 [details] [review]
Bug 19454: (QA follow-up) Fix selection on categorycode

A follow-up on a preceding report introduced a join instead of a
subquery. This made the categorycode ambiguous.

Test plan:
See former patches.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 22 Katrin Fischer 2018-03-18 07:34:01 UTC
Created attachment 73067 [details] [review]
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl

Instead of updating patrons over all patron categories, it would be
helpful if we could filter on a specified category.

Test plan:
[1] Select two patrons A and B in say categories C1 and C2.
[2] Change the msg prefs for A and B away from defaults.
[3] Run borrowers-force-messaging-defaults.pl -doit -cat C1
    Verify that patron A changed and patron B did not.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Amended: Replace -category by --category. (marcelr 20180314)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2018-03-18 07:34:05 UTC
Created attachment 73068 [details] [review]
Bug 19454: (Follow-up) Add a total line when updating

Test plan:
[1] Run the script with -doit and -cat [some_category] and verify that
    the printed total is correct.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2018-03-18 07:34:09 UTC
Created attachment 73069 [details] [review]
Bug 19454: (QA follow-up) Fix selection on categorycode

A follow-up on a preceding report introduced a join instead of a
subquery. This made the categorycode ambiguous.

Test plan:
See former patches.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Jonathan Druart 2018-03-23 15:35:59 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 26 Nick Clemens 2018-03-26 12:07:10 UTC
Enhancement, not backported for 17.11