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

Don't unnecessarily load a belongs_to when saving. #23498

Merged
merged 1 commit into from Aug 30, 2016

Conversation

Projects
None yet
6 participants
@jcoleman
Contributor

jcoleman commented Feb 5, 2016

Previously, if the the association was previously loaded and then
the foreign key changed by itself, a #save call would trigger a
load of the new associated record during autosave. This is unnecessary
and the autosave code (in that case) didn't use the loaded record
anyways.

@rails-bot

This comment has been minimized.

rails-bot commented Feb 5, 2016

r? @pixeltrix

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

@jcoleman

This comment has been minimized.

Contributor

jcoleman commented Feb 5, 2016

@pixeltrix Just FYI, there is an unrelated test that fails both with and without this patch.

@jcoleman jcoleman force-pushed the jcoleman:remove-unnecessary-belongs-to-load branch 2 times, most recently to a94fe29 Aug 26, 2016

Don't unnecessarily load a belongs_to when saving.
Previously, if the the association was previously loaded and then
the foreign key changed by itself, a #save call would trigger a
load of the new associated record during autosave. This is unnecessary
and the autosave code (in that case) didn't use the loaded record
anyways.
@jcoleman

This comment has been minimized.

Contributor

jcoleman commented Aug 26, 2016

@maclover7 @pixeltrix I've rebased the patch (yet again the only conflict was in the changelog). I fixed the issues with code climate, but for some reason it hasn't re-run.

Any way we could this this looked at so that it doesn't need rebasing again?

@arthurnn

This comment has been minimized.

Member

arthurnn commented Aug 30, 2016

LGTM.
@rafaelfranca wanna double check it?

@matthewd matthewd merged commit e912915 into rails:master Aug 30, 2016

1 of 2 checks passed

codeclimate Code Climate encountered an error attempting to analyze this pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

matthewd added a commit that referenced this pull request Sep 1, 2016

Merge pull request #23498 from jcoleman/remove-unnecessary-belongs-to…
…-load

Don't unnecessarily load a belongs_to when saving.
@matthewd

This comment has been minimized.

Member

matthewd commented Sep 1, 2016

Backported in b8eab32

@jcoleman

This comment has been minimized.

Contributor

jcoleman commented Sep 2, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment