From 54b605762c38c1af99607d3992fb75c4f9739679 Mon Sep 17 00:00:00 2001 From: Paulo Margarido <64600052+paulomarg@users.noreply.github.com> Date: Mon, 15 Aug 2022 16:23:08 -0400 Subject: [PATCH] Applying comments from review --- docs/usage/billing.md | 12 +++---- src/billing/__tests__/check.test.ts | 55 +++++++++++++++++++++++++---- src/billing/check.ts | 16 ++++----- 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/docs/usage/billing.md b/docs/usage/billing.md index fba14cdb4..f18202a2b 100644 --- a/docs/usage/billing.md +++ b/docs/usage/billing.md @@ -40,21 +40,21 @@ This method will take in the following parameters: And return the following: -| Parameter | Type | Notes | -| ----------------- | ---------------------- | --------------------------------------------------- | -| `hasPayment` | `boolean` | Whether the store has already paid for the app | -| `confirmationUrl` | `string` / `undefined` | The URL to redirect to if payment is still required | +| Parameter | Type | Notes | +| ------------------- | ---------------------- | --------------------------------------------------- | +| `hasPayment` | `boolean` | Whether the store has already paid for the app | +| `confirmBillingUrl` | `string` / `undefined` | The URL to redirect to if payment is still required | Here's a typical example of how to use `check`: ```ts -const {hasPayment, confirmationUrl} = await Shopify.Billing.check({ +const {hasPayment, confirmBillingUrl} = await Shopify.Billing.check({ session, isTest: true, }); if (!hasPayment) { - return redirect(confirmationUrl); + return redirect(confirmBillingUrl); } // Proceed with app logic diff --git a/src/billing/__tests__/check.test.ts b/src/billing/__tests__/check.test.ts index b24642528..c445ccadc 100644 --- a/src/billing/__tests__/check.test.ts +++ b/src/billing/__tests__/check.test.ts @@ -47,7 +47,7 @@ describe('check', () => { expect(response).toEqual({ hasPayment: false, - confirmationUrl: CONFIRMATION_URL, + confirmBillingUrl: CONFIRMATION_URL, }); expect(fetchMock).toHaveBeenCalledTimes(2); expect(fetchMock).toHaveBeenNthCalledWith( @@ -71,6 +71,26 @@ describe('check', () => { }), ); + test('defaults to test purchases', async () => { + fetchMock.mockResponses(EMPTY_SUBSCRIPTIONS, PURCHASE_ONE_TIME_RESPONSE); + + const response = await check({session}); + + expect(response).toEqual({ + hasPayment: false, + confirmBillingUrl: CONFIRMATION_URL, + }); + expect(fetchMock).toHaveBeenCalledTimes(2); + + const parsedBody = JSON.parse( + fetchMock.mock.calls[1][1]!.body!.toString(), + ); + expect(parsedBody).toMatchObject({ + query: expect.stringContaining('appPurchaseOneTimeCreate'), + variables: expect.objectContaining({test: true}), + }); + }); + test('does not request payment if there is one', async () => { fetchMock.mockResponses(EXISTING_ONE_TIME_PAYMENT); @@ -81,7 +101,7 @@ describe('check', () => { expect(response).toEqual({ hasPayment: true, - confirmationUrl: undefined, + confirmBillingUrl: undefined, }); expect(fetchMock).toHaveBeenCalledTimes(1); expect(fetchMock).toHaveBeenNthCalledWith( @@ -106,7 +126,7 @@ describe('check', () => { expect(response).toEqual({ hasPayment: false, - confirmationUrl: CONFIRMATION_URL, + confirmBillingUrl: CONFIRMATION_URL, }); expect(fetchMock).toHaveBeenCalledTimes(2); expect(fetchMock).toHaveBeenNthCalledWith( @@ -135,7 +155,7 @@ describe('check', () => { expect(response).toEqual({ hasPayment: true, - confirmationUrl: undefined, + confirmBillingUrl: undefined, }); expect(fetchMock).toHaveBeenCalledTimes(2); @@ -193,7 +213,7 @@ describe('check', () => { expect(response).toEqual({ hasPayment: false, - confirmationUrl: CONFIRMATION_URL, + confirmBillingUrl: CONFIRMATION_URL, }); expect(fetchMock).toHaveBeenCalledTimes(2); expect(fetchMock).toHaveBeenNthCalledWith( @@ -217,6 +237,29 @@ describe('check', () => { }), ); + test('defaults to test purchases', async () => { + fetchMock.mockResponses( + EMPTY_SUBSCRIPTIONS, + PURCHASE_SUBSCRIPTION_RESPONSE, + ); + + const response = await check({session}); + + expect(response).toEqual({ + hasPayment: false, + confirmBillingUrl: CONFIRMATION_URL, + }); + expect(fetchMock).toHaveBeenCalledTimes(2); + + const parsedBody = JSON.parse( + fetchMock.mock.calls[1][1]!.body!.toString(), + ); + expect(parsedBody).toMatchObject({ + query: expect.stringContaining('appSubscriptionCreate'), + variables: expect.objectContaining({test: true}), + }); + }); + test('does not request subscription if there is one', async () => { fetchMock.mockResponses(EXISTING_SUBSCRIPTION); @@ -227,7 +270,7 @@ describe('check', () => { expect(response).toEqual({ hasPayment: true, - confirmationUrl: undefined, + confirmBillingUrl: undefined, }); expect(fetchMock).toHaveBeenCalledTimes(1); diff --git a/src/billing/check.ts b/src/billing/check.ts index 2b5e40534..fb30ffb47 100644 --- a/src/billing/check.ts +++ b/src/billing/check.ts @@ -6,31 +6,31 @@ import {requestPayment} from './request_payment'; interface CheckInterface { session: SessionInterface; - isTest: boolean; + isTest?: boolean; } interface CheckReturn { hasPayment: boolean; - confirmationUrl?: string; + confirmBillingUrl?: string; } export async function check({ session, - isTest, + isTest = true, }: CheckInterface): Promise { if (!Context.BILLING) { - return {hasPayment: true, confirmationUrl: undefined}; + return {hasPayment: true, confirmBillingUrl: undefined}; } - let hasPayment; - let confirmationUrl; + let hasPayment: boolean; + let confirmBillingUrl: string | undefined; if (await hasActivePayment(session, isTest)) { hasPayment = true; } else { hasPayment = false; - confirmationUrl = await requestPayment(session, isTest); + confirmBillingUrl = await requestPayment(session, isTest); } - return {hasPayment, confirmationUrl}; + return {hasPayment, confirmBillingUrl}; }