Bug 25816 - Add OPAC messages in SIP display
Summary: Add OPAC messages in SIP display
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Matthias Meusburger
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks: 35397
  Show dependency treegraph
 
Reported: 2020-06-19 09:30 UTC by Matthias Meusburger
Modified: 2023-11-30 14:08 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
New system preference `SIP2AddOpacMessagesToScreenMessage` allows to include patron OPAC messages in the SIP2 screen message. Starting with "Messages for you: ".
Version(s) released in:
23.11.00


Attachments
Bug 25816: Add opac messages in SIP display. (1.68 KB, patch)
2020-06-19 09:32 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 25816: Add opac messages in SIP display. (1.74 KB, patch)
2022-08-23 15:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25816: Add opac messages in SIP display. (3.44 KB, patch)
2023-08-17 11:34 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 25816: Add opac messages in SIP display. (3.50 KB, patch)
2023-09-13 15:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25816: (QA follow-up) Add system preference (8.16 KB, patch)
2023-09-13 18:38 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Meusburger 2020-06-19 09:30:48 UTC

    
Comment 1 Matthias Meusburger 2020-06-19 09:32:41 UTC
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.
Comment 2 Martin Renvoize 2022-08-23 15:10:47 UTC
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>
Comment 3 Martin Renvoize 2022-08-23 15:11:50 UTC
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?
Comment 4 Kyle M Hall 2022-08-23 15:56:04 UTC
(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)
Comment 5 Matthias Meusburger 2023-08-17 11:34:51 UTC
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>
Comment 6 Matthias Meusburger 2023-08-17 11:40:17 UTC
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|
Comment 7 Kyle M Hall 2023-09-11 19:06:00 UTC
|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.
Comment 8 Kyle M Hall 2023-09-12 18:15:04 UTC
(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.
Comment 9 Kyle M Hall 2023-09-13 15:37:40 UTC
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>
Comment 10 Kyle M Hall 2023-09-13 18:38:47 UTC
Created attachment 155596 [details] [review]
Bug 25816: (QA follow-up) Add system preference

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Matthias Meusburger 2023-09-27 13:07:50 UTC
Thanks for the follow-up, Kyle!
Comment 12 Tomás Cohen Arazi 2023-10-20 14:03:43 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 13 Fridolin Somers 2023-10-25 22:06:18 UTC
Enhancement not pushed to 23.05.x