From dc94b4d05b4a28e9eee75439e2f53dd0d62176fa Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Tue, 11 Jul 2017 11:10:01 -0400
Subject: [PATCH] Bug 18928: Move holdallowed, hold_fulfillment_policy,
 returnbranch to circulation_rules

Test Plan:
1) Apply dependancies
2) Apply this patch set
3) Run updatedatabase.pl
4) Ensure holdallowed and hold_fulfillment_policy rules behavior remains unchanged

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 C4/Circulation.pm                             |  82 ++--
 admin/smart-rules.pl                          | 365 ++++++++----------
 .../data/mysql/atomicupdate/bug_18928.perl    |  81 ++++
 .../prog/en/modules/admin/smart-rules.tt      | 131 ++++---
 t/db_dependent/Circulation/Branch.t           |  61 +--
 t/db_dependent/Holds.t                        |  36 +-
 t/db_dependent/Holds/HoldFulfillmentPolicy.t  |  48 ++-
 t/db_dependent/Holds/HoldItemtypeLimit.t      |  19 +-
 t/db_dependent/HoldsQueue.t                   | 133 +++++--
 t/db_dependent/Reserves.t                     |  36 +-
 10 files changed, 595 insertions(+), 397 deletions(-)
 create mode 100644 installer/data/mysql/atomicupdate/bug_18928.perl

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 84349d98cf..7e3c9899ce 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1721,43 +1721,61 @@ Neither C<$branchcode> nor C<$itemtype> should be '*'.
 
 sub GetBranchItemRule {
     my ( $branchcode, $itemtype ) = @_;
-    my $dbh = C4::Context->dbh();
-    my $result = {};
-
-    my @attempts = (
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM branch_item_rules
-            WHERE branchcode = ?
-              AND itemtype = ?', $branchcode, $itemtype],
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM default_branch_circ_rules
-            WHERE branchcode = ?', $branchcode],
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM default_branch_item_rules
-            WHERE itemtype = ?', $itemtype],
-        ['SELECT holdallowed, returnbranch, hold_fulfillment_policy
-            FROM default_circ_rules'],
+
+    # Set search precedences
+    my @params = (
+        {
+            branchcode   => $branchcode,
+            categorycode => undef,
+            itemtype     => $itemtype,
+        },
+        {
+            branchcode   => $branchcode,
+            categorycode => undef,
+            itemtype     => undef,
+        },
+        {
+            branchcode   => undef,
+            categorycode => undef,
+            itemtype     => $itemtype,
+        },
+        {
+            branchcode   => undef,
+            categorycode => undef,
+            itemtype     => undef,
+        },
     );
 
-    foreach my $attempt (@attempts) {
-        my ($query, @bind_params) = @{$attempt};
-        my $search_result = $dbh->selectrow_hashref ( $query , {}, @bind_params )
-          or next;
-
-        # Since branch/category and branch/itemtype use the same per-branch
-        # defaults tables, we have to check that the key we want is set, not
-        # just that a row was returned
-        $result->{'holdallowed'}  = $search_result->{'holdallowed'}  unless ( defined $result->{'holdallowed'} );
-        $result->{'hold_fulfillment_policy'} = $search_result->{'hold_fulfillment_policy'} unless ( defined $result->{'hold_fulfillment_policy'} );
-        $result->{'returnbranch'} = $search_result->{'returnbranch'} unless ( defined $result->{'returnbranch'} );
+    # Initialize default values
+    my $rules = {
+        holdallowed             => undef,
+        hold_fulfillment_policy => undef,
+        returnbranch            => undef,
+    };
+
+    # Search for rules!
+    foreach my $rule_name (qw( holdallowed hold_fulfillment_policy returnbranch )) {
+        foreach my $params (@params) {
+            my $rule = Koha::CirculationRules->search(
+                {
+                    rule_name => $rule_name,
+                    %$params,
+                }
+            )->next();
+
+            if ( $rule ) {
+                $rules->{$rule_name} = $rule->rule_value;
+                last;
+            }
+        }
     }
-    
+
     # built-in default circulation rule
-    $result->{'holdallowed'} = 2 unless ( defined $result->{'holdallowed'} );
-    $result->{'hold_fulfillment_policy'} = 'any' unless ( defined $result->{'hold_fulfillment_policy'} );
-    $result->{'returnbranch'} = 'homebranch' unless ( defined $result->{'returnbranch'} );
+    $rules->{holdallowed} = 2                 unless ( defined $rules->{holdallowed} );
+    $rules->{hold_fulfillment_policy} = 'any' unless ( defined $rules->{hold_fulfillment_policy} );
+    $rules->{returnbranch} = 'homebranch'     unless ( defined $rules->{returnbranch} );
 
-    return $result;
+    return $rules;
 }
 
 =head2 AddReturn
diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
index 927533d14d..41c61f733d 100755
--- a/admin/smart-rules.pl
+++ b/admin/smart-rules.pl
@@ -93,47 +93,124 @@ elsif ($op eq 'delete-branch-cat') {
     my $categorycode  = $input->param('categorycode');
     if ($branch eq "*") {
         if ($categorycode eq "*") {
-            my $sth_delete = $dbh->prepare("DELETE FROM default_circ_rules");
-            $sth_delete->execute();
+             Koha::CirculationRules->set_rules(
+                 {
+                     categorycode => undef,
+                     branchcode   => undef,
+                     itemtype     => undef,
+                     rules        => {
+                         patron_maxissueqty             => undef,
+                         patron_maxonsiteissueqty       => undef,
+                         holdallowed             => undef,
+                         hold_fulfillment_policy => undef,
+                         returnbranch            => undef,
+                     }
+                 }
+             );
+         } else {
+             Koha::CirculationRules->set_rules(
+                 {
+                     categorycode => $categorycode,
+                     branchcode   => undef,
+                     itemtype     => undef,
+                     rules        => {
+                         max_holds         => undef,
+                         patron_maxissueqty       => undef,
+                         patron_maxonsiteissueqty => undef,
+                     }
+                 }
+             );
         }
     } elsif ($categorycode eq "*") {
-        my $sth_delete = $dbh->prepare("DELETE FROM default_branch_circ_rules
-                                        WHERE branchcode = ?");
-        $sth_delete->execute($branch);
-    }
-    Koha::CirculationRules->set_rules(
-        {
-            categorycode => $categorycode eq '*' ? undef : $categorycode,
-            branchcode   => $branch eq '*'       ? undef : $branch,
-            itemtype     => undef,
-            rules        => {
-                max_holds         => undef,
-                patron_maxissueqty             => undef,
-                patron_maxonsiteissueqty       => undef,
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                branchcode   => $branch,
+                itemtype     => undef,
+                rules        => {
+                    patron_maxissueqty             => undef,
+                    patron_maxonsiteissueqty       => undef,
+                    holdallowed             => undef,
+                    hold_fulfillment_policy => undef,
+                    returnbranch            => undef,
+                }
             }
-        }
-    );
+        );
+    } else {
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => $categorycode,
+                branchcode   => $branch,
+                itemtype     => undef,
+                rules        => {
+                    max_holds         => undef,
+                    patron_maxissueqty       => undef,
+                    patron_maxonsiteissueqty => undef,
+                }
+            }
+        );
+    }
 }
 elsif ($op eq 'delete-branch-item') {
     my $itemtype  = $input->param('itemtype');
     if ($branch eq "*") {
         if ($itemtype eq "*") {
-            my $sth_delete = $dbh->prepare("DELETE FROM default_circ_rules");
-            $sth_delete->execute();
+            Koha::CirculationRules->set_rules(
+                {
+                    categorycode => undef,
+                    branchcode   => undef,
+                    itemtype     => undef,
+                    rules        => {
+                        patron_maxissueqty             => undef,
+                        patron_maxonsiteissueqty       => undef,
+                        holdallowed             => undef,
+                        hold_fulfillment_policy => undef,
+                        returnbranch            => undef,
+                    }
+                }
+            );
         } else {
-            my $sth_delete = $dbh->prepare("DELETE FROM default_branch_item_rules
-                                            WHERE itemtype = ?");
-            $sth_delete->execute($itemtype);
+            Koha::CirculationRules->set_rules(
+                {
+                    categorycode => undef,
+                    branchcode   => undef,
+                    itemtype     => $itemtype,
+                    rules        => {
+                        holdallowed             => undef,
+                        hold_fulfillment_policy => undef,
+                        returnbranch            => undef,
+                    }
+                }
+            );
         }
     } elsif ($itemtype eq "*") {
-        my $sth_delete = $dbh->prepare("DELETE FROM default_branch_circ_rules
-                                        WHERE branchcode = ?");
-        $sth_delete->execute($branch);
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                branchcode   => $branch,
+                itemtype     => undef,
+                rules        => {
+                    maxissueqty             => undef,
+                    maxonsiteissueqty       => undef,
+                    holdallowed             => undef,
+                    hold_fulfillment_policy => undef,
+                    returnbranch            => undef,
+                }
+            }
+        );
     } else {
-        my $sth_delete = $dbh->prepare("DELETE FROM branch_item_rules
-                                        WHERE branchcode = ?
-                                        AND itemtype = ?");
-        $sth_delete->execute($branch, $itemtype);
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                branchcode   => $branch,
+                itemtype     => $itemtype,
+                rules        => {
+                    holdallowed             => undef,
+                    hold_fulfillment_policy => undef,
+                    returnbranch            => undef,
+                }
+            }
+        );
     }
 }
 # save the values entered
