Acq: lineitems display as "on order" even after all copies have been cancelled

[Kathy Lussier]

In the PO interface, you can either choose to cancel a selected lineitem or you can choose to cancel individual copies belonging to that lineitem. If all individual copies belonging to the lineitem are set to canceled, the lineitem status on the main PO screen still display as "on order". I have attached a screenshot that shows the copies screen where all copies are set to cancelled followed by the PO screen where the status still displays as on order.

This behavior differs from what happens if individual copies are set to received. Once all copies are set to received, the lineitem status on the main PO screen also updates to received.

This issue is particularly problematic when using EDI. If a vendor posts an EDI response message saying that an order is cancelled, it updates the status at the copy level. However, it would take some digging for staff to see that these copies were cancelled since the status on the PO screen remains as "on order."

[Bill Erickson (berick)]

This issue is resolved with bug #1065270 -- which should be merged soon, after we hammer out the remaining edge cases. Marking as duplicate.

-----
commit a22546583284a9b9f67fc53b79220ca6bf6ddb56
Author: Bill Erickson <email address hidden>
Date: Tue Dec 4 10:00:26 2012 -0500

    EDI: ensure lineitem 'state' matches cancel state

    When cancelling a lineitem becuase all linked copies are cancelled,
    ensure that the lineitem state is set to "cancelled".

[Kathy Lussier (klussier)]

I just tested cancellations in master. When manually setting copies to cancelled, the lineitem still displays as "on order." However, I'm fairly sure EDI is now marking the entire lineitem as cancelled. Perhaps 1065270 fixed this issue with EDI cancellations, but not manual ones?

In any case, I'm removing the duplicate status.

[Jennifer Pringle (jpringle-u)]

Confirmed that this bug is still present on 2.4.0.

[Kathy Lussier (klussier)]

From a discussion in IRC on 7/10/2013

# 09:55:51 berick kmlussier: i assume the LI would just adopt the cancel_reason of the last-canceled item?
# 09:56:23 berick even though the copies themselves may have a variety of cancel reasons..
# 09:57:48 berick hm, or do we need a new cancel reason, "OMG, All My Copies Are Gone!?!"
# 09:58:24 kmlussier berick: Just checked with my acq person. Could it list all the cancel reasons?
# 10:00:13 berick kmlussier: so, my main question is, we need to cancel the lineitem too, and a lineitem can only have 1 cancel reason
# 10:00:52 berick there's no reason the UI can't show all of the copy cancel reasons, but that's a separate question, i think
# 10:55:09 kmlussier berick: In lieu of a new LI cancel reason of "multiple" (or, as you suggest, OMG, All My Copies Are Gone!?!), I think using the reason from the last-canceled copy makes sense. In most cases, I imagine the cancel reasons would be the same.

[Leslie St. John (lstjohn-deactivatedaccount)]

still present in 2.7.1. Manual cancellation of all copies on a LI does not change LI state. One result - can not search on LI state with any accuracy.

[Galen Charlton (gmc)]

Just to confirm, am I correct in assuming that cancelling all of the line item copies should result in doing the full cancellation on the line item, including cancelling any hold requests?

[Galen Charlton (gmc)]

Following up on my comment #7, I note that it appears that if an EDI cancellation cancels the last line item copy, then therefore marks the line item as cancelled... any hold requests on the linked bib get orphaned.

[Jason Etheridge (phasefx)]

If I wanted to move forward with this, would it be sane to modify sub cancel_lineitem_detail in Acq/Order.pm, right before the final return, so that it examines the cancel_reason for all sibling LIDs, and if they're all cancelled (and cancelled with a reason where keep_debits = false?), then invoke sub cancel_lineitem (ignoring whether it succeeds or not) with the same cancel_reason being fed to cancel_lineitem_detail? And then maybe double-check whether the UI needs a tickle or not to refresh the lineitem state? Then any question on canceling holds could be punted to another bug if needed for fixing sub cancel_lineitem?

[Mike Rylander (mrylander)]

Jason, the approach sound sane to me. As for whether to cancel if some LIDs have keep_debits=true, I think we need some clarity from someone that has a better memory than me on the exact semantics of that field. To wit:

