Skip to content

Fix ActiveRecord::ImmutableRelation is raised when collection caching and HTTP caching are used together#32536

Merged
rafaelfranca merged 1 commit intorails:masterfrom
lsylvester:fix-immutable-relation-error-with-http-caching-and-collection-caching
Apr 12, 2018
Merged

Fix ActiveRecord::ImmutableRelation is raised when collection caching and HTTP caching are used together#32536
rafaelfranca merged 1 commit intorails:masterfrom
lsylvester:fix-immutable-relation-error-with-http-caching-and-collection-caching

Conversation

@lsylvester
Copy link
Copy Markdown
Contributor

skip_preloading! was asserting immutability, but the arel may have already been defined by using fresh_when in the controller.

As skip_preloading! has no effect on the sql being generated it should be safe to skip this check when setting this value.

This PR removes skip_preloading from the SINGLE_VALUE_METHODS that are used to define setters that use the assert_immutability check, and replaces it a attribute accessor on the relation.

Fixes #32534

/cc @georgeclaghorn

…'t effect the arel and the arel may already be generated by fresh_when
@rails-bot
Copy link
Copy Markdown

r? @georgeclaghorn

(@rails-bot has picked a reviewer for you, use r? to override)

@rafaelfranca
Copy link
Copy Markdown
Member

Thanks! That was the exactly fix I had in mind.

@rafaelfranca rafaelfranca merged commit 7622b85 into rails:master Apr 12, 2018
rafaelfranca added a commit that referenced this pull request Apr 12, 2018
…r-with-http-caching-and-collection-caching

Fix ActiveRecord::ImmutableRelation is raised when collection caching and HTTP caching are used together
@georgeclaghorn
Copy link
Copy Markdown
Contributor

Thanks for the speedy fix, @lsylvester!

y-yagi added a commit that referenced this pull request Apr 12, 2018
…ion-error-with-http-caching-and-collection-caching"

This reverts commit 8b0cdbe.

Reason: #31250 is not backported to 5-2-stable, and this change broke build.
https://travis-ci.org/rails/rails/builds/365396137
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.

4 participants