Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix updating of the remote CIM after a processor change. #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eigood
Copy link

@eigood eigood commented Mar 7, 2022

We had been using Authorize.Net CIM services successfully for quite a while now. Then, a few weeks ago, the bank and authorize.net decided they didn't like each other(for various reasons), and moqui started saying to connect authorize.net to update the config. Ok, no issues, this sort of thing happens. So, while that reconfiguring was going on(at a snails pace), moqui still needed to be able to process transactions. We then switched the processor to TEST_APPROVE.

Meanwhile, due to another bug(timezone shift), there was a window of time where the end of month subscription hadn't been renewed. Said user then created a new CreditCard, and signed back up again.

So now, the database had, over time, a Party with a gatewayCimId. Then, a paymentMethod with a gatewayCimId, followed by a paymentMethod with out a gatewayCimId(during the point when TEST_APPROVE was active).

Upon switching the processor back to Authorize.Net, and attempting to call store#PaymentMethodInfo, there was an error from the remote that a duplicate entry already existed. This was due to moqui calling "create" instead of "update".

The attached patch fixes this, by checking to see if any previous paymentMethods have a valid gatewayCimId, and using that when the current paymentMethod doesn't have one.

previously, but then due to having to switch to the ALWAYS_APPROVE
processor, if the user happens to create a new CreditCard, and then the
processor is switched back, then make certain to call "update" instead
of "create".

This can occur when the bank and authorize.net have disconnected their
settings, and they are taking time to reconnect the money end of things.
@jonesde
Copy link
Member

jonesde commented Mar 8, 2022

I may be seeing this wrong, but it looks like line 92 is removed in this PR when it should remain... otherwise there is a with no before it.

The other weird thing about this is that it looks for other PaymentMethod records by owner party and gateway config only, meaning if the customer had a PaymentMethod for a different credit card it would be found and updated.

The scenario you are describing is a known pain point with Authorize.NET. The only solution I've found is to ignore their concept of a CIM ID on the Party and always use a separate pair of CIM IDs for each payment method... so that a duplicate CC can still be stored because otherwise Authorize.NET handles these in a horrible way if somehow local data gets at all out of sync with the data they have (horrible design!).

The solution I'd propose, and one I've proposed in the past but was never implemented by the client that ran into it because there are also customer service manual workarounds, is to add a PaymentMethod.gatewayCustomerCimId field as a companion to the existing gatewayCimId field, and use that value instead of Party.gatewayCimId (fallback to that if a record does not have one, but otherwise always do a create with a new customer profile and payment method profile, and save the customer profile ID in the PaymentMethod.gatewayCustomerCimId to be added).

@eigood
Copy link
Author

eigood commented Mar 8, 2022

Bother; you're right, this is a slight bug with working on an external staging server, then a cut-n-paste error when bringing the change back to my desktop. I'll push a fix shortly, after I re-validated the fix.

As for your other comment, about changing the payment method; the "edit" function passes in the current paymentMethodId, with updated info. If that includes a brand new, unrelated cardType/cardNumber, then it just adds a new PaymentMethod, keeping the connected history. It's(the frontend) is not calling "create" methods, just update, so there is a single-line of payment method history.

Authorize.Net also doesn't seem to allow for multiple card profiles. If it did, then calling createCustomerPaymentProfileRequest many times would be allowed(it's not, the error we get when doing so says "Could not save CIM payment method 100518, message: A duplicate customer payment profile already exists."

fixed, then the result processing also needed an additional update(to
place the previous $foundGatewayCimId back into the new $paymentMethod).
@eigood eigood force-pushed the fix-update-cim-after-processor-switcheroo branch from b41a847 to 8e05b65 Compare March 9, 2022 19:17
@jonesde
Copy link
Member

jonesde commented Mar 11, 2022

Going back to my original comments, as long as this entity-find is in the patch without a restriction to avoid picking up other credit cards, etc for the customer, these changes are a bad idea... could use a Payment Method (Credit Card) CIM ID that is really for a totally different credit card:

<entity-find entity-name="mantle.account.method.PaymentMethod" list="partyPaymentMethodsWithCim">
   <econdition field-name="ownerPartyId" from="paymentMethod.ownerPartyId"/>
   <econdition field-name="paymentGatewayConfigId" from="paymentGatewayConfigId"/>
   <econdition field-name="gatewayCimId" operator="is-not-null"/>
   <order-by field-name="-from-date"/>
</entity-find>

From other comments, there might be some confusion about the TWO different CIM IDs that Authorize.NET uses: one for the customer (on Party) and one for the credit card (on PaymentMethod). If different customer profiles are used for new cards, there is never a conflict with an existing card because they are on different customer profiles. That is the workaround that IMO should, sooner or later, be implemented. Anything else is not likely to solve the problem... if you're running into the same problem I've seen in the past which it should like you may be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants