Bug 13572 - Add not-expired parameter to borrowers-force-messaging-defaults script
Summary: Add not-expired parameter to borrowers-force-messaging-defaults script
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Rémi Mayrand-Provencher
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 19472
  Show dependency treegraph
 
Reported: 2015-01-13 15:14 UTC by simith.doliveira
Modified: 2017-11-28 07:14 UTC (History)
11 users (show)

See Also:
Change sponsored?: 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 13572 - Add onlyactives parameter to force_borrower_messaging_defaults script (2.04 KB, patch)
2015-01-13 15:21 UTC, simith.doliveira
Details | Diff | Splinter Review
Bug13572 - Changed "borrowers-force-messaging-defaults" --help parameter's result to better explain the new functionality (2.03 KB, patch)
2015-07-30 14:57 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 13572 - Add onlyactives parameter to force_borrower_messaging_defaults script (2.73 KB, patch)
2016-01-04 03:08 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug13572 - Changed "borrowers-force-messaging-defaults" --help parameter's result to better explain the new functionality (2.08 KB, patch)
2016-01-04 03:08 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 13572 - Add --actives parameter to force_borrower_messaging_defaults script (3.45 KB, patch)
2016-12-15 15:59 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 13572 - Add --actives parameter to force_borrower_messaging_defaults script (3.44 KB, patch)
2017-10-11 14:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 13572: (Follow-up) Add perl extension to script (1.49 KB, patch)
2017-10-11 14:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 13572: (Follow-up) Print since on a separate line (1014 bytes, patch)
2017-10-11 14:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 13572 - Add --actives parameter to force_borrower_messaging_defaults script (3.45 KB, patch)
2017-10-13 09:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 13572: (Follow-up) Add perl extension to script (1.51 KB, patch)
2017-10-13 09:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 13572: (Follow-up) Print since on a separate line (1.01 KB, patch)
2017-10-13 09:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 13572: Patrons are still active if their expiry date is today (987 bytes, patch)
2017-10-16 15:01 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!