From ea02516a7c7a2de542bf25e6fd3b7e25eb498417 Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Mon, 27 Nov 2017 15:31:35 +1300 Subject: [PATCH] Bug 7046: Implemented subscription renewal dropdown sub length element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To make this work I moved the _get_sub_length function from subscription-add.pl to C4/Serials.pm so that the subscription-renew.pl script could also call it to store the sublength for the appropriate field of the subscriptions database table. Test plan: 1. Create a subscription and notice that there is a dropdown box for sub length containing the values: issues, weeks, months 2. Renew the subscription and notice that there are 3 input text boxes: 'number of num', 'number of weeks' and 'number of months' 3. Input a 'Number of weeks' value of 2 4. Query the subscription database table and notice that the value of 2 has been stored in the weeklength field for the subscription record you just renewed 5. Apply the patch 6. Renew the subscription and notice that there is now a sublength dropdown box containing issues, weeks and months 7. Set the month value to 3 8. Query the database and notice that 3 was stored in the monthlength field for the subscription record 9. Create a new subscription and select the sub length values of issues and 3 10. Query the database and notice that the numberlength field for the subscription you just created is set to 3 showing that the sublength dropbox is still working for creating a new subscription Sponsored-By: Catalyst IT Patch applies and functions as expected in line with test plan. Signed-off-by: Dilan Johnpullé Signed-off-by: David Nind Bug 7046: (follow-up) Tidy up to address test feedback * Removed underscores from start of subroutine * Removed tabs in subscription-renew.tt * Added test for C4/Serials/getsublength() in t/db_dependen/Serials/ReNewSubscription.t * Added missing filters Sponsored-By: Catalyst IT Signed-off-by: David Nind Bug 7046: (follow-up) Changed subroutine name and added unit tests * Fixed uninitialised value in $subtype in C4/Serials.pm line 1468 * Fixed sub routine name to 'GetSubscriptionLength' * Removed sublength param from being in value of sublength HTML input field * Added more unit tests to include bad params: undef, letters and special characters Test plan: Please follow test plan in first patch Sponsored-By: Catalyst IT Signed-off-by: David Nind Bug 7046: (follow-up) Update function call in subscription-add.pl Also removed border around Subscription length in subscription-renew.tt Test plan: Please follow test plan in first patch Sponsored-By: Catalyst IT Signed-off-by: David Nind Signed-off-by: Jonathan Druart --- C4/Serials.pm | 25 +++++++- .../en/modules/serials/subscription-renew.tt | 63 ++++++++++++++----- serials/subscription-add.pl | 16 +---- serials/subscription-renew.pl | 12 +++- t/db_dependent/Serials/ReNewSubscription.t | 28 ++++++++- 5 files changed, 109 insertions(+), 35 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index f56309e98c..3315959320 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -36,6 +36,7 @@ use Koha::Serial; use Koha::Subscriptions; use Koha::Subscription::Histories; use Koha::SharedContent; +use Scalar::Util qw( looks_like_number ); use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); @@ -74,7 +75,7 @@ BEGIN { &GetNextSeq &GetSeq &NewIssue &GetSerials &GetLatestSerials &ModSerialStatus &GetNextDate &GetSerials2 - &ReNewSubscription &GetLateOrMissingIssues + &GetSubscriptionLength &ReNewSubscription &GetLateOrMissingIssues &GetSerialInformation &AddItem2Serial &PrepareSerialsData &GetNextExpected &ModNextExpected &GetPreviousSerialid @@ -1483,6 +1484,28 @@ sub NewSubscription { return $subscriptionid; } +=head2 GetSubscriptionLength + +my ($numberlength, $weeklength, $monthlength) = GetSubscriptionLength( $subtype, $sublength ); + +This function calculates the subscription length. + +=cut + +sub GetSubscriptionLength { + my ($subtype, $length) = @_; + + return unless looks_like_number($length); + + return + ( + $subtype eq 'issues' ? $length : 0, + $subtype eq 'weeks' ? $length : 0, + $subtype eq 'months' ? $length : 0, + ); +} + + =head2 ReNewSubscription ReNewSubscription($params); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt index 603a0cd5c7..614e4d5e41 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt @@ -35,26 +35,55 @@
[% INCLUDE 'date-format.inc' %]
-
  • - Subscription length: -
    1. -
    2. -
  • +
  • + + + (enter amount in numerals) + +
  • -
  • - + [% UNLESS ( Independentbranches ) %] + + [% END %] + [% IF CAN_user_serials_superserials %] + [% FOREACH library IN libraries %] + + [% END %] [% END %] - [% END %] - (select a library) -
  • + (select a library) + -
  • +
  • diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl index a7b483be18..6fdc1b29d1 100755 --- a/serials/subscription-add.pl +++ b/serials/subscription-add.pl @@ -265,16 +265,6 @@ sub get_letter_loop { ]; } -sub _get_sub_length { - my ($type, $length) = @_; - return - ( - $type eq 'issues' ? $length : 0, - $type eq 'weeks' ? $length : 0, - $type eq 'months' ? $length : 0, - ); -} - sub _guess_enddate { my ($startdate_iso, $frequencyid, $numberlength, $weeklength, $monthlength) = @_; my ($year, $month, $day); @@ -330,7 +320,7 @@ sub redirect_add_subscription { my $subtype = $query->param('subtype'); my $sublength = $query->param('sublength'); my ( $numberlength, $weeklength, $monthlength ) - = _get_sub_length( $subtype, $sublength ); + = GetSubscriptionLength( $subtype, $sublength ); my $add1 = $query->param('add1'); my $lastvalue1 = $query->param('lastvalue1'); my $innerloop1 = $query->param('innerloop1'); @@ -443,8 +433,8 @@ sub redirect_mod_subscription { my $subtype = $query->param('subtype'); my $sublength = $query->param('sublength'); - my ($numberlength, $weeklength, $monthlength) - = _get_sub_length( $subtype, $sublength ); + my ($numberlength, $weeklength, $monthlength) = GetSubscriptionLength( $subtype, $sublength ); + my $numberpattern = $query->param('numbering_pattern'); my $locale = $query->param('locale'); my $lastvalue1 = $query->param('lastvalue1'); my $innerloop1 = $query->param('innerloop1'); diff --git a/serials/subscription-renew.pl b/serials/subscription-renew.pl index 6fc72984f8..954b007fac 100755 --- a/serials/subscription-renew.pl +++ b/serials/subscription-renew.pl @@ -62,6 +62,10 @@ my $mode = $query->param('mode') || q{}; my $op = $query->param('op') || 'display'; my @subscriptionids = $query->multi_param('subscriptionid'); my $branchcode = $query->param('branchcode'); +my $sublength = $query->param('sublength'); +my $subtype = $query->param('subtype'); +my ($numberlength, $weeklength, $monthlength); + my $done = 0; # for after form has been submitted my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { @@ -80,14 +84,15 @@ if ( $op eq "renew" ) { my $subscription = GetSubscription( $subscriptionid ); output_and_exit( $query, $cookie, $template, 'unknown_subscription') unless $subscription; my $startdate = output_pref( { str => scalar $query->param('startdate'), dateonly => 1, dateformat => 'iso' } ); + ($numberlength, $weeklength, $monthlength) = GetSubscriptionLength( $subtype, $sublength ); ReNewSubscription( { subscriptionid => $subscriptionid, user => $loggedinuser, startdate => $startdate, - numberlength => scalar $query->param('numberlength'), - weeklength => scalar $query->param('weeklength'), - monthlength => scalar $query->param('monthlength'), + numberlength => $numberlength, + weeklength => $weeklength, + monthlength => $monthlength, note => scalar $query->param('note'), branchcode => $branchcode } @@ -130,6 +135,7 @@ my $libraries = Koha::Libraries->search( {}, { order_by => ['branchcode'] }, ); $template->param( op => $op, libraries => $libraries, + subtypes => [ qw( numberlength weeklength monthlength ) ], ); output_html_with_http_headers $query, $cookie, $template->output; diff --git a/t/db_dependent/Serials/ReNewSubscription.t b/t/db_dependent/Serials/ReNewSubscription.t index 73affa201c..c8e4a60bb4 100644 --- a/t/db_dependent/Serials/ReNewSubscription.t +++ b/t/db_dependent/Serials/ReNewSubscription.t @@ -3,6 +3,7 @@ # This script includes tests for ReNewSubscription # Copyright 2015 BibLibre, Paul Poulain +# Copyright 2018 Catalyst IT, Alex Buckley # # This file is part of Koha. # @@ -21,7 +22,7 @@ use Modern::Perl; -use Test::More tests => 1; +use Test::More tests => 7; use Test::MockModule; use t::lib::TestBuilder; use t::lib::Mocks; @@ -93,6 +94,31 @@ ReNewSubscription( monthlength => 12 } ); + +# Calculate the subscription length for the renewal for issues, days and months +my ($numberlength, $weeklength, $monthlength) = GetSubscriptionLength('issues', 7); +is ( $numberlength, 7, "Subscription length is 7 issues"); + +($numberlength, $weeklength, $monthlength) = GetSubscriptionLength('weeks', 7); +is ( $weeklength, 7, "Subscription length is 7 weeks"); + +($numberlength, $weeklength, $monthlength) = GetSubscriptionLength('months', 7); +is ( $monthlength, 7, "Subscription length is 7 months"); + +# Check subscription length when no value is inputted into the numeric sublength field +($numberlength, $weeklength, $monthlength) = GetSubscriptionLength('months', ''); +is ($monthlength, undef, "Subscription length is undef months, invalid month data was not stored"); + +# Check subscription length when a letter is inputted into the numeric sublength field +($numberlength, $weeklength, $monthlength) = GetSubscriptionLength('issues', 'w'); +is ($monthlength, undef, "Subscription length is undef issues, invalid issue data was not stored"); + +# Check subscription length when a special character is inputted into numberic sublength field +($numberlength, $weeklength, $monthlength) = GetSubscriptionLength('weeks', '!'); +is ($weeklength, undef, "Subscription length is undef weeks, invalid weeks data was not stored"); + +# Renew the subscription and check that enddate has not been set + my $history = Koha::Subscription::Histories->find($subscription->{subscriptionid}); is ( $history->histenddate(), undef, 'subscription history not empty after renewal'); -- 2.20.1