Fix org update stats pagination#1504
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds app-scoped organization resolution, billing-period-aware date ranges, paginated daily-version fetching with composite org+app cache keys, and corresponding UI/chart updates plus backend MAU/grouping changes and a new read_device_usage SQL function. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client (UpdateStatsCard / Usage)
participant Cache as ClientCache (org+appId key)
participant Edge as Serverless (useSupabase)
participant DB as Supabase / DB
UI->>Cache: check cached stats (orgId+appId+start+end)
alt cache hit
Cache-->>UI: return cached data
else cache miss
UI->>Edge: request app stats (rangeStart, rangeEnd, appId, orgId)
Edge->>DB: run paginated daily_version/read_device_usage queries (PAGE_SIZE=1000)
DB-->>Edge: return rows/pages
Edge-->>UI: return sorted daily stats / MAU
UI->>Cache: store results under composite key
end
UI->>UI: compute billing-range filter and project finalDailyCounts/finalAppData
UI-->>Chart: render using cycleStart/cycleEnd and projected series
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/dashboard/Usage.vue (1)
82-83: Fix ESLint multiple blank lines error flagged by pipeline.The pipeline reports a
style/no-multiple-empty-linesviolation. Remove the extra blank line.Proposed fix
bandwidth: { [appId: string]: number[] } }>() - // View mode selectors for charts
🤖 Fix all issues with AI agents
In `@src/components/dashboard/UpdateStatsCard.vue`:
- Around line 313-314: Remove the extra blank line after the forEach block in
UpdateStatsCard.vue to satisfy the ESLint style/padded-blocks rule: locate the
forEach callback (the block that iterates with .forEach(...)) and delete the
empty line immediately following its closing brace so the block is not padded by
a blank line.
|
|
||
| // Calculate evolution (compare last two days with data) | ||
| const nonZeroDays = dailyCounts.filter(count => (count || 0) > 0) | ||
| if (nonZeroDays.length >= 2) { | ||
| const lastDayCount = nonZeroDays[nonZeroDays.length - 1] || 0 | ||
| const previousDayCount = nonZeroDays[nonZeroDays.length - 2] || 0 | ||
| if (previousDayCount > 0) { | ||
| lastDayEvolution.value = ((lastDayCount - previousDayCount) / previousDayCount) * 100 | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix ESLint padding error flagged by pipeline.
The pipeline reports a style/padded-blocks violation at line 312-314. Remove the extra blank line after the forEach block.
Proposed fix
}
})
- }
-
- let finalDailyCounts = dailyCounts
+ }
+ let finalDailyCounts = dailyCounts📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Calculate evolution (compare last two days with data) | |
| const nonZeroDays = dailyCounts.filter(count => (count || 0) > 0) | |
| if (nonZeroDays.length >= 2) { | |
| const lastDayCount = nonZeroDays[nonZeroDays.length - 1] || 0 | |
| const previousDayCount = nonZeroDays[nonZeroDays.length - 2] || 0 | |
| if (previousDayCount > 0) { | |
| lastDayEvolution.value = ((lastDayCount - previousDayCount) / previousDayCount) * 100 | |
| } | |
| } | |
| } | |
| }) | |
| } | |
| let finalDailyCounts = dailyCounts |
🧰 Tools
🪛 ESLint
[error] 312-314: Block must not be padded by blank lines.
(style/padded-blocks)
🤖 Prompt for AI Agents
In `@src/components/dashboard/UpdateStatsCard.vue` around lines 313 - 314, Remove
the extra blank line after the forEach block in UpdateStatsCard.vue to satisfy
the ESLint style/padded-blocks rule: locate the forEach callback (the block that
iterates with .forEach(...)) and delete the empty line immediately following its
closing brace so the block is not padded by a blank line.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c63e591a19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const today = new Date() | ||
| today.setHours(0, 0, 0, 0) | ||
|
|
||
| let rangeStart: Date | ||
| let rangeEnd: Date | ||
| const last30DaysStart = new Date(today) | ||
| last30DaysStart.setDate(last30DaysStart.getDate() - 29) |
There was a problem hiding this comment.
In billing-period mode, calculateStats now hardcodes the aggregation window to the last 30 days (last30DaysStart/dayCount = 30) and later just remaps those 30 values into the billing-period arrays. If the current billing cycle is longer than 30 days (e.g., 31‑day months or custom longer periods), any activity from the start of the billing cycle that falls outside the last 30 days is never fetched or counted, so totals and charts underreport usage compared to the previous full-period range. This regression only appears when props.useBillingPeriod is true and the cycle length exceeds 30 days.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This pull request fixes pagination issues in organization update statistics and aligns billing period filtering with the correct app/organization context for accurate chart displays.
Changes:
- Implements pagination for
daily_versiontable queries to handle cases exceeding PostgREST's 1000-row limit - Introduces
effectiveOrganizationcomputed property across components to correctly resolve organization context when viewing app-specific pages - Refactors billing period filtering to apply after fetching last 30 days of data, avoiding redundant API calls
- Adds watchers to refresh app/org-specific usage views when context changes
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/pages/app/[package].vue | Adds appOrganization computed property and awaits organization store initialization to ensure correct org context for app statistics |
| src/components/dashboard/Usage.vue | Implements effectiveOrganization, updates cache keys to include appId, adds direct API fetching for app-level stats, and adds appId watcher |
| src/components/dashboard/UpdateStatsChart.vue | Converts cycleStart and cycleEnd to computed properties using effectiveOrganization, adjusts cycle end to handle expired subscriptions |
| src/components/dashboard/UpdateStatsCard.vue | Adds pagination via fetchDailyVersionStats, implements filterSeriesToBillingPeriod for post-fetch filtering, updates cache keys, and adds appId watcher |
Comments suppressed due to low confidence (1)
src/components/dashboard/UpdateStatsCard.vue:399
- The multiple watchers (lines 371-382, 385-387, 390-392, 395-399) can trigger concurrent calls to
calculateStats, potentially causing race conditions where an older request completes after a newer one, overwriting fresh data with stale results. Consider implementing a request cancellation mechanism or tracking the latest request to ensure only the most recent request updates the component state. A common pattern is to use a request counter or abort controller to ignore stale responses.
watch(() => effectiveOrganization.value?.gid, async (newOrgId, oldOrgId) => {
if (newOrgId && newOrgId !== oldOrgId) {
// Per-org cache will be checked in calculateStats
await calculateStats(true)
}
})
watch(() => props.appId, async (newAppId, oldAppId) => {
if (newAppId !== oldAppId) {
await calculateStats(true)
}
})
// Watch for billing period mode changes - cache is keyed by mode, so no force needed
watch(() => props.useBillingPeriod, async () => {
await calculateStats(false)
})
// Watch for accumulated mode changes - reprocess cached data
watch(() => props.accumulated, async () => {
await calculateStats(false)
})
// Watch for reload trigger - force refetch from API
watch(() => props.reloadTrigger, async (newVal) => {
if (newVal > 0) {
await calculateStats(true)
}
})
| watch(() => effectiveOrganization.value?.gid, async (newOrgId, oldOrgId) => { | ||
| if (newOrgId && newOrgId !== oldOrgId) { | ||
| // Per-org cache will be checked in calculateStats | ||
| await calculateStats(false) | ||
| await calculateStats(true) | ||
| } | ||
| }) |
There was a problem hiding this comment.
The watcher condition should check if oldOrgId is not undefined to avoid unnecessary refetches on initial mount. Currently, when the component mounts and effectiveOrganization first becomes defined, oldOrgId will be undefined, causing the condition newOrgId !== oldOrgId to be true even though the organization hasn't actually changed. Consider adding oldOrgId !== undefined to the condition to prevent this initial refetch, which would be redundant with the onMounted call.
| watch(() => effectiveOrganization.value?.gid, (newOrgId, oldOrgId) => { | ||
| if (newOrgId && newOrgId !== oldOrgId && loadedAlready.value) { | ||
| // Show loading state immediately when org changes (before data is fetched) | ||
| isLoading.value = true | ||
| // Increment reload trigger to force all child charts to refetch | ||
| reloadTrigger.value++ | ||
| } | ||
| }) |
There was a problem hiding this comment.
Similar to UpdateStatsCard, the watcher condition should check if oldOrgId is not undefined to avoid unnecessary actions on initial mount. Consider adding oldOrgId !== undefined to the condition.
|
|
||
| // Cache key includes org and billing mode since date range differs | ||
| const cacheKey = `${currentOrgId}:${props.useBillingPeriod ? 'billing' : '30days'}` | ||
| const cacheKey = `${currentOrgId ?? 'none'}:${props.appId || 'org'}:${props.useBillingPeriod ? 'billing' : '30days'}` |
There was a problem hiding this comment.
The cache key includes useBillingPeriod mode, but the raw data fetched from the API is always the last 30 days (lines 195-204), regardless of this mode. The billing period filtering is applied after fetching (lines 320-332). This means identical data is cached twice under different keys - once for billing mode and once for 30-day mode - wasting memory. Consider removing the billing mode from the cache key and only caching the raw 30-day data, then applying the billing filter as needed when retrieving from cache.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@supabase/migrations/20260125151000_mau_first_seen_device_usage.sql`:
- Around line 3-10: The function signature for read_device_usage (parameters
p_app_id, p_period_start, p_period_end) and the RETURNS TABLE column types must
be fully qualified with pg_catalog to comply with SET search_path = ''. Update
the parameter types to pg_catalog.character varying and pg_catalog.timestamp
without time zone, and update the RETURNS TABLE column types to pg_catalog.date,
pg_catalog.bigint, and pg_catalog.character varying (for "date", "mau", "app_id"
respectively), ensuring every built-in type reference in the CREATE OR REPLACE
FUNCTION header uses the pg_catalog schema.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/dashboard/UpdateStatsCard.vue`:
- Around line 303-306: The sumSeries helper causes TypeScript errors because the
reduce accumulator can be inferred as possibly undefined; update the sumSeries
function (used to compute installedTotal, failedTotal, requestedTotal) to
explicitly type its return as number and call Array.prototype.reduce with a
numeric accumulator type and an initial value (e.g., use reduce<number>(...)
with a 0 initial value) so the accumulator (sum) is always a number and the
function signature is explicit.
♻️ Duplicate comments (1)
src/components/dashboard/UpdateStatsCard.vue (1)
340-351: LGTM! Watchers properly handle organization and app context changes.The watcher at line 341 correctly includes
oldOrgId !== undefinedto prevent unnecessary refetches on initial mount. The app ID watcher appropriately triggers a forced refetch when the app context changes.
|
* chore: update dashboard stats * fix(frontend): paginate update stats for org view * fix(backend): count device usage per day * fix(backend): align mau to period uniques * fix(backend): record mau on stats events * fix(dashboard): use billing range for update stats * fix(dashboard): satisfy prefer-const lint * fix(dashboard): type-safe sumSeries
* chore: update dashboard stats * fix(frontend): paginate update stats for org view * fix(backend): count device usage per day * fix(backend): align mau to period uniques * fix(backend): record mau on stats events * fix(dashboard): use billing range for update stats * fix(dashboard): satisfy prefer-const lint * fix(dashboard): type-safe sumSeries
Summary (AI generated)
Test plan (AI generated)
Screenshots (AI generated)
Checklist (AI generated)
bun run lint:backend && bun run lint.accordingly.
my tests
Generated with AI
Summary by CodeRabbit
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.