Bug 34737 - Enhance SIP2SortBinMapping to support additional match conditions
Summary: Enhance SIP2SortBinMapping to support additional match conditions
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 20517
Blocks:
  Show dependency treegraph
 
Reported: 2023-09-07 12:36 UTC by Martin Renvoize
Modified: 2024-07-04 20:37 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact: Aude Charillon
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/749
Text to go in the release notes:
**Sponsored by** *PTFS Europe Ltd*
Version(s) released in:
23.11.00


Attachments
Bug 34737: Add ability to match on combinations of item fields (4.64 KB, patch)
2023-09-07 16:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: Unit tests (1.93 KB, patch)
2023-09-07 16:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: (follow-up) Squelch a warning (1.03 KB, patch)
2023-09-07 16:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: Add ability to match on combinations of item fields (7.57 KB, patch)
2023-09-08 07:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: Unit tests (1.93 KB, patch)
2023-09-08 07:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: (follow-up) Squelch a warning (1.03 KB, patch)
2023-09-08 07:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: Add ability to match on combinations of item fields (7.62 KB, patch)
2023-09-15 10:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34737: Unit tests (1.99 KB, patch)
2023-09-15 10:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34737: (follow-up) Squelch a warning (1.08 KB, patch)
2023-09-15 10:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34737: Tidy t/db_dependent/SIP/Transaction.t (50.42 KB, patch)
2023-09-15 10:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34737: (follow-up) Code more defensively (1.01 KB, patch)
2023-09-25 10:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: (follow-up) Unit test for skipping bad lines (1.12 KB, patch)
2023-09-25 10:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: Add ability to match on combinations of item fields (7.68 KB, patch)
2023-09-25 14:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: Unit tests (2.04 KB, patch)
2023-09-25 14:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: (follow-up) Squelch a warning (1.14 KB, patch)
2023-09-25 14:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: Tidy t/db_dependent/SIP/Transaction.t (50.47 KB, patch)
2023-09-25 14:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: (follow-up) Unit test for skipping bad lines (2.65 KB, patch)
2023-09-25 14:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: (follow-up) Code more defensively (1.16 KB, patch)
2023-09-25 14:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: (follow-up) Unit test for skipping bad lines (2.65 KB, patch)
2023-09-25 14:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: (follow-up) Code more defensively (1.16 KB, patch)
2023-09-25 14:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 34737: (follow-up) Unit test for skipping bad lines (2.70 KB, patch)
2023-09-28 11:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34737: (follow-up) Code more defensively (1.21 KB, patch)
2023-09-28 11:01 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 Martin Renvoize 2023-09-07 12:36:03 UTC
Currently, the SIP2SortBinMapping preference sorts based on checkin location + any item field comparison.

However, we have a customer who uses multiple item fields in combination to do their filtering.
Comment 1 Martin Renvoize 2023-09-07 16:25:34 UTC
Created attachment 155328 [details] [review]
Bug 34737: Add ability to match on combinations of item fields

This patch updates the sort bin selection logic to allow for multiple
item field comparisons in a single config rule line.

Test plan
Comment 2 Martin Renvoize 2023-09-07 16:25:36 UTC
Created attachment 155329 [details] [review]
Bug 34737: Unit tests

This patch adds unit test to prove the multi-field match functionality
added in this patchset.
Comment 3 Martin Renvoize 2023-09-07 16:25:38 UTC
Created attachment 155330 [details] [review]
Bug 34737: (follow-up) Squelch a warning

We weren't checking whether an item property was defined or not and as
such the new test highlighted a new warning.
Comment 4 Martin Renvoize 2023-09-08 07:01:53 UTC
Created attachment 155360 [details] [review]
Bug 34737: Add ability to match on combinations of item fields

This patch updates the sort bin selection logic to allow for multiple
item field comparisons in a single config rule line.

Test plan
1) Run the unit tests before and after applying this patch and confirm
   they pass in both cases (prove -v t/db_debendant/SIP/Transaction.t
2) 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)
4) Check there is no CL field in the last response
5) Apply the patch, fill in SIP2SortBinMapping with e.g.:
       CPL:itype:eq:BK:ccode:eq:TEEN:3
6) Repeat the first step
7) Check the response contains a CL field with a value of 3 (or what
   you put in the config). The field should look like |CL3|
