Skip to content

AO3-7403 Eager load associations that are used on inbox page#5890

Open
sherin wants to merge 7 commits into
otwcode:masterfrom
sherin:inbox_n_plus_one
Open

AO3-7403 Eager load associations that are used on inbox page#5890
sherin wants to merge 7 commits into
otwcode:masterfrom
sherin:inbox_n_plus_one

Conversation

@sherin

@sherin sherin commented Jun 18, 2026

Copy link
Copy Markdown

Pull Request Checklist

Issue

https://otwarchive.atlassian.net/browse/AO3-7403

Purpose

Removes n+1 from the inbox page.

Testing Instructions

How can the Archive's QA team verify that this is working as you intended?

(I am guessing the following as I am new)

I am not sure if n+1s are tracked in the QA env. If not, making sure the behavior has not changed would be good enough. If sentries are raised in QA for n+1s, then this PR should stop those errors for this page.

Credit

Sherin

@sarken sarken changed the title eager load assocations that are used on inbox page AO3-7403 Eager load associations that are used on inbox page Jun 19, 2026
@sarken

sarken commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Hi, and thank you for your interest in contributing!

If you could take a few minutes to fill out the pull request template even though this is still a draft, we'd appreciate it. For now, I've updated it with what I believe is the right issue number and set that issue's Jira status to In Review so no one else starts to work on it. If I used the wrong issue, please feel free to make any necessary edits and let me know so I can update the right Jira issue.

If you'd like the ability to comment on, assign, and transition issues in the future, you're welcome to create a Jira account! It makes things a bit easier for us on the organizational side if the Full Name on your Jira account either closely matches the name you'd like us to credit in the release notes or includes it in parentheses, e.g. "Nickname (CREDIT NAME)."

Once you've done that (or if you've already done it -- Jira has been unreliable about showing us new accounts in the admin panel lately), you can either reply here or send an email to otw-coders@transformativeworks.org with your account name and email address and we'll set up the permissions for you.

Thanks again for contributing! If you have any questions, you can contact us at the same email address listed above.

@sherin

sherin commented Jun 19, 2026

Copy link
Copy Markdown
Author

Hi, and thank you for your interest in contributing!

If you could take a few minutes to fill out the pull request template even though this is still a draft, we'd appreciate it. For now, I've updated it with what I believe is the right issue number and set that issue's Jira status to In Review so no one else starts to work on it. If I used the wrong issue, please feel free to make any necessary edits and let me know so I can update the right Jira issue.

Thank you @sarken ! Oops I did not think anybody would look at this draft PR :) Thanks for updating it. I have sent an email with the relevant jira information so I will wait for that before opening up this PR for review.

@sherin sherin marked this pull request as ready for review June 21, 2026 04:34
Comment thread app/models/inbox_comment.rb Outdated
Comment thread factories/comments.rb Outdated
user { create(:user) }
feedback_comment { create(:comment) }

trait :with_guest_comment do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than modify the factory I think it's more appropriate to modify your context blocks. We don't need to extract to a shared file for something used in one spot.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed

@sherin

sherin commented Jun 28, 2026

Copy link
Copy Markdown
Author

@nateberkopec thanks for reviewing. i've addressed your feedback.

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.

3 participants