Feature Request: Cancel Transits, Don't Delete Them

Bug #1612752 reported by Chris Sharp
22
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

In PINES we get multiple helpdesk tickets per month from staff who can't figure out what happened to items after a transit is aborted. When system adminstration staff are asked to investigate, we can only glean so much information from the database since the "abort transit" process deletes the action.transit_copy row from the database, leaving gaps in the chain of custody of the item. My suggestion is to add a "cancel_time" row to the action.transit_copy table (which would in turn be inherited by action.hold_transit_copy and action.reservation_transit_copy). I'm working on a branch now.

On a side note, when searching for possible duplicates for this request, I came across bug 1570091, in which the language "Cancel Transit" is being used in the web client. I support a move towards the "cancel" term over and above "abort", which is at best programmer-y and at worst potentially inflammatory. I intend to change "abort" to "cancel" in the branch I'm working on for that reason, but I'm open to opinions or discussion.

Tags: pullrequest
Revision history for this message
Shula Link (slink-g) wrote :

I'll confirm on the user end that this is an issue - aborted transits can cause serious problems when you're trying to track down a missing item.

Changed in evergreen:
status: New → Confirmed
importance: Undecided → Wishlist
assignee: nobody → Chris Sharp (chrissharp123)
Revision history for this message
Chris Sharp (chrissharp123) wrote :

Branch here: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1612752_transit_cancel_improvements

It works on current master as far as I am able to tell - I would appreciate testing and code review.

In my second (top) commit, I changed the end-user-facing "abort" term to "cancel". I'm open to changes for that too.

tags: added: pullrequest
Changed in evergreen:
milestone: none → 2.11-beta
Revision history for this message
Mike Rylander (mrylander) wrote :

Pushing this to 2.next for more testing. One consideration will need to be reports ... we'll need at least a warning in the release notes that this could include canceled transits in with in-process transits.

Changed in evergreen:
milestone: 2.11-beta → 2.next
Changed in evergreen:
assignee: Chris Sharp (chrissharp123) → nobody
Kathy Lussier (klussier)
Changed in evergreen:
milestone: 2.next → 2.12-beta
Revision history for this message
Kathy Lussier (klussier) wrote :

Chris,

I'm getting merge conflicts on this branch. Could you resolve the conflicts and push an updated branch?

While you're in there, could you also add a release notes entry, taking into account the suggestion Mike made in #3. Or is there a way we could distinguish between the in-process transits and canceled transits in reports?

Revision history for this message
Chris Sharp (chrissharp123) wrote :

Conflicts resolved and force pushed. I also added release notes as requested.

Revision history for this message
Bill Erickson (berick) wrote :

Eyeballed the code and the only thing that caught my attention were the permission.perm_list description updates introduced here:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commitdiff;h=52722a04c20c24e5119911468c08b415eebeb5dd#patch3

We normally try to avoid changing configurable text in the DB if it's been locally modified, using something along the lines of:

UPDATE permission.perm_list
SET description = '<NEW DESCRIPTION>'
WHERE code = '<SOME CODE>'
    AND description = '<OLD STOCK DESCRIPTION>';

Revision history for this message
Chris Sharp (chrissharp123) wrote :
Changed in evergreen:
milestone: 2.12-beta → 2.next
Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

More code review and testing. I have pushed a new branch which contains sign-off's to Chris' commits and adds a new commit to prevent displaying canceled transits in the webstaff Transit List UI, consistent with the XUL UI.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1612752-transit-cancel-signoff

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Galen Charlton (gmc) wrote :

Pushed to master, along with a follow-up that applies the terminology change to labels in the web staff client. Thanks, Chris and Bill!

Changed in evergreen:
milestone: 3.next → 3.0-alpha
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.