Bug 20517 - Use the "sort bin" field in SIP2 Checkin Response
Summary: Use the "sort bin" field in SIP2 Checkin Response
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Magnus Enger
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-04 12:41 UTC by Magnus Enger
Modified: 2022-08-08 20:56 UTC (History)
7 users (show)

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


Attachments
Use of the "sort bin" field in SIP2 Checkin Response (11.29 KB, patch)
2018-05-07 14:26 UTC, Christophe Croullebois
Details | Diff | Splinter Review
Use of the "sort bin" field in SIP2 Checkin Response (10.84 KB, patch)
2018-05-16 14:33 UTC, Christophe Croullebois
Details | Diff | Splinter Review
Bug 20517 - Use the "sort bin" field in SIP2 Checkin Response (6.09 KB, patch)
2020-04-24 13:34 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20517 - Use the "sort bin" field in SIP2 Checkin Response (6.21 KB, patch)
2020-06-25 13:27 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20517 - Use the "sort bin" field in SIP2 Checkin Response (6.31 KB, patch)
2022-01-27 09:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: Add unit tests (3.11 KB, patch)
2022-01-27 09:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: Add support for specifying comparitor (3.97 KB, patch)
2022-01-27 09:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: Use the "sort bin" field in SIP2 Checkin Response (6.31 KB, patch)
2022-01-27 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: Add unit tests (3.11 KB, patch)
2022-01-27 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: Add support for specifying comparitor (3.97 KB, patch)
2022-01-27 09:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: Use the "sort bin" field in SIP2 Checkin Response (6.31 KB, patch)
2022-01-27 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: Add unit tests (3.10 KB, patch)
2022-01-27 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: Add support for specifying comparitor (3.97 KB, patch)
2022-01-27 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: (QA follow-up) Fix return undef. (971 bytes, patch)
2022-01-27 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20517: Use the "sort bin" field in SIP2 Checkin Response (6.42 KB, patch)
2022-03-30 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20517: Add unit tests (3.16 KB, patch)
2022-03-30 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20517: Add support for specifying comparitor (4.02 KB, patch)
2022-03-30 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20517: (QA follow-up) Fix return undef. (1.00 KB, patch)
2022-03-30 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20517: (follow-up) Add explanation to syspref and fix QA issues (6.89 KB, patch)
2022-03-30 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20517: Use the "sort bin" field in SIP2 Checkin Response (6.47 KB, patch)
2022-03-30 12:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20517: Add unit tests (3.21 KB, patch)
2022-03-30 12:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20517: Add support for specifying comparitor (4.08 KB, patch)
2022-03-30 12:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20517: (QA follow-up) Fix return undef. (1.06 KB, patch)
2022-03-30 12:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20517: (follow-up) Add explanation to syspref and fix QA issues (6.95 KB, patch)
2022-03-30 12:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 20517: (QA follow-up) Add ability to override syspref with account level settings in SIP config file (2.67 KB, patch)
2022-03-30 12:30 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 Magnus Enger 2018-04-04 12:41:02 UTC
The SIP2 Checkin Response (10) can have a "variable-length optional field" for "sort bin", labelled "CL". In the description of the SIP2 protocol, this field is described as follows: 

"sort bin - CL - variable-length field. This field should contain a bin number that indicates how the items should be sorted. The maximum practical number of sort bins for a patron using 3M SelfCheck system is probably only 3 or 4, but many digits are allowed to accommodate some sort of fantastic sorting device using a hierarchical bin numbering scheme."

I have a customer who wants to use this field to display messages on the screen of self service machines that tell users to put items in a separate bin based on itemtype, location etc. 

The idea is to add a syspref that can contain something like this:

itemtype:ILL:1
location:CLOSEDSTACKS:2

- The first field is an item field. 
- The second field is a value to look for in the given item field
- The third field is the value given to the CL field in the SIP2 Checkin Response if the value in the second field matches the item on hand. 

It should also be possible to set the CL field for items that need to be transferred after they have been returned, that is for items that are returned at another library than the homebranch. This will probably be another syspref. If the returned item needs to be transferred to the homebranch, CL is set to the value of the syspref.
Comment 1 Christophe Croullebois 2018-04-25 14:53:36 UTC
Hello Magnus,

