Conversation
Console (appwrite/console)Project ID: Tip Teams feature lets you group users with membership management and role permissions |
WalkthroughThis PR changes how aggregate totals are handled across the app: the single Usage component now defaults Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR removes aggregate total statistics (counts/sizes displayed above charts) from all usage pages across the Appwrite Console, replacing them with chart-only views. In Key changes:
Confidence Score: 5/5
Important Files Changed
Reviews (1): Last reviewed commit: "fix: remove usage" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <div | ||
| style:max-width="250px" | ||
| style:--input-background-color="var(--bgcolor-neutral-primary)"> | ||
| <InputSelect | ||
| on:change={(e) => goto(`${usagePath}/${e.detail}`)} | ||
| id="period" | ||
| options={[ | ||
| { | ||
| label: '24 hours', | ||
| value: '24h' | ||
| }, | ||
| { | ||
| label: '30 days', | ||
| value: '30d' | ||
| }, | ||
| { | ||
| label: '90 days', | ||
| value: '90d' | ||
| } | ||
| ]} | ||
| value={page.params.period ?? '30d'} /> | ||
| </div> |
There was a problem hiding this comment.
The InputSelect options (24h, 30d, 90d) and the goto navigation pattern here are identical to the implementation already inside usage.svelte (lines 97–118). Since UsageMultiple with showHeader={false} has no built-in period selector, the standalone selector makes sense — but the options list is now duplicated in two places. If the period options ever change in usage.svelte, this page would need a matching update.
Consider extracting the options array to a shared constant (e.g. in a usage.svelte context module export or a helper file) so both places stay in sync automatically.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![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/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte (1)
350-357:⚠️ Potential issue | 🟠 MajorInclude build-only usage in the GB-hours empty-state check.
This card now says it covers executions and builds, but the
{#if}still checks onlydata.usage.executionsMbSecondsTotal. Projects with build usage and zero executions will render “No data to show” even though the computed GB-hours total is non-zero.🐛 Proposed fix
- {`#if` data.usage.executionsMbSecondsTotal} - {`@const` totalGbHours = mbSecondsToGBHours( - data.usage.executionsMbSecondsTotal + data.usage.buildsMbSecondsTotal - )} + {`@const` totalMbSeconds = + data.usage.executionsMbSecondsTotal + data.usage.buildsMbSecondsTotal} + {`#if` totalMbSeconds} + {`@const` totalGbHours = mbSecondsToGBHours(totalMbSeconds)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte around lines 350 - 357, The empty-state conditional currently checks only data.usage.executionsMbSecondsTotal causing builds-only projects to show "No data"; update the {`#if`} to consider the combined MB-seconds (executions + builds) or the computed totalGbHours from mbSecondsToGBHours—e.g., compute totalMbSeconds = data.usage.executionsMbSecondsTotal + data.usage.buildsMbSecondsTotal (or reuse totalGbHours) and use that combined value in the {`#if`} so the card renders when either executions or builds produce usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/routes/`(console)/project-[region]-[project]/databases/usage/[[period]]/+page.svelte:
- Around line 23-43: The period selector InputSelect (id="period") lacks an
accessible name; add one by either rendering a visible label element tied to the
control (e.g., <label for="period">Period</label>) placed before the InputSelect
or by passing an explicit accessible name prop/attribute (e.g.,
aria-label="Period" or aria-labelledby referencing a hidden label) so screen
readers can identify the combobox; update the InputSelect usage where
id="period" and the surrounding markup (and ensure styling keeps layout intact)
and keep the existing on:change/goto and value logic unchanged.
---
Outside diff comments:
In
`@src/routes/`(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte:
- Around line 350-357: The empty-state conditional currently checks only
data.usage.executionsMbSecondsTotal causing builds-only projects to show "No
data"; update the {`#if`} to consider the combined MB-seconds (executions +
builds) or the computed totalGbHours from mbSecondsToGBHours—e.g., compute
totalMbSeconds = data.usage.executionsMbSecondsTotal +
data.usage.buildsMbSecondsTotal (or reuse totalGbHours) and use that combined
value in the {`#if`} so the card renders when either executions or builds produce
usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2b8499f-35a3-4069-b019-2a896f358b00
📒 Files selected for processing (11)
src/lib/layout/usage.sveltesrc/lib/layout/usageMultiple.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/databases/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/storage/bucket-[bucket]/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/storage/usage/[[period]]/+page.svelte
| <div | ||
| style:max-width="250px" | ||
| style:--input-background-color="var(--bgcolor-neutral-primary)"> | ||
| <InputSelect | ||
| on:change={(e) => goto(`${usagePath}/${e.detail}`)} | ||
| id="period" | ||
| options={[ | ||
| { | ||
| label: '24 hours', | ||
| value: '24h' | ||
| }, | ||
| { | ||
| label: '30 days', | ||
| value: '30d' | ||
| }, | ||
| { | ||
| label: '90 days', | ||
| value: '90d' | ||
| } | ||
| ]} | ||
| value={page.params.period ?? '30d'} /> |
There was a problem hiding this comment.
Add an accessible name to the standalone period selector.
This InputSelect no longer lives inside the shared usage header, but it still has no visible label or other accessible name. Screen readers will treat it as an unlabeled combobox.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/routes/`(console)/project-[region]-[project]/databases/usage/[[period]]/+page.svelte
around lines 23 - 43, The period selector InputSelect (id="period") lacks an
accessible name; add one by either rendering a visible label element tied to the
control (e.g., <label for="period">Period</label>) placed before the InputSelect
or by passing an explicit accessible name prop/attribute (e.g.,
aria-label="Period" or aria-labelledby referencing a hidden label) so screen
readers can identify the combobox; update the InputSelect usage where
id="period" and the surrounding markup (and ensure styling keeps layout intact)
and keep the existing on:change/goto and value logic unchanged.
stnguyen90
left a comment
stnguyen90
left a comment
There was a problem hiding this comment.
@HarshMN2345 these are still total stats. We don't want to show any total stats. Instead, we want stuff like reads per day or writes per day or executions per day.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/layout/usageMultiple.svelte (1)
10-11: Consider validating array length alignment betweencountandseriesNames.
seriesNames[index]at line 33 assumes the arrays have matching lengths. If a caller passes mismatched arrays (e.g., 3 metrics series but only 2 names), the chart would renderundefinedas the series name for the extra items.This is likely a minor concern if callers are expected to maintain consistency, but a defensive check or TypeScript tuple type could prevent subtle bugs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/layout/usageMultiple.svelte` around lines 10 - 11, Add a defensive validation for the exported props count and seriesNames: check that count.length === seriesNames.length when the component initializes (e.g., in the script block or onMount) and either throw a clear error or normalize by supplying a fallback name for missing entries (use seriesNames[i] ?? `series ${i + 1}`) before the code that uses seriesNames[index] to render series; reference the exported variables count and seriesNames and the place where seriesNames[index] is used to ensure the check runs prior to rendering.src/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte (1)
21-31: Remove unused data fetching from the loader.The data loader in
+page.ts(lines 60-62) still computesfilesStorageTotal,usersTotal, andexecutionsTotalfrom aggregations, but these values are never accessed in+page.svelte. Removing these unused assignments will clean up unnecessary server-side processing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte around lines 21 - 31, The loader is computing unused aggregations — remove the calculations and assignments for filesStorageTotal, usersTotal, and executionsTotal from the page loader (the code that builds the returned data object), and stop including them in the returned data so server work is not wasted; locate the loader function that sets data.usage and remove the lines that compute filesStorageTotal, usersTotal, and executionsTotal (and any related aggregation calls) and ensure data.usage still contains only the fields referenced by the Svelte page (network, users, executions, imageTransformations, screenshotsGenerated, databasesReads, databasesWrites).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/layout/usageMultiple.svelte`:
- Around line 10-11: Add a defensive validation for the exported props count and
seriesNames: check that count.length === seriesNames.length when the component
initializes (e.g., in the script block or onMount) and either throw a clear
error or normalize by supplying a fallback name for missing entries (use
seriesNames[i] ?? `series ${i + 1}`) before the code that uses
seriesNames[index] to render series; reference the exported variables count and
seriesNames and the place where seriesNames[index] is used to ensure the check
runs prior to rendering.
In
`@src/routes/`(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.svelte:
- Around line 21-31: The loader is computing unused aggregations — remove the
calculations and assignments for filesStorageTotal, usersTotal, and
executionsTotal from the page loader (the code that builds the returned data
object), and stop including them in the returned data so server work is not
wasted; locate the loader function that sets data.usage and remove the lines
that compute filesStorageTotal, usersTotal, and executionsTotal (and any related
aggregation calls) and ensure data.usage still contains only the fields
referenced by the Svelte page (network, users, executions, imageTransformations,
screenshotsGenerated, databasesReads, databasesWrites).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2a1a542c-ec03-44a3-8b02-847fd4b59e55
📒 Files selected for processing (16)
src/lib/charts/legend.sveltesrc/lib/layout/usage.sveltesrc/lib/layout/usageMultiple.sveltesrc/routes/(console)/project-[region]-[project]/auth/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/views/usage/view.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/databases/database-[database]/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/databases/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.tssrc/routes/(console)/project-[region]-[project]/sites/site-[site]/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/storage/bucket-[bucket]/usage/[[period]]/+page.sveltesrc/routes/(console)/project-[region]-[project]/storage/usage/[[period]]/+page.svelte
💤 Files with no reviewable changes (1)
- src/routes/(console)/project-[region]-[project]/settings/usage/[[invoice]]/+page.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/routes/(console)/project-[region]-[project]/functions/usage/[[period]]/+page.svelte
- src/lib/layout/usage.svelte
- src/routes/(console)/project-[region]-[project]/sites/site-[site]/usage/[[period]]/+page.svelte
- src/routes/(console)/project-[region]-[project]/storage/usage/[[period]]/+page.svelte
- src/routes/(console)/project-[region]-[project]/functions/function-[function]/usage/[[period]]/+page.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/usage/[[period]]/+page.svelte
- src/routes/(console)/project-[region]-[project]/storage/bucket-[bucket]/usage/[[period]]/+page.svelte
- src/routes/(console)/project-[region]-[project]/databases/usage/[[period]]/+page.svelte
- src/routes/(console)/project-[region]-[project]/sites/usage/[[period]]/+page.svelte
2 participants
What does this PR do?
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
Improvements
New Features