Skip to content

Display documents count on Topic index (no counter cache) #223

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

Merged
merged 3 commits into from
Jun 29, 2025

Conversation

Oli0li
Copy link
Collaborator

@Oli0li Oli0li commented Jun 28, 2025

What Issue Does This PR Cover, If Any?

Resolves #213

What Changed? And Why Did It Change?

This is an alternative to #217, as discussed with @dcollie2 and @dmitrytrager.

How Has This Been Tested?

I added between 1 and 10 files to all the Topics on the first page and added topic.documents.size.
Bullet was giving these warnings:

  • When adding topic.documents.size without including :documents_attachments:

Capture d'écran 2025-06-28 152457

  • When adding adding includes(:documents_attachments):

Capture d'écran 2025-06-28 153246

I added the rack-mini-profiler gem and checked performance, which seemed to be OK, so I silenced the alert.

Please Provide Screenshots

Capture d'écran 2025-06-28 190250

Additional Comments

N/A

Oli0li added 3 commits June 28, 2025 18:28
Bullet suggests to use a counter cache, but I'm trying it without one
as discussed in PR #217.
I discussed this with Danny and Dmitry and also looked at Rack Mini
Profiler. The Bullet alert seems to be a false positive, it may not
be worth using a counter cache for the documents count.
Copy link
Collaborator

@dmitrytrager dmitrytrager left a comment

Choose a reason for hiding this comment

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

👍🏼

@dcollie2 dcollie2 merged commit 8b90cc9 into main Jun 29, 2025
4 checks passed
@dcollie2 dcollie2 deleted the 213-display-documents-count-without-counter-cache branch June 29, 2025 00:47
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.

Display Training Materials Count on Topics List
3 participants