EDI Order Pusher needs tighter control over PO selection

[Blake GH]

EG 3.5

We've found that the new EDI order pusher (edi_order_pusher.pl) can select more orders for sending than it should. One such scenario is:

A provider is put into use for a long time. One that has never had an EDI account. And then, we add an EDI account. Once that is added, the EDI pusher will collect ALL of the PO's that don't have an associated ORDERS message but have state='on-order'. Which will be a bad thing.

Another scenario:
Once adopting the new order pusher software without checking the database, you might be surprised how many old PO's still have "on-order" and never received an ORDERS message. Those orders will then be pushed to the provider.

These issues can be mitigated if we applied a flag of some sort onto the PO when it's activated. One that would signify the method of delivery that this PO should be using. The related IRC chat:

http://irc.evergreen-ils.org/evergreen/2021-01-14#i_470867

The proposed fix is:

1. Add a column to acq.purchase_order (delivery_method TEXT DEFAULT 'none'
2. Allowed text phrases should be limited to "edi", "manual", "none"
3. "none" would be the default on row insertion for non-activated orders.
4. Teach edi_order_pusher about the new column and filter the targeted PO's accordingly.

[Tiffany Little (tslittle)]

What about your suggestion in bug 1803734 of just including a sane order_date?

[Galen Charlton (gmc)]

Noting that the change made in bug 1803734 did not turn out to be sufficient: old on-order POs that never should have had an EDI ORDERS message sent did get processed by edi_order_pusher.pl for one of our customers that switched to EDI attribute sets.

Setting the importance of this to high; errant EDI messages have the potential to cause financial consequences to the library.

[Tiffany Little (tslittle)]

To pose my original question again--would including a date restriction be sufficient? Say, 24 hours? Does anyone run their pusher less often than that? That would make sense to me, at least.

[Bill Erickson (berick)]

Patch to add a --min-date flag to the order pusher. Not sure this covers all scenarios but addresses the issue for us.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1911786-order-pusher-min-date

[Galen Charlton (gmc)]

I think Bill's patch is part of the solution, but not quite enough. I think Tiffany's idea of defaulting to 24 hours makes sense, and that could be done in conjunction with a --order-age flag. My emphasis would be on _defaulting_ to 24 hours or the like, as otherwise the foot-gun remains open.

(Also: one hole with the --min-date flag: it would be fragile in the case where different providers start out not using EDI but subsequently start using EDI, but on different dates.)

To suggest a different direction (that shouldn't get in the way of our putting in some sort of date/age constraint sooner rather than later), perhaps the state of ready-to-send-via-EDI should be made somewhat orthogonal to the order's activation state. For example, by adding a ready_to_send Boolean flag that defaults to false and gets actively set to true only when the order is activated _and_ the provider has an active EDI profile.

EDIT: And ready_to_send might be better as a edi_order_status column on acq.purchase_order that has several values:

- null: order has not and does not participate in EDI, either because it's not ready yet or the provider doesn't have an active EDI profile
- ready_to_send
- sent
- ready_to_resend

[Galen Charlton (gmc)]

If folks are amenable to the --order-age idea, I'm willing to put a patch together that also preserves the intent of Bill's --min-date proposal.

[Tiffany Little (tslittle)]

Fwiw, I'm in favor of Bill's patch+ the order-age idea. And +1 to defaulting to 24 hours.

[Christine Morgan (cmorgan-z)]

+1 to Bill's patch + the order-age idea and defaulting to 24 hours.

[Christine Morgan (cmorgan-z)]

Confirmed based on comments.