From fcafd2c08a7be85b664a6318489d45c0190a83ce Mon Sep 17 00:00:00 2001 From: Jesse Weaver <jweaver@bywatersolutions.com> Date: Mon, 11 Jan 2016 16:07:33 -0700 Subject: [PATCH] Bug 15531: (followup) Use a quantity of 1, not null, for standing orders This seems to cause fewer problems with the existing acquisitions code. Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com> --- C4/Acquisition.pm | 2 +- Koha/Acquisition/Order.pm | 11 +++-- .../intranet-tmpl/prog/en/modules/acqui/basket.tt | 2 +- .../prog/en/modules/acqui/neworderempty.tt | 46 +++++++++----------- t/db_dependent/Acquisition/NewOrder.t | 5 +-- t/db_dependent/Acquisition/StandingOrders.t | 18 +------- 6 files changed, 31 insertions(+), 53 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 19ecb6b..c091f89 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -1410,7 +1410,7 @@ q{SELECT *, aqbasket.is_standing FROM aqorders LEFT JOIN aqbasket USING (basketn my $sth = $dbh->prepare($query); $sth->execute( - ( $order->{quantity} < $quantrec ? 0 : ( $order->{quantity} - $quantrec ) ), + ( $order->{is_standing} ? 1 : ( $order->{quantity} - $quantrec ) ), ( defined $order_internalnote ? $order_internalnote : () ), ( defined $order_vendornote ? $order_vendornote : () ), $ordernumber diff --git a/Koha/Acquisition/Order.pm b/Koha/Acquisition/Order.pm index de0c49d..ede8ddf 100644 --- a/Koha/Acquisition/Order.pm +++ b/Koha/Acquisition/Order.pm @@ -26,17 +26,16 @@ sub fetch { sub insert { my ($self) = @_; + my $schema = Koha::Database->new->schema; + # Override quantity for standing orders + $self->{quantity} = 1 if ( $self->{basketno} && $schema->resultset('Aqbasket')->find( $self->{basketno} )->is_standing ); + # if these parameters are missing, we can't continue - for my $key (qw( basketno biblionumber budget_id )) { + for my $key (qw( basketno quantity biblionumber budget_id )) { croak "Cannot insert order: Mandatory parameter $key is missing" unless $self->{$key}; } - my $schema = Koha::Database->new->schema; - if ( !$self->{quantity} && !$schema->resultset('Aqbasket')->find( $self->{basketno} )->is_standing ) { - croak "Cannot insert order: Quantity is mandatory for non-standing orders"; - } - $self->{quantityreceived} ||= 0; $self->{entrydate} ||= output_pref( { dt => dt_from_string, dateformat => 'iso' } ); 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 96f75f7..200d9b9 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt @@ -519,7 +519,7 @@ <td class="number gste [% IF books_loo.ecostgste.search(zero_regex) %]error[% END %]">[% books_loo.ecostgste | $Price%]</td> <td class="number gsti [% IF books_loo.rrpgsti.search(zero_regex) %]error[% END %]">[% books_loo.rrpgsti | $Price %]</td> <td class="number gsti [% IF books_loo.ecostgsti.search(zero_regex) %]error[% END %]">[% books_loo.ecostgsti | $Price %]</td> - <td class="number [% IF books_loo.quantity.search(zero_regex) %]error[% END %]">[% IF is_standing and books_loo.quantity == 0 %]N/A[% ELSE %][% books_loo.quantity or 'N/A' %][% END %]</td> + <td class="number [% IF books_loo.quantity.search(zero_regex) %]error[% END %]">[% books_loo.quantity %]</td> <td class="number gste [% IF books_loo.totalgste.search(zero_regex) %]error[% END %]">[% books_loo.totalgste | $Price %]</td> <td class="number gsti [% IF books_loo.totalgsti.search(zero_regex) %]error[% END %]">[% books_loo.totalgsti | $Price %]</td> <td class="number">[% books_loo.gstrate * 100 | $Price %]</td> diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt index 6779109..c01641d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt @@ -43,12 +43,10 @@ function Check(ff) { _alertString += "\n- "+ _("You must select a fund"); } -[% UNLESS basket.is_standing %] if (!(isNum(ff.quantity,0)) || ff.quantity.value == 0){ ok=1; _alertString += "\n- " + _("Quantity must be greater than '0'"); } -[% END %] if (!(isNum(ff.listprice,0))){ ok=1; @@ -415,7 +413,7 @@ $(document).ready(function() </fieldset> [% END %] - [% UNLESS subscriptionid %][% # it is a suggestion, we have not items %] + [% UNLESS subscriptionid || basket.is_standing %][% # it is a suggestion, we have not items %] [% IF (AcqCreateItemOrdering) %] <div id="items_list" style="display:none"> @@ -461,32 +459,30 @@ $(document).ready(function() <fieldset class="rows"> <legend>Accounting details</legend> <ol> - [% UNLESS basket.is_standing %] - <li> - [% IF ( close ) %] - <span class="label required">Quantity: </span> - <input type="hidden" name="quantity" value="[% quantity %]" />[% quantity %] + <li> + [% IF ( close ) %] + <span class="label required">Quantity: </span> + <input type="hidden" name="quantity" value="[% quantity %]" />[% quantity %] + [% ELSE %] + <label class="required" for="quantity">Quantity: </label> + [% IF (AcqCreateItemOrdering) %] + [% IF subscriptionid || basket.is_standing %] + <input type="text" readonly="readonly" size="20" id="quantity" name="quantity" value="1" /> + [% ELSE %] + <input type="text" readonly="readonly" size="20" id="quantity" name="quantity" value="0" /> + [% END %] [% ELSE %] - <label class="required" for="quantity">Quantity: </label> - [% IF (AcqCreateItemOrdering) %] - [% IF subscriptionid %] - <input type="text" readonly="readonly" size="20" id="quantity" name="quantity" value="1" /> - [% ELSE %] - <input type="text" readonly="readonly" size="20" id="quantity" name="quantity" value="0" /> - [% END %] + [% IF subscriptionid || basket.is_standing %] + <input type="text" readonly="readonly" size="20" id="quantity" name="quantity" value="1" /> [% ELSE %] - [% IF subscriptionid %] - <input type="text" readonly="readonly" size="20" id="quantity" name="quantity" value="1" /> - [% ELSE %] - <input type="text" size="20" id="quantity" name="quantity" value="[% quantityrec %]" onchange="updateCosts();" /> - [% END %] + <input type="text" size="20" id="quantity" name="quantity" value="[% quantityrec %]" onchange="updateCosts();" /> [% END %] - <span class="required">Required</span> [% END %] - <!-- origquantityrec only here for javascript compatibility (additem.js needs it, useless here, usefull when receiveing an order --> - <input id="origquantityrec" readonly="readonly" type="hidden" name="origquantityrec" value="1" /> - </li> - [% END %] + <span class="required">Required</span> + [% END %] + <!-- origquantityrec only here for javascript compatibility (additem.js needs it, useless here, usefull when receiveing an order --> + <input id="origquantityrec" readonly="readonly" type="hidden" name="origquantityrec" value="1" /> + </li> <li> [% IF ( close ) %] <span class="label required">Fund: </span> diff --git a/t/db_dependent/Acquisition/NewOrder.t b/t/db_dependent/Acquisition/NewOrder.t index 53279ff..b491075 100644 --- a/t/db_dependent/Acquisition/NewOrder.t +++ b/t/db_dependent/Acquisition/NewOrder.t @@ -67,9 +67,6 @@ foreach my $mandatoryparams_key (@mandatoryparams_keys) { }; $return_error = $@; my $expected_error = "Cannot insert order: Mandatory parameter $mandatoryparams_key is missing"; - if ( $mandatoryparams_key eq 'quantity' ) { - $expected_error = "Cannot insert order: Quantity is mandatory for non-standing orders"; - } ok( ( !( defined $order ) ) && ( index( $return_error, $expected_error ) >= 0 ), @@ -90,4 +87,4 @@ $order = Koha::Acquisition::Order->fetch({ ordernumber => $ordernumber }); is( $order->{quantityreceived}, 0, 'Koha::Acquisition::Order->insert set quantityreceivedto 0 if undef is given' ); is( $order->{entrydate}, output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }), 'Koha::Acquisition::Order->insert set entrydate to today' ); -$schema->storage->txn_rollback(); +$schema->storage->txn_rollback(); \ No newline at end of file diff --git a/t/db_dependent/Acquisition/StandingOrders.t b/t/db_dependent/Acquisition/StandingOrders.t index 61c7d24..32f1b46 100644 --- a/t/db_dependent/Acquisition/StandingOrders.t +++ b/t/db_dependent/Acquisition/StandingOrders.t @@ -2,7 +2,7 @@ use Modern::Perl; -use Test::More tests => 14; +use Test::More tests => 13; use C4::Context; use C4::Acquisition; use C4::Biblio; @@ -68,20 +68,6 @@ my $ordernumber = Koha::Acquisition::Order->new( isnt( $ordernumber, undef, 'standing order successfully created' ); -eval { - Koha::Acquisition::Order->new( - { - basketno => $nonstandingbasketno, - biblionumber => $biblionumber, - budget_id => $budget->{budget_id}, - currency => 'USD', - quantity => 0, - } - )->insert; -}; - -like( $@, qr/quantity/im, 'normal orders cannot be created without quantity' ); - my $search_orders = SearchOrders( { basketno => $basketno, pending => 1, @@ -117,7 +103,7 @@ my $order = Koha::Acquisition::Order->fetch( { ordernumber => $ordernumber } ); my $neworder = Koha::Acquisition::Order->fetch( { ordernumber => $new_ordernumber } ); is( $order->{orderstatus}, 'partial', 'original order set to partially received' ); -is( $order->{quantity}, 0, 'original order quantity unchanged' ); +is( $order->{quantity}, 1, 'original order quantity unchanged' ); is( $order->{quantityreceived}, 0, 'original order has no received items' ); isnt( $order->{unitprice}, 12, 'original order does not get cost' ); is( $neworder->{orderstatus}, 'complete', 'new order set to complete' ); -- 1.7.10.4