Bug 40671

Summary: Expand Koha::Hold->revert_waiting to handle all found statuses
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: blocker    
Priority: P5 - low CC: emily.lamancusa, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 40058    
Bug Blocks: 21572, 40672    
Attachments: Bug 40671: Refactor Koha::Hold->revert_waiting into revert_found
Bug 40671: Update method calls from revert_waiting to revert_found
Bug 40671: Refactor Koha::Hold->revert_waiting into revert_found
Bug 40671: Update method calls from revert_waiting to revert_found

Description Tomás Cohen Arazi (tcohen) 2025-08-19 13:55:36 UTC
The original method -RevertWaitingStatus- was actually used to revert any `found` status, not just waiting. The `Koha::Hold->revert_waiting()` method implemented on bug 40058 throws an exception if the hold is not waiting. This causes errors in a couple places (reverting in transit holds, reverting in processing holds, etc).

We need to document things better and cover all the use cases with tests to avoid this. Will do my best.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-08-19 14:09:59 UTC
Created attachment 185541 [details] [review]
Bug 40671: Refactor Koha::Hold->revert_waiting into revert_found

This patch refactors the revert_waiting method to handle all found statuses
instead of just waiting holds, making it more versatile and consistent.

Changes:
* Renamed method from revert_waiting() to revert_found()
* Expanded functionality to handle all found statuses (W, T, P)
* Updated exception logic to check is_found instead of is_waiting
* Changed exception message from hold_not_waiting to hold_not_found
* Added comprehensive tests for all found statuses
* Updated POD documentation to reflect new behavior

The method now reverts any hold with a found status back to a regular
hold with priority 1, regardless of whether it was waiting, in transit,
or in processing.

To test:
1. Apply the patches
2. Run tests:
   $ ktd --shell
  k$ cd /kohadevbox/koha
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: All tests pass including new subtests for T and P statuses
3. Verify all hold test suites pass:
  k$ prove t/db_dependent/Koha/Hold*.t
=> SUCCESS: All hold-related tests pass
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi (tcohen) 2025-08-19 14:10:02 UTC
Created attachment 185542 [details] [review]
Bug 40671: Update method calls from revert_waiting to revert_found

This patch updates all existing calls to the renamed method throughout
the codebase to maintain compatibility.

Changes:
* Updated C4::Circulation calls to use revert_found()
* Updated C4::Reserves calls to use revert_found()
* Updated test files to use revert_found()
* Maintained all existing functionality and behavior

All existing functionality is preserved - this is purely a method
name change to reflect the expanded capability.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Emily Lamancusa (emlam) 2025-08-19 18:31:21 UTC
Created attachment 185562 [details] [review]
Bug 40671: Refactor Koha::Hold->revert_waiting into revert_found

This patch refactors the revert_waiting method to handle all found statuses
instead of just waiting holds, making it more versatile and consistent.

Changes:
* Renamed method from revert_waiting() to revert_found()
* Expanded functionality to handle all found statuses (W, T, P)
* Updated exception logic to check is_found instead of is_waiting
* Changed exception message from hold_not_waiting to hold_not_found
* Added comprehensive tests for all found statuses
* Updated POD documentation to reflect new behavior

The method now reverts any hold with a found status back to a regular
hold with priority 1, regardless of whether it was waiting, in transit,
or in processing.

To test:
1. Apply the patches
2. Run tests:
   $ ktd --shell
  k$ cd /kohadevbox/koha
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: All tests pass including new subtests for T and P statuses
3. Verify all hold test suites pass:
  k$ prove t/db_dependent/Koha/Hold*.t
=> SUCCESS: All hold-related tests pass
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 4 Emily Lamancusa (emlam) 2025-08-19 18:31:23 UTC
Created attachment 185563 [details] [review]
Bug 40671: Update method calls from revert_waiting to revert_found

This patch updates all existing calls to the renamed method throughout
the codebase to maintain compatibility.

Changes:
* Updated C4::Circulation calls to use revert_found()
* Updated C4::Reserves calls to use revert_found()
* Updated test files to use revert_found()
* Maintained all existing functionality and behavior

All existing functionality is preserved - this is purely a method
name change to reflect the expanded capability.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>