From 8c158cdffb599ad4799ec71d36b73cfef0f44406 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 31 Jul 2012 14:18:22 +0200 Subject: [PATCH] Bug 7167: Improve display + factorize get_queries Despite it's size, this patch is dealing with display questions only: * The text "comments" and "queries" was hardcoded in ajax-updatedb-getinfo.pl script. It has been replaced by a JSON call, returning 2 separate values, "comments:" and "queries:" is now in the template, making it translatable * Some minor tweak in the display (like putting things in bold, displaying OK in green, warnings in yellow and KO in red) * Reordering the column headers for more readability: * Status column is merged with availability, column is after status * Status/availability terms more clear: "Not applied" instead of "unknown", "Applied and OK", "Applied and failed", "Applied and forced" are the 3 other statuses * Removed one click to display comments on DBREv not yet applied: before the patch, one had to click "Show details", then "Get comments", now, "Get comments" is enough Signed-off-by: Paul Poulain --- C4/Update/Database.pm | 75 +++++---- admin/ajax-updatedb-getinfos.pl | 29 ++-- installer/data/mysql/update.pl | 2 + .../prog/en/modules/admin/updatedatabase.tt | 172 +++++++++++--------- 4 files changed, 160 insertions(+), 118 deletions(-) diff --git a/C4/Update/Database.pm b/C4/Update/Database.pm index ac62209..b7106b3 100644 --- a/C4/Update/Database.pm +++ b/C4/Update/Database.pm @@ -130,14 +130,14 @@ sub execute_version { $queries = get_queries ( $filepath ); } when ( /.pl/ ) { - my $versions_dir = C4::Context->intranetdir . '/installer/data/mysql/versions/'; - my $version_file = $versions_dir . $filename; - if ( do $version_file ) { - $queries = _get_queries(); - } else { + eval { + $queries = get_queries ( $filepath ); + }; + if ($@) { $report->{$version} = { error => "LOAD_FUNCTIONS_FAILED", filename => $filename, + error_str => $@, }; } } @@ -364,38 +364,51 @@ sub get_queries { open my $fh, "<", $filepath; my @queries; my @comments; - my $old_delimiter = $/; - while ( <$fh> ) { - my $line = $_; - chomp $line; - $line =~ s/^\s*//; - if ( $line =~ s/^--(.*)$// ) { - my @l = split $old_delimiter; - if ( @l > 1 ) { - my $tmp_query; - for my $l ( @l ) { - if ( $l =~ m/^--/ ) { - push @comments, $l; - next; + if ( $filepath =~ /\.pl$/ ) { + if ( do $filepath ) { + my $infos = _get_queries(); + @queries = @{ $infos->{queries} } if exists $infos->{queries}; + @comments = @{ $infos->{comments} } if exists $infos->{comments}; + } + if ( $@ ) { + die "I can't load $filepath. Please check the execute flag and if this file is a valid perl script ($@)"; + } + } else { + my $old_delimiter = $/; + while ( <$fh> ) { + my $line = $_; + chomp $line; + $line =~ s/^\s*//; + if ( $line =~ /^--/ ) { + my @l = split $old_delimiter, $line; + if ( @l > 1 ) { + my $tmp_query; + for my $l ( @l ) { + if ( $l =~ /^--/ ) { + $l =~ s/^--\s*//; + push @comments, $l; + next; + } + $tmp_query .= $l . $old_delimiter; } - $tmp_query .= $l . $old_delimiter; + push @queries, $tmp_query if $tmp_query; + next; } - push @queries, $tmp_query if $tmp_query; + + $line =~ s/^--\s*//; + push @comments, $line; next; } - - push @comments, $1; - next; - } - if ( $line =~ /^delimiter (.*)$/i ) { - $/ = $1; - next; + if ( $line =~ /^delimiter (.*)$/i ) { + $/ = $1; + next; + } + $line =~ s#$/##; + push @queries, $line if not $line =~ /^\s*$/; # Push if query is not empty } - $line =~ s#$/##; - push @queries, $line if not $line =~ /^\s*$/; # Push if query is not empty + $/ = $old_delimiter; + close $fh; } - $/ = $old_delimiter; - close $fh; return { queries => \@queries, comments => \@comments }; } diff --git a/admin/ajax-updatedb-getinfos.pl b/admin/ajax-updatedb-getinfos.pl index 25ceda5..fb91613 100755 --- a/admin/ajax-updatedb-getinfos.pl +++ b/admin/ajax-updatedb-getinfos.pl @@ -30,32 +30,33 @@ this script returns comments for a updatedatabase version use Modern::Perl; use CGI; +use JSON; use C4::Update::Database; +use C4::Output; my $input = new CGI; my $version = $input->param('version'); -binmode STDOUT, ":utf8"; -print $input->header(-type => 'text/plain', -charset => 'UTF-8'); my $filepath; my $queries; eval { $filepath = C4::Update::Database::get_filepath( $version ); $queries = C4::Update::Database::get_queries( $filepath ); }; -if ( $@ ){ - print $@; - exit; -} -if ( @{ $$queries{comments} } ) { - print "Comments :
" . join ( "
", @{ $$queries{comments} } ); +my $param = {comments => "", queries => ""}; +if ( $@ ){ + $param->{errors} = $@; } else { - print "No comment
"; -} + if ( exists $queries->{comments} and @{ $queries->{comments} } ) { + $param->{comments} = join ( "
", @{ $queries->{comments} } ); + } -if ( @{ $$queries{queries} } ) { - print "

