Bugzilla – Attachment 25892 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]
Bug 11742: A letter code should be unique.
Bug-11742-A-letter-code-should-be-unique.patch (text/plain), 12.36 KB, created by
Jonathan Druart
on 2014-03-06 12:53:28 UTC
(
hide
)
Description:
Bug 11742: A letter code should be unique.
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2014-03-06 12:53:28 UTC
Size:
12.36 KB
patch
obsolete
>From 2fc1df80395243f32d82799665a45c7f0ec99d86 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Thu, 6 Mar 2014 12:24:02 +0100 >Subject: [PATCH] Bug 11742: A letter code should be unique. > >This patch is a dirty way to fix a design issue on notices. >Currently the code assumes that a letter code is unique. Which is wrong, >the primary key is module, code, branchcode. >But the C4::Letters::GetLetters routine returns a hashref with letter >code in keys => only 1 per letter code is returned. > >I tried to fix the API but it is more complicated than I thought. > >Test plan: >Try to duplicate a letter code using edit, add and copy actions. >If you manage to do it, please describe how you did. >--- > .../intranet-tmpl/prog/en/modules/tools/letter.tt | 50 ++++++++++---- > tools/letter.pl | 72 +++++++++++--------- > 2 files changed, 77 insertions(+), 45 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt >index c414787..035cd08 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt >@@ -1,5 +1,5 @@ > [% INCLUDE 'doc-head-open.inc' %] >-<title>Koha › Tools › Notices[% IF ( add_form ) %][% IF ( modify ) %] › Modify notice[% ELSE %] › Add notice[% END %][% END %][% IF ( add_validate ) %] › Notice added[% END %][% IF ( delete_confirm ) %] › Confirm deletion[% END %]</title> >+<title>Koha › Tools › Notices[% IF ( add_form or copy_form ) %][% IF ( modify ) %] › Modify notice[% ELSE %] › Add notice[% END %][% END %][% IF ( add_validate or copy_validate) %] › Notice added[% END %][% IF ( delete_confirm ) %] › Confirm deletion[% END %]</title> > [% INCLUDE 'doc-head-close.inc' %] > <link rel="stylesheet" type="text/css" href="[% themelang %]/css/datatables.css" /> > [% INCLUDE 'datatables.inc' %] >@@ -24,7 +24,7 @@ $(document).ready(function() { > }); > [% END %] > }); >-[% IF ( add_form ) %] >+[% IF add_form or copy_form %] > > function cancel(f) { > $('#op').val(""); >@@ -112,14 +112,18 @@ $(document).ready(function() { > [% INCLUDE 'header.inc' %] > [% INCLUDE 'letters-search.inc' %] > >-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> › [% IF ( add_form ) %][% IF ( modify ) %]<a href="/cgi-bin/koha/tools/letter.pl">Notices & Slips</a> › Modify notice[% ELSE %] <a href="/cgi-bin/koha/tools/letter.pl">Notices & Slips</a> › Add notice[% END %][% ELSE %][% IF ( add_validate ) %] <a href="/cgi-bin/koha/tools/letter.pl">Notices & Slips</a> › Notice added[% ELSE %][% IF ( delete_confirm ) %] <a href="/cgi-bin/koha/tools/letter.pl">Notices & Slips</a> › Confirm deletion[% ELSE %]Notices & Slips[% END %][% END %][% END %]</div> >+<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> › [% IF ( add_form or copy_form) %][% IF ( modify ) %]<a href="/cgi-bin/koha/tools/letter.pl">Notices & Slips</a> › Modify notice[% ELSE %] <a href="/cgi-bin/koha/tools/letter.pl">Notices & Slips</a> › Add notice[% END %][% ELSE %][% IF ( add_validate or copy_validate) %] <a href="/cgi-bin/koha/tools/letter.pl">Notices & Slips</a> › Notice added[% ELSE %][% IF ( delete_confirm ) %] <a href="/cgi-bin/koha/tools/letter.pl">Notices & Slips</a> › Confirm deletion[% ELSE %]Notices & Slips[% END %][% END %][% END %]</div> > >-[% IF ( add_form ) %]<div id="doc" class="yui-t7">[% ELSE %]<div id="doc3" class="yui-t2">[% END %] >+[% IF add_form or copy_form %]<div id="doc" class="yui-t7">[% ELSE %]<div id="doc3" class="yui-t2">[% END %] > > <div id="bd"> > <div id="yui-main"> > <div class="yui-b"> > >+ [% IF already_exists %] >+ <div class="dialog alert">This code already exists. Please change it.</div> >+ [% END %] >+ > [% IF ( no_op_set ) %] > <h1>Notices and Slips</h1> > <form method="get" action="/cgi-bin/koha/tools/letter.pl" id="selectlibrary"> >@@ -180,7 +184,7 @@ $(document).ready(function() { > <td style="white-space: nowrap"> > [% IF !independant_branch || !lette.branchcode %] > <form method="post" action="/cgi-bin/koha/tools/letter.pl"> >- <input type="hidden" name="op" value="copy" /> >+ <input type="hidden" name="op" value="copy_form" /> > <input type="hidden" name="oldbranchcode" value="[% lette.branchcode %]" /> > <input type="hidden" name="module" value="[% lette.module %]" /> > <input type="hidden" name="code" value="[% lette.code %]" /> >@@ -219,10 +223,15 @@ $(document).ready(function() { > [% END %] > > >-[% IF ( add_form ) %] >+[% IF add_form or copy_form %] > <h1>[% IF ( modify ) %]Modify notice[% ELSE %]Add notice[% END %]</h1> > <form action="?" name="Aform" method="post"> >- <input type="hidden" name="op" id="op" value="add_validate" /> >+ [% IF add_form %] >+ <input type="hidden" name="op" id="op" value="add_validate" /> >+ [% ELSE %] >+ <input type="hidden" name="op" id="op" value="copy_validate" /> >+ [% END %] >+ > <input type="hidden" name="checked" value="0" /> > [% IF ( modify ) %] > <input type="hidden" name="add" value="0" /> >@@ -230,7 +239,7 @@ $(document).ready(function() { > <input type="hidden" name="add" value="1" /> > [% END %] > <fieldset class="rows"> >- <input type="hidden" name="oldbranchcode" value="[% branchcode %]" /> >+ <input type="hidden" name="oldbranchcode" value="[% oldbranchcode %]" /> > [% IF independant_branch %] > <input type="hidden" name="branchcode" value="[% independant_branch %]" /> > [% ELSE %] >@@ -248,7 +257,11 @@ $(document).ready(function() { > <li> > <label for="module">Koha module:</label> > <input type="hidden" name="oldmodule" value="[% module %]" /> >- [% IF ( modify ) %]<select name="module" id="module">[% END %] [% IF ( adding ) %] <select name="module" id="module" onchange="javascript:window.location.href = unescape(window.location.pathname)+'?op=add_form&module='+this.value+'&content='+window.document.forms['Aform'].elements['content'].value;">[% END %] >+ [% IF adding %] >+ <select name="module" id="module" onchange="javascript:window.location.href = unescape(window.location.pathname)+'?op=add_form&module='+this.value+'&content='+window.document.forms['Aform'].elements['content'].value;"> >+ [% ELSE %] >+ <select name="module" id="module"> >+ [% END %] > [% IF ( catalogue ) %] > <option value="catalogue" selected="selected">Catalog</option> > [% ELSE %] >@@ -291,9 +304,18 @@ $(document).ready(function() { > [% END %] > </select> > </li> >- <li> >- <span class="label">Code:</span>[% IF ( adding ) %]<input type="text" id="code" name="code" size="20" maxlength="20" />[% ELSE %]<input type="hidden" id="code" name="code" value="[% code %]" />[% code %][% END %] >- </li> >+ <li> >+ <span class="label">Code:</span> >+ [% IF adding or copying %] >+ <input type="text" id="code" name="code" size="20" maxlength="20" value="[% code %]"/> >+ [% IF copying %] >+ You must change this code to reflect the copy. >+ [% END %] >+ [% ELSE %] >+ <input type="hidden" id="code" name="code" value="[% code %]" />[% code %] >+ [% END %] >+ <input type="hidden" id="code" name="oldcode" value="[% oldcode %]" /> >+ </li> > <li> > <label for="name">Name:</label><input type="text" id="name" name="name" size="60" value="[% name %]" /> > </li> >@@ -328,7 +350,7 @@ $(document).ready(function() { > </form> > [% END %] > >-[% IF ( add_validate ) %] >+[% IF ( add_validate or copy_validate) %] > Data recorded > <form action="[% action %]" method="post"> > <input type="submit" value="OK" /> >@@ -377,7 +399,7 @@ $(document).ready(function() { > > </div> > </div> >-[% UNLESS ( add_form ) %] >+[% UNLESS add_form or copy_form %] > <div class="yui-b noprint"> > [% INCLUDE 'tools-menu.inc' %] > </div> >diff --git a/tools/letter.pl b/tools/letter.pl >index 7582e66..892878c 100755 >--- a/tools/letter.pl >+++ b/tools/letter.pl >@@ -76,7 +76,12 @@ sub letter_exists { > my $letter = $dbh->selectrow_hashref("SELECT * $sql", undef, @$args); > return $letter; > } >- >+sub lettercode_exists { >+ my ( $letter_code ) = @_; >+ my $dbh = C4::Context->dbh; >+ my ( $exist ) = $dbh->selectrow_array("SELECT count(*) FROM letter where code=?", undef, $letter_code); >+ return $exist; >+} > # $protected_letters = protected_letters() > # - return a hashref of letter_codes representing letters that should never be deleted > sub protected_letters { >@@ -119,17 +124,37 @@ $template->param( > action => $script_name > ); > >-if ($op eq 'copy') { >- add_copy(); >- $op = 'add_form'; >+if ( $op eq 'add_validate' or $op eq 'copy_validate' ) { >+ my $added = add_validate(); >+ unless ( $added ) { >+ if ( $op eq 'add_validate' ) { >+ $op = 'add_form'; >+ } else { >+ $op = 'copy_form'; >+ $code = $input->param('oldcode'); >+ } >+ $template->param( already_exists => 1 ); >+ } >+ else { >+ $op = q{}; # next operation is to return to default screen >+ } > } >- >-if ($op eq 'add_form') { >- add_form($branchcode, $module, $code); >+if ($op eq 'copy_form') { >+ my $oldbranchcode = $input->param('oldbranchcode') || q||; >+ my $branchcode = $input->param('branchcode') || q||; >+ my $oldcode = $input->param('oldcode') || $input->param('code'); >+ add_form($oldbranchcode, $module, $code); >+ $template->param( >+ oldbranchcode => $oldbranchcode, >+ branchcode => $branchcode, >+ branchloop => _branchloop($branchcode), >+ oldcode => $oldcode, >+ copying => 1, >+ modify => 0, >+ ); > } >-elsif ( $op eq 'add_validate' ) { >- add_validate(); >- $op = q{}; # next operation is to return to default screen >+elsif ( $op eq 'add_form' ) { >+ add_form($branchcode, $module, $code); > } > elsif ( $op eq 'delete_confirm' ) { > delete_confirm($branchcode, $module, $code); >@@ -227,7 +252,6 @@ sub add_form { > > sub add_validate { > my $dbh = C4::Context->dbh; >- my $oldbranchcode = $input->param('oldbranchcode'); > my $branchcode = $input->param('branchcode') || ''; > my $module = $input->param('module'); > my $oldmodule = $input->param('oldmodule'); >@@ -236,12 +260,15 @@ sub add_validate { > my $is_html = $input->param('is_html'); > my $title = $input->param('title'); > my $content = $input->param('content'); >- if (letter_exists($oldbranchcode,$oldmodule, $code)) { >+ if ( lettercode_exists( $code ) ) { >+ return 0; >+ } >+ elsif (letter_exists($branchcode,$oldmodule, $code)) { > $dbh->do( > q{UPDATE letter SET branchcode = ?, module = ?, name = ?, is_html = ?, title = ?, content = ? WHERE branchcode = ? AND module = ? AND code = ?}, > undef, > $branchcode, $module, $name, $is_html || 0, $title, $content, >- $oldbranchcode, $oldmodule, $code >+ $branchcode, $oldmodule, $code > ); > } else { > $dbh->do( >@@ -252,24 +279,7 @@ sub add_validate { > } > # set up default display > default_display($branchcode); >-} >- >-sub add_copy { >- my $dbh = C4::Context->dbh; >- my $oldbranchcode = $input->param('oldbranchcode'); >- my $branchcode = $input->param('branchcode'); >- my $module = $input->param('module'); >- my $code = $input->param('code'); >- >- return if letter_exists($branchcode,$module, $code); >- >- my $old_letter = letter_exists($oldbranchcode,$module, $code); >- >- $dbh->do( >- q{INSERT INTO letter (branchcode,module,code,name,is_html,title,content) VALUES (?,?,?,?,?,?,?)}, >- undef, >- $branchcode, $module, $code, $old_letter->{name}, $old_letter->{is_html}, $old_letter->{title}, $old_letter->{content} >- ); >+ return 1; > } > > sub delete_confirm { >-- >1.7.10.4
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