-
Notifications
You must be signed in to change notification settings - Fork 4
feat: redesign credits page #164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds new credits UI components (CreditsStatistics, CreditsFullHistory, CreditsPeriodPicker), refactors app/pages/credits.vue to show a consolidated credits total and wire the new components, expands useCreditsFilters with year/month period semantics, exposes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (8)
app/constants/months.ts (1)
1-14: Consider i18n implications.Hardcoded English month names won't adapt to user locale. If internationalization becomes a requirement, consider using
Intl.DateTimeFormatto generate locale-aware month names. For now, this is fine if the app is English-only.app/components/credits/CreditsPeriodPicker.vue (1)
89-96: Non-scoped styles may leak.The
.pv-select-panelstyles are unscoped and will apply globally. If this is intentional (to style PrimeVue's portal-rendered panel), consider adding a brief comment explaining why scoped styles won't work here.app/components/credits/CreditsChart.vue (2)
140-140: Date string sorting may fail for 'year-month' labels.If
formatDatereturns labels like "Jan 2025",new Date("Jan 2025")may not parse reliably across browsers. SinceperiodKeysare already generated in chronological order (lines 91-117), the sort is redundant unless the order could be disrupted elsewhere. Consider removing the sort or storing a sortable key alongside the display label.
76-82: Module-level mutable cache is fragile.
lastComputedChangesis mutated inside a computed, which breaks Vue's reactivity expectations. While this works for caching during loading, consider using a separaterefand updating it via awatchfor clearer intent:-let lastComputedChanges: ChangeData[] = []; +const cachedChanges = ref<ChangeData[]>([]); const changes = computed<ChangeData[]>(() => { if (props.loading && lastComputedChanges.length > 0) { - return lastComputedChanges; + return cachedChanges.value; } // ... rest of computation - lastComputedChanges = data; + cachedChanges.value = data; return data; });app/components/credits/CreditsFullHistory.vue (2)
108-120: Consider adding explicit key touseLazyAsyncData.Without an explicit key, Nuxt auto-generates one based on file/line. If this component is used multiple times or if code moves, caching behavior could become unpredictable. Add an explicit key for stability:
const { data: creditsData, pending: loading, error: creditsDataError } = await useLazyAsyncData( + 'credits-full-history', () => minDelay($directus.request<{ changes: CreditsChange[]; count: number }>(customEndpoint({
145-149: Document the magic numbers in viewport calculation.The constants
595and54represent layout-specific pixel values. A brief comment would help future maintainers understand their origin:onMounted(async () => { if (!route.query.limit) { + // 595px = header + statistics section height; 54px = row height itemsPerPage.value = Math.min(Math.max(Math.floor((window.innerHeight - 595) / 54), 5), 15); } });app/composables/useCreditsFilters.ts (2)
131-139: Type narrowing formonthin query construction.Line 138 casts
filter.value.monthasnumber, but the type allows'last' | undefined. WhileshouldAddToQueryguards against non-numeric values reaching this line, explicit narrowing would be safer and clearer.- ...shouldAddToQuery('month', keysToUpdate) && { month: (filter.value.month as number) + 1 }, + ...shouldAddToQuery('month', keysToUpdate) && typeof filter.value.month === 'number' && { month: filter.value.month + 1 },
189-206: Month/year parsing order may benefit from consolidation.The month logic (lines 189–195) partially depends on
yearbeing truthy, and the year logic (lines 197–206) can reset month again at line 205. While functionally correct, consider grouping year/month parsing together to make the interdependency clearer:- if (month && PERMITTED_VALUES.month.includes(Number(month) - 1)) { - filter.value.month = Number(month) - 1; - } else if (year) { - filter.value.month = undefined; - } else { - filter.value.month = DEFAULT_FILTER.month; - } - - if (year) { - if (PERMITTED_VALUES.year.includes(Number(year))) { - filter.value.year = Number(year); - } else if (year === 'last') { - filter.value.year = 'last'; - } - } else { - filter.value.year = DEFAULT_FILTER.year; - filter.value.month = DEFAULT_FILTER.month; - } + // Process year first since month depends on it + if (year) { + if (PERMITTED_VALUES.year.includes(Number(year))) { + filter.value.year = Number(year); + } else if (year === 'last') { + filter.value.year = 'last'; + } + // Now handle month in context of valid year + if (month && PERMITTED_VALUES.month.includes(Number(month) - 1)) { + filter.value.month = Number(month) - 1; + } else { + filter.value.month = undefined; + } + } else { + filter.value.year = DEFAULT_FILTER.year; + filter.value.month = DEFAULT_FILTER.month; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/assets/css/base.css(1 hunks)app/assets/css/global.css(1 hunks)app/components/credits/CreditsChart.vue(4 hunks)app/components/credits/CreditsFilters.vue(3 hunks)app/components/credits/CreditsFullHistory.vue(1 hunks)app/components/credits/CreditsPeriodPicker.vue(1 hunks)app/components/credits/CreditsStatistics.vue(1 hunks)app/composables/useCreditsFilters.ts(4 hunks)app/constants/months.ts(1 hunks)app/pages/credits.vue(2 hunks)app/presets/aura/select/index.js(2 hunks)app/utils/date-formatters.ts(2 hunks)tailwind.config.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
app/presets/aura/select/index.jsapp/constants/months.tsapp/components/credits/CreditsStatistics.vueapp/components/credits/CreditsFilters.vueapp/components/credits/CreditsChart.vueapp/pages/credits.vuetailwind.config.jsapp/utils/date-formatters.tsapp/components/credits/CreditsFullHistory.vueapp/components/credits/CreditsPeriodPicker.vueapp/composables/useCreditsFilters.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 146
File: app/pages/credits.vue:58-62
Timestamp: 2025-10-06T17:46:36.047Z
Learning: In `app/pages/credits.vue`, the `CreditsChange` items rendered in the mobile view (lines 58-62) do not have an `id` field available for use as a v-for key. Using the array index is acceptable in this scenario due to the short list length and pagination-only updates.
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 131
File: pages/probes/index.vue:411-411
Timestamp: 2025-09-15T20:29:09.323Z
Learning: PavelKopecky prefers to delete the page parameter from query params instead of explicitly setting page: '1' when filters change in the GlobalPing Dashboard probes page. This keeps URLs cleaner while maintaining the same functionality, as page 1 is the default when the page parameter is absent.
📚 Learning: 2025-10-06T17:46:36.047Z
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 146
File: app/pages/credits.vue:58-62
Timestamp: 2025-10-06T17:46:36.047Z
Learning: In `app/pages/credits.vue`, the `CreditsChange` items rendered in the mobile view (lines 58-62) do not have an `id` field available for use as a v-for key. Using the array index is acceptable in this scenario due to the short list length and pagination-only updates.
Applied to files:
app/components/credits/CreditsStatistics.vueapp/components/credits/CreditsFilters.vueapp/components/credits/CreditsChart.vueapp/pages/credits.vueapp/components/credits/CreditsFullHistory.vueapp/components/credits/CreditsPeriodPicker.vueapp/composables/useCreditsFilters.ts
📚 Learning: 2025-10-12T14:51:10.711Z
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 154
File: app/components/credits/CreditsFilters.vue:101-106
Timestamp: 2025-10-12T14:51:10.711Z
Learning: In app/components/credits/CreditsFilters.vue, the early return when selectedCount.value === 0 is intentional. Having nothing selected is considered an invalid state, so the component preserves the previous filter values rather than clearing or resetting them.
Applied to files:
app/components/credits/CreditsFilters.vueapp/components/credits/CreditsChart.vueapp/pages/credits.vueapp/components/credits/CreditsFullHistory.vueapp/components/credits/CreditsPeriodPicker.vueapp/composables/useCreditsFilters.ts
📚 Learning: 2025-04-29T19:48:27.503Z
Learnt from: xbpcb
Repo: jsdelivr/globalping-dash PR: 72
File: pages/probes/[id].vue:424-424
Timestamp: 2025-04-29T19:48:27.503Z
Learning: The chart section in the probe detail page is intentionally hidden with v-if="showChart" (where showChart=false) because it's still under construction. This is by design and should not be changed until the feature is ready to be enabled.
Applied to files:
app/components/credits/CreditsChart.vue
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
app/pages/credits.vue
📚 Learning: 2025-04-29T21:58:13.738Z
Learnt from: xbpcb
Repo: jsdelivr/globalping-dash PR: 72
File: pages/probes/[id].vue:511-519
Timestamp: 2025-04-29T21:58:13.738Z
Learning: In the GlobalPing Dashboard application, components fully unmount and remount when navigating between different IDs in dynamic routes (e.g., from `/probes/123` to `/probes/456`), so explicit watchers for route parameter changes are not required.
Applied to files:
app/composables/useCreditsFilters.ts
🧬 Code graph analysis (1)
app/composables/useCreditsFilters.ts (1)
app/constants/months.ts (1)
MONTH_NAMES(1-14)
🔍 Remote MCP
Let me search for general best practices that would apply to this PR:Based on my research into best practices and patterns for Vue 3, Nuxt, and Tailwind CSS, I can now provide relevant context for this PR review.
Summary of Relevant Context
Data Fetching Patterns
useLazyAsyncData should be used when you don't want to block navigation (for non-critical or below-the-fold data), which applies to the new CreditsFullHistory component's use of useLazyAsyncData for paginated timeline data. useLazyAsyncData behaves with lazy: true option set - the async function does not block navigation, which means you will need to handle the situation where the data is null, and this PR properly addresses this with loading state handling via pending.
Always handle loading and error states in your components and use the refresh function or refreshNuxtData(key) after mutations, which the PR does through useErrorToast and loading indicators.
Vue 3 Composables Best Practices
When creating reactive watchers in composables, pass reactive state into the composable function and let the composable create watchers that perform actions using the passed state. The useCreditsFilters composable follows this pattern by accepting and watching route query parameters for type, reason, year, and month.
CSS Variable Management & Tailwind Integration
When working with TailwindCSS, it's a good practice to use vanilla CSS for custom properties (CSS variables) to ensure flexibility and reusability across multiple components or themes, making your theme more dynamic and maintainable. The PR's approach of defining --jsd-orange as a CSS variable in base.css and extending Tailwind config to map it (jsd-orange: var(--jsd-orange)) aligns with this pattern.
Theme variables are special CSS variables that influence which utility classes exist in your project and represent design tokens, supporting the use of the new orange color token across the codebase.
Chart Component Data Handling
Your goal should be to create reusable chart components by utilizing Vue.js props to pass in chart options and chart data, so the parent component does not hold an opinion about fetching data and is only for presentation. The refactored CreditsChart component properly accepts data, options, and loading props rather than managing its own data fetching.
Responsive Design Patterns
The new CreditsFullHistory component implements responsive behavior with async row rendering on smaller screens and DataTable on larger screens, which is consistent with modern Vue 3 responsive patterns.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (16)
tailwind.config.js (1)
66-66: LGTM!The new
jsd-orangecolor token follows the established pattern for other color definitions in this config.app/presets/aura/select/index.js (2)
135-135: LGTM!Adding
focus:outline-primaryimproves keyboard accessibility for select options.
196-196: LGTM!Simplified padding class is cleaner. Note this adds bottom padding where there was none before (
pb-0→p-2).app/assets/css/base.css (2)
60-61: LGTM!New
--jsd-orangevariable is properly defined and will be consumed by the Tailwind color token.
80-81: LGTM!The dark mode override for
--main-900to use--bluegray-0(white) inverts the color for proper text contrast on dark backgrounds.app/assets/css/global.css (1)
74-80: LGTM!Both gradient utilities are well-structured with consistent dark mode handling.
app/utils/date-formatters.ts (1)
3-14: LGTM!The new
'year-month'format is cleanly integrated, following the existing pattern with early return. Consistent use of'en-US'locale matches other formatters in this file.app/pages/credits.vue (2)
56-63: LGTM!The
useLazyAsyncDatapattern with transform and default is well-structured. The reduce with|| 0fallback handles empty arrays correctly.
2-17: LGTM!Clean template composition with proper accessibility (
aria-hiddenon decorative icon) and testability (data-testid).app/components/credits/CreditsFilters.vue (3)
36-36: LGTM!The destructuring now correctly uses
creditsTableFilterKeyfrom the updated composable API.
98-98: LGTM!Watch correctly observes
creditsTableFilterKeyto trigger filter application when the table filter changes.
120-124: LGTM!The partial mutation approach (
filter.value.type,filter.value.reason) correctly preserves other filter properties likeyearandmonthwhile updating only the type/reason fields.app/components/credits/CreditsPeriodPicker.vue (1)
78-86: Verify no update loop between watchers.When
directusDateQuerychanges, the first watch (lines 78-80) updatesselectedOption, which triggers the second watch (lines 82-86) to callonParamChange. IfonParamChangeaffectsdirectusDateQuery, this could cause redundant updates. The logic appears safe becausegetInitialOption()should return the same option if filters already match, but worth confirming during testing.app/components/credits/CreditsStatistics.vue (2)
199-199:tier_changedadditions are fetched but not counted.The fetch at line 129 includes
tier_changedin the reasons filter, butsponsorshipAdditionsonly matches reasons containing'sponsorship'. Iftier_changedcredits should contribute to the "Credits gained" from sponsorship, update the filter:-const sponsorshipAdditions = computed(() => credits.value.additions.reduce((sum, addition) => sum + (addition.reason.includes('sponsorship') ? addition.amount : 0), 0)); +const sponsorshipAdditions = computed(() => credits.value.additions.reduce((sum, addition) => sum + (addition.reason.includes('sponsorship') || addition.reason === 'tier_changed' ? addition.amount : 0), 0));
76-81: LGTM!The chart integration correctly computes the starting balance and passes the additions/deductions arrays with loading state.
app/components/credits/CreditsFullHistory.vue (1)
57-59: LGTM!Using array index as key is acceptable here per prior discussion—the list is short and updates occur only via pagination.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/components/credits/CreditsStatistics.vue (3)
165-180: Makeonline-probestransform resilient to empty aggregate resultsThe transform assumes the aggregate always returns at least one row:
transform: data => data[0].count || 0,If Directus ever returns an empty array, this will throw on
data[0]. A small defensive tweak keeps behavior the same for normal responses and avoids that edge-case crash:- transform: data => data[0].count || 0, + transform: data => data[0]?.count ?? 0,
99-99: AvoidNaNand stale values indailyAdditionsby makingcreditsPerAdoptedProbereactive
creditsPerAdoptedProbeis captured once from the store:const creditsPerAdoptedProbe = metadata.creditsPerAdoptedProbe; const dailyAdditions = computed(() => todayOnlineProbes.value * creditsPerAdoptedProbe);If the metadata store populates
creditsPerAdoptedProbeasynchronously or ever changes it,dailyAdditionswill not update. If the initial value isundefined, the computed will also surfaceNaNin the UI.You can make this reactive and safe with a small change:
-const creditsPerAdoptedProbe = metadata.creditsPerAdoptedProbe; +const creditsPerAdoptedProbe = computed(() => metadata.creditsPerAdoptedProbe ?? 0); -const dailyAdditions = computed(() => todayOnlineProbes.value * creditsPerAdoptedProbe); +const dailyAdditions = computed(() => todayOnlineProbes.value * creditsPerAdoptedProbe.value);Also applies to: 195-199
49-49: Double‑check thegaclass in the probes card
class="ga relative flex …"includes agatoken that doesn’t look like a standard Tailwind utility. If this isn’t a deliberate global class, it’s likely a leftover typo and can be removed to keep the class list clean.app/composables/useCreditsFilters.ts (1)
18-21: Confirm exclusion of current month/year inPERIOD_OPTIONSis intentionalThe options built from
AVAILABLE_YEARSare filtered with:.filter( opt => opt.value.year !== CURRENT_YEAR || (typeof opt.value.month !== 'undefined' && opt.value.month < CURRENT_MONTH), )This removes:
- the pure
CURRENT_YEARentry, and- any
CURRENT_YEARmonth whose index is>= CURRENT_MONTH(so users cannot select the ongoing month or “this year so far”), leaving only fully elapsed months/years plus the rolling “Last year” / “Last month” presets.If the design is supposed to offer YTD and current‑month views, you may want to relax this filter (e.g. keep the year entry for
CURRENT_YEARand/or allowopt.value.month <= CURRENT_MONTH). If the current behavior is by design, consider adding a short comment explaining why future/ongoing periods are excluded.Also applies to: 35-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/components/credits/CreditsStatistics.vue(1 hunks)app/composables/useCreditsFilters.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
app/components/credits/CreditsStatistics.vueapp/composables/useCreditsFilters.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 131
File: pages/probes/index.vue:411-411
Timestamp: 2025-09-15T20:29:09.323Z
Learning: PavelKopecky prefers to delete the page parameter from query params instead of explicitly setting page: '1' when filters change in the GlobalPing Dashboard probes page. This keeps URLs cleaner while maintaining the same functionality, as page 1 is the default when the page parameter is absent.
📚 Learning: 2025-10-06T17:46:36.047Z
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 146
File: app/pages/credits.vue:58-62
Timestamp: 2025-10-06T17:46:36.047Z
Learning: In `app/pages/credits.vue`, the `CreditsChange` items rendered in the mobile view (lines 58-62) do not have an `id` field available for use as a v-for key. Using the array index is acceptable in this scenario due to the short list length and pagination-only updates.
Applied to files:
app/components/credits/CreditsStatistics.vueapp/composables/useCreditsFilters.ts
📚 Learning: 2025-10-12T14:51:10.711Z
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 154
File: app/components/credits/CreditsFilters.vue:101-106
Timestamp: 2025-10-12T14:51:10.711Z
Learning: In app/components/credits/CreditsFilters.vue, the early return when selectedCount.value === 0 is intentional. Having nothing selected is considered an invalid state, so the component preserves the previous filter values rather than clearing or resetting them.
Applied to files:
app/composables/useCreditsFilters.ts
📚 Learning: 2025-04-29T21:58:13.738Z
Learnt from: xbpcb
Repo: jsdelivr/globalping-dash PR: 72
File: pages/probes/[id].vue:511-519
Timestamp: 2025-04-29T21:58:13.738Z
Learning: In the GlobalPing Dashboard application, components fully unmount and remount when navigating between different IDs in dynamic routes (e.g., from `/probes/123` to `/probes/456`), so explicit watchers for route parameter changes are not required.
Applied to files:
app/composables/useCreditsFilters.ts
🧬 Code graph analysis (1)
app/composables/useCreditsFilters.ts (1)
app/constants/months.ts (1)
MONTH_NAMES(1-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (1)
app/components/credits/CreditsStatistics.vue (1)
182-193: Verify thatSponsorshipDetailsis actually in scope for TypeScriptThe
customEndpointcall is parameterized withSponsorshipDetails, but there is no local import or declared interface in this file. If this type isn’t provided via a globald.tsincluded intsconfig, the file will fail type‑checking.Confirm that
SponsorshipDetailsis globally declared; otherwise, add an explicitimport type { SponsorshipDetails } from '…'or drop the generic if you don’t rely on static typing here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/components/credits/CreditsStatistics.vue (3)
170-181: Guard aggregate results before indexing to avoid potential runtime errors
startingBalanceassumesprevAdditions[0]andprevDeductions[0]always exist. If Directus ever returns an empty array (e.g., user with no history),prevAdditions[0].sumwill throw before optional chaining is applied.Consider defensively guarding the first element and using nullish coalescing so
0is preserved:- startingBalance: (prevAdditions[0].sum?.amount || 0) - (prevDeductions[0].sum?.amount || 0), + startingBalance: (prevAdditions[0]?.sum?.amount ?? 0) - (prevDeductions[0]?.sum?.amount ?? 0),Please double‑check Directus aggregate semantics for empty result sets and adjust if it’s guaranteed to always return a row.
185-200: Makeonline-probestransform resilient to empty aggregate responses
transform: data => data[0].count || 0assumesdata[0]always exists. If the aggregate query ever returns an empty array, this will blow up before the fallback is applied.A small defensive change keeps behavior the same while avoiding that risk:
- transform: data => data[0].count || 0, + transform: data => data[0]?.count ?? 0,Please confirm how
aggregate('gp_probes', ...)behaves when no rows match (empty array vs single row with zero).
21-21: Consider centralizing number formatting instead of hard‑coding'en-US'repeatedly
toLocaleString('en-US')is repeated for multiple credit values. If you ever change locale or formatting rules, this will be easy to miss.You could extract a small helper/composable or formatter ref to avoid duplication, e.g. a
formatCreditsfunction or sharedIntl.NumberFormat, and use it consistently in the template.Also applies to: 34-35, 62-63, 69-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/components/credits/CreditsStatistics.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
app/components/credits/CreditsStatistics.vue
🧠 Learnings (2)
📓 Common learnings
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 146
File: app/pages/credits.vue:58-62
Timestamp: 2025-10-06T17:46:36.047Z
Learning: In `app/pages/credits.vue`, the `CreditsChange` items rendered in the mobile view (lines 58-62) do not have an `id` field available for use as a v-for key. Using the array index is acceptable in this scenario due to the short list length and pagination-only updates.
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 131
File: pages/probes/index.vue:411-411
Timestamp: 2025-09-15T20:29:09.323Z
Learning: PavelKopecky prefers to delete the page parameter from query params instead of explicitly setting page: '1' when filters change in the GlobalPing Dashboard probes page. This keeps URLs cleaner while maintaining the same functionality, as page 1 is the default when the page parameter is absent.
📚 Learning: 2025-10-06T17:46:36.047Z
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 146
File: app/pages/credits.vue:58-62
Timestamp: 2025-10-06T17:46:36.047Z
Learning: In `app/pages/credits.vue`, the `CreditsChange` items rendered in the mobile view (lines 58-62) do not have an `id` field available for use as a v-for key. Using the array index is acceptable in this scenario due to the short list length and pagination-only updates.
Applied to files:
app/components/credits/CreditsStatistics.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/components/credits/CreditsStatistics.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
app/components/credits/CreditsStatistics.vue
🧠 Learnings (2)
📓 Common learnings
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 146
File: app/pages/credits.vue:58-62
Timestamp: 2025-10-06T17:46:36.047Z
Learning: In `app/pages/credits.vue`, the `CreditsChange` items rendered in the mobile view (lines 58-62) do not have an `id` field available for use as a v-for key. Using the array index is acceptable in this scenario due to the short list length and pagination-only updates.
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 131
File: pages/probes/index.vue:411-411
Timestamp: 2025-09-15T20:29:09.323Z
Learning: PavelKopecky prefers to delete the page parameter from query params instead of explicitly setting page: '1' when filters change in the GlobalPing Dashboard probes page. This keeps URLs cleaner while maintaining the same functionality, as page 1 is the default when the page parameter is absent.
📚 Learning: 2025-10-06T17:46:36.047Z
Learnt from: PavelKopecky
Repo: jsdelivr/globalping-dash PR: 146
File: app/pages/credits.vue:58-62
Timestamp: 2025-10-06T17:46:36.047Z
Learning: In `app/pages/credits.vue`, the `CreditsChange` items rendered in the mobile view (lines 58-62) do not have an `id` field available for use as a v-for key. Using the array index is acceptable in this scenario due to the short list length and pagination-only updates.
Applied to files:
app/components/credits/CreditsStatistics.vue
🔍 Remote MCP
Let me search for documentation on the GitHub repository using web search to find any available guidelines or README documentation that would help with the review.Based on the search results, I have found relevant documentation about Vue composables and component architecture patterns. However, the DeepWiki tool cannot access the globalping-dash repository documentation directly as it hasn't been indexed. Let me provide you with the gathered context for your PR review:
Summary of Additional Context Found
Repository Technical Context
The project uses PrimeVue as a component library. When adding components, they should be uncommented in presets/aura/index.js, as unused components are commented out to reduce bundle size.
Vue Composables Best Practices (Applicable to useCreditsFilters changes)
The recommendation is to use composables when reusing pure logic, and use components when reusing both logic and visual layout. The PR significantly expands the useCreditsFilters composable with new public API surface (creditsTableFilterKey, directusDateQuery, getTableFilter), which aligns with composable design patterns for managing shared business logic.
Key considerations for the composable changes:
- Each Composable should have one responsibility and one reason to change, ensuring clarity and maintainability.
- Since business logic inside composables are basically TypeScript functions, they are very easy to test with Vitest and allow for more stable systems.
Component Architecture Pattern
Building dashboards with modular architecture involves using modular components, lazy loading, scoped state, and clean folder structures — all while maintaining performance and clarity. The PR introduces new feature-specific components (CreditsStatistics, CreditsFullHistory, CreditsPeriodPicker) which follows this modular pattern.
Data Fetching Pattern Review Points
The PR uses useLazyAsyncData for paginated and filtered data in CreditsFullHistory.vue. This pattern should ensure:
- Proper error handling through useErrorToast
- Loading state management during data transitions
- Debouncing of filter changes via creditsTableFilterKey dependency
[::web_search::] - Documentation on PrimeVue component library usage and bundling practices, Vue 3 composables best practices for state management and logic reuse, and modular dashboard architecture patterns.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (1)
app/components/credits/CreditsStatistics.vue (1)
114-115: I'll help you verify this review comment about the filter key inconsistency. Let me start by searching for the schema definitions and examining the usage patterns in the codebase.
<function_calls>
#!/bin/bashSearch for schema definitions, migrations, or table references
echo "=== Searching for schema or migration files ==="
fd -e sql -e ts -e js | grep -i -E "(migrat|schema|database)" | head -20echo -e "\n=== Searching for gp_credits tables references ==="
rg -n "gp_credits_additions|gp_credits_deductions" -C 3echo -e "\n=== Searching for getUserFilter usage in CreditsStatistics.vue ==="
rg -n "getUserFilter" app/components/credits/CreditsStatistics.vue -B 2 -A 2
</function_calls>Let me also examine the CreditsStatistics.vue file directly to understand the context:
<function_calls>
#!/bin/bashGet line count and examine the file
echo "=== Checking CreditsStatistics.vue file ==="
wc -l app/components/credits/CreditsStatistics.vueecho -e "\n=== Examining lines around 114-115 and 124-125 ==="
sed -n '100,160p' app/components/credits/CreditsStatistics.vueecho -e "\n=== Searching for all getUserFilter calls in this file ==="
grep -n "getUserFilter" app/components/credits/CreditsStatistics.vue
</function_calls>Now let me search for the database schema or any files that define these table structures:
<function_calls>
#!/bin/bashSearch for Directus or database configuration
echo "=== Searching for directus or database configuration ==="
fd -type f -E node_modules -E dist | xargs grep -l "gp_credits" 2>/dev/null | head -10echo -e "\n=== Looking for migration or schema files ==="
find . -type f ( -name "migration" -o -name "schema" ) -not -path "/node_modules/" -not -path "/.git/" 2>/dev/null | head -20echo -e "\n=== Searching for column/field definitions ==="
rg -n "github_id|user_id" app/components/credits/CreditsStatistics.vue -B 1 -A 1
</function_calls>Let me look at the directory structure to better understand the project layout:
<function_calls>
#!/bin/bashCheck project structure
echo "=== Root directory structure ==="
ls -laecho -e "\n=== Look for any database or API configuration ==="
fd -type f -name ".config." -o -name "directus" -o -name "db" | grep -v node_modules | head -20echo -e "\n=== Search for references to credits tables ==="
rg -n "credits" --type ts --type js --type vue | grep -i "query|filter|where" | head -20
</function_calls>Now let me directly examine the actual Vue file content:
<function_calls>
#!/bin/bashRead the Vue file directly
echo "=== Content of app/components/credits/CreditsStatistics.vue ==="
cat -n app/components/credits/CreditsStatistics.vue | head -200
</function_calls>Let me search for the getUserFilter function definition:
<function_calls>
#!/bin/bashFind getUserFilter function
echo "=== Searching for getUserFilter function definition ==="
rg -n "getUserFilter" --type ts --type js -B 2 -A 10 | head -50echo -e "\n=== Looking in composables or utils ==="
fd -type f ( -name ".ts" -o -name ".js" ) | xargs grep -l "getUserFilter" 2>/dev/null
</function_calls>Let me also check for Directus documentation or schema files in the repo:
<function_calls>
#!/bin/bashSearch more broadly for table structure info
echo "=== Searching for API queries or table queries ==="
rg -n "gp_credits" --type ts --type js --type vue -B 3 -A 3echo -e "\n=== Look for import statements in CreditsStatistics ==="
head -50 app/components/credits/CreditsStatistics.vueecho -e "\n=== Check for any type definitions ==="
fd -type f -name "*.d.ts" -o -name "types" | grep -i credit
</function_calls>
| Statistics | ||
| </h3> | ||
| <div class="flex items-center gap-3"> | ||
| <i v-if="creditsDataLoading" class="pi pi-spinner animate-spin text-surface-500 dark:text-bluegray-500"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This component should use (3) AsyncBlocks and then you can remove this loader.
|
TODO after internal discussion:
|
Closes #163