Skip to content

Conversation

@rmcdaniel
Copy link
Contributor

  • Add named routes to webhook registrations for proper prefix handling
  • Update webhookUrl() to use route() helper instead of manual URL construction
  • This ensures webhook URLs include route prefixes (e.g., api/) when routes are registered in prefixed route files
  • Maintains compatibility with route caching and Laravel conventions
  • Builds on previous fix that added workflow slug to signal URLs

Fixes #270

- Add named routes to webhook registrations for proper prefix handling
- Update webhookUrl() to use route() helper instead of manual URL construction
- This ensures webhook URLs include route prefixes (e.g., api/) when routes are registered in prefixed route files
- Maintains compatibility with route caching and Laravel conventions
- Builds on previous fix that added workflow slug to signal URLs
- Add trailing comma in array parameter
- Fix method chaining indentation
@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a80a1ec) to head (0f9a5f5).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##              master      #272   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       310       310           
===========================================
  Files             46        46           
  Lines           1288      1290    +2     
===========================================
+ Hits            1288      1290    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rmcdaniel rmcdaniel requested a review from Copilot October 27, 2025 04:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes webhook URL generation to properly account for route prefixes by using Laravel's named route system. Previously, webhook URLs were manually constructed using the url() helper, which bypassed route prefixes defined in route files (e.g., api/). The fix adds named routes to webhook registrations and updates webhookUrl() to use the route() helper, ensuring generated URLs include any configured prefixes.

Key Changes:

  • Webhook routes now registered with names following pattern workflows.start.{slug} and workflows.signal.{slug}.{signal}
  • webhookUrl() method refactored to use route() helper instead of manual URL construction
  • Tests updated to reflect new URL patterns and route registration behavior

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Webhooks.php Added named route registration for start and signal webhook endpoints
src/Activity.php Refactored webhookUrl() to use route() helper with named routes instead of manual URL construction
tests/Unit/WebhooksTest.php Simplified test to mock route name registration instead of URI pattern matching
tests/Unit/ActivityTest.php Added webhook route registration and updated assertions to match new URL patterns
tests/Fixtures/TestWorkflow.php Added #[Webhook] attributes to class and cancel() method for test coverage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rmcdaniel rmcdaniel merged commit 971239e into master Oct 27, 2025
3 checks passed
@rmcdaniel rmcdaniel deleted the fix-webhook-route-prefixes branch October 27, 2025 04:39
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.

webhookUrl() doesn't account for route prefixes

2 participants