Skip to content

Fix declarative webhook generator route path#2073

Open
afurm wants to merge 1 commit into
Shopify:mainfrom
afurm:af/fix-declarative-webhook-route
Open

Fix declarative webhook generator route path#2073
afurm wants to merge 1 commit into
Shopify:mainfrom
afurm:af/fix-declarative-webhook-route

Conversation

@afurm
Copy link
Copy Markdown

@afurm afurm commented Apr 30, 2026

What this PR does

Fixes #1977.

This updates the shopify_app:add_declarative_webhook generator so the generated Rails route matches the configured webhook --path.

Previously, running:

rails g shopify_app:add_declarative_webhook --topic products/update --path webhooks/product_update

could add only the basename route under an existing namespace :webhooks, which meant apps using the default /api route scope served /api/webhooks/product_update instead of /webhooks/product_update. Shopify then posted to the configured /webhooks/product_update path, which could fall through to ShopifyApp::WebhooksController and raise ShopifyAPI::Errors::NoWebhookHandler.

The generator now creates an explicit route for the configured path and inserts it before the root-mounted ShopifyApp engine.

Reviewer's guide to testing

bundle exec rake test TEST=test/generators/add_declarative_webhook_generator_test.rb

Things to focus on

  1. The generated route uses the full configured webhook path.
  2. The route is inserted before mount ShopifyApp::Engine, at: "/".
  3. The generator test covers the route path and ordering.

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.

@github-actions github-actions Bot added the devtools-gardener Post the issue or PR to Slack for the gardener label Apr 30, 2026
@kennyadsl
Copy link
Copy Markdown
Contributor

I proposed an alternative approach in #2076, which should fix the issue as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devtools-gardener Post the issue or PR to Slack for the gardener

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing WebhookHandler

2 participants