From ed79b8e461968a01ae8cbae9325c3d03f7c6a6c3 Mon Sep 17 00:00:00 2001 From: Julian Maurice 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 40389ed4e4..4f761d2de2 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -1512,9 +1512,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) { @@ -2012,9 +2010,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 ed9c38d714..70c3e5cccf 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) = @_; @@ -425,9 +389,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 ab91cb2c70..a027de736e 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 90becdfb32..e470411dc6 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 %] - Koha › Basket groups for [% booksellername |html %] -[% ELSE %] - Koha › Basket groups -[% END %] +Koha › Basket groups for [% booksellername |html %] [% Asset.css("css/datatables.css") | $raw %] [% INCLUDE 'doc-head-close.inc' %] [% INCLUDE 'datatables.inc' %] @@ -120,7 +116,7 @@ function submitForm(form) { @@ -276,7 +272,7 @@ function submitForm(form) { [% END %] - Cancel + Cancel [% END %] 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 %] Koha › Basket groups for [% bookseller.name |html %] [% ELSE %] Koha › Basket groups [% END %] - - [% Asset.css('css/datatables.css') %] - [% INCLUDE 'doc-head-close.inc' %] - [% INCLUDE 'datatables.inc' %] - [% Asset.js('lib/jquery/plugins/jquery.dataTables.columnFilter.js') %] - +[% INCLUDE 'doc-head-close.inc' %] +[% Asset.css('css/datatables.css') %] [% INCLUDE 'header.inc' %] @@ -113,7 +67,7 @@ Basket group no. [% basketgroup.id %] [% END %] - [% basketgroup.bookseller.name %] + [% basketgroup.vendor.name %] [% Branches.GetName(basketgroup.billingplace) %] [% IF (basketgroup.freedeliveryplace) %] @@ -160,4 +114,54 @@ [% INCLUDE 'acquisitions-menu.inc' %] + + [% MACRO jsinclude BLOCK %] + [% INCLUDE 'datatables.inc' %] + [% Asset.js('lib/jquery/plugins/jquery.dataTables.columnFilter.js') %] + + [% 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 7b3e958045..06d9a499d4 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 @@ [% loop_order.basketname | html %] ([% loop_order.basketno | html %]) [% IF loop_order.basketgroupid %] - [% loop_order.basketgroupname | html %] ([% loop_order.basketgroupid | html %]) + [% loop_order.basketgroupname | html %] ([% loop_order.basketgroupid | html %]) [% ELSE %] No basket group [% END %] @@ -280,7 +280,7 @@ [% order.basketname | html %] ([% order.basketno | html %]) [% IF order.basketgroupid %] - [% order.basketgroupname | html %] ([% order.basketgroupid | html %]) + [% order.basketgroupname | html %] ([% order.basketgroupid | html %]) [% ELSE %] No basket group [% END %] -- 2.17.1