Bugzilla – Attachment 102537 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: Implemented subscription renewal dropdown sub length element
Bug-7046-Implemented-subscription-renewal-dropdown.patch (text/plain), 14.16 KB, created by
Jonathan Druart
on 2020-04-08 09:15:58 UTC
(
hide
)
Description:
Bug 7046: Implemented subscription renewal dropdown sub length element
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-04-08 09:15:58 UTC
Size:
14.16 KB
patch
obsolete
>From ea02516a7c7a2de542bf25e6fd3b7e25eb498417 Mon Sep 17 00:00:00 2001 >From: Alex Buckley <alexbuckley@catalyst.net.nz> >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é <dilan@calyx.net.au> > >Signed-off-by: David Nind <david@davidnind.com> > >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 <david@davidnind.com> > >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> > >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 <david@davidnind.com> > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >--- > 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 @@ > <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 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
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