663/mk/standardize op client requests #971

mkurapov · 2023-01-12T21:04:07Z

Changes proposed in this pull request

Update incoming payment & outgoing payment client list create complete requests to use paymentPointer as an argument instead of url, e.g.:

await client.incomingPayment.create({ url, accessToken, body: { description, incomingAmount } } // before
await client.incomingPayment.create({ paymentPointer, accessToken }, { description, incomingAmount }) // after

Update tests to get 100% coverage:

Context

Parent issue: Create Open Payments client library #663

Checklist

Related issues linked using fixes #number
Tests added/updated
Documentation added
Make sure that all checks pass

…t pointers

mkurapov · 2023-01-12T23:46:00Z

packages/open-payments/src/client/outgoing-payment.test.ts

  const openApiValidators = mockOpenApiResponseValidators()

-  describe('createOutgoingPaymentRoutes', (): void => {


not needed, since routes describe block takes care of this

wilsonianb · 2023-01-12T23:25:29Z

packages/open-payments/src/client/ilp-stream-connection.test.ts

+        jest
+          .spyOn(openApi, 'createResponseValidator')
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          .mockImplementation(mockResponseValidator as any)


Could we check that the spy is called?

It's already testing this (albeit indirectly): if the createResponseValidator doesn't get the proper path and method when its called, the test will fail based on the expectation of true on L57

Ah, the true! 👍

It's not the easiest to see when first reading it, but that was the best solution I could come up with to test the correct validator was passed in

wilsonianb · 2023-01-12T23:28:31Z

packages/open-payments/src/client/incoming-payment.ts

+interface RequestWithUrlArgs {
  url: string
  accessToken: string
 }

-interface PostArgs<T = undefined> {
-  url: string
-  body?: T
-  accessToken: string
-}
-
-interface ListGetArgs {
+interface RequestWithPaymentPointerArgs {


What do you think of (Sub)ResourceRequestArgs and CollectionRequestArgs?

That might also somewhat future-proof this for

Make resource collection URLs explicit open-payments#206

Yup, I like that better

packages/open-payments/src/client/incoming-payment.test.ts

packages/open-payments/src/test/helpers.ts

mkurapov · 2023-01-13T00:12:53Z

packages/open-payments/src/client/index.ts

@@ -41,6 +41,20 @@ export interface RouteDeps extends BaseDeps {
  logger: Logger
 }

+export interface UnauthenticatedResourceRequestArgs {


it's a bit wordy given what this contains, but I like how the types read in the grant.ts, where the first method (to create a grant) has UnauthenticatedResourceRequestArgs, while all the other ones are ResourceRequestArgs with the explicit accessToken

wilsonianb · 2023-01-13T00:20:56Z

packages/open-payments/src/client/index.ts

+export interface ResourceRequestArgs {
+  url: string
+  accessToken: string
+}
+
+export interface CollectionRequestArgs {
+  paymentPointer: string
+  accessToken: string
+}


Suggested change

export interface ResourceRequestArgs {

url: string

accessToken: string

}

export interface CollectionRequestArgs {

paymentPointer: string

accessToken: string

}

interface AuthenticatedRequestArgs {

accessToken: string

}

export interface ResourceRequestArgs

extends UnauthenticatedResourceRequestArgs,

AuthenticatedRequestArgs {}

export interface CollectionRequestArgs extends AuthenticatedRequestArgs {

paymentPointer: string

}

dragosp1011 · 2023-01-13T11:57:20Z

packages/open-payments/src/test/helpers.ts

+    for (const key in override) {
+      process.env[key] = override[key]
+    }


Suggested change

for (const key in override) {

process.env[key] = override[key]

}

Object.assign(process.env, override)

# Conflicts: # packages/open-payments/src/client/quote.ts # packages/open-payments/src/test/helpers.ts

mkurapov · 2023-01-13T21:04:55Z

packages/open-payments/src/client/quote.test.ts

@@ -35,29 +35,6 @@ describe('quote', (): void => {
  const paymentPointer = 'http://localhost:1000/.well-known/pay'
  const accessToken = 'accessToken'

-  describe('createQuoteRoutes', (): void => {


already tested via routes describe block lower in the file

mkurapov added 4 commits January 12, 2023 14:00

chore(open-payments): standardize incoming payment requests

440e588

chore(open-payments): standardize outgoing payment requests

d22c304

chore(open-payments): update tests for ilp-stream-connection & paymen…

c196901

…t pointers

chore(open-payments): add test mock functions

85aa9d5

github-actions bot added pkg: open-payments type: source Changes business logic type: tests Testing related labels Jan 12, 2023

chore(open-payments): add remaining tests for request functions

a59b1e5

mkurapov commented Jan 12, 2023

View reviewed changes

wilsonianb reviewed Jan 12, 2023

View reviewed changes

chore(open-payments): address comments

8a235dd

mkurapov commented Jan 13, 2023

View reviewed changes

mkurapov marked this pull request as ready for review January 13, 2023 00:13

mkurapov requested review from wilsonianb, raducristianpopa, rico191013, beniaminmunteanu, dragosp1011 and Tymmmy January 13, 2023 00:13

wilsonianb previously approved these changes Jan 13, 2023

View reviewed changes

chore(open-payments): use types suggestion

581a5e4

mkurapov dismissed wilsonianb’s stale review via 581a5e4 January 13, 2023 00:40

mkurapov requested a review from wilsonianb January 13, 2023 01:08

dragosp1011 reviewed Jan 13, 2023

View reviewed changes

wilsonianb previously approved these changes Jan 13, 2023

View reviewed changes

mkurapov added 2 commits January 13, 2023 13:45

Merge branch 'main' into 663/mk/standardize-op-client-requests

b7035d2

# Conflicts: # packages/open-payments/src/client/quote.ts # packages/open-payments/src/test/helpers.ts

chore(open-payments): remove extraneous tests

6490fff

mkurapov dismissed wilsonianb’s stale review via 6490fff January 13, 2023 20:57

chore(open-payments): address object copy suggestion

d42bc5b

mkurapov commented Jan 13, 2023

View reviewed changes

mkurapov requested a review from wilsonianb January 13, 2023 21:05

wilsonianb approved these changes Jan 13, 2023

View reviewed changes

mkurapov merged commit b3ce7af into main Jan 13, 2023

mkurapov deleted the 663/mk/standardize-op-client-requests branch January 13, 2023 23:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

663/mk/standardize op client requests #971

663/mk/standardize op client requests #971

mkurapov commented Jan 12, 2023 •

edited

Loading

mkurapov Jan 12, 2023

wilsonianb Jan 12, 2023

mkurapov Jan 13, 2023 •

edited

Loading

wilsonianb Jan 13, 2023

mkurapov Jan 13, 2023

wilsonianb Jan 12, 2023

mkurapov Jan 12, 2023

mkurapov Jan 13, 2023

wilsonianb Jan 13, 2023

dragosp1011 Jan 13, 2023

mkurapov Jan 13, 2023

		const openApiValidators = mockOpenApiResponseValidators()

		describe('createOutgoingPaymentRoutes', (): void => {

663/mk/standardize op client requests #971

663/mk/standardize op client requests #971

Conversation

mkurapov commented Jan 12, 2023 • edited Loading

Changes proposed in this pull request

Context

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkurapov Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkurapov commented Jan 12, 2023 •

edited

Loading

mkurapov Jan 13, 2023 •

edited

Loading