Attachment 72222 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-Circulation.patch (text/plain), 11.11 KB, created by Jesse Weaver on 2018-02-26 18:25:12 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jesse Weaver

Created: 2018-02-26 18:25:12 UTC

Size: 11.11 KB

patch

obsolete

>From baf1d3261727d144e9d0197ab65adacc799538bb 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                                     | 28 ++++++++
> admin/smart-rules.pl                               | 48 ++++++++++++--
> .../prog/en/modules/admin/smart-rules.tt           |  9 +++
> t/db_dependent/Holds.t                             | 76 +++++++++++++++++++++-
> 4 files changed, 156 insertions(+), 5 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 73b86b606c..8226aac085 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;
>@@ -398,6 +399,33 @@ sub CanItemBeReserved {
>         return 'tooManyReserves';
>     }
> 
>+    # Now we need to check hold limits by patron category
>+    my $rule = Koha::CirculationRules->find(
>+        {
>+            categorycode => $borrower->{categorycode},
>+            branchcode   => $borrower->{branchcode},
>+            itemtype     => undef,
>+            rule_name    => 'max_holds',
>+        }
>+    );
>+    $rule ||= Koha::CirculationRules->find(
>+        {
>+            categorycode => $borrower->{categorycode},
>+            branchcode   => undef,,
>+            itemtype     => undef,
>+            rule_name    => 'max_holds',
>+        }
>+    );
>+    if ( $rule ) {
>+        my $total_holds_count = Koha::Holds->search(
>+            {
>+                borrowernumber => $borrower->{borrowernumber}
>+            }
>+        )->count();
>+
>+        return 'tooManyReserves' if $total_holds_count >= $rule->rule_value;
>+    }
>+
>     my $circ_control_branch =
>       C4::Circulation::_GetCircControlBranch( $item->unblessed(), $borrower );
>     my $branchitemrule =
>diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
>index 718b68235c..b1e9d36ca8 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;
> 
> my $input = CGI->new;
>@@ -93,6 +94,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');
>@@ -277,10 +287,20 @@ elsif ($op eq "add-branch-cat") {
>             $sth_search->execute();
>             my $res = $sth_search->fetchrow_hashref();
>             if ($res->{total}) {
>-                $sth_update->execute($maxissueqty, $maxonsiteissueqty);
>+                $sth_update->execute( $maxissueqty, $maxonsiteissueqty );
>             } else {
>-                $sth_insert->execute($maxissueqty, $maxonsiteissueqty);
>+                $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
>@@ -299,10 +319,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, $categorycode);
>+                $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $categorycode );
>             } else {
>-                $sth_insert->execute($categorycode, $maxissueqty, $maxonsiteissueqty);
>+                $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
>@@ -351,6 +381,16 @@ elsif ($op eq "add-branch-cat") {
>         } else {
>             $sth_insert->execute($branch, $categorycode, $maxissueqty, $maxonsiteissueqty);
>         }
>+
>+        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 e1a5839f8a..46e9602fbc 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,4 +1,5 @@
> [% USE Branches %]
>+[% USE CirculationRules %]
> [% SET footerjs = 1 %]
> [% INCLUDE 'doc-head-open.inc' %]
> <title>Koha &rsaquo; Administration &rsaquo; Circulation and fine rules</title>
>@@ -490,6 +491,14 @@
>                                 [% branch_cat_rule_loo.maxonsiteissueqty %]
>                             [% END %]
>                         </td>
>+                        <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 %]
>+                                Unlimited
>+                            [% END %]
>+                        </td>
> 
>                         <td class="actions">
>                             <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete-branch-cat&amp;categorycode=[% branch_cat_rule_loo.categorycode %]&amp;branch=[% current_branch %]"><i class="fa fa-trash"></i> Delete</a>
>diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
>index 1e6c234487..c7767330db 100755
>--- a/t/db_dependent/Holds.t
>+++ b/t/db_dependent/Holds.t
>@@ -7,7 +7,7 @@ use t::lib::TestBuilder;
> 
> use C4::Context;
> 
>-use Test::More tests => 55;
>+use Test::More tests => 56;
> use MARC::Record;
> use C4::Biblio;
> use C4::Items;
>@@ -18,6 +18,7 @@ use Koha::DateUtils qw( dt_from_string output_pref );
> use Koha::Biblios;
> use Koha::Holds;
> use Koha::Patrons;
>+use Koha::CirculationRules;
> 
> BEGIN {
>     use FindBin;
>@@ -411,6 +412,79 @@ my $res_id = AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, );
> is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ),
>     'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' );
> 
>+subtest 'Test max_holds per library/patron category' => sub {
>+    plan tests => 6;
>+
>+    $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 ) =
>+      AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 },
>+        $bibnum );
>+    $dbh->do(
>+        q{
>+            INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
>+            VALUES (?, ?, ?, ?, ?)
>+        },
>+        {},
>+        '*', '*', 'TEST', 99, 99
>+    );
>+    AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, );
>+    AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, );
>+    AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, );
>+
>+    my $count =
>+      Koha::Holds->search( { borrowernumber => $borrowernumbers[0] } )->count();
>+    is( $count, 3, 'Patron now has 3 holds' );
>+
>+    my $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
>+    is( $ret, 'OK', 'Patron can place hold with no borrower circ rules' );
>+
>+    my $rule_all = Koha::CirculationRules->set_rule(
>+        {
>+            categorycode => $category->{categorycode},
>+            branchcode   => undef,
>+            itemtype     => undef,
>+            rule_name    => 'max_holds',
>+            rule_value   => 3,
>+        }
>+    );
>+
>+    my $rule_branch = Koha::CirculationRules->set_rule(
>+        {
>+            branchcode   => $branch_1,
>+            categorycode => $category->{categorycode},
>+            itemtype     => undef,
>+            rule_name    => 'max_holds',
>+            rule_value   => 5,
>+        }
>+    );
>+
>+    $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
>+    is( $ret, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 3' );
>+
>+    $rule_branch->delete();
>+
>+    $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
>+    is( $ret, 'tooManyReserves', 'Patron cannot place hold with only a category rule of 3' );
>+
>+    $rule_all->delete();
>+    $rule_branch->rule_value(3);
>+    $rule_branch->store();
>+
>+    $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
>+    is( $ret, 'tooManyReserves', 'Patron cannot place hold with only a branch/category rule of 3' );
>+
>+    $rule_branch->rule_value(5);
>+    $rule_branch->update();
>+    $rule_branch->rule_value(5);
>+    $rule_branch->store();
>+
>+    $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
>+    is( $ret, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 5' );
>+};
> 
> # Helper method to set up a Biblio.
> sub create_helper_biblio {
>-- 
>2.15.1

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