update user and ordering in KPI dashboard#955
Conversation
Walkthroughget_connectid_user_counts_cumulative in commcare_connect/reports/helpers.py now returns a single dict; HQ SSO query was changed to filter on has_sso_on_hq_app__isnull=False, ordered by month_group, and per-month counts are computed then accumulated into a cumulative dict. Per-month visit data keys were updated to use activated_commcare_users, activated_connect_users, and activated_personalid_accounts. In commcare_connect/reports/tables.py, AdminReportTable columns were reorganized: connectid_users renamed to "PersonalID Accounts", several columns removed, and activated_commcare_users added. A test assertion for non_preregistered_users was removed. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@commcare_connect/reports/helpers.py`:
- Around line 213-224: The current manual cumulative loop builds
hq_sso_users_data only for months that have new SSO users, causing months with
zero new users to return 0; replace that logic by calling the existing
_get_cumulative_count helper to produce a cumulative mapping across the full
month series (use the visit_data_dict month keys as the time series input) and
then use the returned dict for lookups of activated_commcare_users (keep
references to hq_sso_users, hq_sso_users_data, visit_data_dict, and month_group
when making the change).
🧹 Nitpick comments (1)
commcare_connect/reports/tables.py (1)
21-21: Consider whetherSumColumnis appropriate for an average metric.Using
SumColumnmeans the footer will sum all monthly averages together. Summing averages is not statistically meaningful—if the intent is to show a "total" in the footer, this may be confusing for users viewing "Average Earned by Top FLWs."Consider using a regular
columns.Columnor a custom column that computes a weighted average or leaves the footer blank.
Technical Summary
This handles the simple ordering changes and calculation issues from https://dimagi.atlassian.net/browse/CI-481
I did not finish removing the old calculations, just the display, but will do so in a followup with the rest of the ticket. However, the work in this PR is high priority to get out as we need to pull these numbers for January.
Safety Assurance
Safety story
Internal only page
Labels & Review