Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 23376: Clean up order receipt page code

Browse source 
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

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Nick Clemens 2019-07-25 19:52:25 +00:00 committed by Jonathan Druart
parent d7c58c10f1
commit 5d4740dfb0
2 changed files with 86 additions and 114 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

106
acqui/orderreceive.pl
View file

@ -89,30 +89,26 @@ 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(
{
template_name => "acqui/orderreceive.tt",
query => $input,
type => "intranet",
authnotrequired => 0,
flagsrequired => {acquisition => 'order_receive'},
debug => 1,
}
);
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 $currencies = Koha::Acquisition::Currencies->search;
my $active_currency = $currencies->get_active;
@ -130,87 +126,61 @@ 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);
my $items = $order->items;
$template->param(items => $items);
}
$order->{quantityreceived} = '' if $order->{quantityreceived} == 0;
$order->quantityreceived = '' if $order->quantityreceived == 0;
my $unitprice = $order->{unitprice};
my $unitprice = $order->unitprice;
my ( $rrp, $ecost );
if ( $bookseller->invoiceincgst ) {
$rrp = $order->{rrp_tax_included};
$ecost = $order->{ecost_tax_included};
$rrp = $order->rrp_tax_included;
$ecost = $order->ecost_tax_included;
unless ( $unitprice != 0 and defined $unitprice) {
$unitprice = $order->{ecost_tax_included};
$unitprice = $order->ecost_tax_included;
}
} else {
$rrp = $order->{rrp_tax_excluded};
$ecost = $order->{ecost_tax_excluded};
$rrp = $order->rrp_tax_excluded;
$ecost = $order->ecost_tax_excluded;
unless ( $unitprice != 0 and defined $unitprice) {
$unitprice = $order->{ecost_tax_excluded};
$unitprice = $order->ecost_tax_excluded;
}
}
my $tax_rate;
if( defined $order->{tax_rate_on_receiving} ) {
$tax_rate = $order->{tax_rate_on_receiving} + 0.0;
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;
$tax_rate = $order->tax_rate_on_ordering + 0.0;
}
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,
@ -220,29 +190,15 @@ if ( $order->{subscriptionid} ) {
$template->param(
AcqCreateItem => $AcqCreateItem,
count => 1,
biblionumber => $order->{'biblionumber'},
ordernumber => $order->{'ordernumber'},
subscriptionid => $order->{subscriptionid},
booksellerid => $order->{'booksellerid'},
order => $order,
freight => $freight,
name => $bookseller->name,
active_currency => $active_currency,
currencies => scalar $currencies->search({ rate => { '!=' => 1 } }),
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},

94
koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt
View file

@ -3,6 +3,7 @@
[% USE KohaDates %]
[% USE Branches %]
[% USE AuthorisedValues %]
[% USE ItemTypes %]
[% USE Price %]
[% SET footerjs = 1 %]
[% INCLUDE 'doc-head-open.inc' %]
@ -14,14 +15,14 @@
[% INCLUDE 'header.inc' %]
[% INCLUDE 'acquisitions-search.inc' %]
<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/acqui/acqui-home.pl">Acquisitions</a> &rsaquo; <a href="/cgi-bin/koha/acqui/supplier.pl?booksellerid=[% booksellerid | html %]">[% name | html %]</a> &rsaquo; Receive items from : [% name | html %] [% IF ( invoice ) %][[% invoice | html %]][% END %] (order #[% ordernumber | html %])</div>
<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/acqui/acqui-home.pl">Acquisitions</a> &rsaquo; <a href="/cgi-bin/koha/acqui/supplier.pl?booksellerid=[% order.basket.booksellerid | html %]">[% name | html %]</a> &rsaquo; Receive items from : [% name | html %] [% IF ( invoice ) %][[% invoice | html %]][% END %] (order #[% order.ordernumber | html %])</div>
<div class="main container-fluid">
<div class="row">
<div class="col-sm-10 col-sm-push-2">
<main>
<h1>Receive items from : [% name | html %] [% IF ( invoice ) %][[% invoice | html %]] [% END %] (order #[% ordernumber | html %])</h1>
<h1>Receive items from : [% name | html %] [% IF ( invoice ) %][[% invoice | html %]] [% END %] (order #[% order.ordernumber | html %])</h1>
[% IF ( count ) %]
<form action="/cgi-bin/koha/acqui/finishreceive.pl" class="noEnterSubmit" method="post" onsubmit="return Check(this);">
@ -31,15 +32,15 @@
<fieldset class="rows">
<legend>Catalog details</legend>
<ol><li><span class="label">Title: </span><span class="title">[% title | html %]</span></li>
<ol><li><span class="label">Title: </span><span class="title">[% order.biblio.title | html %]</span></li>
<li> <span class="label">Author: </span>
[% author | html %]</li>
[% order.biblio.author | html %]</li>
<li><span class="label">Copyright: </span>
[% copyrightdate | html %]</li>
[% order.biblio.copyrightdate | html %]</li>
<li> <span class="label">ISBN: </span>
[% isbn | html %]</li>
[% order.biblio.biblioitem.isbn | html %]</li>
<li> <span class="label">Series: </span>
[% seriestitle | html %]</li>
[% order.biblio.seriestitle | html %]</li>
</ol>
</fieldset>
@ -85,7 +86,7 @@
</fieldset>
[% END %]
[% IF subscriptionid and orders.count %]
[% IF order.subscriptionid and orders.count %]
<fieldset class="rows">
<legend>Receipt history for this subscription</legend>
<table id="orders">
@ -169,7 +170,7 @@
</div>
</div>
[% UNLESS subscriptionid %]
[% UNLESS order.subscriptionid %]
<fieldset class="rows" id="itemfieldset">
<legend>Item</legend>
[% IF ( NoACQframework ) %]
@ -183,7 +184,7 @@
</fieldset>
[% END %]
[% ELSIF (AcqCreateItem == 'ordering') %]
[% IF (items.size) %]
[% IF (items.count) %]
<h5>Items</h5>
<div style="width:100%;overflow:auto">
<table>
@ -214,15 +215,15 @@
<td>[% item.barcode | html %]</td>
<td>[% Branches.GetName( item.homebranch ) | html %]</td>
<td>[% Branches.GetName( item.holdingbranch ) | html %]</td>
<td>[% item.notforloan | html %]</td>
<td>[% item.restricted | html %]</td>
<td><span class="shelvingloc">[% item.location | html %]</span></td>
<td>[% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.notforloan', authorised_value => item.notforloan ) | html %]</td>
<td>[% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.restricted', authorised_value => item.restricted ) | html %]</td>
<td><span class="shelvingloc">[% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.location ) | html %]</span></td>
<td>[% item.itemcallnumber | html %]</td>
<td>[% item.copynumber | html %]</td>
<td>[% item.stocknumber | html %]</td>
<td>[% item.collection | html %]</td>
<td>[% item.itemtype | html %]</td>
<td>[% item.materials | html %]</td>
<td>[% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode', authorised_value => item.ccode ) | html %]</td>
<td>[% ItemTypes.GetDescription( item.itype ) | html %]</td>
<td>[% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.materials', authorised_value => item.materials ) | html %]</td>
<td>[% item.itemnotes | html %]</td>
</tr>
[% END %]
@ -231,10 +232,10 @@
</div>
[% END %]
[% END %]
<input type="hidden" name="biblionumber" value="[% biblionumber | html %]" />
<input type="hidden" name="biblionumber" value="[% order.biblionumber | html %]" />
<input type="hidden" name="invoiceid" value="[% invoiceid | html %]" />
<input type="hidden" name="ordernumber" value="[% ordernumber | html %]" />
<input type="hidden" name="booksellerid" value="[% booksellerid | html %]" />
<input type="hidden" name="ordernumber" value="[% order.ordernumber | html %]" />
<input type="hidden" name="booksellerid" value="[% order.basket.booksellerid | html %]" />
</div>
<div class="col-sm-6">
<fieldset class="rows">
@ -245,7 +246,7 @@
<input type="text" size="10" id="datereceived" name="datereceived" value="[% datereceived | $KohaDates %]" class="datepicker" />
</li>
<li><label for="bookfund">Fund: </label><select id="bookfund" name="bookfund">
<option value="">Keep current ([% budget_period_description | html %] - [% bookfund | html %])</option>
<option value="">Keep current ([% budget_period_description | html %] - [% order.fund.budget_name | html %])</option>
[% FOREACH period IN budget_loop %]
<optgroup label="[% period.description | html %]">
[% FOREACH fund IN period.funds %]
@ -266,30 +267,30 @@
</span>
</li>
<li><label for="quantity_to_receive">Quantity ordered: </label><span class="label">
[% IF edit or subscriptionid %]
<input type="text" id="quantity_to_receive" name="quantity" value="[% quantity | html %]" />
[% IF edit or order.subscriptionid %]
<input type="text" id="quantity_to_receive" name="quantity" value="[% order.quantity | html %]" />
[% ELSE%]
<input type="text" readonly="readonly" id="quantity_to_receive" name="quantity" value="[% quantity | html %]" />
<input type="text" readonly="readonly" id="quantity_to_receive" name="quantity" value="[% order.quantity | html %]" />
[% END %]
</span></li>
<li><label for="quantity">Quantity received: </label>
[% IF subscriptionid %]
<input type="text" size="20" name="quantityrec" id="quantity" value="[% quantity | html %]" />
<input id="origquantityrec" readonly="readonly" type="hidden" name="origquantityrec" value="[% quantityreceived | html %]" />
[% IF order.subscriptionid %]
<input type="text" size="20" name="quantityrec" id="quantity" value="[% order.quantity | html %]" />
<input id="origquantityrec" readonly="readonly" type="hidden" name="origquantityrec" value="[% order.quantityreceived | html %]" />
[% ELSIF AcqCreateItemReceiving %]
<input readonly="readonly" type="text" size="20" name="quantityrec" id="quantity" value="0" />
[% ELSE %]
[% IF ( quantityreceived ) %]
[% IF ( order.quantityreceived ) %]
[% IF ( edit ) %]
<input type="text" size="20" name="quantityrec" id="quantity" value="[% quantityreceived | html %]" />
<input id="origquantityrec" readonly="readonly" type="hidden" name="origquantityrec" value="[% quantityreceived | html %]" />
<input type="text" size="20" name="quantityrec" id="quantity" value="[% order.quantityreceived | html %]" />
<input id="origquantityrec" readonly="readonly" type="hidden" name="origquantityrec" value="[% order.quantityreceived | html %]" />
[% ELSE %]
[% IF ( items ) %]
<input readonly="readonly" type="text" size="20" name="quantityrec" id="quantity" value="[% quantityreceivedplus1 | html %]" />
<input readonly="readonly" type="text" size="20" name="quantityrec" id="quantity" value="[% order.quantityreceived + 1 | html %]" />
[% ELSE %]
<input type="text" size="20" name="quantityrec" id="quantity" value="[% quantityreceivedplus1 | html %]" />
<input type="text" size="20" name="quantityrec" id="quantity" value="[% quantityreceived + 1 | html %]" />
[% END %]
<input id="origquantityrec" readonly="readonly" type="hidden" name="origquantityrec" value="[% quantityreceived | html %]" />
<input id="origquantityrec" readonly="readonly" type="hidden" name="origquantityrec" value="[% order.quantityreceived | html %]" />
[% END %]
[% ELSE %]
<input type="text" id="quantity" size="20" name="quantityrec" value="1" />
@ -299,13 +300,14 @@
<p class="error">Warning, you have entered more items than expected.
Items will not be created.</p>
</div>
[% END %][%# IF (subscriptionid) ELSIF (AcqCreateItemReceiving) %]
[% END %][%# IF (order.subscriptionid) ELSIF (AcqCreateItemReceiving) %]
</li>
[% IF ( gst_values ) %]
<li>
<label for="tax_rate">Tax rate: </label>
<select name="tax_rate" id="tax_rate">
[% tax_rate = order.tax_rate_on_receiving || order.tax_rate_on_ordering %]
[% FOREACH gst IN gst_values %]
[% IF gst.option == tax_rate %]
<option value="[% gst.option | html %]" selected="selected">[% gst.option * 100 | html %]%</option>
@ -319,16 +321,30 @@
<input type="hidden" name="tax_rate" value="0" />
[% END %]
<li><label for="rrp">Retail price: </label>[% rrp | $Price %] <span class="hint">(adjusted for [% active_currency.currency | html %], [% IF (invoiceincgst == 1) %]tax inclusive[% ELSE %]tax exclusive[% END %])</span></li>
<li><label for="rrp">Retail price: </label>
[% IF (invoiceincgst == 1) %]
[% order.rrp_tax_included | $Price %]<span class="hint">(adjusted for [% active_currency.currency | html %],tax inclusive)</span></li>
[% ELSE %]
[% order.rrp_tax_excluded | $Price %]<span class="hint">(adjusted for [% active_currency.currency | html %],tax exclusive)</span></li>
[% END %]
<li>
<label for="replacementprice">Replacement price:</label>
<input type="text" size="20" name="replacementprice" id="replacementprice" value="[% replacementprice | $Price on_editing => 1 %]" />
<input type="text" size="20" name="replacementprice" id="replacementprice" value="[% order.replacementprice | $Price on_editing => 1 %]" />
</li>
<li><label for="ecost">Budgeted cost: </label>[% ecost | $Price %] <span class="hint">[% IF (invoiceincgst == 1) %](tax inclusive)[% ELSE %](tax exclusive)[% END %]</span></li>
<li>
[% IF (invoiceincgst) %]
<label for="ecost">Budgeted cost: </label>[% order.ecost_tax_included | $Price %] <span class="hint">(tax inclusive)</span>
[% ELSE %]
<label for="ecost">Budgeted cost: </label>[% order.ecost_tax_excluded | $Price %] <span class="hint">(tax exclusive)</span>
[% END %]
</li>
<li>
<label for="unitprice">Actual cost:</label>
<input type="text" size="20" name="unitprice" id="unitprice" value="[% unitprice | $Price on_editing => 1 %]" />
<span class="hint">[% IF (invoiceincgst == 1) %](tax inclusive)[% ELSE %](tax exclusive)[% END %]</span>
[% IF (invoiceincgst) %]
<input type="text" size="20" name="unitprice" id="unitprice" value="[% order.unitprice_tax_included | $Price on_editing => 1 %]" /> <span class="hint">(tax inclusive)</span>
[% ELSE %]
<input type="text" size="20" name="unitprice" id="unitprice" value="[% order.unitprice_tax_excluded | $Price on_editing => 1 %]" /> <span class="hint">(tax exclusive)</span>
[% END %]
<label style="font-weight: inherit; float:none;"><input type="checkbox" name="change_currency">Change currency</label>
</li>
<li id="select_currency">
@ -464,7 +480,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();
});