Skip to content
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

format: true does not override existing format constraints. #9469

Merged
merged 1 commit into from Feb 28, 2013

Conversation

senny
Copy link
Member

@senny senny commented Feb 27, 2013

Closes #9466.

Passing format: true used to override the constraints: { format: /json/ }
with /.+/. This patch only sets the format if there is no constraint present.

Closes rails#9466.

Passing `format: true` used to override the constraints: { format: /json/ }
with `/.+/`. This patch only sets the format if there is no constraint present.
@senny
Copy link
Member Author

senny commented Feb 27, 2013

/cc @carlosantoniodasilva @rafaelfranca

@pixeltrix can you take a look?

pixeltrix added a commit that referenced this pull request Feb 28, 2013
`format: true` does not override existing format constraints.
@pixeltrix pixeltrix merged commit 17cff2c into rails:master Feb 28, 2013
@pixeltrix
Copy link
Contributor

Thanks @senny - I missed this set of options when I refactored the constraints code. This is how I'd do this kind of route:

get '/products.:format', to: 'products#index', format: /json/

i.e. make the required format part of the path.

@rosenfeld
Copy link
Contributor

Thanks, @senny and @pixeltrix. Actually I always found it dumb to have to specify both "format: true" and "format: /json/" in the constraints to force the format. First I did try to do something like:

get '/products.json' => 'products#index'

But that didn't work if I remembered correctly. When I asked in the rails-core list a while ago I was pointed that I should be doing as in the route I pointed out when I reported the issue.

But I certainly prefer your suggested approach, thanks.

@pixeltrix
Copy link
Contributor

@rosenfeld for your example to work you'd have to specify format: false:

get '/products.json' => 'products#index', :format => false

which seems counter-intuitive when you look at it.

@rosenfeld
Copy link
Contributor

@pixeltrix I believe that wouldn't work either for what I want... Look, I don't really care if the URL ends up with ".json" or not. The main reason I started to be concerned about the format is that I realized that when Rails caches some action it doesn't store the response content-type in the cache. So, the first time the action would respond with content-type 'application/json' but next time it would be 'text/html' if I remember correctly. Then I have been told that the cached action will use the content-type based on the format of the request, so I can't use "format: false". And the reason why I want to enforce the format is so that I don't forget about it in my client-side applications when calling some action with jQuery.getJSON (which won't run the callback if the wrong content-type is served). Sorry if this is confusing but I couldn't find any way to simplify that... That's why I ended up grouping all my JSON requests (99.9% of all requests) in a scoped block that would set format to true and the format constraint to /json/ to minimize any maintenance pain from my side...

@rosenfeld
Copy link
Contributor

Ah, and I remembered. Actually my first try was this one if I recall correctly:

get 'products' => 'product#index', format: 'json'

@senny senny deleted the 9466_format_enforcing_routes branch March 1, 2013 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It is not possible to accept a single format anymore on Rails 4 routes
3 participants