Merge and replace multiple preloads ourselves #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jhollinger
force-pushed
the
fix-bad-preload-merge
branch
from
5976f52 to
4eefeaf
Compare
Signed-off-by: Jordan Hollinger <[email protected]>
jhollinger
force-pushed
the
fix-bad-preload-merge
branch
from
4eefeaf to
9ceeeba
Compare
Signed-off-by: Jordan Hollinger <[email protected]>
jhollinger
force-pushed
the
fix-bad-preload-merge
branch
from
b228b73 to
6fe663a
Compare
|
Wasn't the cause. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rails is supposed to handle things gracefully when one set of preloads is layered on top of another (e.g. when a Blueprinter's preloads are added to a query with existing preloads). We even have tests for it. But we're getting reports it may not be working in some cases - that sometimes the original, manually added preloads get (partially??) wiped out.
I haven't been able to duplicate it. But this PR would remove all ambiguity. Every time a Blueprint's preloads are added, they're merged into the existing preloads, instead of being added as a separate preload entry for Rails to deal with.
Has been tested against real-world preloads + autopreloads to ensure they merge as expected. Includes a version bump since this is a critical bugfix.