Fix custom namespace routing. #8876
Conversation
ActiveAdmin 4.0's `isolate_namespace` caused route helpers to fail for custom namespaces (e.g., `:active_admin` instead of `:admin`) because `url_for` looked in the engine's routes instead of the main app's routes. This adds a proxy module that overrides `url_for` to route hash options through `Rails.application.routes`, and updates the base controller to merge request context for relative URL generation. The polymorphic route helpers also default to `only_path: true` to avoid host resolution errors.
Rubocop caught me here.
|
Hi, thanks for this PR. I'm not familiar with the underlying issue and I couldn't reproduce #8279. There isn’t a failing test demonstrating the bug in this PR. Could you add a minimal failing test that reproduces the problem and show why it happens?
At a glance, overriding @javierjulio and @mgrunberg any thought? |
|
@craigmcnamara thank you for attempting to address this. I noticed the test suite is failing for the comments resource still. @tagliala in general I don't know much about engines. The reason for why it fails makes sense. I implemented |
|
|
||
| RSpec.describe "Custom Namespace Routes", type: :request do | ||
| # Test that custom namespaces (other than :admin) can access route helpers | ||
| # This reproduces the bug from: activeadmin-v4-custom-namespace-bug-report.md |
There was a problem hiding this comment.
What is this activeadmin-v4-custom-namespace-bug-report.md file referenced here?
There was a problem hiding this comment.
Sorry, that was a bug report from my app. I need to clean this up a bit.
The reproduction is in spec/requests/custom_namespace_routes_spec.rb If you remove the overrides those fail. My production app successfully loads ActiveAdmin with the patches and fails exactly like the bug report without them. My understanding is when routing params are omitted the routes helper infers them from the current request in the controller or view context. It seems the way the routes are used in ActiveAdmin, the helper doesn't always have the request context and that info isn't picked up. This isn't my first foray into ActiveAdmin, I wrote the patch that allows unlimited sized CSV downloads by implementing a streaming response back in the v1 days. |
3 participants
ActiveAdmin 4.0's
isolate_namespacecaused route helpers to fail for custom namespaces (e.g.,:active_admininstead of:admin) becauseurl_forlooked in the engine'sroutes instead of the main app's routes. This adds a proxy module that overrides
url_forto route hash options throughRails.application.routes, and updates the basecontroller to merge request context for relative URL generation. The polymorphic route helpers also default to
only_path: trueto avoid host resolution errors.Fixes #8279