From 5c4fdcf78aab2d284f3b88f9926756acb6a4d48f Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 7 May 2014 16:12:39 +0200 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. 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: Chris Cormack Signed-off-by: Katrin Fischer --- C4/Letters.pm | 2 + .../prog/en/modules/tools/letter.tt | 92 +++++++++++++------ .../prog/en/modules/tools/overduerules.tt | 12 +-- svc/letters | 63 +++++++++++++ tools/letter.pl | 62 +++++-------- 5 files changed, 154 insertions(+), 77 deletions(-) create mode 100755 svc/letters diff --git a/C4/Letters.pm b/C4/Letters.pm index 4e0ea20367..ff63aac9d0 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -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 : () ) ); 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 b6bdf88a7e..34df1e7caf 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/letter.tt @@ -1,6 +1,6 @@ [% USE Koha %] [% INCLUDE 'doc-head-open.inc' %] -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 %] +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 %] [% INCLUDE 'doc-head-close.inc' %] [% INCLUDE 'datatables.inc' %] @@ -26,7 +26,8 @@ $(document).ready(function() { }); [% END %] - $("#submit").click( function(event) { + $("#submit_form").click( function(event) { + event.preventDefault(); var at_least_one_exists = 0; $("fieldset.mtt").each( function(){ var title = $(this).find('input[name="title"]').val(); @@ -40,7 +41,6 @@ $(document).ready(function() { msg = msg.replace( "%s", mtt ); at_least_one_exists = 1; alert(msg) - event.preventDefault(); return false; } else if ( title.length > 0 && content.length > 0 ) { at_least_one_exists = 1; @@ -48,10 +48,34 @@ $(document).ready(function() { } ); if ( ! at_least_one_exists ) { alert( _("Please fill at least one template.") ); - event.preventDefault(); return false; } - return true; + + // Test if code already exists in DB + var new_lettercode = $("#code").val(); + [% IF copy_form %] + if ( new_lettercode == '[% code %]' ) { + alert( _("Please change the code.") ); + return false; + } + [% END %] + if ( new_lettercode != '[% code %]' ) { + $.ajax({ + data: { code: new_lettercode }, + type: 'GET', + url: '/cgi-bin/koha/svc/letters/', + success: function (data) { + if ( data.letters.length > 0 ) { + alert( _("This letter code is already used for another letter.") ); + return false; + } else { + $("#add_notice").submit(); + } + }, + }); + } else { + $("#add_notice").submit(); + } }); var sms_limit = 160; @@ -65,7 +89,7 @@ $(document).ready(function() { } }); }); -[% IF ( add_form ) %] +[% IF add_form or copy_form %] function cancel(f) { $('#op').val(""); @@ -117,9 +141,9 @@ $(document).ready(function() { [% INCLUDE 'header.inc' %] [% INCLUDE 'letters-search.inc' %] - + -[% IF ( add_form ) %]
[% ELSE %]
[% END %] +[% IF add_form or copy_form %]
[% ELSE %]
[% END %]
@@ -182,7 +206,7 @@ $(document).ready(function() { [% IF !independant_branch || !lette.branchcode %]
- + @@ -221,10 +245,15 @@ $(document).ready(function() { [% END %] -[% IF ( add_form ) %] +[% IF add_form or copy_form %]

[% IF ( modify ) %]Modify notice[% ELSE %]Add notice[% END %]

- - + + [% IF add_form %] + + [% ELSE %] + + [% END %] + [% IF ( modify ) %] @@ -232,7 +261,7 @@ $(document).ready(function() { [% END %]
- + [% IF independant_branch %] [% ELSE %] @@ -250,7 +279,11 @@ $(document).ready(function() {
  • - [% IF ( modify ) %][% END %] + [% IF adding %] + + [% END %] [% IF ( module == "catalogue" ) %] [% ELSE %] @@ -294,20 +327,19 @@ $(document).ready(function() {
  • - [% IF adding %] - - - Required - [% ELSE %] - - [% code %] - [% END %] + + + Required + [% IF copying %] + You must change this code to reflect the copy. + [% END %] +
    • -
  • - - - Required -
    • +
  • + + + Required +
    • [% FOREACH letter IN letters %]
  • @@ -376,12 +408,12 @@ $(document).ready(function() { [% IF code.search('DGST') %] Warning, this is a template for a Digest, as such, any references to branch data ( e.g. branches.branchname ) will refer to the borrower's home branch. [% END %]
    • -
    Cancel
    +
    Cancel
    [% END %] -[% IF ( add_validate ) %] +[% IF ( add_validate or copy_validate) %] Data recorded
    @@ -431,7 +463,7 @@ $(document).ready(function() {
    -[% UNLESS ( add_form ) %] +[% UNLESS add_form or copy_form %]
    [% INCLUDE 'tools-menu.inc' %]
    diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tt index 8c109a59c5..e561859430 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/overduerules.tt @@ -120,20 +120,16 @@ $(document).ready(function() { - [% IF ( value.noletter ) %] - - [% ELSE %] - [% END %] [% IF ( value.debarred ) %] diff --git a/svc/letters b/svc/letters new file mode 100755 index 0000000000..695fe68978 --- /dev/null +++ b/svc/letters @@ -0,0 +1,63 @@ +#!/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 . + +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 ], +); diff --git a/tools/letter.pl b/tools/letter.pl index 06666de6f8..c19d995d53 100755 --- a/tools/letter.pl +++ b/tools/letter.pl @@ -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 { -- 2.39.5