From 82626e1382f839d1cc589c3a8dcede3c53d22580 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 24 Oct 2023 10:20:39 +0200 Subject: [PATCH] Bug 34913: Adjust C4::Utils::DataTables::VirtualShelves And remove C4::Utils::DataTables, which should no longer be reused anyway. --- C4/Utils/DataTables.pm | 154 ------------------ C4/Utils/DataTables/VirtualShelves.pm | 57 ++++--- .../prog/en/modules/virtualshelves/shelves.tt | 78 ++++----- .../virtualshelves/tables/shelves_results.tt | 26 +-- svc/virtualshelves/search | 23 ++- t/db_dependent/Utils/Datatables.t | 61 ------- .../Utils/Datatables_Virtualshelves.t | 28 ++-- 7 files changed, 102 insertions(+), 325 deletions(-) delete mode 100644 C4/Utils/DataTables.pm delete mode 100755 t/db_dependent/Utils/Datatables.t diff --git a/C4/Utils/DataTables.pm b/C4/Utils/DataTables.pm deleted file mode 100644 index ac2faad7cd6..00000000000 --- a/C4/Utils/DataTables.pm +++ /dev/null @@ -1,154 +0,0 @@ -package C4::Utils::DataTables; - -# Copyright 2011 BibLibre -# -# This file is part of Koha. -# -# Koha is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# Koha is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Koha; if not, see . - -use Modern::Perl; -require Exporter; - -use vars qw(@ISA @EXPORT); - -BEGIN { - - @ISA = qw(Exporter); - @EXPORT = qw(dt_build_orderby dt_get_params); -} - -=head1 NAME - -! DEPRECATED - This module is deprecated, the REST API route and REST API Datatables wrapper must be used instead! - -C4::Utils::DataTables - Utility subs for building query when DataTables source is AJAX - -=head1 SYNOPSYS - - use CGI qw ( -utf8 ); - use C4::Context; - use C4::Utils::DataTables; - - my $input = new CGI; - my $vars = $input->Vars; - - my $query = qq{ - SELECT surname, firstname - FROM borrowers - WHERE borrowernumber = ? - }; - $query .= dt_build_orderby($vars); - $query .= " LIMIT ?,? "; - - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare($query); - $sth->execute( - $vars->{'borrowernumber'}, - @$having_params, - $vars->{'iDisplayStart'}, - $vars->{'iDisplayLength'} - ); - ... - -=head1 DESCRIPTION - - This module provide two utility functions to build a part of the SQL query, - depending on DataTables parameters. - One function build the 'ORDER BY' part, and the other the 'HAVING' part. - -=head1 FUNCTIONS - -=head2 dt_build_orderby - - my $orderby = dt_build_orderby($dt_param); - This function takes a reference to a hash containing DataTables parameters - and build the corresponding 'ORDER BY' clause. - This hash must contains the following keys: - iSortCol_N, where N is a number from 0 to the number of columns to sort on minus 1 - sSortDir_N is the sorting order ('asc' or 'desc) for the corresponding column - mDataProp_N is a mapping between the column index, and the name of a SQL field - -=cut - -sub dt_build_orderby { - my $param = shift; - - my $i = 0; - my @orderbys; - my $dbh = C4::Context->dbh; - while(exists $param->{'iSortCol_'.$i}){ - my $iSortCol = $param->{'iSortCol_'.$i}; - my $sSortDir = $param->{'sSortDir_'.$i}; - my $mDataProp = $param->{'mDataProp_'.$iSortCol}; - my @sort_fields = $param->{$mDataProp.'_sorton'} - ? split(' ', $param->{$mDataProp.'_sorton'}) - : (); - if(@sort_fields > 0) { - push @orderbys, "$_ $sSortDir" foreach (@sort_fields); - } else { - push @orderbys, "$mDataProp $sSortDir"; - } - $i++; - } - - return unless @orderbys; - - my @sanitized_orderbys; - - # Trick for virtualshelves, should not be extended - push @sanitized_orderbys, 'count asc' if grep {$_ eq 'count asc'} @orderbys; - push @sanitized_orderbys, 'count desc' if grep {$_ eq 'count desc'} @orderbys; - - # Must be "branches.branchname asc", "borrowers.firstname desc", etc. - @orderbys = grep { /^\w+\.\w+ (asc|desc)$/ } @orderbys; - - for my $orderby (@orderbys) { - my ($identifier, $direction) = split / /, $orderby, 2; - my ($table, $column) = split /\./, $identifier, 2; - my $sanitized_identifier = $dbh->quote_identifier(undef, $table, $column); - my $sanitized_direction = $direction eq 'asc' ? 'ASC' : 'DESC'; - push @sanitized_orderbys, "$sanitized_identifier $sanitized_direction"; - } - - return " ORDER BY " . join(',', @sanitized_orderbys) . " " if @sanitized_orderbys; -} - -=head2 dt_get_params - - my %dtparam = = dt_get_params( $input ) - This function takes a reference to a new CGI object. - It prepares a hash containing Datatable parameters. - -=cut -sub dt_get_params { - my $input = shift; - my %dtparam; - my $vars = $input->Vars; - - foreach(qw/ iDisplayStart iDisplayLength iColumns sSearch bRegex iSortingCols sEcho /) { - $dtparam{$_} = $input->param($_); - } - foreach(grep /(?:_sorton|_filteron)$/, keys %$vars) { - $dtparam{$_} = $vars->{$_}; - } - for(my $i=0; $i<$dtparam{'iColumns'}; $i++) { - foreach(qw/ bSearchable sSearch bRegex bSortable iSortCol mDataProp sSortDir /) { - my $key = $_ . '_' . $i; - $dtparam{$key} = $input->param($key) if defined $input->param($key); - } - } - return %dtparam; -} - -1; diff --git a/C4/Utils/DataTables/VirtualShelves.pm b/C4/Utils/DataTables/VirtualShelves.pm index b27fdcbe59e..8100a65591d 100644 --- a/C4/Utils/DataTables/VirtualShelves.pm +++ b/C4/Utils/DataTables/VirtualShelves.pm @@ -2,7 +2,6 @@ package C4::Utils::DataTables::VirtualShelves; use Modern::Perl; use C4::Context; -use C4::Utils::DataTables qw( dt_build_orderby ); use Koha::Virtualshelves; sub search { @@ -13,13 +12,15 @@ sub search { my $sortby = $params->{sortby}; my $public = $params->{public} // 1; $public = $public ? 1 : 0; - my $dt_params = $params->{dt_params}; + my $order_by = $params->{order_by}; + my $start = $params->{start}; + my $length = $params->{length}; # If not logged in user, be carreful and set the borrowernumber to 0 # to prevent private lists lack my $loggedinuser = C4::Context->userenv->{'number'} || 0; - my ($iTotalRecords, $iTotalDisplayRecords); + my ($recordsTotal, $recordsFiltered); my $dbh = C4::Context->dbh; @@ -62,8 +63,8 @@ sub search { } if ( defined $owner and $owner ne '' ) { push @where_strs, '( bo.firstname LIKE ? OR bo.surname LIKE ? )'; - push @args, "%$owner%", "%$owner%"; - } + push @args, "%$owner%", "%$owner%"; +} if ( defined $sortby and $sortby ne '' ) { push @where_strs, 'sortfield = ?'; push @args, $sortby; @@ -79,18 +80,32 @@ sub search { my $where; $where = " WHERE " . join (" AND ", @where_strs) if @where_strs; - my $orderby = dt_build_orderby($dt_params); - $orderby =~ s|shelfnumber|vs.shelfnumber| if $orderby; + + + if ($order_by) { + my @order_by; + $order_by =~ s|shelfnumber|vs.shelfnumber|; + my @sanitized_orderbys; + for my $order ( split ',', $order_by ) { + my ( $identifier, $direction ) = split / /, $order, 2; + my ( $table, $column ) = split /\./, $identifier, 2; + my $sanitized_identifier = $dbh->quote_identifier( undef, $table, $column ); + my $sanitized_direction = $direction eq 'asc' ? 'ASC' : 'DESC'; + push @sanitized_orderbys, "$sanitized_identifier $sanitized_direction"; + } + + $order_by = ' ORDER BY ' . join( ',', @sanitized_orderbys ); + } my $limit; - # If iDisplayLength == -1, we want to display all shelves - if ( $dt_params->{iDisplayLength} > -1 ) { + # If length == -1, we want to display all shelves + if ( $length > -1 ) { # In order to avoid sql injection - $dt_params->{iDisplayStart} =~ s/\D//g; - $dt_params->{iDisplayLength} =~ s/\D//g; - $dt_params->{iDisplayStart} //= 0; - $dt_params->{iDisplayLength} //= 20; - $limit = "LIMIT $dt_params->{iDisplayStart},$dt_params->{iDisplayLength}"; + $start =~ s/\D//g; + $length =~ s/\D//g; + $start //= 0; + $length //= 20; + $limit = sprintf "LIMIT %s,%s", $start, $length; } my $group_by = " GROUP BY vs.shelfnumber, vs.shelfname, vs.owner, vs.public, @@ -102,20 +117,20 @@ sub search { $from, ($where ? $where : ""), $group_by, - ($orderby ? $orderby : ""), + ($order_by ? $order_by : ""), ($limit ? $limit : "") ); my $shelves = $dbh->selectall_arrayref( $query, { Slice => {} }, @args ); - # Get the iTotalDisplayRecords DataTable variable + # Get the recordsFiltered DataTable variable $query = "SELECT COUNT(vs.shelfnumber) " . $from_total . ($where ? $where : ""); - ($iTotalDisplayRecords) = $dbh->selectrow_array( $query, undef, @args ); + ($recordsFiltered) = $dbh->selectrow_array( $query, undef, @args ); - # Get the iTotalRecords DataTable variable + # Get the recordsTotal DataTable variable $query = q|SELECT COUNT(vs.shelfnumber)| . $from_total . q| WHERE public = ?|; $query .= q| AND (vs.owner = ? OR sh.borrowernumber = ?)| if !$public; @args = !$public ? ( $loggedinuser, $public, $loggedinuser, $loggedinuser ) : ( $public ); - ( $iTotalRecords ) = $dbh->selectrow_array( $query, undef, @args ); + ( $recordsTotal ) = $dbh->selectrow_array( $query, undef, @args ); for my $shelf ( @$shelves ) { my $s = Koha::Virtualshelves->find( $shelf->{shelfnumber} ); @@ -124,8 +139,8 @@ sub search { $shelf->{is_shared} = $s->is_shared; } return { - iTotalRecords => $iTotalRecords, - iTotalDisplayRecords => $iTotalDisplayRecords, + recordsTotal => $recordsTotal, + recordsFiltered => $recordsFiltered, shelves => $shelves, } } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt index 6a1e3bd9081..d668024df3e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt @@ -622,57 +622,35 @@ $(document).ready(function(){ var public = [% public | html %]; + let sorton = [ + 'vs.shelfname', + 'count', + 'vs.public', + 'vs.owner', + 'vs.sortfield', + 'vs.created_on', + 'vs.lastmodified', + ]; + var dtListResults = $("#listresultst").dataTable($.extend(true, {}, dataTablesDefaults, { - "order": [[ 5, "asc" ]], - "serverSide": true, - "ajax": "/cgi-bin/koha/svc/virtualshelves/search", - 'fnServerData': function(sSource, aoData, fnCallback) { - aoData.push({ - 'name': 'public', - 'value': public, - },{ - 'name': 'shelfname', - 'value': $("#searchshelfname_filter").val(), - },{ - 'name': 'owner', - 'value': $("#searchowner_filter").val(), - },{ - 'name': 'sortby', - 'value': $("#searchsortby_filter").val(), - },{ - 'name': 'template_path', - 'value': 'virtualshelves/tables/shelves_results.tt', - },{ - 'name': 'allow_transfer', - 'value': '[% allow_transfer | html %]', - },{ - 'name': 'shelfname_sorton', - 'value': 'vs.shelfname', - },{ - 'name': 'is_shared_sorton', - 'value': 'vs.public', - },{ - 'name': 'owner_sorton', - 'value': 'vs.owner', - },{ - 'name': 'sortby_sorton', - 'value': 'vs.sortfield', - },{ - 'name': 'created_on_sorton', - 'value': 'vs.created_on', - },{ - 'name': 'modification_time_sorton', - 'value': 'vs.lastmodified', - }); - $.ajax({ - 'dataType': 'json', - 'type': 'POST', - 'url': sSource, - 'data': aoData, - 'success': function(json){ - fnCallback(json); - } - }); + order: [[ 5, "asc" ]], + serverSide: true, + ajax: { + url: "/cgi-bin/koha/svc/virtualshelves/search", + type: "POST", + data: function ( d ) { + let order_by = []; + d.order.forEach((o, i) => order_by.push(sorton[o.column - 1] + " " + o.dir)); + return $.extend( {}, d, { + public, + order_by: order_by.join(','), + shelfname: $("#searchshelfname_filter").val(), + owner: $("#searchowner_filter").val(), + sortby: $("#searchsortby_filter").val(), + template_path: 'virtualshelves/tables/shelves_results.tt', + allow_transfer: '[% allow_transfer | html %]', + }); + } }, 'columns':[ { "data": 'dt_public' }, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/shelves_results.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/shelves_results.tt index 48dbeb9bc7e..a25ee522592 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/shelves_results.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/tables/shelves_results.tt @@ -3,30 +3,30 @@ [% USE To %] [% PROCESS 'i18n.inc' %] { - "sEcho": [% sEcho | html %], - "iTotalRecords": [% iTotalRecords | html %], - "iTotalDisplayRecords": [% iTotalDisplayRecords | html %], + "draw": [% draw | html %], + "recordsTotal": [% recordsTotal | html %], + "recordsFiltered": [% recordsFiltered | html %], "data": [ - [% FOREACH data IN aaData %] + [% FOREACH d IN data %] { "dt_public": - "[% data.public | html %]", + "[% d.public | html %]", "dt_shelfname": - "[% data.shelfname | html | $To %]", + "[% d.shelfname | html | $To %]", "dt_count": - "[% tnx('{count} item', '{count} items', count, { count = data.count }) | $raw %]", + "[% tnx('{count} item', '{count} items', count, { count = d.count }) | $raw %]", "dt_is_shared": - "[% IF data.public %]Public[% ELSIF data.is_shared %]Shared[% ELSE %]Private[% END %]", + "[% IF d.public %]Public[% ELSIF d.is_shared %]Shared[% ELSE %]Private[% END %]", "dt_owner": - "[% data.firstname | html | $To %] [% data.surname | html | $To %]", + "[% d.firstname | html | $To %] [% d.surname | html | $To %]", "dt_sortby": - [% IF data.sortby == "author" %]"Author"[% ELSIF data.sortby == "copyrightdate" %]"Year"[% ELSIF data.sortby == "itemcallnumber" %]"Call number"[% ELSIF data.sortby == "dateadded" %]"Date added"[% ELSE %]"Title"[% END %], + [% IF d.sortby == "author" %]"Author"[% ELSIF d.sortby == "copyrightdate" %]"Year"[% ELSIF d.sortby == "itemcallnumber" %]"Call number"[% ELSIF d.sortby == "dateadded" %]"Date added"[% ELSE %]"Title"[% END %], "dt_created_on": - "[% data.created_on | $KohaDates %]", + "[% d.created_on | $KohaDates %]", "dt_modification_time": - "[% data.modification_time | $KohaDates %]", + "[% d.modification_time | $KohaDates %]", "dt_action": - "[% PROCESS action_form shelfnumber=data.shelfnumber can_manage_shelf=data.can_manage_shelf can_delete_shelf=data.can_delete_shelf type=data.type %]" + "[% PROCESS action_form shelfnumber=d.shelfnumber can_manage_shelf=d.can_manage_shelf can_delete_shelf=d.can_delete_shelf type=d.type %]" }[% UNLESS loop.last %],[% END %] [% END %] ] diff --git a/svc/virtualshelves/search b/svc/virtualshelves/search index 5d172562fe9..edb366d921b 100755 --- a/svc/virtualshelves/search +++ b/svc/virtualshelves/search @@ -5,7 +5,6 @@ use CGI; use C4::Auth qw( get_template_and_user ); use C4::Output qw( output_with_http_headers ); -use C4::Utils::DataTables qw( dt_get_params ); use C4::Utils::DataTables::VirtualShelves qw( search ); my $input = CGI->new; @@ -25,14 +24,12 @@ my $owner = $input->param('owner'); my $public = $input->param('public'); my $sortby = $input->param('sortby'); my $allow_transfer = $input->param('allow_transfer'); +my $order_by = $input->param('order_by'); +my $start = $input->param('start'); +my $length = $input->param('length'); # variable information for DataTables (id) -my $sEcho = $input->param('sEcho'); - -my %dt_params = dt_get_params($input); -foreach (grep {$_ =~ /^mDataProp/} keys %dt_params) { - $dt_params{$_} =~ s/^dt_//; -} +my $draw = $input->param('draw'); my $results = C4::Utils::DataTables::VirtualShelves::search( { @@ -41,15 +38,17 @@ my $results = C4::Utils::DataTables::VirtualShelves::search( owner => $owner, public => $public, sortby => $sortby, - dt_params => \%dt_params, + order_by => $order_by, + start => $start, + length => $length, } ); $template->param( - sEcho => $sEcho, - iTotalRecords => $results->{iTotalRecords}, - iTotalDisplayRecords => $results->{iTotalDisplayRecords}, - aaData => $results->{shelves}, + draw => $draw, + recordsTotal => $results->{recordsTotal}, + recordsFiltered => $results->{recordsFiltered}, + data => $results->{shelves}, public => $public, allow_transfer => $allow_transfer, ); diff --git a/t/db_dependent/Utils/Datatables.t b/t/db_dependent/Utils/Datatables.t deleted file mode 100755 index 409dd6ecf6b..00000000000 --- a/t/db_dependent/Utils/Datatables.t +++ /dev/null @@ -1,61 +0,0 @@ -#!/usr/bin/perl - -# This file is part of Koha. -# -# Koha is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# Koha is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Koha; if not, see . - -use Modern::Perl; - -use Test::More tests => 1; - -use C4::Utils::DataTables qw( dt_build_orderby ); - -use t::lib::Mocks; -use t::lib::TestBuilder; - -use Koha::Database; - -my $schema = Koha::Database->new->schema; -$schema->storage->txn_begin; - -my $builder = t::lib::TestBuilder->new; - -subtest 'dt_build_orderby' => sub { - plan tests => 2; - - my $dt_params = { - iSortCol_0 => 5, - sSortDir_0 => "asc", - mDataProp_5 => "branch", - name_sorton => "borrowers.surname borrowers.firstname", - - iSortCol_1 => 2, - sSortDir_1 => "desc", - mDataProp_2 => "name", - branch_sorton => "branches.branchname", - }; - - my $orderby = dt_build_orderby($dt_params); - is( $orderby, " ORDER BY `branches`.`branchname` ASC,`borrowers`.`surname` DESC,`borrowers`.`firstname` DESC ", 'ORDER BY has been correctly built' ); - - $dt_params = { - %$dt_params, - iSortCol_2 => 3, - sSortDir_2 => "asc", - mDataProp_3 => "branch,somethingelse", - }; - - $orderby = dt_build_orderby($dt_params); - is( $orderby, " ORDER BY `branches`.`branchname` ASC,`borrowers`.`surname` DESC,`borrowers`.`firstname` DESC ", 'ORDER BY has been correctly built, even with invalid stuff'); -}; diff --git a/t/db_dependent/Utils/Datatables_Virtualshelves.t b/t/db_dependent/Utils/Datatables_Virtualshelves.t index 183ed92224d..de4398a7c73 100755 --- a/t/db_dependent/Utils/Datatables_Virtualshelves.t +++ b/t/db_dependent/Utils/Datatables_Virtualshelves.t @@ -177,8 +177,8 @@ for my $i ( 6 .. 15 ) { # Set common datatables params my %dt_params = ( - iDisplayLength => 10, - iDisplayStart => 0 + length => 10, + start => 0 ); my $search_results; @@ -187,14 +187,14 @@ t::lib::Mocks::mock_userenv({ patron => $john_doe_patron }); # Search private lists by title $search_results = C4::Utils::DataTables::VirtualShelves::search({ shelfname => "ist", - dt_params => \%dt_params, + %dt_params, public => 0, }); -is( $search_results->{ iTotalRecords }, 2, +is( $search_results->{ recordsTotal }, 2, "There should be 2 private shelves in total" ); -is( $search_results->{ iTotalDisplayRecords }, 2, +is( $search_results->{ recordsFiltered }, 2, "There should be 2 private shelves with title like '%ist%" ); is( @{ $search_results->{ shelves } }, 2, @@ -202,13 +202,13 @@ is( @{ $search_results->{ shelves } }, 2, # Search by type only $search_results = C4::Utils::DataTables::VirtualShelves::search({ - dt_params => \%dt_params, + %dt_params, public => 1, }); -is( $search_results->{ iTotalRecords }, 12, +is( $search_results->{ recordsTotal }, 12, "There should be 12 public shelves in total" ); -is( $search_results->{ iTotalDisplayRecords }, 12, +is( $search_results->{ recordsFiltered }, 12, "There should be 12 private shelves" ); is( @{ $search_results->{ shelves } }, 10, @@ -217,13 +217,13 @@ is( @{ $search_results->{ shelves } }, 10, # Search by owner $search_results = C4::Utils::DataTables::VirtualShelves::search({ owner => "jane", - dt_params => \%dt_params, + %dt_params, public => 1, }); -is( $search_results->{ iTotalRecords }, 12, +is( $search_results->{ recordsTotal }, 12, "There should be 12 public shelves in total" ); -is( $search_results->{ iTotalDisplayRecords }, 2, +is( $search_results->{ recordsFiltered }, 2, "There should be 1 public shelves for jane" ); is( @{ $search_results->{ shelves } }, 2, @@ -233,13 +233,13 @@ is( @{ $search_results->{ shelves } }, 2, $search_results = C4::Utils::DataTables::VirtualShelves::search({ owner => "smith", shelfname => "public list 1", - dt_params => \%dt_params, + %dt_params, public => 1, }); -is( $search_results->{ iTotalRecords }, 12, +is( $search_results->{ recordsTotal }, 12, "There should be 12 public shelves in total" ); -is( $search_results->{ iTotalDisplayRecords }, 6, +is( $search_results->{ recordsFiltered }, 6, "There should be 6 public shelves for john with name like %public list 1%" ); is( @{ $search_results->{ shelves } }, 6, -- 2.34.1