[Bug 24800] New: Koha does incomplete checkin when no return date is provided
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Bug ID: 24800 Summary: Koha does incomplete checkin when no return date is provided Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: katrin.fischer@bsz-bw.de QA Contact: testopia@bugs.koha-community.org CC: colin.campbell@ptfs-europe.com, kyle@bywatersolutions.com, martin.renvoize@ptfs-europe.com We found that when a checkin is requested by the self check without a return date, Koha will check the item in, but fails to send a checkin response and crashes and create inconsistent/invalid data: - It will remove the issue from the patron account and move to old_issues. BUT: - the old_issues.returndate will be an invalid date: 0000-00-00 - there is no return recorded in action_logs - there is no reutrn recorded in statistics - The reading history will show an empty return date. We found this in the context of an 'cancel checkout' feature in a lot of libraries using hardware and software by one of the big international RFID providers. We identified several 100 of those over a period of 1.5 years. What happens? - Self check requests checkout - Koha responds that item was checked out - Self check tries to desensitize - Item has been removed from the self check already! - Self check requests checkin with cancel flag set to Y and return date empty - Koha doesn't respond, but dies This all happens within a second or less.
From the logs: Sip::MsgType::_initialize('Checkin', 'N20200303 151300 APXXXX|AOXXXX|ABb00000057|ACxxxxxx|BIY', 'CA18A18', '37', ...) Mar 3 15:13:00 xxxx xxxx[16798]: new ILS::Item('b00000057'): found with title 'Irrlicht und Feuer :' Mar 3 15:13:00 xxxx xxxx[16798]: raw_transport: shutting down
Note the BIY in the Checkin request. In some older 3M documentation I found the following explanation about the cancel functionality: "Checkin, Message 09 Cancel (Y or N) This field is set to N when a normal checkin of materials is being requested. This field shoudl be set to Y when a previous Checkout message failed. It is possible for a checkout command to fail. This problem occurs when a patron removes the item from the 3M selfCheck system creadle after it has been checked out by the ACS software but before the item has been desensitized by the 3M selfCheck system. If this happens, the 3M SelfCheck system would cancel the Checkout by sending a Checkin message with the cancel field set to Y. The ACS software can also use this feld process the checkin differently (for example don't count it in the Checkin statistics and possibly decrement the Checkout statistics, also) ... Return date In current applications this field is not used." The last bit is strange as other documentation I found clearly list it as a required field in the Checking message: Some possible solutions: 1) Deny checkin, keep the item on the account. - We don't support cancel checkout. - The patron might have items on their account, that they have left in the library. 2) Check-in item and use server time for return date - The patron might leave with an item not checkout out, especially if the library has no gates. - We wil have entries of this happening in like a split second, which will help diagnose the problem. 3) Implement the cancel checkin - This would probably be the hardest to do - we'd need to set some kind of flag that it was a cancelled checkout to indicate that there might have been an issue on the self check machine (maybe 2) + note in the logs?) It would be interesting if someone can tell if they also had issues with this and what they think the best solution would be. Koha SIP2 crashing and making incomplete database entries doesn't appear the right behaviour atm. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ulrich.kleiber@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |clemens.elmlinger@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 100549 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100549&action=edit Bug 24800: Add tests -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 100550 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100550&action=edit Bug 24800: Handle inexistent return date on self checking See bug 24800 comment 0 for a description of the problem. We do not want the SIP server to crash if it receives a checkin request with a return date that is not given. The option this patch chose is to parse it only if provided. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Katrin, do you think this patch fix correctly the problem you faced? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #3)
Katrin, do you think this patch fix correctly the problem you faced?
We tested the patch and we think it's a clear improvement. It's not a complete fix for us missing an "undo" feature that would allow us to highlight the problematic returns, but it prevents SIP from dying, which I think is the most important for now. I suggest to file a separate enhancement request after this fix is pushed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100550|0 |1 is obsolete| | --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102452 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102452&action=edit Bug 24800: Handle inexistent return date on self checking See bug 24800 comment 0 for a description of the problem. We do not want the SIP server to crash if it receives a checkin request with a return date that is not given. The option this patch chose is to parse it only if provided. Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100549|0 |1 is obsolete| | --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102455 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102455&action=edit Bug 24800: Add tests Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102452|0 |1 is obsolete| | --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 102456 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102456&action=edit Bug 24800: Handle inexistent return date on self checking See bug 24800 comment 0 for a description of the problem. We do not want the SIP server to crash if it receives a checkin request with a return date that is not given. The option this patch chose is to parse it only if provided. Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- As written before, this worked in our testing, but sadly the tests fail for me: t/db_dependent/SIP/Transaction.t .. 8/8 # Failed test 'Checkin should have been done successfully' # at t/db_dependent/SIP/Transaction.t line 323. # got: '1' # expected: '0' # Looks like you failed 2 tests of 8. t/db_dependent/SIP/Transaction.t .. 8/8 # Failed test 'do_checkin' # at t/db_dependent/SIP/Transaction.t line 324. # Looks like you failed 1 test of 8. t/db_dependent/SIP/Transaction.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/8 subtests Keeping the sign off, but could you take a look please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It passes for me, sample data using koha-testing-docker. Maybe you have a circ rule or a pref that is blocking the checkin? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Good question - not sure. Are there candidates? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- You should check the return of AddReturn in do_checkin C4/SIP/ILS/Transaction/Checkin.pm 75 my ($return, $messages, $issue, $borrower) = AddReturn($barcode, $branch, undef, $return_date); I bet $messages contains the useful info. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Joubu, the tests started to pass when I tried to add the debug... I believe it was just Plack/Cache or something? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Katrin Fischer from comment #12)
Hi Joubu, the tests started to pass when I tried to add the debug... I believe it was just Plack/Cache or something?
I cannot think of something that would cause such behaviour in these tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Jonathan Druart from comment #13)
(In reply to Katrin Fischer from comment #12)
Hi Joubu, the tests started to pass when I tried to add the debug... I believe it was just Plack/Cache or something?
I cannot think of something that would cause such behaviour in these tests.
Don't know, but I think if we have a QA person verify that the tests pass on another system it shoudl be ok. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102455|0 |1 is obsolete| | Attachment #102456|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 102613 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102613&action=edit Unit tests pass for me! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 102614 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102614&action=edit Bug 24800: Handle inexistent return date on self checking See bug 24800 comment 0 for a description of the problem. We do not want the SIP server to crash if it receives a checkin request with a return date that is not given. The option this patch chose is to parse it only if provided. Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102613|0 |1 is obsolete| | Attachment #102614|0 |1 is obsolete| | --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 102615 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102615&action=edit Bug 24800: Add tests Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 102616 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102616&action=edit Bug 24800: Handle inexistent return date on self checking See bug 24800 comment 0 for a description of the problem. We do not want the SIP server to crash if it receives a checkin request with a return date that is not given. The option this patch chose is to parse it only if provided. Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thx, Kyle! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102615|0 |1 is obsolete| | --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 102690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102690&action=edit Bug 24800: Add tests Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102616|0 |1 is obsolete| | --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 102691 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102691&action=edit Bug 24800: Handle inexistent return date on self checking See bug 24800 comment 0 for a description of the problem. We do not want the SIP server to crash if it receives a checkin request with a return date that is not given. The option this patch chose is to parse it only if provided. Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #23 from Joy Nelson <joy@bywatersolutions.com> --- unit test patch does not apply to 19.11.x Please rebase if needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 --- Comment #24 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104348 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104348&action=edit [19.11.x version] Bug 24800: Add tests Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|20.05.00 |20.05.00, 19.11.06 released in| | --- Comment #25 from Joy Nelson <joy@bywatersolutions.com> --- Thanks for the rebase. Backported to 19.11.x for 19.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24800 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #26 from Lucas Gass <lucas@bywatersolutions.com> --- unit tests wont apply to 19.05.x. I tried both the original unit test patch and the 19.11 rebase. No backport to 19.05.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org