Skip to content

Avoid extra DB operations#215

Merged
sravfeyn merged 1 commit into
mainfrom
sr/bulk
Jan 25, 2026
Merged

Avoid extra DB operations#215
sravfeyn merged 1 commit into
mainfrom
sr/bulk

Conversation

@sravfeyn

@sravfeyn sravfeyn commented Jan 1, 2026

Copy link
Copy Markdown
Member

Technical Summary

Implements high ROI items from https://github.com/dimagi/connect-id/pull/211/files

https://dimagi.atlassian.net/browse/CI-426

Logging and monitoring

Safety Assurance

Safety story

  • I am confident that this change will not break current and/or previous versions of CommCare apps

Automated test coverage

Tests exists

QA Plan

NA

Labels & Review

  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@coderabbitai

coderabbitai Bot commented Jan 1, 2026

Copy link
Copy Markdown

Walkthrough

This pull request introduces two database query optimizations to reduce N+1 query problems. In messaging/views.py, the Channel list view now uses select_related("server") to eagerly load related server data in a single joined query instead of lazy-loading server relationships per channel. In utils/notification.py, the user query is optimized with a Prefetch object that eagerly loads active FCMDevice objects and exposes them via a user.active_devices attribute, replacing the per-user device lookup pattern. Both changes maintain existing function signatures and behavior while improving database efficiency.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Avoid extra DB operations' directly and clearly summarizes the main change: database query optimization to reduce unnecessary database hits in both messaging/views.py and utils/notification.py.
Description check ✅ Passed The description is related to the changeset, referencing PR #211, Jira CI-426, and documenting safety assurance with test coverage confirmation, though it lacks detailed explanation of specific optimizations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sr/bulk

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ace11e6 and 56ce0f1.

📒 Files selected for processing (2)
  • messaging/views.py
  • utils/notification.py
🧰 Additional context used
🧬 Code graph analysis (2)
utils/notification.py (2)
users/models.py (1)
  • ConnectUser (30-118)
conftest.py (2)
  • fcm_device (31-32)
  • user (21-22)
messaging/views.py (2)
messaging/models.py (1)
  • Channel (25-35)
conftest.py (1)
  • user (21-22)
🔇 Additional comments (2)
utils/notification.py (1)

1-1: LGTM! Effective N+1 query optimization.

The prefetch optimization correctly eliminates per-user database queries for active FCM devices. The logic change from .filter(...).first() to accessing the prefetched user.active_devices[0] maintains identical behavior while reducing database round-trips.

Also applies to: 18-20, 30-30

messaging/views.py (1)

380-380: LGTM! Proper select_related optimization.

Using select_related("server") correctly eliminates N+1 queries when accessing channel.server.key_url in the loop at line 388. This ForeignKey optimization complements the prefetch pattern used elsewhere in the codebase.


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.

@sravfeyn sravfeyn merged commit 61aec5e into main Jan 25, 2026
6 checks passed
@sravfeyn sravfeyn deleted the sr/bulk branch January 25, 2026 14:23
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.

4 participants