Bugzilla – Attachment 74869 Details for
Bug 20495
Refactor C4::Reports.Guided - remove get_saved_report
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20495: Remove get_saved_report
Bug-20495-Remove-getsavedreport.patch (text/plain), 13.04 KB, created by
Mark Tompsett
on 2018-04-26 00:22:47 UTC
(
hide
)
Description:
Bug 20495: Remove get_saved_report
Filename:
MIME Type:
Creator:
Mark Tompsett
Created:
2018-04-26 00:22:47 UTC
Size:
13.04 KB
patch
obsolete
>From f741e16a80805405427c2b654633392ad346cb7b Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 >--- > C4/Reports/Guided.pm | 42 +++++++----------------------- > 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, 56 insertions(+), 74 deletions(-) > >diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm >index d61b282..380fbf5 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 create_compound run_compound >+ save_report get_saved_reports execute_query create_compound run_compound > 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,30 +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 create_compound($masterID,$subreportID) > > This will take 2 reports and create a compound report using both of them >@@ -729,12 +705,12 @@ sub create_compound { > my $dbh = C4::Context->dbh(); > > # get the reports >- my $master = get_saved_report($masterID); >- my $mastersql = $master->{savedsql}; >- my $mastertype = $master->{type}; >- my $sub = get_saved_report($subreportID); >- my $subsql = $master->{savedsql}; >- my $subtype = $master->{type}; >+ my $master = Koha::Reports->find($masterID); >+ my $mastersql = $master->savedsql; >+ my $mastertype = $master->type; >+ my $sub = Koha::Reports->find($subreportID); >+ my $subsql = $master->savedsql; >+ my $subtype = $master->type; > > # now we have to do some checking to see how these two will fit together > # or if they will >diff --git a/misc/cronjobs/runreport.pl b/misc/cronjobs/runreport.pl >index cb10125..b7fc9f8 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 f83b12e..49afb3d 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 b5aa1d3..d1f3d92 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 659de68..eb29990 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 0188eaa..662aa58 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 2de7340..827cfc1 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.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20495
:
73423
|
74868
|
74869
|
74870
|
75653
|
75654
|
75716
|
75717