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

PaymentInstrumentType filter on TransactionSearchRequest always returns empty result #117

Open
peter-sabath opened this issue Oct 13, 2021 · 3 comments

Comments

@peter-sabath
Copy link

General information

  • SDK/Library version: 5.9.0
  • Environment: production
  • Language, language version, and OS: Windows 10 (German), Windows Server 2012 (English)

Issue description

We have a problem with PaymentInstrumentType filter on an TransactionSearchRequest
using tsr.PaymentInstrumentType.Is(PaymentInstrumentType.PAYPAL_ACCOUNT.ToString()); results in an empty result set.

looking at the source I don't understand why this is still a MultipleValueNode<TransactionSearchRequest, string> typed filter and not an EnumMultipleValueNode<TransactionSearchRequest, PaymentInstrumentType> one as it is used for other enum based filters.

Another unexpected behavior is that using

tsr.CreditCardCardType.Is(CreditCardCardType.MASTER_CARD.ToString()); returns different results than tsr.CreditCardCardType.Is(CreditCardCardType.MASTER_CARD);

The string typed one returns the expected results, the "older" string-based usage returns fewer elements.

@peter-sabath
Copy link
Author

Update

I checked how those values get serialized to XML and found a difference:

using tsr.PaymentInstrumentType.Is(PaymentInstrumentType.PAYPAL_ACCOUNT.ToString()); creates the following xml:

<?xml version="1.0"?>
<search>
  <payment-instrument-type type="array">
    <item>PAYPAL_ACCOUNT</item>
  </payment-instrument-type>
</search>

while manually using tsr.PaymentInstrumentType.Is("paypal_account"); works and creates that XML

<?xml version="1.0"?>
<search>
  <payment-instrument-type type="array">
    <item>paypal_account</item>
  </payment-instrument-type>
</search>

Similar there is a difference for the CreditCardType (and most likely for the other enums as well)
Using tsr.CreditCardCardType.Is(CreditCardCardType.MASTER_CARD.ToString()); create

<?xml version="1.0"?>
<search>
  <credit-card-card-type type="array">
    <item>MASTER_CARD</item>
  </credit-card-card-type>
</search>

while using tsr.CreditCardCardType.Is(CreditCardCardType.MASTER_CARD); create

<?xml version="1.0"?>
<search>
  <credit-card-card-type type="array">
    <item>MasterCard</item>
  </credit-card-card-type>
</search>

So the mixed-case variant is the right one compared to your docs, but I am wondering why the all-caps version returns any result at all.

For the PaymentInstrumentType it is also an casing issue.

@Epreuve
Copy link
Contributor

Epreuve commented Oct 21, 2021

Hey @peter-sabath

Thanks for the thorough details on the issue. There's a few things at play here, so I figured I'd try to address them one at a time.

  1. So the mixed-case variant is the right one compared to your docs, but I am wondering why the all-caps version returns any result at all.

That's a great question. The API the search portion of the SDK hits is maintained by another team, so I don't have too much knowledge around it's inner workings. I'd assume it's a quirk related to some older SDK version, but I can't say for sure. We'll take a look with that team and see if we can work in unison to make it a bit more failure resistant to slightly-off values to prevent confusion in the future.

  1. We have a problem with PaymentInstrumentType filter on an TransactionSearchRequest
    using tsr.PaymentInstrumentType.Is(PaymentInstrumentType.PAYPAL_ACCOUNT.ToString()); results in an empty result set.

    looking at the source I don't understand why this is still a MultipleValueNode<TransactionSearchRequest, string> typed filter and not an EnumMultipleValueNode<TransactionSearchRequest, PaymentInstrumentType> one as it is used for other enum based filters.

A great find. I also can't find a reason as to why it's currently typed as a String either. It's been that way for a number of years (since 3.0 at least, which was in 2016) so it's likely an artifact of that major version bump that we overlooked.

Ideally we'd move it over to the same EnumMultipleValue collection same as CreditCardCardType and continue to leverage the DescriptionAttributes and GetDescription (via our EnumHelper) on PaymentInstrumentType, same as we do for CreditCardCardType. That should clean up the API there and also help to avoid the unnecessary confusion here.

As a stop gap, I'm going to test if we can at least control the downcase scenario with PaymentInstrumentType (and other string values) in the SDK, which should allow you to use the enum albeit still calling ToString until we can shift to the proper enum type constraint. As I believe this would constitute a breaking change, we'd have to push it in the next major version bump to stay semantic, but I'll see what we can come up with.

@hollabaq86
Copy link
Contributor

For our internal notekeeping, ticket number 1614

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

No branches or pull requests

3 participants