Bill, should the LI cancel if there are canceled keep_debits=true LIDS?

For the UI tickle, perhaps we could add a key to the hash returned by cancel_lineitem_detail that lists the IDs of the remaining uncanceled LIDs. When the list is 0 long the UI could refresh (maybe re-fetch the LI and replace the object)?

Thanks!

[Jason Etheridge (phasefx)]

So one suggestion I received (and apologies if this is spelled out already in the ticket) was cancel the lineitem if all the LIDs are cancelled, but use the last cancel reason where keep_debits=true if there is one, otherwise, use the cancel_reason just used for the LID.

[Jason Etheridge (phasefx)]

I have a branch for this here:

collab/phasefx/lp978095-autocanceling-lineitems @ working

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/phasefx/lp978095-autocanceling-lineitems

[Jason Etheridge (phasefx)]

For testing, this is what I did with a fresh concerto dataset:

Web Client -> Acquisitions -> Purchase Orders -> Search -> 2 -> Copies(2) on lineitem #3
on copy ACQ0001, cancel, Delayed: Backordered, Cancel Copy, OK
on copy ACQ0002, cancel, Canceled: By Vendor, Cancel Copy, OK
the UI should refresh and show lineitem #3 as Backordered

For refreshing the data for iterative testing, I used the following SQL in the database:

BEGIN;

TRUNCATE acq.fund CASCADE;
TRUNCATE acq.fund_allocation CASCADE;
TRUNCATE acq.fund_debit CASCADE;
TRUNCATE acq.funding_source CASCADE;
TRUNCATE acq.funding_source_credit CASCADE;
TRUNCATE acq.lineitem CASCADE;
TRUNCATE acq.lineitem_detail CASCADE;
TRUNCATE acq.provider CASCADE;
TRUNCATE acq.provider_holding_subfield_map CASCADE;
TRUNCATE acq.purchase_order CASCADE;

\i ~/git/Evergreen/Open-ILS/tests/datasets/sql/acq.sql

COMMIT;

[Jason Etheridge (phasefx)]

random aside, lineitems #4 and #5 are already canceled in that dataset, but their copies were canceled with different cancel reasons :)

[Andrea Neiman (aneiman)]

Patch is working for me in light testing, though I'd appreciate someone more knowledgeable about Acq taking a look for signoff purposes.

[Bill Erickson (berick)]

Hey Jason, I'm curious about the choice to handle the lineitem cancellation via a secondary transaction. Any reason not to tack it on to the existing transaction?

[Jason Etheridge (phasefx)]

Oops, sorry Bill, I missed your comment. I think my mindset was: don't want some hypothetical failure at canceling the lineitem to negate the attempt to cancel the lineitem detail.

[Jason Etheridge (phasefx)]

In other words, I was thinking of auto-canceling the lineitem as a convenience thing, not an all or nothing behavior to be bundled with the more specific cancelation. But I could be argued into a different view; don't feel strongly about it.

[Bill Erickson (berick)]

Thanks, Jason. I would argue it's not a convenience thing, but how the application should function (or die loudly trying). I think it should be in the same transaction.

[Jason Etheridge (phasefx)]

Just for reference, I noticed two other branches in the working repo that identify this bug:
  working/user/csharp/lp978095_acq_lineitems_with_canceled_copies
  working/user/ldw/lp978095_cancel_lineitem_no_copies

[Jason Etheridge (phasefx)]

I changed this to use just one transaction, rebased it against master, and force pushed a new branch here:
collab/phasefx/lp978095-autocanceling-lineitems

[Bill Erickson (berick)]

Thanks, Jason! I've pushed a follow up branch which includes sign-off for your commit, plus another commit. The additional commit ensures that the cancel reason for the currently-canceled copy is used if it's a keep-debits reason, instead of choosing one essentially at random from the keep-debits true copies. Updated the docs to clarify. Removed the eval, since it won't save us from a failed transaction (and could mask a useful error message).

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

[Mike Rylander (mrylander)]

Picked to master through rel_3_0. Thanks, Jason and Bill!