Acq: Improvements to account-matching incoming EDI messages

The way the EDI fetcher works gives us a problem. That process iterates
over EDI accounts for which it has FTP host and credential information,
downloads documents from each of those sites, and files the messages
within those documents under the EDI account from which the login
credentials came. The problem is that in practice the exact same host
and login information is used by multiple accounts under the same
vendor, and files relating to these sub-accounts are commingled, so that
you can't make the decision about which messages should be filed under
which accounts based on the name of the document or its location. You
have to make that decision later, based on its contents.

We are already incompletely doing this, distinguishing between
sub-accounts under which we could file our messages when the vendor
specifies the buyer's SAN next to the specific sub-account number *and*
those sub-accounts belong to different Evergreen org units. We still
need ways to distinguish in other cases.

This will do what is natural for at least one vendor, and match the
message content against the vendacct field of the acq.edi_account table.

*Also,*

We were re-retrieving the working acq.edi_message row from the database
before writing it, throwing away possible changes to the object in hand
made by O::A::Acq::EDI::process_parsed_msg(). We should only do that in
the case where that function has raised an exception.

We were doing the same kind of thing in another place actually inside
process_parsed_msg() where we set the edi_message's purchase_order field
based on the first lineitem encountered if the message itself didn't
specify a valid PO identifier.

This supports making account-correction work for ORDRSP messages in
addition to INVOIC messages.

We also propagate that same correction to the provider and shipper
fields of any invoices that get created from said edi_message.

Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>
Signed-off-by: Kathy Lussier <klussier@masslnc.org>
Signed-off-by: Ben Shum <bshum@biblio.org>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
index a7b05a4..ee978e2 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
@@ -253,8 +253,8 @@ sub process_retrieval {
         eval { $class->process_parsed_msg($account, $incoming, $msg_hash) };
 
         $e->xact_begin;
-        $incoming = $e->retrieve_acq_edi_message($incoming->id);
         if ($@) {
+            $incoming = $e->retrieve_acq_edi_message($incoming->id);
             $logger->error($@);
             $incoming->status('proc_error');
             $incoming->error_time('now');
@@ -560,21 +560,60 @@ sub process_message_buyer {
                     $logger->info(
                         $log_prefix . sprintf(
                             "changing edi_account from %d to %d based on " .
-                            "vendcode '%s'",
-                            $orig_acct->id, $message->account, $vendcode
+                            "vendcode '%s' (%d match(es))",
+                            $orig_acct->id, $message->account, $vendcode,
+                            scalar(@$other_accounts)
                         )
                     );
+
+                    # If we've updated the message's account, and if we're
+                    # dealing with an invoice, we should update the invoice's
+                    # provider and shipper fields. XXX what's the difference
+                    # between shipper and provider, really?
+                    if ($eg_inv) {
+                        $eg_inv->provider(
+                            $eg_inv->shipper($other_accounts->[0]->provider)
+                        );
+                    }
                 }
             }
 
             last;
 
-        } elsif ($eg_inv) {
+        } else {
+
+            my $accts = $e->search_acq_edi_account({vendacct => $buyer});
 
-            my $acct = $e->search_acq_edi_account({vendacct => $buyer})->[0];
+            if (@$accts) {
+                if (grep { $_->id == $message->account } @$accts) {
+                    $logger->warn(
+                        $log_prefix . sprintf(
+                            "Not changing edi_account because we found " .
+                            "(%d) matching vendacct(s), one of which " .
+                            "being on the edi_account we already had",
+                            scalar(@$accts)
+                        )
+                    );
+                }
+
+                $logger->info(
+                    $log_prefix . sprintf(
+                        "changing edi_account from %d to %d based on " .
+                        "vendacct '%s' (%d match(es))",
+                        $message->account, $accts->[0]->id, $buyer,
+                        scalar(@$accts)
+                    )
+                );
+
+                # Both $message and $eg_inv should be saved later by the caller.
+                $message->account($accts->[0]->id);
+                if ($eg_inv) {
+                    $eg_inv->receiver($accts->[0]->owner);
+                    $eg_inv->provider(
+                        $eg_inv->shipper($accts->[0]->provider)
+                    );
+                }
 
-            if ($acct) {
-                $eg_inv->receiver($acct->owner);
                 last;
             }
         }
@@ -621,14 +660,18 @@ sub process_parsed_msg {
         if (not $incoming->purchase_order) {                
             # PO should come from the EDI message, but if not...
 
-            # fetch the latest copy
-            $incoming = $e->retrieve_acq_edi_message($incoming->id);
+            # NOTE: We used to refetch $incoming here, but that discarded
+            # changes made by process_message_buyer() above, and is not
+            # necessary since our caller just did that before invoking us.
+
             $incoming->purchase_order($li->purchase_order); 
 
-            unless($e->update_acq_edi_message($incoming)) {
-                $logger->error("EDI: unable to update edi_message " . $e->die_event);
-                next;
-            }
+            # NOTE: $li *just* came from the database, so if this update fails
+            # we should actually die() and thereby abort any changes from this
+            # entire message, because something weird is happening.
+            die(
+                "EDI: unable to update edi_message ". $e->die_event->{textcode}
+            ) unless $e->update_acq_edi_message($incoming);
         }
 
         my $lids = $e->json_query({