Skip to content

Commit

Permalink
Merge pull request #951 from Shopify/liz/remove-v3_webhookAdminContext
Browse files Browse the repository at this point in the history
Remove v3_webhookAdminContext future flag
  • Loading branch information
lizkenyon authored May 30, 2024
2 parents 69f7d0c + d681916 commit 5391201
Show file tree
Hide file tree
Showing 15 changed files with 3,667 additions and 9,956 deletions.
5 changes: 5 additions & 0 deletions .changeset/tame-boats-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/shopify-app-remix': major
---

Remove v3_webhookAdminContext future flag and enable functionality
62 changes: 62 additions & 0 deletions packages/apps/shopify-app-remix/docs/MIGRATION_V3.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,68 @@ export async function loader({request}: LoaderFunctionArgs) {
}
```

## Align the `admin` context object for webhooks

The `v3_webhookAdminContext` future flag has been removed and the feature has been enabled.

The `admin` context returned by `authenticate.webhook` didn't match the object returned by e.g. `authenticate.admin`, which could lead to confusion.

We updated the shape of the object returned by `authenticate.webhook` so that every authentication method returns a consistent format.
That reduces the mental load for developers as they shouldn't have to learn more than one way of doing the same thing.

To update your code:

- GraphQL:
- replace `admin?.graphql.query()` with `admin?.graphql()`
- the graphql query function now takes the query string as the first argument, and an optional settings object, as opposed to a single object
- REST:
- `admin.rest.get|post|put|delete()` remain unchanged
- `admin.rest.Product` (or other resource classes) are now under `admin.rest.resources.Product`

Before:

```ts
export async function action({request}: ActionFunctionArgs) {
const {admin} = await shopify.authenticate.webhook(request);

// GraphQL query
const response = await admin?.graphql.query<any>({
data: {
query: `query { ... }`,
variables: {myVariable: '...'},
},
});

// REST resource
const products = await admin?.rest.Product.all({
// ...
});

// ...
}
```

After:

```ts
export async function action({request}: ActionFunctionArgs) {
const {admin} = await shopify.authenticate.webhook(request);

// GraphQL query
const response = await admin?.graphql(`query { ... }`, {
variables: {myVariable: '...'},
});
const data = await response.json();

// REST resource
const products = await admin?.rest.resources.Product.all({
// ...
});

// ...
}
```

## Root import path deprecation

Apps can no longer import server-side functions using the following statements:
Expand Down
13,316 changes: 3,582 additions & 9,734 deletions packages/apps/shopify-app-remix/docs/generated/generated_docs_data.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,7 @@
"sectionContent": "These are the future flags supported in the current version.",
"listItems": [
{
"name": "v3_webhookAdminContext",
"value": "",
"description": "Returns the same `admin` context (`AdminApiContext`) from `authenticate.webhook` that is returned from `authenticate.admin`.\n\nSee [authenticate.webhook](/docs/api/shopify-app-remix/authenticate/webhook#example-admin) for more details.",
"isOptional": true
},
{
"name": "v3_lineItemBilling",
"name": "lineItemBilling",
"value": "",
"description": "Allows you to pass billing plans with up to two line items when creating a new app subscription.\n\n This allows you to create app subscriptions with both recurring and usage based components.",
"isOptional": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,7 @@ const data: LandingTemplateSchema = {
'These are the future flags supported in the current version.',
listItems: [
{
name: 'v3_webhookAdminContext',
value: '',
description:
'Returns the same `admin` context (`AdminApiContext`) from `authenticate.webhook` that is returned from `authenticate.admin`.' +
'\n\nSee [authenticate.webhook](/docs/api/shopify-app-remix/authenticate/webhook#example-admin) for more details.',
isOptional: true,
},
{
name: 'v3_lineItemBilling',
name: 'lineItemBilling',
value: '',
description:
'Allows you to pass billing plans with up to two line items when creating a new app subscription.' +
Expand Down
65 changes: 0 additions & 65 deletions packages/apps/shopify-app-remix/docs/upcoming_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ You can use it as a guide for migrating your app, and ensuring you're ready for
- [Upcoming breaking changes](#upcoming-breaking-changes)
- [Table of contents](#table-of-contents)
- [Use new authentication strategy for embedded apps](#use-new-authentication-strategy-for-embedded-apps)
- [Align the `admin` context object for webhooks](#align-the-admin-context-object-for-webhooks)
- [Use line item billing configs](#use-line-item-billing-configs)

## Use new authentication strategy for embedded apps
Expand All @@ -27,70 +26,6 @@ Before updating this package in your app, please ensure you've enabled managed i

For more details on how this works, please see the [new embedded authorization strategy](../README.md#new-embedded-authorization-strategy) section in the README.

## Align the `admin` context object for webhooks

> [!NOTE]
> The `v3_webhookAdminContext` future flag enabled this behaviour.
> If you've already enabled the flag, you don't need to follow these instructions.
The `admin` context returned by `authenticate.webhook` didn't match the object returned by e.g. `authenticate.admin`, which could lead to confusion.

We updated the shape of the object returned by `authenticate.webhook` so that every authentication method returns a consistent format.
That reduces the mental load for developers as they shouldn't have to learn more than one way of doing the same thing.

To update your code:

- GraphQL:
- replace `admin?.graphql.query()` with `admin?.graphql()`
- the graphql query function now takes the query string as the first argument, and an optional settings object, as opposed to a single object
- REST:
- `admin.rest.get|post|put|delete()` remain unchanged
- `admin.rest.Product` (or other resource classes) are now under `admin.rest.resources.Product`

Before:

```ts
export async function action({request}: ActionFunctionArgs) {
const {admin} = await shopify.authenticate.webhook(request);

// GraphQL query
const response = await admin?.graphql.query<any>({
data: {
query: `query { ... }`,
variables: {myVariable: '...'},
},
});

// REST resource
const products = await admin?.rest.Product.all({
// ...
});

// ...
}
```

After:

```ts
export async function action({request}: ActionFunctionArgs) {
const {admin} = await shopify.authenticate.webhook(request);

// GraphQL query
const response = await admin?.graphql(`query { ... }`, {
variables: {myVariable: '...'},
});
const data = await response.json();

// REST resource
const products = await admin?.rest.resources.Product.all({
// ...
});

// ...
}
```

## Use line item billing configs

> [!NOTE]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {API_KEY, API_SECRET_KEY, APP_URL} from './const';
* old behaviour.
*/
const TEST_FUTURE_FLAGS: Required<{[key in keyof FutureFlags]: true}> = {
v3_webhookAdminContext: true,
v3_lineItemBilling: true,
unstable_newEmbeddedAuthStrategy: true,
} as const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
expectAdminApiClient,
getHmac,
getThrownResponse,
setUpValidSession,
testConfig,
} from '../../../__test-helpers';

Expand Down Expand Up @@ -105,31 +104,6 @@ describe('Webhook validation', () => {
});
});

it('returns a legacy context when the future flag is disabled', async () => {
// GIVEN
const shopify = shopifyApp(
testConfig({restResources, future: {v3_webhookAdminContext: false}}),
);
const session = await setUpValidSession(shopify.sessionStorage);

// WHEN
const body = {some: 'data'};
const {admin} = await shopify.authenticate.webhook(
new Request(`${APP_URL}/webhooks`, {
method: 'POST',
body: JSON.stringify(body),
headers: webhookHeaders(JSON.stringify(body)),
}),
);

// THEN
expect(admin?.rest.apiVersion).toBe('2023-01');
expect(admin?.rest.session).toBe(session);

expect(admin?.graphql.apiVersion).toBe('2023-01');
expect(admin?.graphql.session).toBe(session);
});

it('throws a 401 on invalid HMAC', async () => {
// GIVEN
const shopify = shopifyApp(testConfig());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,27 @@
import {
ApiVersion,
ShopifyRestResources,
WebhookValidationErrorReason,
} from '@shopify/shopify-api';

import type {BasicParams, MandatoryTopics} from '../../types';
import {AdminApiContext, adminClientFactory} from '../../clients';
import {adminClientFactory} from '../../clients';
import {handleClientErrorFactory} from '../admin/helpers';
import {FutureFlagOptions} from '../../future/flags';

import type {
AuthenticateWebhook,
LegacyWebhookAdminApiContext,
WebhookAdminContext,
WebhookContext,
WebhookContextWithoutSession,
} from './types';

export function authenticateWebhookFactory<
Future extends FutureFlagOptions,
Resources extends ShopifyRestResources,
Topics extends string | number | symbol | MandatoryTopics,
>(params: BasicParams): AuthenticateWebhook<Future, Resources, Topics> {
>(params: BasicParams): AuthenticateWebhook<Resources, Topics> {
const {api, config, logger} = params;

return async function authenticate(
request: Request,
): Promise<WebhookContext<Future, Resources, Topics>> {
): Promise<WebhookContext<Resources, Topics>> {
if (request.method !== 'POST') {
logger.debug(
'Received a non-POST request for a webhook. Only POST requests are allowed.',
Expand Down Expand Up @@ -75,39 +70,16 @@ export function authenticateWebhookFactory<
return webhookContext;
}

let admin:
| AdminApiContext<Resources>
| LegacyWebhookAdminApiContext<Resources>;
if (config.future.v3_webhookAdminContext) {
admin = adminClientFactory<Resources>({
params,
session,
handleClientError: handleClientErrorFactory({request}),
});
} else {
const restClient = new api.clients.Rest({
session,
apiVersion: check.apiVersion as ApiVersion,
});
const graphqlClient = new api.clients.Graphql({
session,
apiVersion: check.apiVersion as ApiVersion,
});

Object.entries(api.rest).forEach(([name, resource]) => {
Reflect.set(restClient, name, resource);
});

admin = {
rest: restClient as typeof restClient & Resources,
graphql: graphqlClient,
};
}
const admin = adminClientFactory<Resources>({
params,
session,
handleClientError: handleClientErrorFactory({request}),
});

return {
...webhookContext,
session,
admin: admin as WebhookAdminContext<Future, Resources>,
admin,
};
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ import {ReferenceEntityTemplateSchema} from '@shopify/generate-docs';

const data: ReferenceEntityTemplateSchema = {
name: 'Webhook',
description:
'Contains functions for verifying Shopify webhooks.' +
'\n\n> Note: The format of the `admin` object returned by this function changes with the `v3_webhookAdminContext` future flag. Learn more about [gradual feature adoption](/docs/api/shopify-app-remix/guide-future-flags).',
description: 'Contains functions for verifying Shopify webhooks.',
category: 'Authenticate',
type: 'object',
isVisualComponent: false,
Expand Down
Loading

0 comments on commit 5391201

Please sign in to comment.