Conversation
📝 WalkthroughWalkthroughAdds organization-level credit statistics: frontend UI and types, a new admin-only backend endpoint that aggregates credit transactions/consumptions, DB migrations restoring function EXECUTE privileges, RPC permission updates, tests, translations, and minor related backend/util tweaks. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin Client
participant API as Admin Backend<br/>/private/admin_credits
participant RPC as DB Function<br/>get_admin_org_credit_stats
participant DB as Database
Admin->>API: GET /org-stats/:orgId (admin auth)
API->>API: verify admin auth & validate orgId
API->>RPC: call get_admin_org_credit_stats(p_org_id, p_since)
RPC->>DB: aggregate usage_credit_* tables
DB-->>RPC: aggregated json
RPC-->>API: json payload
API->>API: normalize & format (totals, last_30_days, usage_by_metric)
API-->>Admin: JSON response
Admin->>Admin: render dashboard metrics and per-metric 30-day values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 SQLFluff (4.0.4)supabase/migrations/20260214054927_restore_top_up_usage_credits_for_service_role.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: supabase/migrations/20260213181740_add_admin_org_credit_stats_function.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: 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 |
![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: 0a02b90f83
ℹ️ 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".
| if (transaction.transaction_type === 'purchase') { | ||
| const value = Math.max(amount, 0) | ||
| totals.purchased += value |
There was a problem hiding this comment.
This branch classifies every purchase transaction as a paid top-up, but admin grants created by /private/admin_credits/grant are written through top_up_usage_credits and are recorded as purchase transactions as well, so manual support grants will inflate purchased and leave granted at zero. In environments where admins use this grant flow, the new stats card will report incorrect buy vs. grant numbers.
Useful? React with 👍 / 👎.
| .from('usage_credit_transactions') | ||
| .select('transaction_type, amount, occurred_at') | ||
| .eq('org_id', orgId), |
There was a problem hiding this comment.
The new org-stats endpoint pulls all rows for an org and then aggregates in application code, which means large orgs with long credit history will transfer and iterate potentially massive datasets on every page load; this can cause slow responses or timeouts for admins. Doing the aggregation in SQL (or at least adding bounded windows for heavy parts) would prevent this scalability regression.
Useful? React with 👍 / 👎.
![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
🤖 Fix all issues with AI agents
In `@src/pages/admin/dashboard/credits.vue`:
- Around line 517-527: The "deduction" card label
(t('credit-transaction-deduction')) is showing consumption values
(orgStats.totals.used / orgStats.last_30_days.used); update the template in
credits.vue to render the transaction deduction fields instead by replacing
orgStats.totals.used and orgStats.last_30_days.used with
orgStats.totals.deducted and orgStats.last_30_days.deducted so the displayed
data matches the label (or, alternatively, change the label to a usage wording
if you prefer the current fields).
🧹 Nitpick comments (4)
supabase/functions/_backend/private/admin_credits.ts (1)
345-354: Unbounded queries may cause performance issues for high-volume orgs.Both queries fetch all rows from
usage_credit_transactionsandusage_credit_consumptionsfor the org with noLIMITor server-side date filter. For orgs with extensive history, this could return thousands of rows, increasing memory usage and latency.Consider pushing the 30-day filter to the DB for
last_30_daysstats (or at minimum adding a reasonable row limit), and computing all-time totals from a materialized/cached source if the data grows large. Since this is admin-only and low-traffic, it's not urgent but worth noting for future scale.src/pages/admin/dashboard/credits.vue (2)
231-256: Missing toast notification on stats load failure.
loadOrgBalanceshowstoast.error(...)on failure (Line 224), butloadOrgStatssilently swallows the error with only aconsole.error. For consistency and admin visibility, consider adding a toast here as well.Proposed fix
catch (error) { console.error('Stats load error:', error) orgStats.value = null + toast.error(t('admin-credits-stats-error')) }
561-563: Reusesadmin-credits-no-balancei18n key for missing stats.This key is semantically intended for missing balance data. Consider using a distinct key (e.g.,
admin-credits-no-stats) for the stats section to avoid confusion if the strings diverge later.tests/admin-credits.test.ts (1)
42-42: Consider usingit.concurrent()for parallel test execution.The coding guidelines recommend using
it.concurrent()instead ofit()for tests within the same file. Most tests in this file are independent and could run concurrently. This applies to both the new and existing tests, so it can be deferred.As per coding guidelines: "Design tests for parallel execution; use
it.concurrent()instead ofit()to run tests in parallel within the same file."Also applies to: 128-128, 140-140, 150-150, 161-161, 205-205, 239-239, 251-251, 275-275, 287-287
There was a problem hiding this comment.
Pull request overview
This PR fixes an admin credit grant production failure by restoring service_role permissions for the top_up_usage_credits(...) RPC, and extends the admin credits dashboard with an org-level stats endpoint plus UI to display credit/usage analytics.
Changes:
- Add a migration to restore
service_roleEXECUTE onpublic.top_up_usage_credits(...). - Add
GET /private/admin_credits/org-stats/:orgIdand CORS preflight handling for admin credits routes. - Extend the admin credits UI to fetch and display org credit/usage statistics.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/admin-credits.test.ts |
Adds access-control coverage for the new org-stats route and adds an OPTIONS/CORS preflight test for admin credits endpoints. |
supabase/migrations/20260211034635_restore_top_up_usage_credits_service_role_execute.sql |
Restores service_role execute permission on the credit top-up RPC to fix admin/server grant failures. |
supabase/functions/_backend/private/admin_credits.ts |
Adds a wildcard OPTIONS handler plus a new admin-only org stats endpoint that aggregates transactions and consumptions. |
src/pages/admin/dashboard/credits.vue |
Fetches org stats on org selection / after grants, and renders summary cards + per-metric usage stats. |
| if (transaction.transaction_type === 'purchase') { | ||
| const value = Math.max(amount, 0) | ||
| totals.purchased += value | ||
| if (isRecent) | ||
| last30Days.purchased += value | ||
| continue | ||
| } | ||
|
|
||
| if (transaction.transaction_type === 'grant' || transaction.transaction_type === 'manual_grant') { | ||
| const value = Math.max(amount, 0) | ||
| totals.granted += value | ||
| if (isRecent) | ||
| last30Days.granted += value | ||
| continue | ||
| } |
There was a problem hiding this comment.
The stats aggregation treats transaction_type === 'purchase' as purchased credits and transaction_type in ('grant','manual_grant') as granted credits, but public.top_up_usage_credits(...) always records credit top-ups as transaction_type = 'purchase' (even for manual/admin grants). This will cause granted to stay 0 and inflate purchased. Consider deriving purchased-vs-granted from usage_credit_grants.source (e.g., stripe_top_up vs manual) by joining usage_credit_transactions to usage_credit_grants (or querying grants directly) instead of relying on transaction_type.
|
|
||
| const [transactionsResult, consumptionsResult] = await Promise.all([ | ||
| adminSupabase | ||
| .from('usage_credit_transactions') | ||
| .select('transaction_type, amount, occurred_at') | ||
| .eq('org_id', orgId), | ||
| adminSupabase | ||
| .from('usage_credit_consumptions') | ||
| .select('credits_used, metric, applied_at') | ||
| .eq('org_id', orgId), |
There was a problem hiding this comment.
This endpoint fetches all usage_credit_transactions and usage_credit_consumptions rows for the org and aggregates them in memory. For orgs with long histories this can become slow and memory-heavy, and it also increases egress from PostgREST. Consider doing aggregation in SQL (SUM/GROUP BY + time-window filtering) and only returning the already-aggregated totals/30-day totals/per-metric stats.
| const [transactionsResult, consumptionsResult] = await Promise.all([ | |
| adminSupabase | |
| .from('usage_credit_transactions') | |
| .select('transaction_type, amount, occurred_at') | |
| .eq('org_id', orgId), | |
| adminSupabase | |
| .from('usage_credit_consumptions') | |
| .select('credits_used, metric, applied_at') | |
| .eq('org_id', orgId), | |
| const sinceIso = new Date(sinceMs).toISOString() | |
| const [transactionsResult, consumptionsResult] = await Promise.all([ | |
| adminSupabase | |
| .from('usage_credit_transactions') | |
| .select('transaction_type, amount, occurred_at') | |
| .eq('org_id', orgId) | |
| .gte('occurred_at', sinceIso), | |
| adminSupabase | |
| .from('usage_credit_consumptions') | |
| .select('credits_used, metric, applied_at') | |
| .eq('org_id', orgId) | |
| .gte('applied_at', sinceIso), |
| </div> | ||
|
|
||
| <div v-else class="mt-2 text-gray-500 dark:text-gray-400"> | ||
| {{ t('admin-credits-no-balance') }} |
There was a problem hiding this comment.
In the stats panel fallback, the UI shows t('admin-credits-no-balance'), which is misleading because this section is about statistics (and balance has its own panel above). Use a dedicated translation key/message for “no stats available” (or reuse a generic “no data” message) so users aren’t confused.
| {{ t('admin-credits-no-balance') }} | |
| {{ t('no-data') }} |
argMaxIf is not supported by Cloudflare Analytics Engine SQL API, causing the entire device list query to fail silently and return []. This broke /private/devices for all apps since 5a4828b.
…1627) * Initial plan * Fix COALESCE query to handle NULL values when column doesn't exist The previous query `COALESCE((to_jsonb(apps) ->> 'allow_device_custom_id')::boolean, true)` would fail when the column doesn't exist because: 1. to_jsonb(apps) ->> 'allow_device_custom_id' returns NULL (as text) 2. NULL::text cast to ::boolean fails with an error 3. COALESCE never gets a chance to provide the default The fix: `COALESCE(to_jsonb(apps) ->> 'allow_device_custom_id', 'true')::boolean` 1. Provide the default 'true' string BEFORE the boolean cast 2. COALESCE returns either the column value or 'true' as text 3. Cast the resulting text to boolean Co-authored-by: riderx <4084527+riderx@users.noreply.github.com> * Fix has_usage_credits COALESCE query with same issue Applied the same fix to the has_usage_credits query in pg.ts to prevent potential failures when the column doesn't exist on replicas. Co-authored-by: riderx <4084527+riderx@users.noreply.github.com> * Simplify allow_device_custom_id query - remove unnecessary COALESCE Since the column is NOT NULL DEFAULT true, we can query it directly without the to_jsonb workaround. Reverted pg.ts change as well since that was based on the same incorrect assumption. Co-authored-by: riderx <4084527+riderx@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: riderx <4084527+riderx@users.noreply.github.com>
…ce-role fix(db): restore service_role EXECUTE on top_up_usage_credits
…top-up endpoint (#1632) Migration 20260104120000 revoked EXECUTE on top_up_usage_credits from all roles including authenticated. PR #1630 restored service_role but not authenticated, breaking the complete-top-up endpoint which called the RPC via supabaseClient (authenticated role). Switch to supabaseAdmin (service_role) for the RPC call only. All authorization is already enforced before the call: JWT auth, RBAC permission check (org.update_billing / super_admin), Stripe session ownership + payment verification, and idempotency guards. This matches how the other two callers (stripe_event.ts webhook and admin_credits.ts) already invoke the same function. Closes #1631
* feat(frontend): improve plans page UX for credits-only orgs - Replace misleading 'Don't want to upgrade?' CTA with info banner for orgs using credits without a subscription plan - Remove blue border highlight on Solo plan for credits-only orgs - Use information icon instead of currency icon for credits-only banner - Add new translation keys in all 15 languages via DeepL * fix(frontend): address CodeRabbit review feedback for credits-only UX - Fix Indonesian: use 'kredit' instead of 'pulsa' for terminology consistency - Fix Italian: use informal singular and imperative CTA form - Fix Russian: use plural 'кредитов' for consistency - Fix Chinese: use '积分' instead of '信贷' for consistency - Fix Polish: use imperative mood for CTA link - Improve accessibility: use <button> instead of <div> for clickable banners
…p-go/capgo into riderx/rebase-credit-stats
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
supabase/functions/_backend/private/credits.ts (1)
450-459:⚠️ Potential issue | 🟠 MajorUse authenticated client for user-facing
/complete-top-upendpoint.Line 452 uses
supabaseAdmin(c)to call the RPC, which bypasses JWT-based authentication at the RPC level and violates the backend security guideline for user-facing APIs. Although authentication and org-level permission checks (org.update_billing) occur before the RPC call, the admin SDK should not be used for user-facing endpoints.Fix by either: (1) granting EXECUTE permission on
top_up_usage_creditsto theauthenticatedrole and switching tosupabaseClient(c, token).rpc(...), or (2) moving the RPC logic to an internal-only endpoint guarded bymiddlewareAPISecret.As per coding guidelines: "Never use the Supabase admin SDK (with service key) for user-facing APIs; always use the client SDK with user authentication to enforce RLS policies; admin SDK should only be used for internal operations, triggers, and CRON jobs."
src/components/CreditsCta.vue (1)
31-91:⚠️ Potential issue | 🟡 MinorUse DaisyUI buttons for the new interactive elements.
Both new/updated
<button>elements should included-btnto comply with the UI guidelines for interactive components.
As per coding guidelines, "Use DaisyUI components (d-btn,d-input,d-card) for interactive elements in Vue components".♻️ Suggested adjustment
- <button + <button v-if="props.creditsOnly" type="button" - class="flex items-center w-full p-4 text-left transition-all duration-200 border cursor-pointer bg-blue-50 dark:bg-blue-900/20 border-blue-200 dark:border-blue-800 hover:border-blue-300 dark:hover:border-blue-700 rounded-xl group" + class="d-btn flex items-center w-full p-4 text-left transition-all duration-200 border cursor-pointer bg-blue-50 dark:bg-blue-900/20 border-blue-200 dark:border-blue-800 hover:border-blue-300 dark:hover:border-blue-700 rounded-xl group" `@click`="goToCredits" >- <button + <button v-else type="button" - class="flex items-center w-full p-4 text-left transition-all duration-200 border cursor-pointer bg-gray-50 dark:bg-gray-900 border-gray-200 dark:border-gray-700 hover:border-blue-300 dark:hover:border-blue-700 rounded-xl group" + class="d-btn flex items-center w-full p-4 text-left transition-all duration-200 border cursor-pointer bg-gray-50 dark:bg-gray-900 border-gray-200 dark:border-gray-700 hover:border-blue-300 dark:hover:border-blue-700 rounded-xl group" `@click`="goToCredits" >
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 4: Remove the manual version bump from package.json by reverting the
change to the "version" field ("version": "12.110.6"); leave the version value
as it was on the base branch (or restore the original file state) so release
automation (semantic-release) can manage versioning, changelog, and deployments
instead of committing any manual version changes in package.json.
In `@supabase/functions/_backend/utils/version.ts`:
- Line 1: The exported constant version ('version') was manually bumped; undo
this manual edit and restore the auto-generated value (or revert the file to the
version from main/HEAD that the generator produces) so release automation
controls version.ts; do not commit manual changes to the version constant and,
if needed, re-run the project's generation script or reset this file to the
auto-generated state before pushing.
In
`@supabase/migrations/20260214054927_restore_top_up_usage_credits_for_service_role.sql`:
- Around line 16-18: Update the stale comment that lists caller `#3` to indicate
the RPC is now invoked via supabaseAdmin (service_role) instead of
supabaseClient (authenticated); specifically edit the comment referencing
"supabase/functions/_backend/private/credits.ts — Frontend complete-top-up
endpoint (user JWT)" to reflect that the complete-top-up endpoint now calls the
RPC using supabaseAdmin (service_role).
- Around line 23-30: The GRANT EXECUTE statement for the function
public.top_up_usage_credits(...) is redundant; remove the duplicate GRANT
line(s) that grant EXECUTE to "service_role" for top_up_usage_credits (the
signature uses timestamptz/timestamp with time zone aliases) so only the earlier
migration restores that privilege—delete the GRANT block referencing
top_up_usage_credits(p_org_id uuid, p_amount numeric, p_expires_at timestamp
with time zone, p_source text, p_source_ref jsonb, p_notes text).
🧹 Nitpick comments (2)
supabase/functions/_backend/private/admin_credits.ts (1)
317-319:as anycast bypasses type safety on the RPC call.The cast suppresses TypeScript errors but means parameter name mismatches (e.g.,
p_org_idvsorgId) or RPC renames won't be caught at compile time. Consider adding the function signature to your Supabase generated types instead.src/pages/admin/dashboard/credits.vue (1)
231-256: Silent failure —loadOrgStatsswallows errors without user feedback.
loadOrgBalanceshowstoast.error(...)on failure, butloadOrgStatsonly logs to console. If the stats endpoint fails, the admin sees a silent "No data" state with no indication of an error.Proposed fix — add a toast on failure
catch (error) { console.error('Stats load error:', error) orgStats.value = null + toast.error(t('failed-to-fetch-statistics')) }
| "name": "capgo-app", | ||
| "type": "module", | ||
| "version": "12.110.1", | ||
| "version": "12.110.6", |
There was a problem hiding this comment.
Revert manual version bump in package.json.
Version changes should be left to release automation; please remove this bump from the PR.
Based on learnings: Do not manually deploy or commit version bumps; CI/CD handles version bumping in package.json, CHANGELOG.md generation via semantic-release, and deployment after merge to main.
🤖 Prompt for AI Agents
In `@package.json` at line 4, Remove the manual version bump from package.json by
reverting the change to the "version" field ("version": "12.110.6"); leave the
version value as it was on the base branch (or restore the original file state)
so release automation (semantic-release) can manage versioning, changelog, and
deployments instead of committing any manual version changes in package.json.
| @@ -1,3 +1,3 @@ | |||
| export const version = '12.110.1' | |||
| export const version = '12.110.6' | |||
There was a problem hiding this comment.
Avoid manual version bumps in generated version.ts.
This file is marked as auto-generated; please let release automation update it.
Based on learnings: Do not manually deploy or commit version bumps; CI/CD handles version bumping in package.json, CHANGELOG.md generation via semantic-release, and deployment after merge to main.
🤖 Prompt for AI Agents
In `@supabase/functions/_backend/utils/version.ts` at line 1, The exported
constant version ('version') was manually bumped; undo this manual edit and
restore the auto-generated value (or revert the file to the version from
main/HEAD that the generator produces) so release automation controls
version.ts; do not commit manual changes to the version constant and, if needed,
re-run the project's generation script or reset this file to the auto-generated
state before pushing.
| -- It is also called via supabaseClient (authenticated) from: | ||
| -- 3. supabase/functions/_backend/private/credits.ts (line ~450) | ||
| -- — Frontend complete-top-up endpoint (user JWT) |
There was a problem hiding this comment.
Stale comment: caller #3 was switched to supabaseAdmin.
The PR commit messages indicate the complete-top-up endpoint was explicitly switched to call the RPC via supabaseAdmin (service_role). This comment still states it uses supabaseClient (authenticated), which is now misleading. Update to reflect the current state.
🤖 Prompt for AI Agents
In
`@supabase/migrations/20260214054927_restore_top_up_usage_credits_for_service_role.sql`
around lines 16 - 18, Update the stale comment that lists caller `#3` to indicate
the RPC is now invoked via supabaseAdmin (service_role) instead of
supabaseClient (authenticated); specifically edit the comment referencing
"supabase/functions/_backend/private/credits.ts — Frontend complete-top-up
endpoint (user JWT)" to reflect that the complete-top-up endpoint now calls the
RPC using supabaseAdmin (service_role).
| GRANT EXECUTE ON FUNCTION "public"."top_up_usage_credits"( | ||
| "p_org_id" "uuid", | ||
| "p_amount" numeric, | ||
| "p_expires_at" timestamp with time zone, | ||
| "p_source" "text", | ||
| "p_source_ref" "jsonb", | ||
| "p_notes" "text" | ||
| ) TO "service_role"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for duplicate/similar migration files granting EXECUTE on top_up_usage_credits
fd -e sql . supabase/migrations | xargs grep -l 'top_up_usage_credits' 2>/dev/null | sort
echo "---"
# Show the content of the earlier migration if it exists
fd 'restore_top_up_usage_credits_service_role_execute' supabase/migrations --exec cat {}Repository: Cap-go/capgo
Length of output: 678
Remove the redundant GRANT statement.
This migration duplicates a GRANT from 20260211034635_restore_top_up_usage_credits_service_role_execute.sql. Both grant EXECUTE on public.top_up_usage_credits(uuid, numeric, timestamptz, text, jsonb, text) to service_role. Although timestamptz and timestamp with time zone are aliases, they resolve to the same function signature. Since the earlier migration already restores this privilege, this grant is redundant and should be removed.
🤖 Prompt for AI Agents
In
`@supabase/migrations/20260214054927_restore_top_up_usage_credits_for_service_role.sql`
around lines 23 - 30, The GRANT EXECUTE statement for the function
public.top_up_usage_credits(...) is redundant; remove the duplicate GRANT
line(s) that grant EXECUTE to "service_role" for top_up_usage_credits (the
signature uses timestamptz/timestamp with time zone aliases) so only the earlier
migration restores that privilege—delete the GRANT block referencing
top_up_usage_credits(p_org_id uuid, p_amount numeric, p_expires_at timestamp
with time zone, p_source text, p_source_ref jsonb, p_notes text).
|
Summary (AI generated)
42501) by restoringservice_roleexecute permission onpublic.top_up_usage_credits(...)GET /private/admin_credits/org-stats/:orgIdplus CORS preflight handling for admin credits routesadmin/dashboard/creditsMotivation (AI generated)
Admin credit grants were failing in production and the admin credits page was missing actionable usage/purchase stats.
Business Impact (AI generated)
Restores manual grant operations for support/admin teams and improves decision-making with immediate credit analytics visibility.
Test Plan (AI generated)
bun lintbun lint:backendbunx eslint tests/admin-credits.test.tsbunx vitest run tests/admin-credits.test.ts(local test target returned404for the new route during this run)Screenshots (AI generated)
Checklist (AI generated)
bun run lint:backend && bun run lint.accordingly.
my tests
Generated with AI
Summary by CodeRabbit
New Features
Bug Fixes
Localization
Tests