From 5ac673104fd178805784aa187732eccae6aec1fb Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Thu, 2 Apr 2020 05:15:00 +0000 Subject: [PATCH] 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 --- C4/Serials.pm | 12 ++++++---- .../prog/en/modules/serials/subscription-renew.tt | 3 ++- serials/subscription-add.pl | 2 +- serials/subscription-renew.pl | 9 ++++---- t/db_dependent/Serials/ReNewSubscription.t | 27 +++++++++++++++------- 5 files changed, 35 insertions(+), 18 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index 320b8e78c8..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 - &getsublength &ReNewSubscription &GetLateOrMissingIssues + &GetSubscriptionLength &ReNewSubscription &GetLateOrMissingIssues &GetSerialInformation &AddItem2Serial &PrepareSerialsData &GetNextExpected &ModNextExpected &GetPreviousSerialid @@ -1483,16 +1484,19 @@ sub NewSubscription { return $subscriptionid; } -=head2 getsublength +=head2 GetSubscriptionLength -my ($numberlength, $weeklength, $monthlength) = getsublength( $subtype, $sublength ); +my ($numberlength, $weeklength, $monthlength) = GetSubscriptionLength( $subtype, $sublength ); This function calculates the subscription length. =cut -sub getsublength { +sub GetSubscriptionLength { my ($subtype, $length) = @_; + + return unless looks_like_number($length); + return ( $subtype eq 'issues' ? $length : 0, 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 d78b6f6ecf..977fcc40ca 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 @@ -66,8 +66,9 @@ [% END %] - (enter amount in numerals) + (enter amount in numerals) +
  • diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl index b09e415a56..a26240441e 100755 --- a/serials/subscription-add.pl +++ b/serials/subscription-add.pl @@ -433,7 +433,7 @@ sub redirect_mod_subscription { my $subtype = $query->param('subtype'); my $sublength = $query->param('sublength'); - my ($numberlength, $weeklength, $monthlength) = C4::Serials->getsublength( $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'); diff --git a/serials/subscription-renew.pl b/serials/subscription-renew.pl index b7140b518f..954b007fac 100755 --- a/serials/subscription-renew.pl +++ b/serials/subscription-renew.pl @@ -64,7 +64,7 @@ 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) = C4::Serials->getsublength( $subtype, $sublength ); +my ($numberlength, $weeklength, $monthlength); my $done = 0; # for after form has been submitted my ( $template, $loggedinuser, $cookie ) = get_template_and_user( @@ -84,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 } diff --git a/t/db_dependent/Serials/ReNewSubscription.t b/t/db_dependent/Serials/ReNewSubscription.t index 7e1ca8158d..c8e4a60bb4 100644 --- a/t/db_dependent/Serials/ReNewSubscription.t +++ b/t/db_dependent/Serials/ReNewSubscription.t @@ -22,7 +22,7 @@ use Modern::Perl; -use Test::More tests => 4; +use Test::More tests => 7; use Test::MockModule; use t::lib::TestBuilder; use t::lib::Mocks; @@ -96,18 +96,29 @@ ReNewSubscription( ); # Calculate the subscription length for the renewal for issues, days and months -my ($numberlength, $weeklength, $monthlength) = getsublength('issues', 7); -is ( $numberlength, 7, "Sub length is 7 issues"); +my ($numberlength, $weeklength, $monthlength) = GetSubscriptionLength('issues', 7); +is ( $numberlength, 7, "Subscription length is 7 issues"); -($numberlength, $weeklength, $monthlength) = getsublength('weeks', 7); -is ( $weeklength, 7, "Sub length is 7 weeks"); +($numberlength, $weeklength, $monthlength) = GetSubscriptionLength('weeks', 7); +is ( $weeklength, 7, "Subscription length is 7 weeks"); -($numberlength, $weeklength, $monthlength) = getsublength('months', 7); -is ( $monthlength, 7, "Sub length is 7 months"); +($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 -ReNewSubscription($subscription->{subscriptionid},'',"2016-01-01",'','',12,''); my $history = Koha::Subscription::Histories->find($subscription->{subscriptionid}); is ( $history->histenddate(), undef, 'subscription history not empty after renewal'); -- 2.11.0