Bugzilla – Attachment 29049 Details for
Bug 11742
Overdue notice/status triggers displaying the wrong notice for default
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[PASSED QA] Bug 11742: Change return type for GetLetters
PASSED-QA-Bug-11742-Change-return-type-for-GetLett.patch (text/plain), 14.58 KB, created by
Katrin Fischer
on 2014-06-19 15:23:25 UTC
(
hide
)
Description:
[PASSED QA] Bug 11742: Change return type for GetLetters
Filename:
MIME Type:
Creator:
Katrin Fischer
Created:
2014-06-19 15:23:25 UTC
Size:
14.58 KB
patch
obsolete
>From 4a478b8b6c5d91de1e9037e5642c2b974e53f50c Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Wed, 7 May 2014 15:55:54 +0200 >Subject: [PATCH] [PASSED QA] Bug 11742: Change return type for GetLetters > >The GetLetters subroutine should return an arrayref with different >letters for a module. > >Test plan: >0/ Delete your notices with module=claimacquisition, claimissues, >serial >1/ Go on the late orders page (acqui/lateorders.pl) and verify you >cannot choose a notice for claiming >2/ Create a notice with module=claimacquisition >3/ Go on the late orders page (acqui/lateorders.pl) and verify you >can choose the notice for claiming >4/ Go on the Claim serials page (serials/claims.pl) and repeat the same >thing with the a "claimissues" notice >5/ Create a new subscription (serials/subscription-add.pl) and verify >you cannot choose a notification for patrons. >6/ Create a notice with module "serial" and verify you can. > >Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> >Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> >Passes all tests and QA script. Additional tests done: > >- copy notice ODUE, on saving you are now prompted to choose > a new CODE for the notice >- edit new notice, try to set code back to ODUE. You are > prompted that the code is already in use. > >This will prevent people from accidentally overwriting a letter >with the same letter code. >--- > C4/Letters.pm | 62 ++++++---------------- > acqui/lateorders.pl | 8 +-- > .../prog/en/modules/serials/claims.tt | 6 +-- > serials/claims.pl | 12 ++--- > serials/subscription-add.pl | 38 +++++++------ > t/Letters.t | 4 +- > tools/overduerules.pl | 46 +++------------- > 7 files changed, 57 insertions(+), 119 deletions(-) > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index 055c8c1..4e0ea20 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -62,58 +62,30 @@ C4::Letters - Give functions for Letters management > > Letters are managed through "alerts" sent by Koha on some events. All "alert" related functions are in this module too. > >-=head2 GetLetters([$category]) >+=head2 GetLetters([$module]) > >- $letters = &GetLetters($category); >+ $letters = &GetLetters($module); > returns informations about letters. >- if needed, $category filters for letters given category >- Create a letter selector with the following code >- >-=head3 in PERL SCRIPT >- >-my $letters = GetLetters($cat); >-my @letterloop; >-foreach my $thisletter (keys %$letters) { >- my $selected = 1 if $thisletter eq $letter; >- my %row =( >- value => $thisletter, >- selected => $selected, >- lettername => $letters->{$thisletter}, >- ); >- push @letterloop, \%row; >-} >-$template->param(LETTERLOOP => \@letterloop); >- >-=head3 in TEMPLATE >- >- <select name="letter"> >- <option value="">Default</option> >- <!-- TMPL_LOOP name="LETTERLOOP" --> >- <option value="<!-- TMPL_VAR name="value" -->" <!-- TMPL_IF name="selected" -->selected<!-- /TMPL_IF -->><!-- TMPL_VAR name="lettername" --></option> >- <!-- /TMPL_LOOP --> >- </select> >+ if needed, $module filters for letters given module > > =cut > > sub GetLetters { >+ my ($filters) = @_; >+ my $module = $filters->{module}; >+ my $dbh = C4::Context->dbh; >+ my $letters = $dbh->selectall_arrayref( >+ q| >+ SELECT module, code, branchcode, name >+ FROM letter >+ WHERE 1 >+ | >+ . ( $module ? q| AND module = ?| : q|| ) >+ . q| GROUP BY code ORDER BY name|, { Slice => {} } >+ , ( $module ? $module : () ) >+ ); > >- # returns a reference to a hash of references to ALL letters... >- my ( $cat ) = @_; >- my %letters; >- my $dbh = C4::Context->dbh; >- my $sth; >- my $query = q{ >- SELECT * FROM letter WHERE 1 >- }; >- $query .= q{ AND module = ? } if defined $cat; >- $query .= q{ GROUP BY code ORDER BY name}; >- $sth = $dbh->prepare($query); >- $sth->execute((defined $cat ? $cat : ())); >- >- while ( my $letter = $sth->fetchrow_hashref ) { >- $letters{ $letter->{'code'} } = $letter->{'name'}; >- } >- return \%letters; >+ return $letters; > } > > # FIXME: using our here means that a Plack server will need to be >diff --git a/acqui/lateorders.pl b/acqui/lateorders.pl >index 1a64a4c..3c96e11 100755 >--- a/acqui/lateorders.pl >+++ b/acqui/lateorders.pl >@@ -159,17 +159,13 @@ foreach (@lateorders){ > $total += $_->{subtotal}; > } > >-my @letters; >-my $letters=GetLetters("claimacquisition"); >-foreach (keys %$letters){ >- push @letters, {code=>$_,name=>$letters->{$_}}; >-} >-$template->param(letters=>\@letters) if (@letters); >+my $letters = GetLetters({ module => "claimacquisition" }); > > $template->param(ERROR_LOOP => \@errors) if (@errors); > $template->param( > lateorders => \@lateorders, > delay => $delay, >+ letters => $letters, > estimateddeliverydatefrom => $estimateddeliverydatefrom, > estimateddeliverydateto => $estimateddeliverydateto, > total => $total, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt >index 34c6d8c..de9102e 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt >@@ -177,12 +177,12 @@ > > <h1>Claims</h1> > >-[% IF ( letter ) %][% UNLESS ( missingissues ) %][% IF ( supplierid ) %] <div class="dialog alert">No missing issues found.</div>[% ELSE %]<div class="dialog message">Please choose a vendor.</div>[% END %][% END %][% END %] >+[% IF letters %][% UNLESS ( missingissues ) %][% IF ( supplierid ) %] <div class="dialog alert">No missing issues found.</div>[% ELSE %]<div class="dialog message">Please choose a vendor.</div>[% END %][% END %][% END %] > > [% IF ( SHOWCONFIRMATION ) %] > <div class="dialog alert">Your notification has been sent.</div> > [% END %] >-[% UNLESS ( letter ) %]<div class="dialog alert">No claims notice defined. <a href="/cgi-bin/koha/tools/letter.pl">Please define one</a>.</div>[% END %] >+[% UNLESS letters %]<div class="dialog alert">No claims notice defined. <a href="/cgi-bin/koha/tools/letter.pl">Please define one</a>.</div>[% END %] > <form id="claims" name="claims" action="claims.pl" method="post"> > <fieldset> > <label for="supplierid">Vendor: </label> >@@ -331,7 +331,7 @@ > <span class="exportSelected"><a id="ExportSelected" href="/cgi-bin/koha/serials/claims.pl">Download selected claims</a></span> > [% END %] > >-[% IF ( letter ) %] >+[% IF ( letters ) %] > <fieldset class="action"> <label for="letter_code">Select notice:</label> > <select name="letter_code" id="letter_code"> > [% FOREACH letter IN letters %] >diff --git a/serials/claims.pl b/serials/claims.pl >index c826804..13167d7 100755 >--- a/serials/claims.pl >+++ b/serials/claims.pl >@@ -58,14 +58,9 @@ for my $s (@{$supplierlist} ) { > } > } > >-my $letters = GetLetters('claimissues'); >-my @letters; >-foreach (keys %{$letters}){ >- push @letters ,{code=>$_,name=> $letters->{$_}}; >-} >+my $letters = GetLetters({ module => 'claimissues' }); > >-my $letter=((scalar(@letters)>1) || ($letters[0]->{name}||$letters[0]->{code})); >-my @missingissues; >+my @missingissues; > my @supplierinfo; > if ($supplierid) { > @missingissues = GetLateOrMissingIssues($supplierid,$serialid,$order); >@@ -85,7 +80,7 @@ if($op && $op eq 'preview'){ > ### $cntupdate SHOULD be equal to scalar(@$serialnums) > } > } >-$template->param('letters'=>\@letters,'letter'=>$letter); >+ > $template->param( > order =>$order, > suploop => $supplierlist, >@@ -99,6 +94,7 @@ $template->param( > supplierloop => \@supplierinfo, > branchloop => $branchloop, > csv_profiles => C4::Csv::GetCsvProfiles( "sql" ), >+ letters => $letters, > (uc(C4::Context->preference("marcflavour"))) => 1 > ); > output_html_with_http_headers $input, $cookie, $template->output; >diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl >index e31b8f2..b17fc60 100755 >--- a/serials/subscription-add.pl >+++ b/serials/subscription-add.pl >@@ -88,7 +88,6 @@ if ($op eq 'modify' || $op eq 'dup' || $op eq 'modsubscription') { > if (!defined $subs->{letter}) { > $subs->{letter}= q{}; > } >- letter_loop($subs->{'letter'}, $template); > my $nextexpected = GetNextExpected($subscriptionid); > $nextexpected->{'isfirstissue'} = $nextexpected->{planneddate} eq $firstissuedate ; > $subs->{nextacquidate} = $nextexpected->{planneddate} if($op eq 'modify'); >@@ -123,6 +122,10 @@ if ($op eq 'modify' || $op eq 'dup' || $op eq 'modsubscription') { > my @fields_id = map { fieldid => $_ }, split '\|', $dont_copy_fields; > $template->param( dont_export_field_loop => \@fields_id ); > } >+ >+ my $letters = get_letter_loop( $subs->{letter} ); >+ $template->param( letterloop => $letters ); >+ > } > > my $onlymine = >@@ -152,7 +155,8 @@ $template->param(branchloop => $branchloop, > ); > # prepare template variables common to all $op conditions: > if ($op!~/^mod/) { >- letter_loop(q{}, $template); >+ my $letters = get_letter_loop(); >+ $template->param( letterloop => $letters ); > } > > if ($op eq 'addsubscription') { >@@ -172,7 +176,10 @@ if ($op eq 'addsubscription') { > } > $template->param(subtype => \@sub_type_data); > >- letter_loop( '', $template ) if ($op ne 'modsubscription' && $op ne 'dup' && $op ne 'modify'); >+ if ( $op ne 'modsubscription' && $op ne 'dup' && $op ne 'modify' ) { >+ my $letters = get_letter_loop(); >+ $template->param( letterloop => $letters ); >+ } > > my $new_biblionumber = $query->param('biblionumber_for_new_subscription'); > if (defined $new_biblionumber) { >@@ -225,19 +232,18 @@ if ($op eq 'addsubscription') { > output_html_with_http_headers $query, $cookie, $template->output; > } > >-sub letter_loop { >- my ($selected_letter, $templte) = @_; >- my $letters = GetLetters('serial'); >- my $letterloop; >- foreach my $thisletter (keys %{$letters}) { >- push @{$letterloop}, { >- value => $thisletter, >- selected => $thisletter eq $selected_letter, >- lettername => $letters->{$thisletter}, >- }; >- } >- $templte->param(letterloop => $letterloop); >- return; >+sub get_letter_loop { >+ my ($selected_lettercode) = @_; >+ my $letters = GetLetters({ module => 'serial' }); >+ return [ >+ map { >+ { >+ value => $_->{code}, >+ lettername => $_->{name}, >+ ( $_->{code} eq $selected_lettercode ? ( selected => 1 ) : () ), >+ } >+ } @$letters >+ ]; > } > > sub _get_sub_length { >diff --git a/t/Letters.t b/t/Letters.t >index 7cbf6c8..0dc4ff5 100755 >--- a/t/Letters.t >+++ b/t/Letters.t >@@ -31,7 +31,9 @@ $dbh->{mock_add_resultset} = $mock_letters; > > my $letters = C4::Letters::GetLetters(); > >-is( $letters->{ISBN}, 'book', 'HASH ref of ISBN is book' ); >+my ( $ISBN_letter ) = grep {$_->{code} eq 'ISBN'} @$letters; >+is( $ISBN_letter->{name}, 'book', 'letter name for "ISBN" letter is book' ); >+is( scalar( @$letters ), 2, 'GetLetters returns the 2 inserted letters' ); > > # Regression test for bug 10843 > # $dt->add takes a scalar, not undef >diff --git a/tools/overduerules.pl b/tools/overduerules.pl >index 810b1fd..11dd9bd 100755 >--- a/tools/overduerules.pl >+++ b/tools/overduerules.pl >@@ -201,9 +201,7 @@ if ($op eq 'save') { > } > my $branchloop = GetBranchesLoop($branch); > >-my $letters = GetLetters("circulation"); >- >-my $countletters = keys %{$letters}; >+my $letters = GetLetters({ module => "circulation" }); > > my @line_loop; > >@@ -222,25 +220,7 @@ for my $data (@categories) { > ); > $row{delay}=$temphash{$data->{'categorycode'}}->{"delay$i"}; > $row{debarred}=$temphash{$data->{'categorycode'}}->{"debarred$i"}; >- if ($countletters){ >- my @letterloop; >- foreach my $thisletter (sort { $letters->{$a} cmp $letters->{$b} } keys %$letters) { >- my $selected; >- if ( $temphash{$data->{categorycode}}->{"letter$i"} && >- $thisletter eq $temphash{$data->{'categorycode'}}->{"letter$i"}) { >- $selected = 1; >- } >- my %letterrow =(value => $thisletter, >- selected => $selected, >- lettername => $letters->{$thisletter}, >- ); >- push @letterloop, \%letterrow; >- } >- $row{letterloop}=\@letterloop; >- } else { >- $row{noletter}=1; >- $row{letter}=$temphash{$data->{'categorycode'}}->{"letter$i"}; >- } >+ $row{selected_lettercode} = $temphash{ $data->{categorycode} }->{"letter$i"}; > my @selected_mtts = @{ GetOverdueMessageTransportTypes( $branch, $data->{'categorycode'}, $i) }; > my @mtts; > for my $mtt ( @$message_transport_types ) { >@@ -268,24 +248,9 @@ for my $data (@categories) { > overduename => $data->{'categorycode'}, > line => $data->{'description'} > ); >- if ($countletters){ >- my @letterloop; >- foreach my $thisletter (sort { $letters->{$a} cmp $letters->{$b} } keys %$letters) { >- my $selected; >- if ($dat->{"letter$i"} && $thisletter eq $dat->{"letter$i"}) { >- $selected = 1; >- } >- my %letterrow =(value => $thisletter, >- selected => $selected, >- lettername => $letters->{$thisletter}, >- ); >- push @letterloop, \%letterrow; >- } >- $row{letterloop}=\@letterloop; >- } else { >- $row{noletter}=1; >- if ($dat->{"letter$i"}){$row{letter}=$dat->{"letter$i"};} >- } >+ >+ $row{selected_lettercode} = $dat->{"letter$i"}; >+ > if ($dat->{"delay$i"}){$row{delay}=$dat->{"delay$i"};} > if ($dat->{"debarred$i"}){$row{debarred}=$dat->{"debarred$i"};} > my @selected_mtts = @{ GetOverdueMessageTransportTypes( $branch, $data->{'categorycode'}, $i) }; >@@ -333,5 +298,6 @@ $template->param( > branch => $branch, > tabs => \@tabs, > message_transport_types => $message_transport_types, >+ letters => $letters, > ); > output_html_with_http_headers $input, $cookie, $template->output; >-- >1.8.3.2
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 11742
:
25892
|
28020
|
28021
|
28687
|
28688
|
28729
|
28956
| 29049 |
29050
|
29051
|
29180