Acq: Try harder to associate incoming EDI messages with exact right account
authorLebbeous Fogle-Weekley <lebbeous@esilibrary.com>
Wed, 23 Jan 2013 22:01:46 +0000 (17:01 -0500)
committerLebbeous Fogle-Weekley <lebbeous@esilibrary.com>
Tue, 5 Feb 2013 22:17:25 +0000 (17:17 -0500)
Sites use many very nearly identical EDI accounts (same host and
credentials) with different values only for the label and the vendcode.
This allows mapping of an order to a profile on the vendor side.

The problem with this is that the edi_fetcher and the processes it kicks
off did not know how to map incoming messages to the right account based
on vendcode.  That code simply iterated through accounts, using host
information and login credentials, and grabbing what it can find, as if
there will be no other Evergreen-side EDI "accounts" with the same
hostname and loging credentials.

This should help with that.

Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>
Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm

index 8a7bfc4..854c80e 100644 (file)
@@ -116,16 +116,25 @@ sub retrieve_core {
             # my $remote_file = $rf_starter . $_;
             my $description = sprintf "%s/%s", $account->host, $remote_file;
             
-            # deduplicate vs. acct/filenames already in DB
+            # deduplicate vs. acct/filenames already in DB.
+            #
+            # The reason we match against host/username/password/in_dir
+            # is that there may be many variant accounts that point to the
+            # same FTP site and credentials.  If we only checked based on
+            # acq.edi_account.id, we'd not find out in those cases that we've
+            # already processed the same file before.
             my $hits = $e->search_acq_edi_message([
                 {
-                    account     => $account->id,
+                    "+acqedi" => {
+                        host => $account->host,
+                        username => $account->username,
+                        password => $account->password,
+                        in_dir => $account->in_dir
+                    },
                     remote_file => $remote_file,
-                    status      => {'in' => [qw/ processed /]},     # if it never got processed, go ahead and get the new one (try again)
-                    # create_time => 'NOW() - 60 DAYS',     # if we wanted to allow filenames to be reused after a certain time
-                    # ideally we would also use the date from FTP, but that info isn't available via RemoteAccount
-                }
-                # { flesh => 1, flesh_fields => {...}, }
+                    status      => {'in' => [qw/ processed /]},
+                },
+                { join => {"acqedi" => {}} }
             ]);
             if (scalar(@$hits)) {
                 $logger->debug("EDI: $remote_file already retrieved.  Skipping");
@@ -426,18 +435,103 @@ sub nice_string {
     # return substr($string,0,$head) . "... " . substr($string, -1*$tail);
 }
 
+# process_message_buyer() is used in processing both INVOIC
+# messages as well as ORDRSP ones.  As such, the $eg_inv parameter is
+# optional.
+sub process_message_buyer {
+    my ($class, $e, $msg_hash, $message,  $log_prefix, $eg_inv) = @_;
+
+    # some vendors encode the account number as the SAN.
+    # starting with the san value, then the account value, 
+    # treat each as a san, then an acct number until the first success
+    for my $buyer ( ($msg_hash->{buyer_san}, $msg_hash->{buyer_acct}) ) {
+        next unless $buyer;
+
+        # some vendors encode the SAN as "$SAN $vendcode"
+        my $vendcode;
+        ($buyer, $vendcode) = $_ =~ /(\S+)\s*(\S+)?$/;
+
+        my $addr = $e->search_actor_org_address(
+            {valid => "t", san => $buyer})->[0];
+
+        if ($addr) {
+
+            $eg_inv->receiver($addr->org_unit) if $eg_inv;
+
+            if (defined $vendcode) {
+                # The vendcode can give us the opportunity to change the
+                # acq.edi_account with which our acq.edi_message is associated
+                # in case it's wrong.
+
+                my $orig_acct = $e->retrieve_acq_edi_account($message->account);
+                my $other_accounts = $e->search_acq_edi_account(
+                    {
+                        id => {"!=" => $orig_acct->id},
+                        vendcode => $vendcode,
+                        host => $orig_acct->host,
+                        username => $orig_acct->username,
+                        password => $orig_acct->password,
+                        in_dir => $orig_acct->in_dir
+                    }
+                );
+
+                if (@$other_accounts) {
+                    if (@$other_accounts == 1) {
+                        # We can update this object because the caller saves
+                        # it with cstore later.
+                        $message->account($other_accounts->[0]->id);
+
+                        $logger->info(
+                            $log_prefix . sprintf(
+                                "changing edi_account from %d to %d based on " .
+                                "vendcode '%s'",
+                                $orig_acct->id, $message->account, $vendcode
+                            )
+                        );
+                    } else {
+                        $logger->info(
+                            $log_prefix .
+                            "too many matching edi_account's based on " .
+                            "vendcode $vendcode to be certain which to use"
+                        );
+                    }
+                }
+            }
+
+            last;
+
+        } elsif ($eg_inv) {
+
+            my $acct = $e->search_acq_edi_account({vendacct => $buyer})->[0];
+
+            if ($acct) {
+                $eg_inv->receiver($acct->owner);
+                last;
+            }
+        }
+    }
+}
+
 # parts of this process can fail without the entire
 # thing failing.  If a catastrophic error occurs,
 # it will occur via die.
 sub process_parsed_msg {
     my ($class, $account, $incoming, $msg_hash) = @_;
 
+    # INVOIC
     if ($incoming->message_type eq 'INVOIC') {
         return $class->create_acq_invoice_from_edi(
             $msg_hash, $account->provider, $incoming);
     }
 
     # ORDRSP
+
+    #  First do this for the whole message...
+    my $e = new_editor;
+    $class->process_message_buyer($e, $msg_hash, $incoming, "ORDRSP processing");
+    $e->disconnect;
+
+    #  ... now do this stuff per-lineitem.
     for my $li_hash (@{$msg_hash->{lineitems}}) {
         my $e = new_editor(xact => 1);
 
@@ -692,8 +786,8 @@ sub edi_date_to_iso {
 # process_jedi().
 # Return boolean success indicator.
 sub create_acq_invoice_from_edi {
-    my ($class, $invoice, $provider, $message) = @_;
-    # $invoice is O::U::EDIReader hash
+    my ($class, $msg_data, $provider, $message) = @_;
+    # $msg_data is O::U::EDIReader hash
     # $provider is only a pkey
     # $message is Fieldmapper::acq::edi_message
 
@@ -710,48 +804,22 @@ sub create_acq_invoice_from_edi {
     $eg_inv->recv_method("EDI");
 
     $eg_inv->recv_date(
-        $class->edi_date_to_iso($invoice->{invoice_date}));
-
-
-    # some vendors encode the account number as the SAN.
-    # starting with the san value, then the account value, 
-    # treat each as a san, then an acct number until the first success
-    for my $buyer ( ($invoice->{buyer_san}, $invoice->{buyer_acct}) ) {
-        next unless $buyer;
-
-        # some vendors encode the SAN as "$SAN $vendcode"
-        $buyer =~ s/\s.*//g;
-
-        my $addr = $e->search_actor_org_address(
-            {valid => "t", san => $buyer})->[0];
-
-        if ($addr) {
+        $class->edi_date_to_iso($msg_data->{invoice_date}));
 
-            $eg_inv->receiver($addr->org_unit);
-            last;
-
-        } else {
 
-            my $acct = $e->search_acq_edi_account({vendacct => $buyer})->[0];
-
-            if ($acct) {
-                $eg_inv->receiver($acct->owner);
-                last;
-            }
-        }
-    }
+    $class->process_message_buyer($e, $msg_data, $message, $log_prefix, $eg_inv);
 
     if (!$eg_inv->receiver) {
         die($log_prefix .
             sprintf("unable to determine buyer (org unit) in invoice; ".
                 "buyer_san=%s; buyer_acct=%s",
-                ($invoice->{buyer_san} || ''), 
-                ($invoice->{buyer_acct} || '')
+                ($msg_data->{buyer_san} || ''), 
+                ($msg_data->{buyer_acct} || '')
             )
         );
     }
 
-    $eg_inv->inv_ident($invoice->{invoice_ident});
+    $eg_inv->inv_ident($msg_data->{invoice_ident});
 
     if (!$eg_inv->inv_ident) {
         die($log_prefix . "no invoice ID # in INVOIC message; " . shift);
@@ -760,9 +828,9 @@ sub create_acq_invoice_from_edi {
     my @eg_inv_entries;
     my @eg_inv_cancel_lis;
 
-    $message->purchase_order($invoice->{purchase_order});
+    $message->purchase_order($msg_data->{purchase_order});
 
-    for my $lineitem (@{$invoice->{lineitems}}) {
+    for my $lineitem (@{$msg_data->{lineitems}}) {
         my $li_id = $lineitem->{id};
 
         if (!$li_id) {
@@ -826,7 +894,7 @@ sub create_acq_invoice_from_edi {
         'DL' => ['SHP', 'Delivery']
     );
 
-    for my $charge (@{$invoice->{misc_charges}}) {
+    for my $charge (@{$msg_data->{misc_charges}}) {
         my $eg_inv_item = Fieldmapper::acq::invoice_item->new;
 
         my $amount = $charge->{charge_amount};