Created attachment 106060 [details] [review] Bug 25816: Add opac messages in SIP display. Test plan: - Add one or more opac messages to a patron (patron detail -> add message -> Add a message for: OPAC) - Check that the messages will be displayed, with correctly formatted dates. You can use src/C4/SIP/interactive_patron_dump.pl for easier testing.
Created attachment 139688 [details] [review] Bug 25816: Add opac messages in SIP display. Test plan: - Add one or more opac messages to a patron (patron detail -> add message -> Add a message for: OPAC) - Check that the messages will be displayed, with correctly formatted dates. You can use src/C4/SIP/interactive_patron_dump.pl for easier testing. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works as expected.. signing off. QA comment.. we should probably have Unit Tests for this. Also, my brain fails me right now; but are there any characters we should be escaping/filtering from messages here?
(In reply to Martin Renvoize from comment #3) > Works as expected.. signing off. > > QA comment.. we should probably have Unit Tests for this. Also, my brain > fails me right now; but are there any characters we should be > escaping/filtering from messages here? Agreed with the unit test. Also, I wonder if this should be controlled by yet another sip account level switch in the sip config. Characters should already get escaped automatically. However, it's not working for me atm: CLI: ./misc/sip_cli_emulator.pl -a 127.0.0.1 -su koha -sp koha -l MPL --patron 23529000035676 -m patron_information Attempting socket connection to 127.0.0.1:6001...connected! Use of uninitialized value $hold_mode in string eq at ./misc/sip_cli_emulator.pl line 120. SEND: 9300CNkoha|COkoha|CPMPL| READ: 941 Trying 'patron_information' SEND: 6300120220823 155418 AOMPL|AA23529000035676|ACkoha| Use of uninitialized value $data in concatenation (.) or string at ./misc/sip_cli_emulator.pl line 352, <GEN0> chunk 1. READ: Logs: [2022/08/23 15:55:51] [26838] [DEBUG] [undef]@[undef]: Configuration::find_service: Trying 6001/tcp C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:51] [26838] [DEBUG] [undef]@[undef]: Configuration::find_service: Trying *:6001/tcp C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:51] [26838] [DEBUG] [undef]@[undef]: Configuration::find_service: Trying 127.0.0.1:6001/tcp C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:51] [26838] [DEBUG] [undef]@[undef]: raw_transport: timeout is 60 C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:51] [26838] [DEBUG] [undef]@[undef]: read_request trimmed 1 character(s) C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:51] [26838] [INFO] [undef]@[undef]: INPUT MSG: '9300CNkoha|COkoha|CPMPL' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:51] [26838] [DEBUG] [undef]@[undef]: Sip::MsgType::new('C4::SIP::Sip::MsgType', '9300CNkoha...', '93'): seq.no '0', protocol 2 C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:51] [26838] [DEBUG] [undef]@[undef]: Sip::MsgType::_initialize('Login', '00CNkoha|COkoha|CPMPL', 'A1A1', '2', ...) C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [INFO] [undef]@[undef]: Successful login/auth for 'koha' of 'kohalibrary' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [DEBUG] [undef]@[undef]: login_core: $VAR1 = 'ILS'; C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [DEBUG] [undef]@[undef]: new ILS 'kohalibrary' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [INFO] [undef]@[undef]: OUTPUT MSG: '941' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [DEBUG] koha@127.0.0.1: raw_transport: uname/inst: 'koha/kohalibrary' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [DEBUG] koha@127.0.0.1: read_request trimmed 2 character(s) C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [INFO] koha@127.0.0.1: INPUT MSG: '6300120220823 155551 AOMPL|AA23529000035676|ACkoha' C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [DEBUG] koha@127.0.0.1: Sip::MsgType::new('C4::SIP::Sip::MsgType', '6300120220...', '63'): seq.no '0', protocol 2 C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [DEBUG] koha@127.0.0.1: Sip::MsgType::_initialize('Patron Info', '00120220823 155551 AOMPL|AA23529000035676|ACkoha', 'A3A18A10', '31', ...) C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223) [2022/08/23 15:55:52] [26838] [INFO] koha@127.0.0.1: raw_transport: shutting down C4::SIP::Sip::siplog /kohadevbox/koha/C4/SIP/Sip.pm (223)
Created attachment 154545 [details] [review] Bug 25816: Add opac messages in SIP display. Test plan: - Add one or more opac messages to a patron (patron detail -> add message -> Add a message for: OPAC) - Check that the messages will be displayed, with correctly formatted dates. You can use src/C4/SIP/interactive_patron_dump.pl for easier testing. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Hello ! I've rebased the patch against master, and added unit tests. Kyle, could you have another try now that it is rebased? It's working for me with the rebased patch: kohadev-koha@kohadevbox:/kohadevbox/koha/misc$ ./sip_cli_emulator.pl -a 127.0.0.1 -su koha -sp koha -l MPL --patron 23529000035676 -m patron_information Attempting socket connection to 127.0.0.1:6001...connected! Use of uninitialized value $hold_mode in string eq at ./sip_cli_emulator.pl line 123. SEND: 9300CNkoha|COkoha|CPMPL| READ: 941 Trying 'patron_information' SEND: 6300120230817 113324 AOMPL|AA23529000035676|ACkoha| READ: 64 00120230817 113324000000000000000000000000AOkohalibrary|AA23529000035676|AEHenry Acevedo|BLY|BV0|CC5|BD4345 Library Rd. Springfield, MA 44224|BF(212) 555-1212|PB19580530|PCS|PIY|AFGreetings from Koha. . Messages for you: 08/17/2023: My message|
|AFGreetings from Koha. | becomes: |AFGreetings from Koha. . Messages for you: 09/11/2023: OPAC MESSAGE| There's an extra period in there. I think you'll need to tweak how you combine everything. You might consider using join on everything. I'm going to say I don't see how this can pass qa without at least a simple on/off switch in the SIP config at the account level. A change like this could break existing systems.
(In reply to Kyle M Hall from comment #7) > |AFGreetings from Koha. | > > becomes: > > |AFGreetings from Koha. . Messages for you: 09/11/2023: OPAC MESSAGE| > > There's an extra period in there. I think you'll need to tweak how you > combine everything. You might consider using join on everything. > > I'm going to say I don't see how this can pass qa without at least a simple > on/off switch in the SIP config at the account level. A change like this > could break existing systems. After reviewing the code, I think making this an account level setting is overly difficult. I think a system preference would be acceptable.
Created attachment 155592 [details] [review] Bug 25816: Add opac messages in SIP display. Test plan: - Add one or more opac messages to a patron (patron detail -> add message -> Add a message for: OPAC) - Check that the messages will be displayed, with correctly formatted dates. You can use src/C4/SIP/interactive_patron_dump.pl for easier testing. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 155596 [details] [review] Bug 25816: (QA follow-up) Add system preference Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Thanks for the follow-up, Kyle!
Pushed to master for 23.11. Nice work everyone, thanks!
Enhancement not pushed to 23.05.x