Bug 28527 - Transfers that are cancelled whilst in transit should still appear as 'in transit' until they are received
Summary: Transfers that are cancelled whilst in transit should still appear as 'in tra...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
Depends on: 28294 28520
Blocks: 22160
  Show dependency treegraph
 
Reported: 2021-06-08 11:07 UTC by Martin Renvoize (ashimema)
Modified: 2025-10-22 14:03 UTC (History)
4 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 28527: Restore cancellation of transfers from Koha::Hold (1.31 KB, patch)
2021-06-09 13:33 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28527: Update detail to display cancelled in transit transfers (2.43 KB, patch)
2021-06-09 13:33 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28527: Deal with cancelled in transfer transfers when enqueueing (2.55 KB, patch)
2021-06-09 13:33 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28527: Add handling for cancelled in transit transfers at checkin (6.39 KB, patch)
2021-06-09 13:33 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 28527: Handle cancelled in-transit transfers (18.47 KB, patch)
2025-10-22 14:01 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2021-06-08 11:07:05 UTC
If a transfer is cancelled whilst it is in transit (datesent set, but datearrived not) then we should still return it as in transit.

Bug 27993 added a condition to the 'in_transit' boolean which should be reverted.
We should also update the get_transfer (and get_transfers) methods to catch such cases and return those still active transfers and deal with them appropriately at check-in. (i.e. invoke the reverse transfer or pick the next transfer from the queue and notify the librarian of what subsequent actions they should take).
Comment 1 Martin Renvoize (ashimema) 2021-06-08 11:59:52 UTC
Bug 28520 is very specific for the Reserve/Holds case, but does not account for transfers triggered by other means or cancelled by other means.
Comment 2 Martin Renvoize (ashimema) 2021-06-09 13:33:36 UTC
Created attachment 121739 [details] [review]
Bug 28527: Restore cancellation of transfers from Koha::Hold

This aptch restores the cancellation of the transfers in transit when a
hold is cancelled.
Comment 3 Martin Renvoize (ashimema) 2021-06-09 13:33:39 UTC
Created attachment 121740 [details] [review]
Bug 28527: Update detail to display cancelled in transit transfers
Comment 4 Martin Renvoize (ashimema) 2021-06-09 13:33:43 UTC
Created attachment 121741 [details] [review]
Bug 28527: Deal with cancelled in transfer transfers when enqueueing

Addeding to the queue should be allowed when the active transfer
(in_transit) has been cancelled.
Comment 5 Martin Renvoize (ashimema) 2021-06-09 13:33:46 UTC
Created attachment 121742 [details] [review]
Bug 28527: Add handling for cancelled in transit transfers at checkin
Comment 6 Koha Training Catalyst 2022-07-07 20:19:39 UTC
Can you please provide a test plan?
Comment 7 Laura Escamilla 2023-05-18 18:55:42 UTC
Blocking for now until a test plan is available for sign offs. Thanks!
Comment 8 Katrin Fischer 2023-05-18 19:09:02 UTC
Missing test plan = Failed QA
Comment 9 Lisette Scheer 2025-04-24 16:31:05 UTC
All the blockers are closed so I'm putting this back to failed QA for no test plan.
Comment 10 Martin Renvoize (ashimema) 2025-10-22 11:50:24 UTC
Behind 15310 commits... I don't hold much hope of the rebase here.
Comment 11 Martin Renvoize (ashimema) 2025-10-22 14:01:33 UTC
Created attachment 188298 [details] [review]
Bug 28527: Handle cancelled in-transit transfers

This patch addresses the issue where items with cancelled in-transit
transfers become stuck in an inconsistent state, preventing new
transfers from being created and causing confusion at checkin.

Problem:
--------
When a hold is cancelled while an item is in transit, the transfer is
cancelled but remains in an incomplete state (datesent populated,
datecancelled populated, datearrived not set). This causes:

1. Items cannot have new transfers created (blocked by cancelled transfer)
2. Checking in items at wrong locations doesn't provide clear messaging
3. No automatic workflow to return items to their original location
4. Cancelled transfers disappear from the system visibility

Solution:
---------
This patch implements a complete workflow for handling cancelled
in-transit transfers:

1. Koha::Item::Transfer->is_cancelled() - New method to check if a
   transfer has been cancelled

2. Koha::Item::Transfer->in_transit() - Modified to return true even
   for cancelled transfers (now only checks datesent and datearrived)

3. Koha::Item->get_transfers() - Modified to include cancelled transfers
   in the result set, making them visible in the system

4. Koha::Item->get_transfer() - Modified to skip over cancelled transfers
   and return the first active transfer

5. Koha::Hold->cancel() - When cancelling a hold, now automatically
   cancels the associated in-transit transfer with reason 'CancelReserve'

6. C4::Circulation::AddReturn() - Modified to loop through and receive
   all cancelled transfers at checkin, track the original holding branch,
   and set new 'TransferCancelled' message

7. circ/returns.pl - Added handler for TransferCancelled message that
   automatically creates a reverse transfer to return the item to its
   original holding branch using reason 'TransferCancellation'

8. returns.tt - Updated checkin modal to display appropriate messages
   for both wrong transfers and cancelled transfers

Test plan:
----------
1. Setup - Create three library branches: BranchA, BranchB, BranchC

2. Test hold cancellation cancels transfer:
   a) Create an item at BranchA
   b) Create a patron at BranchB
   c) Place a hold on the item for the patron
   d) At BranchA, check out the item (triggers transfer to BranchB)
   e) Verify transfer is created: SELECT * FROM branchtransfers WHERE
      itemnumber=<item> ORDER BY datesent DESC LIMIT 1
   f) Cancel the hold
   g) Verify transfer is cancelled: Check datecancelled is populated
   h) Verify cancellation_reason is 'CancelReserve'

3. Test cancelled transfers remain visible:
   a) Using item from test 2 with cancelled transfer
   b) Search for the item in staff interface
   c) View item details
   d) Verify cancelled transfer information is still visible

4. Test new transfers can be created after cancellation:
   a) With the same item from test 2
   b) Go to Circulation > Transfer
   c) Scan the item barcode
   d) Select BranchC as destination
   e) Verify transfer is created successfully (not blocked)
   f) Check branchtransfers table shows both cancelled and new transfer:
      SELECT * FROM branchtransfers WHERE itemnumber=<item>
      ORDER BY datesent DESC

5. Test checkin with cancelled transfer (wrong branch):
   a) Create fresh scenario: Item at BranchA, patron at BranchB
   b) Place hold on item for patron (triggers transfer A->B)
   c) Cancel the hold (cancels the transfer)
   d) Item is still physically in transit
   e) At BranchC (wrong branch), check in the item
   f) Verify modal appears with message: "Transfer was cancelled whilst
      in transit, please return item to: BranchA"
   g) Click OK
   h) Verify new transfer created from BranchC to BranchA
   i) Check branchtransfers table:
      - Old transfer has datearrived populated
      - New transfer exists with reason 'TransferCancellation'

6. Test checkin with cancelled transfer (correct branch):
   a) Create scenario: Item at BranchA with cancelled transfer
   b) Check in item at BranchA (original holding branch)
   c) Verify cancelled transfer is received (datearrived set)
   d) Verify item returns to available status at BranchA
   e) Verify appropriate messaging displayed to staff

7. Test get_transfer() skips cancelled transfers:
   a) Create item with one cancelled transfer and one active transfer
   b) Call $item->get_transfer() via prove or koha-shell
   c) Verify it returns the active transfer, not the cancelled one

8. Test multiple cancelled transfers:
   a) Create item at BranchA
   b) Create and cancel transfer to BranchB
   c) Create and cancel transfer to BranchC
   d) Create active transfer to BranchB (don't cancel)
   e) Verify get_transfer() returns the active transfer
   f) Verify get_transfers() returns all three transfers

9. Run unit tests:
   prove t/db_dependent/Koha/Item/Transfer.t
   prove t/db_dependent/Koha/Item.t
   prove t/db_dependent/Circulation/Returns.t

All tests should pass.
Comment 12 Martin Renvoize (ashimema) 2025-10-22 14:03:15 UTC
Yeah... that rebase was never going to work.  I've reworked it from scratch taking the original as inspiration and included a comprehensive test plan.

I'd love feedback, not sure I've thought it all through.