Grace Period In Database

[Thomas Berezansky]

As it stands the grace period for a transaction is only specified on the fine generator script, but fines can be generated on checkin as well without one. As a result any checkin results in grace periods being negated.

The attached patch adds in-db grace periods for all non-booking transactions, removing the grace period from the fine generator cron job script and placing it on the circulation. Grace periods are set on the recurring fine rule with an override available on the circ matchpoint, identical to how renewal count can be overridden from the duration rule already. These grace periods are respected on checkin fine generation as well as on the fine generator cron job script.

The upgrade script will assume, by default, a grace period of 1, but has a \set line to change this. It will also update all circulations without a checkin by default, but another \set will allow disabling of that.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

Signed-off-by: Thomas Berezansky <email address hidden>

[Thomas Berezansky (tsbere)]

Slightly updated patch. Upgrade script still had a Rollback from testing. Oops.

[Dan Wells (dbw2)]

Thanks for taking this on, it will mean a lot to those of us who need it. Having just given a read-through only (no actual testing yet), here are a few comments in no particular order:

1) In Circ/Circulate.pm, I don't think there is any need to add 'grace' to the @AUTOLOAD_FIELDS. I don't see it used anywhere.

2) In the same file around line 1837, I would probably use "$circ->grace($policy->{grace});" instead, since I am guessing part of the reason get_circ_policy() exists is to centralize any decision making needed for these values (there isn't any now for the 'grace' value, but maybe someday?).

3) It seems you have removed grace support completely from reservations. This is probably more a reminder, but I would think we will need it in there eventually to keep the current behavior from changing.

4) This is more a curiosity, but I am guessing your placement of the 'grace' column in the table def was to keep the INTs together? (Or maybe it was just easiest to copy-paste it in right there.) In any case, I think it is probably just chance that the table ended up being grouped like that, so I would probably put it next to the fine_interval. Hardly worth mentioning, but there it is.

So, nothing major sticks out at me, looking forward to the next version with 'grace_period' support and doing some testing.

Thanks again,
Dan

[Thomas Berezansky (tsbere)]

Dan, going down your list:

1) I was under the impression that I had to have it there for $circ->grace(value) to set grace to value. Am I mistaken?

2) Good point. Will do that on the grace_period version.

3) Yea. I haven't looked deeply enough into that part of the system to add to it yet. Would be a bit of a TODO, though. Should probably add comments in the code to that effect.

4) More of a "I picked renewals_remaining as a search value for making sure I got everywhere" thing, mainly because I knew I had an override from the circ matrix for it already.

[Dan Wells (dbw2)]

For 1) the autoload stuff is to add fields to the Circulator object being defined here, not the circulation Fieldmapper object which is automagically defined elsewhere. So you only need 'grace' in the AUTOLOAD_FIELDS if you intend to say something like '$self->grace' at some point, or if you need access to '->grace' wherever you create/use a Circulator object, like in run_method. As things stand, you don't need it.

[Thomas Berezansky (tsbere)]

New patch, grace_period is an interval that defines the entire grace period now, compared to an integer that defines a number of fine intervals. Upgrade script sets defaults to the recurrence interval on the recurring fine rule, and to the fine_interval on circulations, the latter can be disabled with a change to a \set line at the top of the script.

For those that want to play git instead of patch:
git://git.mvlcstaff.org/tsbere/ILS branch graceintervals

and gitweb link:
http://git.mvlcstaff.org/?p=tsbere/ILS.git;a=shortlog;h=refs/heads/graceintervals

[Thomas Berezansky (tsbere)]

Addition to previous patch of "only generate fines after grace period is over on checkin", requested in IRC.

[Thomas Berezansky (tsbere)]

Learned that $obj->something_invalid is bad. Use $obj->can('something_invalid') instead. In this case "something_invalid" is "grace_period" on a reservation instead of a circ.

[Thomas Berezansky (tsbere)]

Via IRC request:

For those who want to use git to pull my git branch:

To add my remote using id "tsbere" (only needs to be done once):

git remote add tsbere git://git.mvlcstaff.org/tsbere/ILS

If you don't use tsbere as the identifier here all of the below commands referring to tsbere will need to be adjusted.

Don't forget to fetch/update the remote:

git fetch tsbere
OR
git fetch --all

This branch will be tsbere/graceintervals

To check out the branch:

git checkout -b tsbere_graceintervals tsbere/graceintervals

To merge into your current branch (change branches first!):

git merge tsbere/graceintervals

To merge changes you made to your checked out version into the current branch (change branches first!):

git merge tsbere_graceintervals

[Jason Stephenson (jstephenson)]

Please target/nominate to series.