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

Use media_type instead of content_type internally #36854

Merged
merged 1 commit into from
Aug 4, 2019

Conversation

eugeneius
Copy link
Member

These content_type calls were triggering the deprecation from #36490 in upgraded applications.

This problem was reported as a comment on the original pull request: #36490 (comment)

We can use media_type in all of these cases to avoid the deprecation.

These calls to `content_type` were triggering the deprecation from
c631e8d in upgraded applications.

We can use `media_type` in all of these cases to avoid the deprecation.
@rails-bot rails-bot bot added the actionpack label Aug 3, 2019
eugeneius referenced this pull request Aug 4, 2019
I changed return value of `ActionDispatch::Response#content_type` in #36034.
But this change seems to an obstacle to upgrading. #36034 (comment)

Therefore, I restored the behavior of `ActionDispatch::Response#content_type`
to 5.2 and deprecated old behavior. Also, made it possible to control the
behavior with the config.
@kaspth kaspth merged commit 5f3848a into rails:master Aug 4, 2019
@kaspth
Copy link
Contributor

kaspth commented Aug 4, 2019

Sweet!

@btihen
Copy link

btihen commented Aug 4, 2019

Thanks!

@mikker
Copy link
Contributor

mikker commented Aug 5, 2019

Great! Thanks. I've bundled this change up as a monkey patch for use until next rc/final:
https://gist.github.com/mikker/e3b0e3b660edeb09fec72464a0ee739c

@connorshea
Copy link
Contributor

Hmm, shouldn't this be cherry-picked onto the 6-0-stable branch?

tegon added a commit to heartcombo/devise that referenced this pull request Aug 6, 2019
Before Rails 6 RC2, the `ActionDispatch::Response#content_type` method
would return only the media part of the `Content-Type` header, without any
other parts. Now the `#content_type` method returns the entire header -
as it is - and `#media_type` should be used instead to get the previous
behavior.

Ref:
- rails/rails#36034
- rails/rails#36854
tegon added a commit to heartcombo/devise that referenced this pull request Aug 6, 2019
Before Rails 6 RC2, the `ActionDispatch::Response#content_type` method
would return only the media part of the `Content-Type` header, without any
other parts. Now the `#content_type` method returns the entire header -
as it is - and `#media_type` should be used instead to get the previous
behavior.

Ref:
- rails/rails#36034
- rails/rails#36854
tegon added a commit to heartcombo/devise that referenced this pull request Aug 7, 2019
* Fix specs on Rails 6 RC2

`ActiveRecord::MigrationContext` now has a `schema_migration` attribute.
Ref: https://github.com/rails/rails/pull/36439/files#diff-8d3c44120f7b67ff79e2fbe6a40d0ad6R1018

* Use `media_type` instead of `content_type`

Before Rails 6 RC2, the `ActionDispatch::Response#content_type` method
would return only the media part of the `Content-Type` header, without any
other parts. Now the `#content_type` method returns the entire header -
as it is - and `#media_type` should be used instead to get the previous
behavior.

Ref:
- rails/rails#36034
- rails/rails#36854

* Use render template instead of render file

Render file will need the full path in order to avoid security breaches.
In this particular case, there's no need to use render file, it's ok to
use render template.

Ref: rails/rails#35688

* Don't set `represent_boolean_as_integer` on Rails 6

* Update comments [ci skip]
kaspth added a commit that referenced this pull request Aug 8, 2019
…t_type

Use media_type instead of content_type internally
@kaspth
Copy link
Contributor

kaspth commented Aug 8, 2019

@connorshea it should, I just forgot to do it. Here it is: f57aac4

Thanks!

maximumtiu added a commit to maximumtiu/upgrade-rails-workshop that referenced this pull request Aug 13, 2019
rails/rails#36490 caused a deprecation warning
to be raised from within Rails.
rails/rails#36854 is the fix and has been merged
into Rails master.

This commit adds a config option as a work-around
until Rails 6.0 is officially released.
maximumtiu added a commit to maximumtiu/upgrade-rails-workshop that referenced this pull request Aug 13, 2019
rails/rails#36490 caused a deprecation warning
to be raised from within Rails.
rails/rails#36854 is the fix and has been merged
into Rails master.

This commit adds a config option as a work-around
until Rails 6.0 is officially released.
@ekampp
Copy link

ekampp commented Aug 15, 2019

I know this is already merged, but this causes the following error when used with ActiveStorage.

NotImplementedError:
  In order to correctly type cast ImageAttachment.id, ActiveStorage::Blob needs to define a :image_attachment association.

ImageAttachment is the class from my application. Otherwise, my ActiveStorage implementation is plain and simple.

@eugeneius
Copy link
Member Author

That error was introduced in #36847; I don't think it's related to this change.

Can you open a new issue, and include a sample application that reproduces the problem?

@ekampp
Copy link

ekampp commented Aug 16, 2019

I think you're right. Sorry for the miss-fire there.

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.

None yet

6 participants