-
Notifications
You must be signed in to change notification settings - Fork 2
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 webhook validation utility methods #46
Comments
Not sure we should expose this in the user land Not sure our users needs to use this This could be an implementation detail of our e-commerce integration |
my mentor thought it was good that payload validations are inside the php-sdk instead of in the ecommerce integrations if users decide to use only one endpoint to receive webhooks, they need a way to distinguish event types: if ($event === "OPENPIX:TRANSACTION_RECEIVED") {}
// vs
if ($client->webhooks()->isTransactionReceivedPayloadValid($payload)) {} this way we can avoid "pure strings" and move validation of payloads into php-sdk instead of ecommerce integrations in my opinion, if we assume that all the payload sent by the api is valid in case the signature is correct, I believe that there is no need for payload validation and to distinguish the types of webhooks avoiding "pure strings" we can just add constants in the webhooks resource: use OpenPix\PhpSdk\Resources\Webhooks;
if ($event === "OPENPIX:TRANSACTION_RECEIVED") {}
// vs
if ($payload["event"] === Webhooks::TRANSACTION_RECEIVED_EVENT) {} |
Expose const on the sdk as well |
It's better the sdk handle this complexity than the user handles this. I prefer to use an method to validate it, instead of only some variable! |
Remove valid suffix is better, |
Example usage
$client->webhooks()->isChargeCompletedPayloadValid($payload) // true or false
$client->webhooks()->isTransactionReceivedPayloadValid($payload) // true or false
The text was updated successfully, but these errors were encountered: