Skip to content

quickfix/forecaster-count-quick-update#4396

Open
lsabor wants to merge 3 commits intomainfrom
quickfix/forecaster-count-quick-update
Open

quickfix/forecaster-count-quick-update#4396
lsabor wants to merge 3 commits intomainfrom
quickfix/forecaster-count-quick-update

Conversation

@lsabor
Copy link
Contributor

@lsabor lsabor commented Feb 20, 2026

add update_cached_fields method to post model and call it when the button is pressed in admin

I noticed that you can't trigger an update to the forecaster count and other numeric cached fields on the Post object via the admin panel. This allows us to do that now

Summary by CodeRabbit

  • Chores
    • Renamed admin action and button label from "Update Materialized Fields" to "Update Cached Fields" and updated the success message for clarity.
  • New Features
    • Added a consolidated "update cached fields" operation on posts to refresh multiple cached counts and scores in one action.

add update_cached_fields method to post model and call it when the button is pressed in admin
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 20, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

Replaces the "materialized" update flow with a "cached" update flow: adds Post.update_cached_fields() which calls four existing cache-update methods, and updates the admin action/button and success message to invoke and reflect the cached-fields flow.

Changes

Cohort / File(s) Summary
Admin interface updates
posts/admin.py
Admin action renamed and reworded; action now calls post.update_cached_fields() and button label/short description and success message changed from "Materialized" to "Cached".
Model method addition
posts/models.py
Added Post.update_cached_fields() that aggregates update_forecasts_count, update_forecasters_count, update_vote_score, and update_comment_count.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • hlbmtc

Poem

🐰 A cached field hops into view,
Four little counts now marched in a line,
Admin button changed its sign,
Quiet updates, quick and spry,
Hooray — cached fields multiply!

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'quickfix/forecaster-count-quick-update' is partially related to the changeset. It references the forecaster count update capability, which is one aspect of the change, but misses that the primary change is refactoring from 'pseudo materialized fields' to 'cached fields' — a broader architectural shift affecting multiple cached field updates (open time, scheduled close time, forecasters count, vote score, comment count). The title focuses on only one cached field rather than the main architectural change.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch quickfix/forecaster-count-quick-update

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
posts/models.py (1)

873-877: Consider consolidating into a single save() call.

Each of the four delegated methods issues its own self.save(update_fields=[...]), so update_cached_fields() as currently written produces 4 separate DB round-trips. All four fields could be computed and persisted in one write.

♻️ Proposed refactor — single-save implementation
 def update_cached_fields(self):
-    self.update_forecasts_count()
-    self.update_forecasters_count()
-    self.update_vote_score()
-    self.update_comment_count()
+    # Compute all cached values in-memory first
+    self.forecasts_count = (
+        self.forecasts.filter_within_question_period()
+        .exclude(source=Forecast.SourceChoices.AUTOMATIC)
+        .count()
+    )
+    forecasters = self.get_forecasters()
+    questions = self.get_questions()
+    if questions and not questions[0].include_bots_in_aggregates:
+        forecasters = forecasters.filter(is_bot=False)
+    self.forecasters_count = forecasters.count()
+    self.vote_score = self.get_votes_score()
+    self.comment_count = self.get_comment_count()
+    self.save(update_fields=["forecasts_count", "forecasters_count", "vote_score", "comment_count"])

