-
Notifications
You must be signed in to change notification settings - Fork 21.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a failing test for a URL helper that was broken by a6b9ea2. #14684
Add a failing test for a URL helper that was broken by a6b9ea2. #14684
Conversation
cc @pixeltrix |
…-helper-bug Add a failing test for a URL helper that was broken by a6b9ea2.
Since `:shallow` may be set at any point in the resource nesting we should only make the new and collection routes shallow when the parent is shallow. This is a bit of a hack but until the mapper is refactored to an object graph instead of a hash of merged values it's the best we can do. Fixes #14684. (cherry picked from commit e10f26f) Conflicts: actionpack/CHANGELOG.md
This requires a first level resource (i.e. not nested) to set :shallow to true. Is setting :shallow to first-level resources really expected? Thanks! :) |
@leods92 |
@pixeltrix Just to be clear: the expected behaviour is to set :shallow to first-level resources even though they won't change at all, isn't it? Not only should one set :shallow in a nested resource but also in its parent/first-level resource. If that's the expected behaviour, be aware that this commit breaks applications that had been working since version 3, it broke mine... |
@leods92 I've merged some fixes to shallow routes that haven't been released yet - can you test against the 4.1.6.rc1 release and if there's a regression from a previous release please file an issue. |
Between Rails 4.0.3 and 4.0.4 (and into 4.1.0), the routes in this test stopped producing the URL helper
comment_path
-- it becameblog_comment_path
instead. This was fixed in a6b9ea2 but the patch breaks the expectednew_blog_post_comment_path
helper, which was working until that patch landed.