Bug 7046: Implemented subscription renewal dropdown sub length element

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
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/Serials.pm b/C4/Serials.pm
index f56309e..3315959 100644 (file)
--- 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 603a0cd..614e4d5 100644 (file)
--- 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 @@
                 <input type="text" size="10" id="startdate" name="startdate" value="[% startdate | html %]" class="datepicker"/>
                 <div class="hint">[% INCLUDE 'date-format.inc' %]</div>
             </li>
-               <li><fieldset>
-               <legend>Subscription length:</legend>
-               <ol><li><label for="numberlength">Number of num:</label><input type="text" id="numberlength" name="numberlength" value="[% subscription.numberlength | html %]" /></li>
-               <li><label for="weeklength">Number of weeks: </label><input type="text" id="weeklength" name="weeklength" value="[% subscription.weeklength | html %]" /></li>
-               <li><label for="monthlength">Number of months: </label><input type="text" id="monthlength" name="monthlength" value="[% subscription.monthlength | html %]" /></li></ol></fieldset></li>
+    <li>
+    <label>Subscription length:</label>
+    <select name="subtype" id="subtype">
+        [% FOREACH st IN subtypes %]
+            [% SWITCH st %]
+                [% CASE 'numberlength'%]
+                    [% IF st == subtype %]
+                        <option value="issues" selected="selected">
+                    [% ELSE %]
+                        <option value="issues">
+                    [% END %]
+                    issues
+                [% CASE 'weeklength' %]
+                    [% IF st == subtype %]
+                        <option value="weeks" selected="selected">
+                    [% ELSE %]
+                        <option value="weeks">
+                    [% END %]
+                    weeks
+                [% CASE 'monthlength' %]
+                    [% IF st == subtype %]
+                        <option value="months" selected="selected">
+                    [% ELSE %]
+                        <option value="months">
+                    [% END %]
+                    months
+                [% CASE %][% st | html %]
+            [% END %]
+            </option>
+        [% END %]
+    </select>
+    <input type="text" name="sublength" id="sublength" size="3" />(enter amount in numerals)
+    <input type="hidden" name="issuelengthcount">
+    </li>
 
-<li><label for="branchcode">Library:</label>
- <select name="branchcode" id="branchcode" style="width: 20em;">
-     [% UNLESS ( Independentbranches ) %]
-        <option value="">None</option>
-     [% END %]
-     [% IF CAN_user_serials_superserials %]
-        [% FOREACH library IN libraries %]
-             <option value="[% library.branchcode | html %]"> [% library.branchname | html %] </option>
+    <li><label for="branchcode">Library:</label>
+    <select name="branchcode" id="branchcode" style="width: 20em;">
+        [% UNLESS ( Independentbranches ) %]
+            <option value="">None</option>
+        [% END %]
+        [% IF CAN_user_serials_superserials %]
+            [% FOREACH library IN libraries %]
+                <option value="[% library.branchcode | html %]"> [% library.branchname | html %] </option>
+            [% END %]
         [% END %]
-     [% END %]
-</select> (select a library)
-</li>
+    </select> (select a library)
+    </li>
 
-               <li><label for="note">Note for the librarian that will manage your renewal request: </label>
+    <li><label for="note">Note for the librarian that will manage your renewal request: </label>
                <textarea name="note" id="note" rows="5" cols="50"></textarea></li></ol></fieldset>
                <fieldset class="action"><input type="submit" value="Submit" class="button" /></fieldset>
 </form>

diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl
index a7b483b..6fdc1b2 100755 (executable)
--- 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 6fc7298..954b007 100755 (executable)
--- 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 73affa2..c8e4a60 100644 (file)
--- 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');