Description
Marcel de Rooy
2017-10-12 12:06:37 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> 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> 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> 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> Rebased 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)
(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 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> 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> (In reply to Charles Farmer from comment #9) > Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> Thx Charles 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> 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> Simple rebase 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. (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. Created attachment 72849 [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 20180304) Created attachment 72850 [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> Created attachment 72851 [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> 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) 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> 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> 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> 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> 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> Pushed to master for 18.05, thanks to everybody involved! Enhancement, not backported for 17.11 |