Skip to content
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

Corrected potential WebhookConfig issues. #104

Merged
merged 3 commits into from
Nov 24, 2021

Conversation

accu-clw
Copy link
Contributor

@accu-clw accu-clw commented Nov 24, 2021

Hello 👋

On the back of looking at your spatie/laravel-webhook-client package, I was looking at this as an example implementation and believe there may be a couple of minor mistakes.

Please consider this PR as a potential fix.

@freekmurze freekmurze merged commit e6d3af0 into spatie:main Nov 24, 2021
@freekmurze
Copy link
Member

Thank you!

@accu-clw accu-clw deleted the fix/webhook-controller branch November 24, 2021 12:22
@mattvb91
Copy link

This isnt a major issue cause I caught version 3.0.2 breaking in my tests upgrading from 3.0.1 but wouldn't this config change be considered a breaking change so the versioning is off?

@accu-clw change fixed the issue for me by updating my config file to the correct model.

@freekmurze
Copy link
Member

I'm sorry this broken your tests.

I'm thinking that the old version of the code was downright wrong, that's why I'm treating it as a bug fix, and not a breaking change.

If there's anything we can do, to improve let me know.

@accu-clw
Copy link
Contributor Author

This isnt a major issue cause I caught version 3.0.2 breaking in my tests upgrading from 3.0.1 but wouldn't this config change be considered a breaking change so the versioning is off?

@accu-clw change fixed the issue for me by updating my config file to the correct model.

Apologies if this broke for you (and any others!). I would agree with @freekmurze though, as the implementation was using an underlying library incorrectly.

@mattvb91
Copy link

yea no stress all good from my side just thought i'd point it out as there may be others with automatic dependabot updates coming with the same issue!

Thanks for the lib!

@colincameron
Copy link

For the benefit of anyone else who was bitten by this change causing Webhooks failures and is googling their error:

Error: Call to undefined method Spatie\StripeWebhooks\ProcessStripeWebhookJob::storeWebhook() in file /app/vendor/spatie/laravel-webhook-client/src/WebhookProcessor.php on line 48

The above error is caused by the typo fix to src/StripeWebhooksController.php line 22 now using the correct config value, which was incorrectly set to Spatie\StripeWebhooks\ProcessStripeWebhookJob::class instead of \Spatie\WebhookClient\Models\WebhookCall::class

Fixing the config value at stripe-webhooks.php resolves this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants