Attachment 73531 Details for Bug 19974 – Bug 19974: Make MarkLostItemsAsReturned multiple

[patch] Bug 19974: Make MarkLostItemsAsReturned multiple

Bug-19974-Make-MarkLostItemsAsReturned-multiple.patch (text/plain), 8.62 KB, created by Jonathan Druart on 2018-04-02 17:12:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2018-04-02 17:12:11 UTC

Size: 8.62 KB

patch

obsolete

>From f8c09e26028844480cf2aa249ad7a4eb1bfea33f Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 19974: 70525 | 73162 | 73163 | 73531 | 74106 | 74167 | 74238 | 74542 | 74564 | 74565 | 74566 | 74591 | 74594 | 74595 | 74596