https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35461 Anu Seilonen <anu.seilonen@ouka.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |anu.seilonen@ouka.fi -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35461 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=35461 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 160420 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160420&action=edit Bug 35461: Fix function call parameters causing the HASH to show in the renew all responses "Renew All" -function aka. 65-66-messages work like this: 66-return messages has renewed items in BM-fields and non-renewables go into BN-fields. If all items were renewable, then the reply message looks like: Reply: 13.07.2022 10:06:17:530 OUTESTI1 6610005000020220713 100616AOOUPK|BM564N15977171|BM564N09203960|BM564N23261720|BM564N25161958|BM564N23455012|C4::SIP::SIPServer=HASHC4::SIP::SIPServer=HASHAY6AZCEFE If items include renewables and non-renewables, the response includes the renewed items first, then a HASH mixed into the first BN-field, then rest of the renewed ones like they should show in BN-fields and after that, anohter HASH: 13.07.2022 14:41:32:610 OUTESTI1 6610003000420220713 144130AOOUPK|BM564N23455012|BM564N23261720|BM564N25161958|C4::SIP::SIPServer=HASHBN564N09203960|BN564N04191259|BN564N15977171|BN564N13788332|C4::SIP::SIPServer=HASHAY8AZC7C5 If all loans were unrenewable (this test included just one item), you first get a HASH in a BN-field, and another at the end. 13.07.2022 14:49:00:037 OUTESTI1 6610000000120220713 144858AOOUPK|C4::SIP::SIPServer=HASHBN564N19874533|C4::SIP::SIPServer=HASHAY9AZDD3B Our self checkout/checkin machine can handle/parse these reply messages correctly, but this might not be the case with all self checkout/checkin devices. Test Plan: 1) Unit test patch 2) prove t/db_dependent/SIP/Message.t 3) Note failures 4) Apply this patch 5) prove t/db_dependent/SIP/Message.t 6) Tests pass! Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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=35461 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #4 from David Nind <david@davidnind.com> --- The tests pass for me with only the 'Add unit tests' patch. The test plan says that it shouldn't. Is this an issue for sign off (they also pass after applying all the patches)? Note: This was on master. I tried on 22.11, but the patch doesn't apply, prove -v t/db_dependent/SIP/Message.t t/db_dependent/SIP/Message.t .. 1..18 # Subtest: Testing Patron Status Request V2 1..13 ok 1 - At least we got a response. ok 2 - Response code fine ok 3 - Verified institution id ok 4 - Verified patron id ok 5 - Verified patron name ok 6 - Verified code BL ok 7 - Verified code CQ ok 8 - Verified non-empty screen message ok 9 - Verified code CQ for wrong pw ok 10 - code CQ should be N for empty AD ok 11 - Verified code BL for wrong cardnumber ok 12 - Name should be empty now ok 13 - But we have a screen msg ok 1 - Testing Patron Status Request V2 # Subtest: Testing Patron Info Request V2 1..32 ok 1 - At least we got a response. ok 2 - Response code fine ok 3 - Verified institution id ok 4 - Verified patron id ok 5 - Verified patron name ok 6 - Verified code BL ok 7 - Verified code CQ ok 8 - Checked existence of fee limit ok 9 - Address in BD ok 10 - Verified email in BE ok 11 - Verified home phone in BF ok 12 - We have a screen msg ok 13 - Check customized patron name ok 14 - Home address successfully stripped from response ok 15 - Email address successfully stripped from response ok 16 - Home phone successfully stripped from response ok 17 - Date of birth successfully stripped from response ok 18 - Home address successfully stripped from response ok 19 - Email address successfully retained in response ok 20 - Home phone successfully retained in from response ok 21 - Date of birth successfully stripped from response ok 22 - Home address successfully stripped from response ok 23 - Email address successfully stripped from response ok 24 - Home phone successfully retained in from response ok 25 - Date of birth successfully stripped from response ok 26 - code CQ should be N for empty AD ok 27 - code CQ should be Y if empty AD allowed ok 28 - Patron account not locked already ok 29 - Patron account is not locked by patron info messages with empty password ok 30 - Verified code BL for wrong cardnumber ok 31 - Name should be empty now ok 32 - But we have a screen msg ok 2 - Testing Patron Info Request V2 # Subtest: Checkout V2 1..8 ok 1 - Check screen msg ok 2 - Item was not checked out (prevcheckout_block_checkout enabled) ok 3 - Item was checked out (prevcheckout_block_checkout disabled) ok 4 - Found AH field as timestamp in response ok 5 - Found AH field as SQL date in response ok 6 - Check screen msg ok 7 - Item was not checked out (item type matched blocked_item_types) ok 8 - Item was checked out successfully ok 3 - Checkout V2 # Subtest: Test checkout desensitize 1..6 ok 1 - Desensitize flag was not set for patron category in inhouse_patron_categories ok 2 - Desensitize flag was set for patron category not in inhouse_patron_categories ok 3 - Desensitize flag was set for empty inhouse_patron_categories ok 4 - Desensitize flag was not set for itemtype in inhouse_item_types ok 5 - Desensitize flag was set for item type not in inhouse_item_types ok 6 - Desensitize flag was set for empty inhouse_item_types ok 4 - Test checkout desensitize # Subtest: Test renew desensitize 1..6 ok 1 - Desensitize flag was not set for patron category in inhouse_patron_categories ok 2 - Desensitize flag was set for patron category not in inhouse_patron_categories ok 3 - Desensitize flag was set for empty inhouse_patron_categories ok 4 - Desensitize flag was set for item type not in inhouse_item_types ok 5 - Desensitize flag was set for empty inhouse_item_types ok 6 - Desensitize flag was not set for itemtype in inhouse_item_types ok 5 - Test renew desensitize # Subtest: Test renew desensitize 1..3 ok 1 - Found corrent BM for item renewed successfully ok 2 - Found corrent BN for item not renewed ok 3 - String 'C4::SIP::SIPServer' not found in reponse ( Bug 35461 ) ok 6 - Test renew desensitize # Subtest: Checkin V2 1..40 ok 1 - Checkin of invalid item with two warnings ok 2 - Response code fine ok 3 - OK flag is false ok 4 - Alert flag is set ok 5 - Check screen msg ok 6 - Check that AQ is in the response ok 7 - OK flag is false when checking in an item that was not checked out ok 8 - Alert flag is set ok 9 - Check screen msg ok 10 - OK flag is true now with checked_in_ok flag set when checking in an item that was not checked out ok 11 - Alert flag no longer set ok 12 - No screen msg ok 13 - Alert flag is set with check_in_ok, item is checked in but needs transfer ok 14 - Got FID_ALERT_TYPE (CV) field with value 04 ( needs transfer ) ok 15 - No FID_ALERT_TYPE (CV) field ok 16 - FID_ALERT_TYPE (CV) field is 00 ok 17 - Checkin without CV triggers alert flag when cv_triggers_alert is off ok 18 - Checkin without CV does not trigger alert flag when cv_triggers_alert is on ok 19 - No FID_DESTINATION_LOCATION (CT) field ok 20 - FID_DESTINATION_LOCATION (CT) field is empty but present ok 21 - OK flag is false when we check in at the wrong branch and we do not allow it ok 22 - Alert flag is set ok 23 - Check screen msg ok 24 - DBIx error on duplicate issue_id ok 25 - OK flag is false when we encounter data corruption in old_issues ok 26 - Alert flag is set ok 27 - Check screen msg ok 28 - OK flag is true when we checkin after removing the duplicate ok 29 - Alert flag is not set ok 30 - Issue record is gone now ok 31 - Item is checked out ok 32 - Hold was created successfully ok 33 - OK flag is false when we check in an item on hold and we do not allow it ok 34 - Alert flag is set ok 35 - Item was not checked in ok 36 - Hold was not marked as found by SIP when holds_block_checkin enabled ok 37 - OK flag is true when we check in an item on hold and we allow it but do not capture it ok 38 - Alert flag is set ok 39 - Item was checked in ok 40 - Hold was not marked as found by SIP when holds_get_captured disabled ok 7 - Checkin V2 # Subtest: Test hold_patron_bcode 1..2 ok 1 - SIP item with no hold returns empty string ok 2 - maybe_add returns empty string for SIP item with no hold returns empty string ok 8 - Test hold_patron_bcode # Subtest: UseLocationAsAQInSIP syspref tests 1..2 ok 1 - When UseLocationAsAQInSIP is not set SIP item has permanent_location set to value of homebranch ok 2 - When UseLocationAsAQInSIP is set SIP item has permanent_location set to value of item permanent_location ok 9 - UseLocationAsAQInSIP syspref tests # Subtest: hold_patron_name() tests 1..3 ok 1 - SIP item with no hold returns empty string for patron name ok 2 - maybe_add returns empty string for SIP item with no hold returns empty string ok 3 - maybe_add will create the field of the string '0' ok 10 - hold_patron_name() tests # Subtest: Lastseen response 1..6 ok 1 - At least we got a response. ok 2 - Response code fine ok 3 - Last seen not updated if not tracking patrons ok 4 - At least we got a response. ok 5 - Response code fine ok 6 - Last seen updated if tracking patrons ok 11 - Lastseen response # Subtest: Test patron_status_string 1..9 ok 1 - Found 2 checkouts for this patron ok 2 - Found 2 lost checkouts for this patron ok 3 - lost_block_checkout = 0 does not block checkouts with 2 lost checkouts ok 4 - lost_block_checkout = 0 does not block checkouts with 2 lost checkouts ok 5 - lost_block_checkout = 1 does block checkouts with 2 lost checkouts ok 6 - lost_block_checkout = 2 does block checkouts with 2 lost checkouts ok 7 - lost_block_checkout = 3 does not block checkouts with 2 lost checkouts ok 8 - lost_block_checkout = 2, lost_block_checkout_value = 2 does not block checkouts with 2 lost checkouts where only 1 has itemlost = 2 ok 9 - lost_block_checkout = 2, lost_block_checkout_value = 2 does block checkouts with 2 lost checkouts where only 1 has itemlost = 2 ok 12 - Test patron_status_string # Subtest: Test build_additional_item_fields_string 1..2 ok 1 - Attribute field generated correctly with single param ok 2 - Attribute field generated correctly with multiple params ok 13 - Test build_additional_item_fields_string # Subtest: Test build_custom_field_string 1..2 ok 1 - Attribute field generated correctly with single param ok 2 - Attribute field generated correctly with multiple params ok 14 - Test build_custom_field_string # Subtest: Test cr_item_field 1..8 ok 1 - Found correct CR field in response ok 2 - datelastseen remains unchanged ok 3 - datelastseen updated ok 4 - item remains lost ok 5 - datelastseen updated ok 6 - item is no longer lost ok 7 - Found correct CR field in response ok 8 - Found correct CR field in response ok 15 - Test cr_item_field # Subtest: Patron info summary > 5 should not crash server 1..22 ok 1 - At least we got a response. ok 2 - Response code fine ok 3 - At least we got a response. ok 4 - Response code fine ok 5 - At least we got a response. ok 6 - Response code fine ok 7 - At least we got a response. ok 8 - Response code fine ok 9 - At least we got a response. ok 10 - Response code fine ok 11 - At least we got a response. ok 12 - Response code fine ok 13 - At least we got a response. ok 14 - Response code fine ok 15 - At least we got a response. ok 16 - Response code fine ok 17 - At least we got a response. ok 18 - Response code fine ok 19 - At least we got a response. ok 20 - Response code fine ok 21 - At least we got a response. ok 22 - Response code fine ok 16 - Patron info summary > 5 should not crash server # Subtest: SC status tests 1..2 ok 1 - At least we got a response. ok 2 - Dies if sip user cannot be found ok 17 - SC status tests # Subtest: test_allow_additional_materials_checkout 1..4 ok 1 - Check screen msg ok 2 - Item was not checked out (allow_additional_materials_checkout disabled) ok 3 - Check screen msg ok 4 - Item was checked out (allow_additional_materials_checkout enabled ok 18 - test_allow_additional_materials_checkout ok All tests successful. Files=1, Tests=18, 8 wallclock secs ( 0.04 usr 0.01 sys + 5.43 cusr 1.05 csys = 6.53 CPU) Result: PASS -- 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=35461 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160419|0 |1 is obsolete| | Attachment #160420|0 |1 is obsolete| | Attachment #160421|0 |1 is obsolete| | Attachment #160436|0 |1 is obsolete| | --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 160437 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160437&action=edit Bug 35461: Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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=35461 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to David Nind from comment #4)

