-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: authorize webhooks #57
Conversation
🦋 Changeset detectedLatest commit: a0db74e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Coverage report
Show files with reduced coverage 🔻
Test suite run success40 tests passing in 18 suites. Report generated by 🧪jest coverage report action from a0db74e |
src/lib/env.mjs
Outdated
AUTHORIZE_SIGNATURE_KEY: z | ||
.string() | ||
.min(1) | ||
// .optional() |
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.
Should be removed
@@ -0,0 +1,22 @@ | |||
import { type NextApiRequest, type NextApiResponse } from "next"; |
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.
Issue with checking the HMAC could be caused by default NextJS body parser. I had to turn it off in case of calculating checksums for the Stripe or GitHub webhooks.
References:
} | ||
|
||
private getWebhookParams() { | ||
const appUrl = isDevelopment() ? env.LOCAL_APP_URL : `https://${env.VERCEL_URL}`; // todo: get rid of it |
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.
Would use APP_API_BASE_URL
to have compatibility with existing apps.
https://github.com/saleor/apps/blob/main/apps/cms-v2/src/pages/api/manifest.ts#L13
"net.authorize.payment.refund.created", | ||
], | ||
status: "active", | ||
url: `${appUrl}/api/webhooks/authorize`, |
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.
Nitpick: Would use new URL to avoid issues when user provides base URL with trailing slash
@@ -12,7 +12,7 @@ | |||
}, | |||
"type": "module", | |||
"scripts": { | |||
"dev": "pnpm generate && next dev", | |||
"dev": "pnpm generate && next dev --port 8000", |
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.
Please mention the ports in the readme
feedback addressed @krzysztofwolski 🖖 |
README.md
Outdated
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.
💪
|
||
const normalizedError = normalizeError(error); | ||
return responseBuilder.ok({ | ||
amount: 0, // 0 or real amount? |
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.
If we can get the real value, lets use it. If thats displayed in any way do the staff/customer, then theres a value in knowing exact value which got rejected
This PR adds functionality for:
src/modules/authorize-net/webhook/authorize-net-webhook-client.ts
,src/modules/authorize-net/webhook/authorize-net-webhook-manager.ts
)src/modules/authorize-net/webhook/authorize-net-webhook-handler.ts
)src/modules/authorize-net/synchronized-transaction/saleor-transaction-id-converter.ts
,src/modules/authorize-net/synchronized-transaction/create-synchronized-transaction-request.ts
,src/modules/authorize-net/synchronized-transaction/synchronized-transaction-id-resolver.ts
)Next up:
AUTHORIZE_PAYMENT_FORM_URL
env variable