Alternatively, keep the existing helper methods intact but inline only the field assignments here, or add an internal _compute_* variant to each helper that skips the individual save.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@posts/models.py` around lines 873 - 877, The current update_cached_fields
method calls update_forecasts_count, update_forecasters_count,
update_vote_score, and update_comment_count which each perform their own
self.save(...) causing four DB writes; change update_cached_fields to compute
all four values first (either by calling new internal _compute_* helpers or by
inlining the existing calculation logic from
update_forecasts_count/update_forecasters_count/update_vote_score/update_comment_count)
assign the four cached fields on self, then do a single
self.save(update_fields=[...]) to persist them in one DB round-trip.
posts/admin.py (1)

105-118: Optional: align internal identifiers with the new "Cached Fields" terminology.

The externally visible button label was updated to "Update Cached Fields", but the underlying method name (update_pseudo_materialized_fields_button), the URL path (update-pseudo-materialized-fields/), and the Django URL name (posts_post_update_pseudo_materialized_fields) still use the old pseudo_materialized_fields wording. The readonly_fields entry on line 55 also retains the old name.

This is cosmetic but creates a split-brain naming situation that will confuse future maintainers.

♻️ Proposed rename
 readonly_fields = [
     "notebook",
     "hotness_explanation",
     "view_questions",
-    "update_pseudo_materialized_fields_button",
+    "update_cached_fields_button",
 ]
-    def update_pseudo_materialized_fields_button(self, obj):
+    def update_cached_fields_button(self, obj):
         if not obj:
             return ""
         url = reverse(
-            "admin:posts_post_update_pseudo_materialized_fields", args=[obj.pk]
+            "admin:posts_post_update_cached_fields", args=[obj.pk]
         )
         ...

-    update_pseudo_materialized_fields_button.short_description = "Update Cached Fields"
+    update_cached_fields_button.short_description = "Update Cached Fields"
     custom_urls = [
         path(
-            "<int:post_id>/update-pseudo-materialized-fields/",
+            "<int:post_id>/update-cached-fields/",
             self.admin_site.admin_view(
-                self.process_update_pseudo_materialized_fields_request
+                self.process_update_cached_fields_request
             ),
-            name="posts_post_update_pseudo_materialized_fields",
+            name="posts_post_update_cached_fields",
         ),
     ]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@posts/admin.py` around lines 105 - 118, Rename the internal identifiers to
match the new "Cached Fields" wording: change the admin method
update_pseudo_materialized_fields_button to update_cached_fields_button, update
the URL path segment "update-pseudo-materialized-fields/" to
"update-cached-fields/", and rename the Django URL name
posts_post_update_pseudo_materialized_fields to posts_post_update_cached_fields;
also update the readonly_fields entry that references the old name to the new
cached-fields identifier and adjust any view/handler function or URLconf that
uses the old symbols so all references (method name, URL pattern, URL name, and
readonly_fields) are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@posts/admin.py`:
- Around line 105-118: Rename the internal identifiers to match the new "Cached
Fields" wording: change the admin method
update_pseudo_materialized_fields_button to update_cached_fields_button, update
the URL path segment "update-pseudo-materialized-fields/" to
"update-cached-fields/", and rename the Django URL name
posts_post_update_pseudo_materialized_fields to posts_post_update_cached_fields;
also update the readonly_fields entry that references the old name to the new
cached-fields identifier and adjust any view/handler function or URLconf that
uses the old symbols so all references (method name, URL pattern, URL name, and
readonly_fields) are consistent.

In `@posts/models.py`:
- Around line 873-877: The current update_cached_fields method calls
update_forecasts_count, update_forecasters_count, update_vote_score, and
update_comment_count which each perform their own self.save(...) causing four DB
writes; change update_cached_fields to compute all four values first (either by
calling new internal _compute_* helpers or by inlining the existing calculation
logic from
update_forecasts_count/update_forecasters_count/update_vote_score/update_comment_count)
assign the four cached fields on self, then do a single
self.save(update_fields=[...]) to persist them in one DB round-trip.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 20, 2026

🚀 Preview Environment

Your preview environment is ready!

Resource Details
🌐 Preview URL https://metaculus-pr-4396-quickfix-forecaster-count-quic-preview.mtcl.cc
📦 Docker Image ghcr.io/metaculus/metaculus:quickfix-forecaster-count-quick-update-ce6ee2e
🗄️ PostgreSQL NeonDB branch preview/pr-4396-quickfix-forecaster-count-quic
Redis Fly Redis mtc-redis-pr-4396-quickfix-forecaster-count-quic

Details

  • Commit: ce6ee2e2ff677bd50105af0bea392e0f5721d062
  • Branch: quickfix/forecaster-count-quick-update
  • Fly App: metaculus-pr-4396-quickfix-forecaster-count-quic

ℹ️ Preview Environment Info

Isolation:

  • PostgreSQL and Redis are fully isolated from production
  • Each PR gets its own database branch and Redis instance
  • Changes pushed to this PR will trigger a new deployment

Limitations:

  • Background workers and cron jobs are not deployed in preview environments
  • If you need to test background jobs, use Heroku staging environments

Cleanup:

  • This preview will be automatically destroyed when the PR is closed

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.

3 participants