Shopify
added a name field to webhook subscriptions
closes https://github.com/shop/issues-event-foundations/issues/518
WHY are these changes introduced?
Adds support for the name field on webhook subscriptions when registered in the app's shopify.app.toml
WHAT is this pull request doing?
- Adds optional
namefield to webhook subscription TypeScript types - Adds validation for the
namefield in webhook subscription schemas (must be a non-empty string if provided) - Adds test coverage for webhook subscriptions with the
nameattribute
Name validation:
- Field is optional
- Must be a non-empty string if provided
How to test your changes
- Create a webhook subscription in your
shopify.app.tomlwith anamefield while running local app development in dev dash:
Example:
[[webhooks.subscriptions]]
name = "products-create"
topics = ["products/create"]
uri = "https://example.com/webhooks"
- Run
shopify app deployand verify it accepts the configuration - Try with an empty
name = ""and verify validation rejects it - Run the test suite:
pnpm vitest loader.test.ts --run
Tophat:
- [x] Tophatted changes locally verifying that the min and max char validations are working
We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.
[!CAUTION] DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Coverage report
St.:grey_question: |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🟡 | Statements | 79.51% (+0.28% 🔼) |
14177/17830 |
| 🟡 | Branches | 73.57% (+0.46% 🔼) |
6952/9450 |
| 🟡 | Functions | 79.68% (+0.31% 🔼) |
3639/4567 |
| 🟡 | Lines | 79.88% (+0.3% 🔼) |
13401/16777 |
Show new covered files 🐣
St.:grey_question: |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🟢 | ... / admin-as-app.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / metafield_definitions.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / metaobject_definitions.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / bulk-operation-run-mutation.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / bulk-operation-run-query.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / get-bulk-operation-by-id.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / list-bulk-operations.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / staged-uploads-create.ts |
100% | 100% | 100% | 100% |
| 🔴 | ... / execute.ts |
0% | 0% | 0% | 0% |
| 🔴 | ... / status.ts |
0% | 0% | 0% | 0% |
| 🔴 | ... / pull.ts |
0% | 100% | 0% | 0% |
| 🟢 | ... / execute-operation.ts |
92.59% | 75% | 100% | 92.31% |
| 🔴 | ... / pull.ts |
0% | 0% | 0% | 0% |
| 🟢 | ... / bulk-operation-status.ts |
96.55% | 92.11% | 100% | 100% |
| 🟢 | ... / constants.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / download-bulk-operation-results.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / execute-bulk-operation.ts |
92.65% | 86.67% | 100% | 93.94% |
| 🟢 | ... / format-bulk-operation-status.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / run-mutation.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / run-query.ts |
100% | 100% | 100% | 100% |
| 🟡 | ... / stage-file.ts |
73.53% | 62.5% | 85.71% | 72.73% |
| 🟢 | ... / watch-bulk-operation.ts |
100% | 94.74% | 100% | 100% |
| 🟢 | ... / declarative-definitions.ts |
98.54% | 93.18% | 100% | 98.51% |
| 🟢 | ... / common.ts |
97.22% | 93.75% | 100% | 96.55% |
| 🟢 | ... / execute-command-helpers.ts |
100% | 100% | 100% | 100% |
| 🔴 | ... / promiseWithResolvers.ts |
33.33% | 50% | 50% | 33.33% |
Show files with reduced coverage 🔻
St.:grey_question: |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🔴 | ... / execute.ts |
0% | 0% (-100% 🔻) |
0% | 0% |
| 🟢 | ... / extension-instance.ts |
84.8% (+0.23% 🔼) |
77.6% (-0.91% 🔻) |
92.06% (+0.13% 🔼) |
85.11% (+0.24% 🔼) |
| 🟡 | ... / specification.ts |
69.09% | 75.61% (+2.44% 🔼) |
76.47% (-1.31% 🔻) |
68.75% |
| 🟢 | ... / ui_extension.ts |
85.38% (-9.44% 🔻) |
72.34% (-8.91% 🔻) |
84% (-16% 🔻) |
88% (-8.46% 🔻) |
| 🟢 | ... / developer-platform-client.ts |
84.62% (-1.5% 🔻) |
71.43% (+0.84% 🔼) |
81.82% (+1.82% 🔼) |
93.75% (+0.42% 🔼) |
| 🟢 | ... / api.ts |
87.07% (-0.43% 🔻) |
76.71% (-0.1% 🔻) |
100% | 86.49% (-0.43% 🔻) |
| 🟢 | ... / SingleTask.tsx |
84.21% (-15.79% 🔻) |
50% (-50% 🔻) |
80% (-20% 🔻) |
84.21% (-15.79% 🔻) |
| 🔴 | ... / environment.ts |
35% (-5% 🔻) |
41.18% | 40% (-10% 🔻) |
36.84% (-5.26% 🔻) |
| 🔴 | ... / ui.tsx |
50.82% (-0.79% 🔻) |
42.86% (-5.53% 🔻) |
54.55% (+1.42% 🔼) |
50% (-0.82% 🔻) |
| 🟢 | ... / console.ts |
81.82% (+15.15% 🔼) |
75% (-25% 🔻) |
100% (+33.33% 🔼) |
81.82% (+15.15% 🔼) |
| 🔴 | ... / dev.ts |
14.29% (+0.95% 🔼) |
3.13% (+0.18% 🔼) |
50% (-7.14% 🔻) |
14.29% (+0.95% 🔼) |
| 🟢 | ... / init.ts |
88% (-0.89% 🔻) |
71.43% (+4.76% 🔼) |
86.67% (+4.85% 🔼) |
88% (-0.89% 🔻) |
| 🟢 | ... / storefront-renderer.ts |
90.2% (-0.54% 🔻) |
78.95% | 81.82% (-1.52% 🔻) |
90.2% (-0.54% 🔻) |
| 🟡 | ... / theme-polling.ts |
67.12% (-0.93% 🔻) |
68.75% | 78.57% | 66.67% (-0.98% 🔻) |
Test suite run success
3554 tests passing in 1415 suites.
Report generated by 🧪jest coverage report action from 833d75115f6410aefa89d95372335201291436bc
Note: Shopify core changes must be deployed first before the CLI or adding the name field would fail to be recognized and error
Note: Shopify core changes must be deployed first before the CLI or adding the name field would fail to be recognized and error
@georgge293 This PR is already approved but the core changes won't be deployed until after BFCM. I see some chances this PR is still accidentally merged. Which might be fine because afaik there won't be more CLI releases before BFCM, but core pending deploys could take a few days to deploy after BFCM and we should be careful to not release the CLI until that specific PR is deployed if we don't want to release a broken feature.
I may be a bit paranoid here, but what about moving this PR back to draft? You would keep the approvals and prevent an accidental merge from someone who hasn't read your comment on the core changes dependency
Note: Shopify core changes must be deployed first before the CLI or adding the name field would fail to be recognized and error
@georgge293 This PR is already approved but the core changes won't be deployed until after BFCM. I see some chances this PR is still accidentally merged. Which might be fine because afaik there won't be more CLI releases before BFCM, but core pending deploys could take a few days to deploy after BFCM and we should be careful to not release the CLI until that specific PR is deployed if we don't want to release a broken feature.
I may be a bit paranoid here, but what about moving this PR back to draft? You would keep the approvals and prevent an accidental merge from someone who hasn't read your comment on the core changes dependency
Sounds good, will do. Thanks!