Bugzilla – Attachment 31161 Details for
Bug 12826
GST / VAT rewrite - plumbing
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12827: NewOrder should not return basketno
Bug-12827-NewOrder-should-not-return-basketno.patch (text/plain), 18.75 KB, created by
Jonathan Druart
on 2014-08-26 15:20:20 UTC
(
hide
)
Description:
Bug 12827: NewOrder should not return basketno
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2014-08-26 15:20:20 UTC
Size:
18.75 KB
patch
obsolete
>From 82968f174ea3758354a33af454b081e94bda5caa Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Tue, 26 Aug 2014 16:06:10 +0200 >Subject: [PATCH] Bug 12827: NewOrder should not return basketno > >Since the basketno parameter is needed to insert an order, it is useless >to return it. > >This patch changes the prototype for the C4::Acquisition::NewOrder >subroutine. The return value is now a scalar containing the ordernumber >created. > >http://bugs.koha-community.org/show_bug.cgi?id=12826 >--- > C4/Acquisition.pm | 44 +++++++++++----------- > acqui/addorder.pl | 2 +- > acqui/addorderiso2709.pl | 2 +- > t/db_dependent/Acquisition.t | 10 ++--- > t/db_dependent/Acquisition/CancelReceipt.t | 4 +- > .../Acquisition/GetBasketsInfosByBookseller.t | 4 +- > .../Acquisition/GetOrdersByBiblionumber.t | 6 +-- > t/db_dependent/Acquisition/Invoices.t | 6 +-- > t/db_dependent/Acquisition/OrderFromSubscription.t | 2 +- > t/db_dependent/Acquisition/TransferOrder.t | 2 +- > t/db_dependent/Acquisition/close_reopen_basket.t | 4 +- > t/db_dependent/Bookseller.t | 35 +++++++++-------- > t/db_dependent/Budgets.t | 4 +- > 13 files changed, 62 insertions(+), 63 deletions(-) > >diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm >index 0cff235..1bd7f0d 100644 >--- a/C4/Acquisition.pm >+++ b/C4/Acquisition.pm >@@ -31,6 +31,7 @@ use C4::Debug; > use C4::SQLHelper qw(InsertInTable UpdateInTable); > use C4::Bookseller qw(GetBookSellerFromId); > use C4::Templates qw(gettemplate); >+use Koha::DateUtils qw( dt_from_string output_pref ); > > use Time::localtime; > use HTML::Entities; >@@ -1264,13 +1265,9 @@ The following keys are used: "biblionumber", "title", "basketno", "quantity", "o > sub NewOrder { > my $orderinfo = shift; > >- my $dbh = C4::Context->dbh; >- my @params; >- >- > # if these parameters are missing, we can't continue >- for my $key (qw/basketno quantity biblionumber budget_id/) { >- croak "Mandatory parameter $key missing" unless $orderinfo->{$key}; >+ for my $key ( qw( basketno quantity biblionumber budget_id ) ) { >+ croak "Cannot insert order: Mandatory parameter $key is missing" unless $orderinfo->{$key}; > } > > if ( defined $orderinfo->{subscription} && $orderinfo->{'subscription'} eq 'yes' ) { >@@ -1278,25 +1275,29 @@ sub NewOrder { > } else { > $orderinfo->{'subscription'} = 0; > } >- $orderinfo->{'entrydate'} ||= C4::Dates->new()->output("iso"); >- if (!$orderinfo->{quantityreceived}) { >- $orderinfo->{quantityreceived} = 0; >- } > >- my $ordernumber=InsertInTable("aqorders",$orderinfo); >- if (not $orderinfo->{parent_ordernumber}) { >- my $sth = $dbh->prepare(" >- UPDATE aqorders >- SET parent_ordernumber = ordernumber >- WHERE ordernumber = ? >- "); >- $sth->execute($ordernumber); >+ $orderinfo->{entrydate} ||= output_pref({ dt => dt_from_string, dateformat => 'iso'}); >+ >+ $orderinfo->{quantityreceived} ||= 0; >+ >+ # get only the columns of Aqorder >+ my $schema = Koha::Database->new()->schema; >+ my $columns = ' '.join(' ', $schema->source('Aqorder')->columns).' '; >+ my $new_order = { map { $columns =~ / $_ / ? ($_ => $orderinfo->{$_}) : () } keys(%$orderinfo) }; >+ >+ my $rs = $schema->resultset('Aqorder')->create($new_order); >+ my $ordernumber = $rs->id; >+ >+ unless ( $new_order->{parent_ordernumber} ) { >+ $rs->update({ parent_ordernumber => $ordernumber }); > } >- return ( $orderinfo->{'basketno'}, $ordernumber ); >+ >+ return $ordernumber; > } > > > >+ > #------------------------------------------------------------# > > =head3 NewOrderItem >@@ -1535,8 +1536,7 @@ q{SELECT * FROM aqorders WHERE biblionumber=? AND aqorders.ordernumber=?}, > $order->{'rrp'} = $rrp; > $order->{ecost} = $ecost; > $order->{'orderstatus'} = 'complete'; >- my $basketno; >- ( $basketno, $new_ordernumber ) = NewOrder($order); >+ $new_ordernumber = NewOrder($order); > > if ($received_items) { > foreach my $itemnumber (@$received_items) { >@@ -1913,7 +1913,7 @@ sub TransferOrder { > delete $order->{parent_ordernumber}; > $order->{'basketno'} = $basketno; > my $newordernumber; >- (undef, $newordernumber) = NewOrder($order); >+ $newordernumber = NewOrder($order); > > $query = q{ > UPDATE aqorders_items >diff --git a/acqui/addorder.pl b/acqui/addorder.pl >index e048d05..da1beb0 100755 >--- a/acqui/addorder.pl >+++ b/acqui/addorder.pl >@@ -275,7 +275,7 @@ if ( $orderinfo->{quantity} ne '0' ) { > ModOrder( $orderinfo); > } > else { # else, it's a new line >- @$orderinfo{qw(basketno ordernumber )} = NewOrder($orderinfo); >+ $orderinfo->{ordernumber} = NewOrder($orderinfo); > } > > # now, add items if applicable >diff --git a/acqui/addorderiso2709.pl b/acqui/addorderiso2709.pl >index f4d1a25..601482a 100755 >--- a/acqui/addorderiso2709.pl >+++ b/acqui/addorderiso2709.pl >@@ -256,7 +256,7 @@ if ($op eq ""){ > > # remove uncertainprice flag if we have found a price in the MARC record > $orderinfo{uncertainprice} = 0 if $orderinfo{listprice}; >- my ( $basketno, $ordernumber ) = NewOrder( \%orderinfo ); >+ my $ordernumber = NewOrder( \%orderinfo ); > > # 4th, add items if applicable > # parse the item sent by the form, and create an item just for the import_record_id we are dealing with >diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t >index 837024a..2a3e8c7 100755 >--- a/t/db_dependent/Acquisition.t >+++ b/t/db_dependent/Acquisition.t >@@ -151,10 +151,10 @@ my ( $biblionumber4, $biblioitemnumber4 ) = AddBiblio( MARC::Record->new, '' ); > my ( $mandatoryparams, $return_error, $basketnum ); > > # returns undef and croaks if basketno, quantity, biblionumber or budget_id is missing >-eval { ( $basketnum, $ordernumbers[0] ) = C4::Acquisition::NewOrder() }; >+eval { $ordernumbers[0] = C4::Acquisition::NewOrder() }; > $return_error = $@; > ok( >- ( !( defined $basketnum || defined $ordernumbers[0] ) ) >+ ( ! defined $ordernumbers[0] ) > && ( defined $return_error ), > "NewOrder with no params returns undef and croaks" > ); >@@ -170,11 +170,11 @@ foreach my $mandatoryparams_key (@mandatoryparams_keys) { > my %test_missing_mandatoryparams = %$mandatoryparams; > delete $test_missing_mandatoryparams{$mandatoryparams_key}; > eval { >- ( $basketnum, $ordernumbers[0] ) = >+ $ordernumbers[0] = > C4::Acquisition::NewOrder( \%test_missing_mandatoryparams ); > }; > $return_error = $@; >- my $expected_error = "Mandatory parameter $mandatoryparams_key missing"; >+ my $expected_error = "Cannot insert order: Mandatory parameter $mandatoryparams_key is missing"; > ok( > ( !( defined $basketnum || defined $ordernumbers[0] ) ) > && ( index( $return_error, $expected_error ) >= 0 ), >@@ -283,7 +283,7 @@ for ( 0 .. 4 ) { > values %{ $order_content[$_]->{num} }; > @ocontent{ keys %{ $order_content[$_]->{str} } } = > values %{ $order_content[$_]->{str} }; >- ( undef, $ordernumbers[$_] ) = C4::Acquisition::NewOrder( \%ocontent ); >+ $ordernumbers[$_] = C4::Acquisition::NewOrder( \%ocontent ); > $order_content[$_]->{str}->{ordernumber} = $ordernumbers[$_]; > } > >diff --git a/t/db_dependent/Acquisition/CancelReceipt.t b/t/db_dependent/Acquisition/CancelReceipt.t >index e2d4e4d..f00cafb 100644 >--- a/t/db_dependent/Acquisition/CancelReceipt.t >+++ b/t/db_dependent/Acquisition/CancelReceipt.t >@@ -44,7 +44,7 @@ my ($biblionumber, $biblioitemnumber) = AddBiblio(MARC::Record->new, ''); > my $itemnumber = AddItem({}, $biblionumber); > > t::lib::Mocks::mock_preference('AcqCreateItem', 'receiving'); >-my ( undef, $ordernumber ) = C4::Acquisition::NewOrder( >+my $ordernumber = C4::Acquisition::NewOrder( > { > basketno => $basketno1, > quantity => 2, >@@ -70,7 +70,7 @@ my $order = GetOrder( $ordernumber ); > is(scalar GetItemnumbersFromOrder($order->{ordernumber}), 0, "Create items on receiving: 0 item exist after cancelling a receipt"); > > t::lib::Mocks::mock_preference('AcqCreateItem', 'ordering'); >-( undef, $ordernumber ) = C4::Acquisition::NewOrder( >+$ordernumber = C4::Acquisition::NewOrder( > { > basketno => $basketno1, > quantity => 2, >diff --git a/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t b/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t >index 3b4c96b..1c82ced 100644 >--- a/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t >+++ b/t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t >@@ -39,7 +39,7 @@ my ($biblionumber1, $biblioitemnumber1) = AddBiblio(MARC::Record->new, ''); > my ($biblionumber2, $biblioitemnumber2) = AddBiblio(MARC::Record->new, ''); > my ($biblionumber3, $biblioitemnumber3) = AddBiblio(MARC::Record->new, ''); > >-( undef, $ordernumber1 ) = C4::Acquisition::NewOrder( >+$ordernumber1 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 2, >@@ -48,7 +48,7 @@ my ($biblionumber3, $biblioitemnumber3) = AddBiblio(MARC::Record->new, ''); > } > ); > >-( undef, $ordernumber2 ) = C4::Acquisition::NewOrder( >+$ordernumber2 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 4, >diff --git a/t/db_dependent/Acquisition/GetOrdersByBiblionumber.t b/t/db_dependent/Acquisition/GetOrdersByBiblionumber.t >index 8bd0735..be6146d 100644 >--- a/t/db_dependent/Acquisition/GetOrdersByBiblionumber.t >+++ b/t/db_dependent/Acquisition/GetOrdersByBiblionumber.t >@@ -39,7 +39,7 @@ my $budget = C4::Budgets::GetBudget( $budgetid ); > my ($ordernumber1, $ordernumber2, $ordernumber3); > my ($biblionumber1, $biblioitemnumber1) = AddBiblio(MARC::Record->new, ''); > my ($biblionumber2, $biblioitemnumber2) = AddBiblio(MARC::Record->new, ''); >-( undef, $ordernumber1 ) = C4::Acquisition::NewOrder( >+$ordernumber1 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 24, >@@ -48,7 +48,7 @@ my ($biblionumber2, $biblioitemnumber2) = AddBiblio(MARC::Record->new, ''); > } > ); > >-( undef, $ordernumber2 ) = C4::Acquisition::NewOrder( >+$ordernumber2 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 42, >@@ -57,7 +57,7 @@ my ($biblionumber2, $biblioitemnumber2) = AddBiblio(MARC::Record->new, ''); > } > ); > >-( undef, $ordernumber3 ) = C4::Acquisition::NewOrder( >+$ordernumber3 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 4, >diff --git a/t/db_dependent/Acquisition/Invoices.t b/t/db_dependent/Acquisition/Invoices.t >index 4123c1e..9c6dcbc 100644 >--- a/t/db_dependent/Acquisition/Invoices.t >+++ b/t/db_dependent/Acquisition/Invoices.t >@@ -53,7 +53,7 @@ $bibrec1->append_fields( > my ($biblionumber1, $biblioitemnumber1) = AddBiblio($bibrec1, ''); > my ($biblionumber2, $biblioitemnumber2) = AddBiblio(MARC::Record->new, ''); > my ($biblionumber3, $biblioitemnumber3) = AddBiblio(MARC::Record->new, ''); >-( undef, $ordernumber1 ) = C4::Acquisition::NewOrder( >+$ordernumber1 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 2, >@@ -62,7 +62,7 @@ my ($biblionumber3, $biblioitemnumber3) = AddBiblio(MARC::Record->new, ''); > } > ); > >-( undef, $ordernumber2 ) = C4::Acquisition::NewOrder( >+$ordernumber2 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 1, >@@ -71,7 +71,7 @@ my ($biblionumber3, $biblioitemnumber3) = AddBiblio(MARC::Record->new, ''); > } > ); > >-( undef, $ordernumber3 ) = C4::Acquisition::NewOrder( >+$ordernumber3 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 1, >diff --git a/t/db_dependent/Acquisition/OrderFromSubscription.t b/t/db_dependent/Acquisition/OrderFromSubscription.t >index 72d4c68..2753248 100644 >--- a/t/db_dependent/Acquisition/OrderFromSubscription.t >+++ b/t/db_dependent/Acquisition/OrderFromSubscription.t >@@ -57,7 +57,7 @@ ok($basketno = NewBasket($booksellerid, 1), "NewBasket( $booksellerid , 1 ) re > my $cost = 42.00; > my $subscription = GetSubscription( $subscriptionid ); > my $ordernumber; >-( $basketno, $ordernumber ) = NewOrder({ >+$ordernumber = NewOrder({ > biblionumber => $subscription->{biblionumber}, > entrydate => '01-01-2013', > quantity => 1, >diff --git a/t/db_dependent/Acquisition/TransferOrder.t b/t/db_dependent/Acquisition/TransferOrder.t >index 91ee37a..dfe57ff 100644 >--- a/t/db_dependent/Acquisition/TransferOrder.t >+++ b/t/db_dependent/Acquisition/TransferOrder.t >@@ -54,7 +54,7 @@ my $budget = C4::Budgets::GetBudget( $budgetid ); > my ($biblionumber, $biblioitemnumber) = AddBiblio(MARC::Record->new, ''); > my $itemnumber = AddItem({}, $biblionumber); > >-my ( undef, $ordernumber ) = C4::Acquisition::NewOrder( >+my $ordernumber = C4::Acquisition::NewOrder( > { > basketno => $basketno1, > quantity => 2, >diff --git a/t/db_dependent/Acquisition/close_reopen_basket.t b/t/db_dependent/Acquisition/close_reopen_basket.t >index 3e7a280..d5a4789 100644 >--- a/t/db_dependent/Acquisition/close_reopen_basket.t >+++ b/t/db_dependent/Acquisition/close_reopen_basket.t >@@ -44,7 +44,7 @@ my ($biblionumber1, $biblioitemnumber1) = AddBiblio(MARC::Record->new, ''); > my ($biblionumber2, $biblioitemnumber2) = AddBiblio(MARC::Record->new, ''); > > my ($ordernumber1, $ordernumber2); >-( undef, $ordernumber1 ) = C4::Acquisition::NewOrder( >+$ordernumber1 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 24, >@@ -53,7 +53,7 @@ my ($ordernumber1, $ordernumber2); > } > ); > >-( undef, $ordernumber2 ) = C4::Acquisition::NewOrder( >+$ordernumber2 = C4::Acquisition::NewOrder( > { > basketno => $basketno, > quantity => 42, >diff --git a/t/db_dependent/Bookseller.t b/t/db_dependent/Bookseller.t >index c7ea24c..1991077 100644 >--- a/t/db_dependent/Bookseller.t >+++ b/t/db_dependent/Bookseller.t >@@ -139,9 +139,9 @@ is_deeply( \@booksellers, \@tab, > my @bookseller1 = C4::Bookseller::GetBookSeller( $sample_supplier1->{name} ); > #FIXME : if there is 0 basket, GetBookSeller returns 1 as basketcount > #is( $bookseller1[0]->{basketcount}, 0, 'Supplier1 has 0 basket' ); >-my $sample_basket1 = >+my $basketno1 = > C4::Acquisition::NewBasket( $id_supplier1, 'authorisedby1', 'basketname1' ); >-my $sample_basket2 = >+my $basketno2 = > C4::Acquisition::NewBasket( $id_supplier1, 'authorisedby2', 'basketname2' ); > @bookseller1 = C4::Bookseller::GetBookSeller( $sample_supplier1->{name} ); > is( $bookseller1[0]->{basketcount}, 2, 'Supplier1 has 2 baskets' ); >@@ -323,22 +323,22 @@ my $id_supplier3 = C4::Bookseller::AddBookseller($sample_supplier3); > my $id_supplier4 = C4::Bookseller::AddBookseller($sample_supplier4); > > #Add 2 baskets >-my $sample_basket3 = >+my $basketno3 = > C4::Acquisition::NewBasket( $id_supplier3, 'authorisedby3', 'basketname3', > 'basketnote3' ); >-my $sample_basket4 = >+my $basketno4 = > C4::Acquisition::NewBasket( $id_supplier4, 'authorisedby4', 'basketname4', > 'basketnote4' ); > > #Modify the basket to add a close date > my $basket1info = { >- basketno => $sample_basket1, >+ basketno => $basketno1, > closedate => $today, > booksellerid => $id_supplier1 > }; > > my $basket2info = { >- basketno => $sample_basket2, >+ basketno => $basketno2, > closedate => $daysago5, > booksellerid => $id_supplier2 > }; >@@ -348,12 +348,12 @@ my $dur10 = DateTime::Duration->new( days => -10 ); > $dt_today2->add_duration($dur10); > my $daysago10 = output_pref({ dt => $dt_today2, dateformat => 'iso', timeformat => '24hr', dateonly => 1 }); > my $basket3info = { >- basketno => $sample_basket3, >+ basketno => $basketno3, > closedate => $daysago10, > }; > > my $basket4info = { >- basketno => $sample_basket4, >+ basketno => $basketno4, > closedate => $today, > }; > ModBasket($basket1info); >@@ -384,10 +384,9 @@ is(scalar(@subscriptions), 1, 'search for subscriptions by location'); > > #Add 4 orders > my ( $ordernumber1, $ordernumber2, $ordernumber3, $ordernumber4 ); >-my ( $basketno1, $basketno2, $basketno3, $basketno4 ); >-( $basketno1, $ordernumber1 ) = C4::Acquisition::NewOrder( >+$ordernumber1 = C4::Acquisition::NewOrder( > { >- basketno => $sample_basket1, >+ basketno => $basketno1, > quantity => 24, > biblionumber => $biblionumber, > budget_id => $id_budget, >@@ -403,9 +402,9 @@ my ( $basketno1, $basketno2, $basketno3, $basketno4 ); > datereceived => '01-06-2013' > } > ); >-( $basketno2, $ordernumber2 ) = C4::Acquisition::NewOrder( >+$ordernumber2 = C4::Acquisition::NewOrder( > { >- basketno => $sample_basket2, >+ basketno => $basketno2, > quantity => 20, > biblionumber => $biblionumber, > budget_id => $id_budget, >@@ -419,9 +418,9 @@ my ( $basketno1, $basketno2, $basketno3, $basketno4 ); > ecost => 10, > } > ); >-( $basketno3, $ordernumber3 ) = C4::Acquisition::NewOrder( >+$ordernumber3 = C4::Acquisition::NewOrder( > { >- basketno => $sample_basket3, >+ basketno => $basketno3, > quantity => 20, > biblionumber => $biblionumber, > budget_id => $id_budget, >@@ -435,9 +434,9 @@ my ( $basketno1, $basketno2, $basketno3, $basketno4 ); > ecost => 11, > } > ); >-( $basketno4, $ordernumber4 ) = C4::Acquisition::NewOrder( >+$ordernumber4 = C4::Acquisition::NewOrder( > { >- basketno => $sample_basket4, >+ basketno => $basketno4, > quantity => 20, > biblionumber => $biblionumber, > budget_id => $id_budget, >@@ -632,7 +631,7 @@ isnt( exists( $suppliers{$id_supplier4} ), 1, "Supplier4 hasnt late orders" ); > > #Basket1 closedate -> $daysago10 > $basket1info = { >- basketno => $sample_basket1, >+ basketno => $basketno1, > closedate => $daysago10, > }; > ModBasket($basket1info); >diff --git a/t/db_dependent/Budgets.t b/t/db_dependent/Budgets.t >index a565c8d..41bac90 100755 >--- a/t/db_dependent/Budgets.t >+++ b/t/db_dependent/Budgets.t >@@ -304,7 +304,7 @@ my $item_quantity = 2; > my $number_of_orders_to_move = 0; > for my $infos (@order_infos) { > for ( 1 .. $infos->{pending_quantity} ) { >- my ( undef, $ordernumber ) = C4::Acquisition::NewOrder( >+ my $ordernumber = C4::Acquisition::NewOrder( > { > basketno => $basketno, > biblionumber => $biblionumber, >@@ -326,7 +326,7 @@ for my $infos (@order_infos) { > $number_of_orders_to_move++; > } > for ( 1 .. $infos->{spent_quantity} ) { >- my ( undef, $ordernumber ) = C4::Acquisition::NewOrder( >+ my $ordernumber = C4::Acquisition::NewOrder( > { > basketno => $basketno, > biblionumber => $biblionumber, >-- >2.0.0.rc2
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=31161">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12826
:
31161
|
31162