@@ -256,59 +333,32 @@ elsif ($op eq "set-branch-defaults") {
     $max_holds = '' if $max_holds !~ /^\d+/;
 
     if ($branch eq "*") {
-        my $sth_search = $dbh->prepare("SELECT count(*) AS total
-                                        FROM default_circ_rules");
-        my $sth_insert = $dbh->prepare("INSERT INTO default_circ_rules
-                                        (holdallowed, hold_fulfillment_policy, returnbranch)
-                                        VALUES (?, ?, ?)");
-        my $sth_update = $dbh->prepare("UPDATE default_circ_rules
-                                        SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?");
-
-        $sth_search->execute();
-        my $res = $sth_search->fetchrow_hashref();
-        if ($res->{total}) {
-            $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch);
-        } else {
-            $sth_insert->execute($holdallowed, $hold_fulfillment_policy, $returnbranch);
-        }
-
         Koha::CirculationRules->set_rules(
             {
                 categorycode => undef,
                 itemtype     => undef,
                 branchcode   => undef,
                 rules        => {
-                    patron_maxissueqty             => $patron_maxissueqty,
-                    patron_maxonsiteissueqty       => $patron_maxonsiteissueqty,
+                    patron_maxissueqty       => $patron_maxissueqty,
+                    patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
+                    holdallowed              => $holdallowed,
+                    hold_fulfillment_policy  => $hold_fulfillment_policy,
+                    returnbranch             => $returnbranch,
                 }
             }
         );
     } else {
-        my $sth_search = $dbh->prepare("SELECT count(*) AS total
-                                        FROM default_branch_circ_rules
-                                        WHERE branchcode = ?");
-        my $sth_insert = $dbh->prepare("INSERT INTO default_branch_circ_rules
-                                        (branchcode, holdallowed, hold_fulfillment_policy, returnbranch)
-                                        VALUES (?, ?, ?, ?)");
-        my $sth_update = $dbh->prepare("UPDATE default_branch_circ_rules
-                                        SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?
-                                        WHERE branchcode = ?");
-        $sth_search->execute($branch);
-        my $res = $sth_search->fetchrow_hashref();
-        if ($res->{total}) {
-            $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch, $branch);
-        } else {
-            $sth_insert->execute($branch, $holdallowed, $hold_fulfillment_policy, $returnbranch);
-        }
-
         Koha::CirculationRules->set_rules(
             {
                 categorycode => undef,
                 itemtype     => undef,
                 branchcode   => $branch,
                 rules        => {
-                    patron_maxissueqty             => $patron_maxissueqty,
-                    patron_maxonsiteissueqty       => $patron_maxonsiteissueqty,
+                    patron_maxissueqty       => $patron_maxissueqty,
+                    patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
+                    holdallowed              => $holdallowed,
+                    hold_fulfillment_policy  => $hold_fulfillment_policy,
+                    returnbranch             => $returnbranch,
                 }
             }
         );
@@ -402,76 +452,58 @@ elsif ($op eq "add-branch-item") {
 
     if ($branch eq "*") {
         if ($itemtype eq "*") {
-            my $sth_search = $dbh->prepare("SELECT count(*) AS total
-                                            FROM default_circ_rules");
-            my $sth_insert = $dbh->prepare("INSERT INTO default_circ_rules
-                                            (holdallowed, hold_fulfillment_policy, returnbranch)
-                                            VALUES (?, ?, ?)");
-            my $sth_update = $dbh->prepare("UPDATE default_circ_rules
-                                            SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?");
-
-            $sth_search->execute();
-            my $res = $sth_search->fetchrow_hashref();
-            if ($res->{total}) {
-                $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch);
-            } else {
-                $sth_insert->execute($holdallowed, $hold_fulfillment_policy, $returnbranch);
-            }
+            Koha::CirculationRules->set_rules(
+                {
+                    categorycode => undef,
+                    itemtype     => undef,
+                    branchcode   => undef,
+                    rules        => {
+                        holdallowed             => $holdallowed,
+                        hold_fulfillment_policy => $hold_fulfillment_policy,
+                        returnbranch            => $returnbranch,
+                    }
+                }
+            );
         } else {
-            my $sth_search = $dbh->prepare("SELECT count(*) AS total
-                                            FROM default_branch_item_rules
-                                            WHERE itemtype = ?");
-            my $sth_insert = $dbh->prepare("INSERT INTO default_branch_item_rules
-                                            (itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
-                                            VALUES (?, ?, ?, ?)");
-            my $sth_update = $dbh->prepare("UPDATE default_branch_item_rules
-                                            SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?
-                                            WHERE itemtype = ?");
-            $sth_search->execute($itemtype);
-            my $res = $sth_search->fetchrow_hashref();
-            if ($res->{total}) {
-                $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch, $itemtype);
-            } else {
-                $sth_insert->execute($itemtype, $holdallowed, $hold_fulfillment_policy, $returnbranch);
-            }
+            Koha::CirculationRules->set_rules(
+                {
+                    categorycode => undef,
+                    itemtype     => $itemtype,
+                    branchcode   => undef,
+                    rules        => {
+                        holdallowed             => $holdallowed,
+                        hold_fulfillment_policy => $hold_fulfillment_policy,
+                        returnbranch            => $returnbranch,
+                    }
+                }
+            );
         }
     } elsif ($itemtype eq "*") {
-        my $sth_search = $dbh->prepare("SELECT count(*) AS total
-                                        FROM default_branch_circ_rules
-                                        WHERE branchcode = ?");
-        my $sth_insert = $dbh->prepare("INSERT INTO default_branch_circ_rules
-                                        (branchcode, holdallowed, hold_fulfillment_policy, returnbranch)
-                                        VALUES (?, ?, ?, ?)");
-        my $sth_update = $dbh->prepare("UPDATE default_branch_circ_rules
-                                        SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?
-                                        WHERE branchcode = ?");
-        $sth_search->execute($branch);
-        my $res = $sth_search->fetchrow_hashref();
-        if ($res->{total}) {
-            $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch, $branch);
-        } else {
-            $sth_insert->execute($branch, $holdallowed, $hold_fulfillment_policy, $returnbranch);
-        }
+            Koha::CirculationRules->set_rules(
+                {
+                    categorycode => undef,
+                    itemtype     => undef,
+                    branchcode   => $branch,
+                    rules        => {
+                        holdallowed             => $holdallowed,
+                        hold_fulfillment_policy => $hold_fulfillment_policy,
+                        returnbranch            => $returnbranch,
+                    }
+                }
+            );
     } else {
-        my $sth_search = $dbh->prepare("SELECT count(*) AS total
-                                        FROM branch_item_rules
-                                        WHERE branchcode = ?
-                                        AND   itemtype = ?");
-        my $sth_insert = $dbh->prepare("INSERT INTO branch_item_rules
-                                        (branchcode, itemtype, holdallowed, hold_fulfillment_policy, returnbranch)
-                                        VALUES (?, ?, ?, ?, ?)");
-        my $sth_update = $dbh->prepare("UPDATE branch_item_rules
-                                        SET holdallowed = ?, hold_fulfillment_policy = ?, returnbranch = ?
-                                        WHERE branchcode = ?
-                                        AND itemtype = ?");
-
-        $sth_search->execute($branch, $itemtype);
-        my $res = $sth_search->fetchrow_hashref();
-        if ($res->{total}) {
-            $sth_update->execute($holdallowed, $hold_fulfillment_policy, $returnbranch, $branch, $itemtype);
-        } else {
-            $sth_insert->execute($branch, $itemtype, $holdallowed, $hold_fulfillment_policy, $returnbranch);
-        }
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                itemtype     => $itemtype,
+                branchcode   => $branch,
+                rules        => {
+                    holdallowed             => $holdallowed,
+                    hold_fulfillment_policy => $hold_fulfillment_policy,
+                    returnbranch            => $returnbranch,
+                }
+            }
+        );
     }
 }
 elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) {
@@ -554,76 +586,7 @@ while (my $row = $sth2->fetchrow_hashref) {
 
 my @sorted_row_loop = sort by_category_and_itemtype @row_loop;
 
-my $sth_branch_item;
-if ($branch eq "*") {
-    $sth_branch_item = $dbh->prepare("
-        SELECT default_branch_item_rules.*,
-            COALESCE( localization.translation, itemtypes.description ) AS translated_description
-        FROM default_branch_item_rules
-        JOIN itemtypes USING (itemtype)
-        LEFT JOIN localization ON itemtypes.itemtype = localization.code
-            AND localization.entity = 'itemtypes'
-            AND localization.lang = ?
-    ");
-    $sth_branch_item->execute($language);
-} else {
-    $sth_branch_item = $dbh->prepare("
-        SELECT branch_item_rules.*,
-            COALESCE( localization.translation, itemtypes.description ) AS translated_description
-        FROM branch_item_rules
-        JOIN itemtypes USING (itemtype)
-        LEFT JOIN localization ON itemtypes.itemtype = localization.code
-            AND localization.entity = 'itemtypes'
-            AND localization.lang = ?
-        WHERE branch_item_rules.branchcode = ?
-    ");
-    $sth_branch_item->execute($language, $branch);
-}
-
-my @branch_item_rules = ();
-while (my $row = $sth_branch_item->fetchrow_hashref) {
-    push @branch_item_rules, $row;
-}
-my @sorted_branch_item_rules = sort { lc $a->{translated_description} cmp lc $b->{translated_description} } @branch_item_rules;
-
-# note undef holdallowed so that template can deal with them
-foreach my $entry (@sorted_branch_item_rules) {
-    $entry->{holdallowed_any}  = 1 if ( $entry->{holdallowed} == 2 );
-    $entry->{holdallowed_same} = 1 if ( $entry->{holdallowed} == 1 );
-}
-
 $template->param(show_branch_cat_rule_form => 1);
-$template->param(branch_item_rule_loop => \@sorted_branch_item_rules);
-
-my $sth_defaults;
-if ($branch eq "*") {
-    $sth_defaults = $dbh->prepare("
-        SELECT *
-        FROM default_circ_rules
-    ");
-    $sth_defaults->execute();
-} else {
-    $sth_defaults = $dbh->prepare("
-        SELECT *
-        FROM default_branch_circ_rules
-        WHERE branchcode = ?
-    ");
-    $sth_defaults->execute($branch);
-}
-
-my $defaults = $sth_defaults->fetchrow_hashref;
-
-if ($defaults) {
-    $template->param( default_holdallowed_none => 1 ) if ( $defaults->{holdallowed} == 0 );
-    $template->param( default_holdallowed_same => 1 ) if ( $defaults->{holdallowed} == 1 );
-    $template->param( default_holdallowed_any  => 1 ) if ( $defaults->{holdallowed} == 2 );
-    $template->param( default_hold_fulfillment_policy => $defaults->{hold_fulfillment_policy} );
-    $template->param( default_maxissueqty      => $defaults->{maxissueqty} );
-    $template->param( default_maxonsiteissueqty => $defaults->{maxonsiteissueqty} );
-    $template->param( default_returnbranch      => $defaults->{returnbranch} );
-}
-
-$template->param(default_rules => ($defaults ? 1 : 0));
 
 $template->param(
     patron_categories => $patron_categories,
diff --git a/installer/data/mysql/atomicupdate/bug_18928.perl b/installer/data/mysql/atomicupdate/bug_18928.perl
new file mode 100644
index 0000000000..3fcb9d1628
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug_18928.perl
@@ -0,0 +1,81 @@
+$DBversion = 'XXX';  # will be replaced by the RM
+if( CheckVersion( $DBversion ) ) {
+    if ( column_exists( 'default_circ_rules', 'holdallowed' ) ) {
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, NULL, NULL, 'holdallowed', holdallowed
+            FROM default_circ_rules
+        ");
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, NULL, NULL, 'hold_fulfillment_policy', hold_fulfillment_policy
+            FROM default_circ_rules
+        ");
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, NULL, NULL, 'returnbranch', returnbranch
+            FROM default_circ_rules
+        ");
+        $dbh->do("DROP TABLE default_circ_rules");
+    }
+
+    if ( column_exists( 'default_branch_circ_rules', 'holdallowed' ) ) {
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, branchcode, NULL, 'holdallowed', holdallowed
+            FROM default_branch_circ_rules
+        ");
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, branchcode, NULL, 'hold_fulfillment_policy', hold_fulfillment_policy
+            FROM default_branch_circ_rules
+        ");
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, branchcode, NULL, 'returnbranch', returnbranch
+            FROM default_branch_circ_rules
+        ");
+        $dbh->do("DROP TABLE default_branch_circ_rules");
+    }
+
+    if ( column_exists( 'branch_item_rules', 'holdallowed' ) ) {
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, branchcode, itemtype, 'holdallowed', holdallowed
+            FROM branch_item_rules
+        ");
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, branchcode, itemtype, 'hold_fulfillment_policy', hold_fulfillment_policy
+            FROM branch_item_rules
+        ");
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, branchcode, itemtype, 'returnbranch', returnbranch
+            FROM branch_item_rules
+        ");
+        $dbh->do("DROP TABLE branch_item_rules");
+    }
+
+    if ( column_exists( 'default_branch_item_rules', 'holdallowed' ) ) {
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, NULL, itemtype, 'holdallowed', holdallowed
+            FROM default_branch_item_rules
+        ");
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, NULL, itemtype, 'hold_fulfillment_policy', hold_fulfillment_policy
+            FROM default_branch_item_rules
+        ");
+        $dbh->do("
+            INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value )
+            SELECT NULL, NULL, itemtype, 'returnbranch', returnbranch
+            FROM default_branch_item_rules
+        ");
+        $dbh->do("DROP TABLE default_branch_item_rules");
+    }
+
+    SetVersion( $DBversion );
+    print "Upgrade to $DBversion done (Bug 18928 - Move holdallowed, hold_fulfillment_policy, returnbranch to circulation_rules)\n";
+}
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 2fb25d2909..9bfe6760b9 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
@@ -424,24 +424,31 @@
                     </td>
                     <td>
                         <select name="holdallowed">
-                            [% IF ( default_holdallowed_any ) %]
-                            <option value="2" selected="selected">
+                            [% SET holdallowed = CirculationRules.Get( branchcode, undef, undef, 'holdallowed' ) %]
+                            <option value="">
+                                Not set
+                            </option>
+
+                            [% IF holdallowed == 2 %]
+                                <option value="2" selected="selected">
                             [% ELSE %]
-                            <option value="2">
+                                <option value="2">
                             [% END %]
                                 From any library
                             </option>
-                            [% IF ( default_holdallowed_same ) %]
-                            <option value="1" selected="selected">
+
+                            [% IF holdallowed == 1 %]
+                                <option value="1" selected="selected">
                             [% ELSE %]
-                            <option value="1">
+                                <option value="1">
                             [% END %]
                                 From home library
                             </option>
-                            [% IF ( default_holdallowed_none ) %]
-                            <option value="0" selected="selected">
+
+                            [% IF holdallowed == 0 %]
+                                <option value="0" selected="selected">
                             [% ELSE %]
-                            <option value="0">
+                                <option value="0">
                             [% END %]
                                 No holds allowed
                             </option>
@@ -449,7 +456,13 @@
                     </td>
                     <td>
                         <select name="hold_fulfillment_policy">
-                            [% IF default_hold_fulfillment_policy == 'any' %]
+                            [% SET hold_fulfillment_policy = CirculationRules.Get( branchcode, undef, undef, 'hold_fulfillment_policy' ) %]
+
+                            <option value="">
+                                Not set
+                            </option>
+
+                            [% IF hold_fulfillment_policy == 'any' %]
                                 <option value="any" selected="selected">
                                     any library
                                 </option>
@@ -459,7 +472,7 @@
                                 </option>
                             [% END %]
 
-                            [% IF default_hold_fulfillment_policy == 'homebranch' %]
+                            [% IF hold_fulfillment_policy == 'homebranch' %]
                                 <option value="homebranch" selected="selected">
                                     item's home library
                                 </option>
@@ -469,7 +482,7 @@
                                 </option>
                             [% END %]
 
-                            [% IF default_hold_fulfillment_policy == 'holdingbranch' %]
+                            [% IF hold_fulfillment_policy == 'holdingbranch' %]
                                 <option value="holdingbranch" selected="selected">
                                     item's holding library
                                 </option>
@@ -482,21 +495,27 @@
                     </td>
                     <td>
                         <select name="returnbranch">
-                            [% IF ( default_returnbranch == 'homebranch' ) %]
+                            [% SET returnbranch = CirculationRules.Get( branchcode, undef, undef, 'returnbranch' ) %]
+
+                            <option value="">
+                                Not set
+                            </option>
+
+                            [% IF returnbranch == 'homebranch' %]
                             <option value="homebranch" selected="selected">
                             [% ELSE %]
                             <option value="homebranch">
                             [% END %]
                                 Item returns home
                             </option>
-                            [% IF ( default_returnbranch == 'holdingbranch' ) %]
+                            [% IF returnbranch == 'holdingbranch' %]
                             <option value="holdingbranch" selected="selected">
                             [% ELSE %]
                             <option value="holdingbranch">
                             [% END %]
                                 Item returns to issuing library
                             </option>
-                            [% IF ( default_returnbranch == 'noreturn' ) %]
+                            [% IF returnbranch == 'noreturn' %]
                             <option value="noreturn" selected="selected">
                             [% ELSE %]
                             <option value="noreturn">
@@ -702,48 +721,48 @@
                     <th>Return policy</th>
                     <th>&nbsp;</th>
                 </tr>
-                [% FOREACH branch_item_rule_loo IN branch_item_rule_loop %]
-                    [% UNLESS ( loop.odd ) %]
-                    <tr class="highlight">
-                    [% ELSE %]
-                    <tr>
+                [% FOREACH i IN itemtypeloop %]
+                    [% SET holdallowed = CirculationRules.Get( branchcode, undef, i.itemtype, 'holdallowed' ) %]
+                    [% SET hold_fulfillment_policy = CirculationRules.Get( branchcode, undef, i.itemtype, 'hold_fulfillment_policy' ) %]
+                    [% SET returnbranch = CirculationRules.Get( branchcode, undef, i.itemtype, 'returnbranch' ) %]
+
+                    [% IF holdallowed || hold_fulfillment_policy || returnbranch %]
+                        <tr>
+                            <td>
+                                [% i.translated_description %]
+                            </td>
+                            <td>
+                                [% IF holdallowed == 2 %]
+                                    <span>From any library</span>
+                                [% ELSIF holdallowed == 1 %]
+                                    <span>From home library</span>
+                                [% ELSE %]
+                                    <span>No holds allowed</span>
+                                [% END %]
+                            </td>
+                            <td>
+                                [% IF hold_fulfillment_policy == 'any' %]
+                                    <span>any library</span>
+                                [% ELSIF hold_fulfillment_policy == 'homebranch' %]
+                                    <span>item's home library</span>
+                                [% ELSIF hold_fulfillment_policy == 'holdingbranch' %]
+                                    <span>item's holding library</span>
+                                [% END %]
+                            </td>
+                            <td>
+                                [% IF returnbranch == 'homebranch' %]
+                                    <span>Item returns home</span>
+                                [% ELSIF returnbranch == 'holdingbranch' %]
+                                    <span>Item returns to issuing branch</span>
+                                [% ELSIF returnbranch == 'noreturn' %]
+                                    <span>Item floats</span>
+                                [% END %]
+                            </td>
+                            <td class="actions">
+                                <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete-branch-item&amp;itemtype=[% i.itemtype %]&amp;branch=[% current_branch %]"><i class="fa fa-trash"></i> Delete</a>
+                            </td>
+                        </tr>
                     [% END %]
-                        <td>[% IF ( branch_item_rule_loo.default_translated_description ) %]
-                                <em>Default</em>
-                            [% ELSE %]
-                                [% branch_item_rule_loo.translated_description | html %]
-                            [% END %]
-                        </td>
-                        <td>[% IF ( branch_item_rule_loo.holdallowed_any ) %]
-                                <span>From any library</span>
-                            [% ELSIF ( branch_item_rule_loo.holdallowed_same ) %]
-                                <span>From home library</span>
-                            [% ELSE %]
-                                <span>No holds allowed</span>
-                            [% END %]
-                        </td>
-                        <td>[% IF ( branch_item_rule_loo.hold_fulfillment_policy == 'any' ) %]
-                                <span>any library</span>
-                            [% ELSIF ( branch_item_rule_loo.hold_fulfillment_policy == 'homebranch' ) %]
-                                <span>item's home library</span>
-                            [% ELSIF ( branch_item_rule_loo.hold_fulfillment_policy == 'holdingbranch' ) %]
-                                <span>item's holding library</span>
-                            [% END %]
-                        </td>
-                        <td>[% IF ( branch_item_rule_loo.returnbranch == 'homebranch' ) %]
-                                <span>Item returns home</span>
-                            [% ELSIF ( branch_item_rule_loo.returnbranch == 'holdingbranch' ) %]
-                                <span>Item returns to issuing branch</span>
-                            [% ELSIF ( branch_item_rule_loo.returnbranch == 'noreturn' ) %]
-                                <span>Item floats</span>
-                            [% ELSE %]
-                                <span>Error - unknown option</span>
-                            [% END %]
-                        </td>
-                        <td class="actions">
-                            <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete-branch-item&amp;itemtype=[% branch_item_rule_loo.itemtype | html %]&amp;branch=[% current_branch | html %]"><i class="fa fa-trash"></i> Delete</a>
-                        </td>
-                    </tr>
                 [% END %]
                 <tr>
                     <td>
diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t
index db2c169715..1c2d17a65f 100644
--- a/t/db_dependent/Circulation/Branch.t
+++ b/t/db_dependent/Circulation/Branch.t
@@ -44,6 +44,7 @@ can_ok( 'C4::Circulation', qw(
 my $schema = Koha::Database->schema;
 $schema->storage->txn_begin;
 my $dbh = C4::Context->dbh;
+my $query;
 
 $dbh->do(q|DELETE FROM issues|);
 $dbh->do(q|DELETE FROM items|);
@@ -53,10 +54,7 @@ $dbh->do(q|DELETE FROM branches|);
 $dbh->do(q|DELETE FROM categories|);
 $dbh->do(q|DELETE FROM accountlines|);
 $dbh->do(q|DELETE FROM itemtypes|);
-$dbh->do(q|DELETE FROM branch_item_rules|);
-$dbh->do(q|DELETE FROM default_branch_circ_rules|);
-$dbh->do(q|DELETE FROM default_circ_rules|);
-$dbh->do(q|DELETE FROM default_branch_item_rules|);
+$dbh->do(q|DELETE FROM circulation_rules|);
 
 my $builder = t::lib::TestBuilder->new();
 
@@ -164,12 +162,6 @@ Koha::CirculationRules->set_rules(
     }
 );
 
-my $query = q|
-    INSERT INTO default_branch_circ_rules
-    (branchcode, holdallowed, returnbranch)
-    VALUES( ?, ?, ? )
-|;
-$dbh->do( $query, {}, $samplebranch2->{branchcode}, 1, 'holdingbranch' );
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $samplebranch2->{branchcode},
@@ -178,6 +170,8 @@ Koha::CirculationRules->set_rules(
         rules        => {
             patron_maxissueqty       => 3,
             patron_maxonsiteissueqty => 2,
+            holdallowed              => 1,
+            returnbranch             => 'holdingbranch',
         }
     }
 );
@@ -196,27 +190,44 @@ Koha::CirculationRules->set_rules(
         rules        => {
             patron_maxissueqty       => 4,
             patron_maxonsiteissueqty => 5,
+            holdallowed              => 3,
+            returnbranch             => 'homebranch',
         }
     }
 );
 
-$query =
-"INSERT INTO branch_item_rules (branchcode,itemtype,holdallowed,returnbranch) VALUES( ?,?,?,?)";
-my $sth = $dbh->prepare($query);
-$sth->execute(
-    $samplebranch1->{branchcode},
-    $sampleitemtype1->{itemtype},
-    5, 'homebranch'
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $samplebranch1->{branchcode},
+        categorycode => undef,
+        itemtype     => $sampleitemtype1->{itemtype},
+        rules        => {
+            holdallowed       => 5,
+            returnbranch      => 'homebranch',
+        }
+    }
 );
-$sth->execute(
-    $samplebranch2->{branchcode},
-    $sampleitemtype1->{itemtype},
-    5, 'holdingbranch'
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $samplebranch2->{branchcode},
+        categorycode => undef,
+        itemtype     => $sampleitemtype1->{itemtype},
+        rules        => {
+            holdallowed       => 5,
+            returnbranch      => 'holdingbranch',
+        }
+    }
 );
-$sth->execute(
-    $samplebranch2->{branchcode},
-    $sampleitemtype2->{itemtype},
-    5, 'noreturn'
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $samplebranch2->{branchcode},
+        categorycode => undef,
+        itemtype     => $sampleitemtype2->{itemtype},
+        rules        => {
+            holdallowed       => 5,
+            returnbranch      => 'noreturn',
+        }
+    }
 );
 
 #Test GetBranchBorrowerCircRule
diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
index 1515b4bef4..f7919b3827 100755
--- a/t/db_dependent/Holds.t
+++ b/t/db_dependent/Holds.t
@@ -47,6 +47,7 @@ my $borrowers_count = 5;
 
 $dbh->do('DELETE FROM itemtypes');
 $dbh->do('DELETE FROM reserves');
+$dbh->do('DELETE FROM circulation_rules');
 my $insert_sth = $dbh->prepare('INSERT INTO itemtypes (itemtype) VALUES (?)');
 $insert_sth->execute('CAN');
 $insert_sth->execute('CANNOT');
@@ -351,24 +352,35 @@ ok(
 # Test branch item rules
 
 $dbh->do('DELETE FROM issuingrules');
+$dbh->do('DELETE FROM circulation_rules');
 $dbh->do(
     q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
       VALUES (?, ?, ?, ?)},
     {},
     '*', '*', '*', 25
 );
-$dbh->do('DELETE FROM branch_item_rules');
-$dbh->do('DELETE FROM default_branch_circ_rules');
-$dbh->do('DELETE FROM default_branch_item_rules');
-$dbh->do('DELETE FROM default_circ_rules');
-$dbh->do(q{
-    INSERT INTO branch_item_rules (branchcode, itemtype, holdallowed, returnbranch)
-    VALUES (?, ?, ?, ?)
-}, {}, $branch_1, 'CANNOT', 0, 'homebranch');
-$dbh->do(q{
-    INSERT INTO branch_item_rules (branchcode, itemtype, holdallowed, returnbranch)
-    VALUES (?, ?, ?, ?)
-}, {}, $branch_1, 'CAN', 1, 'homebranch');
+Koha::CirculationRules->set_rules(
+    {
+        branchcode => $branch_1,
+        itemtype   => 'CANNOT',
+        categorycode => undef,
+        rules => {
+            holdallowed => 0,
+            returnbranch => 'homebranch',
+        }
+    }
+);
+Koha::CirculationRules->set_rules(
+    {
+        branchcode => $branch_1,
+        itemtype   => 'CAN',
+        categorycode => undef,
+        rules => {
+            holdallowed => 1,
+            returnbranch => 'homebranch',
+        }
+    }
+);
 $biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' });
 ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem(
     { homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $biblio->biblionumber);
diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t
index 42fd5f4fe2..1916d08972 100755
--- a/t/db_dependent/Holds/HoldFulfillmentPolicy.t
+++ b/t/db_dependent/Holds/HoldFulfillmentPolicy.t
@@ -3,6 +3,7 @@
 use Modern::Perl;
 
 use C4::Context;
+use Koha::CirculationRules;
 
 use Test::More tests => 11;
 
@@ -48,10 +49,7 @@ my $library_C = $library3->{branchcode};
 $dbh->do("DELETE FROM transport_cost");
 $dbh->do("DELETE FROM tmp_holdsqueue");
 $dbh->do("DELETE FROM hold_fill_targets");
-$dbh->do("DELETE FROM default_branch_circ_rules");
-$dbh->do("DELETE FROM default_branch_item_rules");
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("DELETE FROM branch_item_rules");
+$dbh->do("DELETE FROM circulation_rules");
 
 $dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$bib_title', '2011-02-01')");
 
@@ -74,8 +72,18 @@ my $itemnumber =
   or BAIL_OUT("Cannot find newly created item");
 
 # With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'homebranch' )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rules(
+    {
+        categorycode => undef,
+        branchcode   => undef,
+        itemtype     => undef,
+        rules        => {
+            holdallowed             => 2,
+            hold_fulfillment_policy => 'homebranch',
+        }
+    }
+);
 
 # Home branch matches pickup branch
 my $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
@@ -96,8 +104,18 @@ is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # With hold_fulfillment_policy = holdingbranch, hold should only be picked up if pickup branch = holdingbranch
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'holdingbranch' )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rules(
+    {
+        categorycode => undef,
+        branchcode   => undef,
+        itemtype     => undef,
+        rules        => {
+            holdallowed             => 2,
+            hold_fulfillment_policy => 'holdingbranch',
+        }
+    }
+);
 
 # Home branch matches pickup branch
 $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
@@ -118,8 +136,18 @@ is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # With hold_fulfillment_policy = any, hold should be pikcup up reguardless of matching home or holding branch
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'any' )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rules(
+    {
+        categorycode => undef,
+        branchcode   => undef,
+        itemtype     => undef,
+        rules        => {
+            holdallowed             => 2,
+            hold_fulfillment_policy => 'any',
+        }
+    }
+);
 
 # Home branch matches pickup branch
 $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
diff --git a/t/db_dependent/Holds/HoldItemtypeLimit.t b/t/db_dependent/Holds/HoldItemtypeLimit.t
index ba3e9c71f1..ce206de531 100644
--- a/t/db_dependent/Holds/HoldItemtypeLimit.t
+++ b/t/db_dependent/Holds/HoldItemtypeLimit.t
@@ -3,6 +3,7 @@
 use Modern::Perl;
 
 use C4::Context;
+use Koha::CirculationRules;
 
 use Test::More tests => 4;
 
@@ -64,10 +65,6 @@ $dbh->do("DELETE FROM biblioitems");
 $dbh->do("DELETE FROM transport_cost");
 $dbh->do("DELETE FROM tmp_holdsqueue");
 $dbh->do("DELETE FROM hold_fill_targets");
-$dbh->do("DELETE FROM default_branch_circ_rules");
-$dbh->do("DELETE FROM default_branch_item_rules");
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("DELETE FROM branch_item_rules");
 
 $dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$bib_title', '2011-02-01')");
 
@@ -89,8 +86,18 @@ my $itemnumber =
   $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber")
   or BAIL_OUT("Cannot find newly created item");
 
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'any' )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rules(
+    {
+        itemtype     => undef,
+        categorycode => undef,
+        branchcode   => undef,
+        rules        => {
+            holdallowed             => 2,
+            hold_fulfillment_policy => 'any',
+        }
+    }
+);
 
 # Itemtypes match
 my $reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $right_itemtype );
diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t
index 1dce46c892..8c8bb430dd 100755
--- a/t/db_dependent/HoldsQueue.t
+++ b/t/db_dependent/HoldsQueue.t
@@ -18,6 +18,7 @@ use Koha::Database;
 use Koha::DateUtils;
 use Koha::Items;
 use Koha::Holds;
+use Koha::CirculationRules;
 
 use t::lib::TestBuilder;
 use t::lib::Mocks;
@@ -32,6 +33,7 @@ BEGIN {
 my $schema = Koha::Database->schema;
 $schema->storage->txn_begin;
 my $dbh = C4::Context->dbh;
+$dbh->do("DELETE FROM circulation_rules");
 
 my $builder = t::lib::TestBuilder->new;
 
@@ -175,9 +177,7 @@ $schema->txn_begin;
 ### Test holds queue builder does not violate holds policy ###
 
 # Clear out existing rules relating to holdallowed
-$dbh->do("DELETE FROM default_branch_circ_rules");
-$dbh->do("DELETE FROM default_branch_item_rules");
-$dbh->do("DELETE FROM default_circ_rules");
+$dbh->do("DELETE FROM circulation_rules");
 
 t::lib::Mocks::mock_preference('UseTransportCostMatrix', 0);
 
@@ -290,8 +290,16 @@ $sth->execute( $borrower3->{borrowernumber}, $biblionumber, $branchcodes[0], 3 )
 
 my $holds_queue;
 
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed ) VALUES ( 1 )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rule(
+    {
+        rule_name    => 'holdallowed',
+        rule_value   => 1,
+        branchcode   => undef,
+        categorycode => undef,
+        itemtype     => undef,
+    }
+);
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} });
 is( @$holds_queue, 2, "Holds queue filling correct number for default holds policy 'from home library'" );
@@ -320,8 +328,16 @@ $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice
 is( scalar( @$holds_queue ), 1, "Holds not filled with items from closed libraries" );
 t::lib::Mocks::mock_preference('HoldsQueueSkipClosed', 0);
 
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed ) VALUES ( 2 )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rule(
+    {
+        rule_name    => 'holdallowed',
+        rule_value   => 2,
+        branchcode   => undef,
+        categorycode => undef,
+        itemtype     => undef,
+    }
+);
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} });
 is( @$holds_queue, 3, "Holds queue filling correct number for holds for default holds policy 'from any library'" );
@@ -340,8 +356,16 @@ t::lib::Mocks::mock_preference( 'HoldsQueueSkipClosed', 0 );
 ## Test LocalHoldsPriority
 t::lib::Mocks::mock_preference('LocalHoldsPriority', 1);
 
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed ) VALUES ( 2 )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rule(
+    {
+        rule_name    => 'holdallowed',
+        rule_value   => 2,
+        branchcode   => undef,
+        categorycode => undef,
+        itemtype     => undef,
+    }
+);
 $dbh->do("DELETE FROM issues");
 
 # Test homebranch = patron branch
@@ -473,10 +497,6 @@ $dbh->do("DELETE FROM biblioitems");
 $dbh->do("DELETE FROM transport_cost");
 $dbh->do("DELETE FROM tmp_holdsqueue");
 $dbh->do("DELETE FROM hold_fill_targets");
-$dbh->do("DELETE FROM default_branch_circ_rules");
-$dbh->do("DELETE FROM default_branch_item_rules");
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("DELETE FROM branch_item_rules");
 
 $dbh->do("
     INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$TITLE', '2011-02-01')
@@ -501,10 +521,17 @@ $dbh->do("
     VALUES ($biblionumber, $biblioitemnumber, '$library_B', '$library_B', 0, 0, 0, 0, NULL, '$itemtype')
 ");
 
-$dbh->do("
-    INSERT INTO branch_item_rules ( branchcode, itemtype, holdallowed, returnbranch ) VALUES
-    ( '$library_A', '$itemtype', 2, 'homebranch' ), ( '$library_B', '$itemtype', 1, 'homebranch' );
-");
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $library_A,
+        itemtype     => $itemtype,
+        categorycode => undef,
+        rules        => {
+            holdallowed  => 2,
+            returnbranch => 'homebranch',
+        }
+    }
+);
 
 $dbh->do( "UPDATE systempreferences SET value = ? WHERE variable = 'StaticHoldsQueueWeight'",
     undef, join( ',', $library_B, $library_A, $library_C ) );
@@ -529,10 +556,6 @@ $dbh->do("DELETE FROM biblioitems");
 $dbh->do("DELETE FROM transport_cost");
 $dbh->do("DELETE FROM tmp_holdsqueue");
 $dbh->do("DELETE FROM hold_fill_targets");
-$dbh->do("DELETE FROM default_branch_circ_rules");
-$dbh->do("DELETE FROM default_branch_item_rules");
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("DELETE FROM branch_item_rules");
 
 t::lib::Mocks::mock_preference("UseTransportCostMatrix",1);
 
@@ -578,10 +601,6 @@ $dbh->do("DELETE FROM biblioitems");
 $dbh->do("DELETE FROM transport_cost");
 $dbh->do("DELETE FROM tmp_holdsqueue");
 $dbh->do("DELETE FROM hold_fill_targets");
-$dbh->do("DELETE FROM default_branch_circ_rules");
-$dbh->do("DELETE FROM default_branch_item_rules");
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("DELETE FROM branch_item_rules");
 
 $dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$TITLE', '2011-02-01')");
 
@@ -600,8 +619,18 @@ $dbh->do("
 ");
 
 # With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'homebranch' )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => undef,
+        itemtype     => undef,
+        categorycode => undef,
+        rules        => {
+            holdallowed             => 2,
+            hold_fulfillment_policy => 'homebranch',
+        }
+    }
+);
 
 # Home branch matches pickup branch
 $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
@@ -625,8 +654,18 @@ is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted
 Koha::Holds->find( $reserve_id )->cancel;
 
 # With hold_fulfillment_policy = holdingbranch, hold should only be picked up if pickup branch = holdingbranch
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'holdingbranch' )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => undef,
+        itemtype     => undef,
+        categorycode => undef,
+        rules        => {
+            holdallowed             => 2,
+            hold_fulfillment_policy => 'holdingbranch',
+        }
+    }
+);
 
 # Home branch matches pickup branch
 $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
@@ -650,8 +689,18 @@ is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted
 Koha::Holds->find( $reserve_id )->cancel;
 
 # With hold_fulfillment_policy = any, hold should be pikcup up reguardless of matching home or holding branch
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'any' )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => undef,
+        itemtype     => undef,
+        categorycode => undef,
+        rules        => {
+            holdallowed             => 2,
+            hold_fulfillment_policy => 'any',
+        }
+    }
+);
 
 # Home branch matches pickup branch
 $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
