[Koha-bugs] [Bug 24194] Add system preference to disable the use of expiration dates for holds

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

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24194

--- Comment #46 from Joonas Kylmälä <joonas.kylmala at helsinki.fi> ---
(In reply to Aleisha Amohia from comment #39)
> (In reply to Joonas Kylmälä from comment #36)
> > ReserveExpiration should be moved out of set_waiting() method because it
> > makes using set_waiting method really confusing because you are really
> > expecting it to only set the waiting status and not do much more,
> > documentation and testing of the function becomes also more difficult. 
> 
> I'm not sure I understand what you mean here, expiration date logic is
> already in the set_waiting method before my patch. I think moving that logic
> out is beyond the scope of this work.

I mean just moving the ReserveExpiration syspref code you added. The rest of
the code is already in OPAC/INTRANET/REST (because that is required due the
nature of it) so having this check there wouldn't add any extra code. Also
please see below for more explanation.


Also note that this DB migration would be needed then:

> > ReservesMaxPickUpDelay value '' => 0 so then when ReservesMaxPickUpDelay is
> > not an integer value it would simply not add expiration date.


> > The best way to do this would be to block adding expiration date already when
> > creating a new hold. So please add the necessary code to OPAC, Staff and
> > REST. For OPAC and Staff I see there is already some code, REST is missing
> > completely. 
> 
> What code is missing for OPAC and staff? I will add it to REST in a
> follow-up.

You had the code to hide the HTML but if someone submits the request directly
it will work but the expiration date is silently dropped. I noticed you handled
it now this way as well in the REST API in the follow-up, IMHO there should be
error given (e.g. raise the previously mentioned exception in Koha::Hold).
Silently discarding data is going to lead to surprises for our API users
especially!

-- 
You are receiving this mail because:
You are watching all bug changes.