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>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

diff --git a/C4/SIP/ILS/Transaction/Checkin.pm b/C4/SIP/ILS/Transaction/Checkin.pm
index 9fa73ed..0e3babe 100644 (file)
--- 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:
 
- <branchcode>:<item field>:<comparitor>:<item field value>:<sort bin number>
+ <branchcode>:<item field>:<comparator>:<item field value>:<sort bin number>
 
 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 012341a..7a05ccd 100644 (file)
--- 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.<br/>"
+            - "The mapping should be of the form 'branchcode:item field:comparator:item field value:sort bin number', with one mapping per line.<br/>"
+            - "- 'branchcode' is the location where the checkin is being performed (i.e. branch assigned to SIP user)<br/>"
+            - "- 'item field' is a database column in the items table<br/>"
+            - "- 'comparator' is the type of comparison, possible values are: eq,<,<=,>,>=,ne<br/>"
+            - "- 'item field value' is the value to compare against the value in the specified 'item field'<br/>"
+            - "- 'sort bin number' is the expected return value in the CL field of the SIP response for an item matching a rule<br/><br/>"
+            - "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:<br/>"
+            - "i.e. \\$holdingbranch<br/></br>"
+            - "Examples:<br/>"
+            - "CPL:itype:eq:BOOK:1 - Will return sort bin 1 for an item of itemtype 'BOOK' returned to CPL.<br/>"
+            - "CPL:itemcallnumber:<:339.6:3 - Will return sort bin 3 for an item with a callnumber less than 339.6 returned to CPL .<br/>"
+            - "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).<br/><br/>"
             - pref: SIP2SortBinMapping
               type: textarea
               class: long

diff --git a/t/db_dependent/SIP/Transaction.t b/t/db_dependent/SIP/Transaction.t
index d0763f8..b54ca6f 100755 (executable)
--- 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 {