From a7f23c0afca81de0b22252650a9a23c8d8b731d9 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 I'm unsure if this is needed, but I have verified it causes no regressions Signed-off-by: Paul Poulain Signed-off-by: Tomas Cohen Arazi No regressions spotted. I'd prefer this code to be fully refactored of course :-P --- 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 a8eb1d08f1..b782407bf5 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 = < $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 = <dbh(); - my $query = $DICTIONARY_BASE_QRY; + my $area_name_sql_snippet = get_area_name_sql_snippet; + my $query = <