From f8c09e26028844480cf2aa249ad7a4eb1bfea33f 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 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 --- C4/Circulation.pm | 15 +++++++++++++-- catalogue/updateitem.pl | 2 +- cataloguing/additem.pl | 2 +- installer/data/mysql/atomicupdate/bug_19974.sql | 1 + installer/data/mysql/sysprefs.sql | 2 +- .../prog/en/modules/admin/preferences/circulation.pref | 11 +++++++---- misc/cronjobs/longoverdue.pl | 2 +- t/db_dependent/Circulation.t | 4 ++-- tools/batchMod.pl | 2 +- 9 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_19974.sql diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 3e711cdb01..b0e3a5418e 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3601,9 +3601,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 = ( $mark_lost_from =~ m|$pref| ); + } 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..a25abbb93b 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, 'updateitem') 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.sql b/installer/data/mysql/atomicupdate/bug_19974.sql new file mode 100644 index 0000000000..3dad64884a --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_19974.sql @@ -0,0 +1 @@ +update systempreferences set type="multiple", options="batchmod|moredetail|cronjob|additem", value="batchmod|moredetail|cronjob|additem" where variable="MarkLostItemsAsReturned"; diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index b9c2e4ee0a..88bf3ebd6f 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -259,7 +259,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('MARCAuthorityControlField008','|| aca||aabn | a|a d',NULL,'Define the contents of MARC21 authority control field 008 position 06-39','Textarea'), ('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 02940df5b0..c6cc3a1506 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 0e82019e6e..d82ccdd43e 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -846,7 +846,7 @@ 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" ); @@ -871,7 +871,7 @@ 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" ); diff --git a/tools/batchMod.pl b/tools/batchMod.pl index 5f540766f6..985f56fc57 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.11.0