[Koha-bugs] [Bug 7408] Expire holds that have been waiting too long

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7408

--- Comment #19 from Kyle M Hall <kyle.m.hall at gmail.com> 2012-02-24 14:58:32 UTC ---
(In reply to comment #16)
> I can agree that having a unified, integrated and systemic way of keeping track
> of when holds expire is a good one.  This should be part of the Holds Rewrite
> project, as we'll be rethinking a great many aspects of the way we do Holds in
> Koha.

Agreed.

> I can also agree that it's better to allow a more granular approach to
> hold-not-picked-up fines than just the blanket charge.  This part of the
> feature was added for one particular ByWater customer, who had a very simple
> use case.  The idea was that if people were to start using the feature and
> demand more flexibility to it, it could later be expanded, but for just one
> library system, it wasn't worth developing more than necessary.  The cronjob
> solved both the specific need of that library system, and the larger issue of
> holds expiry, without having to muck around with the already-bloated Circ
> Matrix.

I can understand that. I guess my instinct when it comes to solutions is to not
focus on the specific problem, but to create an extensible system that happens
to solve that problem. When the CCFLS asked me to create a system to track
summer reading club membership, I created the Clubs & Services module instead.
I'm hoping to get it into Koha proper soon.

> As for your (current) patch, there is a typo in the subroutine name
> (_GetExpireResevesSQL is missing an 'r').  The subroutine is an interesting
> approach to the problem; it's not done like that elsewhere in Koha, so I'm not
> sure how to feel about it.

Good catch. I am really not a big fan of what I did there. The only reason I
used a separate subroutine like that was in an attempt to follow the DRY
principle. Since the sql was used in two subroutines, I didn't want a future
developer to introduce a bug by changing it in one sub but not in another.

> The patch also locks in the expiry date when the hold is marked as ready.  If
> the Max Pickup Delay were to change while a hold is waiting, then the new
> expiry date would not be calculated, leading to incorrect cancellation and
> possibly incorrect fees.  My cronjob-based patch uses the
> ReservesMaxPickupDelay that is current to when the expiry and possible fees are
> applied.

That is a really good point. The new version of the patch I just submitted
resolves this issue. It doesn't muck about with expiration dates at all.
Instead, when CancelExpiredReserves is called, it then looks for holds that
have been waiting for too long and cancels them as well.

-- 
Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.