This came up with a SIP vendor recently - when reading responses they were only getting part of a patron's information. It seemed to have been caused by patrons with multiple restrictions and a new line in the response The simple fix was: <screen_msg_regex find="\n" replace=" -- " /> We should possibly add this as an option rather than relying on regex
Created attachment 171642 [details] [review] Bug 37582: Add ability to for SIP to convert control and separator characters to spaces This came up with a SIP vendor recently - when reading responses they were only getting part of a patron's information. It seemed to have been caused by patrons with multiple restrictions and a new line in the response Test Plan: 1) Add multiple debarments to a patron 2) Run a sip patron information response, note the presence of newlines 3) Apply this patch 4) Add the new option convert_control_characters to your SIP account, set the value to " -- " 5) Restart all the things! 7) Run a sip patron information response, note the newlines have been replaced!
Created attachment 171643 [details] [review] Bug 37582: Add ability to for SIP to convert control and separator characters to spaces This came up with a SIP vendor recently - when reading responses they were only getting part of a patron's information. It seemed to have been caused by patrons with multiple restrictions and a new line in the response This patch also does minor refactoring to write_msg to pass the server variable that contains all the previously passed parameters, and removes the never used "file" parameter. Test Plan: 1) Add multiple debarments to a patron 2) Run a sip patron information response, note the presence of newlines 3) Apply this patch 4) Add the new option convert_control_characters to your SIP account, set the value to " -- " 5) Restart all the things! 7) Run a sip patron information response, note the newlines have been replaced!
Created attachment 171644 [details] [review] Bug 37582: Tidy write_msg
I got pretty close to being able to test this, but I'm missing something. In /etc/koha/SIPconfig.xml I added the new option like this: <login id="term1" password="term1" delimiter="|" error-detect="enabled" institu tion="CPL" encoding="ascii" checked_in_ok="1" convert_nonprinting_characters=" -- "/> Then I restart_all and login to the SIP server and send a patron info request, but I still got a response with a newline after the first -- debarred message kohadev-koha@kohadevbox:koha(main)$ telnet localhost 6001 Trying ::1... Connection failed: Connection refused Trying 127.0.0.1... Connected to localhost. Escape character is '^]'. 9300CNterm1|COterm1|CPCPL| 941 6300020060329 201700Y AOCPL|AAbob|AD1234| 64YYYY 00020241017 205014000000010001000000000000AOCPL|AA23529000695412|AEBob Singleton|BLY|CQY|BHUSD|BV0|CC5|BD4554 Library Rd. Centerville, CA 44262|BF(212) 555-1212|PB19830415|PCPT|PIN|AFGreetings from Koha. -- debarred debarred again|
(In reply to Brendan Lawlor from comment #4) > In /etc/koha/SIPconfig.xml I added the new option like this: > <login id="term1" password="term1" delimiter="|" error-detect="enabled" > institu tion="CPL" encoding="ascii" checked_in_ok="1" > convert_nonprinting_characters=" -- "/> I think you may be updating the wrong file. For koha-testing-docker the file to update would be /etc/koha/sites/kohadev/SIPconfig.xml 🤞
Created attachment 173081 [details] [review] Bug 37582: Add ability to for SIP to convert control and separator characters to spaces This came up with a SIP vendor recently - when reading responses they were only getting part of a patron's information. It seemed to have been caused by patrons with multiple restrictions and a new line in the response This patch also does minor refactoring to write_msg to pass the server variable that contains all the previously passed parameters, and removes the never used "file" parameter. Test Plan: 1) Add multiple debarments to a patron 2) Run a sip patron information response, note the presence of newlines 3) Apply this patch 4) Add the new option convert_control_characters to your SIP account, set the value to " -- " 5) Restart all the things! 7) Run a sip patron information response, note the newlines have been replaced! Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 173082 [details] [review] Bug 37582: Tidy write_msg Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
This works, with one small correction to the test plan. In step #4 the new option should be convert_nonprinting_characters=" -- "
(In reply to Brendan Lawlor from comment #8) > This works, with one small correction to the test plan. > > In step #4 the new option should be convert_nonprinting_characters=" -- " Ack! Thanks for catching that!
Created attachment 173306 [details] [review] Bug 37582: Tidy write_msg Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Created attachment 173502 [details] [review] Bug 37582: Add ability to for SIP to convert control and separator characters to spaces This came up with a SIP vendor recently - when reading responses they were only getting part of a patron's information. It seemed to have been caused by patrons with multiple restrictions and a new line in the response This patch also does minor refactoring to write_msg to pass the server variable that contains all the previously passed parameters, and removes the never used "file" parameter. Test Plan: 1) Add multiple debarments to a patron 2) Run a sip patron information response, note the presence of newlines 3) Apply this patch 4) Add the new option convert_control_characters to your SIP account, set the value to " -- " 5) Restart all the things! 7) Run a sip patron information response, note the newlines have been replaced! Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173503 [details] [review] Bug 37582: Tidy write_msg Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Could we have a unit test for this new option and the changes please?
Pushed for 24.11! Well done everyone, thank you!
Created attachment 173712 [details] [review] Bug 37582: Add unit test [WIP]
Created attachment 173715 [details] [review] Bug 37582: Add unit test
Unit test ready to go!
Thanks, follow-up unit tests grabbed for main :)