-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add missing "X-Shopify-Triggered-At" in ShopifyHeader #1941
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there 👋
Thanks for your contribution!
I agree that we should have this header value.
I believe though for this value to be accessible we will need to modify the validate function to return this header value. Or are you using this in a different way?
Currently I write my own verify webhook function, some needs need to get 'triggered-at' so I just add it in ShopifyHeader enum. I agree that if someone needs it, it should also be edited a bit |
Thanks for the additional context! @llong2195 Would you be able to update a test for the validate function, to confirm this change? Afterwards you will also need to add a changeset ( |
Thanks for the feedback! @lizkenyon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the tests.
Can you please sign the CLA and add a changeset? (pnpm changeset
)
I have signed the CLA! |
WHY are these changes introduced?
Fixes #0000
I read the documentation: https://shopify.dev/docs/apps/build/webhooks#key-terminology and found
X-Shopify-Triggered-At
missingWHAT is this pull request doing?
This pull request adds the missing
X-Shopify-Triggered-At
header to theShopifyHeader
.Type of change
Checklist
pnpm changeset
to create a draft changelog entry (do NOT update theCHANGELOG.md
files manually)