Bug 25814 - SIP: Add a message on successful checkin
Summary: SIP: Add a message on successful checkin
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:
 
Reported: 2020-06-19 09:19 UTC by Matthias Meusburger
Modified: 2023-11-11 01:05 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 25814: SIP: Add a message on successful checkin. (1.11 KB, patch)
2020-06-19 09:20 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 25814: SIP: Add a message on successful checkin. (6.30 KB, patch)
2023-08-22 08:57 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 25814: SIP: Add a message on successful checkin. (6.35 KB, patch)
2023-09-25 02:12 UTC, David Nind
Details | Diff | Splinter Review
Bug 25814: SIP: Add a message on successful checkin. (6.42 KB, patch)
2023-11-02 16:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25814: Add comment to example sip config file (1.16 KB, patch)
2023-11-02 16:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25814: Tidy code (1.41 KB, patch)
2023-11-02 16:44 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:19:39 UTC
Currently, Koha does not return a message on successful SIP checkin.
Comment 1 Matthias Meusburger 2020-06-19 09:20:50 UTC
Created attachment 106058 [details] [review]
Bug 25814: SIP: Add a message on successful checkin.

Currently, Koha does not return a message on successful SIP checkin.

This patchs adds the following message:

 - "Item checked-in: {homebranch} - {location}"

Test plan:

 - Perform a successful checkin using SIP
 - Check that the message is in the checkin response (AF field)
Comment 2 Katrin Fischer 2020-06-22 22:38:58 UTC
I think this one is not correct.

permanent_locaton != homebranch.
But also: What if the item is not returned at the home library? 

The location might apply to the home library or the holding branch, it's hard to tell. I think I would probably not display it by default.

Also I think this would display the codes, not the descriptions?

I think I'd maybe leave it at "Item checked in"
Comment 3 Matthias Meusburger 2023-08-22 08:57:50 UTC
Created attachment 154687 [details] [review]
Bug 25814: SIP: Add a message on successful checkin.

Currently, Koha does not return a message on successful SIP checkin.

This patchs adds the show_checkin_message option to SIPconfig.xml, disabled by
default. When enabled, the following message is displayed on SIP checkin:

"Item checked-in: {homebranch|permanent_location} - {location}"

The UseLocationAsAQInSIP system preference is used to determine whether the
homebranch or the permanent location will be used.

Test plan:

 - Perform a successful checkin using SIP
 - Check that the message is in the checkin response (AF field)
 - prove t/db_dependent/SIP/Transaction.t
Comment 4 Matthias Meusburger 2023-08-22 08:59:03 UTC
Here is a new patch, that makes the message optional, is consistent with the UseLocationAsAQInSIP system preference, and adds unit tests.
Comment 5 David Nind 2023-09-25 02:12:36 UTC
Created attachment 156142 [details] [review]
Bug 25814: SIP: Add a message on successful checkin.

Currently, Koha does not return a message on successful SIP checkin.

This patchs adds the show_checkin_message option to SIPconfig.xml, disabled by
default. When enabled, the following message is displayed on SIP checkin:

"Item checked-in: {homebranch|permanent_location} - {location}"

The UseLocationAsAQInSIP system preference is used to determine whether the
homebranch or the permanent location will be used.

Test plan:

 - Perform a successful checkin using SIP
 - Check that the message is in the checkin response (AF field)
 - prove t/db_dependent/SIP/Transaction.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2023-09-25 02:13:56 UTC
Testing notes (using KTD):

1. Checked out an item to koha using the staff interface: 39999000011418

2. Checked item in using SIP - no message at end after CRREF|: 

misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m checkin --item 39999000011418
Attempting socket connection to localhost:6001...connected!
Use of uninitialized value $hold_mode in string eq at misc/sip_cli_emulator.pl line 123.
SEND: 9300CNterm1|COterm1|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20230925    01381820230925    013818APCPL|AOCPL|AB39999000011418|ACterm1|BIN|
READ: 101YNN20230925    013818AOCPL|AB39999000011418|AQCPL|AJProgramming Perl /|AA42|CRREF|

