Conversation
… and error handling across dashboard
📝 WalkthroughWalkthroughThis PR updates admin dashboard workflows, shared Supabase data access, authentication and authorization routes, configuration editors, error handling, pagination, date handling, release notes, and the frontend application version. ChangesAdmin dashboard and editor workflows
Backend data and authentication platform
Runtime and release
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
admin-web/app/api/admin/create-payment-record/route.ts (1)
38-38: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
subscription_idis validated as required but never persisted.
body.subscription_idis enforced in the required-fields check but is not written anywhere in thepurchase_historyinsert (lines 51-62). If the record should be traceable to a subscription, that association is now silently dropped.🤖 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 `@admin-web/app/api/admin/create-payment-record/route.ts` at line 38, Update the purchase_history insert in the create-payment-record route to persist the validated body.subscription_id value in the corresponding subscription association column. Keep the existing required-field validation and ensure the value is included in both the insert columns and values.admin-web/components/tables/streaks-table.tsx (1)
22-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
parseLocalDateingetStudyStatus
user_study_streaks.last_study_dateis aDATE, sonew Date(lastStudyDate)can shift the day in western timezones and misclassify "Active Today" vs "At Risk". UseparseLocalDate(lastStudyDate)here to match the display logic.🤖 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 `@admin-web/components/tables/streaks-table.tsx` around lines 22 - 33, Update getStudyStatus to parse lastStudyDate with the existing parseLocalDate helper instead of constructing a Date directly, preserving the current day-difference and status logic.admin-web/app/(dashboard)/subscriptions/page.tsx (1)
15-60: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPagination bounds don't match client-side tier/status filtering.
totalPages/totalResultsare computed from the unfiltered server search results, butfilteredUsers(and the tier breakdown stats derived from it) only reflect the tier/status filter applied to the current page of 50 users. With a tier or status filter active, an admin can page through screens with few or zero matching rows whiletotalPagesstill claims more pages exist, and there's no way to browse "all users matching this filter" since filtering never happens server-side. This also means the free/standard/plus/premium stat cards silently changed scope from "all matched users" to "current page's matched users."Consider either passing tier/status to
searchUsersso filtering happens server-side (recomputingtotalPagesfrom the filtered count), or resettingpageto 0 wheneverfilterTier/filterStatuschange and clearly messaging that filters only apply within the loaded page.🤖 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 `@admin-web/app/`(dashboard)/subscriptions/page.tsx around lines 15 - 60, The subscriptions pagination currently derives totalPages from unfiltered search results while tier/status filtering only applies to the current page, causing incomplete results and misleading stats. Update SubscriptionsPage and the searchUsers query to pass filterTier and filterStatus server-side, then compute totalResults and totalPages from the filtered response while preserving the existing filteredUsers and stats behavior against the returned page.admin-web/app/api/admin/analytics/features/route.ts (1)
74-163: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winHandle pagination errors and use a deterministic sort key
All seven
fetchAllRowscalls ignore the returnederror, so a paging failure can still produce partial analytics with no signal. The.order()columns are also non-unique (created_at,unlocked_at,user_id), which can make.range()pages overlap or skip rows when ties exist. Checkerroron each call and add a unique tiebreaker (for exampleid) to each query.🤖 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 `@admin-web/app/api/admin/analytics/features/route.ts` around lines 74 - 163, The seven fetchAllRows calls in the analytics route must propagate or handle their returned errors instead of continuing with partial data; update each destructuring and follow the route’s existing error-response pattern. Make every paginated query’s ordering deterministic by ordering first on its current timestamp or user key and then on a unique stable identifier such as id, including the study-modes query.admin-web/app/(dashboard)/memory-verses/page.tsx (1)
56-77: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAvoid capping suggested verses at 200 here The hard-coded
limit=200is applied beforecategoryFilter, so the page only ever sees the first 200 rows. If this list can grow past that, later verses/categories will disappear from the table and counts. Page through the API or fetch all rows before filtering.🤖 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 `@admin-web/app/`(dashboard)/memory-verses/page.tsx around lines 56 - 77, Update fetchVerses so it retrieves the complete suggested-verses dataset instead of hard-capping the API request at limit=200. Either paginate through all API pages or use the API’s supported unbounded/all-rows option, then preserve the existing client-side category filtering and full-dataset stats behavior.
🧹 Nitpick comments (16)
admin-web/app/api/admin/path-topics/reorder/route.ts (1)
37-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd runtime validation for the request body before forwarding to the Edge Function.
The body is type-asserted as
ReorderTopicsRequestbut never validated at runtime. Malformed input (missinglearning_path_idortopic_orders) is forwarded directly to the Edge Function. Consider validating the body structure before invoking.♻️ Suggested validation
// Parse request body const body: ReorderTopicsRequest = await request.json() + + if (!body.learning_path_id || !Array.isArray(body.topic_orders) || body.topic_orders.length === 0) { + return NextResponse.json( + { error: 'learning_path_id and a non-empty topic_orders array are required' }, + { status: 400 } + ) + } // Call the admin-learning-path-topics Edge Function🤖 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 `@admin-web/app/api/admin/path-topics/reorder/route.ts` around lines 37 - 42, Validate the parsed body in the reorder route before invoking the Edge Function, checking that learning_path_id is present and valid and topic_orders has the expected structure. Return the route’s existing client-error response for malformed input, and only pass validated data to supabaseUser.functions.invoke in the admin-learning-path-topics reorder flow.admin-web/middleware.ts (1)
49-60: 🚀 Performance & Scalability | 🔵 TrivialConsider excluding
/api/*from the matcher to avoid doubling auth checks.Per coding guidelines, every route under
app/api/admin/**/route.tsalready verifies auth viacreateClient()/createAdminClient(). This middleware's matcher also covers those API routes, sosupabase.auth.getUser()(a network round-trip to the Auth server) runs twice per admin API request — once here, once again inside the route handler. Scoping the matcher to page routes only (or accepting the double-check as a deliberate tradeoff) would reduce redundant latency on data-query endpoints.As per coding guidelines, "Every API route in app/api/admin/* must verify user auth and admin status before processing, using createClient() and createAdminClient()."
🤖 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 `@admin-web/middleware.ts` around lines 49 - 60, Update the exported config.matcher so admin API paths under /api/admin are excluded from middleware matching, leaving authentication to each route handler’s createClient/createAdminClient checks. Preserve middleware matching for admin page routes and the existing static, image, favicon, and public-file exclusions.Source: Coding guidelines
admin-web/components/dialogs/edit-system-config-dialog.tsx (1)
59-118: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNo guard against submitting an emptied numeric field.
Each numeric field now stores
''when parsing fails (e.g., user clears the input), replacing the old||default/NaN behavior. That's a UX improvement for visibility, buthandleSubmitstill callsonSave(formData)unconditionally — if a field is left empty,''is sent to the update API instead of a valid token/day count. Consider disabling the Save button (or validating beforeonSave) when any numeric field informDatais''.Also applies to: 122-189, 318-328, 340-357
🤖 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 `@admin-web/components/dialogs/edit-system-config-dialog.tsx` around lines 59 - 118, Update handleSubmit and the Save-button flow in the edit system config dialog to prevent onSave(formData) when any numeric configuration field contains ''. Validate all affected token/day-count fields, keep Save disabled or reject submission while any is empty, and preserve saving when every field has a valid numeric value.admin-web/lib/supabase/list-all-users.ts (1)
31-42: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
getAuthEmailMapfetches the entire user base even for small ID sets.Every caller passes a small set of user IDs (typically 10–500), but
getAuthEmailMapalways callslistAllAuthUsers, which fetches all users (up to 50k across 50 sequential API calls) and then filters in memory. For admin dashboard routes that are called frequently, this is a significant performance hit.Consider using
supabaseAdmin.auth.admin.getUserById(id)in parallel for small ID sets, falling back tolistAllAuthUsersonly whenuserIdsis omitted or exceeds a threshold.♻️ Proposed refactor
export async function getAuthEmailMap( supabaseAdmin: SupabaseClient, userIds?: string[] ): Promise<Record<string, string>> { + // When no filter is provided, fetch all users if (!userIds || userIds.length === 0) { const users = await listAllAuthUsers(supabaseAdmin) - const idFilter = userIds ? new Set(userIds) : null return Object.fromEntries( - users - .filter(u => (idFilter ? idFilter.has(u.id) : true)) - .map(u => [u.id, u.email || '']) + users.map(u => [u.id, u.email || '']) ) } + + // For small ID sets, fetch individually in parallel (avoids scanning entire user base) + if (userIds.length <= 100) { + const results = await Promise.all( + userIds.map(id => supabaseAdmin.auth.admin.getUserById(id)) + ) + return Object.fromEntries( + results + .filter(r => !r.error && r.data?.user) + .map(r => [r.data!.user!.id, r.data!.user!.email || '']) + ) + } + + // For large ID sets, fetch all and filter + const users = await listAllAuthUsers(supabaseAdmin) + const idFilter = new Set(userIds) + return Object.fromEntries( + users + .filter(u => idFilter.has(u.id)) + .map(u => [u.id, u.email || '']) + ) }Please verify that
supabaseAdmin.auth.admin.getUserById(id)is available in@supabase/supabase-jsv2.95.2 and returns{ data: { user: User | null }, error }.🤖 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 `@admin-web/lib/supabase/list-all-users.ts` around lines 31 - 42, Update getAuthEmailMap to use supabaseAdmin.auth.admin.getUserById for each provided ID in parallel when the ID set is below a defined threshold, mapping returned users by ID and preserving empty emails. Fall back to listAllAuthUsers when userIds is omitted or exceeds the threshold, and handle null users or API errors consistently with the existing return contract. Confirm the getUserById response shape supported by `@supabase/supabase-js` v2.95.2.admin-web/components/charts/cost-trend-chart.tsx (1)
25-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
parseLocalDateto a shared utility.This helper is duplicated in
streaks-table.tsx(lines 24–27) anddaily-verses-table.tsx(lines 42–45), with a minor inconsistency: thedaily-verses-table.tsxversion omitsslice(0, 10). Extract it toadmin-web/lib/utils/date.tsto ensure consistency and maintainability.As per coding guidelines: "Follow the DRY (Don't Repeat Yourself) principle by extracting common functionality into reusable components."
♻️ Proposed extraction to shared utility
# admin-web/lib/utils/date.ts — add the shared helper +/** + * Parse a 'YYYY-MM-DD' date string as a LOCAL date. + * (new Date(str) parses as UTC midnight, which shifts the day in western timezones.) + */ +export function parseLocalDate(dateStr: string): Date { + const [year, month, day] = dateStr.slice(0, 10).split('-').map(Number) + return new Date(year, month - 1, day) +}Then in each component, replace the local definition with an import:
# admin-web/components/charts/cost-trend-chart.tsx -const parseLocalDate = (dateStr: string) => { - const [year, month, day] = dateStr.slice(0, 10).split('-').map(Number) - return new Date(year, month - 1, day) -} +import { parseLocalDate } from '`@/lib/utils/date`'🤖 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 `@admin-web/components/charts/cost-trend-chart.tsx` around lines 25 - 28, Extract parseLocalDate into the shared date utility at admin-web/lib/utils/date.ts, preserving the normalized date-only parsing via slice(0, 10). Remove the duplicated local helpers from cost-trend-chart.tsx, streaks-table.tsx, and daily-verses-table.tsx, and import the shared utility in each component.Source: Coding guidelines
admin-web/app/(dashboard)/blogs/page.tsx (1)
61-72: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
loadStatsfires 3 separate requests just to read totals.Each call fetches
limit: 1purely to read.total, discarding the row data. A dedicated aggregate/count endpoint (or a single endpoint returning all three counts) would avoid 3 round-trips on every load/toggle/delete.🤖 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 `@admin-web/app/`(dashboard)/blogs/page.tsx around lines 61 - 72, Update loadStats to use a dedicated aggregate/count API, or a single endpoint returning total, published, and draft counts, instead of issuing three listBlogPosts requests with limit: 1. Preserve the existing setStats mapping and error handling while reducing the operation to one request.admin-web/components/modals/subscription-price-update-modal.tsx (1)
46-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
getCurrencySymbolto a shared utility.This function is duplicated verbatim in
pricing-editor.tsx(lines 148–156). Extract it to a shared module (e.g.,@/lib/utils/currency.ts) so both files import from one place.🤖 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 `@admin-web/components/modals/subscription-price-update-modal.tsx` around lines 46 - 54, Extract the duplicated getCurrencySymbol function from the modal and pricing editor into a shared currency utility module, then import and use that shared function in both components. Preserve the existing currency mappings and fallback behavior.admin-web/app/(dashboard)/learning-paths/[id]/page.tsx (1)
13-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
iconMapto a shared module to avoid duplication.The comment on line 13 explicitly states this map "matches learning-paths-table," confirming the same mapping exists in at least two files. Extract it to a shared constant (e.g.,
@/lib/constants/icon-map.tsor@/types/admin.ts) so both consumers reference a single source of truth.🤖 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 `@admin-web/app/`(dashboard)/learning-paths/[id]/page.tsx around lines 13 - 25, Extract the duplicated icon mapping from iconMap into a shared exported constant, then update this page and the matching learning-paths-table consumer to import and reuse it. Remove the local map while preserving the existing icon-name-to-emoji values and lookup behavior.admin-web/components/dialogs/add-study-guide-dialog.tsx (1)
87-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRetry after a step-2 failure ignores subsequent form edits.
Once
createdTopicIdis set (step 1 succeeded, step 2 failed), a retry skips step 1 entirely. If the user edits Title/Description/etc. before retrying, those edits aren't persisted — the previously-created topic keeps its original values. This is the intended anti-duplication tradeoff, but consider disabling the form fields (or surfacing a note) oncecreatedTopicIdexists so the behavior isn't surprising.🤖 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 `@admin-web/components/dialogs/add-study-guide-dialog.tsx` around lines 87 - 147, When createdTopicId is set after successful topic creation, disable the study-guide form fields and clearly indicate that edits will not be applied during a retry of adding the topic to the path. Update the relevant form controls and submission UI around handleSubmit while preserving the existing anti-duplication retry behavior.admin-web/app/(dashboard)/promo-codes/create/page.tsx (1)
313-369: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider clearing stale eligibility fields when
eligible_forchanges.
eligible_tiers,eligible_user_ids, andeligibleUserIdsTextare not reset when the user switcheseligible_foraway fromspecific_tiers/specific_users. A user who enters IDs and then picksallwill still POST the populatedeligible_user_ids. Whether that matters depends on how/api/admin/create-promo-codeinterprets these fields.♻️ Optional: reset on type change
value={formData.eligible_for} - onChange={(e) => setFormData({ ...formData, eligible_for: e.target.value as EligibilityType })} + onChange={(e) => { + setEligibleUserIdsText('') + setFormData({ + ...formData, + eligible_for: e.target.value as EligibilityType, + eligible_tiers: [], + eligible_user_ids: [], + }) + }}🤖 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 `@admin-web/app/`(dashboard)/promo-codes/create/page.tsx around lines 313 - 369, Update the eligible_for change handler to clear stale eligibility state when switching modes: reset eligible_tiers when leaving specific_tiers, and reset both eligible_user_ids and eligibleUserIdsText when leaving specific_users. Preserve the current values while remaining in their corresponding mode, and ensure the submitted formData does not retain fields from a previous eligibility selection.admin-web/app/api/admin/gamification/achievements/route.ts (1)
65-97: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winO(N×M) nested filter now scans an unbounded dataset.
unlocks = userAchievements.filter(...)runs once per achievement over the entire (now up to 100k-row)userAchievementsarray. A single pass building a lookup map avoids the quadratic blowup.♻️ Suggested refactor
+ const unlocksByAchievement = new Map<string, { count: number; users: Set<string> }>() + ;(userAchievements || []).forEach(ua => { + const entry = unlocksByAchievement.get(ua.achievement_id) ?? { count: 0, users: new Set<string>() } + entry.count++ + entry.users.add(ua.user_id) + unlocksByAchievement.set(ua.achievement_id, entry) + }) + const achievementsWithStats = (achievements || []).map(achievement => { - const unlocks = (userAchievements || []).filter(ua => ua.achievement_id === achievement.id) + const stats = unlocksByAchievement.get(achievement.id) return { ...achievement, title: achievement.name_en, description: achievement.description_en, tier: getTierFromXP(achievement.xp_reward), type: achievement.category, requirement_value: achievement.threshold, - total_unlocks: unlocks.length, - unique_users: new Set(unlocks.map(ua => ua.user_id)).size, + total_unlocks: stats?.count || 0, + unique_users: stats?.users.size || 0, } })🤖 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 `@admin-web/app/api/admin/gamification/achievements/route.ts` around lines 65 - 97, Replace the per-achievement userAchievements.filter call in achievementsWithStats with a single-pass lookup keyed by achievement_id, storing unlock counts and unique user IDs. Use that lookup when mapping achievements to preserve total_unlocks and unique_users while avoiding repeated scans of the full dataset.admin-web/app/api/dashboard/stats/route.ts (1)
38-61: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider parallelizing the independent
fetchAllRowscalls.
llmCosts,previousLLMCosts,tokenUsage, andpreviousTokenUsagedon't depend on each other but are awaited sequentially; each may itself require several round trips.Promise.allwould cut overall latency.const [ { data: llmCosts }, { data: previousLLMCosts }, { data: tokenUsage }, { data: previousTokenUsage }, ] = await Promise.all([ fetchAllRows((from, to) => supabase.from('llm_api_costs').select('total_cost').gte('created_at', thirtyDaysAgo.toISOString()).order('created_at', { ascending: true }).range(from, to)), fetchAllRows((from, to) => supabase.from('llm_api_costs').select('total_cost').gte('created_at', sixtyDaysAgo.toISOString()).lt('created_at', thirtyDaysAgo.toISOString()).order('created_at', { ascending: true }).range(from, to)), fetchAllRows((from, to) => supabase.from('token_usage_history').select('token_cost').gte('created_at', thirtyDaysAgo.toISOString()).order('created_at', { ascending: true }).range(from, to)), fetchAllRows((from, to) => supabase.from('token_usage_history').select('token_cost').gte('created_at', sixtyDaysAgo.toISOString()).lt('created_at', thirtyDaysAgo.toISOString()).order('created_at', { ascending: true }).range(from, to)), ])Also applies to: 103-123
🤖 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 `@admin-web/app/api/dashboard/stats/route.ts` around lines 38 - 61, Parallelize the independent data retrieval in the dashboard stats handler by replacing the sequential fetchAllRows calls for llmCosts, previousLLMCosts, tokenUsage, and previousTokenUsage with one Promise.all, preserving each query’s existing date filters, selections, ordering, and range handling. Destructure the results in the same logical order and keep the downstream aggregation behavior unchanged.admin-web/app/api/admin/analytics/engagement/route.ts (1)
72-119: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider running the independent fetches in parallel.
dailyEvents,streaksData,completedTopicsCount, andenrollmentsDatadon't depend on each other but are awaited sequentially;Promise.allwould reduce overall latency.🤖 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 `@admin-web/app/api/admin/analytics/engagement/route.ts` around lines 72 - 119, Run the independent analytics queries for dailyEvents, streaksData, completedTopicsCount, and enrollmentsData concurrently with Promise.all, preserving each existing query and result mapping. Update the surrounding analytics flow so all four results are awaited together before calculating active users, streak averages, completed topics, and enrollment metrics.admin-web/app/api/admin/analytics/features/route.ts (1)
74-163: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider parallelizing the 7 independent
fetchAllRowscalls withPromise.all.None of these queries depend on each other's results, but they run strictly sequentially, each potentially needing multiple round trips.
🤖 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 `@admin-web/app/api/admin/analytics/features/route.ts` around lines 74 - 163, Parallelize the independent feature analytics queries by creating promises for all seven fetchAllRows calls and awaiting them together with Promise.all. Update the surrounding result handling to use the corresponding resolved values while preserving each query’s filters, ordering, pagination, and existing metrics calculations.admin-web/app/api/admin/token-usage-history/route.ts (1)
4-5: 🚀 Performance & Scalability | 🔵 Trivial
getAuthEmailMapfetches the entire auth user list on every request.Per its definition,
getAuthEmailMapcallslistAllAuthUsers(supabaseAdmin)(paginating through all auth users) before filtering down touserIdsin memory — it doesn't acceptuserIdsas a targeted lookup. Every call to this history endpoint therefore re-fetches every user account via the Admin API, which will scale poorly as the user base grows, even though this endpoint only needs emails for the users referenced in up to 500 rows.This is a shared-helper concern (not introduced by this diff alone — it's also used in
token-purchases/route.ts), but worth verifyinglistAllAuthUsers's pagination behavior/caching before this refactor is deployed broadly.Also applies to: 115-124
🤖 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 `@admin-web/app/api/admin/token-usage-history/route.ts` around lines 4 - 5, Replace the per-request getAuthEmailMap usage in the token-usage history handler with a targeted or cached email lookup for only the user IDs present in the fetched rows, and verify listAllAuthUsers pagination does not retrieve the full auth list unnecessarily. Apply the same optimization to the corresponding token-purchases usage if it shares this helper.admin-web/components/system-config/spaced-repetition-editor.tsx (1)
34-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicated edit-mode state machine across multiple editor components.
This exact
internalEditing/editing/handleEditStart/exitEditModepattern is repeated verbatim ingamification-editor.tsx,unlock-limits-editor.tsx,verse-quota-editor.tsx, and (per related graph context)practice-modes-editor.tsx/pricing-editor.tsx. Extracting this into a shared hook (e.g.useEditableSection(initialValue, { isEditing, onEditStart, onCancel })) would remove ~20 duplicated lines per file and keep future fixes in one place.🤖 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 `@admin-web/components/system-config/spaced-repetition-editor.tsx` around lines 34 - 55, Extract the duplicated edit-mode state machine from SpacedRepetitionEditor and the corresponding gamification, unlock-limits, verse-quota, practice-modes, and pricing editors into a shared useEditableSection hook. The hook should own internalEditing, derive editing from optional isEditing, reset initialValue and invoke onEditStart in handleEditStart, and clear internal state while invoking onCancel in exitEditMode; update each editor to use the shared hook without changing its external behavior.
🤖 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 `@admin-web/app/`(dashboard)/blogs/page.tsx:
- Around line 41-59: Update the blog post search flow around loadPosts and the
search effect so search is not silently limited to the currently loaded
paginated page. Prefer forwarding the search term through listBlogPosts and the
backend search parameter if supported; otherwise reset page to 1 when searching
and clearly indicate in the UI that filtering applies only to the current page.
Keep pagination, locale, and status filtering behavior intact.
In `@admin-web/app/`(dashboard)/iap-config/page.tsx:
- Around line 41-58: Update the queryFn in the IAP configuration useQuery to use
Promise.allSettled for the provider requests, preventing one failed provider
from discarding successful results. Aggregate fulfilled responses into configs
and surface a query error when partial results are not acceptable, preserving
the existing provider fetch behavior and error messaging.
In `@admin-web/app/`(dashboard)/learning-paths/export/page.tsx:
- Around line 61-75: Update the per-path callback in the Promise.all mapping to
catch fetch or response-parsing errors and return null for that path, matching
the existing non-OK response handling. Preserve successful results so paths that
fetch correctly still export and failedCount remains accurate.
In `@admin-web/app/`(dashboard)/llm-costs/page.tsx:
- Line 27: Memoize effectiveRange in the page component so
getDateRangePreset('7days') runs only when dateRange changes, preserving stable
from and to Date references across refetch-driven renders. Keep the queryFn’s
fresh date calculation unchanged so data windows continue updating while
DetailedLogsTable pagination is not reset unnecessarily.
In `@admin-web/app/`(dashboard)/topics/export/page.tsx:
- Around line 62-76: Wrap each topic fetch in the Promise.all mapping within the
topics export flow in try/catch, returning null for thrown network errors just
as for non-OK responses. Preserve successfully fetched topics so Promise.all
resolves and the existing filtering, failure count, and empty-result handling
continue to work.
In `@admin-web/app/`(dashboard)/topics/import/page.tsx:
- Around line 130-142: Update the import flow around bulkImportTopics and the
setImportResults error construction so server-reported errors use the original
CSV row numbers, matching clientErrors. Preserve the existing client-side row
numbering and either carry each source row index through the request or remap
result.errors using the filtered csvTopics metadata before rendering.
In `@admin-web/app/api/admin/analytics/engagement/route.ts`:
- Around line 72-79: Update the fetchAllRows calls for dailyEvents and
enrollmentsData to inspect and propagate their returned errors instead of
processing partial results. Make both pagination queries use a deterministic
ordering by adding a unique tie-breaker after created_at/enrolled_at, using the
appropriate row identifier, while preserving the existing ascending order.
In `@admin-web/app/api/admin/analytics/events/route.ts`:
- Around line 87-89: Update the events ordering or selection logic near the
query’s order('created_at') call and the events.slice(0, 100) usage so the
“latest 100 events” result contains the newest records. Preserve the intended
chronological output order while selecting the final 100 events from the
filtered range.
- Around line 76-89: Update the analytics events query in the fetchAllRows
callback to sort by created_at ascending with the unique id column as a
deterministic tiebreaker, preventing unstable range pagination for equal
timestamps. After fetching, ensure the response selects the newest 100 events
rather than the oldest 100 returned by ascending order, preserving the intended
event ordering.
In `@admin-web/app/api/admin/analytics/features/route.ts`:
- Around line 73-84: Merge the paginated user_study_guides fetches used by the
Study Guides and study modes analytics into one query, selecting both user_id
and study_mode while retaining the shared created_at filter, ordering, and range
pagination. Update countDistinctUsers, totalStudyGuides, and study-mode
calculations to reuse the single result, removing the duplicate fetch and its
separate studyModesData query.
In `@admin-web/app/api/admin/create-payment-record/route.ts`:
- Around line 48-62: Remove the fabricated token value from the manual payment
insert in the create-payment-record handler and update the purchase_history
schema or persistence model to represent manual subscription payments without
token-purchase data. Ensure token analytics exclude these records while
preserving the existing payment fields and completed status.
In `@admin-web/app/api/admin/gamification/achievements/route.ts`:
- Around line 66-72: Update the achievement-loading flow around fetchAllRows to
capture and handle its returned error before calculating total_unlocks or
unique_users. Propagate an appropriate error response to the caller when
pagination fails, while preserving the existing aggregation behavior for
successful results.
In `@admin-web/app/api/admin/gamification/streaks/route.ts`:
- Around line 115-116: Wrap the getAuthEmailMap call in the streaks endpoint
with try/catch so transient auth admin API failures do not fail the entire
request. On failure, log the error using the route’s existing logging pattern
and continue with an empty email map, preserving normal email enrichment when
the lookup succeeds.
In `@admin-web/app/api/admin/gamification/user-achievements/route.ts`:
- Around line 73-74: Wrap the getAuthEmailMap call in the user achievements
route with a local try/catch, matching the graceful-degradation behavior in the
other admin routes. On failure, log the error using the route’s existing logging
pattern and continue with an empty emailsMap so achievements still return with
“N/A” email values instead of triggering the outer 500 response.
In `@admin-web/app/api/admin/user-token-balances/route.ts`:
- Around line 72-86: The query in the user-token balance route must cap search
results before the per-row token_usage_history lookups and safely escape the
search value used in the raw query.or clause. Update the search branch around
the searchConditions construction and query limit logic to quote/escape search
according to PostgREST syntax rather than stripping characters, and apply the
intended result limit for both searched and unsearched queries.
In `@admin-web/app/api/dashboard/stats/route.ts`:
- Around line 38-61: Update both fetchAllRows calls in the dashboard stats
handler to capture and handle their returned errors, preventing totals from
being calculated from partial data. Add a unique secondary ordering key such as
id alongside created_at in each query, preserving ascending order for stable
pagination when timestamps collide.
In `@admin-web/lib/supabase/fetch-all-rows.ts`:
- Around line 18-25: Update the pagination loop in the fetch-all-rows function
to detect when MAX_PAGES is exhausted while every fetched page remains full, and
emit a warning or equivalent truncation signal before returning the capped rows.
Preserve the existing early-stop behavior for partial or empty pages and the
current error handling.
In `@admin-web/middleware.ts`:
- Around line 1-47: Update middleware’s Supabase cookie adapter to use the
current getAll and setAll callbacks instead of get, set, and remove. In setAll,
apply every cookie to the request and response while preserving the existing
response instance and request headers, so multiple cookie writes are not lost;
keep the auth session refresh and final response behavior unchanged.
In `@backend/supabase/functions/admin-update-subscription/index.ts`:
- Around line 149-153: Validate body.next_billing_at before calling toISOString
in the nextBillingAt resolution. Parse the provided value, verify the resulting
Date is valid, and raise the established AppError for invalid user input;
otherwise preserve the admin override and periodEnd fallback behavior without
allowing RangeError to escape.
---
Outside diff comments:
In `@admin-web/app/`(dashboard)/memory-verses/page.tsx:
- Around line 56-77: Update fetchVerses so it retrieves the complete
suggested-verses dataset instead of hard-capping the API request at limit=200.
Either paginate through all API pages or use the API’s supported
unbounded/all-rows option, then preserve the existing client-side category
filtering and full-dataset stats behavior.
In `@admin-web/app/`(dashboard)/subscriptions/page.tsx:
- Around line 15-60: The subscriptions pagination currently derives totalPages
from unfiltered search results while tier/status filtering only applies to the
current page, causing incomplete results and misleading stats. Update
SubscriptionsPage and the searchUsers query to pass filterTier and filterStatus
server-side, then compute totalResults and totalPages from the filtered response
while preserving the existing filteredUsers and stats behavior against the
returned page.
In `@admin-web/app/api/admin/analytics/features/route.ts`:
- Around line 74-163: The seven fetchAllRows calls in the analytics route must
propagate or handle their returned errors instead of continuing with partial
data; update each destructuring and follow the route’s existing error-response
pattern. Make every paginated query’s ordering deterministic by ordering first
on its current timestamp or user key and then on a unique stable identifier such
as id, including the study-modes query.
In `@admin-web/app/api/admin/create-payment-record/route.ts`:
- Line 38: Update the purchase_history insert in the create-payment-record route
to persist the validated body.subscription_id value in the corresponding
subscription association column. Keep the existing required-field validation and
ensure the value is included in both the insert columns and values.
In `@admin-web/components/tables/streaks-table.tsx`:
- Around line 22-33: Update getStudyStatus to parse lastStudyDate with the
existing parseLocalDate helper instead of constructing a Date directly,
preserving the current day-difference and status logic.
---
Nitpick comments:
In `@admin-web/app/`(dashboard)/blogs/page.tsx:
- Around line 61-72: Update loadStats to use a dedicated aggregate/count API, or
a single endpoint returning total, published, and draft counts, instead of
issuing three listBlogPosts requests with limit: 1. Preserve the existing
setStats mapping and error handling while reducing the operation to one request.
In `@admin-web/app/`(dashboard)/learning-paths/[id]/page.tsx:
- Around line 13-25: Extract the duplicated icon mapping from iconMap into a
shared exported constant, then update this page and the matching
learning-paths-table consumer to import and reuse it. Remove the local map while
preserving the existing icon-name-to-emoji values and lookup behavior.
In `@admin-web/app/`(dashboard)/promo-codes/create/page.tsx:
- Around line 313-369: Update the eligible_for change handler to clear stale
eligibility state when switching modes: reset eligible_tiers when leaving
specific_tiers, and reset both eligible_user_ids and eligibleUserIdsText when
leaving specific_users. Preserve the current values while remaining in their
corresponding mode, and ensure the submitted formData does not retain fields
from a previous eligibility selection.
In `@admin-web/app/api/admin/analytics/engagement/route.ts`:
- Around line 72-119: Run the independent analytics queries for dailyEvents,
streaksData, completedTopicsCount, and enrollmentsData concurrently with
Promise.all, preserving each existing query and result mapping. Update the
surrounding analytics flow so all four results are awaited together before
calculating active users, streak averages, completed topics, and enrollment
metrics.
In `@admin-web/app/api/admin/analytics/features/route.ts`:
- Around line 74-163: Parallelize the independent feature analytics queries by
creating promises for all seven fetchAllRows calls and awaiting them together
with Promise.all. Update the surrounding result handling to use the
corresponding resolved values while preserving each query’s filters, ordering,
pagination, and existing metrics calculations.
In `@admin-web/app/api/admin/gamification/achievements/route.ts`:
- Around line 65-97: Replace the per-achievement userAchievements.filter call in
achievementsWithStats with a single-pass lookup keyed by achievement_id, storing
unlock counts and unique user IDs. Use that lookup when mapping achievements to
preserve total_unlocks and unique_users while avoiding repeated scans of the
full dataset.
In `@admin-web/app/api/admin/path-topics/reorder/route.ts`:
- Around line 37-42: Validate the parsed body in the reorder route before
invoking the Edge Function, checking that learning_path_id is present and valid
and topic_orders has the expected structure. Return the route’s existing
client-error response for malformed input, and only pass validated data to
supabaseUser.functions.invoke in the admin-learning-path-topics reorder flow.
In `@admin-web/app/api/admin/token-usage-history/route.ts`:
- Around line 4-5: Replace the per-request getAuthEmailMap usage in the
token-usage history handler with a targeted or cached email lookup for only the
user IDs present in the fetched rows, and verify listAllAuthUsers pagination
does not retrieve the full auth list unnecessarily. Apply the same optimization
to the corresponding token-purchases usage if it shares this helper.
In `@admin-web/app/api/dashboard/stats/route.ts`:
- Around line 38-61: Parallelize the independent data retrieval in the dashboard
stats handler by replacing the sequential fetchAllRows calls for llmCosts,
previousLLMCosts, tokenUsage, and previousTokenUsage with one Promise.all,
preserving each query’s existing date filters, selections, ordering, and range
handling. Destructure the results in the same logical order and keep the
downstream aggregation behavior unchanged.
In `@admin-web/components/charts/cost-trend-chart.tsx`:
- Around line 25-28: Extract parseLocalDate into the shared date utility at
admin-web/lib/utils/date.ts, preserving the normalized date-only parsing via
slice(0, 10). Remove the duplicated local helpers from cost-trend-chart.tsx,
streaks-table.tsx, and daily-verses-table.tsx, and import the shared utility in
each component.
In `@admin-web/components/dialogs/add-study-guide-dialog.tsx`:
- Around line 87-147: When createdTopicId is set after successful topic
creation, disable the study-guide form fields and clearly indicate that edits
will not be applied during a retry of adding the topic to the path. Update the
relevant form controls and submission UI around handleSubmit while preserving
the existing anti-duplication retry behavior.
In `@admin-web/components/dialogs/edit-system-config-dialog.tsx`:
- Around line 59-118: Update handleSubmit and the Save-button flow in the edit
system config dialog to prevent onSave(formData) when any numeric configuration
field contains ''. Validate all affected token/day-count fields, keep Save
disabled or reject submission while any is empty, and preserve saving when every
field has a valid numeric value.
In `@admin-web/components/modals/subscription-price-update-modal.tsx`:
- Around line 46-54: Extract the duplicated getCurrencySymbol function from the
modal and pricing editor into a shared currency utility module, then import and
use that shared function in both components. Preserve the existing currency
mappings and fallback behavior.
In `@admin-web/components/system-config/spaced-repetition-editor.tsx`:
- Around line 34-55: Extract the duplicated edit-mode state machine from
SpacedRepetitionEditor and the corresponding gamification, unlock-limits,
verse-quota, practice-modes, and pricing editors into a shared
useEditableSection hook. The hook should own internalEditing, derive editing
from optional isEditing, reset initialValue and invoke onEditStart in
handleEditStart, and clear internal state while invoking onCancel in
exitEditMode; update each editor to use the shared hook without changing its
external behavior.
In `@admin-web/lib/supabase/list-all-users.ts`:
- Around line 31-42: Update getAuthEmailMap to use
supabaseAdmin.auth.admin.getUserById for each provided ID in parallel when the
ID set is below a defined threshold, mapping returned users by ID and preserving
empty emails. Fall back to listAllAuthUsers when userIds is omitted or exceeds
the threshold, and handle null users or API errors consistently with the
existing return contract. Confirm the getUserById response shape supported by
`@supabase/supabase-js` v2.95.2.
In `@admin-web/middleware.ts`:
- Around line 49-60: Update the exported config.matcher so admin API paths under
/api/admin are excluded from middleware matching, leaving authentication to each
route handler’s createClient/createAdminClient checks. Preserve middleware
matching for admin page routes and the existing static, image, favicon, and
public-file exclusions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9adf18d-4ba4-4faa-8ec2-fa1c5421e896
⛔ Files ignored due to path filters (1)
admin-web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (107)
admin-web/app/(dashboard)/admin-management/page.tsxadmin-web/app/(dashboard)/analytics/page.tsxadmin-web/app/(dashboard)/blogs/[id]/page.tsxadmin-web/app/(dashboard)/blogs/new/page.tsxadmin-web/app/(dashboard)/blogs/page.tsxadmin-web/app/(dashboard)/crons/page.tsxadmin-web/app/(dashboard)/gamification/page.tsxadmin-web/app/(dashboard)/iap-config/page.tsxadmin-web/app/(dashboard)/issues/[feedbackId]/page.tsxadmin-web/app/(dashboard)/issues/page.tsxadmin-web/app/(dashboard)/issues/purchase/[issueId]/page.tsxadmin-web/app/(dashboard)/learning-paths/[id]/edit/page.tsxadmin-web/app/(dashboard)/learning-paths/[id]/page.tsxadmin-web/app/(dashboard)/learning-paths/export/page.tsxadmin-web/app/(dashboard)/learning-paths/import/page.tsxadmin-web/app/(dashboard)/learning-paths/page.tsxadmin-web/app/(dashboard)/llm-costs/page.tsxadmin-web/app/(dashboard)/memory-verses/page.tsxadmin-web/app/(dashboard)/page.tsxadmin-web/app/(dashboard)/promo-codes/create/page.tsxadmin-web/app/(dashboard)/promo-codes/page.tsxadmin-web/app/(dashboard)/security/page.tsxadmin-web/app/(dashboard)/study-generator/page.tsxadmin-web/app/(dashboard)/subscriptions/[userId]/edit/page.tsxadmin-web/app/(dashboard)/subscriptions/[userId]/page.tsxadmin-web/app/(dashboard)/subscriptions/page.tsxadmin-web/app/(dashboard)/system-config/page.tsxadmin-web/app/(dashboard)/token-management/[userId]/page.tsxadmin-web/app/(dashboard)/token-management/sections/purchase-history-section.tsxadmin-web/app/(dashboard)/token-management/sections/usage-history-section.tsxadmin-web/app/(dashboard)/token-management/sections/user-balances-section.tsxadmin-web/app/(dashboard)/token-management/tabs/issues-tab.tsxadmin-web/app/(dashboard)/token-management/tabs/overview-tab.tsxadmin-web/app/(dashboard)/token-management/tabs/purchase-history-tab.tsxadmin-web/app/(dashboard)/token-management/tabs/usage-history-tab.tsxadmin-web/app/(dashboard)/token-management/tabs/user-balances-tab.tsxadmin-web/app/(dashboard)/topics/[id]/content/page.tsxadmin-web/app/(dashboard)/topics/[id]/edit/page.tsxadmin-web/app/(dashboard)/topics/create/page.tsxadmin-web/app/(dashboard)/topics/export/page.tsxadmin-web/app/(dashboard)/topics/import/page.tsxadmin-web/app/(dashboard)/topics/page.tsxadmin-web/app/api/admin/admin-logs/route.tsadmin-web/app/api/admin/analytics/engagement/route.tsadmin-web/app/api/admin/analytics/events/route.tsadmin-web/app/api/admin/analytics/features/route.tsadmin-web/app/api/admin/create-payment-record/route.tsadmin-web/app/api/admin/feedback/route.tsadmin-web/app/api/admin/gamification/achievements/route.tsadmin-web/app/api/admin/gamification/streaks/route.tsadmin-web/app/api/admin/gamification/user-achievements/route.tsadmin-web/app/api/admin/iap/config/route.tsadmin-web/app/api/admin/iap/products/route.tsadmin-web/app/api/admin/iap/verify-receipt/route.tsadmin-web/app/api/admin/path-topics/[pathId]/[topicId]/milestone/route.tsadmin-web/app/api/admin/path-topics/reorder/route.tsadmin-web/app/api/admin/path-topics/route.tsadmin-web/app/api/admin/purchase-issues/route.tsadmin-web/app/api/admin/search-users/route.tsadmin-web/app/api/admin/security-events/route.tsadmin-web/app/api/admin/token-purchases/route.tsadmin-web/app/api/admin/token-usage-history/route.tsadmin-web/app/api/admin/topics/bulk-import/route.tsadmin-web/app/api/admin/update-purchase-issue/route.tsadmin-web/app/api/admin/update-subscription/route.tsadmin-web/app/api/admin/user-token-balances/route.tsadmin-web/app/api/dashboard/stats/route.tsadmin-web/app/auth/callback/route.tsadmin-web/app/layout.tsxadmin-web/app/test-console/page.tsxadmin-web/components/charts/cost-trend-chart.tsxadmin-web/components/dialogs/add-study-guide-dialog.tsxadmin-web/components/dialogs/create-learning-path-dialog.tsxadmin-web/components/dialogs/edit-learning-path-dialog.tsxadmin-web/components/dialogs/edit-subscription-config-dialog.tsxadmin-web/components/dialogs/edit-system-config-dialog.tsxadmin-web/components/modals/subscription-price-update-modal.tsxadmin-web/components/sidebar.tsxadmin-web/components/study-generator/content-editor.tsxadmin-web/components/system-config/gamification-editor.tsxadmin-web/components/system-config/practice-modes-editor.tsxadmin-web/components/system-config/pricing-editor.tsxadmin-web/components/system-config/spaced-repetition-editor.tsxadmin-web/components/system-config/unlock-limits-editor.tsxadmin-web/components/system-config/verse-quota-editor.tsxadmin-web/components/tables/achievements-table.tsxadmin-web/components/tables/daily-verses-table.tsxadmin-web/components/tables/learning-paths-table.tsxadmin-web/components/tables/streaks-table.tsxadmin-web/components/tables/top-heavy-users-table.tsxadmin-web/components/theme-provider.tsxadmin-web/components/ui/create-promo-code-dialog.tsxadmin-web/components/ui/date-range-picker.tsxadmin-web/components/ui/translation-editor.tsxadmin-web/components/ui/user-search-input.tsxadmin-web/lib/api/admin.tsadmin-web/lib/supabase/fetch-all-rows.tsadmin-web/lib/supabase/list-all-users.tsadmin-web/middleware.tsadmin-web/package.jsonadmin-web/tailwind.config.tsbackend/supabase/functions/admin-update-subscription/index.tsdistribution/whatsnew/whatsnew-en-INdistribution/whatsnew/whatsnew-en-USdistribution/whatsnew/whatsnew-hi-INdistribution/whatsnew/whatsnew-ml-INfrontend/pubspec.yaml
💤 Files with no reviewable changes (10)
- admin-web/app/(dashboard)/token-management/sections/user-balances-section.tsx
- admin-web/app/(dashboard)/token-management/tabs/overview-tab.tsx
- admin-web/app/test-console/page.tsx
- admin-web/app/(dashboard)/token-management/tabs/purchase-history-tab.tsx
- admin-web/app/(dashboard)/token-management/sections/usage-history-section.tsx
- admin-web/app/(dashboard)/token-management/tabs/user-balances-tab.tsx
- admin-web/app/(dashboard)/token-management/sections/purchase-history-section.tsx
- admin-web/app/(dashboard)/gamification/page.tsx
- admin-web/app/(dashboard)/token-management/tabs/usage-history-tab.tsx
- admin-web/app/(dashboard)/token-management/tabs/issues-tab.tsx
| const loadPosts = useCallback(async () => { | ||
| setIsLoading(true) | ||
| setError(null) | ||
| try { | ||
| const data = await listBlogPosts({ limit: 100 }) | ||
| const data = await listBlogPosts({ | ||
| page, | ||
| limit: PAGE_SIZE, | ||
| locale: localeFilter !== 'all' ? localeFilter : undefined, | ||
| status: statusFilter !== 'all' ? statusFilter : undefined, | ||
| }) | ||
| setPosts(data.posts) | ||
| setTotal(data.total) | ||
| } catch (err) { | ||
| console.error(err) | ||
| setError('Failed to load blog posts.') | ||
| } finally { | ||
| setIsLoading(false) | ||
| } | ||
| }, [page, localeFilter, statusFilter]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Search is now scoped only to the currently loaded page, not all posts.
listBlogPosts supports locale/status/page/limit but no search parameter, and loadPosts now fetches a paginated 25-row server page. The search effect filters posts, which is just that single page — so typing a search term will silently miss matches on other pages, with no UI indication that search is page-scoped. This is a functional regression from a previous (unpaginated) full-list search.
Consider either passing the search term to a server-side search parameter (if the backend route supports it) or resetting to page 1 and messaging that search only applies within the current page's results.
Also applies to: 80-92
🤖 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 `@admin-web/app/`(dashboard)/blogs/page.tsx around lines 41 - 59, Update the
blog post search flow around loadPosts and the search effect so search is not
silently limited to the currently loaded paginated page. Prefer forwarding the
search term through listBlogPosts and the backend search parameter if supported;
otherwise reset page to 1 when searching and clearly indicate in the UI that
filtering applies only to the current page. Keep pagination, locale, and status
filtering behavior intact.
| // Fetch IAP Configuration (the API requires a provider param, so fetch both) | ||
| const { data: iapConfigData, isLoading: configLoading } = useQuery({ | ||
| queryKey: ['iap-config', activeEnvironment], | ||
| queryFn: async () => { | ||
| const res = await fetch( | ||
| `/api/admin/iap/config?environment=${activeEnvironment}`, | ||
| { credentials: 'include' } | ||
| const providers: Provider[] = ['google_play', 'apple_appstore'] | ||
| const results = await Promise.all( | ||
| providers.map(async (provider) => { | ||
| const res = await fetch( | ||
| `/api/admin/iap/config?provider=${provider}&environment=${activeEnvironment}`, | ||
| { credentials: 'include' } | ||
| ) | ||
| if (!res.ok) throw new Error('Failed to fetch IAP config') | ||
| return res.json() | ||
| }) | ||
| ) | ||
| if (!res.ok) throw new Error('Failed to fetch IAP config') | ||
| return res.json() | ||
| return { configs: results.flatMap((r) => r.configs || []) } | ||
| }, | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -B2 -A10 'new QueryClient\(' "admin-web/app/layout.tsx" 2>/dev/null
fd -t f . admin-web/app | xargs rg -l 'QueryClientProvider|defaultOptions' 2>/dev/nullRepository: fennsaji/disciplefy
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target file and the query provider setup.
git ls-files 'admin-web/app/layout.tsx' 'admin-web/app/(dashboard)/iap-config/page.tsx' 'admin-web/**/query*' 'admin-web/**/QueryClient*' 'admin-web/**/providers*' | sed -n '1,200p'
echo '--- layout.tsx ---'
if [ -f 'admin-web/app/layout.tsx' ]; then
cat -n 'admin-web/app/layout.tsx' | sed -n '1,260p'
fi
echo '--- iap-config/page.tsx outline ---'
ast-grep outline 'admin-web/app/(dashboard)/iap-config/page.tsx' --view expanded
echo '--- relevant query options / error handling ---'
rg -n -A6 -B6 'useQuery\(|QueryClientProvider|new QueryClient\(|defaultOptions|staleTime|refetchOnWindowFocus|isError|error' admin-web/app admin-web/lib admin-web/components 2>/dev/nullRepository: fennsaji/disciplefy
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- files ---'
git ls-files 'admin-web/app/layout.tsx' 'admin-web/app/(dashboard)/iap-config/page.tsx' 'admin-web/**/*provider*' 'admin-web/**/*Provider*' 'admin-web/**/*query*' 'admin-web/**/*QueryClient*' | sed -n '1,200p'
echo '--- admin-web/app/layout.tsx ---'
if [ -f 'admin-web/app/layout.tsx' ]; then
sed -n '1,240p' 'admin-web/app/layout.tsx'
fi
echo '--- admin-web/app/(dashboard)/iap-config/page.tsx ---'
if [ -f 'admin-web/app/(dashboard)/iap-config/page.tsx' ]; then
sed -n '1,220p' 'admin-web/app/(dashboard)/iap-config/page.tsx'
fi
echo '--- QueryClient-related matches ---'
rg -n -A4 -B4 'new QueryClient\(|QueryClientProvider|defaultOptions|staleTime|refetchOnWindowFocus|useQuery\(' admin-web/app admin-web/components admin-web/lib 2>/dev/null | sed -n '1,220p'Repository: fennsaji/disciplefy
Length of output: 26531
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- admin-web/lib/providers/query-provider.tsx ---'
sed -n '1,220p' 'admin-web/lib/providers/query-provider.tsx'
echo '--- any explicit React Query defaults in admin-web/lib or admin-web/app ---'
rg -n -A6 -B6 'staleTime|refetchOnWindowFocus|defaultOptions|QueryClient\(' admin-web/lib admin-web/app 2>/dev/null | sed -n '1,220p'Repository: fennsaji/disciplefy
Length of output: 17152
Use Promise.allSettled for the IAP config fetch. A single provider error currently rejects the whole query, leaving both credential panels empty; surface a query error state if partial results aren’t acceptable.
🤖 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 `@admin-web/app/`(dashboard)/iap-config/page.tsx around lines 41 - 58, Update
the queryFn in the IAP configuration useQuery to use Promise.allSettled for the
provider requests, preventing one failed provider from discarding successful
results. Aggregate fulfilled responses into configs and surface a query error
when partial results are not acceptable, preserving the existing provider fetch
behavior and error messaging.
Source: Coding guidelines
| const results = await Promise.all( | ||
| Array.from(selectedPaths).map(async (pathId) => { | ||
| const response = await fetch(`/api/admin/learning-paths/${pathId}`) | ||
| if (!response.ok) return null | ||
| const data = await response.json() | ||
| return data.learning_path | ||
| return data.learning_path || null | ||
| }) | ||
| ) | ||
| const pathsToExport = results.filter((p) => p !== null) | ||
| const failedCount = results.length - pathsToExport.length | ||
|
|
||
| if (pathsToExport.length === 0) { | ||
| toast.error('Failed to fetch the selected learning paths. Please try again.') | ||
| return | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Network errors inside Promise.all bypass the partial-failure handling.
Non-OK responses become null and are excluded gracefully, but if fetch itself throws (e.g., network drop) for any single path, the whole Promise.all rejects and the entire export is aborted via the generic catch block — even for paths that already fetched successfully. Wrap each fetch in try/catch to normalize to null consistently.
🔧 Proposed fix
const results = await Promise.all(
Array.from(selectedPaths).map(async (pathId) => {
- const response = await fetch(`/api/admin/learning-paths/${pathId}`)
- if (!response.ok) return null
- const data = await response.json()
- return data.learning_path || null
+ try {
+ const response = await fetch(`/api/admin/learning-paths/${pathId}`)
+ if (!response.ok) return null
+ const data = await response.json()
+ return data.learning_path || null
+ } catch {
+ return null
+ }
})
)📝 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.
| const results = await Promise.all( | |
| Array.from(selectedPaths).map(async (pathId) => { | |
| const response = await fetch(`/api/admin/learning-paths/${pathId}`) | |
| if (!response.ok) return null | |
| const data = await response.json() | |
| return data.learning_path | |
| return data.learning_path || null | |
| }) | |
| ) | |
| const pathsToExport = results.filter((p) => p !== null) | |
| const failedCount = results.length - pathsToExport.length | |
| if (pathsToExport.length === 0) { | |
| toast.error('Failed to fetch the selected learning paths. Please try again.') | |
| return | |
| } | |
| const results = await Promise.all( | |
| Array.from(selectedPaths).map(async (pathId) => { | |
| try { | |
| const response = await fetch(`/api/admin/learning-paths/${pathId}`) | |
| if (!response.ok) return null | |
| const data = await response.json() | |
| return data.learning_path || null | |
| } catch { | |
| return null | |
| } | |
| }) | |
| ) | |
| const pathsToExport = results.filter((p) => p !== null) | |
| const failedCount = results.length - pathsToExport.length | |
| if (pathsToExport.length === 0) { | |
| toast.error('Failed to fetch the selected learning paths. Please try again.') | |
| return | |
| } |
🤖 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 `@admin-web/app/`(dashboard)/learning-paths/export/page.tsx around lines 61 -
75, Update the per-path callback in the Promise.all mapping to catch fetch or
response-parsing errors and return null for that path, matching the existing
non-OK response handling. Preserve successful results so paths that fetch
correctly still export and failedCount remains accurate.
| // left open past midnight doesn't keep serving a stale date window. | ||
| const [dateRange, setDateRange] = useState<DateRange | null>(null) | ||
|
|
||
| const effectiveRange = dateRange ?? getDateRangePreset('7days') |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Memoize effectiveRange to prevent DetailedLogsTable pagination reset.
When dateRange is null, getDateRangePreset('7days') creates new Date objects on every render. DetailedLogsTable's useEffect depends on dateRange.from and dateRange.to as Date objects — React compares these with Object.is, so new object references trigger the effect even when the time values are identical. With refetchInterval: 60000, the parent re-renders every minute, causing DetailedLogsTable to reset to page 1 each time.
🔒 Proposed fix with useMemo
-import { useState } from 'react'
+import { useState, useMemo } from 'react'- const effectiveRange = dateRange ?? getDateRangePreset('7days')
+ const effectiveRange = useMemo(
+ () => dateRange ?? getDateRangePreset('7days'),
+ [dateRange]
+ )This keeps effectiveRange referentially stable when dateRange is null. The parent's queryFn still recomputes fresh dates on each refetch, so the dashboard won't serve stale windows. DetailedLogsTable will use the mount-time preset, but its day-level boundaries only drift past midnight — a minor tradeoff compared to the pagination reset bug.
📝 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.
| const effectiveRange = dateRange ?? getDateRangePreset('7days') | |
| import { useState, useMemo } from 'react' | |
| const effectiveRange = useMemo( | |
| () => dateRange ?? getDateRangePreset('7days'), | |
| [dateRange] | |
| ) |
🤖 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 `@admin-web/app/`(dashboard)/llm-costs/page.tsx at line 27, Memoize
effectiveRange in the page component so getDateRangePreset('7days') runs only
when dateRange changes, preserving stable from and to Date references across
refetch-driven renders. Keep the queryFn’s fresh date calculation unchanged so
data windows continue updating while DetailedLogsTable pagination is not reset
unnecessarily.
| const results = await Promise.all( | ||
| Array.from(selectedTopics).map(async (topicId) => { | ||
| const response = await fetch(`/api/admin/topics/${topicId}`) | ||
| if (!response.ok) return null | ||
| const data = await response.json() | ||
| return data.topic | ||
| return data.topic || null | ||
| }) | ||
| ) | ||
| const topicsToExport = results.filter((t) => t !== null) | ||
| const failedCount = results.length - topicsToExport.length | ||
|
|
||
| if (topicsToExport.length === 0) { | ||
| toast.error('Failed to fetch the selected topics. Please try again.') | ||
| return | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Same network-error gap as the learning-paths export page.
If fetch throws (not just returns non-OK) for any selected topic, Promise.all rejects entirely and the whole export aborts, discarding topics that already fetched successfully. Apply the same try/catch normalization as suggested for admin-web/app/(dashboard)/learning-paths/export/page.tsx.
🤖 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 `@admin-web/app/`(dashboard)/topics/export/page.tsx around lines 62 - 76, Wrap
each topic fetch in the Promise.all mapping within the topics export flow in
try/catch, returning null for thrown network errors just as for non-OK
responses. Preserve successfully fetched topics so Promise.all resolves and the
existing filtering, failure count, and empty-result handling continue to work.
| // Apply search filter in the query (email-matched user IDs or identifier substring) | ||
| if (search) { | ||
| const searchConditions = [`identifier.ilike.%${search}%`] | ||
| if (emailMatchedIds.length > 0) { | ||
| searchConditions.push(`identifier.in.(${emailMatchedIds.join(',')})`) | ||
| } | ||
| query = query.or(searchConditions.join(',')) | ||
| } | ||
|
|
||
| // Execute query with ordering; limit only applies to the unfiltered browse case | ||
| query = query.order('updated_at', { ascending: false }) | ||
| if (!search) { | ||
| query = query.limit(100) | ||
| } | ||
| const { data: tokenBalances, error: balancesError } = await query |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n "user-token-balances|searchConditions|identifier\.ilike|query\.or|emailMatchedIds|token_usage_history" admin-web/app/api/admin/user-token-balances/route.tsRepository: fennsaji/disciplefy
Length of output: 592
🏁 Script executed:
sed -n '1,220p' admin-web/app/api/admin/user-token-balances/route.tsRepository: fennsaji/disciplefy
Length of output: 5922
🏁 Script executed:
python3 - <<'PY'
import json, re, pathlib
p = pathlib.Path('admin-web/package.json')
if p.exists():
data = json.loads(p.read_text())
print(data.get('dependencies', {}).get('`@supabase/supabase-js`') or data.get('devDependencies', {}).get('`@supabase/supabase-js`'))
PYRepository: fennsaji/disciplefy
Length of output: 165
🌐 Web query:
supabase-js v2.95.2 PostgREST .or filter URL encoding commas injection
💡 Result:
In the context of supabase-js and the underlying PostgREST API, there is no "comma injection" vulnerability in the traditional sense, but there is a well-documented challenge regarding reserved character handling in the .or() filter [1][2][3]. The PostgREST URL grammar treats characters like commas (,), parentheses (()), and colons (:) as reserved [1][3]. When these characters appear within data values used in an .or() query, they can cause the API to incorrectly parse the filter, potentially leading to unintended query logic [1][3]. To prevent parsing errors, PostgREST requires that any filter value containing reserved characters be surrounded by double quotes, which are then percent-encoded as %22 in the final URL [1][3]. For example, if you need to filter a column for a value containing a comma, such as "Doe, John", the value must be encoded as "%22Doe, John%22" [1][3]. Key technical considerations: 1. Manual Sanitization: Because the .or() method in supabase-js accepts raw PostgREST filter strings, the library does not automatically escape these characters for you [4]. Developers are responsible for ensuring that any dynamic data inserted into an .or() string is properly quoted and escaped [4]. 2. Escaping Reserved Characters: If the data itself contains a double quote (") or a backslash (\), these must be escaped with a backslash (e.g., \" and \\) before the entire value is wrapped in double quotes [1][3]. 3. Risk: Failure to properly sanitize inputs used in an .or() query can lead to "broken" queries where the filter is misinterpreted by the server [5][6]. While this is a data-parsing issue rather than a code-execution injection, it can be exploited to bypass filters or cause unexpected behavior if input is sourced from untrusted users. To handle this securely, it is best practice to sanitize input strings by checking for reserved characters and applying the necessary double-quoting and escaping before passing them to the .or() function [4][3].
Citations:
- 1: https://github.com/postgrest/postgrest/blob/v12/docs/references/api/url_grammar.rst
- 2: https://stackoverflow.com/questions/76065534/how-to-escape-string-characters-in-supabase-js-or-query
- 3: https://docs.postgrest.org/en/stable/references/api/url_grammar.html
- 4: https://supabase.github.io/postgrest-js/v2/classes/PostgrestFilterBuilder.html
- 5: Handle Postgrest special characters supabase/postgrest-js#164
- 6: fix(postgrest): escape embedded quotes and backslashes in in()/notIn() filter values supabase/supabase-js#2489
Cap search results and escape the raw .or() clause
- Broad searches still fan out into the per-row
token_usage_historylookups, so apply a cap in the search path too. searchis still interpolated into raw PostgREST syntax; deleting[, .()]is not enough. Quote/escape the value for.or()instead of stripping characters.
🤖 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 `@admin-web/app/api/admin/user-token-balances/route.ts` around lines 72 - 86,
The query in the user-token balance route must cap search results before the
per-row token_usage_history lookups and safely escape the search value used in
the raw query.or clause. Update the search branch around the searchConditions
construction and query limit logic to quote/escape search according to PostgREST
syntax rather than stripping characters, and apply the intended result limit for
both searched and unsearched queries.
| const { data: llmCosts } = await fetchAllRows((from, to) => | ||
| supabase | ||
| .from('llm_api_costs') | ||
| .select('total_cost') | ||
| .gte('created_at', thirtyDaysAgo.toISOString()) | ||
| .order('created_at', { ascending: true }) | ||
| .range(from, to) | ||
| ) | ||
|
|
||
| const totalLLMCost = llmCosts?.reduce((sum, record) => sum + (record.total_cost || 0), 0) || 0 | ||
|
|
||
| // Get LLM costs for previous 30 days for comparison | ||
| const sixtyDaysAgo = new Date() | ||
| sixtyDaysAgo.setDate(sixtyDaysAgo.getDate() - 60) | ||
|
|
||
| const { data: previousLLMCosts } = await supabase | ||
| .from('llm_api_costs') | ||
| .select('total_cost') | ||
| .gte('created_at', sixtyDaysAgo.toISOString()) | ||
| .lt('created_at', thirtyDaysAgo.toISOString()) | ||
| const { data: previousLLMCosts } = await fetchAllRows((from, to) => | ||
| supabase | ||
| .from('llm_api_costs') | ||
| .select('total_cost') | ||
| .gte('created_at', sixtyDaysAgo.toISOString()) | ||
| .lt('created_at', thirtyDaysAgo.toISOString()) | ||
| .order('created_at', { ascending: true }) | ||
| .range(from, to) | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the dashboard stats route and the pagination helper.
git ls-files 'admin-web/app/api/dashboard/stats/route.ts' 'admin-web/**/*' | sed -n '1,120p'
echo
echo '--- fetchAllRows references ---'
rg -n "fetchAllRows" admin-web -S
echo
echo '--- route file (relevant section) ---'
sed -n '1,220p' admin-web/app/api/dashboard/stats/route.ts
echo
echo '--- helper definition(s) ---'
rg -n "function fetchAllRows|const fetchAllRows|export .*fetchAllRows" admin-web -SRepository: fennsaji/disciplefy
Length of output: 14560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' admin-web/lib/supabase/fetch-all-rows.ts
echo
echo '--- all current fetchAllRows call sites that check error ---'
rg -n "fetchAllRows\\(.*error|error: .*Error" admin-web/app/api admin-web/lib -SRepository: fennsaji/disciplefy
Length of output: 23507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for schema/type references and existing query patterns for the two tables.
rg -n "llm_api_costs|token_usage_history" admin-web -S
echo
echo '--- likely table definitions / generated types ---'
fd -a 'schema|types|database|supabase' admin-web | sed -n '1,120p'Repository: fennsaji/disciplefy
Length of output: 1047
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect generated DB types for both tables.
rg -n "llm_api_costs|token_usage_history" admin-web/types admin-web/lib/supabase -S
echo
echo '--- database type file candidates ---'
fd -a 'types.*\.ts$|database.*\.ts$|supabase.*\.ts$' admin-web/types admin-web/lib/supabase | sed -n '1,120p'Repository: fennsaji/disciplefy
Length of output: 157
Handle fetchAllRows errors and add a stable tie-breaker to the sort. fetchAllRows returns { data, error }, so ignoring error can turn a pagination failure into a partial, plausible-looking total. created_at also needs a unique secondary order (for example id) so page boundaries stay stable when timestamps collide.
🤖 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 `@admin-web/app/api/dashboard/stats/route.ts` around lines 38 - 61, Update both
fetchAllRows calls in the dashboard stats handler to capture and handle their
returned errors, preventing totals from being calculated from partial data. Add
a unique secondary ordering key such as id alongside created_at in each query,
preserving ascending order for stable pagination when timestamps collide.
| const rows: T[] = [] | ||
| for (let page = 0; page < MAX_PAGES; page++) { | ||
| const from = page * PAGE_SIZE | ||
| const { data, error } = await buildQuery(from, from + PAGE_SIZE - 1) | ||
| if (error) return { data: rows, error } | ||
| rows.push(...(data || [])) | ||
| if (!data || data.length < PAGE_SIZE) break | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Silent truncation past the 100k-row safety cap.
If a query has more than MAX_PAGES * PAGE_SIZE (100k) rows, the loop just breaks with no warning, and callers get truncated data indistinguishable from a complete result. Several callers sum this into cost/token dashboards, so results could silently under-report once volumes grow.
🩹 Suggested fix: warn (or flag) when the cap is hit
rows.push(...(data || []))
if (!data || data.length < PAGE_SIZE) break
+ if (page === MAX_PAGES - 1) {
+ console.warn(`fetchAllRows: hit MAX_PAGES cap (${MAX_PAGES * PAGE_SIZE} rows); results may be truncated`)
+ }
}📝 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.
| const rows: T[] = [] | |
| for (let page = 0; page < MAX_PAGES; page++) { | |
| const from = page * PAGE_SIZE | |
| const { data, error } = await buildQuery(from, from + PAGE_SIZE - 1) | |
| if (error) return { data: rows, error } | |
| rows.push(...(data || [])) | |
| if (!data || data.length < PAGE_SIZE) break | |
| } | |
| const rows: T[] = [] | |
| for (let page = 0; page < MAX_PAGES; page++) { | |
| const from = page * PAGE_SIZE | |
| const { data, error } = await buildQuery(from, from + PAGE_SIZE - 1) | |
| if (error) return { data: rows, error } | |
| rows.push(...(data || [])) | |
| if (!data || data.length < PAGE_SIZE) break | |
| if (page === MAX_PAGES - 1) { | |
| console.warn(`fetchAllRows: hit MAX_PAGES cap (${MAX_PAGES * PAGE_SIZE} rows); results may be truncated`) | |
| } | |
| } |
🤖 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 `@admin-web/lib/supabase/fetch-all-rows.ts` around lines 18 - 25, Update the
pagination loop in the fetch-all-rows function to detect when MAX_PAGES is
exhausted while every fetched page remains full, and emit a warning or
equivalent truncation signal before returning the capped rows. Preserve the
existing early-stop behavior for partial or empty pages and the current error
handling.
| import { createServerClient, type CookieOptions } from '@supabase/ssr' | ||
| import { NextResponse, type NextRequest } from 'next/server' | ||
|
|
||
| export async function middleware(request: NextRequest) { | ||
| let response = NextResponse.next({ | ||
| request: { | ||
| headers: request.headers, | ||
| }, | ||
| }) | ||
|
|
||
| const supabase = createServerClient( | ||
| process.env.NEXT_PUBLIC_SUPABASE_URL!, | ||
| process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, | ||
| { | ||
| cookies: { | ||
| get(name: string) { | ||
| return request.cookies.get(name)?.value | ||
| }, | ||
| set(name: string, value: string, options: CookieOptions) { | ||
| request.cookies.set({ name, value, ...options }) | ||
| response = NextResponse.next({ | ||
| request: { | ||
| headers: request.headers, | ||
| }, | ||
| }) | ||
| response.cookies.set({ name, value, ...options }) | ||
| }, | ||
| remove(name: string, options: CookieOptions) { | ||
| request.cookies.set({ name, value: '', ...options }) | ||
| response = NextResponse.next({ | ||
| request: { | ||
| headers: request.headers, | ||
| }, | ||
| }) | ||
| response.cookies.set({ name, value: '', ...options }) | ||
| }, | ||
| }, | ||
| } | ||
| ) | ||
|
|
||
| // Refresh the auth session so rotated tokens are persisted back to cookies. | ||
| // Without this, expired access tokens are never refreshed in Server | ||
| // Components (which cannot write cookies), causing random admin logouts. | ||
| await supabase.auth.getUser() | ||
|
|
||
| return response | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package manifests =="
git ls-files 'admin-web/package.json' 'package.json' 'pnpm-lock.yaml' 'package-lock.json' 'yarn.lock' | cat
echo
echo "== middleware file =="
cat -n admin-web/middleware.ts
echo
echo "== search for createServerClient usage =="
rg -n "createServerClient\(" admin-web -g '!**/node_modules/**' || true
echo
echo "== search for getAll/setAll cookie API =="
rg -n "getAll\(|setAll\(" admin-web node_modules . -g '!**/node_modules/**' || true
echo
echo "== package.json snippets =="
for f in package.json admin-web/package.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat "$f" | sed -n '1,220p'
fi
doneRepository: fennsaji/disciplefy
Length of output: 5146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== auth callback cookie handling =="
cat -n admin-web/app/auth/callback/route.ts | sed -n '210,280p'
echo
echo "== Supabase SSR package metadata if present in lockfile =="
python3 - <<'PY'
from pathlib import Path
import re
for path in [Path("package-lock.json"), Path("admin-web/package-lock.json")]:
if not path.exists():
continue
txt = path.read_text(errors="ignore")
print(f"--- {path} ---")
for key in ["`@supabase/ssr`", "`@supabase/supabase-js`"]:
idx = txt.find(key)
if idx != -1:
snippet = txt[max(0, idx-300):idx+1200]
print(snippet[:1500])
print()
PYRepository: fennsaji/disciplefy
Length of output: 5808
🌐 Web query:
Supabase SSR middleware cookies getAll setAll deprecated get set remove official guidance 0.8.0
💡 Result:
In the @supabase/ssr library, the cookie methods get, set, and remove are officially deprecated and will be removed in future major versions [1][2]. The official guidance is to use getAll and setAll instead [1][2]. This transition is required because the older methods (get, set, remove) are difficult to implement correctly and do not adequately handle important edge cases, such as reliable session management and proper cache header application [1][2][3]. Failing to implement getAll and setAll, or continuing to use the deprecated methods, can lead to significant authentication issues, including random logouts, early session termination, and inconsistent state [1][2]. The getAll and setAll methods are designed to be more robust: - getAll: Retrieves all cookies available to the client [1][4]. - setAll: Receives an array of cookies to set and a headers object (containing essential cache headers like Cache-Control) to apply to the HTTP response [3][4]. Applying these cache headers is critical to prevent CDNs from caching responses and leaking session data to other users [3][5][4]. If your environment does not allow setting response headers (e.g., in some server components), you may omit setAll, but it is strongly recommended to configure it wherever the capability exists [1]. For most standard SSR implementations, providing both getAll and setAll is the required configuration [1][2].
Citations:
- 1: https://github.com/supabase/ssr/blob/a1b60ba8/src/createServerClient.ts
- 2: https://github.com/supabase/ssr/blob/main/src/createBrowserClient.ts
- 3: https://supabase.com/docs/guides/auth/server-side/creating-a-client?environment=route-handler
- 4: https://supabase.com/docs/guides/auth/server-side/creating-a-client?environment=middleware
- 5: https://supabase.com/docs/guides/auth/server-side/advanced-guide
Migrate Supabase middleware to getAll/setAll
get/set/remove are deprecated in @supabase/ssr, and recreating response inside each callback can drop earlier cookie writes when Supabase sets multiple cookies in one call. Switch this middleware to the current getAll/setAll API.
🤖 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 `@admin-web/middleware.ts` around lines 1 - 47, Update middleware’s Supabase
cookie adapter to use the current getAll and setAll callbacks instead of get,
set, and remove. In setAll, apply every cookie to the request and response while
preserving the existing response instance and request headers, so multiple
cookie writes are not lost; keep the auth session refresh and final response
behavior unchanged.
| // Resolve next billing date — honour admin override if provided, else next renewal | ||
| const nextBillingAt = body.next_billing_at | ||
| ? new Date(body.next_billing_at).toISOString() | ||
| : periodEnd.toISOString() | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate body.next_billing_at before parsing to prevent unhandled RangeError.
new Date(body.next_billing_at).toISOString() throws RangeError: Invalid time value if the string is not a parseable date. While the admin panel uses a <input type="date">, the Edge Function should still validate per coding guidelines (input validation for all user data, AppError for all error handling).
🛡️ Proposed fix to validate the date before parsing
const nextBillingAt = body.next_billing_at
- ? new Date(body.next_billing_at).toISOString()
+ ? (() => {
+ const parsed = new Date(body.next_billing_at)
+ if (isNaN(parsed.getTime())) {
+ throw new AppError('Invalid next_billing_at date', 'VALIDATION_ERROR', 400)
+ }
+ return parsed.toISOString()
+ })()
: periodEnd.toISOString()📝 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.
| // Resolve next billing date — honour admin override if provided, else next renewal | |
| const nextBillingAt = body.next_billing_at | |
| ? new Date(body.next_billing_at).toISOString() | |
| : periodEnd.toISOString() | |
| // Resolve next billing date — honour admin override if provided, else next renewal | |
| const nextBillingAt = body.next_billing_at | |
| ? (() => { | |
| const parsed = new Date(body.next_billing_at) | |
| if (isNaN(parsed.getTime())) { | |
| throw new AppError('Invalid next_billing_at date', 'VALIDATION_ERROR', 400) | |
| } | |
| return parsed.toISOString() | |
| })() | |
| : periodEnd.toISOString() |
🤖 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 `@backend/supabase/functions/admin-update-subscription/index.ts` around lines
149 - 153, Validate body.next_billing_at before calling toISOString in the
nextBillingAt resolution. Parse the provided value, verify the resulting Date is
valid, and raise the established AppError for invalid user input; otherwise
preserve the admin override and periodEnd fallback behavior without allowing
RangeError to escape.
Source: Coding guidelines
1 participant
Summary
Admin-web bug audit fixes (
6012b83e)Full-codebase audit (~78 verified findings) fixed across 103 files:
user_idcolumn (entire page 403'd); create-payment-record inserted non-existent columns (always 500); five topic/path routes gated on a never-set cookie with no is_admin check; topics export pointed at repurposed endpoint; learning-path edit offered invalid disciple levelsauth.admin.listUsers()(was silently capped at 50 users) via newlib/supabase/list-all-users.ts; paginated analytics aggregations past PostgREST's 1000-row cap via newlib/supabase/fetch-all-rows.tsmiddleware.tswith @supabase/ssr session refresh — fixes random admin logouts on token rotationtest-consolepage, 8 unrouted token-management files@tailwindcss/typographydependency (markdown preview was unstyled)Verified: type-check clean, lint 0 errors, production build passes.
Android 1.0.1 release prep (
961d94d2)pubspec.yaml: 1.0.0+1 → 1.0.1+2Note:
backend/supabase/functions/admin-update-subscriptionchanged in this release (honorsnext_billing_atoverride). No new DB migrations.