Attachment 162320 Details for Bug 36018 – Bug 36018: biblio->active_orders should be ->uncancelled_orders

[patch] Bug 36018: biblio->active_orders should be ->uncancelled_orders

Bug-36018-biblio-activeorders-should-be--uncancell.patch (text/plain), 10.69 KB, created by Marcel de Rooy on 2024-02-21 14:55:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2024-02-21 14:55:58 UTC

Size: 10.69 KB

patch

obsolete

>From f92f380d7b9cabed24955fc456068417bd770961 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Wed, 21 Feb 2024 14:39:00 +0000
>Subject: [PATCH] Bug 36018: biblio->active_orders should be
> ->uncancelled_orders
>Content-Type: text/plain; charset=utf-8
>
>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 <m.de.rooy@rijksmuseum.nl>
>---
> 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 e0fea073f1..7cee0504d1 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 7d0b244085..2be4ea6d8a 100644
>--- a/Koha/Biblio.pm
>+++ b/Koha/Biblio.pm
>@@ -138,20 +138,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 2140a3f19c..aea3e6203a 100755
>--- a/acqui/basket.pl
>+++ b/acqui/basket.pl
>@@ -144,7 +144,7 @@ if ( $op eq 'delete_confirm' ) {
> 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,
> };
>@@ -473,7 +473,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 260597281d..fcb54fb582 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 3ab04f9af1..94f95534f9 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 | html %];
> var holdcount = [% holdcount | 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 dcb9379874..1fc1944c93 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt
>@@ -392,7 +392,7 @@
> },
> "embed": [
> "basket.basket_group",
>- "biblio.active_orders+count",
>+ "biblio.uncancelled_orders+count",
> "biblio.holds+count",
> "biblio.items+count",
> "biblio.suggestions.suggester",
>@@ -606,7 +606,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 += '@@ -626,10 +626,10 @@
> +'">' + (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 += ' + _("Can't delete catalog record, delete other orders linked to it first") + '">'
>- + (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 94c30fa3d4..4679fb7489 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 13be09f466..ae09a0968c 100755
>--- a/t/db_dependent/Koha/Biblio.t
>+++ b/t/db_dependent/Koha/Biblio.t
>@@ -660,7 +660,7 @@ subtest 'get_volumes_query' => sub {
> );
> };
> 
>-subtest 'orders() and active_orders() tests' => sub {
>+subtest 'orders() and uncancelled_orders() tests' => sub {
> 
> plan tests => 5;
> 
>@@ -668,11 +668,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) {
>@@ -697,12 +700,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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 36018: 162318 | 162320 | 162321 | 163769 | 163770 | 163771 | 163772 | 164953 | 164954 | 164955 | 164956 | 165153 | 166503 | 166504 | 166505 | 166506 | 166507 | 166677