From 2462d7f6138d56e80c96eca5cdd7af140498b91c Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Mon, 14 Oct 2013 17:28:07 -0400 Subject: [PATCH] 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 Signed-off-by: Kathy Lussier Signed-off-by: Ben Shum --- .../lib/OpenILS/Application/Acq/EDI.pm | 69 +++++++++++++++---- 1 file changed, 56 insertions(+), 13 deletions(-) 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 a7b05a4684..ee978e23c4 100644 --- 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({ -- 2.43.2