@@ -690,10 +739,6 @@ $dbh->do("DELETE FROM biblioitems");
 $dbh->do("DELETE FROM transport_cost");
 $dbh->do("DELETE FROM tmp_holdsqueue");
 $dbh->do("DELETE FROM hold_fill_targets");
-$dbh->do("DELETE FROM default_branch_circ_rules");
-$dbh->do("DELETE FROM default_branch_item_rules");
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("DELETE FROM branch_item_rules");
 
 $dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$TITLE', '2011-02-01')");
 
@@ -712,8 +757,18 @@ $dbh->do("
 ");
 
 # With hold_fulfillment_policy = homebranch, hold should only be picked up if pickup branch = homebranch
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("INSERT INTO default_circ_rules ( holdallowed, hold_fulfillment_policy ) VALUES ( 2, 'any' )");
+$dbh->do("DELETE FROM circulation_rules");
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => undef,
+        itemtype     => undef,
+        categorycode => undef,
+        rules        => {
+            holdallowed             => 2,
+            hold_fulfillment_policy => 'any',
+        }
+    }
+);
 
 # Home branch matches pickup branch
 $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $wrong_itemtype );
@@ -747,10 +802,6 @@ t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'homebranch');
 $dbh->do("DELETE FROM tmp_holdsqueue");
 $dbh->do("DELETE FROM hold_fill_targets");
 $dbh->do("DELETE FROM reserves");
