From e1bf33dafb43d6d96125442cdf341b87f4bf75ea Mon Sep 17 00:00:00 2001 From: Mathieu Saby Date: Sat, 14 Sep 2013 12:28:21 +0200 Subject: [PATCH] Bug 10869: Allow the deletion of order line if the record is deleted Content-Type: text/plain; charset="utf-8" This patch fixes 2 issues : (medium issue) It is not possible to delete an order in a basket if the biblio record is deleted. (minor issue) There is a useless hyperlink around "Deleted bibliographic notice", and "notice" is a frenchism for "record" Biblionumber was passed to the function as 1st parameter, which made impossible to suppress an order whose associated biblio record was suppressed. Moreover, the biblionumber is not needed to get and suppress the order, but only to suppress items, in a second step. Change made to Acquisitions::DelOrder: - I changed the order of the 2 parameters: now $ordernumber as 1st parameter and $biblionumber as 2d parameter - I renamed $bibnum to $biblionumber - I removed $biblionumber from the main SQL query of the function - I suppressed useless $sth->finish - if no $biblionumber is provided, Koha does not try to suppress the items linked created by acq process Change made to the calls to Acquisitions::DelOrder in Koha's code, to change the order of the parameters - in addorder.pl - in basket.pl - in UT t/db_dependant/Acquisition.t Change made in template basket.tt. This bug was signed off by Cedric some time ago, but it needs a second sign off, because I had to made some little changes. To test : 1. Fill a basket with some records (some records with items or holds, some without items) 2. Supress one of the record 3. In the basket (refresh the screen) check that "Deleted bibliographic notice" line was replaced with "Deleted bibliographic record", without hyperlink arround 4. Delete this order with the link "Delete order" to the right column of the table 5. Check there is a new line in the "Deleted orders" table 6. Check there is no regression : delete an other order (one with no item), then check you cannot delete an order with an item (like before patch) Signed-off-by: Cedric Vita --- C4/Acquisition.pm | 17 +++--- acqui/addorder.pl | 2 +- acqui/basket.pl | 2 +- .../intranet-tmpl/prog/en/modules/acqui/basket.tt | 57 +++++++++++--------- t/db_dependent/Acquisition.t | 2 +- 5 files changed, 44 insertions(+), 36 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 33aa8e3..2e0a888 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -1751,7 +1751,7 @@ sub SearchOrders { =head3 DelOrder - &DelOrder($biblionumber, $ordernumber); + &DelOrder($ordernumber, $biblionumber); Cancel the order with the given order and biblio numbers. It does not delete any entries in the aqorders table, it merely marks them as @@ -1760,18 +1760,20 @@ cancelled. =cut sub DelOrder { - my ( $bibnum, $ordernumber ) = @_; + my ($ordernumber,$biblionumber) = @_; my $dbh = C4::Context->dbh; my $query = " UPDATE aqorders SET datecancellationprinted=now(), orderstatus='cancelled' - WHERE biblionumber=? AND ordernumber=? + WHERE ordernumber=? "; my $sth = $dbh->prepare($query); - $sth->execute( $bibnum, $ordernumber ); - my @itemnumbers = GetItemnumbersFromOrder( $ordernumber ); - foreach my $itemnumber (@itemnumbers){ - C4::Items::DelItem( $dbh, $bibnum, $itemnumber ); + $sth->execute($ordernumber); + if ($biblionumber) { + my @itemnumbers = GetItemnumbersFromOrder( $ordernumber ); + foreach my $itemnumber (@itemnumbers){ + C4::Items::DelItem( $dbh, $biblionumber, $itemnumber ); + } } return; } @@ -2235,7 +2237,6 @@ sub GetHistory { LEFT JOIN deletedbiblio ON deletedbiblio.biblionumber=aqorders.biblionumber LEFT JOIN deletedbiblioitems ON deletedbiblioitems.biblionumber=aqorders.biblionumber "; - if ( C4::Context->preference("IndependentBranches") ) { $query .= " LEFT JOIN borrowers ON aqbasket.authorisedby=borrowers.borrowernumber"; } diff --git a/acqui/addorder.pl b/acqui/addorder.pl index 1b05f07..5a34856 100755 --- a/acqui/addorder.pl +++ b/acqui/addorder.pl @@ -323,7 +323,7 @@ if ( $orderinfo->{quantity} ne '0' ) { else { # qty=0, delete the line my $biblionumber = $input->param('biblionumber'); - DelOrder( $biblionumber, $$orderinfo{ordernumber} ); + DelOrder($$orderinfo{ordernumber}, $biblionumber ); if ($orderinfo->{delbiblio} == 1){ DelBiblio($biblionumber); } diff --git a/acqui/basket.pl b/acqui/basket.pl index 047ae84..659475d 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -114,7 +114,7 @@ if ( $op eq 'delete_confirm' ) { my @orders = GetOrders($basketno); #Delete all orders included in that basket, and all items received. foreach my $myorder (@orders){ - DelOrder($myorder->{biblionumber},$myorder->{ordernumber}); + DelOrder($myorder->{ordernumber},$myorder->{biblionumber}); } # if $delbiblio = 1, delete the records if possible if ((defined $delbiblio)and ($delbiblio ==1)){ 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 cd31176..4c401a4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt @@ -476,10 +476,13 @@ [% books_loo.ordernumber %] -

