From 21c87aeb15e6380d71b41f8c5e8ff28df74b3edd Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 21 Feb 2024 14:39:00 +0000 Subject: [PATCH] Bug 36018: biblio->active_orders should be ->uncancelled_orders Active orders are more than just not cancelled. See filter_by_active in Koha::Acquisition::Orders. They are still in the acq process; we still need to receive items on those orders. When we cancel and want to delete a biblio, we should check for not cancelled orders (broader than active orders as in Orders.pm). Test plan: Git grep active_orders. Run t/db_dependent/Koha/Biblio.t Run t/db_dependent/Koha/Acquisition/Order.t Add two order lines in a basket referring to same biblio. Try to cancel with delete biblio. Should not be possible. Signed-off-by: Marcel de Rooy Signed-off-by: Janusz Kaczmarek Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer --- Koha/Acquisition/Order.pm | 6 +++--- Koha/Biblio.pm | 12 +++++------ acqui/basket.pl | 4 ++-- api/v1/swagger/paths/acquisitions_orders.yaml | 4 ++-- .../prog/en/includes/catalog-strings.inc | 2 +- .../prog/en/modules/acqui/parcel.tt | 8 +++---- t/db_dependent/Koha/Acquisition/Order.t | 2 +- t/db_dependent/Koha/Biblio.t | 21 +++++++++++-------- 8 files changed, 30 insertions(+), 29 deletions(-) diff --git a/Koha/Acquisition/Order.pm b/Koha/Acquisition/Order.pm index 0e3c8201e8..d869b9764d 100644 --- a/Koha/Acquisition/Order.pm +++ b/Koha/Acquisition/Order.pm @@ -161,7 +161,7 @@ sub cancel { if ( $biblio and $delete_biblio ) { if ( - $biblio->active_orders->search( + $biblio->uncancelled_orders->search( { ordernumber => { '!=' => $self->ordernumber } } )->count == 0 and $biblio->subscriptions->count == 0 @@ -182,10 +182,10 @@ sub cancel { my $message; - if ( $biblio->active_orders->search( + if ( $biblio->uncancelled_orders->search( { ordernumber => { '!=' => $self->ordernumber } } )->count > 0 ) { - $message = 'error_delbiblio_active_orders'; + $message = 'error_delbiblio_uncancelled_orders'; } elsif ( $biblio->subscriptions->count > 0 ) { $message = 'error_delbiblio_subscriptions'; diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index e066c4f68c..2cdd629d8b 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -144,20 +144,18 @@ sub orders { return Koha::Acquisition::Orders->_new_from_dbic($orders); } -=head3 active_orders +=head3 uncancelled_orders -my $active_orders = $biblio->active_orders(); +my $uncancelled_orders = $biblio->uncancelled_orders; -Returns the active acquisition orders related to this biblio. -An order is considered active when it is not cancelled (i.e. when datecancellation -is not undef). +Returns acquisition orders related to this biblio that are not cancelled. =cut -sub active_orders { +sub uncancelled_orders { my ( $self ) = @_; - return $self->orders->search({ datecancellationprinted => undef }); + return $self->orders->filter_out_cancelled; } =head3 tickets diff --git a/acqui/basket.pl b/acqui/basket.pl index 64779bdffe..8cc57bbd10 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -160,7 +160,7 @@ if ( $op eq 'cud-delete-order' ) { biblionumber => $biblio->id, title => $biblio->title // '', author => $biblio->author // '', - countbiblio => $biblio->active_orders->count, + countbiblio => $biblio->uncancelled_orders->count, itemcount => $biblio->items->count, subscriptions => $biblio->subscriptions->count, }; @@ -489,7 +489,7 @@ sub get_order_infos { my $biblionumber = $order->{'biblionumber'}; if ( $biblionumber ) { # The biblio still exists my $biblio = Koha::Biblios->find( $biblionumber ); - my $countbiblio = $biblio->active_orders->count; + my $countbiblio = $biblio->uncancelled_orders->count; my $ordernumber = $order->{'ordernumber'}; my $cnt_subscriptions = $biblio->subscriptions->count; diff --git a/api/v1/swagger/paths/acquisitions_orders.yaml b/api/v1/swagger/paths/acquisitions_orders.yaml index 1cd70a4d2c..caa47d85aa 100644 --- a/api/v1/swagger/paths/acquisitions_orders.yaml +++ b/api/v1/swagger/paths/acquisitions_orders.yaml @@ -54,7 +54,7 @@ - basket.basket_group - basket.creator - biblio - - biblio.active_orders+count + - biblio.uncancelled_orders+count - biblio.holds+count - biblio.items+count - biblio.suggestions.suggester @@ -174,7 +174,7 @@ - basket.basket_group - basket.creator - biblio - - biblio.active_orders+count + - biblio.uncancelled_orders+count - biblio.holds+count - biblio.items+count - biblio.suggestions.suggester diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/catalog-strings.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/catalog-strings.inc index fc88b79abf..6ea79d927a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/catalog-strings.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/catalog-strings.inc @@ -6,7 +6,7 @@ var count = [% count || 0 | html %]; var holdcount = [% holdcount || 0 | html %]; [% SET orders = biblio.orders %] - [% SET current = Context.Scalar(orders, "filter_by_current") %] + [% SET current = Context.Scalar(orders, "filter_out_cancelled") %] [% SET cancelled = Context.Scalar(orders, "filter_by_cancelled") %] var countorders = [% current.count || 0 | html %]; var countdeletedorders = [% cancelled.count || 0 | html %]; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt index 33dd94ba60..e230833e77 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt @@ -404,7 +404,7 @@ }, "embed": [ "basket.basket_group", - "biblio.active_orders+count", + "biblio.uncancelled_orders+count", "biblio.holds+count", "biblio.items+count", "biblio.suggestions.suggester", @@ -618,7 +618,7 @@ if ( row.biblio != null ) { if ( row.biblio.items_count - row.items.length > 0 || - row.biblio.active_orders_count > 1 || + row.biblio.uncancelled_orders_count > 1 || row.biblio.subscriptions_count > 0 || row.biblio.holds_count > 0 ) { // biblio can be deleted result += '' + (row.biblio.items_count - row.items.length) + _(" item(s) left") + '
'; } - if ( row.biblio.active_orders_count > 1 ) { + if ( row.biblio.uncancelled_orders_count > 1 ) { result += '' - + (row.biblio.active_orders_count - 1) + _(" order(s) left") + '
'; + + (row.biblio.uncancelled_orders_count - 1) + _(" order(s) left") + '
'; } if ( row.biblio.subscriptions_count > 0 ) { diff --git a/t/db_dependent/Koha/Acquisition/Order.t b/t/db_dependent/Koha/Acquisition/Order.t index 973170d4ca..d80e1ee60e 100755 --- a/t/db_dependent/Koha/Acquisition/Order.t +++ b/t/db_dependent/Koha/Acquisition/Order.t @@ -816,7 +816,7 @@ subtest 'cancel() tests' => sub { is( Koha::Items->find($item->id), undef, 'The item is no longer present' ); is( ref(Koha::Biblios->find($biblio_id)), 'Koha::Biblio', 'The biblio is still present' ); @messages = @{ $order->object_messages }; - is( $messages[0]->message, 'error_delbiblio_active_orders', 'Cannot delete biblio and it gets notified' ); + is( $messages[0]->message, 'error_delbiblio_uncancelled_orders', 'Cannot delete biblio and it gets notified' ); # Scenario: # * order with one item attached diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t index f248118f81..acb130f54a 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -942,7 +942,7 @@ subtest 'get_volumes_query' => sub { ); }; -subtest 'orders() and active_orders() tests' => sub { +subtest 'orders() and uncancelled_orders() tests' => sub { plan tests => 5; @@ -950,11 +950,14 @@ subtest 'orders() and active_orders() tests' => sub { my $biblio = $builder->build_sample_biblio(); - my $orders = $biblio->orders; - my $active_orders = $biblio->active_orders; + my $orders = $biblio->orders; + my $uncancelled_orders = $biblio->uncancelled_orders; is( ref($orders), 'Koha::Acquisition::Orders', 'Result type is correct' ); - is( $biblio->orders->count, $biblio->active_orders->count, '->orders->count returns the count for the resultset' ); + is( + $biblio->orders->count, $biblio->uncancelled_orders->count, + '->orders->count returns the count for the resultset' + ); # Add a couple orders foreach (1..2) { @@ -979,12 +982,12 @@ subtest 'orders() and active_orders() tests' => sub { } ); - $orders = $biblio->orders; - $active_orders = $biblio->active_orders; + $orders = $biblio->orders; + $uncancelled_orders = $biblio->uncancelled_orders; - is( ref($orders), 'Koha::Acquisition::Orders', 'Result type is correct' ); - is( ref($active_orders), 'Koha::Acquisition::Orders', 'Result type is correct' ); - is( $orders->count, $active_orders->count + 2, '->active_orders->count returns the rigt count' ); + is( ref($orders), 'Koha::Acquisition::Orders', 'Result type is correct' ); + is( ref($uncancelled_orders), 'Koha::Acquisition::Orders', 'Result type is correct' ); + is( $orders->count, $uncancelled_orders->count + 2, '->uncancelled_orders->count returns the right count' ); $schema->storage->txn_rollback; }; -- 2.39.5