Bugzilla – Attachment 102429 Details for
Bug 7046
subscription renewal period should be a pull down
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 7046: (follow-up) Changed subroutine name and added unit tests
Bug-7046-follow-up-Changed-subroutine-name-and-add.patch (text/plain), 7.88 KB, created by
David Nind
on 2020-04-06 00:06:06 UTC
(
hide
)
Description:
Bug 7046: (follow-up) Changed subroutine name and added unit tests
Filename:
MIME Type:
Creator:
David Nind
Created:
2020-04-06 00:06:06 UTC
Size:
7.88 KB
patch
obsolete
>From ce3f39ba0ea47fe358a7983a9cfc650de83e43fa Mon Sep 17 00:00:00 2001 >From: Alex Buckley <alexbuckley@catalyst.net.nz> >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 > >Signed-off-by: David Nind <david@davidnind.com> >--- > 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 @@ > </option> > [% END %] > </select> >- <input type="text" name="sublength" id="sublength" value="[% sublength | html %]" size="3" />(enter amount in numerals) >+ <input type="text" name="sublength" id="sublength" size="3" />(enter amount in numerals) > <input type="hidden" name="issuelengthcount"> >+ </fieldset> > </li> > > <li><label for="branchcode">Library:</label> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 7046
:
5938
|
5939
|
69355
|
69356
|
69408
|
81641
|
81642
|
102275
|
102276
|
102278
|
102425
|
102427
|
102428
|
102429
|
102430
|
102535
|
102536
|
102537
|
102538
|
102564