Bug 14576: Allow arbitrary automatic update of location on checkin
authorNick Clemens <nick@bywatersolutions.com>
Wed, 4 Oct 2017 15:19:26 +0000 (15:19 +0000)
committerNick Clemens <nick@bywatersolutions.com>
Thu, 25 Apr 2019 11:36:23 +0000 (11:36 +0000)
This patch adds a new syspref "UpdateItemLocationOnCheckin" which
accepts pairs of shelving locations.  On check-in the items location is
compared ot the location on the left and, if it matches, is updated to
the location on the left.

This preference replaces ReturnToShelvingCart and
InProcessingToShelvingCart preferences.  The update statement should
insert values that replciate these functions.  Note existing
functionality of all items in PROC location being returned to
permanent_location is preserved by default.  Also, any items issued from
CART location will be returned to their permanent location on issue (if
it differs)

Special values for this pref are:
_ALL_ - used on left side only to affect all items
_BLANK_ - used on either side to match on/set to blank (actual blanks
        will work, but this is an easier to read option)
_PERM_ - used on right side only to return items to permanent location

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Set the new system preference UpdateitemLocationOnCheckin
   to the following (assuming sample data):
   NEW: FIC
   FIC: GEN
4) Create an item, set its location to NEW
5) Check in the item, note its location is now FIC
6) Check in the item again, note its location is now GEN
7) Check in the item again, note its location remains GEN
8) Test using _ALL_, _BLANK_ and _PERM_ for updates
9) Try entering various incorrect syntax in the pref and note you are warned

Sponsored by:
    Arcadia Public Library (http://library.ci.arcadia.ca.us/)
    Middletown Township Public Library (http://www.mtpl.org/)
    Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/)

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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

12 files changed:
C4/Circulation.pm
C4/Items.pm
C4/Reserves.pm
C4/UsageStats.pm
circ/returns.pl
installer/data/mysql/atomicupdate/bug_14576_add_UpdateItemLocationOnCheckin.perl [new file with mode: 0644]
installer/data/mysql/sysprefs.sql
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences.tt
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
koha-tmpl/intranet-tmpl/prog/js/pages/preferences.js
svc/checkin

index 53b74bb..5514298 100644 (file)
@@ -1428,8 +1428,8 @@ sub AddIssue {
                 )->store;
             }
 
