Bug 23463: Replace ModItem with Koha::Item->store
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Mon, 18 Mar 2019 17:56:19 +0000 (14:56 -0300)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Mon, 23 Mar 2020 09:26:30 +0000 (09:26 +0000)
Starting to replace the ModItem calls with Koha::Item->store

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

23 files changed:
C4/Circulation.pm
C4/CourseReserves.pm
C4/Items.pm
Koha/Item.pm
acqui/finishreceive.pl
catalogue/updateitem.pl
cataloguing/additem.pl
circ/pendingreserves.pl
misc/cronjobs/longoverdue.pl
misc/maintenance/touch_all_items.pl
misc/migration_tools/fix_onloan.pl
offline_circ/process_koc.pl
serials/serials-edit.pl
svc/checkin
t/db_dependent/Circulation.t
t/db_dependent/Circulation/Returns.t
t/db_dependent/Holds.t
t/db_dependent/Items.t
t/db_dependent/Items/AutomaticItemModificationByAge.t
t/db_dependent/Items_DelItemCheck.t
t/db_dependent/Labels/t_Label.t
t/db_dependent/Reserves.t
tools/inventory.pl

index b152b50..49a81e7 100644 (file)
@@ -1462,18 +1462,12 @@ sub AddIssue {
                 }
             }
 
-            ModItem(
-                {
-                    issues        => ( $item_object->issues || 0 ) + 1,
-                    holdingbranch => C4::Context->userenv->{'branch'},
-                    itemlost      => 0,
-                    onloan        => $datedue->ymd(),
-                    datelastborrowed => DateTime->now( time_zone => C4::Context->tz() )->ymd(),
-                },
-                $item_object->biblionumber,
-                $item_object->itemnumber,
-                { log_action => 0 }
-            );
+            $item_object->issues( ( $item_object->issues || 0 ) + 1);
+            $item_object->holdingbranch(C4::Context->userenv->{'branch'});
+            $item_object->itemlost(0);
+            $item_object->onloan($datedue->ymd());
+            $item_object->datelastborrowed(DateTime->now( time_zone => C4::Context->tz() )->ymd()); # FIXME we should use dt_from_string here
+            $item_object->store({log_action => 0});
             ModDateLastSeen( $item_object->itemnumber );
 
             # If it costs to borrow this book, charge it to the patron's account.
@@ -1883,7 +1877,8 @@ sub AddReturn {
                 . Dumper($issue->unblessed) . "\n";
     } else {
         $messages->{'NotIssued'} = $barcode;
-        ModItem({ onloan => undef }, $item->biblionumber, $item->itemnumber) if defined $item->onloan;
+        $item->onloan(undef)->store if defined $item->onloan;
+
         # even though item is not on loan, it may still be transferred;  therefore, get current branch info
         $doreturn = 0;
         # No issue, no borrowernumber.  ONLY if $doreturn, *might* you have a $borrower later.
@@ -1894,7 +1889,6 @@ sub AddReturn {
         }
     }
 
-    my $item_unblessed = $item->unblessed;
         # 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
