From d32791b5db11c20e7e445d0c9e10e8663883598f Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 2 Apr 2018 13:55:16 -0300 Subject: [PATCH] Bug 19974: Make MarkLostItemsAsReturned multiple MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Given the confusion regarding this behaviour it sounds better to make it configurable. This pref will take 4 different values, 1 per place an item can be marked as lost. Test plan: Mark items as lost and confirm the item is returned or not, depending on the value of the system preference. - from the longoverdue cronjob (--mark-returned takes precedence if set) - from the batch item modification tool - when cataloguing an item - from the items tab of the catalog module Signed-off-by: Séverine QUEUNE Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart --- C4/Circulation.pm | 15 +++++++++-- catalogue/updateitem.pl | 2 +- cataloguing/additem.pl | 2 +- .../data/mysql/atomicupdate/bug_19974.perl | 26 +++++++++++++++++++ installer/data/mysql/sysprefs.sql | 2 +- .../admin/preferences/circulation.pref | 11 +++++--- misc/cronjobs/longoverdue.pl | 2 +- t/db_dependent/Circulation.t | 9 ++++--- tools/batchMod.pl | 2 +- 9 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_19974.perl diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 1f1a431c7f..c6dc6afacd 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3613,9 +3613,20 @@ sub ReturnLostItem{ sub LostItem{ - my ($itemnumber, $mark_returned) = @_; + my ($itemnumber, $mark_lost_from, $force_mark_returned) = @_; - $mark_returned //= C4::Context->preference('MarkLostItemsAsReturned'); + unless ( $mark_lost_from ) { + # Temporary check to avoid regressions + die q|LostItem called without $mark_lost_from, check the API.|; + } + + my $mark_returned; + if ( $force_mark_returned ) { + $mark_returned = 1; + } else { + my $pref = C4::Context->preference('MarkLostItemsAsReturned') // q{}; + $mark_returned = ( $pref =~ m|$mark_lost_from| ); + } my $dbh = C4::Context->dbh(); my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title diff --git a/catalogue/updateitem.pl b/catalogue/updateitem.pl index b7cbc9445c..f06dfedfb1 100755 --- a/catalogue/updateitem.pl +++ b/catalogue/updateitem.pl @@ -80,6 +80,6 @@ elsif (defined $itemnotes) { # i.e., itemnotes parameter passed from form ModItem($item_changes, $biblionumber, $itemnumber); -LostItem($itemnumber) if $itemlost; +LostItem($itemnumber, 'moredetail') if $itemlost; print $cgi->redirect("moredetail.pl?biblionumber=$biblionumber&itemnumber=$itemnumber#item$itemnumber"); diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl index 802e908f01..1b746f7e30 100755 --- a/cataloguing/additem.pl +++ b/cataloguing/additem.pl @@ -695,7 +695,7 @@ if ($op eq "additem") { $itemnumber = q{}; my $olditemlost = $item->{itemlost}; my $newitemlost = $newitem->{itemlost}; - LostItem( $item->{itemnumber} ) + LostItem( $item->{itemnumber}, 'additem' ) if $newitemlost && $newitemlost ge '1' && !$olditemlost; } $nextop="additem"; diff --git a/installer/data/mysql/atomicupdate/bug_19974.perl b/installer/data/mysql/atomicupdate/bug_19974.perl new file mode 100644 index 0000000000..78ff577d0a --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_19974.perl @@ -0,0 +1,26 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + my ( $original_value ) = $dbh->selectrow_array(q| + SELECT value FROM systempreferences WHERE variable="MarkLostItemsAsReturned" + |); + if ( $original_value and $original_value eq '1' ) { + $dbh->do(q{ + UPDATE systempreferences + SET type="multiple", + options="batchmod|moredetail|cronjob|additem", + value="batchmod|moredetail|cronjob|additem" + WHERE variable="MarkLostItemsAsReturned" + }); + } else { + $dbh->do(q{ + UPDATE systempreferences + SET type="multiple", + options="batchmod|moredetail|cronjob|additem", + value="" + WHERE variable="MarkLostItemsAsReturned" + }); + } + + SetVersion( $DBversion ); + print "Upgrade to $DBversion done (Bug 19974 - Make MarkLostItemsAsReturned multiple)\n"; +} diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index ad8f41d677..f01f572a49 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -261,7 +261,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('MarcFieldDocURL', NULL, NULL, 'URL used for MARC field documentation. Following substitutions are available: {MARC} = marc flavour, eg. "MARC21" or "UNIMARC". {FIELD} = field number, eg. "000" or "048". {LANG} = user language, eg. "en" or "fi-FI"', 'free'), ('MarcFieldsToOrder','',NULL,'Set the mapping values for a new order line created from a MARC record in a staged file. In a YAML format.','textarea'), ('MarcItemFieldsToOrder','',NULL,'Set the mapping values for new item records created from a MARC record in a staged file. In a YAML format.','textarea'), -('MarkLostItemsAsReturned','1','','Mark items as returned when flagged as lost','YesNo'), +('MarkLostItemsAsReturned','batchmod|moredetail|cronjob|additem','batchmod|moredetail|cronjob|additem','Mark items as returned when flagged as lost','multiple'), ('MARCOrgCode','OSt','','Define MARC Organization Code for MARC21 records - http://www.loc.gov/marc/organizations/orgshome.html','free'), ('MaxFine',NULL,'','Maximum fine a patron can have for all late returns at one moment. Single item caps are specified in the circulation rules matrix.','Integer'), ('MaxItemsToDisplayForBatchDel','1000',NULL,'Display up to a given number of items in a single item deletionbatch.','Integer'), 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 8ec6a6b5b6..7aa79bacbc 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 @@ -413,11 +413,14 @@ Circulation: nothing : "do nothing" - . - + - "Mark items as returned when flagged as lost " - pref: MarkLostItemsAsReturned - choices: - yes: "Mark" - no: "Do not mark" - - "items as returned when flagged as lost" + multiple: + cronjob: "from the longoverdue cronjob" + batchmod: "from the batch item modification tool" + additem: "when cataloguing an item" + moredetail: "from the items tab of the catalog module" + - . - - pref: AllowMultipleIssuesOnABiblio choices: diff --git a/misc/cronjobs/longoverdue.pl b/misc/cronjobs/longoverdue.pl index 394000bd2f..f8fd871b1f 100755 --- a/misc/cronjobs/longoverdue.pl +++ b/misc/cronjobs/longoverdue.pl @@ -310,7 +310,7 @@ foreach my $startrange (sort keys %$lost) { if($confirm) { ModItem({ itemlost => $lostvalue }, $row->{'biblionumber'}, $row->{'itemnumber'}); if ( $charge && $charge eq $lostvalue ) { - LostItem( $row->{'itemnumber'}, $mark_returned ); + LostItem( $row->{'itemnumber'}, 'cronjob', $mark_returned ); } elsif ( $mark_returned ) { my $patron = Koha::Patrons->find( $row->{borrowernumber} ); MarkIssueReturned($row->{borrowernumber},$row->{itemnumber},undef,undef,$patron->privacy) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index bc14b5382b..468fdfac37 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 114; +use Test::More tests => 116; use DateTime; use POSIX qw( floor ); @@ -846,10 +846,12 @@ C4::Context->dbh->do("DELETE FROM accountlines"); t::lib::Mocks::mock_preference('WhenLostForgiveFine','0'); t::lib::Mocks::mock_preference('WhenLostChargeReplacementFee','0'); - LostItem( $itemnumber, 1 ); + LostItem( $itemnumber, 'test', 1 ); my $item = Koha::Database->new()->schema()->resultset('Item')->find($itemnumber); ok( !$item->onloan(), "Lost item marked as returned has false onloan value" ); + my $checkout = Koha::Checkouts->find({ itemnumber => $itemnumber }); + is( $checkout, undef, 'LostItem called with forced return has checked in the item' ); my $total_due = $dbh->selectrow_array( 'SELECT SUM( amountoutstanding ) FROM accountlines WHERE borrowernumber = ?', @@ -871,10 +873,11 @@ C4::Context->dbh->do("DELETE FROM accountlines"); } ); - LostItem( $itemnumber2, 0 ); + LostItem( $itemnumber2, 'test', 0 ); my $item2 = Koha::Database->new()->schema()->resultset('Item')->find($itemnumber2); ok( $item2->onloan(), "Lost item *not* marked as returned has true onloan value" ); + ok( Koha::Checkouts->find({ itemnumber => $itemnumber2 }), 'LostItem called without forced return has checked in the item' ); $total_due = $dbh->selectrow_array( 'SELECT SUM( amountoutstanding ) FROM accountlines WHERE borrowernumber = ?', diff --git a/tools/batchMod.pl b/tools/batchMod.pl index 94062f79ed..192deb965d 100755 --- a/tools/batchMod.pl +++ b/tools/batchMod.pl @@ -207,7 +207,7 @@ if ($op eq "action") { if ( $modified ) { eval { if ( my $item = ModItemFromMarc( $localmarcitem, $itemdata->{biblionumber}, $itemnumber ) ) { - LostItem($itemnumber) if $item->{itemlost} and not $itemdata->{itemlost}; + LostItem($itemnumber, 'batchmod') if $item->{itemlost} and not $itemdata->{itemlost}; } }; } -- 2.39.5