fix(dashboard): tab placement, period selector, org notification stats - #2786
Conversation
Move org dashboard tabs to the layout chrome like app/settings pages, restyle the day range control as a segmented pill, and replace the notifications tab settings panel with org-wide push notification stats (with beta badge). Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughAdds organization-wide notification statistics aggregation and a dashboard panel with selectable periods, localized states, demo data, multi-app query support, and new preview assets. ChangesOrganization notification statistics
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant OrgNotificationStatsPanel
participant NotificationsAPI
participant readOrgNotificationOverview
participant readNotificationStatsCF
participant notification_events
Dashboard->>OrgNotificationStatsPanel: select organization and day range
OrgNotificationStatsPanel->>NotificationsAPI: POST /org_notification_stats with org_id and days
NotificationsAPI->>readOrgNotificationOverview: load overview metrics
NotificationsAPI->>readNotificationStatsCF: aggregate batched app IDs
readNotificationStatsCF->>notification_events: query notification event counts
NotificationsAPI-->>OrgNotificationStatsPanel: return stats and overview
OrgNotificationStatsPanel-->>Dashboard: render cards and event breakdown
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3616912c-a7fe-4df3-9789-2d108f4c0d2f) |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Use one multi-app Analytics Engine query (capped at 64 apps) with throw-on-error for org dashboard stats, and ignore stale frontend responses when the period changes mid-flight. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c9c15931-f0b2-4365-9dd1-c303c8405c00) |
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Keep Analytics Engine queries bounded at 64 app indexes each, but loop chunks until every org app is included so dashboard totals stay complete. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fdc20d60-30cb-47a9-bba3-6a8627f303c9) |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/pr-previews/dashboard-tabs-notif-stats.webp`:
- Line 1: Remove the committed PR-preview screenshot asset from the repository;
do not replace it with another tracked file. Keep preview imagery attached only
to the pull request description so it is excluded from the asset pipeline and
repository history.
In `@messages/en.json`:
- Around line 2582-2587: Add the missing not-authenticated translation key to
messages/en.json, using the existing English authentication wording convention.
Keep the surrounding organization notification statistics keys unchanged and
ensure OrgNotificationStatsPanel.vue resolves this key to a human-readable
message.
In `@src/components/dashboard/OrgNotificationStatsPanel.vue`:
- Around line 113-121: Update the notifications stats request in the panel’s
fetch flow to use the API-key authentication scheme expected by middlewareKey(),
replacing the Supabase session bearer token; preserve the existing request URL
and headers otherwise.
In `@supabase/functions/_backend/public/notifications/index.ts`:
- Around line 859-877: Update readOrgNotificationOverview to obtain the
PostgreSQL client with getPgClient(c, true) so the analytical query uses the
read replica. Remove the separate apps COUNT subquery and derive the returned
apps value from the aggregated app_ids array, using the filtered array’s length
while preserving the existing campaigns and configured_providers results.
In `@supabase/functions/_backend/utils/nativeNotifications.ts`:
- Around line 590-591: Update the days normalization before constructing since
so non-finite values, including NaN from invalid query input, fall back to 30
days; retain the existing truncation and 1–92 day clamping for valid values, and
ensure since is always created from a valid date.
In `@tests/native-notifications-ae.unit.test.ts`:
- Around line 198-210: Add tests covering both new validation paths in
buildNotificationStatsQuery: assert that a query without appId or appIds throws,
and that campaignId combined with multiple appIds throws. Keep the tests focused
on the expected rejection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6c3176d7-92a8-49c7-8f64-2eb4256ca036
📒 Files selected for processing (8)
assets/pr-previews/dashboard-tabs-notif-stats.webpmessages/en.jsonsrc/components/dashboard/OrgNotificationStatsPanel.vuesrc/components/dashboard/PeriodDaySelector.vuesrc/pages/dashboard.vuesupabase/functions/_backend/public/notifications/index.tssupabase/functions/_backend/utils/nativeNotifications.tstests/native-notifications-ae.unit.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/pr-previews/dashboard-tabs-notif-stats.webp`:
- Line 1: Remove the committed PR-preview screenshot asset from the repository;
do not replace it with another tracked file. Keep preview imagery attached only
to the pull request description so it is excluded from the asset pipeline and
repository history.
In `@messages/en.json`:
- Around line 2582-2587: Add the missing not-authenticated translation key to
messages/en.json, using the existing English authentication wording convention.
Keep the surrounding organization notification statistics keys unchanged and
ensure OrgNotificationStatsPanel.vue resolves this key to a human-readable
message.
In `@src/components/dashboard/OrgNotificationStatsPanel.vue`:
- Around line 113-121: Update the notifications stats request in the panel’s
fetch flow to use the API-key authentication scheme expected by middlewareKey(),
replacing the Supabase session bearer token; preserve the existing request URL
and headers otherwise.
In `@supabase/functions/_backend/public/notifications/index.ts`:
- Around line 859-877: Update readOrgNotificationOverview to obtain the
PostgreSQL client with getPgClient(c, true) so the analytical query uses the
read replica. Remove the separate apps COUNT subquery and derive the returned
apps value from the aggregated app_ids array, using the filtered array’s length
while preserving the existing campaigns and configured_providers results.
In `@supabase/functions/_backend/utils/nativeNotifications.ts`:
- Around line 590-591: Update the days normalization before constructing since
so non-finite values, including NaN from invalid query input, fall back to 30
days; retain the existing truncation and 1–92 day clamping for valid values, and
ensure since is always created from a valid date.
In `@tests/native-notifications-ae.unit.test.ts`:
- Around line 198-210: Add tests covering both new validation paths in
buildNotificationStatsQuery: assert that a query without appId or appIds throws,
and that campaignId combined with multiple appIds throws. Keep the tests focused
on the expected rejection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6c3176d7-92a8-49c7-8f64-2eb4256ca036
📒 Files selected for processing (8)
assets/pr-previews/dashboard-tabs-notif-stats.webpmessages/en.jsonsrc/components/dashboard/OrgNotificationStatsPanel.vuesrc/components/dashboard/PeriodDaySelector.vuesrc/pages/dashboard.vuesupabase/functions/_backend/public/notifications/index.tssupabase/functions/_backend/utils/nativeNotifications.tstests/native-notifications-ae.unit.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
🛑 Comments failed to post (1)
assets/pr-previews/dashboard-tabs-notif-stats.webp (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
PR-preview screenshots don't need to live in the repo.
Attach the image to the PR description instead of committing it under
assets/, which may be picked up by the asset pipeline and grows repo history permanently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/pr-previews/dashboard-tabs-notif-stats.webp` at line 1, Remove the committed PR-preview screenshot asset from the repository; do not replace it with another tracked file. Keep preview imagery attached only to the pull request description so it is excluded from the asset pipeline and repository history.
Move org dashboard notification aggregation to /private/org_notification_stats with middlewareAuth, harden days parsing, add missing not-authenticated copy, and cover new query validation paths in unit tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c01f4f81-c76e-4445-b1a1-f73a32242298) |
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Reject non-string and non-UUID org_id with 400 instead of throwing on trim or casting invalid values in permission checks. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f7d78f32-fe5f-4403-93eb-4a35b738a157) |
Capture the real /dashboard latency and notifications tabs from the local worktree app so the PR preview matches the UI. Co-authored-by: Cursor <cursoragent@cursor.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_bda49035-24d3-4182-a760-2a2e6763f0ef) |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/functions/_backend/public/notifications/index.ts (1)
859-860: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the existing organization stats API contract.
Requiring
app_idhere breaks prior/stats?org_id=...callers before they can reach the new private organization endpoint. Keep the organization path compatible—delegate it internally or version-detect the request—rather than removing its accepted parameter and response behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/functions/_backend/public/notifications/index.ts` around lines 859 - 860, Update the notifications stats handler around assertAppPermission to preserve existing /stats?org_id=... callers: distinguish organization requests from app_id requests and route organization requests through the existing internal/private organization stats flow. Continue enforcing app permissions and returning the current app response for app_id requests, while retaining the established organization parameter and response contract.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@supabase/functions/_backend/public/notifications/index.ts`:
- Around line 859-860: Update the notifications stats handler around
assertAppPermission to preserve existing /stats?org_id=... callers: distinguish
organization requests from app_id requests and route organization requests
through the existing internal/private organization stats flow. Continue
enforcing app permissions and returning the current app response for app_id
requests, while retaining the established organization parameter and response
contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd307307-1e9b-490c-af5c-5dcdcd3adf6c
📒 Files selected for processing (10)
assets/pr-previews/dashboard-notifications.webpassets/pr-previews/dashboard-tabs-notif-stats.webpcloudflare_workers/api/index.tsmessages/en.jsonsrc/components/dashboard/OrgNotificationStatsPanel.vuesupabase/functions/_backend/private/org_notification_stats.tssupabase/functions/_backend/public/notifications/index.tssupabase/functions/_backend/utils/nativeNotifications.tssupabase/functions/private/index.tstests/native-notifications-ae.unit.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
|



Summary (AI generated)
/dashboardtabs with app/settings/admin layout chrome (tabs above content, blue content panel) instead of nesting them under the trial banner in the scroll area.PeriodDaySelectoras a segmented pill control matching Usage chart toggles (touch-friendly, clearer selected state).POST /private/org_notification_stats(org.read).Live screenshots (AI generated)
Latency tab + period selector + beta badges (local worktree app):
Notifications tab with org push stats aggregation:
Motivation (AI generated)
The org dashboard tabs did not sit in the same place as every other tabbed surface, the day range control looked inconsistent/low-quality next to Usage controls, and the Notifications tab incorrectly exposed email prefs (already available under org settings) instead of org-level notification analytics.
Business Impact (AI generated)
Operators can scan org push notification health from the main dashboard without digging into per-app screens or confusing settings with analytics, and the shared period control feels consistent across Capgo charts.
Test Plan (AI generated)
/dashboardand confirm tabs sit at the top like app/settings layouts, with trial banner inside the blue content areaPOST /private/org_notification_statswith Bearer JWT +{ org_id, days }returns mergeddata+overviewfor an org memberGenerated with AI
Note
Medium Risk
New org-scoped analytics endpoint queries CF across many apps and changes what the dashboard Notifications tab shows; layout-only risk is low.
Overview
Org dashboard tabs now sit above the blue content panel (aligned with other tabbed pages), the Notifications tab is marked beta, and its content switches from org email notification settings to org-wide push notification stats via a new
OrgNotificationStatsPanel(period selector, summary cards, event breakdown, demo mode when billing blocks real data).A new authenticated
POST /private/org_notification_statsendpoint loads campaign/provider/app counts from Postgres and merges Cloudflare Analytics notification events across all org apps (batched,org.readRBAC).buildNotificationStatsQuery/readNotificationStatsCFnow accept multipleappIdsand optionalthrowOnErrorfor that aggregation.PeriodDaySelectoris restyled as a segmented pill control with improved a11y. Copy added inen.jsonfor the new panel.Reviewed by Cursor Bugbot for commit a5b8930. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit