From: Lebbeous Fogle-Weekley Date: Wed, 23 Jan 2013 22:01:46 +0000 (-0500) Subject: Acq: Try harder to associate incoming EDI messages with exact right account X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=4292695f7894b4918aab75cdfc3e3f14e2e9d7f3;p=working%2FEvergreen.git Acq: Try harder to associate incoming EDI messages with exact right account 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 --- 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 8a7bfc496a..854c80e905 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm @@ -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};