Bug 14834 - Make membership_expiry cronjob more flexible
Summary: Make membership_expiry cronjob more flexible
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on: 6810
Blocks: 15543
  Show dependency treegraph
 
Reported: 2015-09-16 14:55 UTC by Marcel de Rooy
Modified: 2016-12-05 21:27 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14834: Add a range parameter (in days) to membership_expiry cronjob (11.60 KB, patch)
2015-09-17 11:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14834: Add range parameters to membership_expiry cronjob (12.08 KB, patch)
2015-09-17 11:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14834: Make membership_expiry cronjob more flexible (12.04 KB, patch)
2016-01-18 08:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14834: Make membership_expiry cronjob more flexible (12.26 KB, patch)
2016-04-20 13:35 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 14834: Make membership_expiry cronjob more flexible (12.30 KB, patch)
2016-04-28 12:53 UTC, Kyle M Hall
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 2015-09-16 14:55:20 UTC
The cronjob from bug 6810 now only looks at the exact day a membership expires. You have to run it each night.
But what if you want to run it once a week?
Or rerun it a day later and want to include the expires of the day before?

An additional range parameter would be handy..
Comment 1 Marcel de Rooy 2015-09-17 11:10:35 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2015-09-17 11:16:26 UTC
Wait..
Comment 3 Marcel de Rooy 2015-09-17 11:54:46 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2016-01-15 10:56:24 UTC
Does not apply anymore. Rebasing this soon.
Comment 5 Marcel de Rooy 2016-01-18 08:58:32 UTC Comment hidden (obsolete)
Comment 6 Bernardo Gonzalez Kriegel 2016-04-20 13:35:51 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2016-04-20 13:42:51 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #6)
> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Thx for your fast reply :)
Comment 8 Marcel de Rooy 2016-04-20 13:48:59 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #6)
> New parameters work with one (-) or two(--) dashes, no problem
> with that but convention suggest that 'long' options use two-dashes.

Did not really look at that. Just expanded on what was there.
Could be that we need something like o|option to make that work..
Comment 9 Marcel de Rooy 2016-04-20 14:11:35 UTC
(In reply to Marcel de Rooy from comment #8)
> (In reply to Bernardo Gonzalez Kriegel from comment #6)
> > New parameters work with one (-) or two(--) dashes, no problem
> > with that but convention suggest that 'long' options use two-dashes.
> 
> Did not really look at that. Just expanded on what was there.
> Could be that we need something like o|option to make that work..

Found out that this can be arranged (more or less) with:
$Getopt::Long::bundling = 1;

But note that we never used this in the Koha codebase. So I guess this patch should not start it..
Comment 10 Kyle M Hall 2016-04-28 12:53:21 UTC
Created attachment 50912 [details] [review]
Bug 14834: Make membership_expiry cronjob more flexible

This patch adds three parameters to the cron job: -before and -after, and
-branch.

You can run the cronjob now in an adjusted frequency: say once a week with
before 6 or after 6 (not both together). If your pref is set to 14, running
before=6 will include expiries from 8 days to 14 days ahead. When you
use after=6, you would include 14 days to 20 days ahead, etc.

You could also rerun the job of yesterday by setting before=1 and after=-1;
this could help in case of problem recovery.

Obviously, the branch parameter can be used as a filter.

NOTE: Why are these parameters passed only via the command line?
Well, obviously the branch parameter is not suitable for a pref.
The before/after parameter allows you to handle expiry mails different from
the normal scheme or could be used in some sort of recovery. In those cases
it will be more practical to use a command line parameter than editing a
pref.

NOTE: The unit test has been adjusted for the above reasons, but I also
added some lines to let existing expires not interfere with the added
borrowers by an additional count and using the branchcode parameter.

Test plan:
[1] Run the adjusted unit test GetUpcomingMembershipExpires.t
[2] Set the expiry date for patron A to now+16 (with pref 14).
    Set the expiry date for patron B to now+11.
[3] Run the cronjob without range. You should not see A and B.
[4] Run the cronjob with before 3. You should see patron B.
[5] Run the cronjob with before 3 and after 2. You should see A and B.
[6] Repeat step 5 with a branchcode that does not exist. No patrons.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described following test plan.
Test pass
No errors

New parameters work with one (-) or two(--) dashes, no problem
with that but convention suggest that 'long' options use two-dashes.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Kyle M Hall 2016-04-29 16:58:41 UTC
Pushed to master for Koha 16.05, thanks Marcel!