From e5dbc4ba1f59f5dd8a5553a1c355343e561ba9bb Mon Sep 17 00:00:00 2001 From: Aleisha Amohia 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() { - [% FOREACH rule IN rules %] - - [% IF ( rule.default_humancategorycode ) %] - All - [% ELSE %] - [% rule.humancategorycode %] - [% END %] - - [% IF rule.default_translated_description %] - All - [% ELSE %] - [% rule.translated_description %] - [% END %] - - [% IF ( rule.unlimited_maxissueqty ) %] - Unlimited - [% ELSE %] - [% rule.maxissueqty %] - [% END %] - - [% IF rule.unlimited_maxonsiteissueqty %] - Unlimited - [% ELSE %] - [% rule.maxonsiteissueqty %] - [% END %] - - [% rule.issuelength %] - - [% rule.lengthunit %] - - - [% IF ( rule.hardduedate ) %] - [% IF ( rule.hardduedatebefore ) %] - before [% rule.hardduedate %] - - [% ELSIF ( rule.hardduedateexact ) %] - on [% rule.hardduedate %] - - [% ELSIF ( rule.hardduedateafter ) %] - after [% rule.hardduedate %] - - [% END %] - [% ELSE %] - None defined - [% END %] - - [% rule.fine %] - [% rule.chargeperiod %] - [% IF rule.chargeperiod_charge_at %]Start of interval[% ELSE %]End of interval[% END %] - [% rule.firstremind %] - [% rule.overduefinescap FILTER format("%.2f") %] - - [% IF rule.cap_fine_to_replacement_price %] - - [% ELSE %] - - [% END %] - - [% rule.finedays %] - [% rule.maxsuspensiondays %] - [% rule.renewalsallowed %] - [% rule.renewalperiod %] - [% rule.norenewalbefore %] - - [% IF ( rule.auto_renew ) %] - Yes - [% ELSE %] - No - [% END %] - - [% rule.no_auto_renewal_after %] - [% rule.reservesallowed %] - [% rule.holds_per_record %] - - [% IF rule.onshelfholds == 1 %] - Yes - [% ELSIF rule.onshelfholds == 2 %] - If all unavailable - [% ELSE %] - If any unavailable - [% END %] - [% IF rule.opacitemholds == 'F'%]Force[% ELSIF rule.opacitemholds == 'Y'%]Allow[% ELSE %]Don't allow[% END %] - - [% 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 %] - - [% rule.decreaseloanholds %] - [% rule.rentaldiscount %] - - Edit - Delete - + [% FOREACH rule IN rules %] + + [% IF ( rule.default_humancategorycode ) %]All[% ELSE %][% rule.humancategorycode %][% END %] + [% IF rule.default_translated_description %]All[% ELSE %][% rule.translated_description %][% END %] + [% IF ( rule.unlimited_maxissueqty ) %]Unlimited[% ELSE %][% rule.maxissueqty %][% END %] + [% IF rule.unlimited_maxonsiteissueqty %]Unlimited[% ELSE %][% rule.maxonsiteissueqty %][% END %] + [% rule.issuelength %] + [% rule.lengthunit %] + [% IF ( rule.hardduedate ) %] + [% IF ( rule.hardduedatebefore ) %] + before [% rule.hardduedate %] + [% ELSIF ( rule.hardduedateexact ) %] + on [% rule.hardduedate %] + [% ELSIF ( rule.hardduedateafter ) %] + after [% rule.hardduedate %] + [% END %] + [% ELSE %] + None defined + [% END %] + [% rule.fine %] + [% rule.chargeperiod %] + [% IF rule.chargeperiod_charge_at %]Start of interval[% ELSE %]End of interval[% END %] + [% rule.firstremind %] + [% rule.overduefinescap FILTER format("%.2f") %] + [% IF rule.cap_fine_to_replacement_price %][% ELSE %][% END %] + [% rule.finedays %] + [% rule.maxsuspensiondays %] + [% rule.renewalsallowed %] + [% rule.renewalperiod %] + [% rule.norenewalbefore %] + [% IF ( rule.auto_renew ) %]Yes[% ELSE %]No[% END %] + [% rule.no_auto_renewal_after %] + [% rule.reservesallowed %] + [% rule.holds_per_record %] + [% IF rule.onshelfholds == 1 %] + Yes + [% ELSIF rule.onshelfholds == 2 %] + If all unavailable + [% ELSE %] + If any unavailable + [% END %] + [% IF rule.opacitemholds == 'F'%]Force[% ELSIF rule.opacitemholds == 'Y'%]Allow[% ELSE %]Don't allow[% END %] + [% 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 %] + [% rule.decreaseloanholds %] + [% rule.rentaldiscount %] + + Edit + Delete + + [% END %] -- 2.1.4