3. Applied the patches.

4. Edited /etc/koha/sites/kohadev/SIPconfig.xml and added show_checkin_message="1":
...
  <accounts>
       <login id="term1"  password="term1" delimiter="|" error-detect="enabled" institution="CPL" encoding="ascii" checked_in_ok="1" show_checkin_message="1"/>
....

5. Restarted everything (restart_all).

6. Checked out the item again and then checked in using SIP - message displayed when UseLocationAsAQInSIP = permanent_location:

misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m checkin --item 39999000011418
Attempting socket connection to localhost:6001...connected!
Use of uninitialized value $hold_mode in string eq at misc/sip_cli_emulator.pl line 123.
SEND: 9300CNterm1|COterm1|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20230925    02031820230925    020318APCPL|AOCPL|AB39999000011418|ACterm1|BIN|
READ: 101YNN20230925    020318AOCPL|AB39999000011418|AQGEN|AJProgramming Perl /|AA42|CRREF|AFItem checked-in: GEN - GEN.|

7. Checked out the item again and the checked in using SIP - message displayed when UseLocationAsAQInSIP = homebranch:

misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m checkin --item 39999000011418
Attempting socket connection to localhost:6001...connected!
Use of uninitialized value $hold_mode in string eq at misc/sip_cli_emulator.pl line 123.
SEND: 9300CNterm1|COterm1|CPCPL|
READ: 941

Trying 'checkin'
SEND: 09N20230925    02083420230925    020834APCPL|AOCPL|AB39999000011418|ACterm1|BIN|
READ: 101YNN20230925    020834AOCPL|AB39999000011418|AQCPL|AJProgramming Perl /|AA42|CRREF|AFItem checked-in: Centerville - GEN.|
Comment 7 Katrin Fischer 2023-11-01 20:27:49 UTC
Tried to have a look at this one, but I would feel better if someone with more SIP knowledge double checked. 

Some notes:

0) Set assignee to Matthias
1) We need some perltidy


Processing files before patches
|========================>| 3 / 3 (100.00%)
Processing files after patches
|========================>| 3 / 3 (100.00%)

 WARN	C4/SIP/ILS.pm
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 186, now: 189)

 OK	etc/SIPconfig.xml

 WARN	t/db_dependent/SIP/Transaction.t
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 1, now: 17)

2) The tests were failing with me on my test database because I had added some new locations and used them on an item testing bug 21159. After a reset_all the tests are passing.

3) I'd love some type of documentation/notes in SIPconfig.xml to explain the use of the new option.

4) Also, never got a reply to my comment#2 about the codes vs. descriptions.
Comment 8 Kyle M Hall 2023-11-02 16:43:55 UTC
Created attachment 158273 [details] [review]
Bug 25814: SIP: Add a message on successful checkin.

Currently, Koha does not return a message on successful SIP checkin.

This patchs adds the show_checkin_message option to SIPconfig.xml, disabled by
default. When enabled, the following message is displayed on SIP checkin:

"Item checked-in: {homebranch|permanent_location} - {location}"

The UseLocationAsAQInSIP system preference is used to determine whether the
homebranch or the permanent location will be used.

Test plan:

 - Perform a successful checkin using SIP
 - Check that the message is in the checkin response (AF field)
 - prove t/db_dependent/SIP/Transaction.t

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall 2023-11-02 16:44:05 UTC
Created attachment 158274 [details] [review]
Bug 25814: Add comment to example sip config file

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall 2023-11-02 16:44:07 UTC
Created attachment 158275 [details] [review]
Bug 25814: Tidy code

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Kyle M Hall 2023-11-02 16:45:19 UTC
I've love to see the language moved to TT markup in the sip config, but I don't consider that a blocker.
Comment 12 Tomás Cohen Arazi 2023-11-08 14:42:29 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 13 Fridolin Somers 2023-11-11 01:05:52 UTC
Enhancement not pushed to 23.05.x