moov-io / paygate

A RESTful API enabling electronic payments to be submitted and received without a deep understanding payment file specification
http://moov.io
Apache License 2.0
129 stars 31 forks source link

Enrich accountIDs and customerIDs #573

Closed joshsadler closed 3 years ago

joshsadler commented 3 years ago

When building a UI against the transfers endpoint, it provides source and destination accountIDs and customerIDs, which are necessary for showing a meaningful UI.

For each transfer in a list (default limit/count is ~25 transfers per request) there are likely two customers and two accounts, which is 4 requests. That would equal 100 requests a browser/client would have to make to hydrate a UI, which is WELL above the number of concurrent requests that can be made. Related, there is no endpoint to fetch a single accountID at this time.

Ideally, we'd have some orchestration in a service that would return the customer and account models in the source and destination objects, perhaps like:

{
  "transferID": "UUID",
  "source": {
    "customer": {
      "customerID": "UUID",
      "firstName": "Josh",
      "lastName": "Sadler",
      "email": "josh@moov.io",
      ...
    },
    "account": {
      "accountID": "UUID",
      "type": "Checking",
      "maskedAccountNumber": "***1234",
      "status": "validated"
      ...
   }
  "amount": {
    "currency": "USD",
    "value": "1000"
  }
  ...
}

I have no strong opinions about how we achieve this, just that it not be orchestrated on a frontend.

adamdecaf commented 3 years ago

This change would use https://github.com/moov-io/customers/issues/170 to grab those two Accounts.

jmbrown412 commented 3 years ago

I talked with @InfernoJJ about this and we came up with a different solution than stuffing all of this orchestration into paygate.

In the short term, what if we created an endpoint in customers that could perform a batch fetch of customers and accounts in one call? Something like: POST /customers-reports/customers-by-account [ <account id1>, <account id2>, ...] => [ { account: {...}, customer: {...} }, { account: {...}, customer: {...} }, { account: {...}, customer: {...} } ... ]

So for a transfer, the UI could make one API call to the customers service to get all of the customer and account objects for that transfer?

In the longer term, we should probably look at creating a new service named something like ux-reports which can make these orchestration kind of calls in one call from the UI.

@joshsadler, @adamdecaf Thoughts?

InfernoJJ commented 3 years ago

The biggest bonus of this is the decrease I/O pre-LB and post-LB and effectively eliminating it because it would just be one DB quiet rather than 100.

pre-LB = 10 seconds to load. post-LB = 2 seconds to load. Batching = 50-100ms.

On Fri, Sep 25, 2020 at 10:23 AM Jordan Brown notifications@github.com wrote:

I talked with @InfernoJJ https://github.com/InfernoJJ about this and we came up with a different solution than stuffing all of this orchestration into paygate.

In the short term, what if we created an endpoint in customers that could perform a batch fetch of customers and accounts in one call? Something like:

POST /customers-reports/customers-by-account [ , <account id2>, ...] => [ { account: {...}, customer: {...} }, { account: {...}, customer: {...} }, { account: {...}, customer: {...} } ... ]

So for a transfer, the UI could make one API call to the customers service to get all of the customer and account objects for that transfer?

In the longer term, we should probably look at creating a new service named something like ux-reports which can make these orchestration kind of calls in one call from the UI.

@joshsadler https://github.com/joshsadler Thoughts?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/moov-io/paygate/issues/573#issuecomment-698992474, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFMTIX6FP2QZWZ2PBDSU5DSHSYYLANCNFSM4RTM3ELA .

joshsadler commented 3 years ago

@jmbrown412 the individual transfer UI or API isn't the concern, it's the transfers list UI for an organization.

The data we get for the list of transfers matches the functionality we want to expose through the UI, with the exception of displayable information for customers and accounts on each individual transfer in the list.

I don't think this counter-proposal solves the problem.

jmbrown412 commented 3 years ago

The proposed new endpoint in customers should take care of the exception you mentioned for the displayable information for customers and accounts. @joshsadler Wanna jump on a short call to discuss?

jmbrown412 commented 3 years ago

Moving this to blocked/waiting until we come up with an agreeable approach to solving the backend orchestration of data from multiple services.

InfernoJJ commented 3 years ago

https://drive.google.com/file/d/114_VJQxm-tiPKnRnsqMtyNNxcDgpoxBa/view?usp=sharing