-            if ( C4::Context->preference('ReturnToShelvingCart') ) {
-                # ReturnToShelvingCart is on, anything issued should be taken off the cart.
+            if ( $item_object->location eq 'CART' && $item_object->permanent_location ne 'CART'  ) {
+            ## Item was moved to cart via UpdateItemLocationOnCheckin, anything issued should be taken off the cart.
                 CartToShelf( $item_object->itemnumber );
             }
 
@@ -1876,17 +1876,6 @@ sub AddReturn {
     }
 
     my $item_unblessed = $item->unblessed;
-    if ( $item->location eq 'PROC' ) {
-        if ( C4::Context->preference("InProcessingToShelvingCart") ) {
-            $item_unblessed->{location} = 'CART';
-        }
-        else {
-            $item_unblessed->{location} = $item->permanent_location;
-        }
-
-        ModItem( $item_unblessed, $item->biblionumber, $item->itemnumber, { log_action => 0 } );
-    }
-
         # full item data, but no borrowernumber or checkout info (no issue)
     my $hbr = GetBranchItemRule($item->homebranch, $itemtype)->{'returnbranch'} || "homebranch";
         # get the proper branch to which to return the item
@@ -1896,6 +1885,37 @@ sub AddReturn {
     my $borrowernumber = $patron ? $patron->borrowernumber : undef;    # we don't know if we had a borrower or not
     my $patron_unblessed = $patron ? $patron->unblessed : {};
 
+    my $yaml_loc = C4::Context->preference('UpdateItemLocationOnCheckin');
+    if ($yaml_loc) {
+        $yaml_loc = "$yaml_loc\n\n";  # YAML is strict on ending \n. Surplus does not hurt
+        my $rules;
+        eval { $rules = YAML::Load($yaml_loc); };
+        if ($@) {
+            warn "Unable to parse UpdateItemLocationOnCheckin syspref : $@";
+        }
+        else {
+            if (defined $rules->{_ALL_}) {
+                if ($rules->{_ALL_} eq '_PERM_') { $rules->{_ALL_} = $item->{permanent_location}; }
+                if ($rules->{_ALL_} eq '_BLANK_') { $rules->{_ALL_} = ''; }
+                if ( $item->{location} ne $rules->{_ALL_}) {
+                    $messages->{'ItemLocationUpdated'} = { from => $item->{location}, to => $rules->{_ALL_} };
+                    ModItem( { location => $rules->{_ALL_} }, undef, $itemnumber );
+                }
+            }
+            else {
+                foreach my $key ( keys %$rules ) {
+                    if ( $rules->{$key} eq '_PERM_' ) { $rules->{$key} = $item->{permanent_location}; }
+                    if ( $rules->{$key} eq '_BLANK_') { $rules->{$key} = '' ;}
+                    if ( ($item->{location} eq $key && $item->{location} ne $rules->{$key}) || ($key eq '_BLANK_' && $item->{location} eq '' && $rules->{$key} ne '') ) {
+                        $messages->{'ItemLocationUpdated'} = { from => $item->{location}, to => $rules->{$key} };
+                        ModItem( { location => $rules->{$key} }, undef, $itemnumber );
+                        last;
+                    }
+                }
+            }
+        }
+    }
+
     my $yaml = C4::Context->preference('UpdateNotForLoanStatusOnCheckin');
     if ($yaml) {
         $yaml = "$yaml\n\n";  # YAML is anal on ending \n. Surplus does not hurt
@@ -1987,15 +2007,12 @@ sub AddReturn {
             );
             $sth->execute( $item->itemnumber );
             # if we have a reservation with valid transfer, we can set it's status to 'W'
-            ShelfToCart( $item->itemnumber ) if ( C4::Context->preference("ReturnToShelvingCart") );
             C4::Reserves::ModReserveStatus($item->itemnumber, 'W');
         } else {
             $messages->{'WrongTransfer'}     = $tobranch;
             $messages->{'WrongTransferItem'} = $item->itemnumber;
         }
         $validTransfert = 1;
-    } else {
-        ShelfToCart( $item->itemnumber ) if ( C4::Context->preference("ReturnToShelvingCart") );
     }
 
     # fix up the accounts.....
index 14fcdcf..f4e2d14 100644 (file)
@@ -47,7 +47,6 @@ BEGIN {
         DelItemCheck
         MoveItemFromBiblio
         CartToShelf
-        ShelfToCart
         GetAnalyticsCount
         SearchItemsByField
         SearchItems
@@ -141,25 +140,6 @@ sub CartToShelf {
     }
 }
 
-=head2 ShelfToCart
-
-  ShelfToCart($itemnumber);
-
-Set the current shelving location of the item
-to shelving cart ('CART').
-
-=cut
-
-sub ShelfToCart {
-    my ( $itemnumber ) = @_;
-
-    unless ( $itemnumber ) {
-        croak "FAILED ShelfToCart() - no itemnumber supplied";
-    }
-
-    ModItem({ location => 'CART'}, undef, $itemnumber);
-}
-
 =head2 AddItemFromMarc
 
   my ($biblionumber, $biblioitemnumber, $itemnumber) 
@@ -566,9 +546,10 @@ sub ModItemTransfer {
     my ( $itemnumber, $frombranch, $tobranch ) = @_;
 
     my $dbh = C4::Context->dbh;
+    my $item = GetItem( $itemnumber );
 
     # Remove the 'shelving cart' location status if it is being used.
-    CartToShelf( $itemnumber ) if ( C4::Context->preference("ReturnToShelvingCart") );
+    CartToShelf( $itemnumber ) if ( $item->{'location'} eq 'CART' && $item->{'permanent_location'} ne 'CART' );
 
     $dbh->do("UPDATE branchtransfers SET datearrived = NOW(), comments = ? WHERE itemnumber = ? AND datearrived IS NULL", undef, "Canceled, new transfer from $frombranch to $tobranch created", $itemnumber);
 
index 621e008..e2e08c7 100644 (file)
@@ -1029,7 +1029,8 @@ sub ModReserveStatus {
     my $sth_set = $dbh->prepare($query);
     $sth_set->execute( $newstatus, $itemnumber );
 
-    if ( C4::Context->preference("ReturnToShelvingCart") && $newstatus ) {
+    my $item = GetItem($itemnumber);
+    if ( ( $item->{'location'} eq 'CART' && $item->{'permanent_location'} ne 'CART'  ) && $newstatus ) {
       CartToShelf( $itemnumber );
     }
 }
@@ -1081,9 +1082,9 @@ sub ModReserveAffect {
       if ( !$transferToDo && !$already_on_shelf );
 
     _FixPriority( { biblionumber => $biblionumber } );
-
-    if ( C4::Context->preference("ReturnToShelvingCart") ) {
-        CartToShelf($itemnumber);
+    my $item = GetItem($itemnumber);
+    if ( ( $item->{'location'} eq 'CART' && $item->{'permanent_location'} ne 'CART'  ) ) {
+      CartToShelf( $itemnumber );
     }
 
     return;
index 4f66111..f205767 100644 (file)
@@ -144,7 +144,6 @@ sub BuildReport {
         AutoRemoveOverduesRestrictions
         CircControl
         HomeOrHoldingBranch
-        InProcessingToShelvingCart
         IssueLostItem
         IssuingInProcess
         ManInvInNoissuesCharge
@@ -153,7 +152,6 @@ sub BuildReport {
         RenewalSendNotice
         RentalsInNoissuesCharge
         ReturnBeforeExpiry
-        ReturnToShelvingCart
         TransfersMaxDaysWarning
         UseBranchTransferLimits
         useDaysMode
index 6072e72..657229a 100755 (executable)
@@ -533,6 +533,9 @@ foreach my $code ( keys %$messages ) {
     elsif ( $code eq 'ForeverDebarred' ) {
         $err{foreverdebarred}        = $messages->{'ForeverDebarred'};
     }
+    elsif ( $code eq 'ItemLocationUpdated' ) {
+        $err{ItemLocationUpdated} = $messages->{ItemLocationUpdated};
+    }
     elsif ( $code eq 'NotForLoanStatusUpdated' ) {
         $err{NotForLoanStatusUpdated} = $messages->{NotForLoanStatusUpdated};
     }
diff --git a/installer/data/mysql/atomicupdate/bug_14576_add_UpdateItemLocationOnCheckin.perl b/installer/data/mysql/atomicupdate/bug_14576_add_UpdateItemLocationOnCheckin.perl
new file mode 100644 (file)
index 0000000..bb16511
--- /dev/null
@@ -0,0 +1,20 @@
+$DBversion = 'XXX';  # will be replaced by the RM
+if( CheckVersion( $DBversion ) ) {
+    $dbh->do(q{
+        INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('UpdateItemLocationOnCheckin', 'PROC: _PERM_\n', 'NULL', 'This a list of value pairs. When an item is checked in, if the location value on the left matches the items location value t will be updated to the right-hand value. E.g. ''PROC: FIC'' will cause an item that was set to ''Book Cart'' to now be in the ''Fiction'' shelving location. Note that PROC and CART are special values, for these locations only can location and permanent_location differ.  In all other cases an update will affect both.  Items in the CART location will be returned to their permanent location on checkout.  You can also use the special term _BLANK_ on either side of a pair to update/remove items with no locaiton assigned.  You can use the special term _ALL_ on the left side to affect all items and the special term _PERM_ on the right side to return items to their permanent location', 'Free');
+    });
+    $dbh->do(q{
+        UPDATE systempreferences s1, (SELECT IF(value,'PROC: CART\n','') AS p2c FROM systempreferences WHERE variable='InProcessingToShelvingCart') s2 SET s1.value= CONCAT(s2.p2c, REPLACE(s1.value,'PROC: _PERM_\n','') ) WHERE s1.variable='UpdateItemLocationOnCheckin' AND s1.value NOT LIKE '%PROC: CART%';
+    });
+    $dbh->do(q{
+        DELETE FROM systempreferences WHERE variable='InProcessingToShelvingCart';
+    });
+    $dbh->do(q{
+        UPDATE systempreferences s1, (SELECT IF(value,'_ALL_: CART\n','') AS rtc FROM systempreferences WHERE variable='ReturnToShelvingCart') s2 SET s1.value= CONCAT(s2.rtc,s1.value) WHERE s1.variable='UpdateItemLocationOnCheckin' AND s1.value NOT LIKE '%_ALL_: CART%';
+    });
+    $dbh->do(q{
+        DELETE FROM systempreferences WHERE variable='ReturnToShelvingCart';
+    });
+    SetVersion( $DBversion );
+    print "Upgrade to $DBversion done (Bug 14576: Add UpdateItemLocationOnCheckin syspref)\n";
+}
index 35c9ff0..b309fa6 100644 (file)
@@ -234,7 +234,6 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
 ('RecordedBooksLibraryID','','','Library ID for RecordedBooks integration','Integer'),
 ('OnSiteCheckouts','0','','Enable/Disable the on-site checkouts feature','YesNo'),
 ('OnSiteCheckoutsForce','0','','Enable/Disable the on-site for all cases (Even if a user is debarred, etc.)','YesNo'),
-('InProcessingToShelvingCart','0','','If set, when any item with a location code of PROC is \'checked in\', it\'s location code will be changed to CART.','YesNo'),
 ('INTRAdidyoumean','',NULL,'Did you mean? configuration for the Intranet. Do not change, as this is controlled by /cgi-bin/koha/admin/didyoumean.pl.','Free'),
 ('IntranetBiblioDefaultView','normal','normal|marc|isbd|labeled_marc','Choose the default detail view in the staff interface; choose between normal, labeled_marc, marc or isbd','Choice'),
 ('intranetbookbag','1','','If ON, enables display of Cart feature in the intranet','YesNo'),
@@ -510,7 +509,6 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
 ('ReturnBeforeExpiry','0',NULL,'If ON, checkout will be prevented if returndate is after patron card expiry','YesNo'),
 ('ReturnLog','1',NULL,'If ON, enables the circulation (returns) log','YesNo'),
 ('ReturnpathDefault','',NULL,'Use this email address as return path or bounce address for undeliverable emails','Free'),
-('ReturnToShelvingCart','0','','If set, when any item is \'checked in\', it\'s location code will be changed to CART.','YesNo'),
 ('reviewson','1','','If ON, enables patron reviews of bibliographic records in the OPAC','YesNo'),
 ('RisExportAdditionalFields',  '', NULL ,  'Define additional RIS tags to export from MARC records in YAML format as an associative array with either a marc tag/subfield combination as the value, or a list of tag/subfield combinations.',  'textarea'),
 ('RoutingListAddReserves','0','','If ON the patrons on routing lists are automatically added to holds on the issue.','YesNo'),
@@ -613,6 +611,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
 ('UpdateItemWhenLostFromHoldList','',NULL,'This is a list of values to update an item when it is marked as lost from the holds to pull screen','Free'),
 ('UniqueItemFields','barcode','','Space-separated list of fields that should be unique (used in acquisition module for item creation). Fields must be valid SQL column names of items table','Free'),
 ('UnsubscribeReflectionDelay','',NULL,'Delay for locking unsubscribers', 'Integer'),
+('UpdateItemLocationOnCheckin', '', 'NULL', 'This is a list of value pairs. When an item is checked in, if the location value on the left matches the items location value it will be updated to the right-hand value. E.g. ''PROC: FIC'' will cause an item that was set to ''Book Cart'' to now be in the ''Fiction'' shelving location. Note that PROC and CART are special values, for these locations only can location and permanent_location differ.  In all other cases an update will affect both.  Items in the CART location will be returned to their permanent location on checkout.  You can also use the special term _BLANK_ on either side of a pair to update/remove items with no locaiton assigned.  You can use the special term _ALL_ on the left side to affect all items and the special term _PERM_ on the right side to return items to their permanent location', 'Free'),
 ('UpdateNotForLoanStatusOnCheckin', '', 'NULL', 'This is a list of value pairs. When an item is checked in, if the not for loan value on the left matches the items not for loan value it will be updated to the right-hand value. E.g. ''-1: 0'' will cause an item that was set to ''Ordered'' to now be available for loan. Each pair of values should be on a separate line.', 'Free'),
 ('UpdateTotalIssuesOnCirc','0',NULL,'Whether to update the totalissues field in the biblio on each circ.','YesNo'),
 ('UploadPurgeTemporaryFilesDays','',NULL,'If not empty, number of days used when automatically deleting temporary uploads','integer'),
index 205fd2f..54d27fe 100644 (file)
         var MSG_SESSION_TIMED_OUT = _( "You need to log in again, your session has timed out" );
         var MSG_DATA_NOT_SAVED = _( "Error; your data might not have been saved" );
         var MSG_LOADING = _( "Loading..." );
+        var MSG_ALL_VALUE_WARN = _("Note: _ALL_ value will override all other values");
+        var MSG_UPD_LOC_FORMAT_WARN = _("The following values are not formatted correctly:");
     </script>
     [% Asset.js("lib/jquery/plugins/humanmsg.js") | $raw %]
     [% Asset.js("js/ajax.js") | $raw %]
index 41c7d77..a187e22 100644 (file)
@@ -222,20 +222,6 @@ Circulation:
                   no: "Don't allow"
             - staff to manually override and check out items to patrons who have more than noissuescharge in fines.
         -
-            - pref: InProcessingToShelvingCart
-              choices:
-                  yes: Move
-                  no: "Don't move"
-            - items that have the location PROC to the location CART when they are checked in.
-            - "<br><strong>NOTE:</strong> This system preference requires the <code>misc/cronjobs/cart_to_shelf.pl</code> cronjob. Ask your system administrator to schedule it."
-        -
-            - pref: ReturnToShelvingCart
-              choices:
-                  yes: Move
-                  no: "Don't move"
-            - all items to the location CART when they are checked in.
-            - "<br><strong>NOTE:</strong> This system preference requires the <code>misc/cronjobs/cart_to_shelf.pl</code> cronjob. Ask your system administrator to schedule it."
-        -
             - pref: AutomaticItemReturn
               choices:
                   yes: Do
@@ -538,6 +524,21 @@ Circulation:
             - calculate and update overdue charges when an item is returned.
             - "<br /><strong>NOTE: If you are doing hourly loans then you should have this on.</strong>"
         -
+            - pref: UpdateItemLocationOnCheckin
+              type: textarea
+              class: code
+            - This is is a list of value pairs, the first value is followed immediately by colon space then the second value i.e.:<br/>
+            - "PROC: FIC"
+            - <br/> When an item is checked in, if the location value on the left matches the items location value
+            - "it will be updated to the right-hand value.<br/> E.g. ''PROC: FIC'' will cause an item that was set to ''Book Cart'' to now be in the ''Fiction'' shelving location."
+            - <br/>Note that PROC and CART are special values, for these locations only can location and permanent_location differ.  In all other cases an update will affect both.
+            - <br/>Items in the CART location will be returned to their permanent location on checkout
+            - <br/>You can use the special term _BLANK_ on either side of a pair to update/remove items with no location assigned
+            - <br>You can use the special term _ALL_ on the left side to affect all items
+            - and the special term _PERM_ on the right side to return items to their permanent location
+            - <br>**Use of an _ALL_ rule will override/ignore any other values**
+            - <br>Each pair of values should be on a separate line.
+        -
             - pref: UpdateNotForLoanStatusOnCheckin
               type: textarea
               class: code
index 7f80f6e..fcca6be 100644 (file)
                     [% END %]
                 </p>
             [% END %]
+            [% IF ( errmsgloo.ItemLocationUpdated ) %]
+                 <p class="problem">
+                     Item shelving location updated.
+                    <br />Old value:
+                    [% IF errmsgloo.ItemLocationUpdated.from %]
+                        [% IF errmsgloo.ItemLocationUpdated.from == '' %]
+                            empty
+                        [% ELSIF AuthorisedValues.GetByCode( 'LOC', errmsgloo.ItemLocationUpdated.from ) == '' %]
+                            [% errmsgloo.ItemLocationUpdated.from | html %] (No description available)
+                        [% ELSE %]
+                            [% AuthorisedValues.GetByCode( 'LOC', errmsgloo.ItemLocationUpdated.from ) | html %]
+                        [% END %]
+                    [% ELSE %]
+                        "Blank"
+                    [% END %]
+                    <br />New value:
+                    [% IF errmsgloo.ItemLocationUpdated.to %]
+                        [% IF errmsgloo.ItemLocationUpdated.to == '' %]
+                            empty
+                        [% ELSIF AuthorisedValues.GetByCode( 'LOC', errmsgloo.ItemLocationUpdated.to ) == '' %]
+                            [% errmsgloo.ItemLocationUpdated.to | html %] (Not an authorized value)
+                        [% ELSE %]
+                            [% AuthorisedValues.GetByCode( 'LOC', errmsgloo.ItemLocationUpdated.to ) | html %]
+                        [% END %]
+                    [% ELSE %]
+                        "Blank"
+                    [% END %]
+                 </p>
+            [% END %]
             [% IF ( errmsgloo.badbarcode ) %]
                 <p class="problem">No item with barcode: [% errmsgloo.msg | html %]</p>
             [% END %]
                 [% ELSE %]
                     <p class="problem">Item was lost, now found.</p>
                 [% END %]
-
                 [% IF LostItemFeeRefunded and not Koha.Preference('BlockReturnOfLostItems') %]
                     <p class="problem">A refund has been applied to the borrowing patron's account.</p>
                 [% ELSIF Koha.Preference('BlockReturnOfLostItems') %]
index f666a91..0fa0238 100644 (file)
@@ -162,4 +162,23 @@ $( document ).ready( function () {
     if ( search_jumped ) {
         document.location.hash = "jumped";
     }
+
+    $("#pref_UpdateItemLocationOnCheckin").change(function(){
+        var the_text = $(this).val();
+        var alert_text = '';
+        if ( the_text.indexOf('_ALL_:') != -1 ) alert_text = MSG_ALL_VALUE_WARN + '\n';
+        var split_text  =the_text.split("\n");
+        var alert_issues = '';
+        var issue_count = 0;
+        var reg_check = /.*:\s.*/;
+        for (var i=0; i < split_text.length; i++){
+            if ( !split_text[i].match(reg_check) && split_text[i].length ) {
+                alert_issues+=split_text[i]+"\n";
+                issue_count++;
+            }
+        }
+        if (issue_count) alert_text += "\n"+ MSG_UPD_LOC_FORMAT_WARN  +"\n"+alert_issues;
+        if ( alert_text.length )  alert(alert_text);
+    });
+
 } );
index e0dc21c..9b41e33 100755 (executable)
@@ -63,18 +63,6 @@ $data->{itemnumber}     = $itemnumber;
 $data->{borrowernumber} = $borrowernumber;
 $data->{branchcode}     = $branchcode;
 
-if ( C4::Context->preference("InProcessingToShelvingCart") ) {
-    my $item = Koha::Items->find($itemnumber);
-    if ( $item->location eq 'PROC' ) {
-        ModItem( { location => 'CART' }, $item->biblionumber, $item->itemnumber );
-    }
-}
-
-if ( C4::Context->preference("ReturnToShelvingCart") ) {
-    my $item = Koha::Items->find($itemnumber);
-    ModItem( { location => 'CART' }, $item->biblionumber, $item->itemnumber );
-}
-
 my $checkout = Koha::Checkouts->find({ itemnumber => $itemnumber });
 $data->{patronnote} = $checkout ? $checkout->note : q||;