Skip to content

Allow custom handler to be passed when registering a webhook#2014

Closed
jonfrench wants to merge 1 commit intomainfrom
jonfrench-patch-1
Closed

Allow custom handler to be passed when registering a webhook#2014
jonfrench wants to merge 1 commit intomainfrom
jonfrench-patch-1

Conversation

@jonfrench
Copy link

@jonfrench jonfrench commented Oct 16, 2025

What this PR does

When an app uses a custom WebhooksController instead of using the Gem provided WebhooksController, the app assumes responsibility for receiving and processing web hook requests, and thus does not need to register ActiveJob handlers which match the name of the webhook's topic as required by WebhooksController.

There is a bug in WebhookManager which requires the app to maintain an ActiveJob class matching the name of the topic even if the app uses a custom WebhooksController.

In this Pull Request, I add an additional WebhookManager webhooks configuration property called handler which is the name of the ActiveJob class to enqueue if using the default WebhooksController or nil to unset the handler. The latter feature - the ability to set handler: nil will fix the bug.

Reviewer's guide to testing

Because this introduces a new configuration property but maintains backward compatibility with current configuration (not setting the handler property behaves exactly the same), I rely on unit tests to assert the change.

Things to focus on

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@jonfrench
Copy link
Author

jonfrench commented Oct 17, 2025

Arg. I just realized that you cannot register a http webhook in the WebhookRegistry without a handler:

https://github.com/Shopify/shopify-api-ruby/blob/8b20cf374f41f5c06ce423d173e67cefdb4cb068/lib/shopify_api/webhooks/registry.rb#L36

@jonfrench jonfrench closed this Oct 17, 2025
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.

1 participant