If a renewal via SIP would produce an error due to being on reserve, or exceeding maximum renewals, Koha's SIP2 implementation will refuse to renew the item even if the "no block" flag is set to Y.
It appears this happens for checkouts as well.
There are a number of bugs meaning that Koha does not implement no_block handling although some of the ILS routines appear to handle it. The formats parsing fixed fields are incorrect for single character flags such as no_block In the template they are defined as C which does not return a single character string but the integer value of that octet so where the code tests if ($no_block eq 'Y') that always fails because no_block == 89 If you fix the above issuing a non_block checkout calls a method checkout_no_block unfortunately no such method is present and the SIPServer instance crashes Will try fixing these and testing if we can then get the correct behaviour
Created attachment 37446 [details] [review] patch incorrect formats An initial patch correct the formats. But beware we dont want to deploy this until the affected handlers have been corrected, this bug has in effect been hiding a bigger one
Switching status to 'needs signoff'
Reverted back to assigned as a much more extensive patch is required. For Info the no block flag is placed when the unit uploads transaction which werte recorded when the unit was offline. It flags that these Checkins, Checkouts and Renewals have occured and cannot be refused. We need to address 3 layers of bugs in the existing code. 1. Interpret the flag correctly (this was addressed by the original patch) at present the code has errors in testing the flag. 2. Supply the missing methods. The current code calls no-existent methods if ho_block is set which would cause the server to abort. The missing methods need to be supplied 3. Make the appropriate checks in those methods. Currently no checks are made that the transaction is processed correctly e.g. a no block checkin my record a return from loan at 1pm but if the book was reissued at 2pm it is an error to apply that checkin to the second issue Working on a fuller patch to address these issues
Hi Colin, could you give an update here? Is the old patch something that could be reused or should we obsolete it?
(In reply to Katrin Fischer from comment #6) > Hi Colin, could you give an update here? Is the old patch something that > could be reused or should we obsolete it? think the old patch is still valid, I'll set up a test against a current system to confirm. I think some of the effects of the underlying problem are worked around by some later changes
Thx!
Created attachment 137578 [details] [review] Bug 12225: Fix SIP message templates If a renewal via SIP would produce an error due to being on reserve, or exceeding maximum renewals, Koha's SIP2 implementation will refuse to renew the item even if the "no block" flag is set to Y.
Do you understand the patch here Kyle.. must admit it's way over my head ;-|
Created attachment 137622 [details] [review] Bug 12225: Re-implement no block flag for checkouts Test Plan ( assuming Koha Testing Docker or kohadevbox ): 1) Check out master 2) Start the SIP server ( edit the SIP config koha instutution to be "CPL" instead of "kohalibrary" ) 3) Telnet to 6001 4) Send 9300CNkoha|COkoha|CPCPL| 5) Send 11YY20220711 16350220250711 163502AOCPL|AA23529000035676|AB39999000001396|ACkoha|BON|BIN| 6) Note the due date for the checkout in Koha is not in the year 2025: Henry Acevedo (23529000035676) checked out Philippics. by Cicero, Marcus Tullius. 39999000001396 7) Apply this patch set 8) Restart all the things! 9) Check in the checkout 10) Repeat steps 3 through 5 11) Note the due date is now in 2025!
Created attachment 137655 [details] [review] Bug 12225: Fix SIP message templates If a renewal via SIP would produce an error due to being on reserve, or exceeding maximum renewals, Koha's SIP2 implementation will refuse to renew the item even if the "no block" flag is set to Y. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 137656 [details] [review] Bug 12225: Re-implement no block flag for checkouts Test Plan ( assuming Koha Testing Docker or kohadevbox ): 1) Check out master 2) Start the SIP server ( edit the SIP config koha instutution to be "CPL" instead of "kohalibrary" ) 3) Telnet to 6001 4) Send 9300CNkoha|COkoha|CPCPL| 5) Send 11YY20220711 16350220250711 163502AOCPL|AA23529000035676|AB39999000001396|ACkoha|BON|BIN| 6) Note the due date for the checkout in Koha is not in the year 2025: Henry Acevedo (23529000035676) checked out Philippics. by Cicero, Marcus Tullius. 39999000001396 7) Apply this patch set 8) Restart all the things! 9) Check in the checkout 10) Repeat steps 3 through 5 11) Note the due date is now in 2025! Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using koha-testing-docker) 1. To install telnet: apt-get install telnet 2. Restart the SIP server: koha-sip --restart kohadev 3. To access SIP server using telnet: telnet 127.0.0.1 6001 4. To exit telnet: CTRL+] and then quit
Created attachment 137683 [details] [review] Bug 12225: Use ProcessOfflineIssue
David, can you test again with the latest patch? It switches to using the offline circ checkout code. The idea behind "no block" is the patron already has it. It's basically SIP offline circ ( aka store and forward ) where the patron already has it so we'd better check it out to them! It makes sense to use the same code for both.
(In reply to Kyle M Hall from comment #16) > David, can you test again with the latest patch? It switches to using the > offline circ checkout code. The idea behind "no block" is the patron already > has it. It's basically SIP offline circ ( aka store and forward ) where the > patron already has it so we'd better check it out to them! It makes sense to > use the same code for both. Hi Kyle. I have retested When I post the command in step 5 with the patches applied and after restarting everything: - Telnet exits saying: Connection closed by foreign host. - The item is not checked out. Commands entered after applying patches, flush_memcached, restart_all, and koha-sip --restart kohadev: telnet 127.0.0.1 6001 Trying 127.0.0.1... Connected to 127.0.0.1. Escape character is '^]'. 9300CNkoha|COkoha|CPCPL| 941 11YY20220711 16350220250711 163502AOCPL|AA23529000035676|AB39999000001396|ACkoha|BON|BIN| Connection closed by foreign host. From the sip.log: 87 [2022/07/13 21:16:28] [4660] [DEBUG] [undef]@[undef]: Configuration::find_service: Trying 6001/tcp C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 88 [2022/07/13 21:16:28] [4660] [DEBUG] [undef]@[undef]: Configuration::find_service: Trying *:6001/tcp C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 89 [2022/07/13 21:16:28] [4660] [DEBUG] [undef]@[undef]: Configuration::find_service: Trying 127.0.0.1:6001/tcp C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 90 [2022/07/13 21:16:28] [4660] [DEBUG] [undef]@[undef]: raw_transport: timeout is 60 C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 91 [2022/07/13 21:16:43] [4660] [DEBUG] [undef]@[undef]: read_request trimmed 1 character(s) C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 92 [2022/07/13 21:16:43] [4660] [INFO] [undef]@[undef]: INPUT MSG: '9300CNkoha|COkoha|CPCPL' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 93 [2022/07/13 21:16:43] [4660] [DEBUG] [undef]@[undef]: Sip::MsgType::new('C4::SIP::Sip::MsgType', '9300CNkoha...', '93'): seq.no '0', protocol 2 C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 94 [2022/07/13 21:16:43] [4660] [DEBUG] [undef]@[undef]: Sip::MsgType::_initialize('Login', '00CNkoha|COkoha|CPCPL', 'A1A1', '2', ...) C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 95 [2022/07/13 21:16:43] [4660] [INFO] [undef]@[undef]: Successful login/auth for 'koha' of 'CPL' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 96 [2022/07/13 21:16:43] [4660] [DEBUG] [undef]@[undef]: login_core: $VAR1 = 'ILS'; 97 C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 98 [2022/07/13 21:16:43] [4660] [DEBUG] [undef]@[undef]: new ILS 'CPL' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 99 [2022/07/13 21:16:43] [4660] [INFO] [undef]@[undef]: OUTPUT MSG: '941' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 100 [2022/07/13 21:16:43] [4660] [DEBUG] koha@127.0.0.1: raw_transport: uname/inst: 'koha/CPL' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 101 [2022/07/13 21:16:56] [4660] [DEBUG] koha@127.0.0.1: read_request trimmed 2 character(s) C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 102 [2022/07/13 21:16:56] [4660] [INFO] koha@127.0.0.1: INPUT MSG: '11YY20220711 16350220250711 163502AOCPL|AA23529000035676|AB39999000001396|ACkoha|BON|BIN' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 103 [2022/07/13 21:16:56] [4660] [DEBUG] koha@127.0.0.1: Sip::MsgType::new('C4::SIP::Sip::MsgType', '11YY202207...', '11'): seq.no '0', protocol 2 C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 104 [2022/07/13 21:16:56] [4660] [DEBUG] koha@127.0.0.1: Sip::MsgType::_initialize('Checkout', 'YY20220711 16350220250711 163502AOCPL|AA2 3529000035676|AB39999000001396|ACkoha|BON|BIN', 'A1A1A18A18', '38', ...) C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 105 [2022/07/13 21:16:56] [4660] [WARN] koha@127.0.0.1: received no-block checkout from terminal 'koha' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 106 [2022/07/13 21:16:56] [4660] [DEBUG] koha@127.0.0.1: new ILS::Patron(23529000035676): found patron '23529000035676' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 107 [2022/07/13 21:16:56] [4660] [DEBUG] koha@127.0.0.1: new ILS::Item('39999000001396'): found with title 'Philippics.' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) 108 [2022/07/13 21:16:56] [4660] [DEBUG] koha@127.0.0.1: ILS::Transaction::Checkout performing checkout... C4::SIP::Sip::siplog /kohadevbox/koh a/C4/SIP/Sip.pm (220) 109 [2022/07/13 21:16:56] [4660] [INFO] koha@127.0.0.1: raw_transport: shutting down C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (220) I'm assuming all three patches should be applied.
Created attachment 137712 [details] [review] Bug 12225: Use ProcessOfflineIssue
(In reply to David Nind from comment #17) > I have retested > When I post the command in step 5 with the patches applied and after > restarting everything: > - Telnet exits saying: Connection closed by foreign host. > - The item is not checked out. This new version of the patch should fix that! Would you mind giving it another test David? Thanks!
Created attachment 137713 [details] [review] Bug 12225: Use ProcessOfflineIssue Signed-off-by: David Nind <david@davidnind.com>
Created attachment 139684 [details] [review] Bug 12225: Fix SIP message templates If a renewal via SIP would produce an error due to being on reserve, or exceeding maximum renewals, Koha's SIP2 implementation will refuse to renew the item even if the "no block" flag is set to Y. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 139685 [details] [review] Bug 12225: Re-implement no block flag for checkouts Test Plan ( assuming Koha Testing Docker or kohadevbox ): 1) Check out master 2) Start the SIP server ( edit the SIP config koha instutution to be "CPL" instead of "kohalibrary" ) 3) Telnet to 6001 4) Send 9300CNkoha|COkoha|CPCPL| 5) Send 11YY20220711 16350220250711 163502AOCPL|AA23529000035676|AB39999000001396|ACkoha|BON|BIN| 6) Note the due date for the checkout in Koha is not in the year 2025: Henry Acevedo (23529000035676) checked out Philippics. by Cicero, Marcus Tullius. 39999000001396 7) Apply this patch set 8) Restart all the things! 9) Check in the checkout 10) Repeat steps 3 through 5 11) Note the due date is now in 2025! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 139686 [details] [review] Bug 12225: Use ProcessOfflineIssue Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
OK, this took a while to wrap my head around fully but I do agree with the changes now I have. QA scripts are happy and existing tests are still passing.. however.. I think we should be covering this change with new tests, please. Failing QA for lack of tests.
Created attachment 139691 [details] [review] Bug 12225: Add unit tests
Created attachment 139706 [details] [review] Bug 12225: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice one.. PQA
Created attachment 139707 [details] [review] Bug 12225: Fix SIP message templates If a renewal via SIP would produce an error due to being on reserve, or exceeding maximum renewals, Koha's SIP2 implementation will refuse to renew the item even if the "no block" flag is set to Y. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 139708 [details] [review] Bug 12225: Re-implement no block flag for checkouts Test Plan ( assuming Koha Testing Docker or kohadevbox ): 1) Check out master 2) Start the SIP server ( edit the SIP config koha instutution to be "CPL" instead of "kohalibrary" ) 3) Telnet to 6001 4) Send 9300CNkoha|COkoha|CPCPL| 5) Send 11YY20220711 16350220250711 163502AOCPL|AA23529000035676|AB39999000001396|ACkoha|BON|BIN| 6) Note the due date for the checkout in Koha is not in the year 2025: Henry Acevedo (23529000035676) checked out Philippics. by Cicero, Marcus Tullius. 39999000001396 7) Apply this patch set 8) Restart all the things! 9) Check in the checkout 10) Repeat steps 3 through 5 11) Note the due date is now in 2025! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 139709 [details] [review] Bug 12225: Use ProcessOfflineIssue Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 139710 [details] [review] Bug 12225: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for upcoming 22.05.06 release
That one brings in conflicts plus the added tests don't pass on 21.11.x if I fix the conflicts. Could you add backport patches for 21.11.x if that is needed? If that is needed I could spend more time on it but I won't unless you ask. Arthur
funny. checkout_no_block doesn't exist since forever, and this patch fixes it after all these years... but no fix for checkin_no_block. Any 09Y call crashes and die.
(In reply to Blou from comment #35) > funny. checkout_no_block doesn't exist since forever, and this patch fixes > it after all these years... but no fix for checkin_no_block. Any 09Y call > crashes and die. Bug 32515 might help.