Bug 40672

Summary: `desk_id` not cleared when `revert_found()` called
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: Hold requestsAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: emily.lamancusa, gmcharlt, lisette, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 40671, 24412    
Bug Blocks:    
Attachments: Bug 40672: Reset desk_id when reverting waiting holds
Bug 40672: Reset desk_id when reverting waiting holds

Description Tomás Cohen Arazi (tcohen) 2025-08-19 14:28:06 UTC
When `ModReserveAffect` is used to set a waiting status for a hold, if the session has a `desk_id` set, then the hold is marked as waiting and the `desk_id` is set for the hold.

Neither the original `RevertWaitingStatus` or the new `revert_found` methods reset the `desk_id`.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-08-19 14:50:29 UTC
Created attachment 185543 [details] [review]
Bug 40672: Reset desk_id when reverting waiting holds

This patch adds desk_id reset functionality to the revert_found method,
but only for waiting holds where desk_id is actually relevant.

Changes:
* Modified revert_found() to conditionally reset desk_id
* Only resets desk_id for waiting holds (found=W)
* Leaves desk_id unchanged for in transit (T) and in processing (P) holds
* Added comprehensive test coverage for all scenarios
* Updated POD documentation to explain desk_id behavior

The logic is conservative and only clears desk_id when it makes sense:
waiting holds have their desk assignment cleared since they are no longer
waiting at a specific desk, while transit and processing holds remain
unchanged as they do not use desk_id.

To test:
1. Apply patch
2. Run tests:
   $ ktd --shell
  k$ cd /kohadevbox/koha
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: All tests pass including new desk_id handling tests
3. Verify all hold test suites still pass:
  k$ prove t/db_dependent/Koha/Hold*.t
=> SUCCESS: All hold-related tests pass
4. Sign off :-D
Comment 2 Tomás Cohen Arazi (tcohen) 2025-08-19 14:52:18 UTC
I haven't found anything obvious for not doing it. My main goal is to fully document the behavior for this methods and make things explicit. If something's not correct we will fix the tests and change the code. But this seems about right.
Comment 3 Emily Lamancusa (emlam) 2025-08-19 19:06:50 UTC
Created attachment 185567 [details] [review]
Bug 40672: Reset desk_id when reverting waiting holds

This patch adds desk_id reset functionality to the revert_found method,
but only for waiting holds where desk_id is actually relevant.

Changes:
* Modified revert_found() to conditionally reset desk_id
* Only resets desk_id for waiting holds (found=W)
* Leaves desk_id unchanged for in transit (T) and in processing (P) holds
* Added comprehensive test coverage for all scenarios
* Updated POD documentation to explain desk_id behavior

The logic is conservative and only clears desk_id when it makes sense:
waiting holds have their desk assignment cleared since they are no longer
waiting at a specific desk, while transit and processing holds remain
unchanged as they do not use desk_id.

To test:
1. Apply patch
2. Run tests:
   $ ktd --shell
  k$ cd /kohadevbox/koha
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: All tests pass including new desk_id handling tests
3. Verify all hold test suites still pass:
  k$ prove t/db_dependent/Koha/Hold*.t
=> SUCCESS: All hold-related tests pass
4. Sign off :-D

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>