[Bug 23548] New: AQ field required in checkin response
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Bug ID: 23548 Summary: AQ field required in checkin response Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: stefan.berndtsson@ub.gu.se QA Contact: testopia@bugs.koha-community.org CC: colin.campbell@ptfs-europe.com When doing checkin through SIP, the AQ field is a required field in the response (code 10), however it is only provided when the item checked in is found. There should be an empty "AQ|" in the return message when the item is not found to not violate the required field status. For context, this is an actual issue with the checkin stations from PV-SUPA when you want them to accept any book, even if they're not found by Koha. It gives an error, and refuses to handle the checkin further. -- 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=23548 --- Comment #1 from David Gustafsson <glasklas@gmail.com> --- Created attachment 128657 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128657&action=edit Bug 23548: Set AQ to empty when item not exist -- 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=23548 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |glasklas@gmail.com -- 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=23548 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #2 from David Nind <david@davidnind.com> --- I'm not familiar with SIP2, but I would like to test. Are you able to provide a test plan similar to bug 26871 or bug 20517? Ideally with the commands to give, and the responses expected. Thanks! -- 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=23548 --- Comment #3 from Stefan Berndtsson <stefan.berndtsson@ub.gu.se> --- (In reply to David Nind from comment #2)
I'm not familiar with SIP2, but I would like to test.
I don't have an instance without the patch near me at the moment that can run SIP commands, but it should be something like (tweak credentials and such as needed): Barcode 123123123 is a barcode that is not in the database. 1. Have SIP-server running 2. ./sip_cli_emulator.pl -m checkin -a localhost -p 6001 -l 123 -su sipuser -sp sippass -t CR --item 123123123 3. See that AQ is missing completely. 4. Apply patch 5. Run same command again. 6. See that AQ| is present. That is the expected situation, but as mentioned, I haven't tested the non-patched version. -- 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=23548 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- 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=23548 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128657|0 |1 is obsolete| | --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 135533 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135533&action=edit Bug 23548: Set AQ to empty when item not exist Signed-off-by: David Nind <david@davidnind.com> -- 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=23548 --- Comment #5 from David Nind <david@davidnind.com> --- Thanks Stefan! With koha-testing-docker, SIP2 is already configured. SIP user and password is term1. Changed step 2 from comment #3 to ./misc/sip_cli_emulator.pl -m checkin -a localhost -p 6001 -l 123 -su term1 -sp term1 -t CR --item 123123123 Output before patch applied: Attempting socket connection to localhost:6001...connected! Use of uninitialized value $hold_mode in string eq at ./misc/sip_cli_emulator.pl line 120. SEND: 9300CNterm1|COterm1|CP123| READ: 941 Trying 'checkin' SEND: 09N20220601 10213620220601 102136AP123|AO123|AB123123123|ACterm1|BIN| READ: 100NUY20220601 102136AO123|AB123123123|CV99|AFInvalid Item| Output after patch applied: Attempting socket connection to localhost:6001...connected! Use of uninitialized value $hold_mode in string eq at ./misc/sip_cli_emulator.pl line 120. SEND: 9300CNterm1|COterm1|CP123| READ: 941 Trying 'checkin' SEND: 09N20220601 10232820220601 102328AP123|AO123|AB123123123|ACterm1|BIN| READ: 100NUY20220601 102329AO123|AB123123123|AQ|CV99|AFInvalid Item| Last line now as |AQ| -- 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=23548 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |per.larsson@ub.gu.se |ity.org | -- 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=23548 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135533|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135732 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135732&action=edit Bug 23548: Set AQ to empty when item not exist Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |martin.renvoize@ptfs-europe | |.com --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This works as described, but I'm not sure I follow the workflow.. how can you check in an item that Koha can't find? There's clearly a need, even if I don't fully understand the real-life situation this can happen. I'd love to see a unit test cover this change however, so right now it's failing on that front.. please provide a unit test and then we can pass. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 --- Comment #8 from Stefan Berndtsson <stefan.berndtsson@ub.gu.se> --- (In reply to Martin Renvoize from comment #7)
This works as described, but I'm not sure I follow the workflow.. how can you check in an item that Koha can't find?
The point is that you can't, but the resulting reply (failed to find the item), is required to have the AQ field present (empty) according to the specification. Machines from PV-SUPA crashes when the reply does not contain that required field. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm convinced, and looked up the specs to confirm. Now we just need a unit test to protect us from later regressions ;-). Are you able to submit that, of should I add it to my list to try and get to soon? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 --- Comment #10 from Stefan Berndtsson <stefan.berndtsson@ub.gu.se> ---
Are you able to submit that, of should I add it to my list to try and get to soon?
Unfortunately I'm currently tied up with other urgent things here before my vacation, so I don't see myself having time within a 6-8 weeks right now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 --- Comment #11 from Stefan Berndtsson <stefan.berndtsson@ub.gu.se> --- (In reply to Martin Renvoize from comment #9)
I'm convinced, and looked up the specs to confirm. Now we just need a unit test to protect us from later regressions ;-).
Are you able to submit that, of should I add it to my list to try and get to soon?
I'm unable to run the existing tests for two reasons. 1. I get: DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_koha`.`checkout_renewals`, CONSTRAINT `renewals_renewer_id` FOREIGN KEY (`renewer_id`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE) 2. test_checkout_v2 assumes that circulation rules are set in some specific way in the database and does not mock those values in the test. (This may apply to other tests as well). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Some of the tests are not as independent as they should be. Ususally we assume all tests need to pass on a standard sample database as provided by ktd or kohadevbox. You could also probably download the SQL with the data for testing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 --- Comment #13 from Matthias Meusburger <matthias.meusburger@biblibre.com> --- Created attachment 154557 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154557&action=edit Bug 23548: Unit test - Set AQ to empty when item not exist -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Matthias Meusburger <matthias.meusburger@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |matthias.meusburger@biblibr | |e.com Status|Failed QA |Signed Off --- Comment #14 from Matthias Meusburger <matthias.meusburger@biblibre.com> --- Hello! I've added a unit test for this patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154557|0 |1 is obsolete| | --- Comment #15 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 155046 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155046&action=edit Bug 23548: Unit test - Set AQ to empty when item not exist Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135732|0 |1 is obsolete| | --- Comment #16 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 155047 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155047&action=edit Bug 23548: Set AQ to empty when item not exist Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 --- Comment #17 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 155048 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155048&action=edit Bug 23548: (QA follow-up) tidy up code -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net Status|Signed Off |Passed QA --- Comment #18 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Thanks Matthias for the tests :) Martin had validated almost everything, so I'm just completing the missing stuff and doing some double checking: Test code ok and it works as expected to force having the implementation do it's job. Implementation code looks good. QA script happy with the follow-up. passing QA, great teamwork, thanks all! :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155046|0 |1 is obsolete| | Attachment #155047|0 |1 is obsolete| | Attachment #155048|0 |1 is obsolete| | --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 155131 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155131&action=edit Bug 23548: Unit test - Set AQ to empty when item not exist Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> 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=23548 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155131|0 |1 is obsolete| | --- Comment #20 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 155132 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155132&action=edit Bug 23548: Unit test - Set AQ to empty when item not exist Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> 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=23548 --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 155133 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155133&action=edit Bug 23548: Set AQ to empty when item not exist Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> 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=23548 --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 155134 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155134&action=edit Bug 23548: (QA follow-up) tidy up code 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=23548 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fixes SIP return release notes| |messages so that there is | |an "AQ|" field, even if it | |is empty (this is a | |required field according to | |the specification, and some | |machines (such as PV-SUPA) | |crash if it is not | |present). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.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=23548 --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Version(s)|23.11.00 |23.11.00,23.05.04 released in| | Status|Pushed to master |Pushed to stable --- Comment #24 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.11.00,23.05.04 |23.11.00,23.05.04,22.11.10 released in| | Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23548 --- Comment #25 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to oldstable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org