Bug 17898: Automagically convert SQL reports
Bug 17196 move the marcxml out of the biblioitems table. That will break SQL reports using it. It would be handy to propose an automagically way to convert the SQL reports. We do not want to update the reports automatically without user inputs, it will be too hasardous. However we can lead the user to convert them. In this patchset I suggest to warn the user if a report is subject to be updated. TODO: Add a way to mark this job done (using a pref?) to remove the check and not to display false positives. Test plan: - Create some SQL reports (see https://wiki.koha-community.org/wiki/SQL_Reports_Library) - Go on the report list page (/reports/guided_reports.pl?phase=Use saved) - For the reports using biblioitems.marcxml you will see a new column warning you that it is obsolete - Click on update link => that will open a modal with the converted SQL query - Click on the update button => you will be informed that the query has been updated If all the reports are updated, the new column "Update" will no longer be displayed. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Jonathan Druart
Kyle M Hall
parent
9719284622
commit
f22d2e7200
|
|
@ -854,7 +854,6 @@ sub get_sql {
|
|||
sub get_results {
|
||||
my ( $report_id ) = @_;
|
||||
my $dbh = C4::Context->dbh;
|
||||
warn $report_id;
|
||||
return $dbh->selectall_arrayref(q|
|
||||
SELECT id, report, date_run
|
||||
FROM saved_reports
|
||||
|
|
@ -988,6 +987,26 @@ sub _get_display_value {
|
|||
return $original_value;
|
||||
}
|
||||
|
||||
|
||||
=head3 convert_sql
|
||||
|
||||
my $updated_sql = C4::Reports::Guided::convert_sql( $sql );
|
||||
|
||||
Convert a sql query using biblioitems.marcxml to use the new
|
||||
biblio_metadata.metadata field instead
|
||||
|
||||
=cut
|
||||
|
||||
sub convert_sql {
|
||||
my ( $sql ) = @_;
|
||||
my $updated_sql = $sql;
|
||||
if ( $sql =~ m|biblioitems| and $sql =~ m|marcxml| ) {
|
||||
$updated_sql =~ s|biblioitems|biblio_metadata|;
|
||||
$updated_sql =~ s|marcxml|metadata|;
|
||||
}
|
||||
return $updated_sql;
|
||||
}
|
||||
|
||||
1;
|
||||
__END__
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,19 @@
|
|||
[% INCLUDE 'doc-head-open.inc' %]
|
||||
<title>Koha › Reports › Convert report</title>
|
||||
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
|
||||
</head>
|
||||
<body id="report_convert" class="catalog">
|
||||
<div id="main" style="direction: ltr;">
|
||||
[% IF msg == 'no_report' %]
|
||||
There is no valid report for this id.
|
||||
[% ELSIF msg == 'can_be_updated' %]
|
||||
<h3>Existing sql</h3>
|
||||
<pre>[% current_sql %]</pre>
|
||||
<h3>Updated sql</h3>
|
||||
<pre>[% updated_sql %]</pre>
|
||||
[% ELSE %]
|
||||
Something went wrong.
|
||||
[% END %]
|
||||
</div>
|
||||
</body>
|
||||
</html>
|
||||
|
|
@ -98,6 +98,7 @@ $("#delColumn").on("click",function(){
|
|||
],
|
||||
"aoColumns": [
|
||||
null,null,null,null,null,null,null,null,{ "sType": "title-string" },null,null,null,[% IF (usecache) %]null,[% END %]null,null
|
||||
null,null,null,null,null,null,null,null,{ "sType": "title-string" },null,null,null,[% IF (usecache) %]null,[% END %]null,null[% IF has_obsolete_reports %],null[% END %]
|
||||
],
|
||||
'oLanguage': {
|
||||
'sZeroRecords': _("No matching reports found")
|
||||
|
|
@ -166,6 +167,20 @@ $("#delColumn").on("click",function(){
|
|||
return false;
|
||||
}
|
||||
});
|
||||
|
||||
$("body").on("click", ".update_sql", function(e){
|
||||
e.preventDefault();
|
||||
var ltitle = $(this).text();
|
||||
var report_id = $(this).data("report_id");
|
||||
var page = $(this).attr("href");
|
||||
$("#update_sql .modal-body").load(page + " div");
|
||||
$('#update_sql').modal({show:true});
|
||||
$("#update_sql_button").attr("href", "/cgi-bin/koha/reports/guided_reports.pl?phase=Use saved&op=convert&report_id=" + report_id);
|
||||
});
|
||||
$("#update_sql").on("hidden", function(){
|
||||
$("#update_sql_label").html("");
|
||||
$("#update_sql .modal-body").html("<div id=\"loading\"><img src=\"[% interface %]/[% theme %]/img/spinner-small.gif\" alt=\"\" /> "+_("Loading")+"</div>");
|
||||
});
|
||||
[% END %]
|
||||
|
||||
[% IF ( showsql ) %]
|
||||
|
|
@ -267,6 +282,20 @@ $("#delColumn").on("click",function(){
|
|||
[% END %]
|
||||
</div>
|
||||
|
||||
<div id="update_sql" class="modal hide" tabindex="-1" role="dialog" aria-labelledby="update_sql_label" aria-hidden="true">
|
||||
<div class="modal-header">
|
||||
<button type="button" class="closebtn" data-dismiss="modal" aria-hidden="true">×</button>
|
||||
<h3 id="update_sql_label">Update SQL</h3>
|
||||
</div>
|
||||
<div class="modal-body">
|
||||
<div id="loading"> <img src="[% interface %]/[% theme %]/img/spinner-small.gif" alt="" /> Loading </div>
|
||||
</div>
|
||||
<div class="modal-footer">
|
||||
<a href="#" class="btn" id="update_sql_button" role="button" data-toggle="modal">Update</a>
|
||||
<button class="btn" data-dismiss="modal" aria-hidden="true">Close</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div id="doc3" class="yui-t1">
|
||||
<div id="bd">
|
||||
<div id="yui-main">
|
||||
|
|
@ -306,6 +335,12 @@ canned reports and writing custom SQL reports.</p>
|
|||
</form>
|
||||
[% END %]
|
||||
|
||||
[% IF report_converted %]
|
||||
<div class="dialog message">
|
||||
The report "[% report_converted %]" has been converted.
|
||||
</div>
|
||||
[% END %]
|
||||
|
||||
[% IF ( saved1 ) %]
|
||||
[% IF ( savedreports ) %]<h1>Saved reports</h1>
|
||||
|
||||
|
|
@ -342,6 +377,7 @@ canned reports and writing custom SQL reports.</p>
|
|||
<th>Public</th>
|
||||
[% IF (usecache) %] <th>Cache expiry (seconds)</th> [% END %]
|
||||
<th>Saved results</th>
|
||||
[% IF has_obsolete_reports %]<th>Update</th>[% END %]
|
||||
<th> </th>
|
||||
</tr>
|
||||
</thead>
|
||||
|
|
@ -381,6 +417,14 @@ canned reports and writing custom SQL reports.</p>
|
|||
<br/>
|
||||
[% END %]
|
||||
</td>
|
||||
[% IF has_obsolete_reports %]
|
||||
<td>
|
||||
[% IF savedreport.seems_obsolete %]
|
||||
This report seems obsolete, it uses biblioitems.marcxml field.
|
||||
<a href="/cgi-bin/koha/svc/convert_report?report_id=[% savedreport.id %]" data-report_id="[% savedreport.id %]" class="update_sql btn btn-mini" title="Update SQL"><i class="fa fa-eye"></i> Update SQL</a>
|
||||
[% END %]
|
||||
</td>
|
||||
[% END %]
|
||||
<td>
|
||||
<div class="dropup">
|
||||
<div class="btn-group">
|
||||
|
|
|
|||
|
|
@ -89,6 +89,7 @@ elsif ($session and not $input->param('clear_filters')) {
|
|||
$filter = $session->param('report_filter');
|
||||
}
|
||||
|
||||
my $op = $input->param('op') || q||;
|
||||
|
||||
my @errors = ();
|
||||
if ( !$phase ) {
|
||||
|
|
@ -106,6 +107,29 @@ elsif ( $phase eq 'Build new' ) {
|
|||
);
|
||||
} elsif ( $phase eq 'Use saved' ) {
|
||||
|
||||
if ( $op eq 'convert' ) {
|
||||
my $report_id = $input->param('report_id');
|
||||
my $report = C4::Reports::Guided::get_saved_report($report_id);
|
||||
warn $report_id;
|
||||
use Data::Printer colored => 1; warn p $report;
|
||||
if ($report) {
|
||||
my $updated_sql = C4::Reports::Guided::convert_sql( $report->{savedsql} );
|
||||
C4::Reports::Guided::update_sql(
|
||||
$report_id,
|
||||
{
|
||||
sql => $updated_sql,
|
||||
name => $report->{report_name},
|
||||
group => $report->{report_group},
|
||||
subgroup => $report->{report_subgroup},
|
||||
notes => $report->{notes},
|
||||
public => $report->{public},
|
||||
cache_expiry => $report->{cache_expiry},
|
||||
}
|
||||
);
|
||||
$template->param( report_converted => $report->{report_name} );
|
||||
}
|
||||
}
|
||||
|
||||
# use a saved report
|
||||
# get list of reports and display them
|
||||
my $group = $input->param('group');
|
||||
|
|
@ -113,8 +137,13 @@ elsif ( $phase eq 'Build new' ) {
|
|||
$filter->{group} = $group;
|
||||
$filter->{subgroup} = $subgroup;
|
||||
my $reports = get_saved_reports($filter);
|
||||
my $has_obsolete_reports;
|
||||
for my $report ( @$reports ) {
|
||||
$report->{results} = C4::Reports::Guided::get_results( $report->{id} );
|
||||
if ( $report->{savedsql} =~ m|marcxml| ) {
|
||||
$report->{seems_obsolete} = 1;
|
||||
$has_obsolete_reports++;
|
||||
}
|
||||
}
|
||||
$template->param(
|
||||
'saved1' => 1,
|
||||
|
|
@ -122,6 +151,7 @@ elsif ( $phase eq 'Build new' ) {
|
|||
'usecache' => $usecache,
|
||||
'groups_with_subgroups'=> groups_with_subgroups($group, $subgroup),
|
||||
filters => $filter,
|
||||
has_obsolete_reports => $has_obsolete_reports,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,53 @@
|
|||
#!/usr/bin/perl
|
||||
|
||||
# This file is part of Koha.
|
||||
#
|
||||
# Copyright 2017 Koha Development Team
|
||||
#
|
||||
# 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::Auth;
|
||||
use C4::Reports::Guided;
|
||||
use C4::Output;
|
||||
use CGI qw ( -utf8 );
|
||||
|
||||
my $query = CGI->new();
|
||||
my $report_id = $query->param('report_id');
|
||||
|
||||
my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
|
||||
{
|
||||
template_name => "reports/convert_report.tt",
|
||||
query => $query,
|
||||
type => "intranet",
|
||||
authnotrequired => 0,
|
||||
flagsrequired => { reports => 'execute_reports' },
|
||||
}
|
||||
);
|
||||
|
||||
my $report = get_saved_report( $report_id );
|
||||
|
||||
my $params;
|
||||
if ( $report ) {
|
||||
my $sql = $report->{savedsql};
|
||||
my $updated_sql = C4::Reports::Guided::convert_sql( $sql );
|
||||
$params = { msg => 'can_be_updated', updated_sql => $updated_sql, current_sql => $sql };
|
||||
} else {
|
||||
$params = { msg => 'no_report' };
|
||||
}
|
||||
|
||||
$template->param( %$params );
|
||||
|
||||
output_html_with_http_headers $query, $cookie, $template->output;
|
||||
|
|
@ -18,7 +18,7 @@
|
|||
|
||||
use Modern::Perl;
|
||||
|
||||
use Test::More tests => 8;
|
||||
use Test::More tests => 9;
|
||||
use Test::Warn;
|
||||
|
||||
use t::lib::TestBuilder;
|
||||
|
|
@ -288,6 +288,61 @@ subtest 'Ensure last_run is populated' => sub {
|
|||
isnt( $report->last_run, $previous_last_run, 'Second run of report updates last_run' );
|
||||
};
|
||||
|
||||
subtest 'convert_sql' => sub {
|
||||
plan tests => 3;
|
||||
|
||||
my $sql = q|
|
||||
SELECT biblionumber, ExtractValue(marcxml,
|
||||
'count(//datafield[@tag="505"])') AS count505
|
||||
FROM biblioitems
|
||||
HAVING count505 > 1|;
|
||||
my $expected_converted_sql = q|
|
||||
SELECT biblionumber, ExtractValue(metadata,
|
||||
'count(//datafield[@tag="505"])') AS count505
|
||||
FROM biblio_metadata
|
||||
HAVING count505 > 1|;
|
||||
|
||||
is( C4::Reports::Guided::convert_sql( $sql ), $expected_converted_sql, "Simple query should have been correctly converted");
|
||||
|
||||
$sql = q|
|
||||
SELECT biblionumber, substring(
|
||||
ExtractValue(marcxml,'//controlfield[@tag="008"]'), 8,4 ) AS 'PUB DATE',
|
||||
title
|
||||
FROM biblioitems
|
||||
INNER JOIN biblio USING (biblionumber)
|
||||
WHERE biblionumber = 14|;
|
||||
|
||||
$expected_converted_sql = q|
|
||||
SELECT biblionumber, substring(
|
||||
ExtractValue(metadata,'//controlfield[@tag="008"]'), 8,4 ) AS 'PUB DATE',
|
||||
title
|
||||
FROM biblio_metadata
|
||||
INNER JOIN biblio USING (biblionumber)
|
||||
WHERE biblionumber = 14|;
|
||||
is( C4::Reports::Guided::convert_sql( $sql ), $expected_converted_sql, "Query with biblio info should have been correctly converted");
|
||||
|
||||
$sql = q|
|
||||
SELECT concat(b.title, ' ', ExtractValue(m.marcxml,
|
||||
'//datafield[@tag="245"]/subfield[@code="b"]')) AS title, b.author,
|
||||
count(h.reservedate) AS 'holds'
|
||||
FROM biblio b
|
||||
LEFT JOIN biblioitems m USING (biblionumber)
|
||||
LEFT JOIN reserves h ON (b.biblionumber=h.biblionumber)
|
||||
GROUP BY b.biblionumber
|
||||
HAVING count(h.reservedate) >= 42|;
|
||||
|
||||
$expected_converted_sql = q|
|
||||
SELECT concat(b.title, ' ', ExtractValue(m.metadata,
|
||||
'//datafield[@tag="245"]/subfield[@code="b"]')) AS title, b.author,
|
||||
count(h.reservedate) AS 'holds'
|
||||
FROM biblio b
|
||||
LEFT JOIN biblio_metadata m USING (biblionumber)
|
||||
LEFT JOIN reserves h ON (b.biblionumber=h.biblionumber)
|
||||
GROUP BY b.biblionumber
|
||||
HAVING count(h.reservedate) >= 42|;
|
||||
is( C4::Reports::Guided::convert_sql( $sql ), $expected_converted_sql, "Query with 2 joins should have been correctly converted");
|
||||
};
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
|
||||
sub trim {
|
||||
|
|
|
|||
Loading…
Reference in New Issue