@@ -1913,7 +1907,7 @@ sub AddReturn {
             if ($update_loc_rules->{_ALL_} eq '_BLANK_') { $update_loc_rules->{_ALL_} = ''; }
             if ( $item->location ne $update_loc_rules->{_ALL_}) {
                 $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{_ALL_} };
-                ModItem( { location => $update_loc_rules->{_ALL_} }, undef, $itemnumber );
+                $item->location($update_loc_rules->{_ALL_})->store;
             }
         }
         else {
@@ -1922,7 +1916,7 @@ sub AddReturn {
                 if ( $update_loc_rules->{$key} eq '_BLANK_') { $update_loc_rules->{$key} = '' ;}
                 if ( ($item->location eq $key && $item->location ne $update_loc_rules->{$key}) || ($key eq '_BLANK_' && $item->location eq '' && $update_loc_rules->{$key} ne '') ) {
                     $messages->{'ItemLocationUpdated'} = { from => $item->location, to => $update_loc_rules->{$key} };
-                    ModItem( { location => $update_loc_rules->{$key} }, undef, $itemnumber );
+                    $item->location($update_loc_rules->{$key})->store;
                     last;
                 }
             }
@@ -1941,7 +1935,7 @@ sub AddReturn {
             foreach my $key ( keys %$rules ) {
                 if ( $item->notforloan eq $key ) {
                     $messages->{'NotForLoanStatusUpdated'} = { from => $item->notforloan, to => $rules->{$key} };
-                    ModItem( { notforloan => $rules->{$key} }, undef, $itemnumber, { log_action => 0 } );
+                    $item->notforloan($rules->{$key})->store({ log_action => 0 });
                     last;
                 }
             }
@@ -1949,7 +1943,7 @@ sub AddReturn {
     }
 
     # check if the return is allowed at this branch
-    my ($returnallowed, $message) = CanBookBeReturned($item_unblessed, $branch);
+    my ($returnallowed, $message) = CanBookBeReturned($item->unblessed, $branch);
     unless ($returnallowed){
         $messages->{'Wrongbranch'} = {
             Wrongbranch => $branch,
@@ -1979,7 +1973,7 @@ sub AddReturn {
             };
             unless ( $@ ) {
                 if ( C4::Context->preference('CalculateFinesOnReturn') && !$item->itemlost ) {
-                    _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed, return_date => $return_date } );
+                    _CalculateAndUpdateFine( { issue => $issue, item => $item->unblessed, borrower => $patron_unblessed, return_date => $return_date } );
                 }
             } else {
                 carp "The checkin for the following issue failed, Please go to the about page, section 'data corrupted' to know how to fix this problem ($@)" . Dumper( $issue->unblessed );
@@ -1992,7 +1986,7 @@ sub AddReturn {
 
         }
 
-        ModItem( { onloan => undef }, $item->biblionumber, $item->itemnumber, { log_action => 0 } );
+        $item->onloan(undef)->store({ log_action => 0 });
     }
 
     # the holdingbranch is updated if the document is returned to another location.
@@ -2000,7 +1994,7 @@ sub AddReturn {
     my $item_holding_branch = $item->holdingbranch;
     if ($item->holdingbranch ne $branch) {
         UpdateHoldingbranch($branch, $item->itemnumber);
-        $item_unblessed->{'holdingbranch'} = $branch; # update item data holdingbranch too # FIXME I guess this is for the _debar_user_on_return call later
+        $item->holdingbranch($branch); # update item data holdingbranch too # FIXME I guess this is for the _debar_user_on_return call later
     }
 
     my $leave_item_lost = C4::Context->preference("BlockReturnOfLostItems") ? 1 : 0;
@@ -2054,7 +2048,7 @@ sub AddReturn {
 
         if ( $issue and $issue->is_overdue ) {
         # fix fine days
-            my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item_unblessed, dt_from_string($issue->date_due), $return_date );
+            my ($debardate,$reminder) = _debar_user_on_return( $patron_unblessed, $item->unblessed, dt_from_string($issue->date_due), $return_date );
             if ($reminder){
                 $messages->{'PrevDebarred'} = $debardate;
             } else {
@@ -2113,7 +2107,7 @@ sub AddReturn {
         if ($doreturn && $circulation_alert->is_enabled_for(\%conditions)) {
             SendCirculationAlert({
                 type     => 'CHECKIN',
-                item     => $item_unblessed,
+                item     => $item->unblessed,
                 borrower => $patron->unblessed,
                 branch   => $branch,
             });
@@ -2150,7 +2144,7 @@ sub AddReturn {
              ! IsBranchTransferAllowed($branch, $returnbranch, $item->$BranchTransferLimitsType )
            )) {
             $debug and warn sprintf "about to call ModItemTransfer(%s, %s, %s, %s)", $item->itemnumber,$branch, $returnbranch, $transfer_trigger;
-            $debug and warn "item: " . Dumper($item_unblessed);
+            $debug and warn "item: " . Dumper($item->unblessed);
             ModItemTransfer($item->itemnumber, $branch, $returnbranch, $transfer_trigger);
             $messages->{'WasTransfered'} = 1;
         } else {
@@ -2240,7 +2234,7 @@ sub MarkIssueReturned {
         # And finally delete the issue
         $issue->delete;
 
-        ModItem( { 'onloan' => undef }, undef, $itemnumber, { log_action => 0 } );
+        $issue->item->onloan(undef)->store({ log_action => 0 });
 
         if ( C4::Context->preference('StoreLastBorrower') ) {
             my $item = Koha::Items->find( $itemnumber );
@@ -2542,6 +2536,8 @@ sub _FixAccountForLostAndFound {
     $accountline->discard_changes->status('FOUND');
     $accountline->store;
 
+    $accountline->item->paidfor('')->store({ log_action => 0 });
+
     if ( defined $account and C4::Context->preference('AccountAutoReconcile') ) {
         $account->reconcile_balance;
     }
@@ -3003,7 +2999,9 @@ sub AddRenewal {
 
         # Update the renewal count on the item, and tell zebra to reindex
         $renews = ( $item_object->renewals || 0 ) + 1;
-        ModItem( { renewals => $renews, onloan => $datedue->strftime('%Y-%m-%d %H:%M')}, $item_object->biblionumber, $itemnumber, { log_action => 0 } );
+        $item_object->renewals($renews);
+        $item_object->onloan($datedue);
+        $item_object->store({ log_action => 0 });
 
         # Charge a new rental fee, if applicable
         my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
@@ -3835,7 +3833,7 @@ sub LostItem{
 
     #When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch)
     if (my ( $datesent,$frombranch,$tobranch ) = GetTransfers($itemnumber)) {
-        ModItem({holdingbranch => $frombranch}, undef, $itemnumber);
+        Koha::Items->find($itemnumber)->holdingbranch($frombranch)->store;
     }
     my $transferdeleted = DeleteTransfer($itemnumber);
 }
@@ -3903,12 +3901,9 @@ sub ProcessOfflineReturn {
                 $itemnumber,
                 $operation->{timestamp},
             );
-            ModItem(
-                { renewals => 0, onloan => undef },
-                $issue->{'biblionumber'},
-                $itemnumber,
-                { log_action => 0 }
-            );
+            $item->renewals(0);
+            $item->onloan(undef);
+            $item->store({ log_action => 0 });
             return "Success.";
         } else {
             return "Item not issued.";
index 2c130af..3b9bbd4 100644 (file)
@@ -20,7 +20,6 @@ use Modern::Perl;
 use List::MoreUtils qw(any);
 
 use C4::Context;
-use C4::Items qw(ModItem);
 use C4::Circulation qw(GetOpenIssue);
 
 use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $DEBUG @FIELDS);
@@ -538,7 +537,9 @@ sub _UpdateCourseItem {
           : ()
       } @FIELDS;
 
-    ModItem( \%mod_params, undef, $course_item->{'itemnumber'} );
+    Koha::Items->find( $course_item->{itemnumber} )
+               ->set( \%mod_params )
+               ->store;
 }
 
 =head2 _ModStoredFields
@@ -587,14 +588,12 @@ sub _RevertFields {
     return unless ($ci_id);
 
     my $course_item = GetCourseItem( ci_id => $params{'ci_id'} );
-
-    my $mod_item_params;
+    my $course_item_object;
     foreach my $field ( @FIELDS ) {
         next unless defined $course_item->{$field};
-        $mod_item_params->{$field} = $course_item->{$field};
+        $course_item->$field($course_item->{$field});
     }
-
-    ModItem( $mod_item_params, undef, $course_item->{'itemnumber'} ) if $mod_item_params && %$mod_item_params;
+    $course_item_object->store;
 }
 
 =head2 _SwapAllFields
@@ -610,16 +609,14 @@ sub _SwapAllFields {
     my $course_item = GetCourseItem( ci_id => $ci_id );
     my $item = Koha::Items->find($course_item->{'itemnumber'});
 
-    my %course_item_fields;
     my %item_fields;
     foreach (@FIELDS) {
         if ( defined( $course_item->{$_} ) ) {
-            $course_item_fields{$_} = $course_item->{$_};
             $item_fields{$_}        = $item->$_ || q{};
+            $item->$_($course_item->{$_});
         }
     }
-
-    ModItem( \%course_item_fields, undef, $course_item->{'itemnumber'} ) if %course_item_fields;
+    $item->store;
     _ModStoredFields( %item_fields, ci_id => $ci_id );
 }
 
index 214f071..a5d9d85 100644 (file)
@@ -134,7 +134,7 @@ sub CartToShelf {
 
     my $item = Koha::Items->find($itemnumber);
     if ( $item->location eq 'CART' ) {
-        ModItem({ location => $item->permanent_location}, undef, $itemnumber);
+        $item->location($item->permanent_location)->store;
     }
 }
 
@@ -368,8 +368,10 @@ sub ModItemFromMarc {
     }
     my $unlinked_item_subfields = _get_unlinked_item_subfields( $localitemmarc, $frameworkcode );
 
-    ModItem( $item, $biblionumber, $itemnumber, { unlinked_item_subfields => $unlinked_item_subfields } );
-    return $item;
+    my $item_object = Koha::Items->find($itemnumber);
+    $item_object->more_subfields_xml(_get_unlinked_subfields_xml($unlinked_item_subfields))->store;
+
+    return $item_object->get_from_storage->unblessed;
 }
 
 =head2 ModItem
@@ -498,7 +500,8 @@ sub ModItemTransfer {
         VALUES (?, ?, NOW(), ?, ?)");
     $sth->execute($itemnumber, $frombranch, $tobranch, $trigger);
 
-    ModItem({ holdingbranch => $frombranch }, undef, $itemnumber, { log_action => 0 });
+    # FIXME we are fetching the item twice in the 2 next statements!
+    Koha::Items->find($itemnumber)->holdingbranch($frombranch)->store({ log_action => 0 });
     ModDateLastSeen($itemnumber);
     return;
 }
@@ -518,11 +521,10 @@ sub ModDateLastSeen {
 
     my $today = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 });
 
-    my $params;
-    $params->{datelastseen} = $today;
-    $params->{itemlost} = 0 unless $leave_item_lost;
-
-    ModItem( $params, undef, $itemnumber, { log_action => 0 } );
+    my $item = Koha::Items->find($itemnumber);
+    $item->datelastseen($today);
+    $item->itemlost(0) unless $leave_item_lost;
+    $item->store({ log_action => 0 });
 }
 
 =head2 DelItem
@@ -2508,13 +2510,16 @@ sub ToggleNewStatus {
         while ( my $values = $sth->fetchrow_hashref ) {
             my $biblionumber = $values->{biblionumber};
             my $itemnumber = $values->{itemnumber};
+            my $item = Koha::Items->find($itemnumber);
             for my $substitution ( @$substitutions ) {
+                my $field = $substitution->{item_field};
+                my $value = $substitution->{value};
                 next unless $substitution->{field};
                 next if ( defined $values->{ $substitution->{item_field} } and $values->{ $substitution->{item_field} } eq $substitution->{value} );
-                C4::Items::ModItem( { $substitution->{item_field} => $substitution->{value} }, $biblionumber, $itemnumber )
-                    unless $report_only;
+                $item->$field($value);
                 push @{ $report->{$itemnumber} }, $substitution;
             }
+            $item->store unless $report_only;
         }
     }
 
index af1444f..a38385e 100644 (file)
@@ -21,6 +21,7 @@ use Modern::Perl;
 
 use Carp;
 use List::MoreUtils qw(any);
+use Data::Dumper;
 
 use Koha::Database;
 use Koha::DateUtils qw( dt_from_string );
@@ -58,7 +59,9 @@ Koha::Item - Koha Item object class
 =cut
 
 sub store {
-    my ($self) = @_;
+    my ($self, $params) = @_;
+
+    my $log_action = $params->{log_action} // 1;
 
     # We do not want to oblige callers to pass this value
     # Dev conveniences vs performance?
@@ -71,6 +74,11 @@ sub store {
         $self->itype($self->biblio->biblioitem->itemtype);
     }
 
+    if ( $self->itemcallnumber ) { # This could be improved, we should recalculate it only if changed
+        my $cn_sort = GetClassSort($self->cn_source, $self->itemcallnumber, "");
+        $self->cn_sort($cn_sort);
+    }
+
     my $today = dt_from_string;
     unless ( $self->in_storage ) { #AddItem
         unless ( $self->permanent_location ) {
@@ -87,15 +95,56 @@ sub store {
             $self->dateaccessioned($today);
         }
 
-        if ( $self->itemcallnumber ) { # This could be improved, we should recalculate it only if changed
-            my $cn_sort = GetClassSort($self->cn_source, $self->itemcallnumber, "");
-            $self->cn_sort($cn_sort);
+        C4::Biblio::ModZebra( $self->biblionumber, "specialUpdate", "biblioserver" );
+
+        logaction( "CATALOGUING", "ADD", $self->itemnumber, "item" )
+          if $log_action && C4::Context->preference("CataloguingLog");
+
+        $self->_after_item_action_hooks({ action => 'create' });
+
+    } else { # ModItem
+
+        { # Update *_on  fields if needed
+          # Why not for AddItem as well?
+            my @fields = qw( itemlost withdrawn damaged );
+
+            # Only retrieve the item if we need to set an "on" date field
+            if ( $self->itemlost || $self->withdrawn || $self->damaged ) {
+                my $pre_mod_item = $self->get_from_storage;
+                for my $field (@fields) {
+                    if (    $self->$field
+                        and not $pre_mod_item->$field )
+                    {
+                        my $field_on = "${field}_on";
+                        $self->$field_on(
+                          DateTime::Format::MySQL->format_datetime( dt_from_string() )
+                        );
+                    }
+                }
+            }
+
+            # If the field is defined but empty, we are removing and,
+            # and thus need to clear out the 'on' field as well
+            for my $field (@fields) {
+                if ( defined( $self->$field ) && !$self->$field ) {
+                    my $field_on = "${field}_on";
+                    $self->$field_on(undef);
+                }
+            }
+        }
+
+        if ( defined $self->location and $self->location ne 'CART' and $self->location ne 'PROC' and not $self->permanent_location ) {
+            $self->permanent_location($self->location);
         }
 
+        $self->timestamp(undef) if $self->timestamp; # Maybe move this to Koha::Object->store?
+
         C4::Biblio::ModZebra( $self->biblionumber, "specialUpdate", "biblioserver" );
 
-        logaction( "CATALOGUING", "ADD", $self->itemnumber, "item" )
-          if C4::Context->preference("CataloguingLog");
+        $self->_after_item_action_hooks({ action => 'modify' });
+
+        logaction( "CATALOGUING", "MODIFY", $self->itemnumber, "item " . Dumper($self->unblessed) )
+          if $log_action && C4::Context->preference("CataloguingLog");
 
     }
 
index 0c127b0..cc05526 100755 (executable)
@@ -172,18 +172,13 @@ if ($quantityrec > $origquantityrec ) {
 my $new_order_object = Koha::Acquisition::Orders->find( $new_ordernumber ); # FIXME we should not need to refetch it
 my $items = $new_order_object->items;
 while ( my $item = $items->next )  {
-    ModItem(
-        {
-            booksellerid         => $booksellerid,
-            dateaccessioned      => $datereceived,
-            datelastseen         => $datereceived,
-            price                => $unitprice,
-            replacementprice     => $replacementprice,
-            replacementpricedate => $datereceived,
-        },
-        $biblionumber,
-        $item->itemnumber,
-    );
+    $item->booksellerid($booksellerid); # TODO This should be done using ->set, but bug 21761 is not resolved
+    $item->dateaccessioned($datereceived);
+    $item->datelastseen($datereceived);
+    $item->price($unitprice);
+    $item->replacementprice($replacementprice);
+    $item->replacementpricedate($datereceived);
+    $item->store;
 }
 
 print $input->redirect("/cgi-bin/koha/acqui/parcel.pl?invoiceid=$invoiceid&sticky_filters=1");
index 6c6bd29..f8295a0 100755 (executable)
@@ -45,7 +45,8 @@ my $confirm=$cgi->param('confirm');
 my $dbh = C4::Context->dbh;
 
 # get the rest of this item's information
-my $item_data_hashref = Koha::Items->find($itemnumber)->unblessed;
+my $item = Koha::Items->find($itemnumber);
+my $item_data_hashref = $item->unblessed;
 
 # make sure item statuses are set to 0 if empty or NULL
 for ($damaged,$itemlost,$withdrawn) {
@@ -55,31 +56,30 @@ for ($damaged,$itemlost,$withdrawn) {
 }
 
 # modify MARC item if input differs from items table.
-my $item_changes = {};
 if ( $op eq "set_non_public_note" ) {
     checkauth($cgi, 0, {editcatalogue => 'edit_items'}, 'intranet');
     if ((not defined  $item_data_hashref->{'itemnotes_nonpublic'}) or $itemnotes_nonpublic ne $item_data_hashref->{'itemnotes_nonpublic'}) {
-        $item_changes->{'itemnotes_nonpublic'} = $itemnotes_nonpublic;
+        $item->itemnotes_nonpublic($itemnotes_nonpublic);
     }
 }
 elsif ( $op eq "set_public_note" ) { # i.e., itemnotes parameter passed from form
     checkauth($cgi, 0, {editcatalogue => 'edit_items'}, 'intranet');
     if ((not defined  $item_data_hashref->{'itemnotes'}) or $itemnotes ne $item_data_hashref->{'itemnotes'}) {
-        $item_changes->{'itemnotes'} = $itemnotes;
+        $item->itemnotes($itemnotes);
     }
 } elsif ( $op eq "set_lost" && $itemlost ne $item_data_hashref->{'itemlost'}) {
-    $item_changes->{'itemlost'} = $itemlost;
+    $item->itemlost($itemlost);
 } elsif ( $op eq "set_withdrawn" && $withdrawn ne $item_data_hashref->{'withdrawn'}) {
-    $item_changes->{'withdrawn'} = $withdrawn;
+    $item->withdrawn($withdrawn);
 } elsif ( $op eq "set_damaged" && $damaged ne $item_data_hashref->{'damaged'}) {
-    $item_changes->{'damaged'} = $damaged;
+    $item->damaged($damaged);
 } else {
     #nothings changed, so do nothing.
     print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber");
        exit;
 }
 
-ModItem($item_changes, $biblionumber, $itemnumber);
+$item->store;
 
 LostItem($itemnumber, 'moredetail') if $op eq "set_lost";
 
index 0bfeae7..616e55a 100755 (executable)
@@ -76,19 +76,14 @@ sub set_item_default_location {
     my $itemnumber = shift;
     my $item       = Koha::Items->find($itemnumber);
     if ( C4::Context->preference('NewItemsDefaultLocation') ) {
-        ModItem(
-            {
-                permanent_location => $item->location,
-                location => C4::Context->preference('NewItemsDefaultLocation')
-            },
-            undef,
-            $itemnumber
-        );
+        $item->permanent_location($item->location);
+        $item->location(C4::Context->preference('NewItemsDefaultLocation'));
     }
     else {
-        ModItem( { permanent_location => $item->location }, undef, $itemnumber )
-          unless defined $item->permanent_location;
+        # It seems that we are dealing with that in too many places
+        $item->permanent_location($item->location) unless defined $item->permanent_location;
     }
+    $item->store;
 }
 
 # NOTE: This code is subject to change in the future with the implemenation of ajax based autobarcode code
index dc30a7a..7f20629 100755 (executable)
@@ -26,7 +26,7 @@ use C4::Output;
 use CGI qw ( -utf8 );
 use C4::Auth;
 use C4::Debug;
-use C4::Items qw( ModItem ModItemTransfer );
+use C4::Items qw( ModItemTransfer );
 use C4::Reserves qw( ModReserveCancelAll );
 use Koha::Biblios;
 use Koha::DateUtils;
@@ -114,7 +114,10 @@ if ( $op eq 'cancel_reserve' and $reserve_id ) {
             }
             else {
                 eval {
-                    C4::Items::ModItem( $assignments, undef, $item->itemnumber );
+                    while ( my ( $f, $v ) = each( %$assignments ) ) {
+                        $item->$f($v);
+                    }
+                    $item->store;
                 };
                 warn "Unable to modify item itemnumber=" . $item->itemnumber . ": $@" if $@;
             }
index c75ffd2..3cdeb85 100755 (executable)
@@ -383,7 +383,8 @@ foreach my $startrange (sort keys %$lost) {
             }
             printf ("Due %s: item %5s from borrower %5s to lost: %s\n", $row->{date_due}, $row->{itemnumber}, $row->{borrowernumber}, $lostvalue) if($verbose);
             if($confirm) {
-                ModItem({ itemlost => $lostvalue }, $row->{'biblionumber'}, $row->{'itemnumber'});
+                Koha::Items->find( $row->{itemnumber} )->itemlost($lostvalue)
+                  ->store;
                 if ( $charge && $charge eq $lostvalue ) {
                     LostItem( $row->{'itemnumber'}, 'cronjob', $mark_returned );
                 } elsif ( $mark_returned ) {
index 9b3093a..c6bcf47 100755 (executable)
@@ -32,6 +32,7 @@ use Getopt::Long;
 use Koha::Script;
 use C4::Context;
 use C4::Items;
+use Koha::Items;
 use Pod::Usage;
 
 
@@ -75,13 +76,14 @@ if (defined $outfile) {
    open(OUT, ">&STDOUT") || die ("Couldn't duplicate STDOUT: $!");
 }
 
+# FIXME Would be better to call Koha::Items->search here
 my $sth_fetch = $dbh->prepare("SELECT biblionumber, itemnumber, itemcallnumber FROM items $whereclause");
 $sth_fetch->execute();
 
 # fetch info from the search
 while (my ($biblionumber, $itemnumber, $itemcallnumber) = $sth_fetch->fetchrow_array){
    
-  eval { ModItem({itemcallnumber => $itemcallnumber}, $biblionumber, $itemnumber); };
+  eval { Koha::Items->find($itemnumber)->itemcallnumber($itemcallnumber)->store; };
   my $modok = $@ ? 0 : 1;
 
   if ($modok) {
index d70c3a5..a7a8638 100755 (executable)
@@ -1,13 +1,8 @@
 #!/usr/bin/perl
 
-use strict;
-#use warnings; FIXME - Bug 2505
-
+use Modern::Perl;
 use Koha::Script;
-use C4::Context;
-use C4::Items;
-use C4::Biblio;
-
+use Koha::Items;
 #
 # the items.onloan field did not exist in koha 2.2
 # in koha 3.0, it's used to define item availability
@@ -15,16 +10,10 @@ use C4::Biblio;
 # and put it in the MARC::Record of the item
 #
 
-my $dbh=C4::Context->dbh;
+my $items = Koha::Items->({ onloan => { '!=' => undef } });
 
-# if (C4::Context->preference("marcflavour") ne "UNIMARC") {
-#     print "this script is for UNIMARC only\n";
-#     exit;
-# }
-my $rqbiblios=$dbh->prepare("SELECT biblionumber,itemnumber,onloan FROM items WHERE items.onloan IS NOT NULL");
-$rqbiblios->execute;
 $|=1;
-while (my ($biblionumber,$itemnumber,$onloan)= $rqbiblios->fetchrow){
-    ModItem({onloan => "$onloan"}, $biblionumber, $itemnumber);
-    print "Onloan : $onloan for $biblionumber / $itemnumber\n";
+while ( my $item = $items->next ) {
+    $item->store;
+    print sprintf "Onloan : %s for %s / %s\n", $item->onloan, $item->biblionumber, $item->itemnumber;
 }
index aacfeda..935440b 100755 (executable)
@@ -339,7 +339,7 @@ sub kocReturnItem {
             $patron->privacy
         );
 
-        ModItem({ onloan => undef }, $biblio->biblionumber, $item->itemnumber);
+        $item->onloadn(undef)->store;
         ModDateLastSeen( $item->itemnumber );
 
         push @output,
index 749e126..fd1b7bd 100755 (executable)
@@ -268,7 +268,12 @@ if ( $op and $op eq 'serialchangestatus' ) {
                     my $subscriptioninfos = GetSubscription($subscriptionids[$i]);
 
                     # Changing the status to "available" and the itemtype according to the previousitemtype db field
-                    ModItem({notforloan => 0, itype => $subscriptioninfos->{'previousitemtype'} }, undef, $itemnumber);
+                    $serialitem->set(
+                        {
+                            notforloan => 0,
+                            itype => $subscriptioninfos->{'previousitemtype'}
+                        }
+                    )->store;
                 }
             }
         }
index def661d..2d39276 100755 (executable)
@@ -24,7 +24,6 @@ use CGI;
 use JSON qw(to_json);
 
 use C4::Circulation;
-use C4::Items qw(ModItem);
 use C4::Context;
 use C4::Auth qw(check_cookie_auth);
 use Koha::Checkouts;
index e579e78..babe72b 100755 (executable)
@@ -487,10 +487,10 @@ subtest "CanBookBeRenewed tests" => sub {
     is( $renewokay, 1, 'Can renew item 2, item-level hold is on item 1');
 
     # Items can't fill hold for reasons
-    ModItem({ notforloan => 1 }, $biblio->biblionumber, $item_1->itemnumber);
+    $item_1->notforloan(1)->store;
     ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1);
     is( $renewokay, 1, 'Can renew, item is marked not for loan, hold does not block');
-    ModItem({ notforloan => 0, itype => $itemtype }, $biblio->biblionumber, $item_1->itemnumber);
+    $item_1->set({notforloan => 0, itype => $itemtype })->store;
 
     # FIXME: Add more for itemtype not for loan etc.
 
@@ -1487,7 +1487,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
     is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with a hold on the record if AllowRenewalIfOtherItemsAvailable and onshelfhold are enabled' );
 
     # Setting item not checked out to be not for loan but holdable
-    ModItem({ notforloan => -1 }, $biblio->biblionumber, $item_2->itemnumber);
+    $item_2->notforloan(-1)->store;
 
     ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber );
     is( $renewokay, 0, 'Bug 14337 - Verify the borrower can not renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled but the only available item is notforloan' );
@@ -2455,7 +2455,7 @@ subtest '_FixAccountForLostAndFound' => sub {
         AddIssue( $patron->unblessed, $item->barcode );
 
         # Simulate item marked as lost
-        ModItem( { itemlost => 3 }, $biblio->biblionumber, $item->itemnumber );
+        $item->itemlost(3)->store;
         LostItem( $item->itemnumber, 1 );
 
         my $processing_fee_lines = Koha::Account::Lines->search(
@@ -2519,7 +2519,7 @@ subtest '_FixAccountForLostAndFound' => sub {
         AddIssue( $patron->unblessed, $item->barcode );
 
         # Simulate item marked as lost
-        ModItem( { itemlost => 1 }, $biblio->biblionumber, $item->itemnumber );
+        $item->itemlost(1)->store;
         LostItem( $item->itemnumber, 1 );
 
         my $processing_fee_lines = Koha::Account::Lines->search(
@@ -2589,7 +2589,7 @@ subtest '_FixAccountForLostAndFound' => sub {
         AddIssue( $patron->unblessed, $item->barcode );
 
         # Simulate item marked as lost
-        ModItem( { itemlost => 3 }, $biblio->biblionumber, $item->itemnumber );
+        $item->itemlost(3)->store;
         LostItem( $item->itemnumber, 1 );
 
         my $processing_fee_lines = Koha::Account::Lines->search(
@@ -2642,7 +2642,7 @@ subtest '_FixAccountForLostAndFound' => sub {
         AddIssue( $patron->unblessed, $item->barcode );
 
         # Simulate item marked as lost
-        ModItem( { itemlost => 1 }, $biblio->biblionumber, $item->itemnumber );
+        $item->itemlost(1)->store;
         LostItem( $item->itemnumber, 1 );
 
         my $processing_fee_lines = Koha::Account::Lines->search(
@@ -2742,7 +2742,7 @@ subtest '_FixAccountForLostAndFound' => sub {
                 }
             }
         );
-        my $item_id = Koha::Item->new(
+        my $item = Koha::Item->new(
             {
                 biblionumber     => $biblio->biblionumber,
                 homebranch       => $library->branchcode,
@@ -2751,16 +2751,16 @@ subtest '_FixAccountForLostAndFound' => sub {
                 replacementprice => $replacement_amount,
                 itype            => $item_type->itemtype
             },
-        )->store->itemnumber;
+        )->store;
 
         AddIssue( $patron->unblessed, $barcode );
 
         # Simulate item marked as lost
-        ModItem( { itemlost => 1 }, $biblio->biblionumber, $item_id );
-        LostItem( $item_id, 1 );
+        $item->itemlost(1)->store;
+        LostItem( $item->itemnumber, 1 );
 
         my $lost_fee_lines = Koha::Account::Lines->search(
-            { borrowernumber => $patron->id, itemnumber => $item_id, debit_type_code => 'LOST' } );
+            { borrowernumber => $patron->id, itemnumber => $item->itemnumber, debit_type_code => 'LOST' } );
         is( $lost_fee_lines->count, 1, 'Only one lost item fee produced' );
         my $lost_fee_line = $lost_fee_lines->next;
         is( $lost_fee_line->amount + 0, $replacement_amount, 'The right LOST amount is generated' );
@@ -2792,7 +2792,7 @@ subtest '_FixAccountForLostAndFound' => sub {
 
         t::lib::Mocks::mock_preference( 'AccountAutoReconcile', 1 );
 
-        my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item_id, $patron->id );
+        my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id );
         my $credit_return = Koha::Account::Lines->find($credit_return_id);
 
         is( $account->balance, $manual_debit_amount - $payment_amount, 'Balance is PROCESSING - payment (LOST_FOUND)' );
index 5d34559..b1475fc 100644 (file)
@@ -335,7 +335,7 @@ subtest 'BlockReturnOfLostItems' => sub {
     my $checkout = AddIssue( $patron->unblessed, $item->barcode );
 
     # Mark the item as lost
-    ModItem({itemlost => 1}, $biblio->biblionumber, $item->itemnumber);
+    $item->itemlost(1)->store;
 
     t::lib::Mocks::mock_preference('BlockReturnOfLostItems', 1);
     my ( $doreturn, $messages, $issue ) = AddReturn($item->barcode);
index 7b0e31c..61979bb 100755 (executable)
@@ -323,7 +323,7 @@ ok(
     is( $hold3->discard_changes->priority, 1, "After ModReserve, the 3rd reserve becomes the first on the waiting list" );
 }
 
-ModItem({ damaged => 1 }, $biblio->biblionumber, $itemnumber);
+Koha::Items->find($itemnumber)->damaged(1)->store; # FIXME The $itemnumber is a bit confusing here
 t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 1 );
 is( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'OK', "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" );
 ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold can be trapped for damaged item with AllowHoldsOnDamagedItems enabled" );
@@ -346,7 +346,7 @@ ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for d
 
 # Regression test for bug 9532
 $biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' });
-$itemnumber = $builder->build_sample_item({ library => $branch_1, itype => 'CANNOT', biblionumber => $biblio->biblionumber})->itemnumber;
+$item = $builder->build_sample_item({ library => $branch_1, itype => 'CANNOT', biblionumber => $biblio->biblionumber});
 AddReserve(
     {
         branchcode     => $branch_1,
@@ -356,19 +356,20 @@ AddReserve(
     }
 );
 is(
-    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'tooManyReserves',
+    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'tooManyReserves',
     "cannot request item if policy that matches on item-level item type forbids it"
 );
-ModItem({ itype => 'CAN' }, $biblio->biblionumber, $itemnumber);
+
+$item->itype('CAN')->store;
 ok(
-    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'OK',
+    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status} eq 'OK',
     "can request item if policy that matches on item type allows it"
 );
 
 t::lib::Mocks::mock_preference('item-level_itypes', 0);
-ModItem({ itype => undef }, $biblio->biblionumber, $itemnumber);
+$item->itype(undef)->store;
 ok(
-    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'tooManyReserves',
+    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status} eq 'tooManyReserves',
     "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)"
 );
 
index c59cbf7..2997e57 100755 (executable)
@@ -82,10 +82,10 @@ subtest 'General Add, Get and Del tests' => sub {
     # Do not modify anything, and do not explode!
     my $dbh = C4::Context->dbh;
     local $dbh->{RaiseError} = 1;
-    ModItem({}, $biblio->biblionumber, $itemnumber);
+    $getitem->set({})->store;
 
     # Modify item; setting barcode.
-    ModItem({ barcode => '987654321' }, $biblio->biblionumber, $itemnumber);
+    $getitem->barcode('987654321')->store;
     my $moditem = Koha::Items->find($itemnumber);
     cmp_ok($moditem->barcode, '==', '987654321', 'Modified item barcode successfully to: '.$moditem->barcode . '.');
 
@@ -107,12 +107,12 @@ subtest 'General Add, Get and Del tests' => sub {
     is( $getitem->location, $location, "The location should not have been modified" );
     is( $getitem->permanent_location, 'my permanent location', "The permanent_location should not have modified" );
 
-    ModItem({ location => $location }, $biblio->biblionumber, $itemnumber);
+    $getitem->location($location)->store;
     $getitem = Koha::Items->find($itemnumber);
     is( $getitem->location, $location, "The location should have been set to correct location" );
     is( $getitem->permanent_location, $location, "The permanent_location should have been set to location" );
 
-    ModItem({ location => 'CART' }, $biblio->biblionumber, $itemnumber);
+    $getitem->location('CART')->store;
     $getitem = Koha::Items->find($itemnumber);
     is( $getitem->location, 'CART', "The location should have been set to CART" );
     is( $getitem->permanent_location, $location, "The permanent_location should not have been set to CART" );
@@ -139,13 +139,11 @@ subtest 'ModItem tests' => sub {
     for my $field (@fields) {
         my $field_on = $field."_on";
 
-        $item->$field(1);
-        ModItem( $item->unblessed, $item->biblionumber, $item->itemnumber );
+        $item->$field(1)->store;
         $item->discard_changes;
         is( output_pref({ str => $item->$field_on, dateonly => 1 }), output_pref({ dt => dt_from_string(), dateonly => 1 }), "When updating $field, $field_on is updated" );
 
-        $item->$field(0);
-        ModItem( $item->unblessed, $item->biblionumber, $item->itemnumber );
+        $item->$field(0)->store;
         $item->discard_changes;
         is( $item->$field_on, undef, "When clearing $field, $field_on is cleared" );
     }
@@ -997,28 +995,28 @@ subtest 'Test logging for ModItem' => sub {
     my $biblio = $builder->build_sample_biblio();
 
     # Add an item.
-    my $itemnumber = $builder->build_sample_item(
+    my $item = $builder->build_sample_item(
         {
             biblionumber => $biblio->biblionumber,
             library      => $library->{homebranch},
             location     => $location,
             itype        => $itemtype->{itemtype}
         }
-    )->itemnumber;
+    );
 
     # False means no logging
     $schema->resultset('ActionLog')->search()->delete();
-    ModItem({ location => $location }, $biblio->biblionumber, $itemnumber, { log_action => 0 });
+    $item->location($location)->store({ log_action => 0 });
     is( $schema->resultset('ActionLog')->count(), 0, 'False value does not trigger logging' );
 
     # True means logging
     $schema->resultset('ActionLog')->search()->delete();
-    ModItem({ location => $location }, $biblio->biblionumber, $itemnumber, { log_action => 1 });
+    $item->location($location)->store({ log_action => 1 });
     is( $schema->resultset('ActionLog')->count(), 1, 'True value does trigger logging' );
 
     # Undefined defaults to true
     $schema->resultset('ActionLog')->search()->delete();
-    ModItem({ location => $location }, $biblio->biblionumber, $itemnumber);
+    $item->location($location)->store();
     is( $schema->resultset('ActionLog')->count(), 1, 'Undefined value defaults to true, triggers logging' );
 
     $schema->storage->txn_rollback;
index 86f2eb3..f047252 100644 (file)
@@ -124,7 +124,7 @@ is( $marc_item->subfield($tagfield, $new_tagfield), 'updated_value', q|ToggleNew
 my $dt_today = dt_from_string;
 my $days5ago = $dt_today->add_duration( DateTime::Duration->new( days => -5 ) );
 
-C4::Items::ModItem( { dateaccessioned => $days5ago }, $biblionumber, $itemnumber );
+$modified_item->dateaccessioned($days5ago)->store;
 
 @rules = (
     {
index ce06dfe..3ccd492 100644 (file)
@@ -80,9 +80,9 @@ my $biblio = $builder->build(
     }
 );
 
-my $item = $builder->build(
+my $item = $builder->build_object(
     {
-        source => 'Item',
+        class => 'Koha::Items',
         value  => {
             biblionumber  => $biblio->{biblionumber},
             homebranch    => $branch->{branchcode},
@@ -95,41 +95,41 @@ my $item = $builder->build(
 
 # book_on_loan
 
-AddIssue( $patron, $item->{barcode} );
+AddIssue( $patron, $item->barcode );
 
 is(
-    ItemSafeToDelete( $biblio->{biblionumber}, $item->{itemnumber} ),
+    ItemSafeToDelete( $biblio->{biblionumber}, $item->itemnumber ),
     'book_on_loan',
     'ItemSafeToDelete reports item on loan',
 );
 
 is(
-    DelItemCheck( $biblio->{biblionumber}, $item->{itemnumber} ),
+    DelItemCheck( $biblio->{biblionumber}, $item->itemnumber ),
     'book_on_loan',
     'item that is on loan cannot be deleted',
 );
 
-AddReturn( $item->{barcode}, $branch->{branchcode} );
+AddReturn( $item->barcode, $branch->{branchcode} );
 
 # book_reserved is tested in t/db_dependent/Reserves.t
 
 # not_same_branch
 t::lib::Mocks::mock_preference('IndependentBranches', 1);
-ModItem( { homebranch => $branch2->{branchcode}, holdingbranch => $branch2->{branchcode} }, $biblio->{biblionumber}, $item->{itemnumber} );
+$item->set( { homebranch => $branch2->{branchcode}, holdingbranch => $branch2->{branchcode} })->store;
 
 is(
-    ItemSafeToDelete( $biblio->{biblionumber}, $item->{itemnumber} ),
+    ItemSafeToDelete( $biblio->{biblionumber}, $item->itemnumber ),
     'not_same_branch',
     'ItemSafeToDelete reports IndependentBranches restriction',
 );
 
 is(
-    DelItemCheck( $biblio->{biblionumber}, $item->{itemnumber} ),
+    DelItemCheck( $biblio->{biblionumber}, $item->itemnumber ),
     'not_same_branch',
     'IndependentBranches prevents deletion at another branch',
 );
 
-ModItem( { homebranch => $branch->{branchcode}, holdingbranch => $branch->{branchcode} }, $biblio->{biblionumber}, $item->{itemnumber} );
+$item->set( { homebranch => $branch->{branchcode}, holdingbranch => $branch->{branchcode} })->store;
 
 # linked_analytics
 
@@ -139,13 +139,13 @@ ModItem( { homebranch => $branch->{branchcode}, holdingbranch => $branch->{branc
     $module->mock( GetAnalyticsCount => sub { return 1 } );
 
     is(
-        ItemSafeToDelete( $biblio->{biblionumber}, $item->{itemnumber} ),
+        ItemSafeToDelete( $biblio->{biblionumber}, $item->itemnumber ),
         'linked_analytics',
         'ItemSafeToDelete reports linked analytics',
     );
 
     is(
-        DelItemCheck( $biblio->{biblionumber}, $item->{itemnumber} ),
+        DelItemCheck( $biblio->{biblionumber}, $item->itemnumber ),
         'linked_analytics',
         'Linked analytics prevents deletion of item',
     );
@@ -153,14 +153,14 @@ ModItem( { homebranch => $branch->{branchcode}, holdingbranch => $branch->{branc
 }
 
 is(
-    ItemSafeToDelete( $biblio->{biblionumber}, $item->{itemnumber} ),
+    ItemSafeToDelete( $biblio->{biblionumber}, $item->itemnumber ),
     1,
     'ItemSafeToDelete shows item safe to delete'
 );
 
-DelItemCheck( $biblio->{biblionumber}, $item->{itemnumber} );
+DelItemCheck( $biblio->{biblionumber}, $item->itemnumber );
 
-my $test_item = Koha::Items->find( $item->{itemnumber} );
+my $test_item = Koha::Items->find( $item->itemnumber );
 
 is( $test_item, undef,
     "DelItemCheck should delete item if ItemSafeToDelete returns true"
index 93e3c92..0d69d05 100644 (file)
@@ -76,7 +76,7 @@ my $itemnumber = $item->itemnumber;
 
 # Modify item; setting barcode.
 my $testbarcode = '97531';
-ModItem( { barcode => $testbarcode }, $bibnum, $itemnumber );
+$item->barcode($testbarcode)->store;
 
 my $layout = C4::Labels::Layout->new( layout_name => 'TEST' );
 
@@ -103,7 +103,7 @@ my $dummy_template_values = {
 
 my $label_info = {
     batch_id         => $batch_id,
-    item_number      => $itemnumber,
+    item_number      => $item->itemnumber,
     llx              => $llx,
     lly              => $lly,
     width            => $dummy_template_values->{'label_width'},
index ed3d095..d352472 100755 (executable)
@@ -85,7 +85,7 @@ t::lib::Mocks::mock_userenv({ branchcode => $branch_1 });
 my $bibnum = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber;
 
 # Create a helper item instance for testing
-my $itemnumber = $builder->build_sample_item({ biblionumber => $bibnum, library => $branch_1, itype => $itemtype })->itemnumber;
+my $item = $builder->build_sample_item({ biblionumber => $bibnum, library => $branch_1, itype => $itemtype });
 
 my $biblio_with_no_item = $builder->build({
     source => 'Biblio'
@@ -94,7 +94,7 @@ my $biblio_with_no_item = $builder->build({
 
 # Modify item; setting barcode.
 my $testbarcode = '97531';
-ModItem({ barcode => $testbarcode }, $bibnum, $itemnumber);
+$item->barcode($testbarcode)->store; # FIXME We should not hardcode a barcode! Also, what's the purpose of this?
 
 # Create a borrower
 my %data = (
index 19d7090..13c2f07 100755 (executable)
@@ -199,21 +199,21 @@ if ( $uploadbarcodes && length($uploadbarcodes) > 0 ) {
         } else {
             my $item = Koha::Items->find({barcode => $barcode});
             if ( $item ) {
-                $item = $item->unblessed;
+                my $item_unblessed = $item->unblessed;
                 # Modify date last seen for scanned items, remove lost status
-                ModItem( { itemlost => 0, datelastseen => $date }, undef, $item->{'itemnumber'} );
+                $item->set({ itemlost => 0, datelastseen => $date })->store;
                 $moddatecount++;
                 # update item hash accordingly
-                $item->{itemlost} = 0;
-                $item->{datelastseen} = $date;
+                $item_unblessed->{itemlost} = 0;
+                $item_unblessed->{datelastseen} = $date;
                 unless ( $dont_checkin ) {
                     $qonloan->execute($barcode);
                     if ($qonloan->rows){
                         my $data = $qonloan->fetchrow_hashref;
                         my ($doreturn, $messages, $iteminformation, $borrower) =AddReturn($barcode, $data->{homebranch});
                         if( $doreturn ) {
-                            $item->{onloan} = undef;
-                            $item->{datelastseen} = dt_from_string;
+                            $item_unblessed->{onloan} = undef;
+                            $item_unblessed->{datelastseen} = dt_from_string;
                         } else {
                             push @errorloop, { barcode => $barcode, ERR_ONLOAN_NOT_RET => 1 };
                         }