[Bug 38284] New: handle_patron_status dies if patron not found
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Bug ID: 38284 Summary: handle_patron_status dies if patron not found Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: SIP2 Assignee: pedro.amorim@ptfs-europe.com Reporter: pedro.amorim@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: kyle@bywatersolutions.com, nick@bywatersolutions.com Depends on: 36605 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36605 [Bug 36605] TrackLastPatronActivity for SIP should track both patron status and patron information requests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #1 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 173603 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173603&action=edit Bug 38284: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #2 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 173604 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173604&action=edit Bug 38284: Add patron check for TrackLastPatronActivityTriggers If an invalid or empty cardnumber is supplied to patron status request SIP message, SIP dies silently and no 'READ:' exists on the response. Test plan: 1) Apply tests plan, run tests: $ prove t/db_dependent/SIP/Message.t 2) Verify tests fail. Apply 2nd patch. Run tests again. Verify they pass. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.field@ptfs-europe. | |com, | |martin.renvoize@ptfs-europe | |.com, | |pedro.amorim@ptfs-europe.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- With just the first patch, the tests still pass for me. Is anything else required to trigger the failure? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #4 from Jonathan Field <jonathan.field@ptfs-europe.com> --- (In reply to David Nind from comment #3)
With just the first patch, the tests still pass for me.
Is anything else required to trigger the failure?
Hi David, Yes, select some values from the TrackLastPatronActivityTriggers system preference. If none are selected it should work, if you select some, it should fail! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173603|0 |1 is obsolete| | --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 173735 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173735&action=edit Bug 38284: Add tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173604|0 |1 is obsolete| | --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 173736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173736&action=edit Bug 38284: Add patron check for TrackLastPatronActivityTriggers If an invalid or empty cardnumber is supplied to patron status request SIP message, SIP dies silently and no 'READ:' exists on the response. Test plan: 1) Apply tests plan, run tests: $ prove t/db_dependent/SIP/Message.t 2) Verify tests fail. Apply 2nd patch. Run tests again. Verify they pass. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #7 from David Nind <david@davidnind.com> --- (In reply to Jonathan Field from comment #4) ...
Yes, select some values from the TrackLastPatronActivityTriggers system preference. If none are selected it should work, if you select some, it should fail!
Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA QA Contact|testopia@bugs.koha-communit |aleisha@catalyst.net.nz |y.org | CC| |aleisha@catalyst.net.nz --- Comment #8 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Slight QA test fail: OK C4/SIP/Sip/MsgType.pm WARN t/db_dependent/SIP/Message.t WARN tidiness The file is less tidy than before (bad/messy lines before: 531, now: 539) Processing additional checks OK! * Was this bug sponsored by anyone? Running tests (1) * Proving /kohadevbox/koha/t/db_dependent/SIP/Message.t OK! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #9 from Jonathan Field <jonathan.field@ptfs-europe.com> --- (In reply to Aleisha Amohia from comment #8)
* Was this bug sponsored by anyone?
No, just came up as a bug reported by a library. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #10 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to David Nind from comment #3)
With just the first patch, the tests still pass for me.
Is anything else required to trigger the failure?
Hi David, thanks! This means I must revisit the test patch. I must've written and tested it with the syspref enabled on my local environment but this needs to be mocked on test instead. Thanks Jonathan for chipping in. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173735|0 |1 is obsolete| | --- Comment #11 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 173922 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173922&action=edit Bug 38284: Add tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173736|0 |1 is obsolete| | --- Comment #12 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 173923 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173923&action=edit Bug 38284: Add patron check for TrackLastPatronActivityTriggers If an invalid or empty cardnumber is supplied to patron status request SIP message, SIP dies silently and no 'READ:' exists on the response. Test plan: 1) Apply tests plan, run tests: $ prove t/db_dependent/SIP/Message.t 2) Verify tests fail. Apply 2nd patch. Run tests again. Verify they pass. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #13 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi all, thanks for looking here. I've squashed tidy fixes to the patches and revisited/rewritten the tests. They now should consistently fail when applied tests patch only, and consistently pass when fix patch is applied. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173922|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 174284 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174284&action=edit Bug 38284: Add tests Sponsored-by: PTFS Europe <https://ptfs-europe.com> 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=38284 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173923|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 174285 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174285&action=edit Bug 38284: Add patron check for TrackLastPatronActivityTriggers If an invalid or empty cardnumber is supplied to patron status request SIP message, SIP dies silently and no 'READ:' exists on the response. Test plan: 1) Apply tests plan, run tests: $ prove t/db_dependent/SIP/Message.t 2) Verify tests fail. Apply 2nd patch. Run tests again. Verify they pass. 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=38284 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> 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=38284 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|aleisha@catalyst.net.nz |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=38284 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.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=38284 --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Lari Strand <lmstrand@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lmstrand@gmail.com --- Comment #17 from Lari Strand <lmstrand@gmail.com> --- I noticed this issue too and actually without this patch the SIP server replies with a 96 message Request SC Resend with a checksum field (96AZFEF6 for example) when either cardnumber or pin is wrong. This patch not only fixes the problem with invalid cardnumbers but also when the pin is invalid and I no longer receive 96 messages as replies but 24 messages with this patch. 24 00020241115 140328AEtest test|AA66666666|BLY|CQN|BV0|AFGreetings from Koha. -- Invalid password|AOJOE_JOE| and 24YYYY 00020241115 140438AE|AA66666665|BLN|AFInvalid cardnumber|AOJOE_JOE| -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #18 from Lari Strand <lmstrand@gmail.com> --- ps. Without a checksum in the 23 request nothing was returned without the patch, it seems. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to main |Pushed to stable --- Comment #19 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Backported to 24.05.x for 24.05.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=38284 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.11.00 |24.11.00,24.05.06 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fixes patron status release notes| |SIP2 request responses if | |the request has an invalid | |or empty card number or | |PIN, and options are set | |for the | |TrackLastPatronActivityTrig | |gers system preference. The | |SIP request was silently | |dying. --- Comment #20 from David Nind <david@davidnind.com> --- I've added a release note, but I'm not sure that I correctly understand what the issue is and what was fixed. Please update the release note if I have not described this correctly! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #21 from Jonathan Field <jonathan.field@ptfs-europe.com> --- (In reply to David Nind from comment #20)
I've added a release note, but I'm not sure that I correctly understand what the issue is and what was fixed.
Please update the release note if I have not described this correctly!
I think you have described it perfectly, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #22 from David Nind <david@davidnind.com> --- (In reply to Jonathan Field from comment #21)
(In reply to David Nind from comment #20)
I've added a release note, but I'm not sure that I correctly understand what the issue is and what was fixed.
Please update the release note if I have not described this correctly!
I think you have described it perfectly, thanks!
Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.11.00,24.05.06 |24.11.00,24.05.06,23.11.11 released in| | Status|Pushed to oldstable |Pushed to oldoldstable CC| |fridolin.somers@biblibre.co | |m --- Comment #23 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Jesse Maseto <jesse@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldoldstable |RESOLVED Resolution|--- |FIXED CC| |jesse@bywatersolutions.com --- Comment #24 from Jesse Maseto <jesse@bywatersolutions.com> --- Not pushed to LTS. Marked Resolved. If you feel this should be in LTS please reply with your reason. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 --- Comment #25 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Jesse Maseto from comment #24)
Not pushed to LTS. Marked Resolved.
If you feel this should be in LTS please reply with your reason.
I believe this is a fixing an issue introduced by bug 36605. I've not tested and may be wrong but as bug 36605 has been pushed to 23.05.x, this should likely also be. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38284 Bug 38284 depends on bug 36605, which changed state. Bug 36605 Summary: TrackLastPatronActivity for SIP should track both patron status and patron information requests https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36605 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org