Bug 13572

Summary: Add not-expired parameter to borrowers-force-messaging-defaults script
Product: Koha Reporter: simith.doliveira
Component: PatronsAssignee: Rémi Mayrand-Provencher <remi.mayrand-provencher>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: abbyrobertson.student, eric.begin, francois.charbonnier, gmcharlt, jonathan.druart, katrin.fischer, kyle.m.hall, m.de.rooy, marjorie.barry-vila, philippe.blouin, remi.mayrand-provencher
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18197
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 19472    
Attachments: Bug 13572 - Add onlyactives parameter to force_borrower_messaging_defaults script
Bug13572 - Changed "borrowers-force-messaging-defaults" --help parameter's result to better explain the new functionality
Bug 13572 - Add onlyactives parameter to force_borrower_messaging_defaults script
Bug13572 - Changed "borrowers-force-messaging-defaults" --help parameter's result to better explain the new functionality
Bug 13572 - Add --actives parameter to force_borrower_messaging_defaults script
Bug 13572 - Add --actives parameter to force_borrower_messaging_defaults script
Bug 13572: (Follow-up) Add perl extension to script
Bug 13572: (Follow-up) Print since on a separate line
Bug 13572 - Add --actives parameter to force_borrower_messaging_defaults script
Bug 13572: (Follow-up) Add perl extension to script
Bug 13572: (Follow-up) Print since on a separate line
Bug 13572: Patrons are still active if their expiry date is today

Description simith.doliveira 2015-01-13 15:14:58 UTC
This patch adds the onlyactives parameter to force_borrower_messaging_defaults script to allow to update just actives (not expired) user records if changes in the user categories are made.

Sponsored by CCSR
Comment 1 simith.doliveira 2015-01-13 15:20:29 UTC
To test:

1) Apply the patch
2) Active the preference system “EnhancedMessagingPreferences”
3) In “Administration › Patron categories” modify the default messaging preferences for a
 category. Ex. Student 
4) Run the script
5) Validate: expired Students don't have their patron messaging preferences
 changed
6) Validate: active Students do have their patron messaging preferences
 changed
Comment 2 simith.doliveira 2015-01-13 15:21:47 UTC Comment hidden (obsolete)
Comment 3 Abby 2015-01-15 02:14:31 UTC
I followed the test plan but I can't verify the patch fixes it.
Comment 4 Rémi Mayrand-Provencher 2015-07-30 14:57:45 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2016-01-04 03:08:21 UTC Comment hidden (obsolete)
Comment 6 Chris Cormack 2016-01-04 03:08:48 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-01-04 12:53:51 UTC
It would make more sense to have a --active[s] instead of --onlyactives, it will allow to add another flag later.
Comment 8 Charles Farmer 2016-12-15 15:59:39 UTC
Created attachment 58206 [details] [review]
Bug 13572 - Add --actives parameter to force_borrower_messaging_defaults script

1) Apply the patch
2) Create a new patron with random values, except for it expiration date, make it expired (Patrons > New Patron > Student)
3) Enable the system preference called “EnhancedMessagingPreferences”
4) In “Administration" > "Patron categories" > Student, modify the "days in advance", then click "Save"
5) run the script "./misc/maintenance/borrowers-force-messaging-defaults --doit --actives"
6) Validate that the student created in step 2 hasn't changed (Patrons > search)
7) Validate that any other student that isn't expired has changed.

---

This patch is a squashed version of the two previous ones, rebased on the current master, plus the changes asked in comment #7
Comment 9 Marcel de Rooy 2017-10-11 14:29:52 UTC
Created attachment 67959 [details] [review]
Bug 13572 - Add --actives parameter to force_borrower_messaging_defaults script

1) Apply the patch
2) Create a new patron with random values, except for it expiration date, make it expired (Patrons > New Patron > Student)
3) Enable the system preference called “EnhancedMessagingPreferences”
4) In “Administration" > "Patron categories" > Student, modify the "days in advance", then click "Save"
5) run the script "./misc/maintenance/borrowers-force-messaging-defaults --doit --actives"
6) Validate that the student created in step 2 hasn't changed (Patrons > search)
7) Validate that any other student that isn't expired has changed.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2017-10-11 14:30:20 UTC
Created attachment 67960 [details] [review]
Bug 13572: (Follow-up) Add perl extension to script

There is no reason to keep this perl script without the regular extension.
Please see other scripts in the same folder too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2017-10-11 14:30:25 UTC
Created attachment 67961 [details] [review]
Bug 13572: (Follow-up) Print since on a separate line

Very trivial change.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Nick Clemens 2017-10-13 09:47:21 UTC
Created attachment 68059 [details] [review]
Bug 13572 - Add --actives parameter to force_borrower_messaging_defaults script

1) Apply the patch
2) Create a new patron with random values, except for it expiration date, make it expired (Patrons > New Patron > Student)
3) Enable the system preference called “EnhancedMessagingPreferences”
4) In “Administration" > "Patron categories" > Student, modify the "days in advance", then click "Save"
5) run the script "./misc/maintenance/borrowers-force-messaging-defaults --doit --actives"
6) Validate that the student created in step 2 hasn't changed (Patrons > search)
7) Validate that any other student that isn't expired has changed.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Nick Clemens 2017-10-13 09:47:25 UTC
Created attachment 68060 [details] [review]
Bug 13572: (Follow-up) Add perl extension to script

There is no reason to keep this perl script without the regular extension.
Please see other scripts in the same folder too.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Nick Clemens 2017-10-13 09:47:28 UTC
Created attachment 68061 [details] [review]
Bug 13572: (Follow-up) Print since on a separate line

Very trivial change.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 15 Jonathan Druart 2017-10-16 12:42:58 UTC
Comment on attachment 68060 [details] [review]
Bug 13572: (Follow-up) Add perl extension to script

Please do that on its own bug report (can break scripts if not highlight in the release notes).
Comment 16 Jonathan Druart 2017-10-16 12:44:02 UTC
Just asking: maybe --not-expired is more meaningful here?
I do not think we already use "active" on the interface or scripts.
Comment 17 Marcel de Rooy 2017-10-16 12:48:56 UTC
(In reply to Jonathan Druart from comment #16)
> Just asking: maybe --not-expired is more meaningful here?
> I do not think we already use "active" on the interface or scripts.

No objection
Comment 18 Francois Charbonnier 2017-10-16 13:41:06 UTC
No objection
Comment 19 Jonathan Druart 2017-10-16 15:01:35 UTC
Created attachment 68183 [details] [review]
Bug 13572: Patrons are still active if their expiry date is today
Comment 20 Jonathan Druart 2017-10-16 15:02:27 UTC
Pushed to master for 17.11, thanks to everybody involved!