Attachment 58324 Details for Bug 14866 – [FOLLOW-UP] Bug 14866: Make high holds work with different item types

[patch] [FOLLOW-UP] Bug 14866: Make high holds work with different item types

FOLLOW-UP-Bug-14866-Make-high-holds-work-with-diff.patch (text/plain), 17.39 KB, created by Aleisha Amohia on 2016-12-21 00:31:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Aleisha Amohia

Created: 2016-12-21 00:31:13 UTC

Size: 17.39 KB

patch

obsolete

>From e5dbc4ba1f59f5dd8a5553a1c355343e561ba9bb Mon Sep 17 00:00:00 2001
>From: Aleisha Amohia <aleishaamohia@hotmail.com>
>Date: Wed, 21 Dec 2016 00:25:50 +0000
>Subject: [PATCH] [FOLLOW-UP] Bug 14866: Make high holds work with different
> item types
>
>This patch fixes merge conflicts, fixes the indentation of the table in
>smart-rules.tt and uses Koha::IssuingRules->get_effective_issuing_rule.
>
>Keep in mind - I am sure that when the original patch for this was
>written (before get_effective_issuing_rule was used), it all worked
>fine. I had to add an extra line of code in my patch to get this method
>to work because it was fetching the WRONG circulation rule - one that
>did not have decreaseloanholds defined. I don't know if it was my patch
>that is broken or the get_effective_issuing_rule method that is not
>fetching the 'most specific/relevant' rule.
>
>Sponsored-by: Region Halland
>---
> C4/Circulation.pm                                  |  14 +-
> Koha/Schema/Result/Issuingrule.pm                  |  11 +-
> admin/smart-rules.pl                               |   2 +-
> .../bug_14866-add_decreaseloanholds.sql            |   2 +-
> installer/data/mysql/kohastructure.sql             |  11 +-
> .../prog/en/modules/admin/smart-rules.tt           | 157 ++++++++-------------
> 6 files changed, 85 insertions(+), 112 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index ca7dc01..db7347c 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -22,7 +22,6 @@ package C4::Circulation;
> use strict;
> #use warnings; FIXME - Bug 2505
> use DateTime;
>-use Koha::DateUtils;
> use C4::Context;
> use C4::Stats;
> use C4::Reserves;
>@@ -40,6 +39,7 @@ use Algorithm::CheckDigits;
> 
> use Data::Dumper;
> use Koha::Account;
>+use Koha::DateUtils;
> use Koha::AuthorisedValues;
> use Koha::DateUtils;
> use Koha::Calendar;
>@@ -1055,14 +1055,20 @@ sub CanBookBeIssued {
>         # Decrease loan period for item if rule exist in smart-rules.pl
>         # Overrides decreaseLoanHighHoldsValue syspref
>         my $branch = _GetCircControlBranch($item,$borrower);
>-        my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branch);
>-        my $issuedate = DateTime->now( time_zone => C4::Context->tz() );
>+        my $issuingrule = Koha::IssuingRules->get_effective_issuing_rule(
>+            {   categorycode => $borrower->{categorycode},
>+                itemtype     => $item->{itype},
>+                branchcode   => $branch
>+            }
>+        );
>+        my $issuedate = dt_from_string();
>         my $calendar = Koha::Calendar->new( branchcode => $branch );
>+        $issuingrule->{decreaseloanholds} = 0 if not defined $issuingrule->{decreaseloanholds};
>         my $reduced_datedue = $calendar->addDate( $issuedate, $issuingrule->{decreaseloanholds} );
> 
>         if ( defined($issuingrule->{decreaseloanholds}) ) {
>             $needsconfirmation{HIGHHOLDS} = {
>-                num_holds  => $num,
>+                num_holds  => $issuingrule->{outstanding},
>                 duration   => $issuingrule->{decreaseloanholds},
>                 returndate => output_pref($reduced_datedue),
>             }
>diff --git a/Koha/Schema/Result/Issuingrule.pm b/Koha/Schema/Result/Issuingrule.pm
>index 62f35f2..9e5c6bf 100644
>--- a/Koha/Schema/Result/Issuingrule.pm
>+++ b/Koha/Schema/Result/Issuingrule.pm
>@@ -209,6 +209,11 @@ __PACKAGE__->table("issuingrules");
>   extra: {list => ["no","yes","bib_only","item_only"]}
>   is_nullable: 0
> 
>+=head2 decreaseloanholds
>+
>+  data_type: 'integer'
>+  is_nullable: 1
>+
> =cut
> 
> __PACKAGE__->add_columns(
>@@ -286,6 +291,8 @@ __PACKAGE__->add_columns(
>     extra => { list => ["no", "yes", "bib_only", "item_only"] },
>     is_nullable => 0,
>   },
>+  "decreaseloanholds",
>+  { data_type => "integer", is_nullable => 1 },
> );
> 
> =head1 PRIMARY KEY
>@@ -305,8 +312,8 @@ __PACKAGE__->add_columns(
> __PACKAGE__->set_primary_key("branchcode", "categorycode", "itemtype");
> 
> 
>-# Created by DBIx::Class::Schema::Loader v0.07042 @ 2016-11-02 10:33:37
>-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:t9AhZLbm4SE7mx25LAyhGA
>+# Created by DBIx::Class::Schema::Loader v0.07042 @ 2016-12-20 01:08:00
>+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:2kV+hezVsq+y+4OcEVg2ig
> 
> 
> # You can replace this text with custom code or comments, and it will be preserved on regeneration
>diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
>index 6d9c319..d428d4a 100755
>--- a/admin/smart-rules.pl
>+++ b/admin/smart-rules.pl
>@@ -157,7 +157,7 @@ elsif ($op eq 'add') {
>     my $article_requests = $input->param('article_requests') || 'no';
>     my $overduefinescap = $input->param('overduefinescap') || undef;
>     my $cap_fine_to_replacement_price = $input->param('cap_fine_to_replacement_price') eq 'on';
>-    my $decreaseloanholds = $input->param('decreaseloanholds');
>+    my $decreaseloanholds = $input->param('decreaseloanholds') || undef;
>     $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price";
> 
>     my $params = {
>diff --git a/installer/data/mysql/atomicupdate/bug_14866-add_decreaseloanholds.sql b/installer/data/mysql/atomicupdate/bug_14866-add_decreaseloanholds.sql
>index c041a1c..f38c199 100644
>--- a/installer/data/mysql/atomicupdate/bug_14866-add_decreaseloanholds.sql
>+++ b/installer/data/mysql/atomicupdate/bug_14866-add_decreaseloanholds.sql
>@@ -1 +1 @@
>-ALTER TABLE issuingrules ADD decreaseloanholds INTEGER;
>+ALTER IGNORE TABLE issuingrules ADD decreaseloanholds INTEGER(11) DEFAULT NULL AFTER `article_requests`;
>diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
>index 514c13d..55edc34 100644
>--- a/installer/data/mysql/kohastructure.sql
>+++ b/installer/data/mysql/kohastructure.sql
>@@ -873,11 +873,12 @@ CREATE TABLE `issuingrules` ( -- circulation and fine rules
>   `reservesallowed` smallint(6) NOT NULL default "0", -- how many holds are allowed
>   `holds_per_record` SMALLINT(6) NOT NULL DEFAULT 1, -- How many holds a patron can have on a given bib
>   `branchcode` varchar(10) NOT NULL default '', -- the branch this rule is for (branches.branchcode)
>-  overduefinescap decimal(28,6) default NULL, -- the maximum amount of an overdue fine
>-  cap_fine_to_replacement_price BOOLEAN NOT NULL DEFAULT  '0', -- cap the fine based on item's replacement price
>-  onshelfholds tinyint(1) NOT NULL default 0, -- allow holds for items that are on shelf
>-  opacitemholds char(1) NOT NULL default 'N', -- allow opac users to place specific items on hold
>-  article_requests enum('no','yes','bib_only','item_only') NOT NULL DEFAULT 'no', -- allow article requests to be placed,
>+  `overduefinescap` decimal(28,6) default NULL, -- the maximum amount of an overdue fine
>+  `cap_fine_to_replacement_price` BOOLEAN NOT NULL DEFAULT  '0', -- cap the fine based on item's replacement price
>+  `onshelfholds` tinyint(1) NOT NULL default 0, -- allow holds for items that are on shelf
>+  `opacitemholds` char(1) NOT NULL default 'N', -- allow opac users to place specific items on hold
>+  `article_requests` enum('no','yes','bib_only','item_only') NOT NULL DEFAULT 'no', -- allow article requests to be placed
>+  `decreaseloanholds` int(11) default NULL,
>   PRIMARY KEY  (`branchcode`,`categorycode`,`itemtype`),
>   KEY `categorycode` (`categorycode`),
>   KEY `itemtype` (`itemtype`)
>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 9ca6a3e..6e80827 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
>@@ -197,105 +197,64 @@ $(document).ready(function() {
>             </tr>
>             </thead>
>             <tbody>
>-				[% FOREACH rule IN rules %]
>-					<tr id="row_[% loop.count %]">
>-							<td>[% IF ( rule.default_humancategorycode ) %]
>-									<em>All</em>
>-								[% ELSE %]
>-									[% rule.humancategorycode %]
>-								[% END %]
>-							</td>
>-                            <td>[% IF rule.default_translated_description %]
>-									<em>All</em>
>-								[% ELSE %]
>-									[% rule.translated_description %]
>-								[% END %]
>-							</td>
>-							<td>[% IF ( rule.unlimited_maxissueqty ) %]
>-									Unlimited
>-								[% ELSE %]
>-									[% rule.maxissueqty %]
>-								[% END %]
>-							</td>
>-                            <td>[% IF rule.unlimited_maxonsiteissueqty %]
>-                                    Unlimited
>-                                [% ELSE %]
>-                                    [% rule.maxonsiteissueqty %]
>-                                [% END %]
>-                            </td>
>-							<td>[% rule.issuelength %]</td>
>-							<td>
>-							    [% rule.lengthunit %]
>-							</td>
>-                            <td>
>-                              [% IF ( rule.hardduedate ) %]
>-                                [% IF ( rule.hardduedatebefore ) %]
>-                                  before [% rule.hardduedate %]
>-                                  <input type="hidden" name="hardduedatecomparebackup" value="-1" />
>-                                [% ELSIF ( rule.hardduedateexact ) %]
>-                                  on [% rule.hardduedate %]
>-                                  <input type="hidden" name="hardduedatecomparebackup" value="0" />
>-                                [% ELSIF ( rule.hardduedateafter ) %]
>-                                  after [% rule.hardduedate %]
>-                                  <input type="hidden" name="hardduedatecomparebackup" value="1" />
>-                                [% END %]
>-                              [% ELSE %]
>-                                None defined
>-                              [% END %]
>-                            </td>
>-							<td>[% rule.fine %]</td>
>-							<td>[% rule.chargeperiod %]</td>
>-                <td>[% IF rule.chargeperiod_charge_at %]Start of interval[% ELSE %]End of interval[% END %]</td>
>-							<td>[% rule.firstremind %]</td>
>-                            <td>[% rule.overduefinescap FILTER format("%.2f") %]</td>
>-                            <td>
>-                                [% IF rule.cap_fine_to_replacement_price %]
>-                                    <input type="checkbox" checked="checked" disabled="disabled" />
>-                                [% ELSE %]
>-                                    <input type="checkbox" disabled="disabled" />
>-                                [% END %]
>-                            </td>
>-							<td>[% rule.finedays %]</td>
>-                            <td>[% rule.maxsuspensiondays %]</td>
>-							<td>[% rule.renewalsallowed %]</td>
>-                            <td>[% rule.renewalperiod %]</td>
>-                            <td>[% rule.norenewalbefore %]</td>
>-                            <td>
>-                                [% IF ( rule.auto_renew ) %]
>-                                Yes
>-                                [% ELSE %]
>-                                No
>-                                [% END %]
>-                            </td>
>-                            <td>[% rule.no_auto_renewal_after %]</td>
>-							<td>[% rule.reservesallowed %]</td>
>-                                                        <td>[% rule.holds_per_record %]</td>
>-                                                        <td>
>-                                                            [% IF rule.onshelfholds == 1 %]
>-                                                                Yes
>-                                                            [% ELSIF rule.onshelfholds == 2 %]
>-                                                                If all unavailable
>-                                                            [% ELSE %]
>-                                                                If any unavailable
>-                                                            [% END %]</td>
>-                                                        <td>[% IF rule.opacitemholds == 'F'%]Force[% ELSIF rule.opacitemholds == 'Y'%]Allow[% ELSE %]Don't allow[% END %]</td>
>-                                                        <td>
>-                                                            [% IF rule.article_requests == 'no' %]
>-                                                                No
>-                                                            [% ELSIF rule.article_requests == 'yes' %]
>-                                                                Yes
>-                                                            [% ELSIF rule.article_requests == 'bib_only' %]
>-                                                                Record only
>-                                                            [% ELSIF rule.article_requests == 'item_only' %]
>-                                                                Item only
>-                                                            [% END %]
>-                                                        </td>
>-                                                        <td>[% rule.decreaseloanholds %]</td>
>-                                                        <td>[% rule.rentaldiscount %]</td>
>-                                                        <td class="actions">
>-                                                          <a href="#" class="editrule btn btn-mini"><i class="fa fa-pencil"></i> Edit</a>
>-                                                          <a class="btn btn-mini delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&amp;itemtype=[% rule.itemtype %]&amp;categorycode=[% rule.categorycode %]&amp;branch=[% rule.current_branch %]"><i class="fa fa-trash"></i> Delete</a></td>
>-            </tr>
>+            [% FOREACH rule IN rules %]
>+                <tr id="row_[% loop.count %]">
>+                    <td>[% IF ( rule.default_humancategorycode ) %]<em>All</em>[% ELSE %][% rule.humancategorycode %][% END %]</td>
>+                    <td>[% IF rule.default_translated_description %]<em>All</em>[% ELSE %][% rule.translated_description %][% END %]</td>
>+                    <td>[% IF ( rule.unlimited_maxissueqty ) %]Unlimited[% ELSE %][% rule.maxissueqty %][% END %]</td>
>+                    <td>[% IF rule.unlimited_maxonsiteissueqty %]Unlimited[% ELSE %][% rule.maxonsiteissueqty %][% END %]</td>
>+                    <td>[% rule.issuelength %]</td>
>+                    <td>[% rule.lengthunit %]</td>
>+                    <td>[% IF ( rule.hardduedate ) %]
>+                        [% IF ( rule.hardduedatebefore ) %]
>+                            before [% rule.hardduedate %] <input type="hidden" name="hardduedatecomparebackup" value="-1" />
>+                        [% ELSIF ( rule.hardduedateexact ) %]
>+                            on [% rule.hardduedate %] <input type="hidden" name="hardduedatecomparebackup" value="0" />
>+                        [% ELSIF ( rule.hardduedateafter ) %]
>+                            after [% rule.hardduedate %] <input type="hidden" name="hardduedatecomparebackup" value="1" />
>+                        [% END %]
>+                    [% ELSE %]
>+                        None defined
>+                    [% END %]</td>
>+                    <td>[% rule.fine %]</td>
>+                    <td>[% rule.chargeperiod %]</td>
>+                    <td>[% IF rule.chargeperiod_charge_at %]Start of interval[% ELSE %]End of interval[% END %]</td>
>+                    <td>[% rule.firstremind %]</td>
>+                    <td>[% rule.overduefinescap FILTER format("%.2f") %]</td>
>+                    <td>[% IF rule.cap_fine_to_replacement_price %]<input type="checkbox" checked="checked" disabled="disabled" />[% ELSE %]<input type="checkbox" disabled="disabled" />[% END %]</td>
>+                    <td>[% rule.finedays %]</td>
>+                    <td>[% rule.maxsuspensiondays %]</td>
>+                    <td>[% rule.renewalsallowed %]</td>
>+                    <td>[% rule.renewalperiod %]</td>
>+                    <td>[% rule.norenewalbefore %]</td>
>+                    <td>[% IF ( rule.auto_renew ) %]Yes[% ELSE %]No[% END %]</td>
>+                    <td>[% rule.no_auto_renewal_after %]</td>
>+                    <td>[% rule.reservesallowed %]</td>
>+                    <td>[% rule.holds_per_record %]</td>
>+                    <td>[% IF rule.onshelfholds == 1 %]
>+                        Yes
>+                    [% ELSIF rule.onshelfholds == 2 %]
>+                        If all unavailable
>+                    [% ELSE %]
>+                        If any unavailable
>+                    [% END %]</td>
>+                    <td>[% IF rule.opacitemholds == 'F'%]Force[% ELSIF rule.opacitemholds == 'Y'%]Allow[% ELSE %]Don't allow[% END %]</td>
>+                    <td>[% IF rule.article_requests == 'no' %]
>+                        No
>+                    [% ELSIF rule.article_requests == 'yes' %]
>+                        Yes
>+                    [% ELSIF rule.article_requests == 'bib_only' %]
>+                        Record only
>+                    [% ELSIF rule.article_requests == 'item_only' %]
>+                        Item only
>+                    [% END %]</td>
>+                    <td>[% rule.decreaseloanholds %]</td>
>+                    <td>[% rule.rentaldiscount %]</td>
>+                    <td class="actions">
>+                        <a href="#" class="editrule btn btn-mini"><i class="fa fa-pencil"></i> Edit</a>
>+                        <a class="btn btn-mini delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&amp;itemtype=[% rule.itemtype %]&amp;categorycode=[% rule.categorycode %]&amp;branch=[% rule.current_branch %]"><i class="fa fa-trash"></i> Delete</a>
>+                    </td>
>+                </tr>
>             [% END %]
>                 <tr id="edit_row">
>                     <td>
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 14866: 42751 | 58324 | 75861 | 75862 | 75863 | 103323 | 103427 | 110149 | 110224 | 110441 | 111937 | 111938 | 112225 | 112226 | 112936