Skip to content

Commit

Permalink
Merge pull request #3862 from Shopify/dp-add-webhook-filters
Browse files Browse the repository at this point in the history 
Add filter to webhooks app config
  • Loading branch information
@dpeacock
dpeacock authored May 10, 2024
2 parents 3a0d2f3 + 90ca954 commit 2787780
Show file tree
Hide file tree
Showing 8 changed files with 131 additions and 10 deletions.
57 changes: 51 additions & 6 deletions packages/app/src/cli/models/app/loader.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2734,7 +2734,7 @@ describe('WebhooksSchema', () => {
}
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic` and `uri`',
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
fatal: true,
path: ['webhooks', 'subscriptions', 0, 'topics', 1, 'products/create'],
}
Expand All @@ -2753,7 +2753,7 @@ describe('WebhooksSchema', () => {
}
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic` and `uri`',
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/create'],
}
Expand Down Expand Up @@ -2847,7 +2847,7 @@ describe('WebhooksSchema', () => {
}
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic` and `uri`',
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/create'],
}
Expand All @@ -2872,7 +2872,7 @@ describe('WebhooksSchema', () => {
}
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic` and `uri`',
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/create'],
}
Expand All @@ -2897,7 +2897,7 @@ describe('WebhooksSchema', () => {
}
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic` and `uri`',
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/create'],
}
Expand All @@ -2924,7 +2924,7 @@ describe('WebhooksSchema', () => {
}
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic` and `uri`',
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'metaobjects/create'],
}
Expand All @@ -2949,6 +2949,51 @@ describe('WebhooksSchema', () => {
},
],
}
})

test('does not allow identical topic and uri and filter in different subscriptions', async () => {
const webhookConfig: WebhooksConfig = {
api_version: '2021-07',
subscriptions: [
{
topics: ['products/update'],
uri: 'https://example.com',
filter: 'title:shoes',
},
{
topics: ['products/update'],
uri: 'https://example.com',
filter: 'title:shoes',
},
],
}
const errorObj = {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
fatal: true,
path: ['webhooks', 'subscriptions', 1, 'topics', 0, 'products/update'],
}

const {abortOrReport, expectedFormatted} = await setupParsing(errorObj, webhookConfig)
expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp', [errorObj])
})

