AO3-7457 Preload unrevealed collections for work blurbs#5907
Open
nateberkopec wants to merge 4 commits into
Open
AO3-7457 Preload unrevealed collections for work blurbs#5907nateberkopec wants to merge 4 commits into
nateberkopec wants to merge 4 commits into
Conversation
nateberkopec
changed the title
nateberkopec
force-pushed
the
nb/preload-unrevealed-work-blurbs
branch
from
73fd150 to
9d22ce5
Compare
nateberkopec
commented
Jun 23, 2026
| <% if item.approved_collections.unrevealed.present? %> | ||
| <h5 class="heading"><%= h(ts("Part of ")) + show_collections_data(item.approved_collections.unrevealed) %></h5> | ||
| <% if approved_unrevealed_collections.any? %> | ||
| <h5 class="heading"><%= h(t(".part_of", default: "Part of ")) + show_collections_data(item.approved_unrevealed_collections) %></h5> |
Contributor
Author
There was a problem hiding this comment.
Change is unrelated but was enforced on me by Rubocop
nateberkopec
force-pushed
the
nb/preload-unrevealed-work-blurbs
branch
4 times, most recently
from
4755e28 to
0f54639
Compare
nateberkopec
force-pushed
the
nb/preload-unrevealed-work-blurbs
branch
from
0f54639 to
19131f7
Compare
sarken
changed the title
Contributor
Author
|
Noticed some missing stuff re polymorphism. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Issue
https://otwarchive.atlassian.net/browse/AO3-7457
Purpose
This removes an N+1 when rendering mystery blurbs on many, many different pages.
Prior to this change, rendering a mystery blurb called:
This is a guaranteed N+1, because scope calls on associations cannot be preloaded.
The fix here is to change that into a relation,
approved_unrevealed_collections, which can be preloaded.As a regression test/enforcement mechanism, we use strict_loading: true to ensure that exceptions are raised in dev/test if this new association is ever not preloaded. Once that exception was raised, all I had to do was backfill through all the controllers to add
for_blurbpreloads where missing and then the exceptions went away.Because this adds
for_blurbpreloads in a number of places, it probably also removes additional N+1s from the non-mystery blurb as well.This is part of (but does not close) AO3-7457 because it is cleaning up data access for this set of partials before adding fragment caching around it.