From 29f0aefcc7c25a2a7c1ab2310fff8571d8ccfc21 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 30 Mar 2022 11:09:04 +0000 Subject: [PATCH] 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 --- C4/SIP/ILS/Transaction/Checkin.pm | 17 ++++++++--------- .../modules/admin/preferences/circulation.pref | 14 +++++++++++++- t/db_dependent/SIP/Transaction.t | 9 +++++---- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/C4/SIP/ILS/Transaction/Checkin.pm b/C4/SIP/ILS/Transaction/Checkin.pm index 9fa73eda0d..0e3babe023 100644 --- a/C4/SIP/ILS/Transaction/Checkin.pm +++ b/C4/SIP/ILS/Transaction/Checkin.pm @@ -204,7 +204,7 @@ value that should be returned for an item checked in via SIP2. The mapping should be: - :::: + :::: For example: @@ -242,10 +242,9 @@ sub _get_sort_bin { # Iterate over the mapping. The first hit wins. my $rule = 0; foreach my $line (@lines) { - warn "Rule: " . $rule++ . " - " . $line . "\n"; # Split the line into fields - my ( $branchcode, $item_property, $comparitor, $value, $sort_bin ) = + my ( $branchcode, $item_property, $comparator, $value, $sort_bin ) = split /:/, $line; if ( $value =~ s/^\$// ) { $value = $item->$value; @@ -253,22 +252,22 @@ sub _get_sort_bin { # Check the fields against values in the item if ( $branch eq $branchcode ) { my $property = $item->$item_property; - if ( ( $comparitor eq 'eq' || $comparitor eq '=' ) && ( $property eq $value ) ) { + if ( ( $comparator eq 'eq' || $comparator eq '=' ) && ( $property eq $value ) ) { return $sort_bin; } - if ( ( $comparitor eq 'ne' || $comparitor eq '!=' ) && ( $property ne $value ) ) { + if ( ( $comparator eq 'ne' || $comparator eq '!=' ) && ( $property ne $value ) ) { return $sort_bin; } - if ( ( $comparitor eq '<' ) && ( $property < $value ) ) { + if ( ( $comparator eq '<' ) && ( $property < $value ) ) { return $sort_bin; } - if ( ( $comparitor eq '>' ) && ( $property > $value ) ) { + if ( ( $comparator eq '>' ) && ( $property > $value ) ) { return $sort_bin; } - if ( ( $comparitor eq '<=' ) && ( $property <= $value ) ) { + if ( ( $comparator eq '<=' ) && ( $property <= $value ) ) { return $sort_bin; } - if ( ( $comparitor eq '>=' ) && ( $property >= $value ) ) { + if ( ( $comparator eq '>=' ) && ( $property >= $value ) ) { return $sort_bin; } } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index 012341aa3d..7a05ccdf1a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -1287,7 +1287,19 @@ Circulation: SIP2: - - - "Use the following mappings to determine the sort_bin of a returned item. The mapping should be on the form 'branchcode:item field:item field value:sort bin number', with one mapping per line." + - "Use the following mappings to determine the sort_bin of a returned item.
" + - "The mapping should be of the form 'branchcode:item field:comparator:item field value:sort bin number', with one mapping per line.
" + - "- 'branchcode' is the location where the checkin is being performed (i.e. branch assigned to SIP user)
" + - "- 'item field' is a database column in the items table
" + - "- 'comparator' is the type of comparison, possible values are: eq,<,<=,>,>=,ne
" + - "- 'item field value' is the value to compare against the value in the specified 'item field'
" + - "- 'sort bin number' is the expected return value in the CL field of the SIP response for an item matching a rule

" + - "NOTE: Specifying 'item_field_value' with a leading '\\$' and an item field name will use the value of that field in the item for comparison:
" + - "i.e. \\$holdingbranch

" + - "Examples:
" + - "CPL:itype:eq:BOOK:1 - Will return sort bin 1 for an item of itemtype 'BOOK' returned to CPL.
" + - "CPL:itemcallnumber:<:339.6:3 - Will return sort bin 3 for an item with a callnumber less than 339.6 returned to CPL .
" + - "CPL:homebranch:ne:\\$holdingbranch:X - Wil return sort bin 'X' for an item returned to CPL where the holdingbranch is not equal to the homebranch (i.e. any item belonging to a different branch than CPL).

" - pref: SIP2SortBinMapping type: textarea class: long diff --git a/t/db_dependent/SIP/Transaction.t b/t/db_dependent/SIP/Transaction.t index d0763f8a64..b54ca6f085 100755 --- a/t/db_dependent/SIP/Transaction.t +++ b/t/db_dependent/SIP/Transaction.t @@ -564,16 +564,17 @@ RULES # Set holdingbranch as though item returned to library other than homebranch (As AddReturn would) $item_cd->holdingbranch($library2->branchcode)->store(); $bin = C4::SIP::ILS::Transaction::Checkin::_get_sort_bin( $item_cd, $library2->branchcode ); - is($bin, 'X', "Item parameter on RHS of comparison works (ne comparitor)"); + is($bin, 'X', "Item parameter on RHS of comparison works (ne comparator)"); # Reset holdingbranch as though item returned to home library $item_cd->holdingbranch($library->branchcode)->store(); $bin = C4::SIP::ILS::Transaction::Checkin::_get_sort_bin( $item_cd, $library->branchcode ); - is($bin, '0', "Fixed value on RHS of comparison works (eq comparitor)"); + is($bin, '0', "Fixed value on RHS of comparison works (eq comparator)"); $bin = C4::SIP::ILS::Transaction::Checkin::_get_sort_bin( $item_book, $library->branchcode ); - is($bin, '1', "Rules applied in order (< comparitor)"); + is($bin, '1', "Rules applied in order (< comparator)"); $item_book->itemcallnumber('350.20')->store(); - is($bin, '2', "Rules applied in order (< comparitor)"); + $bin = C4::SIP::ILS::Transaction::Checkin::_get_sort_bin( $item_book, $library->branchcode ); + is($bin, '2', "Rules applied in order (< comparator)"); }; subtest item_circulation_status => sub { -- 2.30.2