Queries :
" . join ( "
", @{ $$queries{queries} } ); -} else { - print "
No queries"; + if ( exists $queries->{queries} and @{ $queries->{queries} } ) { + $param->{queries} = join ( "
", @{ $queries->{queries} } ); + } } + +my $json_text = to_json( $param, { utf8 => 1 } ); + +output_with_http_headers $input, undef, $json_text, 'json'; diff --git a/installer/data/mysql/update.pl b/installer/data/mysql/update.pl index 605fe6c..33e123f 100644 --- a/installer/data/mysql/update.pl +++ b/installer/data/mysql/update.pl @@ -64,6 +64,8 @@ if ( $version or $all ) { my ( $v, $r ) = each %$report; if ( ref( $r ) eq 'HASH' ) { say "\t$v => $r->{error}"; + } elsif ( ref ( $r ) eq 'ARRAY' ) { + say "\t$_" for @$r; } else { say "\t$v => $r"; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/updatedatabase.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/updatedatabase.tt index 706dbf7..2c68f91 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/updatedatabase.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/updatedatabase.tt @@ -22,15 +22,32 @@ } ); } function get_infos(version, node){ - $.ajax({ - url: "/cgi-bin/koha/admin/ajax-updatedb-getinfos.pl", - data: { - version: version - }, - success: function(data){ - $(node).replaceWith(data); - }, - }); + $.getJSON('/cgi-bin/koha/admin/ajax-updatedb-getinfos.pl', + { version: version }, + function(param) { + if ( param['errors'] ) { + $(node).replaceWith(_("Errors occured: ") + param['errors']); + } + var s; + s = "" + _("Comments:") + ""; + s += '
'; + if ( param['comments'] ) { + s += param['comments']; + } else { + s += _("No comments"); + } + s += '

'; + + s += "" + _("Queries:") + ""; + s += '
'; + if ( param['queries'] ) { + s += param['queries']; + } else { + s += _("No queries"); + } + $(node).replaceWith(s); + } + ); } //]]> @@ -56,16 +73,30 @@
  • [% report_loo.version %] -- [% FOREACH r IN report_loo.report %] - [% IF r.error.error == "ALREADY_EXISTS" %] - This file ( [% r.error.filepath %] ) still already execute in version [% r.error.old_version %] : same md5 ([% r.error.md5 %]) - [Mark as OK] - [% ELSIF r.error.error == "LOAD_FUNCTIONS_FAILED" %] - Load functions in [% r.error.filename %] failed - [% ELSIF r.error.error == "BAD_EXTENSION" %] - This extension ([% r.error.extension %]) is not take into account (only .pl or .sql)"; + [% IF r.error.error == "ALREADY_EXISTS" %] + + [% r.error.filepath %] already executed in version [% r.error.old_version %] : same md5 ([% r.error.md5 %]) + [Mark as OK] + + [% ELSIF r.error.error == "LOAD_FUNCTIONS_FAILED" %] + + Load functions in [% r.error.filename %] failed ([% r.error.error_str %]) + + [% ELSIF r.error.error == "BAD_EXTENSION" %] + + This extension ([% r.error.extension %]) is not take into account (only .pl or .sql)"; + + [% ELSE %] + [% IF r.error == "OK" %] + + [% r.error %]; + [% ELSE %] + [% r.error %]; + [% END %] + [% END %] [% END %]
  • [% END %] @@ -96,9 +127,8 @@ DB revision - Comments Status - Availability + Comments Details @@ -107,74 +137,70 @@ [% v.version %] - [% FOREACH c IN v.comments %] - [% c.comment %] - [% END %] - - [% IF v.available %] - Unknown + Not applied + [% IF (dev_mode) %] + [Execute] + [% END %] [% ELSE %] - [% IF v.status %] - OK - [% IF v.status == 2 %] - [FORCED] - [% END %] - [% ELSE %] - Failed + [% SWITCH v.status %] + [% CASE 0 %] + Applied and failed + [% CASE 1 %] + Applied and OK + [% CASE 2 %] + Applied and Forced + [% CASE %] + Status does not exist ! [% END %] [% END %] - [% IF v.available %] - Available - [% IF (dev_mode) %] - [Execute] - [% END %] - [% ELSE %] - Already executed + [% FOREACH c IN v.comments %] + [% c.comment %]
    [% END %] + [% IF v.available %] + Get comments + [% ELSE %] Show details + [% END %] [% END %] -- 1.7.9.5