The tests pass for me with only the 'Add unit tests' patch.

The test plan says that it shouldn't.

Is this an issue for sign off (they also pass after applying all the patches)?

Fixed the bad test and squashed those changes into the original patch set! -- 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=35461 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160437|0 |1 is obsolete| | Attachment #160438|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 160453 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160453&action=edit Bug 35461: Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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=35461 --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to David Nind from comment #9)

I hope I haven't confused things....

Nope, the unit test was missing a comma causing it to fail all the time and in my haste I didn't re-test the "fixed" version against the patch to ensure it passed afterward! It's ready to test again! -- 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=35461 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160453|0 |1 is obsolete| | --- Comment #13 from David Nind <david@davidnind.com> --- Created attachment 160496 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160496&action=edit Bug 35461: Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> 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=35461 --- Comment #15 from David Nind <david@davidnind.com> --- (In reply to Kyle M Hall from comment #12)

Nope, the unit test was missing a comma causing it to fail all the time and in my haste I didn't re-test the "fixed" version against the patch to ensure it passed afterward! It's ready to test again!

Those pesky commas! 8-; Thanks Kyle. Now 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=35461 Lari Strand <lmstrand@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160497|0 |1 is obsolete| | --- Comment #16 from Lari Strand <lmstrand@gmail.com> --- Created attachment 160685 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160685&action=edit Bug 35461: Fix function call parameters causing the HASH to show in the renew all responses "Renew All" -function aka. 65-66-messages work like this: 66-return messages has renewed items in BM-fields and non-renewables go into BN-fields. If all items were renewable, then the reply message looks like: Reply: 13.07.2022 10:06:17:530 OUTESTI1 6610005000020220713 100616AOOUPK|BM564N15977171|BM564N09203960|BM564N23261720|BM564N25161958|BM564N23455012|C4::SIP::SIPServer=HASHC4::SIP::SIPServer=HASHAY6AZCEFE If items include renewables and non-renewables, the response includes the renewed items first, then a HASH mixed into the first BN-field, then rest of the renewed ones like they should show in BN-fields and after that, anohter HASH: 13.07.2022 14:41:32:610 OUTESTI1 6610003000420220713 144130AOOUPK|BM564N23455012|BM564N23261720|BM564N25161958|C4::SIP::SIPServer=HASHBN564N09203960|BN564N04191259|BN564N15977171|BN564N13788332|C4::SIP::SIPServer=HASHAY8AZC7C5 If all loans were unrenewable (this test included just one item), you first get a HASH in a BN-field, and another at the end. 13.07.2022 14:49:00:037 OUTESTI1 6610000000120220713 144858AOOUPK|C4::SIP::SIPServer=HASHBN564N19874533|C4::SIP::SIPServer=HASHAY9AZDD3B Our self checkout/checkin machine can handle/parse these reply messages correctly, but this might not be the case with all self checkout/checkin devices. Test Plan: 1) Unit test patch 2) prove t/db_dependent/SIP/Message.t 3) Note failures 4) Apply this patch 5) prove t/db_dependent/SIP/Message.t 6) Tests pass! Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: lmstrand <lmstrand@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35461 --- Comment #18 from Lari Strand <lmstrand@gmail.com> --- Also tested with a Lyngsoe Systems self checkout/service device and it was happy. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35461 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Kyle M Hall from comment #19)

(In reply to Katrin Fischer from comment #17)

...

Love to see the sign-offs - Kyle, can we count you for QA on this one?

I wrote the patches so not unless you think it's sufficient :)

Good point :D I was only looking at the sign-off lines. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35461 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160496|0 |1 is obsolete| | --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 161501 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161501&action=edit Bug 35461: Add unit tests Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.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=35461 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 | Version|22.11 |master 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=35461 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA --- Comment #24 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Never mind, I fixed it inline. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35461 --- Comment #25 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! 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=35461 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|24.05.00,23.11.03 |24.05.00,23.11.03,23.05.09 released in| | --- Comment #27 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.09 -- You are receiving this mail because: You are watching all bug changes.