Commit 5c4fdcf7 authored by joubu's avatar joubu Committed by Tomas Cohen Arazi

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.

Maybe we should add a primary key (id) for the letter table in order to
pass the id to the template and correctly manage the letter code
duplication.

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.
Signed-off-by: default avatarChris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer's avatarKatrin Fischer <Katrin.Fischer.83@web.de>
parent 1976ec20
......@@ -73,6 +73,7 @@ C4::Letters - Give functions for Letters management
sub GetLetters {
my ($filters) = @_;
my $module = $filters->{module};
my $code = $filters->{code};
my $dbh = C4::Context->dbh;
my $letters = $dbh->selectall_arrayref(
q|
......@@ -81,6 +82,7 @@ sub GetLetters {
WHERE 1
|
. ( $module ? q| AND module = ?| : q|| )
. ( $code ? q| AND code = ?| : q|| )
. q| GROUP BY code ORDER BY name|, { Slice => {} }
, ( $module ? $module : () )
);
......
......@@ -120,20 +120,16 @@ $(document).ready(function() {
<input type="text" name="delay[% tab.number %]-[% value.overduename %]" size="5" value="[% value.delay %]" />
</td>
<td>
[% IF ( value.noletter ) %]
<input type="text" name="letter[% tab.number %]-[% value.overduename %]" value="[% value.letter %]" />
[% ELSE %]
<select name="letter[% tab.number %]-[% value.overduename %]">
<option value="">No notice</option>
[% FOREACH letterloop IN value.letterloop %]
[% IF ( letterloop.selected ) %]
<option value="[% letterloop.value %]" selected="selected">[% letterloop.lettername %]</option>
[% FOREACH letter IN letters %]
[% IF letter.code == value.selected_lettercode %]
<option value="[% letter.code %]" selected="selected">[% letter.name %]</option>
[% ELSE %]
<option value="[% letterloop.value %]">[% letterloop.lettername %]</option>
<option value="[% letter.code %]">[% letter.name %]</option>
[% END %]
[% END %]
</select>
[% END %]
</td>
<td>
[% IF ( value.debarred ) %]
......
#!/usr/bin/perl
# This file is part of Koha.
#
# Copyright 2014 BibLibre
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use C4::Service;
use C4::Letters qw( GetLetters );
=head1 NAME
svc/letters - Web service for getting letters
=head1 SYNOPSIS
GET /svc/letters
=head1 DESCRIPTION
For the moment, this service is only used to get a letter from a letter code.
=head1 METHODS
=cut
=head2 set_preference
=over 4
GET /svc/letters/$code
=back
Used to get letters with a given letter code.
=cut
our ( $query, $response ) = C4::Service->init( tools => 'edit_notices' );
sub get_letters {
my $letters = GetLetters({ code => $query->param('code') });
$response->param( letters => $letters );
C4::Service->return_success( $response );
}
C4::Service->dispatch(
[ 'GET /', [ 'code' ], \&get_letters ],
);
......@@ -78,7 +78,6 @@ sub get_letters {
my $letter = $dbh->selectall_hashref("SELECT * $sql", 'message_transport_type', undef, @$args);
return $letter;
}
# $protected_letters = protected_letters()
# - return a hashref of letter_codes representing letters that should never be deleted
sub protected_letters {
......@@ -121,17 +120,26 @@ $template->param(
action => $script_name
);
if ($op eq 'copy') {
add_copy();
$op = 'add_form';
if ( $op eq 'add_validate' or $op eq 'copy_validate' ) {
add_validate();
$op = q{}; # we return to the default screen for the next operation
}
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);
......@@ -258,7 +266,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');
......@@ -271,12 +278,12 @@ sub add_validate {
my $is_html = $input->param("is_html_$mtt");
my $title = shift @title;
my $content = shift @content;
my $letter = get_letters($oldbranchcode,$oldmodule, $code, $mtt);
my $letter = get_letters($branchcode,$oldmodule, $code, $mtt);
unless ( $title and $content ) {
delete_confirmed( $oldbranchcode, $oldmodule, $code, $mtt );
delete_confirmed( $branchcode, $oldmodule, $code, $mtt );
next;
}
if ( exists $letter->{$mtt} ) {
elsif ( exists $letter->{$mtt} ) {
$dbh->do(
q{
UPDATE letter
......@@ -285,7 +292,7 @@ sub add_validate {
},
undef,
$branchcode, $module, $name, $is_html || 0, $title, $content,
$oldbranchcode, $oldmodule, $code, $mtt
$branchcode, $oldmodule, $code, $mtt
);
} else {
$dbh->do(
......@@ -297,30 +304,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 keys %{ get_letters($branchcode,$module, $code, '*') };
my $old_letters = get_letters($oldbranchcode,$module, $code, '*');
my $message_transport_types = GetMessageTransportTypes();
for my $mtt ( @$message_transport_types ) {
next unless exists $old_letters->{$mtt};
my $old_letter = $old_letters->{$mtt};
$dbh->do(
q{INSERT INTO letter (branchcode,module,code,name,is_html,title,content,message_transport_type) VALUES (?,?,?,?,?,?,?,?)},
undef,
$branchcode, $module, $code, $old_letter->{name}, $old_letter->{is_html}, $old_letter->{title}, $old_letter->{content}, $mtt
);
}
return 1;
}
sub delete_confirm {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment