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.
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
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.
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.
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|
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.
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.
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>
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>
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>
Created attachment 155666 [details] [review] Bug 34737: Tidy t/db_dependent/SIP/Transaction.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 156147 [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.
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.
Created attachment 156148 [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.
Created attachment 156170 [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: Toni Ford <Toni.Ford@newcastle.gov.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 156171 [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: Toni Ford <Toni.Ford@newcastle.gov.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 156172 [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: Toni Ford <Toni.Ford@newcastle.gov.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 156173 [details] [review] Bug 34737: Tidy t/db_dependent/SIP/Transaction.t Signed-off-by: Toni Ford <Toni.Ford@newcastle.gov.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 156174 [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>
Created attachment 156175 [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>
Created attachment 156176 [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>
Created attachment 156177 [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>
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>
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>
Pushed to master for 23.11. Nice work everyone, thanks!
Enhancement not pushed to 23.05.x
Documented alongside the preference.. opting to mark as RESOLVED FIXED :)
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.
Aude added it in the manual. Thanks Aude!