Bug 19454 - Add the ability to filter on patron category for borrowers-force-messaging-defaults.pl
Summary: Add the ability to filter on patron category for borrowers-force-messaging-de...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact: Katrin Fischer
URL:
Whiteboard:
Keywords:
Depends on: 19452
Blocks:
  Show dependency treegraph
 
Reported: 2017-10-12 12:06 UTC by Marcel de Rooy
Modified: 2018-04-06 13:28 UTC (History)
5 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:


Attachments
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl (3.41 KB, patch)
2017-10-16 10:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: (Follow-up) Add a total line when updating (1.43 KB, patch)
2017-10-16 11:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl (3.47 KB, patch)
2017-10-17 12:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: (Follow-up) Add a total line when updating (1.43 KB, patch)
2017-10-17 12:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl (3.49 KB, patch)
2017-11-17 20:36 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19454: (Follow-up) Add a total line when updating (1.45 KB, patch)
2017-11-17 20:36 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl (3.07 KB, patch)
2018-02-15 13:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: (Follow-up) Add a total line when updating (1.46 KB, patch)
2018-02-15 13:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl (3.14 KB, patch)
2018-03-14 12:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: (Follow-up) Add a total line when updating (1.46 KB, patch)
2018-03-14 12:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: (QA follow-up) Fix selection on categorycode (1.18 KB, patch)
2018-03-14 12:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl (3.14 KB, patch)
2018-03-14 12:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: (Follow-up) Add a total line when updating (1.46 KB, patch)
2018-03-14 12:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: (QA follow-up) Fix selection on categorycode (1.18 KB, patch)
2018-03-14 12:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19454: Add -category parameter to borrowers-force-messaging-defaults.pl (3.15 KB, patch)
2018-03-18 07:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19454: (Follow-up) Add a total line when updating (1.48 KB, patch)
2018-03-18 07:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19454: (QA follow-up) Fix selection on categorycode (1.20 KB, patch)
2018-03-18 07: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 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