My understanding is that both 'waiting' and 'in transit' holds are 'found', and in both cases suspending them should be disallowed. I haven't looked at the places in which this could happen, but the code implementing the checks in Koha::Hold clearly misses the use case. The method should be renamed to the new guidelines (->suspend) and it should raise exceptions in both cases.
Indeed, the condition exist template-side only. * in holds_table.inc (for request.pl) 177 [% IF SuspendHoldsIntranet %] 178 <td> 179 [% UNLESS ( hold.found ) %] * in holds.js (for "Check out", "Details" from the patron's module) 152 if ( oObj.found ) { 153 return ""; 154 } else if ( oObj.suspend == 1 ) { 155 return "<a class='hold-resume btn btn-default btn-xs' id='resume" + oObj.reserve_id + "'>" 156 + "<i class='fa fa-play'></i> " + RESUME + "</a>"; 157 } else { 158 return "<a class='hold-suspend btn btn-default btn-xs' id='suspend" + oObj.reserve_id + "'>" 159 + "<i class='fa fa-pause'></i> " + SUSPEND + "</a>"; 160 } Other calls to ModReserve do not suspend/resume, except the one from Koha/REST/V1/Hold.pm.
Created attachment 84409 [details] [review] Bug 21478: Add Koha::Exceptions::Hold
Created attachment 84410 [details] [review] Bug 21478: Unit tests
Created attachment 84411 [details] [review] Bug 21478: Make Koha::Hold->suspend reject found holds
Created attachment 84432 [details] [review] Bug 21478: Add Koha::Exceptions::Hold Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84433 [details] [review] Bug 21478: Unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84434 [details] [review] Bug 21478: Make Koha::Hold->suspend reject found holds Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84456 [details] [review] Bug 21478: Add Koha::Exceptions::Hold Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 84457 [details] [review] Bug 21478: Unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 84458 [details] [review] Bug 21478: Make Koha::Hold->suspend reject found holds Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Awesome work all! Pushed to master for 19.05
Created attachment 84487 [details] [review] Bug 21478: (QA follow-up) Make sure tests don't fail randomly Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to 18.11.x for 18.11.03
pushed to 18.05.x for 18.05.09
Needs some work for 17.11.x. I'm not sure it is worth doing it.
(In reply to Fridolin SOMERS from comment #15) > Needs some work for 17.11.x. > I'm not sure it is worth doing it. No worries about it.