From 9ffac50209ce8f67133bbb7a21a29bb6d326ab72 Mon Sep 17 00:00:00 2001
From: Srdjan <srdjan@catalyst.net.nz>
Date: Mon, 20 Aug 2012 09:20:15 +0200
Subject: [PATCH 2/2] Bug 7993 Follow-up

---
 C4/Reports/Guided.pm                               |   24 +++++++--
 .../prog/en/modules/reports/dictionary.tt          |    2 +-
 .../en/modules/reports/guided_reports_start.tt     |   24 +++++++--
 opac/svc/report                                    |   49 +++++++++---------
 reports/guided_reports.pl                          |    4 +-
 svc/report                                         |   54 ++++++++------------
 6 files changed, 90 insertions(+), 67 deletions(-)

diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm
index c677f85..0bd1774 100644
--- a/C4/Reports/Guided.pm
+++ b/C4/Reports/Guided.pm
@@ -400,7 +400,7 @@ sub nb_rows($) {
 
 =item execute_query
 
-  ($results, $total, $error) = execute_query($sql, $offset, $limit)
+  ($results, $error) = execute_query($sql, $offset, $limit)
 
 
 When passed C<$sql>, this function returns an array ref containing a result set
@@ -648,8 +648,26 @@ sub get_saved_reports {
 sub get_saved_report {
     my ($id) = @_;
     my $dbh   = C4::Context->dbh();
-    my $query = " SELECT * FROM saved_sql WHERE id = ?";
-    return $dbh->selectrow_hashref($query, undef, $id);
+    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);
 }
 
 =item create_compound($masterID,$subreportID)
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt
index a03a1b6..2e15834 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/dictionary.tt
@@ -33,7 +33,7 @@
 		<form action="/cgi-bin/koha/reports/dictionary.pl" method="post">
         <input type="hidden" name="phase" value="View Dictionary" />
 		[% IF ( areas ) %]
-                  Filter by area <select name="area">
+			Filter by area <select name="area">
 			<option value="">All</option>
 			[% FOREACH area IN areas %]
 			    [% IF ( area.selected ) %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt
index fd345d0..4041043 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt
@@ -517,7 +517,11 @@ canned reports and writing custom SQL reports.</p>
     [% IF groups_with_subgroups %]
     <li><label for="group">Report group: </label><select name="group" id="group" onChange="load_group_subgroups();">
         [% FOR g IN groups_with_subgroups %]
-    <option value="[% g.id %]" [% IF g.selected %]selected[% END %]>[% g.name %]</option>
+            [% IF g.selected %]
+    <option value="[% g.id %]" selected="selected">[% g.name %]</option>
+            [% ELSE %]
+    <option value="[% g.id %]">[% g.name %]</option>
+            [% END %]
     <script type="text/javascript">
         var g_sg = new Array();
             [% FOR sg IN g.subgroups %]
@@ -607,7 +611,11 @@ $(document).ready(function() {
     [% IF groups_with_subgroups %]
     <li><label for="group">Report group: </label><select name="group" id="group" onChange="load_group_subgroups();">
         [% FOR g IN groups_with_subgroups %]
-    <option value="[% g.id %]" [% IF g.selected %]selected[% END %]>[% g.name %]</option>
+            [% IF g.selected %]
+    <option value="[% g.id %]" selected="selected">[% g.name %]</option>
+            [% ELSE %]
+    <option value="[% g.id %]">[% g.name %]</option>
+            [% END %]
     <script type="text/javascript">
         var g_sg = new Array();
             [% FOR sg IN g.subgroups %]
@@ -724,7 +732,11 @@ $(document).ready(function() {
     [% IF groups_with_subgroups %]
     <li><label for="group">Report group: </label><select name="group" id="group" onChange="load_group_subgroups();">
         [% FOR g IN groups_with_subgroups %]
-    <option value="[% g.id %]" [% IF g.selected %]selected[% END %]>[% g.name %]</option>
+            [% IF g.selected %]
+    <option value="[% g.id %]" selected="selected">[% g.name %]</option>
+            [% ELSE %]
+    <option value="[% g.id %]">[% g.name %]</option>
+            [% END %]
     <script type="text/javascript">
         var g_sg = new Array();
             [% FOR sg IN g.subgroups %]
@@ -825,7 +837,11 @@ $(document).ready(function() {
     <select name="group" id="group" onChange="load_group_subgroups();">
         <option value="">-- All --</option>
     [% FOR g IN groups_with_subgroups %]
-        <option value="[% g.id %]" [% IF g.selected %]selected[% END %]>[% g.name %]</option>
+        [% IF g.selected %]
+        <option value="[% g.id %]" selected="selected">[% g.name %]</option>
+        [% ELSE %]
+        <option value="[% g.id %]">[% g.name %]</option>
+        [% END %]
         <script type="text/javascript">
             var g_sg = new Array();
         [% FOR sg IN g.subgroups %]
diff --git a/opac/svc/report b/opac/svc/report
index 17d12f4..38cbd42 100755
--- a/opac/svc/report
+++ b/opac/svc/report
@@ -31,35 +31,34 @@ my $query  = CGI->new();
 my $report_id = $query->param('id');
 my $report_name = $query->param('name');
 
-my $cache;
-my $sql;
-my $type;
-my $notes;
-my $cache_expiry;
-my $public;
-
-my $report_rec = get_saved_report($report_id);
+my $report_rec = get_saved_report( $report_name ? { 'name' => $report_name } : { 'id' => $report_id } );
 die "Sorry this report is not public\n" unless $report_rec->{public};
 
-if (Koha::Cache->is_cache_active) {
-    $cache = Koha::Cache->new(
-    );
-    my $page = $cache->get_from_cache("opac:report:$report_id");
-    if ($page) {
-        print $query->header;
-        print $page;
-        exit;
+
+my $cache_active = Koha::Cache->is_cache_active;
+my ($cache_key, $cache, $json_text);
+if ($cache_active) {
+    $cache_key = "opac:report:".($report_name ? "name:$report_name" : "id:$report_id");
+    $cache = Koha::Cache->new();
+    $json_text = $cache->get_from_cache($cache_key);
+}
+
+unless ($json_text) {
+    my $offset = 0;
+    my $limit  = C4::Context->preference("SvcMaxReportRows") || 10;
+    my ( $sth, $errors ) = execute_query( $report_rec->{savedsql}, $offset, $limit );
+    if ($sth) {
+        my $lines     = $sth->fetchall_arrayref;
+        $json_text = to_json($lines);
+
+        if ($cache_active) {
+            $cache->set_in_cache( $cache_key, $json_text, $report_rec->{cache_expiry} );
+        }
+    }
+    else {
+        $json_text = to_json($errors);
     }
 }
 
 print $query->header;
-my $offset = 0;
-my $limit  = C4::Context->preference("SvcMaxReportRows") || 10;
-my ( $sth, $errors ) = execute_query( $report_rec->{sql}, $offset, $limit );
-my $lines     = $sth->fetchall_arrayref;
-my $json_text = to_json($lines);
 print $json_text;
-
-if (Koha::Cache->is_cache_active) {
-    $cache->set_in_cache( "opac:report:$report_id", $json_text, $report_rec->{cache_expiry} );
-}
diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl
index 547d8ee..b249a90 100755
--- a/reports/guided_reports.pl
+++ b/reports/guided_reports.pl
@@ -126,8 +126,8 @@ elsif ( $phase eq 'Show SQL'){
         'id'      => $id,
         'reportname' => $report->{report_name},
         'notes'      => $report->{notes},
-     'sql'     => $report->{savedsql},
-      'showsql' => 1,
+        'sql'     => $report->{savedsql},
+        'showsql' => 1,
     );
 }
 
diff --git a/svc/report b/svc/report
index f1eae70..654ef15 100755
--- a/svc/report
+++ b/svc/report
@@ -33,13 +33,6 @@ my $query  = CGI->new();
 my $report_id = $query->param('id');
 my $report_name = $query->param('name');
 
-my $cache;
-my $sql;
-my $type;
-my $notes;
-my $cache_expiry;
-my $public;
-
 my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     {
         template_name   => "intranet-main.tmpl",
@@ -50,34 +43,31 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     }
 );
 
-if (Koha::Cache->is_cache_active) {
-    if ($report_name) { # When retrieving by name, we have to hit the
-                        # database to get the ID before we can check
-                        # the cache. Yuck.
-        ( $sql, $type, $report_name, $notes, $cache_expiry, $public, $report_id ) =
-            get_saved_report( { 'name' => $report_name } );
-    }
-
+my $cache_active = Koha::Cache->is_cache_active;
+my ($cache_key, $cache, $json_text);
+if ($cache_active) {
+    $cache_key = "intranet:report:".($report_name ? "name:$report_name" : "id:$report_id");
     $cache = Koha::Cache->new();
-    my $page = $cache->get_from_cache("intranet:report:$report_id");
-    if ($page) {
-        print $query->header;
-        print $page;
-        exit;
+    $json_text = $cache->get_from_cache($cache_key);
+}
+
+unless ($json_text) {
+    my $report_rec = get_saved_report($report_name ? { 'name' => $report_name } : { 'id' => $report_id });
+    my $offset = 0;
+    my $limit  = C4::Context->preference("SvcMaxReportRows") || 10;
+    my ( $sth, $errors ) = execute_query( $report_rec->{savedsql}, $offset, $limit );
+    if ($sth) {
+        my $lines     = $sth->fetchall_arrayref;
+        $json_text = to_json($lines);
+
+        if ($cache_active) {
+            $cache->set_in_cache( $cache_key, $json_text, $report_rec->{cache_expiry} );
+        }
+    }
+    else {
+        $json_text = to_json($errors);
     }
 }
 
 print $query->header;
-
-# $public isnt used for intranet
-my $report_rec = get_saved_report($report_id);
-my $offset = 0;
-my $limit  = C4::Context->preference("SvcMaxReportRows") || 10;
-my ( $sth, $errors ) = execute_query( $report_rec->{sql}, $offset, $limit );
-my $lines     = $sth->fetchall_arrayref;
-my $json_text = to_json($lines);
 print $json_text;
-
-if (Koha::Cache->is_cache_active) {
-    $cache->set_in_cache( "intranet:report:$report_id", $json_text, $report_rec->{cache_expiry} );
-}
-- 
1.7.10.4