From b9324fe40ec4a266aee405314c76c95d92adeb38 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 13 Oct 2015 09:49:55 +0100 Subject: [PATCH] Bug 14100: Better errors handling This patch mainly improves the errors handling. It's now not possible to add a translation for the same entity-code-lang combination. Note that we could add entity,code,lang,translation as a unique key but translation is TEXT (TEXT column 'translation' used in key specification without a key length). Let's see later what's is best here. It also: - wording: Translate for other languages => into - fixes encoding issues on add/update (was just display) - add entity and code to the title of the pop-up window Signed-off-by: Jonathan Druart --- .../prog/en/modules/admin/itemtypes.tt | 2 +- .../prog/en/modules/admin/localization.tt | 54 +++++++++++---- svc/localization | 80 +++++++++++++--------- 3 files changed, 89 insertions(+), 47 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt index 8b48ad6..8e2a297 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt @@ -126,7 +126,7 @@ Item types administration [% END %]
  • Required - Translate for other languages + Translate into other languages
  • [% IF ( noItemTypeImages ) %]
  • Image: Item type images are disabled. To enable them, turn off the noItemTypeImages system preference
  • diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt index ab6ca72..68310e7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt @@ -17,7 +17,13 @@ message.text("Entity %s (code %s) for lang %s has correctly been updated with '%s'".format(data.entity, data.code, data.lang, data.translation)); } else if ( type == 'error_on_update' ) { message = $('
    '); - message.text("An error occurred when updating this translation"); + if ( data.error_code == 'already_exists' ) { + message.text("This translation already exists for this language."); + } else if ( data.error_code == 'no_change' ) { + message.text("No change made for this translation."); + } else { + message.text("An error occurred when updating this translation."); + } } else if ( type == 'success_on_delete' ) { message = $('
    '); message.text("The translation (id %s) has been removed successfully".format(data.id)); @@ -29,7 +35,12 @@ message.text("Translation (id %s) has been added successfully".format(data.id)); } else if ( type == 'error_on_insert' ) { message = $('
    '); - message.text("An error occurred when adding this translation"); + message = $('
    '); + if ( data.error_code == 'already_exists' ) { + message.text("This translation already exists for this language."); + } else { + message.text("An error occurred when adding this translation"); + } } $(messages).append(message); @@ -45,14 +56,27 @@ type: 'PUT', url: '/cgi-bin/koha/svc/localization', success: function (data) { - if ( data.is_changed ) { - $(cell).css('background-color', '#00FF00'); - show_message({ type: 'success_on_update', data: data }); - } - if ( $(cell).hasClass('lang') ) { - $(cell).text(data.lang) - } else if ( $(cell).hasClass('translation') ) { - $(cell).text(data.translation) + if ( data.error ) { + $(cell).css('background-color', '#FF0000'); + if ( $(cell).hasClass('lang') ) { + $(cell).text(data.lang) + } else if ( $(cell).hasClass('translation') ) { + $(cell).text(data.translation) + } + show_message({ type: 'error_on_update', data: data }); + } else { + if ( data.is_changed ) { + $(cell).css('background-color', '#00FF00'); + show_message({ type: 'success_on_update', data: data }); + } else { + $(cell).css('background-color', '#FF6400'); + show_message({ type: 'error_on_update', data: { error_code: 'no_change' } }); + } + if ( $(cell).hasClass('lang') ) { + $(cell).text(data.lang) + } else if ( $(cell).hasClass('translation') ) { + $(cell).text(data.translation) + } } }, error: function (data) { @@ -149,9 +173,13 @@ type: 'POST', url: '/cgi-bin/koha/svc/localization', success: function (data) { - // FIXME Should append the delete link - table.row.add( [ data.id, data.entity, data.code, data.lang, data.translation, "" ] ).draw(); - show_message({ type: 'success_on_insert', data: data }); + if ( data.error ) { + show_message({ type: 'error_on_insert', data: data }); + } else { + // FIXME Should append the delete link + table.row.add( [ data.id, data.entity, data.code, data.lang, data.translation, "" ] ).draw(); + show_message({ type: 'success_on_insert', data: data }); + } }, error: function (data) { show_message({ type: 'error_on_insert', data: data }); diff --git a/svc/localization b/svc/localization index ebec0e2..b5d6688 100755 --- a/svc/localization +++ b/svc/localization @@ -1,6 +1,7 @@ #!/usr/bin/perl use Modern::Perl; +use Encode qw( encode ); use C4::Service; use Koha::Localizations; @@ -8,15 +9,15 @@ use Koha::Localizations; our ( $query, $response ) = C4::Service->init( parameters => 'parameters_remaining_permissions' ); sub get_translations { - my $rs = Koha::Localizations->search({ type => $query->param('type'), code => $query->param('code') }); + my $rs = Koha::Localizations->search({ entity => $query->param('entity'), code => $query->param('code') }); my @translations; while ( my $s = $rs->next ) { push @translations, { - id => $s->localization_id, - type => $s->type, - code => $s->code, - lang => $s->lang, - translation => $s->translation, + id => Encode::encode( 'utf-8', $s->localization_id ), + entity => Encode::encode( 'utf-8', $s->entity ), + code => Encode::encode( 'utf-8', $s->code ), + lang => Encode::encode( 'utf-8', $s->lang ), + translation => Encode::encode( 'utf-8', $s->translation ), } } $response->param( translations => \@translations ); @@ -27,26 +28,31 @@ sub update_translation { my $id = $query->param('id'); my $translation = $query->param('translation'); my $lang = $query->param('lang'); - my $localization = Koha::Localizations->find( $id ); + my $localization = Koha::Localizations->find( $id ); if ( defined $lang and $localization->lang ne $lang ) { $localization->lang( $lang ) } if ( defined $translation and $localization->translation ne $translation ) { $localization->translation( $translation ) } - my $is_changed; + my %params; if ( $localization->is_changed ) { - $is_changed = 1; - $localization->store; + unless ( Koha::Localizations->search( { entity => $localization->entity, code => $localization->code, lang => $lang, translation => $translation, id => { '!=' => $localization->id }, } )->count ) { + $params{is_changed} = 1; + $localization->store; + } else { + $params{error} = 1; + $params{error_code} = 'already_exists'; + } } $response->param( - id => $localization->localization_id, - entity => $localization->entity, - code => $localization->code, - lang => $localization->lang, - translation => $localization->translation, - is_changed => $is_changed, + %params, + id => Encode::encode('utf-8', $localization->localization_id), + entity => Encode::encode('utf-8', $localization->entity), + code => Encode::encode('utf-8', $localization->code), + lang => Encode::encode('utf-8', $localization->lang), + translation => Encode::encode('utf-8', $localization->translation), ); C4::Service->return_success( $response ); } @@ -56,24 +62,32 @@ sub add_translation { my $code = $query->param('code'); my $lang = $query->param('lang'); my $translation = $query->param('translation'); - my $localization = Koha::Localization->new( - { - entity => $entity, - code => $code, - lang => $lang, - translation => $translation, - } - ); - $localization->store; - $localization->localization_id; - $response->param( - id => $localization->localization_id, - entity => $localization->entity, - code => $localization->code, - lang => $localization->lang, - translation => $localization->translation, - ); + unless ( Koha::Localizations->search({entity => $entity, code => $code, lang => $lang, translation => $translation, })->count ) { + my $localization = Koha::Localization->new( + { + entity => $entity, + code => $code, + lang => $lang, + translation => $translation, + } + ); + warn $translation; + warn $localization->translation; + warn Encode::encode('utf-8', $localization->translation); + $localization->store; + $localization->localization_id; + $response->param( + id => Encode::encode('utf-8', $localization->localization_id), + entity => Encode::encode('utf-8', $localization->entity), + code => Encode::encode('utf-8', $localization->code), + lang => Encode::encode('utf-8', $localization->lang), + translation => Encode::encode('utf-8', $localization->translation), + ); + + } else { + $response->param( error => 1, error_code => 'already_exists', ); + } C4::Service->return_success( $response ); } -- 2.1.0