From f25f6b3b662c55fb410f3cc86ae9fb0adccfefcd Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 25 Jul 2019 19:52:25 +0000 Subject: [PATCH] Bug 23376: Clean up order recipt page This patchset switches from using DB lookups to using an order object for most things on orderreceive.pl It simplifies the script and makes minimal changes to the template To test: 1 - Place some orders and receive them 2 - Have orders with or without subscriptions attached 3 - Try with different AcqCreateItems settings 4 - Apply patch 5 - No behaviour should change 6 - Read code to ensure things make sense --- acqui/orderreceive.pl | 114 ++++----------------- .../prog/en/modules/acqui/orderreceive.tt | 94 ++++++++++------- 2 files changed, 75 insertions(+), 133 deletions(-) diff --git a/acqui/orderreceive.pl b/acqui/orderreceive.pl index 0b9ef232b6..fb08d0a5a9 100755 --- a/acqui/orderreceive.pl +++ b/acqui/orderreceive.pl @@ -42,8 +42,6 @@ to know on what supplier this script has to display receive order. the id of this invoice. -=item freight - =item biblio The biblionumber of this order. @@ -85,14 +83,10 @@ my $dbh = C4::Context->dbh; my $invoiceid = $input->param('invoiceid'); my $invoice = GetInvoice($invoiceid); my $booksellerid = $invoice->{booksellerid}; -my $freight = $invoice->{shipmentcost}; my $ordernumber = $input->param('ordernumber'); my $bookseller = Koha::Acquisition::Booksellers->find( $booksellerid ); -my $results; -$results = SearchOrders({ - ordernumber => $ordernumber -}) if $ordernumber; +my $order = Koha::Acquisition::Orders->find( $ordernumber ); my ( $template, $loggedinuser, $cookie, $userflags ) = get_template_and_user( { @@ -105,15 +99,13 @@ my ( $template, $loggedinuser, $cookie, $userflags ) = get_template_and_user( } ); -unless ( $results and @$results) { +unless ( $order ) { output_html_with_http_headers $input, $cookie, $template->output; exit; } # prepare the form for receiving -my $order = $results->[0]; -my $order_object = Koha::Acquisition::Orders->find( $ordernumber ); -my $basket = $order_object->basket; +my $basket = $order->basket; my $active_currency = Koha::Acquisition::Currencies->get_active; # Check if ACQ framework exists @@ -130,89 +122,38 @@ if ($AcqCreateItem eq 'receiving') { ); } elsif ($AcqCreateItem eq 'ordering') { my $fw = ($acq_fw) ? 'ACQ' : ''; - my $items = $order_object->items; - my @items; - while ( my $i = $items->next ) { - my $item = $i->unblessed; - my $descriptions; - $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => $fw, kohafield => 'items.notforloan', authorised_value => $item->{notforloan} }); - $item->{notforloan} = $descriptions->{lib} // ''; - - $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => $fw, kohafield => 'items.restricted', authorised_value => $item->{restricted} }); - $item->{restricted} = $descriptions->{lib} // ''; - - $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => $fw, kohafield => 'items.location', authorised_value => $item->{location} }); - $item->{location} = $descriptions->{lib} // ''; - - $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => $fw, kohafield => 'items.ccode', authorised_value => $item->{ccode} }); - $item->{collection} = $descriptions->{lib} // ''; - - $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => $fw, kohafield => 'items.materials', authorised_value => $item->{materials} }); - $item->{materials} = $descriptions->{lib} // ''; - - my $itemtype = Koha::ItemTypes->find($i->effective_itemtype); - if (defined $itemtype) { - # We should not do that here, but call ->itemtype->description when needed instead - $item->{itemtype} = $itemtype->description; # FIXME Should not it be translated_description? - } - push @items, $item; - } - $template->param(items => \@items); -} - -$order->{quantityreceived} = '' if $order->{quantityreceived} == 0; - -my $unitprice = $order->{unitprice}; -my ( $rrp, $ecost ); -if ( $bookseller->invoiceincgst ) { - $rrp = $order->{rrp_tax_included}; - $ecost = $order->{ecost_tax_included}; - unless ( $unitprice != 0 and defined $unitprice) { - $unitprice = $order->{ecost_tax_included}; - } -} else { - $rrp = $order->{rrp_tax_excluded}; - $ecost = $order->{ecost_tax_excluded}; - unless ( $unitprice != 0 and defined $unitprice) { - $unitprice = $order->{ecost_tax_excluded}; - } -} - -my $tax_rate; -if( defined $order->{tax_rate_on_receiving} ) { - $tax_rate = $order->{tax_rate_on_receiving} + 0.0; -} else { - $tax_rate = $order->{tax_rate_on_ordering} + 0.0; + my $items = $order->items; + $template->param(items => $items); } my $suggestion = GetSuggestionInfoFromBiblionumber($order->{biblionumber}); -my $creator = Koha::Patrons->find( $order->{created_by} ); +my $creator = Koha::Patrons->find( $order->created_by ); -my $budget = GetBudget( $order->{budget_id} ); +my $budget = GetBudget( $order->budget_id ); -my $datereceived = $order->{datereceived} ? dt_from_string( $order->{datereceived} ) : dt_from_string; +my $datereceived = $order->datereceived ? dt_from_string( $order->datereceived ) : dt_from_string; # get option values for gist syspref my @gst_values = map { option => $_ + 0.0 }, split( '\|', C4::Context->preference("gist") ); -my $order_internalnote = $order->{order_internalnote}; -my $order_vendornote = $order->{order_vendornote}; -if ( $order->{subscriptionid} ) { +my $order_internalnote = $order->order_internalnote; +my $order_vendornote = $order->order_vendornote; +if ( $order->subscriptionid ) { # Order from a subscription, we will display an history of what has been received my $orders = Koha::Acquisition::Orders->search( { - subscriptionid => $order->{subscriptionid}, - parent_ordernumber => $order->{ordernumber}, - ordernumber => { '!=' => $order->{ordernumber} } + subscriptionid => $order->subscriptionid, + parent_ordernumber => $order->ordernumber, + ordernumber => { '!=' => $order->ordernumber } } ); - if ( $order->{parent_ordernumber} != $order->{ordernumber} ) { - my $parent_order = Koha::Acquisition::Orders->find($order->{parent_ordernumber}); - $order_internalnote = $parent_order->{order_internalnote}; - $order_vendornote = $parent_order->{order_vendornote}; + if ( $order->parent_ordernumber != $order->ordernumber ) { + my $parent_order = Koha::Acquisition::Orders->find($order->parent_ordernumber); + $order_internalnote = $parent_order->order_internalnote; + $order_vendornote = $parent_order->order_vendornote; } $template->param( orders => $orders, @@ -222,29 +163,12 @@ if ( $order->{subscriptionid} ) { $template->param( AcqCreateItem => $AcqCreateItem, count => 1, - biblionumber => $order->{'biblionumber'}, - ordernumber => $order->{'ordernumber'}, - subscriptionid => $order->{subscriptionid}, - booksellerid => $order->{'booksellerid'}, - freight => $freight, + order => $order, name => $bookseller->name, cur_active_sym => $active_currency->symbol, cur_active => $active_currency->currency, invoiceincgst => $bookseller->invoiceincgst, - title => $order->{'title'}, - author => $order->{'author'}, - copyrightdate => $order->{'copyrightdate'}, - isbn => $order->{'isbn'}, - seriestitle => $order->{'seriestitle'}, bookfund => $budget->{budget_name}, - quantity => $order->{'quantity'}, - quantityreceivedplus1 => $order->{'quantityreceived'} + 1, - quantityreceived => $order->{'quantityreceived'}, - rrp => $rrp, - replacementprice => $order->{'replacementprice'}, - ecost => $ecost, - unitprice => $unitprice, - tax_rate => $tax_rate, creator => $creator, invoiceid => $invoice->{invoiceid}, invoice => $invoice->{invoicenumber}, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt index bf940446cf..7c9fe7b4cf 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt @@ -2,6 +2,8 @@ [% USE Asset %] [% USE KohaDates %] [% USE Branches %] +[% USE AuthorisedValues %] +[% USE ItemTypes %] [% USE Price %] [% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] @@ -13,14 +15,14 @@ [% INCLUDE 'header.inc' %] [% INCLUDE 'acquisitions-search.inc' %] - +
-

Receive items from : [% name | html %] [% IF ( invoice ) %][[% invoice | html %]] [% END %] (order #[% ordernumber | html %])

+

Receive items from : [% name | html %] [% IF ( invoice ) %][[% invoice | html %]] [% END %] (order #[% order.ordernumber | html %])

[% IF ( count ) %]
@@ -30,15 +32,15 @@
Catalog details -
  1. Title: [% title | html %]
  2. +
    1. Title: [% order.biblio.title | html %]
    2. Author: - [% author | html %]
    3. + [% order.biblio.author | html %]
    4. Copyright: - [% copyrightdate | html %]
    5. + [% order.biblio.copyrightdate | html %]
    6. ISBN: - [% isbn | html %]
    7. + [% order.biblio.biblioitem.isbn | html %]
    8. Series: - [% seriestitle | html %]
    9. + [% order.biblio.seriestitle | html %]
@@ -54,7 +56,7 @@ [% END %] - [% IF subscriptionid and orders.count %] + [% IF order.subscriptionid and orders.count %]
Receipt history for this subscription @@ -138,7 +140,7 @@ - [% UNLESS subscriptionid %] + [% UNLESS order.subscriptionid %]
Item [% IF ( NoACQframework ) %] @@ -152,7 +154,7 @@
[% END %] [% ELSIF (AcqCreateItem == 'ordering') %] - [% IF (items.size) %] + [% IF (items.count) %]
Items
@@ -183,15 +185,15 @@ - - - + + + - - - + + + [% END %] @@ -200,10 +202,10 @@ [% END %] [% END %] - + - - + +
@@ -214,7 +216,7 @@
  • + [% IF edit or order.subscriptionid %] + [% ELSE%] - + [% END %]
  • - [% IF subscriptionid %] - - + [% IF order.subscriptionid %] + + [% ELSIF AcqCreateItemReceiving %] [% ELSE %] - [% IF ( quantityreceived ) %] + [% IF ( order.quantityreceived ) %] [% IF ( edit ) %] - - + + [% ELSE %] [% IF ( items ) %] - + [% ELSE %] - + [% END %] - + [% END %] [% ELSE %] @@ -268,13 +270,14 @@

    Warning, you have entered more items than expected. Items will not be created.

  • - [% END %][%# IF (subscriptionid) ELSIF (AcqCreateItemReceiving) %] + [% END %][%# IF (order.subscriptionid) ELSIF (AcqCreateItemReceiving) %] [% IF ( gst_values ) %]
  • [% END %] -
  • [% rrp | $Price %] (adjusted for [% cur_active | html %], [% IF (invoiceincgst == 1) %]tax inclusive[% ELSE %]tax exclusive[% END %])
  • +
  • + [% IF (invoiceincgst == 1) %] + [% order.rrp_tax_included | $Price %](adjusted for [% cur_active | html %],tax inclusive)
  • + [% ELSE %] + [% order.rrp_tax_excluded | $Price %](adjusted for [% cur_active | html %],tax exclusive) + [% END %]
  • - +
  • -
  • [% ecost | $Price %] [% IF (invoiceincgst == 1) %](tax inclusive)[% ELSE %](tax exclusive)[% END %]
  • +
  • + [% IF (invoiceincgst) %] + [% order.ecost_tax_included | $Price %] (tax inclusive) + [% ELSE %] + [% order.ecost_tax_excluded | $Price %] (tax exclusive) + [% END %] +
  • - [% IF (invoiceincgst == 1) %](tax inclusive)[% ELSE %](tax exclusive)[% END %] + [% IF (invoiceincgst) %] + (tax inclusive) + [% ELSE %] + (tax inclusive) + [% END %]
  • [% IF order_vendornote %] @@ -420,7 +438,7 @@ $(document).ready(function() { [% IF (AcqCreateItemReceiving) %] cloneItemBlock(0, '[% UniqueItemFields | html %]'); - [% ELSIF (AcqCreateItem == 'ordering') && not subscriptionid %] + [% ELSIF (AcqCreateItem == 'ordering') && not order.subscriptionid %] $("input[name='items_to_receive']").change(function() { CalcQtyToReceive(); }); -- 2.11.0
    [% item.barcode | html %] [% Branches.GetName( item.homebranch ) | html %] [% Branches.GetName( item.holdingbranch ) | html %][% item.notforloan | html %][% item.restricted | html %][% item.location | html %][% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.notforloan', authorised_value => item.notforloan ) | html %][% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.restricted', authorised_value => item.restricted ) | html %][% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.location ) | html %] [% item.itemcallnumber | html %] [% item.copynumber | html %] [% item.stocknumber | html %][% item.collection | html %][% item.itemtype | html %][% item.materials | html %][% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode', authorised_value => item.ccode ) | html %][% ItemTypes.GetDescription( item.itype ) | html %][% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.materials', authorised_value => item.materials ) | html %] [% item.itemnotes | html %]