From 4654d341fa73608d07d84b9d13d8142837ae79f9 Mon Sep 17 00:00:00 2001 From: Julian Maurice <julian.maurice@biblibre.com> Date: Fri, 14 Sep 2018 17:26:14 +0200 Subject: [PATCH] Bug 11708: Fix QA issues - Change how closedate is set in database update - Reword bookseller to vendor in Koha::Acquisition::Basketgroup - Do not "cache" baskets in Koha::Acquisition::Basketgroup - Remove useless IF in basketgroup.tt - Move JS to bottom in basketgroups.tt - Fix some links to basketgroup.pl - Remove useless use in acqui/basketgroup.pl - Remove useless sub displaybasketgroups in acqui/basketgroup.pl - Remove useless code in C4::Acquisition --- C4/Acquisition.pm | 8 +- Koha/Acquisition/Basketgroup.pm | 12 +- acqui/basketgroup.pl | 41 ------- .../data/mysql/atomicupdate/bug_11708.perl | 9 +- .../prog/en/modules/acqui/basket.tt | 2 +- .../prog/en/modules/acqui/basketgroup.tt | 10 +- .../prog/en/modules/acqui/basketgroups.tt | 104 +++++++++--------- .../prog/en/modules/acqui/parcel.tt | 4 +- 8 files changed, 76 insertions(+), 114 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index c2e065584b..881d25ac9b 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -1510,9 +1510,7 @@ sub ModReceiveOrder { $order->{datereceived} = $datereceived; $order->{invoiceid} = $invoice->{invoiceid}; $order->{orderstatus} = 'complete'; - my @columns = Koha::Acquisition::Orders->columns; - my %filtered_order = map { exists $order->{$_} ? ($_ => $order->{$_}) : () } @columns; - $new_ordernumber = Koha::Acquisition::Order->new(\%filtered_order)->store->ordernumber; + $new_ordernumber = Koha::Acquisition::Order->new($order)->store->ordernumber; if ($received_items) { foreach my $itemnumber (@$received_items) { @@ -2010,9 +2008,7 @@ sub TransferOrder { delete $order->{parent_ordernumber}; $order->{'basketno'} = $basketno; - my @columns = Koha::Acquisition::Orders->columns; - my %filtered_order = map { exists $order->{$_} ? ($_ => $order->{$_}) : () } @columns; - my $newordernumber = Koha::Acquisition::Order->new(\%filtered_order)->store->ordernumber; + my $newordernumber = Koha::Acquisition::Order->new($order)->store->ordernumber; $query = q{ UPDATE aqorders_items diff --git a/Koha/Acquisition/Basketgroup.pm b/Koha/Acquisition/Basketgroup.pm index 26df6b94d8..b65d5ed5ba 100644 --- a/Koha/Acquisition/Basketgroup.pm +++ b/Koha/Acquisition/Basketgroup.pm @@ -31,15 +31,15 @@ Koha::Acquisition::Basketgroup =head2 Methods -=head3 bookseller +=head3 vendor -Returns the basketgroup's bookseller (Koha::Acquisition::Bookseller) +Returns the basketgroup's vendor (Koha::Acquisition::Bookseller) - my $bookseller = $basketgroup->bookseller; + my $vendor = $basketgroup->vendor; =cut -sub bookseller { +sub vendor { my ($self) = @_; return scalar Koha::Acquisition::Booksellers->find($self->booksellerid); @@ -57,9 +57,9 @@ Returns the basketgroup's baskets sub baskets { my ($self) = @_; - $self->{_baskets} ||= Koha::Acquisition::Baskets->search({ basketgroupid => $self->id }); + my $baskets = Koha::Acquisition::Baskets->search({ basketgroupid => $self->id }); - return wantarray ? $self->{_baskets}->as_list : $self->{_baskets}; + return wantarray ? $baskets->as_list : $baskets; } =head3 baskets_count diff --git a/acqui/basketgroup.pl b/acqui/basketgroup.pl index c4edfe3cd2..162e747cf6 100755 --- a/acqui/basketgroup.pl +++ b/acqui/basketgroup.pl @@ -58,8 +58,6 @@ use Koha::Acquisition::Booksellers; use Koha::ItemTypes; use Koha::Patrons; -use List::MoreUtils qw/uniq/; - our $input=new CGI; our ($template, $loggedinuser, $cookie) @@ -87,40 +85,6 @@ sub BasketTotal { return $total; } -#displays all basketgroups and all closed baskets (in their respective groups) -sub displaybasketgroups { - my ($basketgroups, $bookseller, $baskets, $template) = @_; - if (scalar @$basketgroups != 0) { - foreach my $basketgroup (@$basketgroups){ - my $i = 0; - my $basketsqty = 0; - while($i < scalar(@$baskets)){ - my $basket = @$baskets[$i]; - if($basket->{'basketgroupid'} && $basket->{'basketgroupid'} == $basketgroup->{'id'}){ - $basket->{total} = BasketTotal($basket->{basketno}, $bookseller); - push(@{$basketgroup->{'baskets'}}, $basket); - splice(@$baskets, $i, 1); - ++$basketsqty; - --$i; - } - ++$i; - } - $basketgroup -> {'basketsqty'} = $basketsqty; - } - $template->param(basketgroups => $basketgroups); - } - for(my $i=0; $i < scalar @$baskets; ++$i) { - if( ! @$baskets[$i]->{'closedate'} ) { - splice(@$baskets, $i, 1); - --$i; - }else{ - @$baskets[$i]->{total} = BasketTotal(@$baskets[$i]->{basketno}, $bookseller); - } - } - $template->param(baskets => $baskets); - $template->param( booksellername => $bookseller->name); -} - sub printbasketgrouppdf{ my ($basketgroupid) = @_; @@ -403,9 +367,4 @@ $deliveryplace = $deliveryplace || $borrower->branchcode; $template->param( booksellerid => $booksellerid ); -# the template will display a unique basketgroup -my $basketgroups = &GetBasketgroups($booksellerid); -my $baskets = &GetBasketsByBookseller($booksellerid); -displaybasketgroups($basketgroups, $bookseller, $baskets, $template); - output_html_with_http_headers $input, $cookie, $template->output; diff --git a/installer/data/mysql/atomicupdate/bug_11708.perl b/installer/data/mysql/atomicupdate/bug_11708.perl index 35543cdac5..54d7e72f8b 100644 --- a/installer/data/mysql/atomicupdate/bug_11708.perl +++ b/installer/data/mysql/atomicupdate/bug_11708.perl @@ -8,9 +8,16 @@ if( CheckVersion( $DBversion ) ) { } if (column_exists('aqbasketgroups', 'closed')) { + # Set basketgroup's close date to the latest close date of its baskets, + # or NOW() if it has no baskets $dbh->do(q{ UPDATE aqbasketgroups - SET closeddate = CAST(NOW() AS DATE) + LEFT JOIN ( + SELECT basketgroupid, MAX(closedate) AS closedate + FROM aqbasket + GROUP BY basketgroupid + ) AS aqbasket ON (aqbasketgroups.id = aqbasket.basketgroupid) + SET aqbasketgroups.closeddate = COALESCE(aqbasket.closedate, CAST(NOW() AS DATE)) WHERE closed = 1 AND closeddate IS NULL }); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt index b870d835e8..ca431e283a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt @@ -807,7 +807,7 @@ $(document).ready(function(){ $("#basketgroupid").change(function(){ if($(this).val() == "new"){ - location.href="/cgi-bin/koha/acqui/basketgroup.pl?op=add&booksellerid=[% booksellerid | html %]"; + location.href="/cgi-bin/koha/acqui/basketgroups.pl?op=add&booksellerid=[% booksellerid | html %]"; } }); }); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroup.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroup.tt index 5ecb65dea9..aa1a0c46b3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroup.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroup.tt @@ -4,11 +4,7 @@ [% USE Price %] [% USE KohaDates %] [% INCLUDE 'doc-head-open.inc' %] -[% IF booksellerid %] - <title>Koha › Basket groups for [% booksellername |html %]</title> -[% ELSE %] - <title>Koha › Basket groups</title> -[% END %] +<title>Koha › Basket groups for [% booksellername |html %]</title> [% Asset.css("css/datatables.css") | $raw %] [% INCLUDE 'doc-head-close.inc' %] [% INCLUDE 'datatables.inc' %] @@ -120,7 +116,7 @@ function submitForm(form) { <div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/acqui/acqui-home.pl">Acquisitions</a> › <a href="/cgi-bin/koha/acqui/supplier.pl?booksellerid=[% booksellerid | html %]">[% booksellername | html %]</a> › - <a href="/cgi-bin/koha/acqui/basketgroup.pl?booksellerid=[% booksellerid | html %]">Basket groups</a> + <a href="/cgi-bin/koha/acqui/basketgroups.pl?booksellerid=[% booksellerid | html %]">Basket groups</a> › Add basket group for [% booksellername | html %] </div> @@ -274,7 +270,7 @@ function submitForm(form) { <input type="hidden" name="basketgroupid" value="[% basketgroupid | html %]" /> [% END %] <input type="hidden" name="op" value="attachbasket" /> - <input type="submit" value="Save" /> <a href="/cgi-bin/koha/acqui/basketgroup.pl?booksellerid=[% booksellerid | html %]" class="cancel">Cancel</a> + <input type="submit" value="Save" /> <a href="/cgi-bin/koha/acqui/basketgroups.pl?booksellerid=[% booksellerid | html %]" class="cancel">Cancel</a> </fieldset> [% END %] </form> diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroups.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroups.tt index e9258a9f23..67e7cb0b67 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroups.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basketgroups.tt @@ -2,61 +2,15 @@ [% USE Branches %] [% USE KohaDates %] +[% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] [% IF bookseller %] <title>Koha › Basket groups for [% bookseller.name |html %]</title> [% ELSE %] <title>Koha › Basket groups</title> [% END %] - - [% Asset.css('css/datatables.css') %] - [% INCLUDE 'doc-head-close.inc' %] - [% INCLUDE 'datatables.inc' %] - [% Asset.js('lib/jquery/plugins/jquery.dataTables.columnFilter.js') %] - <script type="text/javascript"> - $(document).ready(function() { - var options = { - "paging": false, - "autoWidth": false, - "columnDefs": [ - { "visible": false, "targets": 1 }, - { "orderable": false, "targets": -1 } - ], - "orderFixed": [[ 1, 'asc' ]] - }; - [% UNLESS bookseller %] - options.drawCallback = function(settings) { - var api = this.api(); - var rows = api.rows({page: 'current'}).nodes(); - var last = null; - - api.column(1, {page: 'current'}).data().each(function(group, i) { - if (last !== group) { - $(rows).eq(i).before( - '<tr><td class="group" colspan="8">' + group + '</td></tr>' - ); - last = group; - } - }); - }; - [% END %] - $("#basketgroups-table").kohaDataTable(options); - - $('#basketgroups-table').on('click', '.closeandprint', function(e) { - e.preventDefault(); - var w = window.open($(this).attr('href')); - var timer = setInterval(function() { - if (w.closed === true) { - clearInterval(timer); - window.location.reload(true); - } - }, 1000); - }); - $('#basketgroups-table').on('click', '.delete', function() { - return confirm(_("Are you sure you want to delete this basketgroup ?")); - }); - }); - </script> +[% INCLUDE 'doc-head-close.inc' %] +[% Asset.css('css/datatables.css') %] </head> <body id="acq_basketgroup" class="acq"> [% INCLUDE 'header.inc' %] @@ -113,7 +67,7 @@ Basket group no. [% basketgroup.id %] [% END %] </td> - <td>[% basketgroup.bookseller.name %]</td> + <td>[% basketgroup.vendor.name %]</td> <td>[% Branches.GetName(basketgroup.billingplace) %]</td> <td> [% IF (basketgroup.freedeliveryplace) %] @@ -160,4 +114,54 @@ [% INCLUDE 'acquisitions-menu.inc' %] </div> </div> + + [% MACRO jsinclude BLOCK %] + [% INCLUDE 'datatables.inc' %] + [% Asset.js('lib/jquery/plugins/jquery.dataTables.columnFilter.js') %] + <script type="text/javascript"> + $(document).ready(function() { + var options = { + "paging": false, + "autoWidth": false, + "columnDefs": [ + { "visible": false, "targets": 1 }, + { "orderable": false, "targets": -1 } + ], + "orderFixed": [[ 1, 'asc' ]] + }; + [% UNLESS bookseller %] + options.drawCallback = function(settings) { + var api = this.api(); + var rows = api.rows({page: 'current'}).nodes(); + var last = null; + + api.column(1, {page: 'current'}).data().each(function(group, i) { + if (last !== group) { + $(rows).eq(i).before( + '<tr><td class="group" colspan="8">' + group + '</td></tr>' + ); + last = group; + } + }); + }; + [% END %] + $("#basketgroups-table").kohaDataTable(options); + + $('#basketgroups-table').on('click', '.closeandprint', function(e) { + e.preventDefault(); + var w = window.open($(this).attr('href')); + var timer = setInterval(function() { + if (w.closed === true) { + clearInterval(timer); + window.location.reload(true); + } + }, 1000); + }); + $('#basketgroups-table').on('click', '.delete', function() { + return confirm(_("Are you sure you want to delete this basketgroup ?")); + }); + }); + </script> + [% END %] + [% INCLUDE 'intranet-bottom.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt index 3eff906ddf..244813d389 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt @@ -133,7 +133,7 @@ <td class="basketfilterclass">[% loop_order.basketname | html %] (<a href="/cgi-bin/koha/acqui/basket.pl?basketno=[% loop_order.basketno | html %]">[% loop_order.basketno | html %]</a>)</td> <td> [% IF loop_order.basketgroupid %] - [% loop_order.basketgroupname | html %] (<a href="/cgi-bin/koha/acqui/basketgroup.pl?booksellerid=[% loop_order.booksellerid | html %]">[% loop_order.basketgroupid | html %]</a>) + [% loop_order.basketgroupname | html %] (<a href="/cgi-bin/koha/acqui/basketgroup.pl?booksellerid=[% loop_order.booksellerid | html %]&basketgroupid=[% loop_order.basketgroupid | html %]">[% loop_order.basketgroupid | html %]</a>) [% ELSE %] No basket group [% END %] @@ -280,7 +280,7 @@ <td>[% order.basketname | html %] (<a href="/cgi-bin/koha/acqui/basket.pl?basketno=[% order.basketno | html %]">[% order.basketno | html %]</a>)</td> <td> [% IF order.basketgroupid %] - [% order.basketgroupname | html %] (<a href="/cgi-bin/koha/acqui/basketgroup.pl?booksellerid=[% order.booksellerid | html %]">[% order.basketgroupid | html %]</a>) + [% order.basketgroupname | html %] (<a href="/cgi-bin/koha/acqui/basketgroup.pl?booksellerid=[% order.booksellerid | html %]&basketgroupid=[% order.basketgroupid | html %]">[% order.basketgroupid | html %]</a>) [% ELSE %] No basket group [% END %] -- 2.17.1