Bug 26992

Summary: On serial collection page, impossible to delete issues and related items
Product: Koha Reporter: Frédéric Demians <frederic>
Component: SerialsAssignee: Frédéric Demians <frederic>
Status: RESOLVED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: critical    
Priority: P3 CC: andrew, colin.campbell, david, jonathan.druart, joonas.kylmala, philippe.blouin, tomascohen, victor
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.07
Bug Depends on: 23463, 17674    
Bug Blocks:    
Attachments: Bug 26992 - On serial collection page, impossible to delete issues and related items
Bug 26992 - On serial collection page, impossible to delete issues and related items
Bug 26992: Fix serial issue and related items deletion on serial collection page
Bug 26992 - On serial collection page, impossible to delete issues and related items
Bug 26992 - On serial collection page, impossible to delete issues and related items
Bug 26992 - Fix serial issue and related items deletion on serial collection page
Bug 26992: Fix serial issue and related items deletion on serial collection page
Bug 26992: Fix serial issue and related items deletion on serial collection page
Bug 26992: Fix serial issue and related items deletion on serial collection page

Description Frédéric Demians 2020-11-10 15:59:12 UTC
Since bug 17674, it's possible to delete subscriptions issues and their items in batch. But items deletion doesn't work anymore because it uses C4::Items::DelItemCheck() function which has been obsoleted.
Comment 1 Frédéric Demians 2020-11-10 16:03:35 UTC
Created attachment 113417 [details] [review]
Bug 26992 - On serial collection page, impossible to delete issues and related items

serial-collection.pl uses C4::Items::DelItemCheck() function which is obsolte.

Test plan:

1. Create a subscription with items created when receiving.
2. Receive several issues, and create corresponding items.
3. On Serial collection page (serial-collection.pl), select several issues.
4. Click on button Delete selected issues
5. A confirmation page is displayed. Enable "Delete associated items?".
   Then click on "Yes, delete" button.

   You get this error: Undefined subroutine &C4::Items::DelItemCheck

6. Apply the patch an repeat steps 3-5.
   Check that issues and related items are deleted.
Comment 2 Blou 2020-11-12 20:28:35 UTC
Created attachment 113582 [details] [review]
Bug 26992 - On serial collection page, impossible to delete issues and related items

serial-collection.pl uses C4::Items::DelItemCheck() function which is obsolte.

Test plan:

1. Create a subscription with items created when receiving.
2. Receive several issues, and create corresponding items.
3. On Serial collection page (serial-collection.pl), select several issues.
4. Click on button Delete selected issues
5. A confirmation page is displayed. Enable "Delete associated items?".
   Then click on "Yes, delete" button.

   You get this error: Undefined subroutine &C4::Items::DelItemCheck
6. Apply the patch an repeat steps 3-5.
   Check that issues and related items are deleted.

Signed-off-by: Blou <blou@inlibro.com>
Comment 3 Blou 2020-11-12 20:30:37 UTC
This functionality crashes without this patch.  Not sure how it got this far (probably not used much), but this should be fixed for 20.11
Comment 4 Julian Maurice 2020-11-13 12:56:18 UTC
Created attachment 113599 [details] [review]
Bug 26992: Fix serial issue and related items deletion on serial collection page

serial-collection.pl uses C4::Items::DelItemCheck() function which is
obsolete.

Test plan:

1. Create a subscription with items created when receiving.
2. Receive several issues, and create corresponding items.
3. On Serial collection page (serial-collection.pl), select several issues.
4. Click on button Delete selected issues
5. A confirmation page is displayed. Enable "Delete associated items?".
   Then click on "Yes, delete" button.

   You get this error: Undefined subroutine &C4::Items::DelItemCheck

6. Apply the patch an repeat steps 3-5.
   Check that issues and related items are deleted.

Signed-off-by: Blou <blou@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 5 Jonathan Druart 2020-11-16 09:46:14 UTC
Why not using find?

