Bug 38838: Clean C4::Reports::Guided->get_columns

reports/dictionary.tt and reports/guided_reports_start.tt display a list
of the columns available using optgroup to group the columns per table.

The template code is quite ugly because of the perl structure choose in
C4::Reports::Guided->get_columns

This patch removes $cgi and the 'first' variable that were not used.
Now we have an usual structure with the table names as keys and column
info in an arrayref.

Test plan:
Create a report and a new dictionary definition.
At step 3 you should see the list of the columns.
Note that they are now identical. The list when you create a dictionary
now show the description of the columns.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Jonathan Druart 2025-01-07 10:23:22 +01:00 committed by Katrin Fischer
parent 72ad69926d
commit 10a41979e6
Signed by: kfischer
GPG key ID: 0EF6E2C03357A834
6 changed files with 72 additions and 70 deletions

View file

@ -191,42 +191,37 @@ This will return a list of all columns for a report area
=cut
sub get_columns {
# this calls the internal function _get_columns
my ( $area, $cgi ) = @_;
my ($area) = @_;
my %table_areas = get_table_areas;
my $tables = $table_areas{$area}
or die qq{Unsuported report area "$area"};
my $tables = $table_areas{$area}
or die qq{Unsupported report area "$area"};
my @allcolumns;
my $first = 1;
foreach my $table (@$tables) {
my @columns = _get_columns($table,$cgi, $first);
$first = 0;
push @allcolumns, @columns;
my $columns;
for my $table (@$tables) {
$columns->{$table} = _get_columns($table);
}
return ( \@allcolumns );
return $columns;
}
sub _get_columns {
my ($tablename,$cgi, $first) = @_;
my $dbh = C4::Context->dbh();
my $sth = $dbh->prepare("show columns from $tablename");
$sth->execute();
my @columns;
my $columns = Koha::Database::Columns->columns;
my %tablehash;
$tablehash{'table'}=$tablename;
$tablehash{'__first__'} = $first;
push @columns, \%tablehash;
while ( my $data = $sth->fetchrow_arrayref() ) {
my %temphash;
$temphash{'name'} = "$tablename.$data->[0]";
$temphash{'description'} = $columns->{$tablename}->{$data->[0]};
push @columns, \%temphash;
}
$sth->finish();
return (@columns);
my ($table_name) = @_;
my $dbh = C4::Context->dbh();
my $all_columns = Koha::Database::Columns->columns;
# Sanitize input, just in case
die sprintf( 'Invalid table name %s', $table_name ) unless exists $all_columns->{$table_name};
my $columns = $dbh->selectall_arrayref( sprintf( q{SHOW COLUMNS FROM %s}, $table_name ), { Slice => {} } );
return [
map {
my $column_name = $_->{Field};
{
name => sprintf( "%s.%s", $table_name, $column_name ),
description => $all_columns->{$table_name}->{$column_name},
};
} @$columns
];
}
=head2 build_query($columns,$criteria,$orderby,$area)

View file

@ -191,25 +191,20 @@
<input type="hidden" name="definition_name" value="[% definition_name | html %]" />
<input type="hidden" name="definition_description" value="[% definition_description | html %]" />
<select id="availableColumns" name="columns" size="25" style="width:200px;height:300px;margin:1em;">
[% FOREACH column IN columns %]
[% IF ( column.table ) %]
[% IF ( loop.first ) %]
[% ELSE %]
</optgroup>
[% END %]
<optgroup label="[% column.table | html %]">
[% ELSE %]
<option value="[% column.name | html %]">
[% IF ( column.description ) %][% column.description | html %]
[% ELSE %]
[% column.name | html %]
[% END %]
</option>
[% END %]
[% END %]
</optgroup>
<select id="availableColumns" name="columns" size="25" style="min-width:200px; height:300px; margin:1em;">
[% FOREACH table IN columns.keys.sort %]
<optgroup label="[% table | html %]">
[% FOREACH column IN columns.item(table) %]
<option value="[% column.name | html %]">
[% IF ( column.description ) %]
[% column.description | html %] &nbsp; / &nbsp; [% column.name | html %]
[% ELSE %]
[% column.name | html %]
[% END %]
</option>
[% END %]
</optgroup>
[% END %]
</select>
<input type="hidden" name="op" value="cud-add_form_4" />

View file

@ -546,25 +546,20 @@
<div class="row">
<div class="col-sm-6">
<div style="float: left;">
<select id="availableColumns" name="oldcolumns2" multiple="multiple" size="25" style="min-width: 200px;height:300px;">
[% FOREACH column IN columns %]
[% IF ( column.table ) %]
[% IF ( loop.first ) %]
[% ELSE %]
</optgroup>
<select id="availableColumns" name="oldcolumns2" multiple="multiple" size="25" style="min-width:200px; height:300px;">
[% FOREACH table IN columns.keys.sort %]
<optgroup label="[% table | html %]">
[% FOREACH column IN columns.item(table) %]
<option value="[% column.name | html %]">
[% IF ( column.description ) %]
[% column.description | html %] &nbsp; / &nbsp; [% column.name | html %]
[% ELSE %]
[% column.name | html %]
[% END %]
</option>
[% END %]
<optgroup label="[% column.table | html %]">
[% ELSE %]
<option value="[% column.name | html %]">
[% IF ( column.description ) %]
[% column.description | html %] &nbsp; / &nbsp; [% column.name | html %]
[% ELSE %]
[% column.name | html %]
[% END %]
</option>
[% END %]
</optgroup>
[% END %]
</optgroup>
</select>
</div>
<div style="width: 6.3em; float: right; margin-top: 100px">

View file

@ -67,11 +67,10 @@ elsif ( $op eq 'cud-add_form_2' ) {
elsif ( $op eq 'cud-add_form_3' ) {
# Choosing the columns
my $columns = get_columns( $area, $input );
$template->param(
'step_3' => 1,
'area' => $area,
'columns' => $columns,
'columns' => get_columns($area),
'definition_name' => $definition_name,
'definition_description' => $definition_description,
);

View file

@ -313,7 +313,7 @@ elsif ( $op eq 'cud-choose_type' ) {
'build3' => 1,
'area' => $area,
'type' => $type,
columns => get_columns($area,$input),
columns => get_columns($area),
'cache_expiry' => scalar $input->param('cache_expiry'),
'public' => scalar $input->param('public'),
);

View file

@ -20,7 +20,7 @@
use Modern::Perl;
use Test::More tests => 12;
use Test::More tests => 13;
use Test::Warn;
use t::lib::TestBuilder;
@ -535,6 +535,24 @@ subtest 'Returning passwords tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'get_columns' => sub {
plan tests => 7;
$schema->storage->txn_begin;
my $area = 'foo';
my $success = eval { C4::Reports::Guided::get_columns($area) };
ok !$success && $@ =~ m{^Unsupported report area "$area"}, 'Call with wrong report area explodes';
my $columns = C4::Reports::Guided::get_columns('CAT');
ok exists $columns->{biblio};
ok exists $columns->{biblioitems};
ok exists $columns->{items};
ok scalar grep { $_->{name} eq 'biblio.biblionumber' } @{ $columns->{biblio} };
ok scalar grep { $_->{description} =~ m{Biblio number} } @{ $columns->{biblio} };
ok !scalar grep { $_->{name} eq 'not_a_column' } @{ $columns->{biblio} };
$schema->storage->txn_rollback;
};
sub trim {
my ($s) = @_;
$s =~ s/^\s*(.*?)\s*$/$1/s;