Attachment 78678 Details for Bug 18887 – Bug 18887: Port max_holds rules to new CirculationRules system

[patch] Bug 18887: Port max_holds rules to new CirculationRules system

Bug-18887-Port-maxholds-rules-to-new-CirculationRu.patch (text/plain), 10.76 KB, created by Jonathan Druart on 2018-09-14 17:51:08 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2018-09-14 17:51:08 UTC

Size: 10.76 KB

patch

obsolete

>From 92ef8fcde21432526603dd228ba2391cf43ccf85 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Fri, 30 Jun 2017 14:23:55 -0400
>Subject: [PATCH] Bug 18887: Port max_holds rules to new CirculationRules
> system
>
>This is the first step in the circulation rules revamp as detailed
>in the RFF https://wiki.koha-community.org/wiki/Circulation_Rules_Interface_and_Backend_Revamp_RFC
>
>This patch moves the recent max_holds rule to the new circulation_rules table.
>
>Test Plan:
>1) Apply this patch
>2) Run updatedatabase.pl
>3) Go to the circ rules editor, note the new max holds rules
>   by patron category in the "Checkout limit by patron category".
>   ( Should we rename this section? )
>4) Create find a patron that is allowed to place a hold, count the
>   number of holds that patron has. Lets make that number 'X'.
>5) Set the new max holds rule to X for "All libraries"
>6) Note the patron can no longer place another hold
>7) Set the new max holds rule to X + 1 for the patron's home library
>8) Note the patron can again place another hold
>9) Set the new max holds rule to X for the patron's home library
>10) Note the patron can no longer place another hold
>---
> C4/Reserves.pm                                     | 19 +++++----
> admin/smart-rules.pl                               | 48 ++++++++++++++++++++--
> .../prog/en/modules/admin/smart-rules.tt           |  9 ++--
> t/db_dependent/Holds.t                             | 30 +++++++++-----
> 4 files changed, 81 insertions(+), 25 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 213c7c376d..21775a5e8c 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -48,6 +48,7 @@ use Koha::IssuingRules;
> use Koha::Items;
> use Koha::ItemTypes;
> use Koha::Patrons;
>+use Koha::CirculationRules;
> 
> use List::MoreUtils qw( firstidx any );
> use Carp;
>@@ -412,26 +413,30 @@ sub CanItemBeReserved {
>     }
> 
>     # Now we need to check hold limits by patron category
>-    my $schema = Koha::Database->new()->schema();
>-    my $rule = $schema->resultset('BranchBorrowerCircRule')->find(
>+    my $rule = Koha::CirculationRules->find(
>         {
>-            branchcode   => $branchcode,
>             categorycode => $borrower->{categorycode},
>+            branchcode   => $branchcode,
>+            itemtype     => undef,
>+            rule_name    => 'max_holds',
>         }
>     );
>-    $rule ||= $schema->resultset('DefaultBorrowerCircRule')->find(
>+    $rule ||= Koha::CirculationRules->find(
>         {
>-            categorycode => $borrower->{categorycode}
>+            categorycode => $borrower->{categorycode},
>+            branchcode   => undef,,
>+            itemtype     => undef,
>+            rule_name    => 'max_holds',
>         }
>     );
>-    if ( $rule && defined $rule->max_holds ) {
>+    if ( $rule ) {
>         my $total_holds_count = Koha::Holds->search(
>             {
>                 borrowernumber => $borrower->{borrowernumber}
>             }
>         )->count();
> 
>-        return { status => 'tooManyReserves', limit => $rule->max_holds } if $total_holds_count >= $rule->max_holds;
>+        return { status => 'tooManyReserves', limit => $rule->rule_value} if $total_holds_count >= $rule->rule_value;
>     }
> 
>     my $circ_control_branch =
>diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
>index d87d8e5451..ae14a8e8eb 100755
>--- a/admin/smart-rules.pl
>+++ b/admin/smart-rules.pl
>@@ -32,6 +32,7 @@ use Koha::Logger;
> use Koha::RefundLostItemFeeRule;
> use Koha::RefundLostItemFeeRules;
> use Koha::Libraries;
>+use Koha::CirculationRules;
> use Koha::Patron::Categories;
> use Koha::Caches;
> 
>@@ -97,6 +98,15 @@ elsif ($op eq 'delete-branch-cat') {
>                                         AND categorycode = ?");
>         $sth_delete->execute($branch, $categorycode);
>     }
>+    Koha::CirculationRules->set_rule(
>+        {
>+            branchcode   => $branch,
>+            categorycode => $categorycode,
>+            itemtype     => undef,
>+            rule_name    => 'max_holds',
>+            rule_value   => undef,
>+        }
>+    );
> }
> elsif ($op eq 'delete-branch-item') {
>     my $itemtype  = $input->param('itemtype');
>@@ -287,10 +297,20 @@ elsif ($op eq "add-branch-cat") {
>             $sth_search->execute();
>             my $res = $sth_search->fetchrow_hashref();
>             if ($res->{total}) {
>-                $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $max_holds );
>+                $sth_update->execute( $maxissueqty, $maxonsiteissueqty );
>             } else {
>-                $sth_insert->execute( $maxissueqty, $maxonsiteissueqty, $max_holds );
>+                $sth_insert->execute( $maxissueqty, $maxonsiteissueqty );
>             }
>+
>+            Koha::CirculationRules->set_rule(
>+                {
>+                    branchcode   => undef,
>+                    categorycode => undef,
>+                    itemtype     => undef,
>+                    rule_name    => 'max_holds',
>+                    rule_value   => $max_holds,
>+                }
>+            );
>         } else {
>             my $sth_search = $dbh->prepare("SELECT count(*) AS total
>                                             FROM default_borrower_circ_rules
>@@ -310,10 +330,20 @@ elsif ($op eq "add-branch-cat") {
>             $sth_search->execute($categorycode);
>             my $res = $sth_search->fetchrow_hashref();
>             if ($res->{total}) {
>-                $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $max_holds, $categorycode );
>+                $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $categorycode );
>             } else {
>-                $sth_insert->execute( $categorycode, $maxissueqty, $maxonsiteissueqty, $max_holds );
>+                $sth_insert->execute( $categorycode, $maxissueqty, $maxonsiteissueqty );
>             }
>+
>+            Koha::CirculationRules->set_rule(
>+                {
>+                    branchcode   => undef,
>+                    categorycode => $categorycode,
>+                    itemtype     => undef,
>+                    rule_name    => 'max_holds',
>+                    rule_value   => $max_holds,
>+                }
>+            );
>         }
>     } elsif ($categorycode eq "*") {
>         my $sth_search = $dbh->prepare("SELECT count(*) AS total
>@@ -363,6 +393,16 @@ elsif ($op eq "add-branch-cat") {
>         } else {
>             $sth_insert->execute($branch, $categorycode, $maxissueqty, $maxonsiteissueqty, $max_holds);
>         }
>+
>+        Koha::CirculationRules->set_rule(
>+            {
>+                branchcode   => $branch,
>+                categorycode => $categorycode,
>+                itemtype     => undef,
>+                rule_name    => 'max_holds',
>+                rule_value   => $max_holds,
>+            }
>+        );
>     }
> }
> elsif ($op eq "add-branch-item") {
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>index 5bc7e003cf..440dbef5a6 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>@@ -1,6 +1,7 @@
> [% USE raw %]
> [% USE Asset %]
> [% USE Branches %]
>+[% USE CirculationRules %]
> [% SET footerjs = 1 %]
> [% INCLUDE 'doc-head-open.inc' %]
> <title>Koha &rsaquo; Administration &rsaquo; Circulation and fine rules</title>
>@@ -512,10 +513,12 @@
>                                 [% branch_cat_rule_loo.maxonsiteissueqty | html %]
>                             [% END %]
>                         </td>
>-                        <td>[% IF ( branch_cat_rule_loo.unlimited_max_holds ) %]
>-                                Unlimited
>+                        <td>
>+                            [% SET rule_value = CirculationRules.Get( branch_cat_rule_loo.branchcode, branch_cat_rule_loo.categorycode, branch_cat_rule_loo.itemtype, 'max_holds' ) %]
>+                            [% IF rule_value  %]
>+                                [% rule_value %]
>                             [% ELSE %]
>-                                [% branch_cat_rule_loo.max_holds | html %]
>+                                Unlimited
>                             [% END %]
>                         </td>
> 
>diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
>index 31a96a9dac..030ec03cad 100755
>--- a/t/db_dependent/Holds.t
>+++ b/t/db_dependent/Holds.t
>@@ -21,6 +21,8 @@ use Koha::Biblios;
> use Koha::Holds;
> use Koha::Items;
> use Koha::Libraries;
>+use Koha::Patrons;
>+use Koha::CirculationRules;
> 
> BEGIN {
>     use FindBin;
>@@ -418,6 +420,7 @@ subtest 'Test max_holds per library/patron category' => sub {
> 
>     $dbh->do('DELETE FROM reserves');
>     $dbh->do('DELETE FROM issuingrules');
>+    $dbh->do('DELETE FROM circulation_rules');
> 
>     ( $bibnum, $title, $bibitemnum ) = create_helper_biblio('TEST');
>     ( $item_bibnum, $item_bibitemnum, $itemnumber ) =
>@@ -442,20 +445,25 @@ subtest 'Test max_holds per library/patron category' => sub {
>     my $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
>     is( $ret->{status}, 'OK', 'Patron can place hold with no borrower circ rules' );
> 
>-    my $rule_all = $schema->resultset('DefaultBorrowerCircRule')->new(
>+    my $rule_all = Koha::CirculationRules->set_rule(
>         {
>             categorycode => $category->{categorycode},
>-            max_holds    => 3,
>+            branchcode   => undef,
>+            itemtype     => undef,
>+            rule_name    => 'max_holds',
>+            rule_value   => 3,
>         }
>-    )->insert();
>+    );
> 
>-    my $rule_branch = $schema->resultset('BranchBorrowerCircRule')->new(
>+    my $rule_branch = Koha::CirculationRules->set_rule(
>         {
>             branchcode   => $branch_1,
>             categorycode => $category->{categorycode},
>-            max_holds    => 5,
>+            itemtype     => undef,
>+            rule_name    => 'max_holds',
>+            rule_value   => 5,
>         }
>-    )->insert();
>+    );
> 
>     $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
>     is( $ret->{status}, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 3' );
>@@ -466,16 +474,16 @@ subtest 'Test max_holds per library/patron category' => sub {
>     is( $ret->{status}, 'tooManyReserves', 'Patron cannot place hold with only a category rule of 3' );
> 
>     $rule_all->delete();
>-    $rule_branch->max_holds(3);
>-    $rule_branch->insert();
>+    $rule_branch->rule_value(3);
>+    $rule_branch->store();
> 
>     $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
>     is( $ret->{status}, 'tooManyReserves', 'Patron cannot place hold with only a branch/category rule of 3' );
> 
>-    $rule_branch->max_holds(5);
>+    $rule_branch->rule_value(5);
>     $rule_branch->update();
>-    $rule_all->max_holds(5);
>-    $rule_all->insert();
>+    $rule_branch->rule_value(5);
>+    $rule_branch->store();
> 
>     $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
>     is( $ret->{status}, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 5' );
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 18887: 65233 | 65234 | 65235 | 65236 | 65237 | 67620 | 70986 | 70987 | 70988 | 70989 | 70990 | 70991 | 71031 | 71042 | 72218 | 72219 | 72220 | 72221 | 72222 | 72223 | 72224 | 77080 | 77081 | 77082 | 77083 | 77084 | 77085 | 77086 | 77087 | 77207 | 77208 | 77209 | 77210 | 77211 | 77212 | 77213 | 77214 | 77215 | 77218 | 77238 | 77649 | 77650 | 77651 | 77652 | 77653 | 77654 | 77655 | 77656 | 77657 | 77658 | 77659 | 78346 | 78347 | 78348 | 78349 | 78350 | 78351 | 78352 | 78353 | 78354 | 78355 | 78356 | 78674 | 78675 | 78676 | 78677 | 78678 | 78679 | 78680 | 78681 | 78682 | 78683 | 78684 | 78743 | 78744 | 78745 | 78746 | 78747 | 78748 | 78749 | 78750 | 78751 | 78752 | 78753 | 78879 | 79119 | 79626 | 79627 | 79628 | 79629 | 79630 | 79631 | 79632 | 79633 | 79634 | 79635 | 79636 | 79637 | 79638 | 79639 | 79640 | 79641 | 79761 | 79762 | 79763 | 79764 | 79765 | 79766 | 79767 | 79768 | 79769 | 79770 | 79771 | 79772 | 79773 | 79774 | 79775 | 79776 | 79778 | 79891