-$dbh->do("DELETE FROM default_branch_circ_rules");
-$dbh->do("DELETE FROM default_branch_item_rules");
-$dbh->do("DELETE FROM default_circ_rules");
-$dbh->do("DELETE FROM branch_item_rules");
 
 $item = Koha::Items->find( { biblionumber => $biblionumber } );
 $item->holdingbranch( $item->homebranch );
diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
index 624335196a..cdc5f06633 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -40,6 +40,7 @@ use Koha::Libraries;
 use Koha::Notice::Templates;
 use Koha::Patrons;
 use Koha::Patron::Categories;
+use Koha::CirculationRules;
 
 BEGIN {
     require_ok('C4::Reserves');
@@ -50,6 +51,7 @@ my $database = Koha::Database->new();
 my $schema = $database->schema();
 $schema->storage->txn_begin();
 my $dbh = C4::Context->dbh;
+$dbh->do('DELETE FROM circulation_rules');
 
 my $builder = t::lib::TestBuilder->new;
 
@@ -189,10 +191,6 @@ $requesters{$branch_3} = Koha::Patron->new({
 # to fill holds from anywhere.
 
 $dbh->do('DELETE FROM issuingrules');
-$dbh->do('DELETE FROM branch_item_rules');
-$dbh->do('DELETE FROM default_branch_item_rules');
-$dbh->do('DELETE FROM default_branch_circ_rules');
-$dbh->do('DELETE FROM default_circ_rules');
 $dbh->do(
     q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
       VALUES (?, ?, ?, ?)},
@@ -201,19 +199,29 @@ $dbh->do(
 );
 
 # CPL allows only its own patrons to request its items
-$dbh->do(
-    q{INSERT INTO default_branch_circ_rules (branchcode, holdallowed, returnbranch)
-      VALUES (?, ?, ?)},
-    {},
-    $branch_1, 1, 'homebranch',
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $branch_1,
+        categorycode => undef,
+        itemtype     => undef,
+        rules        => {
+            holdallowed  => 1,
+            returnbranch => 'homebranch',
+        }
+    }
 );
 
 # ... while FPL allows anybody to request its items
-$dbh->do(
-    q{INSERT INTO default_branch_circ_rules (branchcode, holdallowed, returnbranch)
-      VALUES (?, ?, ?)},
-    {},
-    $branch_2, 2, 'homebranch',
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $branch_2,
+        categorycode => undef,
+        itemtype     => undef,
+        rules        => {
+            holdallowed  => 2,
+            returnbranch => 'homebranch',
+        }
+    }
 );
 
 my $bibnum2 = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber;
-- 
2.39.5