Bugzilla – Attachment 32100 Details for
Bug 10126
C4::Reports::Guided is not Plack safe
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10126: Remove my variables at module level
Bug-10126-Remove-my-variables-at-module-level.patch (text/plain), 8.35 KB, created by
Chris Cormack
on 2014-10-09 15:52:22 UTC
(
hide
)
Description:
Bug 10126: Remove my variables at module level
Filename:
MIME Type:
Creator:
Chris Cormack
Created:
2014-10-09 15:52:22 UTC
Size:
8.35 KB
patch
obsolete
>From 9c628f22aecf25934ce9ee78effb74b6cf99c2f8 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Thu, 9 Oct 2014 17:16:34 +0200 >Subject: [PATCH] Bug 10126: Remove my variables at module level > >In C4::Reports::Guided, a lot of variables was defined at module level >and reused in subroutine. > >I didn't find any problem with Plack, so I'm not sure this patch is >useful. > >Test plan: >Verify there is no more ^my in the module and you don't find any >regression with the guided reports tools (with and without Plack) > >Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> > >I'm unsure if this is needed, but I have verified it causes no >regressions >--- > C4/Reports/Guided.pm | 126 +++++++++++++++++++++++++++++---------------------- > 1 file changed, 71 insertions(+), 55 deletions(-) > >diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm >index a8eb1d0..b782407 100644 >--- a/C4/Reports/Guided.pm >+++ b/C4/Reports/Guided.pm >@@ -72,17 +72,19 @@ This will return a list of all the available report areas > > =cut > >-my @REPORT_AREA = ( >- [CIRC => "Circulation"], >- [CAT => "Catalogue"], >- [PAT => "Patrons"], >- [ACQ => "Acquisition"], >- [ACC => "Accounts"], >-); >-my $AREA_NAME_SQL_SNIPPET >- = "CASE report_area " . >+sub get_area_name_sql_snippet { >+ my @REPORT_AREA = ( >+ [CIRC => "Circulation"], >+ [CAT => "Catalogue"], >+ [PAT => "Patrons"], >+ [ACQ => "Acquisition"], >+ [ACC => "Accounts"], >+ ); >+ >+ return "CASE report_area " . > join (" ", map "WHEN '$_->[0]' THEN '$_->[1]'", @REPORT_AREA) . > " END AS areaname"; >+} > > sub get_report_areas { > >@@ -91,48 +93,14 @@ sub get_report_areas { > return $report_areas; > } > >-my %table_areas = ( >+sub get_table_areas { >+ return ( > CIRC => [ 'borrowers', 'statistics', 'items', 'biblioitems' ], > CAT => [ 'items', 'biblioitems', 'biblio' ], > PAT => ['borrowers'], > ACQ => [ 'aqorders', 'biblio', 'items' ], > ACC => [ 'borrowers', 'accountlines' ], >-); >-my %keys = ( >- CIRC => [ 'statistics.borrowernumber=borrowers.borrowernumber', >- 'items.itemnumber = statistics.itemnumber', >- 'biblioitems.biblioitemnumber = items.biblioitemnumber' ], >- CAT => [ 'items.biblioitemnumber=biblioitems.biblioitemnumber', >- 'biblioitems.biblionumber=biblio.biblionumber' ], >- PAT => [], >- ACQ => [ 'aqorders.biblionumber=biblio.biblionumber', >- 'biblio.biblionumber=items.biblionumber' ], >- ACC => ['borrowers.borrowernumber=accountlines.borrowernumber'], >-); >- >-# have to do someting here to know if its dropdown, free text, date etc >-my %criteria = ( >- CIRC => [ 'statistics.type', 'borrowers.categorycode', 'statistics.branch', >- 'biblioitems.publicationyear|date', 'items.dateaccessioned|date' ], >- CAT => [ 'items.itemnumber|textrange', 'items.biblionumber|textrange', >- 'items.barcode|textrange', 'biblio.frameworkcode', >- 'items.holdingbranch', 'items.homebranch', >- 'biblio.datecreated|daterange', 'biblio.timestamp|daterange', >- 'items.onloan|daterange', 'items.ccode', >- 'items.itemcallnumber|textrange', 'items.itype', 'items.itemlost', >- 'items.location' ], >- PAT => [ 'borrowers.branchcode', 'borrowers.categorycode' ], >- ACQ => ['aqorders.datereceived|date'], >- ACC => [ 'borrowers.branchcode', 'borrowers.categorycode' ], >-); >- >-# Adds itemtypes to criteria, according to the syspref >-if ( C4::Context->preference('item-level_itypes') ) { >- unshift @{ $criteria{'CIRC'} }, 'items.itype'; >- unshift @{ $criteria{'CAT'} }, 'items.itype'; >-} else { >- unshift @{ $criteria{'CIRC'} }, 'biblioitems.itemtype'; >- unshift @{ $criteria{'CAT'} }, 'biblioitems.itemtype'; >+ ); > } > > =head2 get_report_types >@@ -216,6 +184,7 @@ sub get_columns { > > # this calls the internal fucntion _get_columns > my ( $area, $cgi ) = @_; >+ my %table_areas = get_table_areas; > my $tables = $table_areas{$area} > or die qq{Unsuported report area "$area"}; > >@@ -260,8 +229,23 @@ This is what get_columns returns. > > sub build_query { > my ( $columns, $criteria, $orderby, $area, $totals, $definition ) = @_; >+ >+ my %keys = ( >+ CIRC => [ 'statistics.borrowernumber=borrowers.borrowernumber', >+ 'items.itemnumber = statistics.itemnumber', >+ 'biblioitems.biblioitemnumber = items.biblioitemnumber' ], >+ CAT => [ 'items.biblioitemnumber=biblioitems.biblioitemnumber', >+ 'biblioitems.biblionumber=biblio.biblionumber' ], >+ PAT => [], >+ ACQ => [ 'aqorders.biblionumber=biblio.biblionumber', >+ 'biblio.biblionumber=items.biblionumber' ], >+ ACC => ['borrowers.borrowernumber=accountlines.borrowernumber'], >+ ); >+ >+ > ### $orderby > my $keys = $keys{$area}; >+ my %table_areas = get_table_areas; > my $tables = $table_areas{$area}; > > my $sql = >@@ -331,6 +315,33 @@ Returns an arraref to hashrefs suitable for using in a tmpl_loop. With the crite > sub get_criteria { > my ($area,$cgi) = @_; > my $dbh = C4::Context->dbh(); >+ >+ # have to do someting here to know if its dropdown, free text, date etc >+ my %criteria = ( >+ CIRC => [ 'statistics.type', 'borrowers.categorycode', 'statistics.branch', >+ 'biblioitems.publicationyear|date', 'items.dateaccessioned|date' ], >+ CAT => [ 'items.itemnumber|textrange', 'items.biblionumber|textrange', >+ 'items.barcode|textrange', 'biblio.frameworkcode', >+ 'items.holdingbranch', 'items.homebranch', >+ 'biblio.datecreated|daterange', 'biblio.timestamp|daterange', >+ 'items.onloan|daterange', 'items.ccode', >+ 'items.itemcallnumber|textrange', 'items.itype', 'items.itemlost', >+ 'items.location' ], >+ PAT => [ 'borrowers.branchcode', 'borrowers.categorycode' ], >+ ACQ => ['aqorders.datereceived|date'], >+ ACC => [ 'borrowers.branchcode', 'borrowers.categorycode' ], >+ ); >+ >+ # Adds itemtypes to criteria, according to the syspref >+ if ( C4::Context->preference('item-level_itypes') ) { >+ unshift @{ $criteria{'CIRC'} }, 'items.itype'; >+ unshift @{ $criteria{'CAT'} }, 'items.itype'; >+ } else { >+ unshift @{ $criteria{'CIRC'} }, 'biblioitems.itemtype'; >+ unshift @{ $criteria{'CAT'} }, 'biblioitems.itemtype'; >+ } >+ >+ > my $crit = $criteria{$area}; > my $column_defs = _get_column_defs($cgi); > my @criteria_array; >@@ -627,8 +638,11 @@ sub delete_report { > return $sth->execute(@ids); > } > >-my $SAVED_REPORTS_BASE_QRY = <<EOQ; >-SELECT s.*, r.report, r.date_run, $AREA_NAME_SQL_SNIPPET, av_g.lib AS groupname, av_sg.lib AS subgroupname, >+sub get_saved_reports_base_query { >+ >+ my $area_name_sql_snippet = get_area_name_sql_snippet; >+ return <<EOQ; >+SELECT s.*, r.report, r.date_run, $area_name_sql_snippet, av_g.lib AS groupname, av_sg.lib AS subgroupname, > b.firstname AS borrowerfirstname, b.surname AS borrowersurname > FROM saved_sql s > LEFT JOIN saved_reports r ON r.report_id = s.id >@@ -636,7 +650,8 @@ LEFT OUTER JOIN authorised_values av_g ON (av_g.category = 'REPORT_GROUP' AND av > LEFT OUTER JOIN authorised_values av_sg ON (av_sg.category = 'REPORT_SUBGROUP' AND av_sg.lib_opac = s.report_group AND av_sg.authorised_value = s.report_subgroup) > LEFT OUTER JOIN borrowers b USING (borrowernumber) > EOQ >-my $DATE_FORMAT = "%d/%m/%Y"; >+} >+ > sub get_saved_reports { > # $filter is either { date => $d, author => $a, keyword => $kw, } > # or $keyword. Optional. >@@ -645,7 +660,7 @@ sub get_saved_reports { > my ($group, $subgroup) = @_; > > my $dbh = C4::Context->dbh(); >- my $query = $SAVED_REPORTS_BASE_QRY; >+ my $query = get_saved_reports_base_query; > my (@cond,@args); > if ($filter) { > if (my $date = $filter->{date}) { >@@ -797,14 +812,15 @@ sub save_dictionary { > return 1; > } > >-my $DICTIONARY_BASE_QRY = <<EOQ; >-SELECT d.*, $AREA_NAME_SQL_SNIPPET >-FROM reports_dictionary d >-EOQ > sub get_from_dictionary { > my ( $area, $id ) = @_; > my $dbh = C4::Context->dbh(); >- my $query = $DICTIONARY_BASE_QRY; >+ my $area_name_sql_snippet = get_area_name_sql_snippet; >+ my $query = <<EOQ; >+SELECT d.*, $area_name_sql_snippet >+FROM reports_dictionary d >+EOQ >+ > if ($area) { > $query .= " WHERE report_area = ?"; > } elsif ($id) { >-- >1.9.1
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 10126
:
32092
|
32100
|
32199
|
32409