Attachment 75716 Details for Bug 20495 – Bug 20495: Remove get_saved_report

[patch] Bug 20495: Remove get_saved_report

Bug-20495-Remove-getsavedreport.patch (text/plain), 12.45 KB, created by Jonathan Druart on 2018-05-31 19:34:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2018-05-31 19:34:41 UTC

Size: 12.45 KB

patch

obsolete

>From fd6dbf01418c1eec2e221624e40337b62a20c337 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
>
>Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> 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.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 20495: 73423 | 74868 | 74869 | 74870 | 75653 | 75654 | 75716 | 75717