test('allows identical topic and uri if filter is different', async () => {
const webhookConfig: WebhooksConfig = {
api_version: '2021-07',
subscriptions: [
{
topics: ['products/update'],
uri: 'https://example.com',
filter: 'title:shoes',
},
{
topics: ['products/update'],
uri: 'https://example.com',
filter: 'title:shirts',
},
],
}

const {abortOrReport, parsedConfiguration} = await setupParsing({}, webhookConfig)
expect(abortOrReport).not.toHaveBeenCalled()
Expand Down
50 changes: 50 additions & 0 deletions packages/app/src/cli/models/extensions/specifications/app_config_webhook.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ describe('webhooks', () => {
uri: 'arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/1234567890/SOME_PATH',
sub_topic: 'type:metaobject_one',
},
{
topics: ['products/create', 'products/update'],
uri: 'https://example.com/webhooks/products',
filter: 'title:shoes',
},
],
},
}
Expand Down Expand Up @@ -101,6 +106,16 @@ describe('webhooks', () => {
topic: 'metaobjects/delete',
uri: 'arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/1234567890/SOME_PATH',
},
{
filter: 'title:shoes',
topic: 'products/create',
uri: 'https://example.com/webhooks/products',
},
{
filter: 'title:shoes',
topic: 'products/update',
uri: 'https://example.com/webhooks/products',
},
],
})
})
Expand Down Expand Up @@ -146,6 +161,11 @@ describe('webhooks', () => {
topic: 'orders/create',
uri: 'https://valid-url',
},
{
filter: 'title:shoes',
topic: 'products/create',
uri: 'https://example.com/webhooks/products',
},
],
}
const webhookSpec = spec
Expand Down Expand Up @@ -176,6 +196,11 @@ describe('webhooks', () => {
topics: ['orders/create'],
uri: 'https://valid-url',
},
{
filter: 'title:shoes',
topics: ['products/create'],
uri: 'https://example.com/webhooks/products',
},
],
},
})
Expand Down Expand Up @@ -233,6 +258,21 @@ describe('webhooks', () => {
sub_topic: 'subtopic',
uri: 'https://example.com/webhooks',
},
{
topics: ['products/create'],
uri: 'https://example.com/webhooks',
filter: 'title:shoes',
},
{
topics: ['products/update'],
uri: 'https://example.com/webhooks',
filter: 'title:shoes',
},
{
topics: ['products/update'],
uri: 'https://example.com/webhooks',
filter: 'title:shirts',
},
],
privacy_compliance: undefined,
},
Expand Down Expand Up @@ -268,6 +308,16 @@ describe('webhooks', () => {
sub_topic: 'subtopic',
uri: 'https://example.com/webhooks',
},
{
topics: ['products/create', 'products/update'],
uri: 'https://example.com/webhooks',
filter: 'title:shoes',
},
{
topics: ['products/update'],
uri: 'https://example.com/webhooks',
filter: 'title:shirts',
},
],
privacy_compliance: undefined,
},
Expand Down
3 changes: 2 additions & 1 deletion packages/app/src/cli/models/extensions/specifications/app_config_webhook.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ function mergeAllWebhooks(subscriptions: WebhookSubscription[]): WebhookSubscrip
(sub) =>
sub.uri === subscription.uri &&
sub.sub_topic === subscription.sub_topic &&
sub.include_fields === subscription.include_fields,
sub.include_fields === subscription.include_fields &&
sub.filter === subscription.filter,
)
if (existingSubscription) {
if (subscription.compliance_topics) {
Expand Down
1 change: 1 addition & 0 deletions ...odels/extensions/specifications/app_config_webhook_schemas/webhook_subscription_schema.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const WebhookSubscriptionSchema = zod.object({
uri: zod.preprocess(removeTrailingSlash, UriValidation, {required_error: 'Missing value at'}),
sub_topic: zod.string({invalid_type_error: 'Value must be a string'}).optional(),
include_fields: zod.array(zod.string({invalid_type_error: 'Value must be a string'})).optional(),
filter: zod.string({invalid_type_error: 'Value must be a string'}).optional(),
compliance_topics: zod
.array(
zod.enum([ComplianceTopic.CustomersRedact, ComplianceTopic.CustomersDataRequest, ComplianceTopic.ShopRedact]),
Expand Down
22 changes: 22 additions & 0 deletions ...ages/app/src/cli/models/extensions/specifications/app_config_webhook_subscription.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ describe('webhook_subscription', () => {
uri: 'arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/1234567890/SOME_PATH',
sub_topic: 'type:metaobject_one',
},
{
topics: ['products/update'],
uri: 'https://example.com/webhooks/products',
filter: 'title:shoes',
},
],
},
}
Expand Down Expand Up @@ -110,6 +115,12 @@ describe('webhook_subscription', () => {
topic: 'metaobjects/delete',
uri: 'arn:aws:events:us-west-2::event-source/aws.partner/shopify.com/1234567890/SOME_PATH',
},
{
api_version: '2024-01',
filter: 'title:shoes',
topic: 'products/update',
uri: 'https://example.com/webhooks/products',
},
],
})
})
Expand Down Expand Up @@ -160,6 +171,12 @@ describe('webhook_subscription', () => {
topic: 'orders/create',
uri: 'https://valid-url',
},
{
api_version: '2024-01',
filter: 'title:shoes',
topic: 'products/update',
uri: 'https://example.com/webhooks/products',
},
],
}
const webhookSpec = spec
Expand Down Expand Up @@ -189,6 +206,11 @@ describe('webhook_subscription', () => {
topics: ['orders/create'],
uri: 'https://valid-url',
},
{
filter: 'title:shoes',
topics: ['products/update'],
uri: 'https://example.com/webhooks/products',
},
],
},
})
Expand Down
1 change: 1 addition & 0 deletions packages/app/src/cli/models/extensions/specifications/app_config_webhook_subscription.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface TransformedWebhookSubscription {
compliance_topics?: string[]
sub_topic?: string
include_fields?: string[]
filter?: string
}

/* this transforms webhooks from the TOML config to be parsed remotely
Expand Down
1 change: 1 addition & 0 deletions packages/app/src/cli/models/extensions/specifications/types/app_config_webhook.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export interface WebhookSubscription {
compliance_topics?: string[]
sub_topic?: string
include_fields?: string[]
filter?: string
}

interface PrivacyComplianceConfig {
Expand Down
6 changes: 3 additions & 3 deletions packages/app/src/cli/models/extensions/specifications/validation/app_config_webhook.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function validateSubscriptions(webhookConfig: WebhooksConfig) {
}

// eslint-disable-next-line @typescript-eslint/naming-convention
for (const [i, {uri, topics = [], compliance_topics = [], sub_topic = ''}] of subscriptions.entries()) {
for (const [i, {uri, topics = [], compliance_topics = [], sub_topic = '', filter = ''}] of subscriptions.entries()) {
const path = ['subscriptions', i]

if (!topics.length && !compliance_topics.length) {
Expand All @@ -50,12 +50,12 @@ function validateSubscriptions(webhookConfig: WebhooksConfig) {
}

for (const [j, topic] of topics.entries()) {
const key = `${topic}::${sub_topic}::${uri}`
const key = `${topic}::${sub_topic}::${uri}::${filter}`

if (uniqueSubscriptionSet.has(key)) {
return {
code: zod.ZodIssueCode.custom,
message: 'You can’t have duplicate subscriptions with the exact same `topic` and `uri`',
message: 'You can’t have duplicate subscriptions with the exact same `topic`, `uri` and `filter`',
fatal: true,
path: [...path, 'topics', j, topic],
}
Expand Down

0 comments on commit 2787780

Please sign in to comment.