Skip to content

Conversation

jgraichen
Copy link
Owner

Continuation of #42 because GitHub failed to accept pushes to editable PR branches.


The RFC6570 path-only code previously skipped the Rails URL generation backend but only returned the template path. This resulted in skipping the prefix from mounted engines.

The PR changes the code to always use ActionDispatch::Http::URL.url_for for all URL generation after expanding the template and to pass only_path as an option when needed. This way, full URL and path-only generation behave identically, and both will include the correct Rails mount paths now.

The RFC6570 path-only code previously skipped the Rails URL generation
backend but only returned the template path. This resulted in skipping
the prefix from mounted engines.

This commit changes the code to always use
`ActionDispatch::Http::URL.url_for` for all URL generation after
expanding the template and to pass `only_path` as an option when need.
This way, full URL and path only generation behaves identical and both
will include the correct rails mount paths.
@jgraichen jgraichen self-assigned this Sep 25, 2025
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (dd6a80f) to head (b3471fc).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #43   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          162       161    -1     
=========================================
- Hits           162       161    -1     

☔ 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.

@jgraichen jgraichen merged commit 34277af into main Sep 25, 2025
48 checks passed
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.

2 participants