Bug 32529

Summary: Holds in processing should block item deletion
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, kyle, lucas, martin.renvoize, nugged, tomascohen
Version: Main   
Hardware: All   
OS: All   
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
Circulation function:
Bug Depends on: 12556    
Bug Blocks: 32528    
Attachments: Bug 32529: Add Koha::Holds->filter_by_found
Bug 32529: Regression tests
Bug 32529: Make Koha::Item->safe_to_delete consider 'P' holds as found
Bug 32529: Add Koha::Holds->filter_by_found
Bug 32529: Regression tests
Bug 32529: Make Koha::Item->safe_to_delete consider 'P' holds as found
Bug 32529: Add Koha::Holds->filter_by_found
Bug 32529: Regression tests
Bug 32529: Make Koha::Item->safe_to_delete consider 'P' holds as found

Description Tomás Cohen Arazi (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (kidclamp) 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 (kidclamp) 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 (kidclamp) 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 (tcohen) 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 (lukeg) 2023-01-24 18:19:03 UTC
Does not apply cleanly to 22.05.x, no backport. If needed please rebase