Created attachment 173603 [details] [review] Bug 38284: Add tests
Created attachment 173604 [details] [review] 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.
With just the first patch, the tests still pass for me. Is anything else required to trigger the failure?
(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!
Created attachment 173735 [details] [review] Bug 38284: Add tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 173736 [details] [review] 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>
(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!
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!
(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.
(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.
Created attachment 173922 [details] [review] Bug 38284: Add tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 173923 [details] [review] 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>
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.
Created attachment 174284 [details] [review] 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>
Created attachment 174285 [details] [review] 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>
Pushed for 24.11! Well done everyone, thank you!
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|
ps. Without a checksum in the 23 request nothing was returned without the patch, it seems.