Comment 5 Martin Renvoize 2023-09-08 07:01:55 UTC
Created attachment 155361 [details] [review]
Bug 34737: Unit tests

This patch adds unit test to prove the multi-field match functionality
added in this patchset.
Comment 6 Martin Renvoize 2023-09-08 07:01:58 UTC
Created attachment 155362 [details] [review]
Bug 34737: (follow-up) Squelch a warning

We weren't checking whether an item property was defined or not and as
such the new test highlighted a new warning.
Comment 7 Kyle M Hall 2023-09-15 10:41:34 UTC
Created attachment 155663 [details] [review]
Bug 34737: Add ability to match on combinations of item fields

This patch updates the sort bin selection logic to allow for multiple
item field comparisons in a single config rule line.

Test plan
1) Run the unit tests before and after applying this patch and confirm
   they pass in both cases (prove -v t/db_debendant/SIP/Transaction.t
2) 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)
4) Check there is no CL field in the last response
5) Apply the patch, fill in SIP2SortBinMapping with e.g.:
       CPL:itype:eq:BK:ccode:eq:TEEN:3
6) Repeat the first step
7) Check the response contains a CL field with a value of 3 (or what
   you put in the config). The field should look like |CL3|

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall 2023-09-15 10:41:40 UTC
Created attachment 155664 [details] [review]
Bug 34737: Unit tests

This patch adds unit test to prove the multi-field match functionality
added in this patchset.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall 2023-09-15 10:41:43 UTC
Created attachment 155665 [details] [review]
Bug 34737: (follow-up) Squelch a warning

We weren't checking whether an item property was defined or not and as
such the new test highlighted a new warning.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall 2023-09-15 10:41:45 UTC
Created attachment 155666 [details] [review]
Bug 34737: Tidy t/db_dependent/SIP/Transaction.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Martin Renvoize 2023-09-25 10:30:52 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize 2023-09-25 10:32:34 UTC
This is working at a live site, but such usage highlighted some brittleness in it.. I've added a follow-up with some defensive coding.. unit test to follow shortly.
Comment 13 Martin Renvoize 2023-09-25 10:40:38 UTC Comment hidden (obsolete)
Comment 14 Martin Renvoize 2023-09-25 14:31:19 UTC Comment hidden (obsolete)
Comment 15 Martin Renvoize 2023-09-25 14:31:22 UTC Comment hidden (obsolete)
Comment 16 Martin Renvoize 2023-09-25 14:31:24 UTC Comment hidden (obsolete)
Comment 17 Martin Renvoize 2023-09-25 14:31:27 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize 2023-09-25 14:31:30 UTC Comment hidden (obsolete)
Comment 19 Martin Renvoize 2023-09-25 14:31:32 UTC Comment hidden (obsolete)
Comment 20 Martin Renvoize 2023-09-25 14:52:15 UTC Comment hidden (obsolete)
Comment 21 Martin Renvoize 2023-09-25 14:52:18 UTC Comment hidden (obsolete)
Comment 22 Kyle M Hall 2023-09-28 11:01:20 UTC
Created attachment 156314 [details] [review]
Bug 34737: (follow-up) Unit test for skipping bad lines

We update the test rules set to include some malformed, empty and
comment lines. No actual tests are added, but the skipping is checked by
all other tests in this way.

Signed-off-by: Toni Ford <Toni.Ford@newcastle.gov.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 23 Kyle M Hall 2023-09-28 11:01:28 UTC
Created attachment 156315 [details] [review]
Bug 34737: (follow-up) Code more defensively

This patch adds some defense for errant preference lines, allowing for
empty lines, comment lines and even skipping easily identified malformed
lines.

Signed-off-by: Toni Ford <Toni.Ford@newcastle.gov.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 24 Tomás Cohen Arazi 2023-10-04 15:09:38 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 25 Fridolin Somers 2023-10-06 00:32:17 UTC
Enhancement not pushed to 23.05.x
Comment 26 Martin Renvoize 2023-11-08 17:07:05 UTC
Documented alongside the preference.. opting to mark as RESOLVED FIXED :)
Comment 27 Aude Charillon 2023-11-08 17:21:50 UTC
The Manual includes the system preferences descriptions. So if the description was updated in Koha, the Manual also needs to be updated. Changing the status back to Needs documenting.
Comment 28 Caroline Cyr La Rose 2023-11-10 14:56:18 UTC
Aude added it in the manual. Thanks Aude!