we have in Biblibre a specific dev on this topic.
We do not use it to indicate to the user where to put the item, but to be used by a sorting machine.
I think it is the same, because we put the number of sort bin expected in the CL field.
It is more complex than your suggestion, we use two config files .yaml, the first just have the bin number in case of an hold on the item.
The second one is more complex, you can use conditions with eq/neq and/or to chose the number of the sortbin based on all 995$ subfields (item fields in Unimarc).
So the Library can give many conditions for each sortbin based on all item's Marc subfield.
We do not submit this dev because we thought it was not sure it will be agreed by the community.
Anyway if you want to examine it I could send it to you.
Best regards
Comment 2 Magnus Enger 2018-04-30 19:19:58 UTC
Christophe! That sounds like an excellent enhancement to me! If you submit it, I will try to sign off!
Comment 3 Christophe Croullebois 2018-05-07 14:26:28 UTC
Created attachment 75126 [details] [review]
Use of the "sort bin" field in SIP2 Checkin Response
Comment 4 Christophe Croullebois 2018-05-07 14:34:44 UTC
(In reply to Magnus Enger from comment #2)
> Christophe! That sounds like an excellent enhancement to me! If you submit
> it, I will try to sign off!

Hello Magnus,

I have just rebased the patch, as you will see examples in sortbin_reserves.yaml and sortbin_transform.yaml are in French as it was for our use.
If this patch seems relevant I maybe could do some changes but I prefer to have advices before because I have not many time.

Best regards
Comment 5 Colin Campbell 2018-05-08 08:37:41 UTC
Patch changes the path to the ILS modules - needs to not do that to apply to master.
Comment 6 Christophe Croullebois 2018-05-16 13:56:12 UTC
(In reply to Colin Campbell from comment #5)
> Patch changes the path to the ILS modules - needs to not do that to apply to
> master.

Hi Colin, thx I will change that.
Comment 7 Christophe Croullebois 2018-05-16 14:33:57 UTC
Created attachment 75369 [details] [review]
Use of the "sort bin" field in SIP2 Checkin Response
Comment 8 Magnus Enger 2019-12-16 12:23:45 UTC
Just adding a quick note that another way to set the sort bin could be to let plugins do it - that would allow for even more dynamic rules etc.
Comment 9 Magnus Enger 2020-04-24 13:34:47 UTC Comment hidden (obsolete)
Comment 10 Magnus Enger 2020-04-24 13:37:00 UTC
I have been putting this off for so long, but I finally found some time to make a patch of my own simple solution. If anyone wants to make a full patch of Christophe's more complex solution that is fine by me.
Comment 11 Magnus Enger 2020-06-05 11:49:51 UTC
Need to revise this a bit.
Comment 12 Magnus Enger 2020-06-25 13:27:20 UTC
Created attachment 106291 [details] [review]
Bug 20517 - Use the "sort bin" field in SIP2 Checkin Response

This bug adds the SIP2SortBinMapping to make it possible to tell
the Koha SIP2 server to include a sort_bin/CL field in the
Checkin Response, if appropriate.

To test:
- Check out an item and return it via SIP2:
  $ telnet localhost 8023
  9300CNterm1|COterm1|CPCPL|
  09N20200422    12430020200422    124300APCPL|AOCPL|AB001|AC|
  (Where 001 in |AB001| is the barcode of the item in question)
- Check there is no CL field in the last response
- Apply the patch, fill in SIP2SortBinMapping with e.g.:
       CPL:itype:BK:3
- Repeat the first step
- Check the respons contains a CL field with a value of 3 (or what
  you put in the config). The field should look like |CL3|
- Signe ye offe

2020-06-25: Pass an item to _get_sort_bin, not a checkout. Rebase.

https://bugs.koha-community.org/show_bug.cgi?id=20571
Comment 13 Martin Renvoize 2022-01-25 11:20:15 UTC
We now have a few local sort bind handlers here, so I have a better understanding of the wider requirements than I did the first time I came across this bug.

I have a couple of refinements that I think could help.

I think we could do with being able to define the comparison operator rather than always being equality.  We have a customer who uses classmark/callnumber for their sorting and as such rather than 'eq' they need '<' comparisons.

Also, I think it would be helpful to be able to call item methods not just refer to the internal hashref data.. think 'effective_itemtype'.

There may be a use for some special comparison options on the right hand side.. for example one may wish to compare permanent_location to current checking branch to filter items that need transfer into their own bin.
Comment 14 Martin Renvoize 2022-01-25 15:31:42 UTC
Also.. we need unit tests for this.

Working on some follow-ups
Comment 15 Martin Renvoize 2022-01-27 09:52:27 UTC
Created attachment 129846 [details] [review]
Bug 20517 - Use the "sort bin" field in SIP2 Checkin Response

This bug adds the SIP2SortBinMapping to make it possible to tell
the Koha SIP2 server to include a sort_bin/CL field in the
Checkin Response, if appropriate.

To test:
- Check out an item and return it via SIP2:
  $ telnet localhost 8023
  9300CNterm1|COterm1|CPCPL|
  09N20200422    12430020200422    124300APCPL|AOCPL|AB001|AC|
  (Where 001 in |AB001| is the barcode of the item in question)
- Check there is no CL field in the last response
- Apply the patch, fill in SIP2SortBinMapping with e.g.:
       CPL:itype:BK:3
- Repeat the first step
- Check the respons contains a CL field with a value of 3 (or what
  you put in the config). The field should look like |CL3|
- Signe ye offe

2020-06-25: Pass an item to _get_sort_bin, not a checkout. Rebase.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2022-01-27 09:52:30 UTC
Created attachment 129847 [details] [review]
Bug 20517: Add unit tests

This patch adds some unit tests for the new _get_sort_bin function.

We're testing that rules are applied in order and that the different
comparison operators work as expected.
Comment 17 Martin Renvoize 2022-01-27 09:52:35 UTC
Created attachment 129848 [details] [review]
Bug 20517: Add support for specifying comparitor

This patch adds support for specifying the comparison operator used in
the sort bin method. It allows for more complex cases where one may wish
to sort on groups of callnumbers for example.
Comment 18 Martin Renvoize 2022-01-27 09:53:16 UTC
Created attachment 129849 [details] [review]
Bug 20517: Use the "sort bin" field in SIP2 Checkin Response

This bug adds the SIP2SortBinMapping to make it possible to tell
the Koha SIP2 server to include a sort_bin/CL field in the
Checkin Response, if appropriate.

To test:
- Check out an item and return it via SIP2:
  $ telnet localhost 8023
  9300CNterm1|COterm1|CPCPL|
  09N20200422    12430020200422    124300APCPL|AOCPL|AB001|AC|
  (Where 001 in |AB001| is the barcode of the item in question)
- Check there is no CL field in the last response
- Apply the patch, fill in SIP2SortBinMapping with e.g.:
       CPL:itype:BK:3
- Repeat the first step
- Check the respons contains a CL field with a value of 3 (or what
  you put in the config). The field should look like |CL3|
- Signe ye offe

2020-06-25: Pass an item to _get_sort_bin, not a checkout. Rebase.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize 2022-01-27 09:53:21 UTC
Created attachment 129850 [details] [review]
Bug 20517: Add unit tests

This patch adds some unit tests for the new _get_sort_bin function.

We're testing that rules are applied in order and that the different
comparison operators work as expected.
Comment 20 Martin Renvoize 2022-01-27 09:53:25 UTC
Created attachment 129851 [details] [review]
Bug 20517: Add support for specifying comparitor

This patch adds support for specifying the comparison operator used in
the sort bin method. It allows for more complex cases where one may wish
to sort on groups of callnumbers for example.
Comment 21 Martin Renvoize 2022-01-27 10:02:45 UTC
Created attachment 129852 [details] [review]
Bug 20517: Use the "sort bin" field in SIP2 Checkin Response

This bug adds the SIP2SortBinMapping to make it possible to tell
the Koha SIP2 server to include a sort_bin/CL field in the
Checkin Response, if appropriate.

To test:
- Check out an item and return it via SIP2:
  $ telnet localhost 8023
  9300CNterm1|COterm1|CPCPL|
  09N20200422    12430020200422    124300APCPL|AOCPL|AB001|AC|
  (Where 001 in |AB001| is the barcode of the item in question)
- Check there is no CL field in the last response
- Apply the patch, fill in SIP2SortBinMapping with e.g.:
       CPL:itype:BK:3
- Repeat the first step
- Check the respons contains a CL field with a value of 3 (or what
  you put in the config). The field should look like |CL3|
- Signe ye offe

2020-06-25: Pass an item to _get_sort_bin, not a checkout. Rebase.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize 2022-01-27 10:02:49 UTC
Created attachment 129853 [details] [review]
Bug 20517: Add unit tests

This patch adds some unit tests for the new _get_sort_bin function.

We're testing that rules are applied in order and that the different
comparison operators work as expected.
Comment 23 Martin Renvoize 2022-01-27 10:02:53 UTC
Created attachment 129854 [details] [review]
Bug 20517: Add support for specifying comparitor

This patch adds support for specifying the comparison operator used in
the sort bin method. It allows for more complex cases where one may wish
to sort on groups of callnumbers for example.
Comment 24 Martin Renvoize 2022-01-27 10:02:56 UTC
Created attachment 129855 [details] [review]
Bug 20517: (QA follow-up) Fix return undef.
Comment 25 Nick Clemens 2022-03-30 11:12:20 UTC
Created attachment 132555 [details] [review]
Bug 20517: Use the "sort bin" field in SIP2 Checkin Response

This bug adds the SIP2SortBinMapping to make it possible to tell
the Koha SIP2 server to include a sort_bin/CL field in the
Checkin Response, if appropriate.

To test:
- Check out an item and return it via SIP2:
  $ telnet localhost 8023
  9300CNterm1|COterm1|CPCPL|
  09N20200422    12430020200422    124300APCPL|AOCPL|AB001|AC|
  (Where 001 in |AB001| is the barcode of the item in question)
- Check there is no CL field in the last response
- Apply the patch, fill in SIP2SortBinMapping with e.g.:
       CPL:itype:BK:3
- Repeat the first step
- Check the respons contains a CL field with a value of 3 (or what
  you put in the config). The field should look like |CL3|
- Signe ye offe

2020-06-25: Pass an item to _get_sort_bin, not a checkout. Rebase.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>


Current status: Signed Off
Comment 26 Nick Clemens 2022-03-30 11:12:25 UTC
Created attachment 132556 [details] [review]
Bug 20517: Add unit tests

This patch adds some unit tests for the new _get_sort_bin function.

We're testing that rules are applied in order and that the different
comparison operators work as expected.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 27 Nick Clemens 2022-03-30 11:12:30 UTC
Created attachment 132557 [details] [review]
Bug 20517: Add support for specifying comparitor

This patch adds support for specifying the comparison operator used in
the sort bin method. It allows for more complex cases where one may wish
to sort on groups of callnumbers for example.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 28 Nick Clemens 2022-03-30 11:12:35 UTC
Created attachment 132558 [details] [review]
Bug 20517: (QA follow-up) Fix return undef.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 29 Nick Clemens 2022-03-30 11:12:40 UTC
Created attachment 132559 [details] [review]
Bug 20517: (follow-up) Add explanation to syspref and fix QA issues

I added explanatory text to staff interface on the preference to explain how it works

Removed a debug warn in the _get_sort_bin routine

changed comparitor => comparator

fixed a missing call in the tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 30 Nick Clemens 2022-03-30 11:17:50 UTC
Martin's follow-ups here added a decent bit of functionality, so this needs another QA run.

I love this enhancement, I think partners will find it useful, but I have a few concerns:
1 - In a larger consortium this could start to have performance impacts as we iterate through all the lines
2 - We are moving from SIPConfig to syspref control - I like this, but as above it removes config per SIP login to global
3 - The syntax is non-standard, I would love to see this use YAML, that could allow for things like 'field_value:X' and 'compare_field:holdingbranch' rather than '\$holdingbranch'

I don't think these are blockers, as it is this is simple and straightforward, so I don't want to mess with that. 

Leaving for another QA
Comment 31 Kyle M Hall 2022-03-30 12:30:31 UTC
Created attachment 132591 [details] [review]
Bug 20517: Use the "sort bin" field in SIP2 Checkin Response

This bug adds the SIP2SortBinMapping to make it possible to tell
the Koha SIP2 server to include a sort_bin/CL field in the
Checkin Response, if appropriate.

To test:
- Check out an item and return it via SIP2:
  $ telnet localhost 8023
  9300CNterm1|COterm1|CPCPL|
  09N20200422    12430020200422    124300APCPL|AOCPL|AB001|AC|
  (Where 001 in |AB001| is the barcode of the item in question)
- Check there is no CL field in the last response
- Apply the patch, fill in SIP2SortBinMapping with e.g.:
       CPL:itype:BK:3
- Repeat the first step
- Check the respons contains a CL field with a value of 3 (or what
  you put in the config). The field should look like |CL3|
- Signe ye offe

2020-06-25: Pass an item to _get_sort_bin, not a checkout. Rebase.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 32 Kyle M Hall 2022-03-30 12:30:42 UTC
Created attachment 132592 [details] [review]
Bug 20517: Add unit tests

This patch adds some unit tests for the new _get_sort_bin function.

We're testing that rules are applied in order and that the different
comparison operators work as expected.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 33 Kyle M Hall 2022-03-30 12:30:46 UTC
Created attachment 132593 [details] [review]
Bug 20517: Add support for specifying comparitor

This patch adds support for specifying the comparison operator used in
the sort bin method. It allows for more complex cases where one may wish
to sort on groups of callnumbers for example.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Kyle M Hall 2022-03-30 12:30:50 UTC
Created attachment 132594 [details] [review]
Bug 20517: (QA follow-up) Fix return undef.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 35 Kyle M Hall 2022-03-30 12:30:54 UTC
Created attachment 132595 [details] [review]
Bug 20517: (follow-up) Add explanation to syspref and fix QA issues

I added explanatory text to staff interface on the preference to explain how it works

Removed a debug warn in the _get_sort_bin routine

changed comparitor => comparator

fixed a missing call in the tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Kyle M Hall 2022-03-30 12:30:58 UTC
Created attachment 132596 [details] [review]
Bug 20517: (QA follow-up) Add ability to override syspref with account level settings in SIP config file

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 37 Fridolin Somers 2022-04-08 13:52:20 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