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>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 2da5c04c2d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Koha/Hold.pm

@@ -896,6 +896,10 @@ sub fill {
 Reverts any 'found' hold back to a regular hold with a priority of 1.
 This method can revert holds in 'waiting' (W), 'in transit' (T), or 'in processing' (P) status.
 
+For waiting holds, the desk_id is also cleared since the hold is no longer waiting
+at a specific desk. For in transit and in processing holds, desk_id remains unchanged
+(typically NULL as these statuses don't set desk_id).
+
 =cut
 
 sub revert_found {
@@ -928,6 +932,7 @@ sub revert_found {
                     waitingdate    => undef,
                     expirationdate => $self->patron_expiration_date,
                     itemnumber     => $self->item_level_hold ? $self->itemnumber : undef,
+                    ( $self->is_waiting ? ( desk_id => undef ) : () ),
                 }
             )->store( { hold_reverted => 1 } );
 

t/db_dependent/Koha/Hold.t

@@ -1264,7 +1264,7 @@ subtest 'strings_map() tests' => sub {
 
 subtest 'revert_found() tests' => sub {
 
-    plan tests => 5;
+    plan tests => 6;
 
     subtest 'item-level holds tests' => sub {
 
@@ -1591,4 +1591,85 @@ subtest 'revert_found() tests' => sub {
 
         $schema->storage->txn_rollback;
     };
+
+    subtest 'desk_id handling tests' => sub {
+
+        plan tests => 12;
+
+        $schema->storage->txn_begin;
+
+        my $library = $builder->build_object( { class => 'Koha::Libraries' } );
+        my $desk    = $builder->build_object(
+            {
+                class => 'Koha::Desks',
+                value => { branchcode => $library->branchcode }
+            }
+        );
+        my $patron = $builder->build_object(
+            {
+                class => 'Koha::Patrons',
+                value => { branchcode => $library->branchcode }
+            }
+        );
+        my $item = $builder->build_sample_item( { library => $library->branchcode } );
+
+        my $hold = $builder->build_object(
+            {
+                class => 'Koha::Holds',
+                value => {
+                    borrowernumber => $patron->borrowernumber,
+                    biblionumber   => $item->biblionumber,
+                    itemnumber     => $item->itemnumber,
+                    branchcode     => $library->branchcode,
+                    priority       => 1,
+                    found          => undef,
+                }
+            }
+        );
+
+        # Test 1: Waiting hold - desk_id should be cleared
+        $hold->set_waiting( $desk->desk_id );
+        $hold->discard_changes;
+        is( $hold->desk_id, $desk->desk_id, 'desk_id set for waiting hold' );
+        ok( $hold->is_waiting, 'Hold is in waiting status' );
+
+        $hold->revert_found();
+        $hold->discard_changes;
+        is( $hold->desk_id, undef, 'desk_id cleared when reverting waiting hold' );
+        ok( !$hold->is_found, 'Hold is no longer in found status' );
+
+        # Test 2: In transit hold with desk_id - desk_id should be preserved
+        $hold->set_transfer();
+        $hold->desk_id( $desk->desk_id )->store();    # Manually set desk_id
+        $hold->discard_changes;
+        is( $hold->desk_id, $desk->desk_id, 'desk_id manually set for transit hold' );
+        ok( $hold->is_in_transit, 'Hold is in transit status' );
+
+        $hold->revert_found();
+        $hold->discard_changes;
+        is( $hold->desk_id, $desk->desk_id, 'desk_id preserved when reverting transit hold' );
+
+        # Test 3: In processing hold with desk_id - desk_id should be preserved
+        $hold->set_processing();
+        $hold->desk_id( $desk->desk_id )->store();    # Manually set desk_id
+        $hold->discard_changes;
+        is( $hold->desk_id, $desk->desk_id, 'desk_id manually set for processing hold' );
+        ok( $hold->is_in_processing, 'Hold is in processing status' );
+
+        $hold->revert_found();
+        $hold->discard_changes;
+        is( $hold->desk_id, $desk->desk_id, 'desk_id preserved when reverting processing hold' );
+
+        # Test 4: In transit hold without desk_id - desk_id should remain NULL
+        $hold->set_transfer();
+        $hold->desk_id(undef)->store();               # Ensure desk_id is NULL
+        $hold->discard_changes;
+        is( $hold->desk_id, undef, 'desk_id is NULL for transit hold without desk_id' );
+
+        $hold->revert_found();
+        $hold->discard_changes;
+        is( $hold->desk_id, undef, 'desk_id remains NULL after reverting transit hold without desk_id' );
+
+        $schema->storage->txn_rollback;
+    };
 };