Conversation
add update_cached_fields method to post model and call it when the button is pressed in admin
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughReplaces the "materialized" update flow with a "cached" update flow: adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
posts/models.py (1)
873-877: Consider consolidating into a singlesave()call.Each of the four delegated methods issues its own
self.save(update_fields=[...]), soupdate_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 oldpseudo_materialized_fieldswording. Thereadonly_fieldsentry 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.
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
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