Attachment 152953 Details for Bug 30708 – Bug 30708: Do not allow deletion of processings that are used

[patch] Bug 30708: Do not allow deletion of processings that are used

Bug-30708-Do-not-allow-deletion-of-processings-tha.patch (text/plain), 6.08 KB, created by Jonathan Druart on 2023-07-03 13:29:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2023-07-03 13:29:45 UTC

Size: 6.08 KB

patch

obsolete

>From 20322788d3fe2167a6d3a24941fcfeb3cda4b0a9 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Fri, 7 Apr 2023 11:35:06 +0200
>Subject: [PATCH] Bug 30708: Do not allow deletion of processings that are used
>
>Sponsored-by: BULAC - http://www.bulac.fr/
>Signed-off-by: BULAC - http://www.bulac.fr/
>---
> Koha/Preservation/Processing.pm               | 20 ++++++++++
> Koha/REST/V1/Preservation/Processings.pm      |  9 ++++-
> .../Preservation/SettingsProcessings.vue      | 16 ++++++--
> .../Koha/Preservation/Processings.t           | 37 ++++++++++++++++++-
> 4 files changed, 77 insertions(+), 5 deletions(-)
>
>diff --git a/Koha/Preservation/Processing.pm b/Koha/Preservation/Processing.pm
>index e3c5ef95dc0..5fe6d2b85bf 100644
>--- a/Koha/Preservation/Processing.pm
>+++ b/Koha/Preservation/Processing.pm
>@@ -21,6 +21,8 @@ use Koha::Database;
> 
> use base qw(Koha::Object);
> 
>+use Koha::Preservation::Trains;
>+use Koha::Preservation::Train::Items;
> use Koha::Preservation::Processing::Attributes;
> 
> =head1 NAME
>@@ -80,6 +82,24 @@ sub attributes {
>     return Koha::Preservation::Processing::Attributes->_new_from_dbic($attributes_rs);
> }
> 
>+=head3 can_be_deleted
>+
>+A processing can be deleted if it is not used from any trains or items.
>+Note that we do not enforce that in ->delete, the callers are supposed to deal with that correctly.
>+
>+=cut
>+
>+sub can_be_deleted {
>+    my ($self) = @_;
>+
>+    my $trains_using_it = Koha::Preservation::Trains->search(
>+        { default_processing_id => $self->processing_id } )->count;
>+    my $items_using_it = Koha::Preservation::Train::Items->search(
>+        { processing_id => $self->processing_id } )->count;
>+
>+    return ( $trains_using_it || $items_using_it ) ? 0 : 1;
>+}
>+
> =head2 Internal methods
> 
> =head3 _type
>diff --git a/Koha/REST/V1/Preservation/Processings.pm b/Koha/REST/V1/Preservation/Processings.pm
>index 6b04e9b2f5d..98a924b68a0 100644
>--- a/Koha/REST/V1/Preservation/Processings.pm
>+++ b/Koha/REST/V1/Preservation/Processings.pm
>@@ -227,7 +227,8 @@ sub update {
> sub delete {
>     my $c = shift->openapi->valid_input or return;
> 
>-    my $processing = Koha::Preservation::Processings->find( $c->validation->param('processing_id') );
>+    my $processing_id = $c->validation->param('processing_id');
>+    my $processing = Koha::Preservation::Processings->find( $processing_id );
>     unless ($processing) {
>         return $c->render(
>             status  => 404,
>@@ -235,6 +236,12 @@ sub delete {
>         );
>     }
> 
>+    unless ($processing->can_be_deleted) {
>+        return $c->render(
>+            status => 409,
>+            openapi => { error => "Processing is already used" },
>+        );
>+    }
>     return try {
>         $processing->delete;
>         return $c->render(
>diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Preservation/SettingsProcessings.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Preservation/SettingsProcessings.vue
>index ac7e54d7aa5..5c0c9c5f186 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Preservation/SettingsProcessings.vue
>+++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Preservation/SettingsProcessings.vue
>@@ -44,8 +44,9 @@ import { APIClient } from "../../fetch/api-client.js"
> 
> export default {
>     setup() {
>-        const { setConfirmationDialog, setMessage } = inject("mainStore")
>-        return { setConfirmationDialog, setMessage }
>+        const { setConfirmationDialog, setMessage, setError } =
>+            inject("mainStore")
>+        return { setConfirmationDialog, setMessage, setError }
>     },
>     data() {
>         return {
>@@ -90,7 +91,16 @@ export default {
>                                 error => {}
>                             )
>                         },
>-                        error => {}
>+                        error => {
>+                            // FIXME We need a better way to do that
>+                            if (error.toString().match(/409/)) {
>+                                this.setError(
>+                                    this.$__(
>+                                        "This processing cannot be deleted, it is already in used."
>+                                    )
>+                                )
>+                            }
>+                        }
>                     )
>                 }
>             )
>diff --git a/t/db_dependent/Koha/Preservation/Processings.t b/t/db_dependent/Koha/Preservation/Processings.t
>index d0c1bb1d399..62e8542b057 100755
>--- a/t/db_dependent/Koha/Preservation/Processings.t
>+++ b/t/db_dependent/Koha/Preservation/Processings.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 1;
>+use Test::More tests => 2;
> use Test::Exception;
> use Test::Warn;
> 
>@@ -56,3 +56,38 @@ subtest 'attributes' => sub {
> 
>     $schema->storage->txn_rollback;
> };
>+
>+subtest 'can_be_deleted' => sub {
>+
>+    plan tests => 5;
>+
>+    $schema->storage->txn_begin;
>+
>+    my $processing = $builder->build_object( { class => 'Koha::Preservation::Processings' } );
>+    my $another_processing = $builder->build_object( { class => 'Koha::Preservation::Processings' } );
>+
>+    is( $processing->can_be_deleted, 1, 'processing is not used, it can be deleted' );
>+
>+    my $train = $builder->build_object(
>+        {
>+            class => 'Koha::Preservation::Trains',
>+            value => {
>+                not_for_loan => 42,
>+                default_processing_id => $processing->processing_id,
>+                closed_on    => undef,
>+                sent_on      => undef,
>+                received_on  => undef
>+            }
>+        }
>+    );
>+
>+    is( $processing->can_be_deleted, 0, 'processing is used, it cannot be deleted' );
>+    is( $another_processing->can_be_deleted, 1, 'processing is not used, it can be deleted' );
>+
>+    my $item = $builder->build_sample_item;
>+    $train->add_item( { item_id => $item->itemnumber, processing_id => $another_processing->processing_id }, { skip_waiting_list_check => 1 } );
>+    is( $processing->can_be_deleted, 0, 'processing is used, it cannot be deleted' );
>+    is( $another_processing->can_be_deleted, 0, 'processing is used, it cannot be deleted' );
>+
>+    $schema->storage->txn_rollback;
>+};
>-- 
>2.25.1

Actions: View | Diff | Splinter Review

Attachments on bug 30708: 134715 | 134716 | 134732 | 134733 | 134734 | 134736 | 134737 | 134738 | 134782 | 134783 | 134784 | 134785 | 152939 | 152940 | 152941 | 152942 | 152943 | 152944 | 152945 | 152946 | 152947 | 152948 | 152949 | 152950 | 152951 | 152952 | 152953 | 152954 | 152955 | 152956 | 152957 | 152958 | 152959 | 152960 | 152961 | 152962 | 152963 | 152964 | 152965 | 152966 | 152967 | 152968 | 152969 | 152970 | 153102 | 153576 | 154167 | 154168 | 154169 | 154170 | 154171 | 154172 | 154173 | 154174 | 154175 | 154176 | 154177 | 154178 | 154179 | 154180 | 154181 | 154182 | 154183 | 154184 | 154185 | 154186 | 154187 | 154188 | 154189 | 154190 | 154191 | 154192 | 154193 | 154194 | 154195 | 154196 | 154197 | 154198 | 154199 | 154200 | 154201 | 154202 | 154203 | 156592 | 156596 | 156597 | 156636 | 156637 | 156638 | 156639 | 156640 | 156641 | 156642 | 156643 | 156644 | 156645 | 156646 | 156647 | 156648 | 156649 | 156650 | 156651 | 156652 | 156653 | 156654 | 156655 | 156656 | 156657 | 156658 | 156659 | 156660 | 156661 | 156662 | 156663 | 156664 | 156665 | 156666 | 156667 | 156668 | 156669 | 156670 | 156671 | 156672 | 156673 | 156674 | 156675 | 157502 | 157503 | 157506 | 157508 | 157512 | 157514 | 157557