- [% IF ( books_loo.order_received ) %] (rcvd)[% END %] - [% books_loo.title |html %] by [% books_loo.author %] -
+

[% IF ( books_loo.order_received ) %] (rcvd)[% END %] + [% IF (books_loo.biblionumber) %] + [% books_loo.title |html %] by [% books_loo.author %] +
+ [% ELSE %] + Deleted bibliographic record, can't find title
+ [% END %] [% IF ( books_loo.isbn ) %] - [% books_loo.isbn %][% END %] [% IF ( books_loo.issn ) %] - [% books_loo.issn %][% END %] [% IF ( books_loo.publishercode ) %], [% books_loo.publishercode %][% END %] @@ -531,27 +534,31 @@ [% END %] - [% IF ( books_loo.left_holds_on_order ) %] - Can't delete order
- [% ELSE %] - Delete order
- [% END %] - [% IF ( books_loo.can_del_bib ) %] - Delete order and catalog record
+ [% UNLESS (books_loo.biblionumber) %] + Delete order
[% ELSE %] - Can't delete order and catalog record
- [% END %] - [% IF ( books_loo.left_item ) %] - [% books_loo.items %] item(s) left
- [% END %] - [% IF ( books_loo.left_biblio ) %] - [% books_loo.biblios %] order(s) left
- [% END %] - [% IF ( books_loo.left_subscription ) %] - [% books_loo.subscriptions %] subscription(s) left
- [% END %] - [% IF ( books_loo.left_holds ) %] - [% books_loo.holds %] hold(s) left + [% IF ( books_loo.left_holds_on_order ) %] + Can't delete order
+ [% ELSE %] + Delete order
+ [% END %] + [% IF ( books_loo.can_del_bib ) %] + Delete order and catalog record
+ [% ELSE %] + Can't delete order and catalog record
+ [% END %] + [% IF ( books_loo.left_item ) %] + [% books_loo.items %] item(s) left
+ [% END %] + [% IF ( books_loo.left_biblio ) %] + [% books_loo.biblios %] order(s) left
+ [% END %] + [% IF ( books_loo.left_subscription ) %] + [% books_loo.subscriptions %] subscription(s) left
+ [% END %] + [% IF ( books_loo.left_holds ) %] + [% books_loo.holds %] hold(s) left + [% END %] [% END %] [% END %] @@ -593,7 +600,7 @@

[% IF ( order.order_received ) %] (rcvd)[% END %] - [% IF (order.title) %] + [% IF (order.biblionumber) %] [% order.title |html %] by [% order.author %]
[% ELSE %] Deleted bibliographic record, can't find title
diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t index f2ee820..32fb2cd 100755 --- a/t/db_dependent/Acquisition.t +++ b/t/db_dependent/Acquisition.t @@ -519,7 +519,7 @@ my @base_expectedfields = qw( ( @base_expectedfields, ( 'transferred_from_timestamp', 'transferred_from' ) ); is( GetOrders(), undef, "GetOrders with no params returns undef" ); -DelOrder( $order_content[3]->{str}->{biblionumber}, $ordernumbers[3] ); +DelOrder( $ordernumbers[3], $order_content[3]->{str}->{biblionumber}); my @get_orders = GetOrders($basketno); ( $test_missing_fields, $test_extra_fields, -- 1.7.9.5