Bug 32529 - Holds in processing should block item deletion
Summary: Holds in processing should block item deletion
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 12556
Blocks: 32528
  Show dependency treegraph
 
Reported: 2022-12-23 20:44 UTC by Tomás Cohen Arazi
Modified: 2023-06-08 22:32 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.02


Attachments
Bug 32529: Add Koha::Holds->filter_by_found (2.87 KB, patch)
2022-12-26 18:31 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32529: Regression tests (4.24 KB, patch)
2022-12-26 18:31 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32529: Make Koha::Item->safe_to_delete consider 'P' holds as found (1.23 KB, patch)
2022-12-26 18:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32529: Add Koha::Holds->filter_by_found (2.92 KB, patch)
2022-12-27 13:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32529: Regression tests (4.29 KB, patch)
2022-12-27 13:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32529: Make Koha::Item->safe_to_delete consider 'P' holds as found (1.29 KB, patch)
2022-12-27 13:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32529: Add Koha::Holds->filter_by_found (2.98 KB, patch)
2022-12-28 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32529: Regression tests (4.35 KB, patch)
2022-12-28 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32529: Make Koha::Item->safe_to_delete consider 'P' holds as found (1.34 KB, patch)
2022-12-28 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2022-12-23 20:44:57 UTC
Currently, the Koha::Item->safe_to_delete method checks (among other things) if there are existing waiting or in-transit holds.

It seems that 'In processing' should block item deletion as well. I'm not familiar with SIP or how the 'P' flag is removed later in the process, but this feels wrong on its own.
Comment 1 Tomás Cohen Arazi 2022-12-26 18:31:50 UTC
Created attachment 144839 [details] [review]
Bug 32529: Add Koha::Holds->filter_by_found

This patch adds a method for filtering Koha::Holds resultsets by the
status belonging to Koha's concept of 'found'.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Holds.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 2 Tomás Cohen Arazi 2022-12-26 18:31:54 UTC
Created attachment 144840 [details] [review]
Bug 32529: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2022-12-26 18:32:00 UTC
Created attachment 144841 [details] [review]
Bug 32529: Make Koha::Item->safe_to_delete consider 'P' holds as found

Currently, the Koha::Item->safe_to_delete method checks (among other things) if there are existing waiting or in-transit holds.

It seems that 'In processing' should block item deletion as well.

This patch implements that.

To test:
1. Apply up to the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Item.t
=> FAIL: Boo! Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Nick Clemens 2022-12-27 13:08:44 UTC
Created attachment 144848 [details] [review]
Bug 32529: Add Koha::Holds->filter_by_found

This patch adds a method for filtering Koha::Holds resultsets by the
status belonging to Koha's concept of 'found'.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Holds.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Nick Clemens 2022-12-27 13:08:47 UTC
Created attachment 144849 [details] [review]
Bug 32529: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Nick Clemens 2022-12-27 13:08:51 UTC
Created attachment 144850 [details] [review]
Bug 32529: Make Koha::Item->safe_to_delete consider 'P' holds as found

Currently, the Koha::Item->safe_to_delete method checks (among other things) if there are existing waiting or in-transit holds.

It seems that 'In processing' should block item deletion as well.

This patch implements that.

To test:
1. Apply up to the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Item.t
=> FAIL: Boo! Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Katrin Fischer 2022-12-28 20:58:22 UTC
Created attachment 144874 [details] [review]
Bug 32529: Add Koha::Holds->filter_by_found

This patch adds a method for filtering Koha::Holds resultsets by the
status belonging to Koha's concept of 'found'.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Holds.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2022-12-28 20:58:27 UTC
Created attachment 144875 [details] [review]
Bug 32529: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2022-12-28 20:58:32 UTC
Created attachment 144876 [details] [review]
Bug 32529: Make Koha::Item->safe_to_delete consider 'P' holds as found

Currently, the Koha::Item->safe_to_delete method checks (among other things) if there are existing waiting or in-transit holds.

It seems that 'In processing' should block item deletion as well.

This patch implements that.

To test:
1. Apply up to the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Item.t
=> FAIL: Boo! Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Tomás Cohen Arazi 2023-01-05 12:26:49 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 11 Jacob O'Mara 2023-01-13 16:08:51 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 12 Lucas Gass 2023-01-24 18:19:03 UTC
Does not apply cleanly to 22.05.x, no backport. If needed please rebase