for my $itemnumber (@itemnumbers) {
    my $items = Koha::Items->search({ itemnumber => $itemnumber });
    while ( my $item = $items->next ) {
        my $deleted_item = eval { $item->safe_delete };

vs

for my $itemnumber (@itemnumbers) {
    my $item = Koha::Items->find( $itemnumber );
    my $deleted_item = eval { $item->safe_delete };


or, even

my $items = Koha::Items->search({ itemnumber => \@itemnumbers });
while ( my $item = $items->next ) {
    my $deleted_item = eval { $item->safe_delete };
Comment 6 Frédéric Demians 2020-11-16 12:04:49 UTC
> my $items = Koha::Items->search({ itemnumber => \@itemnumbers });
> while ( my $item = $items->next ) {
>     my $deleted_item = eval { $item->safe_delete };

Yes, it's better.
Comment 7 Frédéric Demians 2020-11-16 12:05:36 UTC
Created attachment 113661 [details] [review]
Bug 26992 - On serial collection page, impossible to delete issues and related items

serial-collection.pl uses C4::Items::DelItemCheck() function which is obsolte.

Test plan:

1. Create a subscription with items created when receiving.
2. Receive several issues, and create corresponding items.
3. On Serial collection page (serial-collection.pl), select several issues.
4. Click on button Delete selected issues
5. A confirmation page is displayed. Enable "Delete associated items?".
   Then click on "Yes, delete" button.

   You get this error: Undefined subroutine &C4::Items::DelItemCheck

6. Apply the patch an repeat steps 3-5.
   Check that issues and related items are deleted.
Comment 8 Frédéric Demians 2020-11-16 12:11:08 UTC
Sorry, I had to reverse to "Needs Signoff"
Comment 9 David Nind 2020-11-16 18:44:25 UTC
Created attachment 113681 [details] [review]
Bug 26992 - On serial collection page, impossible to delete issues and related items

serial-collection.pl uses C4::Items::DelItemCheck() function which is obsolte.

Test plan:

1. Create a subscription with items created when receiving.
2. Receive several issues, and create corresponding items.
3. On Serial collection page (serial-collection.pl), select several issues.
4. Click on button Delete selected issues
5. A confirmation page is displayed. Enable "Delete associated items?".
   Then click on "Yes, delete" button.

   You get this error: Undefined subroutine &C4::Items::DelItemCheck

6. Apply the patch an repeat steps 3-5.
   Check that issues and related items are deleted.

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Joonas Kylmälä 2020-11-17 09:38:11 UTC
The commit title needs to tell what the commit does, not what the bug is (<https://wiki.koha-community.org/wiki/Commit_messages#Examples>). Also the formatting is not right, so something like

Bug 26992: Fix broken issues deletion on serial collection page

would be better.

Then I'm also wondering why there needs to be eval here:

> my $deleted_item = eval { $item->safe_delete };
Comment 11 Frédéric Demians 2020-11-17 10:04:03 UTC
> Then I'm also wondering why there needs to be eval here:

> > my $deleted_item = eval { $item->safe_delete };

I copy-paste this from somewhere, I don't recall where... It seems from the
Koha::Item module, and usage from other scripts, that the canonical form
should be now:

my $error = $item->safe_delete;
if ($error eq 1) { # not deleted

I will re-factorize the patch appropriately. Thanks.
Comment 12 Frédéric Demians 2020-11-17 10:12:11 UTC
Created attachment 113694 [details] [review]
Bug 26992 - Fix serial issue and related items deletion on serial collection page

serial-collection.pl uses C4::Items::DelItemCheck() function which is obsolete.

Test plan:

1. Create a subscription with items created when receiving.
2. Receive several issues, and create corresponding items.
3. On Serial collection page (serial-collection.pl), select several issues.
4. Click on button Delete selected issues
5. A confirmation page is displayed. Enable "Delete associated items?".
   Then click on "Yes, delete" button.

   You get this error: Undefined subroutine &C4::Items::DelItemCheck

6. Apply the patch an repeat steps 3-5.
   Check that issues and related items are deleted.
Comment 13 Frédéric Demians 2020-11-17 10:14:45 UTC
To David Nind: very sorry for having to resubmit and clear your signoff.
Comment 14 Joonas Kylmälä 2020-11-17 10:16:59 UTC
Thanks, one more thing: please use the "Bug XXX: syntax" instead of "Bug XXX -"
Comment 15 Jonathan Druart 2020-11-17 11:41:08 UTC
Created attachment 113703 [details] [review]
Bug 26992: Fix serial issue and related items deletion on serial collection page

serial-collection.pl uses C4::Items::DelItemCheck() function which is obsolete.

Test plan:

1. Create a subscription with items created when receiving.
2. Receive several issues, and create corresponding items.
3. On Serial collection page (serial-collection.pl), select several issues.
4. Click on button Delete selected issues
5. A confirmation page is displayed. Enable "Delete associated items?".
   Then click on "Yes, delete" button.

   You get this error: Undefined subroutine &C4::Items::DelItemCheck

6. Apply the patch an repeat steps 3-5.
   Check that issues and related items are deleted.
Comment 16 Frédéric Demians 2020-11-17 12:06:01 UTC
Thanks Joonas and Jonathan.
Comment 17 David Nind 2020-11-17 17:59:32 UTC
(In reply to Frédéric Demians from comment #13)
> To David Nind: very sorry for having to resubmit and clear your signoff.

No problem! Sign off on the way.
Comment 18 David Nind 2020-11-17 18:00:39 UTC
Created attachment 113753 [details] [review]
Bug 26992: Fix serial issue and related items deletion on serial collection page

serial-collection.pl uses C4::Items::DelItemCheck() function which is obsolete.

Test plan:

1. Create a subscription with items created when receiving.
2. Receive several issues, and create corresponding items.
3. On Serial collection page (serial-collection.pl), select several issues.
4. Click on button Delete selected issues
5. A confirmation page is displayed. Enable "Delete associated items?".
   Then click on "Yes, delete" button.

   You get this error: Undefined subroutine &C4::Items::DelItemCheck

6. Apply the patch an repeat steps 3-5.
   Check that issues and related items are deleted.

Signed-off-by: David Nind <david@davidnind.com>
Comment 19 Tomás Cohen Arazi 2020-11-19 14:05:25 UTC
Created attachment 113815 [details] [review]
Bug 26992: Fix serial issue and related items deletion on serial collection page

serial-collection.pl uses C4::Items::DelItemCheck() function which is obsolete.

Test plan:

1. Create a subscription with items created when receiving.
2. Receive several issues, and create corresponding items.
3. On Serial collection page (serial-collection.pl), select several issues.
4. Click on button Delete selected issues
5. A confirmation page is displayed. Enable "Delete associated items?".
   Then click on "Yes, delete" button.

   You get this error: Undefined subroutine &C4::Items::DelItemCheck

6. Apply the patch an repeat steps 3-5.
   Check that issues and related items are deleted.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Jonathan Druart 2020-11-20 07:19:03 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 21 Andrew Fuerste-Henry 2020-12-09 21:34:04 UTC
Pushed to 20.05.x for 20.05.07
Comment 22 Victor Grousset/tuxayo 2020-12-10 16:40:19 UTC
Missing dependencies for 19.11.x, shouldn't affect it then.