From 525beda0cb861574cbdb633fdfda1dd1d7a000eb Mon Sep 17 00:00:00 2001
From: Mike Rylander <mrylander@gmail.com>
Date: Tue, 16 Aug 2016 12:07:25 -0400
Subject: [PATCH] LP#1306666: Be more mindful with transit status unwinding

When unwinding the copy status wrapper provided by transits, we need to
make sure we don't cause problems due to potentially broken legacy data.
One such incarnation is the "hanging transit", or a transit that should
have been closed but, through some series of events was not, and now
risks damaging a circulation that occured after the transit.  To that end,
we will avoid unwinding the copy status based on an open transit unless
the copy is in "In Transit" status.

Secondly, while hold transits aborted by staff are supposed to be followed up
by a scan of the item (either immediately if the item is in hand, or when
the item physically arrives), such does not always happen.  This can lead
to confusion of other staff members.  To help address this, we will leave
these hold transited items in "In Transit" status, so that they will not
appear available at their home location, and not show up on pull lists.

Note that because of this second change these items will not be available for
opportunistic hold capture when they are scanned in the intended, above-
described worflow, as would be the case before this commit.

Signed-off-by: Mike Rylander <mrylander@gmail.com>
---
 .../lib/OpenILS/Application/Circ/Transit.pm        | 39 ++++++++++++++++------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Transit.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Transit.pm
index 32a616425d..28587a493a 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Transit.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Transit.pm
@@ -243,18 +243,37 @@ sub __abort_transit {
 
     return $e->die_event unless $e->delete_action_transit_copy($transit);
 
-    # Only change the copy status if the copy status is "In Transit" and the transit is aborted at home.
-    if ($copy->status == OILS_COPY_STATUS_IN_TRANSIT && $e->requestor->ws_ou == $copy->circ_lib) {
-        # recover the copy status
-        $copy->status( $transit->copy_status );
-        $copy->editor( $e->requestor->id );
-        $copy->edit_date('now');
-
+    # Only change the copy status if the copy status is "In Transit", to avoid clobbering
+    # circulations in the face of bad, legacy data.
+    if ($copy->status == OILS_COPY_STATUS_IN_TRANSIT) {
+
+        # If a hold transit (meaning, it had to be available) we force the status
+        # to reshelving at home and to in-transit when not at home. The latter indicates
+        # that the copy is not at the right location, and should be looked at. Unfortunately
+        # this will defeat op-capture when the copy is later scanned, possibly causing more
+        # work for staff.
         if ( $holdtransit ) {
-            $logger->info("setting copy to reshelving on hold transit abort");
-            $copy->status( OILS_COPY_STATUS_RESHELVING );
+            if ($e->requestor->ws_ou == $copy->circ_lib) {
+                $logger->info("setting copy to reshelving on hold transit abort at home");
+                $copy->status( OILS_COPY_STATUS_RESHELVING );
+                $copy->ischanged(1);
+            } else {
+                if (!$no_reset_hold) {
+                    $e->commit unless $no_commit;
+                    return 1;
+                }
+                $logger->info("leaving copy untouched on hold transit abort away from home");
+            }
+        } else {
+            $copy->status( $transit->copy_status );
+            $copy->ischanged(1);
+        }
+
+        if ($copy->ischanged) {
+            $copy->editor( $e->requestor->id );
+            $copy->edit_date('now');
+            return $e->die_event unless $e->update_asset_copy($copy);
         }
-        return $e->die_event unless $e->update_asset_copy($copy);
     }
 
 
-- 
2.11.0