From 4ec5a67c6f989635b14f048c3720bea7a8d86d38 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 29 Mar 2018 13:32:19 +0000 Subject: [PATCH] Bug 20495: Remove get_saved_report To test: 1 - prove t/db_dependent/Reports/Guided.t 2 - grep "get_saved_report" - ensure there are no occurences of the singular form 3 - create, save, edit, and convert a report 4 - access a public report and report json from opac and staff client 5 - Ensure all function as expected Signed-off-by: Mark Tompsett Signed-off-by: Jonathan Druart Signed-off-by: Nick Clemens --- C4/Reports/Guided.pm | 31 ++---------------- misc/cronjobs/runreport.pl | 9 +++--- opac/svc/report | 9 +++--- reports/guided_reports.pl | 57 +++++++++++++++++---------------- svc/convert_report | 5 +-- svc/report | 7 ++-- t/db_dependent/Reports/Guided.t | 1 + 7 files changed, 50 insertions(+), 69 deletions(-) diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm index ecebe86838..f8d98e6f1e 100644 --- a/C4/Reports/Guided.pm +++ b/C4/Reports/Guided.pm @@ -39,7 +39,7 @@ BEGIN { @ISA = qw(Exporter); @EXPORT = qw( get_report_types get_report_areas get_report_groups get_columns build_query get_criteria - save_report get_saved_reports execute_query get_saved_report + save_report get_saved_reports execute_query get_column_type get_distinct_values save_dictionary get_from_dictionary delete_definition delete_report format_results get_sql nb_rows update_sql @@ -622,8 +622,8 @@ sub delete_report { my (@ids) = @_; return unless @ids; foreach my $id (@ids) { - my $data = get_saved_report($id); - logaction( "REPORTS", "DELETE", $id, "$data->{'report_name'} | $data->{'savedsql'} " ) if C4::Context->preference("ReportsLog"); + my $data = Koha::Reports->find($id); + logaction( "REPORTS", "DELETE", $id, $data->report_name." | ".$data->savedsql ) if C4::Context->preference("ReportsLog"); } my $dbh = C4::Context->dbh; my $query = 'DELETE FROM saved_sql WHERE id IN (' . join( ',', ('?') x @ids ) . ')'; @@ -694,31 +694,6 @@ sub get_saved_reports { return $result; } -sub get_saved_report { - my $dbh = C4::Context->dbh(); - my $query; - my $report_arg; - if ($#_ == 0 && ref $_[0] ne 'HASH') { - ($report_arg) = @_; - $query = " SELECT * FROM saved_sql WHERE id = ?"; - } elsif (ref $_[0] eq 'HASH') { - my ($selector) = @_; - if ($selector->{name}) { - $query = " SELECT * FROM saved_sql WHERE report_name = ?"; - $report_arg = $selector->{name}; - } elsif ($selector->{id} || $selector->{id} eq '0') { - $query = " SELECT * FROM saved_sql WHERE id = ?"; - $report_arg = $selector->{id}; - } else { - return; - } - } else { - return; - } - return $dbh->selectrow_hashref($query, undef, $report_arg); -} - - =head2 get_column_type($column) This takes a column name of the format table.column and will return what type it is diff --git a/misc/cronjobs/runreport.pl b/misc/cronjobs/runreport.pl index cb10125938..b7fc9f84c2 100755 --- a/misc/cronjobs/runreport.pl +++ b/misc/cronjobs/runreport.pl @@ -21,6 +21,7 @@ use Modern::Perl; use C4::Reports::Guided; # 0.12 +use Koha::Reports; use C4::Context; use C4::Log; use Koha::Email; @@ -231,14 +232,14 @@ my $today = dt_from_string(); my $date = $today->ymd(); foreach my $report_id (@ARGV) { - my $report = get_saved_report($report_id); + my $report = Koha::Reports->find( $report_id ); unless ($report) { warn "ERROR: No saved report $report_id found"; next; } - my $sql = $report->{savedsql}; - my $report_name = $report->{report_name}; - my $type = $report->{type}; + my $sql = $report->savedsql; + my $report_name = $report->report_name; + my $type = $report->type; $verbose and print "SQL: $sql\n\n"; if ( $subject eq "" ) diff --git a/opac/svc/report b/opac/svc/report index f83b12e70e..49afb3df27 100755 --- a/opac/svc/report +++ b/opac/svc/report @@ -24,6 +24,7 @@ use Modern::Perl; use C4::Reports::Guided; +use Koha::Reports; use JSON; use CGI qw ( -utf8 ); @@ -34,10 +35,10 @@ my $report_id = $query->param('id'); my $report_name = $query->param('name'); my $report_annotation = $query->param('annotated'); -my $report_rec = get_saved_report( $report_name ? { 'name' => $report_name } : { 'id' => $report_id } ); +my $report_rec = Koha::Reports->find( $report_name ? { 'name' => $report_name } : { 'id' => $report_id } ); if (!$report_rec) { die "There is no such report.\n"; } -die "Sorry this report is not public\n" unless $report_rec->{public}; +die "Sorry this report is not public\n" unless $report_rec->public; my @sql_params = $query->param('sql_params'); @@ -55,7 +56,7 @@ if ($cache_active) { unless ($json_text) { my $offset = 0; my $limit = C4::Context->preference("SvcMaxReportRows") || 10; - my $sql = $report_rec->{savedsql}; + my $sql = $report_rec->savedsql; # convert SQL parameters to placeholders $sql =~ s/(<<.*?>>)/\?/g; @@ -74,7 +75,7 @@ unless ($json_text) { if ($cache_active) { $cache->set_in_cache( $cache_key, $json_text, - { expiry => $report_rec->{cache_expiry} } ); + { expiry => $report_rec->cache_expiry } ); } } else { diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 2273f1cc62..0989dfdbd1 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -25,6 +25,7 @@ use URI::Escape; use File::Temp; use File::Basename qw( dirname ); use C4::Reports::Guided; +use Koha::Reports; use C4::Auth qw/:DEFAULT get_session/; use C4::Output; use C4::Debug; @@ -109,22 +110,22 @@ elsif ( $phase eq 'Build new' ) { if ( $op eq 'convert' ) { my $report_id = $input->param('report_id'); - my $report = C4::Reports::Guided::get_saved_report($report_id); + my $report = Koha::Reports->find($report_id); if ($report) { - my $updated_sql = C4::Reports::Guided::convert_sql( $report->{savedsql} ); + 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}, + 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} ); + $template->param( report_converted => $report->report_name ); } } @@ -172,29 +173,29 @@ elsif ( $phase eq 'Delete Saved') { elsif ( $phase eq 'Show SQL'){ my $id = $input->param('reports'); - my $report = get_saved_report($id); + my $report = Koha::Reports->find($id); $template->param( 'id' => $id, - 'reportname' => $report->{report_name}, - 'notes' => $report->{notes}, - 'sql' => $report->{savedsql}, - 'showsql' => 1, + 'reportname' => $report->report_name, + 'notes' => $report->notes, + 'sql' => $report->savedsql, + 'showsql' => 1, ); } elsif ( $phase eq 'Edit SQL'){ my $id = $input->param('reports'); - my $report = get_saved_report($id); - my $group = $report->{report_group}; - my $subgroup = $report->{report_subgroup}; + my $report = Koha::Reports->find($id); + my $group = $report->report_group; + my $subgroup = $report->report_subgroup; $template->param( - 'sql' => $report->{savedsql}, - 'reportname' => $report->{report_name}, + 'sql' => $report->savedsql, + 'reportname' => $report->report_name, 'groups_with_subgroups' => groups_with_subgroups($group, $subgroup), - 'notes' => $report->{notes}, + 'notes' => $report->notes, 'id' => $id, - 'cache_expiry' => $report->{cache_expiry}, - 'public' => $report->{public}, + 'cache_expiry' => $report->cache_expiry, + 'public' => $report->public, 'usecache' => $usecache, 'editsql' => 1, ); @@ -686,10 +687,10 @@ elsif ($phase eq 'Run this report'){ ); my ( $sql, $original_sql, $type, $name, $notes ); - if (my $report = get_saved_report($report_id)) { - $sql = $original_sql = $report->{savedsql}; - $name = $report->{report_name}; - $notes = $report->{notes}; + if (my $report = Koha::Reports->find($report_id)) { + $sql = $original_sql = $report->savedsql; + $name = $report->report_name; + $notes = $report->notes; my @rows = (); # if we have at least 1 parameter, and it's not filled, then don't execute but ask for parameters @@ -850,8 +851,8 @@ elsif ($phase eq 'Export'){ # export results to tab separated text or CSV my $report_id = $input->param('report_id'); - my $report = get_saved_report($report_id); - my $sql = $report->{savedsql}; + my $report = Koha::Reports->find($report_id); + my $sql = $report->savedsql; my @param_names = $input->multi_param('param_name'); my @sql_params = $input->multi_param('sql_params'); my $format = $input->param('format'); diff --git a/svc/convert_report b/svc/convert_report index 659de68547..eb2999011d 100755 --- a/svc/convert_report +++ b/svc/convert_report @@ -21,6 +21,7 @@ use Modern::Perl; use C4::Auth; use C4::Reports::Guided; +use Koha::Reports; use C4::Output; use CGI qw ( -utf8 ); @@ -37,11 +38,11 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); -my $report = get_saved_report( $report_id ); +my $report = Koha::Reports->find( $report_id ); my $params; if ( $report ) { - my $sql = $report->{savedsql}; + 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 { diff --git a/svc/report b/svc/report index 0188eaadd7..662aa58c52 100755 --- a/svc/report +++ b/svc/report @@ -22,6 +22,7 @@ use Modern::Perl; use C4::Auth; use C4::Reports::Guided; +use Koha::Reports; use JSON; use CGI qw ( -utf8 ); @@ -33,7 +34,7 @@ my $report_id = $query->param('id'); my $report_name = $query->param('name'); my $report_annotation = $query->param('annotated'); -my $report_rec = get_saved_report( $report_name ? { 'name' => $report_name } : { 'id' => $report_id } ); +my $report_rec = Koha::Reports->find( $report_name ? { 'name' => $report_name } : { 'id' => $report_id } ); if (!$report_rec) { die "There is no such report.\n"; } my @sql_params = $query->param('sql_params'); @@ -60,7 +61,7 @@ if ($cache_active) { unless ($json_text) { my $offset = 0; my $limit = C4::Context->preference("SvcMaxReportRows") || 10; - my $sql = $report_rec->{savedsql}; + my $sql = $report_rec->savedsql; # convert SQL parameters to placeholders $sql =~ s/(<<.*?>>)/\?/g; @@ -77,7 +78,7 @@ unless ($json_text) { $json_text = encode_json($lines); if ($cache_active) { - $cache->set_in_cache( $cache_key, $json_text, { expiry => $report_rec->{cache_expiry} } ); + $cache->set_in_cache( $cache_key, $json_text, { expiry => $report_rec->cache_expiry } ); } } else { diff --git a/t/db_dependent/Reports/Guided.t b/t/db_dependent/Reports/Guided.t index 2de73402ee..827cfc17a6 100644 --- a/t/db_dependent/Reports/Guided.t +++ b/t/db_dependent/Reports/Guided.t @@ -24,6 +24,7 @@ use Test::Warn; use t::lib::TestBuilder; use C4::Context; use Koha::Database; +use Koha::Reports; use_ok('C4::Reports::Guided'); can_ok( -- 2.39.5