Bug 13315 Changed the return of C4::Circulation::AddIssue to a hash reflecting the added issue ; However if AddIssue is called with the borrower currently having the item on loan a renewal is carried out nut as no issue is created in the db undef is returned rather than the issue structure. The same bug made the first thing the sipserver does is to extract the due_date from that non-existent structure causing a fatal error if it is a renewal. This is potentially a major bug as almost all self-issue units use checkout requests rather than renewal requests when renewing borrowers items
Created attachment 41452 [details] [review] Proposed patch Not the most elegant solution, but this should allow renewals to proceed without causing 'Communications Error' at the client end
Upping priority as this needs applying to releases from 3.18.7 to stop communication errors on renew. The change causing the error was bug 13315
Created attachment 42355 [details] [review] Bug 14673 Work around change to AddIssue return Return from AddIssue used to be due date or undef. Now it is less straightforward returning am issue object if an issue row is created or undef. If the issue is a renewal undef is returned. As that case was not handled properly it caused the server site to crash the listener causing a communications error on the client. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> We have this patch in production, it does solve the issue
Colin, Wouldn't it possible to move the duedatefromissue subroutine to a method in C4::SIP::ILS::Transaction? It'd avoid c/p code.
Colin: What do you think of the proposal from Jonathan? Jonathan: Could you provide a counter patch? Avoiding copypasta sounds like a good idea.
(In reply to Magnus Enger from comment #5) > Colin: What do you think of the proposal from Jonathan? > Jonathan: Could you provide a counter patch? Avoiding copypasta sounds like > a good idea. No, I would like to keep my QA token.
(In reply to Magnus Enger from comment #5) > Colin: What do you think of the proposal from Jonathan? > Jonathan: Could you provide a counter patch? Avoiding copypasta sounds like > a good idea. (In reply to Jonathan Druart from comment #6) > (In reply to Magnus Enger from comment #5) > > Colin: What do you think of the proposal from Jonathan? > > Jonathan: Could you provide a counter patch? Avoiding copypasta sounds like > > a good idea. > > No, I would like to keep my QA token. Its not really a transaction method but a utility but will attempt to test that solution
Created attachment 43014 [details] [review] Revised patch Revised to remove duplication of method
Revised patch to inherit method from base class - works for me with Checkin and Renewal requests - please test
Created attachment 44328 [details] [review] Bug 14673 Work around change to AddIssue return Return from AddIssue used to be due date or undef. Now it is less straightforward returning am issue object if an issue row is created or undef. If the issue is a renewal undef is returned. As that case was not handled properly it caused the server site to crash the listener causing a communications error on the client. Signed-off-by: Frederic Demians <f.demians@tamil.fr>
Created attachment 44548 [details] [review] [PASSED QA] Bug 14673 Work around change to AddIssue return Return from AddIssue used to be due date or undef. Now it is less straightforward returning am issue object if an issue row is created or undef. If the issue is a renewal undef is returned. As that case was not handled properly it caused the server site to crash the listener causing a communications error on the client. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patch pushed to master. Thanks Colin!
This patch has been pushed to 3.20.x, will be in 3.20.6.
(In reply to Colin Campbell from comment #2) > Upping priority as this needs applying to releases from 3.18.7 to stop > communication errors on renew. The change causing the error was bug 13315 I see that this has been pushed to 3.20.x, can we get it back-ported to 3.18.x?
(In reply to Barton Chittenden from comment #14) > (In reply to Colin Campbell from comment #2) > > Upping priority as this needs applying to releases from 3.18.7 to stop > > communication errors on renew. The change causing the error was bug 13315 > > I see that this has been pushed to 3.20.x, can we get it back-ported to > 3.18.x? Patch as is should apply to 3.18.x
Pushed to 3.18.13, and released.