-
Notifications
You must be signed in to change notification settings - Fork 72
feat(billing): add usage ui #1524
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
Conversation
WalkthroughThis change introduces a comprehensive usage tracking and analytics feature across both backend and frontend. It renames all lease-related metrics to deployment-related ones, updates schemas, repositories, and services accordingly, and adds new endpoints and data types for usage history and statistics. The frontend implements new pages, charts, hooks, and service integrations to visualize and export usage data, with full test coverage and feature flag control. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant UsageHttpService
participant API
participant UsageService
participant DeploymentRepository
User->>Frontend: Navigates to /usage (feature flag enabled)
Frontend->>UsageHttpService: GET /v1/usage/history (with address, dates)
UsageHttpService->>API: GET /v1/usage/history
API->>UsageService: getHistory(address, startDate, endDate)
UsageService->>DeploymentRepository: Query usage data
DeploymentRepository-->>UsageService: Usage history data
UsageService-->>API: Usage history response
API-->>UsageHttpService: JSON usage history
UsageHttpService-->>Frontend: UsageHistory[]
Frontend->>User: Renders charts and stats
Note over Frontend,UsageHttpService: Similar flow for /v1/usage/history/stats
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1524 +/- ##
==========================================
- Coverage 72.16% 72.07% -0.09%
==========================================
Files 592 597 +5
Lines 13787 13902 +115
Branches 2335 2356 +21
==========================================
+ Hits 9949 10020 +71
- Misses 3605 3662 +57
+ Partials 233 220 -13
🚀 New features to boost your workflow:
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 5
🧹 Nitpick comments (17)
apps/deploy-web/src/components/usage/usage-tab/charts/MonthlySpendingBarChart.tsx (1)
1-1
: Acknowledge placeholder for future implementation.The TODO comment indicates this component is pending implementation. This is acceptable for incremental development.
Would you like me to help implement the MonthlySpendingBarChart component using recharts, or open an issue to track this task?
apps/deploy-web/src/utils/apiUtils.ts (1)
84-89
: Avoidthis
in static methods
AccessingbaseApiUrl
viathis
inside static methods can be confusing. Consider referencing the class directly:static usage() { - return `${this.baseApiUrl}/v1/usage/history`; + return `${ApiUrlService.baseApiUrl}/v1/usage/history`; } static usageStats() { - return `${this.baseApiUrl}/v1/usage/history/stats`; + return `${ApiUrlService.baseApiUrl}/v1/usage/history/stats`; }apps/deploy-web/src/pages/usage/index.tsx (2)
2-2
: Address access control TODO
This page currently permits unauthenticated access. Do you want to wrap it withRegisteredUsersOnly
to restrict it to logged-in users?
3-3
: Clean up commented code
If you decide not to useRegisteredUsersOnly
here, remove the commented import to keep the file tidy.apps/deploy-web/src/pages/billing/index.tsx (2)
2-2
: Address access control TODO
Consider whetherBillingPage
should be protected byRegisteredUsersOnly
to enforce authentication.
3-3
: Clean up commented code
Remove or implement the commented import to avoid dead code.apps/deploy-web/src/components/usage/billing-tab/BillingPage.tsx (1)
8-17
: Placeholder implementation noted.The component structure looks good with proper SEO and layout setup. The TODO comment clearly indicates the pending transactions list implementation.
Once you're ready to implement the transactions list, I can help generate the component structure and data fetching logic if needed.
apps/api/src/deployment/repositories/deployment/deployment.repository.ts (1)
27-31
: Method implementation looks correct.The
countByOwner
method follows proper Sequelize patterns and integrates well with the shift from lease-based to deployment-based usage tracking.For future optimization, consider adding indexing on the
owner
column if not already present, as this count operation could become expensive with large datasets.apps/deploy-web/src/components/usage/usage-tab/mock-credits-usage-data.ts (1)
2-94
: Clarify the relationship between credits and used values.Several entries show "used" values exceeding "credits" values (e.g., line 5: credits: 290, used: 373). This seems logically inconsistent unless:
- These represent different metrics than the names suggest
- There's an overdraft mechanism allowing usage beyond available credits
- The mock data contains errors
Please clarify the intended relationship between these values.
Consider adding type definitions for the data structure to improve code clarity:
+interface CreditsUsageData { + date: string; + credits: number; + used: number; +} + +const data: CreditsUsageData[] = [ -const data = [apps/deploy-web/src/components/usage/UsageLayout.tsx (1)
31-34
: Remove redundant case clause.The
case UsageTabs.USAGE:
clause is redundant since it's immediately followed by thedefault:
clause that handles the same logic.Apply this diff to simplify the switch statement:
- case UsageTabs.USAGE: default: router.push(UrlService.usage()); break;
apps/api/src/billing/services/usage/mock-history-data.ts (2)
1-1
: Address the TODO: Consider removing mock data before production merge.The TODO comment indicates this mock data is temporary and should be removed before merging. Ensure this is tracked and addressed before the PR goes to production.
Would you like me to open an issue to track the removal of this mock data and replacement with real data sources?
4-902
: Consider generating mock data instead of hardcoding large arrays.The 82 hardcoded objects create maintenance overhead and increase bundle size. Consider using a data generator function to create this mock data dynamically.
-export default [ - { - date: "2025-03-25", - activeDeployments: 5, - // ... 82 objects - } -]; +function generateMockUsageData(startDate: string, days: number) { + const data = []; + const start = new Date(startDate); + + for (let i = 0; i < days; i++) { + const date = new Date(start); + date.setDate(start.getDate() + i); + + data.push({ + date: date.toISOString().split('T')[0], + activeDeployments: Math.floor(Math.random() * 5) + 1, + dailyAktSpent: Number((Math.random() * 100).toFixed(2)), + // ... generate other fields + }); + } + return data; +} + +export default generateMockUsageData("2025-03-25", 82);apps/deploy-web/src/components/usage/usage-tab/UsageContainer.tsx (3)
30-30
: Replace hardcoded address with actual user address.The hardcoded address needs to be replaced with the current user's address before production deployment.
Do you need help implementing user context or authentication integration to get the current user's address?
59-84
: Clean up commented code before merging.Multiple lines of commented code should be removed to improve readability and maintainability.
- // usageHistoryData: [], usageHistoryData, - // usageHistoryStatsData: { - // totalSpent: 0, - // averageSpentPerDay: 0, - // totalDeployments: 0, - // averageDeploymentsPerDay: 0 - // }, usageHistoryStatsData, /* TODO: Replace mockCreditsUsageData with actual data after figuring out where it should come from */ - // creditsUsageData: [], creditsUsageData: mockCreditsUsageData.filter(item => { const date = new Date(item.date); return date >= startDate && date <= endDate; }), dateRange, onDateRangeChange: setDateRange, - // isFetchingUsageHistory: true, isFetchingUsageHistory, - // isUsageHistoryError: true, isUsageHistoryError, - // isFetchingUsageHistoryStats: true, isFetchingUsageHistoryStats, - // isUsageHistoryStatsError: true, isUsageHistoryStatsError
68-74
: Track the TODO for replacing mock credits usage data.The mock credits usage data filtering should be replaced with real data once the data source is determined.
Would you like me to help identify potential data sources for credits usage or create an issue to track this task?
apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx (1)
43-53
: Consider more graceful error handling for invalid daysAgo values.The current implementation throws an error for invalid
daysAgo
values, which could crash the component. Consider defaulting to a valid value instead.React.useEffect(() => { - if (Number.isNaN(Number(daysAgo))) { - throw new Error("daysAgo should be a valid number string"); - } + const daysAgoNumber = Number(daysAgo); + if (Number.isNaN(daysAgoNumber)) { + console.warn("Invalid daysAgo value, defaulting to 90"); + setDaysAgo("90"); + return; + } onDateRangeChange( createDateRange({ - startDate: subDays(new Date(), Number(daysAgo)) + startDate: subDays(new Date(), daysAgoNumber) }) ); }, [daysAgo, onDateRangeChange]);apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx (1)
29-53
: Consider improving the spending change calculation for better performance and edge case handling.The calculation logic is generally correct, but there are a few areas for improvement:
- The data filtering and sorting happens on every render
- Edge case handling could be more explicit
Consider memoizing the calculation or moving it to a custom hook:
+ import { useMemo } from "react"; export const CumulativeSpendingLineChart: FC<CumulativeSpendingLineChartProps> = ({ isFetching, data }) => { - const calculateSpendingChange = () => { + const spendingChange = useMemo(() => { if (data.length < 2) return 0; const tomorrow = startOfTomorrow(); const monthAgo = subDays(tomorrow, 31); const lastMonthData = data .filter(item => { const date = new Date(item.date); return isAfter(date, monthAgo) && isBefore(date, tomorrow); }) .sort((a, b) => new Date(a.date).getTime() - new Date(b.date).getTime()); if (lastMonthData.length < 2) return 0; const firstValue = lastMonthData[0].totalUsdSpent; const lastValue = lastMonthData[lastMonthData.length - 1].totalUsdSpent; if (firstValue === 0) return 0; const percentageChange = ((lastValue - firstValue) / firstValue) * 100; return Math.round(percentageChange * 100) / 100; - }; + }, [data]);And update the usage:
- <TrendIndicator change={calculateSpendingChange()} /> + <TrendIndicator change={spendingChange} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (31)
apps/api/src/billing/http-schemas/usage.schema.ts
(2 hunks)apps/api/src/billing/repositories/usage/usage.repository.spec.ts
(6 hunks)apps/api/src/billing/repositories/usage/usage.repository.ts
(3 hunks)apps/api/src/billing/services/usage/mock-history-data.ts
(1 hunks)apps/api/src/billing/services/usage/usage.service.spec.ts
(11 hunks)apps/api/src/billing/services/usage/usage.service.ts
(1 hunks)apps/api/src/deployment/repositories/deployment/deployment.repository.ts
(1 hunks)apps/api/src/deployment/repositories/lease/lease.repository.ts
(0 hunks)apps/api/test/functional/usage.spec.ts
(5 hunks)apps/api/test/seeders/billing-usage.seeder.ts
(2 hunks)apps/deploy-web/package.json
(1 hunks)apps/deploy-web/src/components/layout/AccountMenu.tsx
(2 hunks)apps/deploy-web/src/components/usage/UsageLayout.tsx
(1 hunks)apps/deploy-web/src/components/usage/UsagePage.tsx
(1 hunks)apps/deploy-web/src/components/usage/billing-tab/BillingPage.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/UsageContainer.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/CreditsUsageAreaChart.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/MonthlySpendingBarChart.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/mock-credits-usage-data.ts
(1 hunks)apps/deploy-web/src/pages/billing/index.tsx
(1 hunks)apps/deploy-web/src/pages/usage/index.tsx
(1 hunks)apps/deploy-web/src/queries/index.ts
(1 hunks)apps/deploy-web/src/queries/queryKeys.ts
(1 hunks)apps/deploy-web/src/queries/useUsage.ts
(1 hunks)apps/deploy-web/src/types/billing.ts
(1 hunks)apps/deploy-web/src/utils/apiUtils.ts
(1 hunks)apps/deploy-web/src/utils/dateUtils.ts
(2 hunks)apps/deploy-web/src/utils/urlUtils.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/deployment/repositories/lease/lease.repository.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/deploy-web/src/utils/apiUtils.ts
[error] 85-85: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 88-88: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
apps/deploy-web/src/components/usage/UsageLayout.tsx
[error] 31-31: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: validate-api
- GitHub Check: test-api-build
- GitHub Check: test-deploy-web-build
🔇 Additional comments (27)
apps/deploy-web/package.json (1)
119-119
: Verify that the recharts version is secure and up-to-date.Ensure that version
^2.15.4
of recharts is the latest stable version and free from known security vulnerabilities.What is the latest version of recharts and are there any security vulnerabilities in version 2.15.4?
apps/deploy-web/src/utils/dateUtils.ts (2)
1-1
: Good refactoring to use date-fns library.Replacing custom date utilities with the well-tested date-fns library improves reliability and reduces maintenance burden.
46-60
: Well-implemented date range utility function.The
createDateRange
function has proper validation, sensible defaults (30-day range ending today), and clear error handling. The implementation correctly handles edge cases and provides a consistent API for date range operations.apps/deploy-web/src/queries/index.ts (1)
7-7
: LGTM - follows existing export pattern.The export addition is consistent with the file's structure and enables centralized access to the new usage query hooks.
apps/deploy-web/src/utils/urlUtils.ts (1)
40-41
: LGTM - URL utilities follow established patterns.The new
usage()
andbilling()
methods are consistent with the existing UrlService implementation and provide centralized URL management for the new pages.apps/api/src/billing/repositories/usage/usage.repository.spec.ts (6)
84-84
: Consistent rename in test assertion
The propertyactiveDeployments
has been correctly updated fromactiveLeases
in the base case where no deployments exist.
134-134
: Consistent rename in daily AKT test
TheactiveDeployments
field is correctly asserted in the AKT-denominated usage test.
143-143
: Consistent rename in cumulative AKT test
TheactiveDeployments
assertion matches the domain shift for the second day.
179-179
: Consistent rename in USDC test
TheactiveDeployments
count is correctly updated in the USDC-denominated usage case.
224-224
: Consistent rename in mixed-denomination test
TheactiveDeployments
count of 2 is correctly asserted when multiple deployments are present.
267-267
: Consistent rename in exclusion test
TheactiveDeployments
count correctly excludes leases from other addresses.apps/deploy-web/src/components/layout/AccountMenu.tsx (2)
16-16
: Ensure lucide-react dependency
Verify thatlucide-react
has been added topackage.json
so theLineChart
import resolves without build errors.
96-98
: Add Billing & Usage menu item
The new dropdown entry correctly usesUrlService.usage()
and theLineChart
icon to navigate to the usage page.apps/deploy-web/src/pages/usage/index.tsx (1)
1-1
: Entry point export
Re-exportingUsagePage
as the default export correctly integrates this page into Next.js routing.apps/deploy-web/src/pages/billing/index.tsx (1)
1-1
: Entry point export
Re-exportingBillingPage
for Next.js routing is correct.apps/deploy-web/src/queries/queryKeys.ts (1)
3-4
: LGTM! Query keys follow established patterns.The new usage data query keys are implemented consistently with the existing codebase patterns and will properly support React Query caching.
apps/api/src/billing/repositories/usage/usage.repository.ts (1)
8-8
: Consistent terminology update from leases to deployments.The renaming from
activeLeases
toactiveDeployments
is applied consistently across the interface definition, SQL query alias, and result mapping. This aligns well with the broader refactoring effort.Also applies to: 77-77, 102-102
apps/deploy-web/src/types/billing.ts (1)
6-22
: LGTM! Clean type definitions for usage tracking.The new
UsageHistory
andUsageHistoryStats
types are well-structured and provide clear interfaces for the usage tracking functionality. The property naming is consistent and follows the deployment terminology used throughout the refactoring.apps/api/test/seeders/billing-usage.seeder.ts (1)
8-8
: LGTM! Consistent terminology update.The renaming from
activeLeases
toactiveDeployments
aligns with the broader refactoring effort to standardize deployment-related terminology throughout the codebase.Also applies to: 18-18
apps/deploy-web/src/components/usage/UsagePage.tsx (1)
9-18
: LGTM! Well-structured usage page component.The component follows React best practices with proper TypeScript typing, clean imports, and effective use of the render prop pattern. The NextSeo integration and layout structure are appropriate for a usage dashboard page.
apps/api/test/functional/usage.spec.ts (1)
420-427
: LGTM! Consistent test updates for deployment terminology.The property name changes throughout the test assertions correctly reflect the refactoring from lease-based to deployment-based terminology. All updated property names (
averageSpentPerDay
,totalDeployments
,averageDeploymentsPerDay
) are consistent with the new API schema.Also applies to: 450-451, 461-463, 507-507, 518-519
apps/api/src/billing/services/usage/usage.service.spec.ts (1)
6-251
: LGTM: Comprehensive terminology update from leases to deployments.The test file has been systematically updated to reflect the change from lease-based to deployment-based usage tracking. The changes are comprehensive and maintain the original test logic while properly updating:
- Import statements and type references
- Mock setup and variable names
- Test assertions and expected values
- Edge case handling
The test coverage remains thorough with proper error handling scenarios.
apps/deploy-web/src/queries/useUsage.ts (1)
1-54
: LGTM: Well-structured React Query hooks with proper error handling.The implementation demonstrates good practices:
- Clean separation of concerns with dedicated async functions
- Proper TypeScript typing for parameters and return values
- Consistent date formatting using date-fns
- Appropriate query key construction for caching
- Good integration with the API service layer
The hooks provide a clean interface for components to fetch usage data and statistics.
apps/deploy-web/src/components/usage/usage-tab/charts/CreditsUsageAreaChart.tsx (1)
1-81
: Well-implemented chart component with proper TypeScript typing and loading states.The component follows React best practices with clean TypeScript typing, proper loading state handling, and good use of the recharts library. The conditional styling for loading states and date formatting implementation are particularly well done.
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.tsx (1)
1-61
: Consistent and well-implemented bar chart component.The component follows the same solid patterns as other chart components in the codebase, with proper TypeScript typing, loading state management, and date formatting. Good consistency across chart implementations.
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx (1)
98-107
: LGTM on the TrendIndicator component.Clean implementation with appropriate conditional rendering and proper use of Lucide icons.
apps/api/src/billing/services/usage/usage.service.ts (1)
46-56
: Statistics calculation logic looks correct.The calculation properly handles edge cases and uses appropriate rounding for financial data. The renaming from lease-based to deployment-based terminology is consistent.
… stripe charges list endpoin
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: 5
🧹 Nitpick comments (1)
apps/deploy-web/src/utils/apiUtils.ts (1)
83-88
: Consider using class name instead ofthis
in static methods.While the usage of
this.baseApiUrl
is consistent with other static methods in this class, static analysis tools recommend using the class name directly in static contexts for better clarity.static usage() { - return `${this.baseApiUrl}/v1/usage/history`; + return `${ApiUrlService.baseApiUrl}/v1/usage/history`; } static usageStats() { - return `${this.baseApiUrl}/v1/usage/history/stats`; + return `${ApiUrlService.baseApiUrl}/v1/usage/history/stats`; }Note: This change would be most beneficial if applied consistently across all static methods in the class.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apps/api/src/billing/http-schemas/usage.schema.ts
(3 hunks)apps/api/src/billing/repositories/usage/usage.repository.spec.ts
(6 hunks)apps/api/src/billing/services/usage/usage.service.spec.ts
(11 hunks)apps/api/src/billing/services/usage/usage.service.ts
(1 hunks)apps/api/src/deployment/repositories/deployment/deployment.repository.ts
(1 hunks)apps/api/test/functional/usage.spec.ts
(7 hunks)apps/deploy-web/package.json
(1 hunks)apps/deploy-web/src/components/layout/AccountMenu.tsx
(3 hunks)apps/deploy-web/src/components/usage/BillingPage.tsx
(1 hunks)apps/deploy-web/src/components/usage/UsageLayout.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/UsageContainer.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/UsageContainer.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/UsageView.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.tsx
(1 hunks)apps/deploy-web/src/pages/billing/index.tsx
(1 hunks)apps/deploy-web/src/pages/usage/index.tsx
(1 hunks)apps/deploy-web/src/queries/featureFlags.ts
(1 hunks)apps/deploy-web/src/queries/index.ts
(1 hunks)apps/deploy-web/src/queries/queryKeys.ts
(2 hunks)apps/deploy-web/src/queries/useUsageQuery.ts
(1 hunks)apps/deploy-web/src/utils/apiUtils.ts
(1 hunks)apps/deploy-web/tests/seeders/usage.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/api/src/billing/repositories/usage/usage.repository.spec.ts
- apps/deploy-web/src/queries/featureFlags.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- apps/deploy-web/package.json
- apps/deploy-web/src/queries/index.ts
- apps/api/src/deployment/repositories/deployment/deployment.repository.ts
- apps/deploy-web/src/components/layout/AccountMenu.tsx
- apps/deploy-web/src/pages/usage/index.tsx
- apps/api/src/billing/services/usage/usage.service.spec.ts
- apps/api/test/functional/usage.spec.ts
- apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx
- apps/api/src/billing/services/usage/usage.service.ts
- apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx
- apps/deploy-web/src/components/usage/usage-tab/UsageContainer.tsx
- apps/api/src/billing/http-schemas/usage.schema.ts
- apps/deploy-web/src/queries/queryKeys.ts
- apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.tsx
🧰 Additional context used
📓 Path-based instructions (2)
`apps/{deploy-web,provider-console}/**/*.spec.tsx`: Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
apps/{deploy-web,provider-console}/**/*.spec.tsx
: UsequeryBy
methods instead ofgetBy
methods in test expectations in.spec.tsx
files
📄 Source: CodeRabbit Inference Engine (.cursor/rules/query-by-in-tests.mdc)
List of files the instruction was applied to:
apps/deploy-web/src/components/usage/usage-tab/UsageContainer.spec.tsx
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx
apps/deploy-web/src/components/usage/usage-tab/UsageView.spec.tsx
`**/*.spec.{ts,tsx}`: Use `setup` function instead of `beforeEach` in test files...
**/*.spec.{ts,tsx}
: Usesetup
function instead ofbeforeEach
in test files
setup
function must be at the bottom of the rootdescribe
block
setup
function creates an object under test and returns it
setup
function should accept a single parameter with inline type definition
Don't use shared state insetup
function
Don't specify return type ofsetup
function
📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)
List of files the instruction was applied to:
apps/deploy-web/src/components/usage/usage-tab/UsageContainer.spec.tsx
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx
apps/deploy-web/src/components/usage/usage-tab/UsageView.spec.tsx
🧠 Learnings (6)
apps/deploy-web/src/components/usage/usage-tab/UsageContainer.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx (6)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/tests/seeders/usage.ts (5)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
apps/deploy-web/src/queries/useUsageQuery.ts (2)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
apps/deploy-web/src/components/usage/usage-tab/UsageView.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
🧬 Code Graph Analysis (7)
apps/deploy-web/src/components/usage/usage-tab/UsageContainer.spec.tsx (5)
apps/deploy-web/src/queries/useUsageQuery.ts (2)
useUsage
(28-34)useUsageStats
(48-54)apps/deploy-web/src/components/usage/usage-tab/UsageContainer.tsx (2)
UsageContainer
(20-54)ChildrenProps
(7-14)apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx (2)
UsageHistory
(11-20)UsageHistoryStats
(22-27)apps/deploy-web/tests/seeders/usage.ts (2)
buildUsageHistory
(27-33)buildUsageHistoryStats
(35-43)apps/deploy-web/tests/unit/container-testing-child-capturer.tsx (1)
createContainerTestingChildCapturer
(25-27)
apps/deploy-web/src/pages/billing/index.tsx (4)
apps/deploy-web/src/hoc/registered-users-only/registered-users-only.hoc.tsx (1)
RegisteredUsersOnly
(6-22)apps/deploy-web/src/components/usage/BillingPage.tsx (1)
BillingPage
(7-14)apps/deploy-web/src/pages/usage/index.tsx (1)
getServerSideProps
(7-7)apps/deploy-web/src/services/route-protector/index.ts (1)
routeProtector
(6-6)
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx (1)
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.tsx (1)
DailyUsageBarChart
(33-61)
apps/deploy-web/tests/seeders/usage.ts (1)
apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx (2)
UsageHistory
(11-20)UsageHistoryStats
(22-27)
apps/deploy-web/src/queries/useUsageQuery.ts (2)
apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx (2)
UsageHistory
(11-20)UsageHistoryStats
(22-27)apps/deploy-web/src/queries/queryKeys.ts (1)
QueryKeys
(1-86)
apps/deploy-web/src/components/usage/usage-tab/UsageView.spec.tsx (4)
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.tsx (1)
DailyUsageBarChartProps
(28-31)apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx (1)
CumulativeSpendingLineChartProps
(22-25)apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx (2)
UsageView
(38-130)UsageViewProps
(29-36)apps/deploy-web/tests/seeders/usage.ts (2)
buildUsageHistory
(27-33)buildUsageHistoryStats
(35-43)
apps/deploy-web/src/components/usage/UsageLayout.tsx (2)
apps/deploy-web/src/utils/urlUtils.ts (1)
UrlService
(16-80)apps/provider-console/src/utils/styleUtils.ts (1)
cn
(4-6)
🪛 Biome (1.9.4)
apps/deploy-web/src/components/usage/UsageLayout.tsx
[error] 31-31: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
apps/deploy-web/src/utils/apiUtils.ts
[error] 84-84: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 87-87: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🔇 Additional comments (9)
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx (2)
7-20
: Well-structured mocking approach.The mocks for UI components are comprehensive and appropriate for isolating the component under test. The test IDs are properly implemented to enable reliable testing.
22-27
: Appropriate chart mocking strategy.The recharts mocks effectively isolate the component from external chart dependencies while preserving the data flow needed for testing.
apps/deploy-web/src/components/usage/BillingPage.tsx (1)
7-14
: LGTM! Clean implementation with clear placeholder.The component structure is well-organized and follows React best practices. The TODO comment clearly indicates the planned implementation of the transactions list.
apps/deploy-web/src/pages/billing/index.tsx (1)
1-7
: LGTM! Proper access control implementation.The page correctly implements both client-side and server-side access controls using the RegisteredUsersOnly HOC and routeProtector with the appropriate feature flag.
apps/deploy-web/src/components/usage/usage-tab/UsageContainer.spec.tsx (1)
27-113
: LGTM! Excellent test implementation following all coding guidelines.The test correctly implements all the established patterns:
- Uses a
setup
function instead ofbeforeEach
setup
function is positioned at the bottom of the describe blocksetup
function accepts a single parameter with inline type definitionsetup
function creates the object under test and returns it- No shared state in the
setup
function- Comprehensive test coverage of the container's prop passing behavior
apps/deploy-web/src/components/usage/UsageLayout.tsx (1)
23-54
: LGTM! Well-structured component with proper TypeScript typing.The component provides a clean tabbed interface with proper error boundary handling and follows React best practices. The use of enum for tab values and TypeScript typing enhances maintainability.
apps/deploy-web/src/queries/useUsageQuery.ts (1)
10-54
: LGTM! Excellent implementation of React Query hooks.The hooks are well-structured with:
- Proper TypeScript typing and error handling
- Consistent patterns between
useUsage
anduseUsageStats
- Appropriate use of query keys for caching
- Clean date formatting for API consumption
- Proper separation of concerns with dedicated async functions
apps/deploy-web/src/components/usage/usage-tab/UsageView.spec.tsx (1)
43-55
: LGTM! Excellent test setup function implementation.The
setup
function correctly follows all the coding guidelines:
- Positioned at the bottom of the describe block ✓
- Accepts a single parameter with inline type definition ✓
- Doesn't use shared state ✓
- Doesn't specify return type ✓
- Creates an object under test and returns it ✓
apps/deploy-web/tests/seeders/usage.ts (1)
5-43
: LGTM! Well-designed test data seeder implementation.The seeder functions are excellently structured with:
- Proper use of faker.js for realistic test data generation
- Flexible factory pattern supporting partial overrides
- Clean TypeScript typing with proper import of types
- Consistent patterns across all three builder functions
- Appropriate default value ranges for realistic test scenarios
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/usage/usage-tab/UsageView.spec.tsx
Outdated
Show resolved
Hide resolved
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 (2)
apps/deploy-web/src/utils/apiUtils.ts (1)
83-88
: Replacethis
with class name in static methods.The static analysis tool correctly identifies that using
this
in static context can be confusing. For better clarity, use the class name instead.static usage() { - return `${this.baseApiUrl}/v1/usage/history`; + return `${ApiUrlService.baseApiUrl}/v1/usage/history`; } static usageStats() { - return `${this.baseApiUrl}/v1/usage/history/stats`; + return `${ApiUrlService.baseApiUrl}/v1/usage/history/stats`; }The URL construction logic is correct and follows the established pattern.
packages/http-sdk/src/usage/usage-http.service.ts (1)
14-16
: Remove the unnecessary constructor.The constructor only calls
super(config)
without any additional logic, making it redundant.export class UsageHttpService extends HttpService { - constructor(config?: AxiosRequestConfig) { - super(config); - } async getUsage(params: UsageParams): Promise<UsageHistory> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (7)
apps/deploy-web/package.json
(1 hunks)apps/deploy-web/src/queries/useUsageQuery.ts
(1 hunks)apps/deploy-web/src/services/app-di-container/app-di-container.ts
(2 hunks)apps/deploy-web/src/utils/apiUtils.ts
(1 hunks)packages/http-sdk/src/index.ts
(1 hunks)packages/http-sdk/src/usage/usage-http.service.ts
(1 hunks)packages/http-sdk/src/usage/usage.types.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/http-sdk/src/index.ts
- packages/http-sdk/src/usage/usage.types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/deploy-web/package.json
- apps/deploy-web/src/queries/useUsageQuery.ts
🧰 Additional context used
🧠 Learnings (2)
packages/http-sdk/src/usage/usage-http.service.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
🧬 Code Graph Analysis (2)
packages/http-sdk/src/usage/usage-http.service.ts (2)
packages/http-sdk/src/http/http.service.ts (1)
HttpService
(3-15)packages/http-sdk/src/usage/usage.types.ts (2)
UsageHistory
(1-10)UsageHistoryStats
(12-17)
apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
packages/http-sdk/src/usage/usage-http.service.ts (1)
UsageHttpService
(13-41)
🪛 Biome (1.9.4)
packages/http-sdk/src/usage/usage-http.service.ts
[error] 14-16: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
apps/deploy-web/src/utils/apiUtils.ts
[error] 84-84: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 87-87: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
⏰ 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). (12)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (3)
apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
3-11
: Import addition looks good.The
UsageHttpService
import is properly added to the existing import statement.packages/http-sdk/src/usage/usage-http.service.ts (2)
18-32
: Well-structured service methods.The
getUsage
andgetUsageStats
methods are well-implemented:
- Proper async/await usage
- Correct use of inherited
extractData
method- Consistent parameter normalization
- Appropriate type annotations
34-40
: Correct parameter normalization.The
normalizeUsageParams
method correctly handles:
- Required address parameter
- Optional date parameters with proper formatting using
date-fns
- Consistent "y-MM-dd" format for API consumption
apps/deploy-web/src/services/app-di-container/app-di-container.ts
Outdated
Show resolved
Hide resolved
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: 2
🧹 Nitpick comments (5)
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx (2)
10-28
: Good test coverage for core functionality.The test cases effectively cover the main scenarios: loading state, data rendering, and pointer events behavior.
Consider adding tests for edge cases to improve coverage:
- Empty data array handling
- Invalid data formats
- Error states
- Different data ranges
30-51
: Setup function follows guidelines well with minor consideration.The setup function correctly follows the coding guidelines: positioned at the bottom, accepts inline typed parameters, doesn't use shared state, and provides comprehensive mocks.
The function doesn't return the render result, though the guideline suggests it should "create an object under test and return it". However, the current approach using the global
screen
object works effectively for these tests.apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx (2)
63-87
: Consider timezone handling and add error handling for date operations.The
calculateSpendingChange
function has a few potential issues:
- Timezone concerns:
startOfTomorrow()
uses local timezone, which might cause inconsistencies if server data uses UTC.- Unnecessary sorting: The data is sorted on line 75, but if the input data is already chronologically sorted, this adds unnecessary computational overhead.
- Missing error handling: No validation for invalid dates or malformed data.
const calculateSpendingChange = () => { if (data.length < 2) return 0; const tomorrow = startOfTomorrow(); const monthAgo = subDays(tomorrow, 31); const lastMonthData = data .filter(item => { const date = new Date(item.date); + // Add error handling for invalid dates + if (isNaN(date.getTime())) { + console.warn(`Invalid date found: ${item.date}`); + return false; + } return isAfter(date, monthAgo) && isBefore(date, tomorrow); }) - .sort((a, b) => new Date(a.date).getTime() - new Date(b.date).getTime()); + // Remove sorting if data is already chronologically ordered + .sort((a, b) => new Date(a.date).getTime() - new Date(b.date).getTime()); if (lastMonthData.length < 2) return 0; const firstValue = lastMonthData[0].totalUsdSpent; const lastValue = lastMonthData[lastMonthData.length - 1].totalUsdSpent; if (firstValue === 0) return 0; const percentageChange = ((lastValue - firstValue) / firstValue) * 100; return Math.round(percentageChange * 100) / 100; };
63-87
: Consider memoizing thecalculateSpendingChange
function to improve performance.The
calculateSpendingChange
function performs date filtering, sorting, and calculations on every render, which could impact performance with large datasets. Consider usinguseMemo
to memoize the result.+import React, { type FC, useMemo } from "react"; export const CumulativeSpendingLineChart: FC<CumulativeSpendingLineChartProps> = ({ isFetching, data, dependencies = DEPENDENCIES }) => { // ... existing code ... - const calculateSpendingChange = () => { + const spendingChange = useMemo(() => { if (data.length < 2) return 0; const tomorrow = startOfTomorrow(); const monthAgo = subDays(tomorrow, 31); const lastMonthData = data .filter(item => { const date = new Date(item.date); return isAfter(date, monthAgo) && isBefore(date, tomorrow); }) .sort((a, b) => new Date(a.date).getTime() - new Date(b.date).getTime()); if (lastMonthData.length < 2) return 0; const firstValue = lastMonthData[0].totalUsdSpent; const lastValue = lastMonthData[lastMonthData.length - 1].totalUsdSpent; if (firstValue === 0) return 0; const percentageChange = ((lastValue - firstValue) / firstValue) * 100; return Math.round(percentageChange * 100) / 100; - }; + }, [data]); // ... in the render section ... - <TrendIndicator change={calculateSpendingChange()} dependencies={dependencies} /> + <TrendIndicator change={spendingChange} dependencies={dependencies} />apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx (1)
29-55
: Consider adding edge case tests for the 31-day filtering logic.While the current tests verify the trend calculation works correctly, consider adding tests for edge cases:
- Data spanning more than 31 days: Test that only the last 31 days are considered
- Data with gaps: Test behavior when there are missing days in the dataset
- Edge case with zero values: Test behavior when spending values are zero
+ it("only considers data from the last 31 days for trend calculation", () => { + const today = new Date(); + const oldDate = subDays(today, 35); // Outside 31-day window + const recentDate = subDays(today, 10); // Within 31-day window + const data = [ + { date: format(oldDate, "yyyy-MM-dd"), totalUsdSpent: 200 }, + { date: format(recentDate, "yyyy-MM-dd"), totalUsdSpent: 100 } + ]; + setup({ isFetching: false, data }); + // Should not show trend since old data is outside the 31-day window + expect(screen.queryByText(/Trending/)).toBeNull(); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.tsx
(1 hunks)apps/deploy-web/src/pages/billing/index.tsx
(1 hunks)apps/deploy-web/src/pages/usage/index.tsx
(1 hunks)apps/deploy-web/src/services/app-di-container/app-di-container.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/pages/usage/index.tsx
- apps/deploy-web/src/services/app-di-container/app-di-container.ts
- apps/deploy-web/src/components/usage/usage-tab/UsageView.tsx
- apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.tsx
🧰 Additional context used
📓 Path-based instructions (2)
`apps/{deploy-web,provider-console}/**/*.spec.tsx`: Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
apps/{deploy-web,provider-console}/**/*.spec.tsx
: UsequeryBy
methods instead ofgetBy
methods in test expectations in.spec.tsx
files
📄 Source: CodeRabbit Inference Engine (.cursor/rules/query-by-in-tests.mdc)
List of files the instruction was applied to:
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx
`**/*.spec.{ts,tsx}`: Use `setup` function instead of `beforeEach` in test files...
**/*.spec.{ts,tsx}
: Usesetup
function instead ofbeforeEach
in test files
setup
function must be at the bottom of the rootdescribe
block
setup
function creates an object under test and returns it
setup
function should accept a single parameter with inline type definition
Don't use shared state insetup
function
Don't specify return type ofsetup
function
📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)
List of files the instruction was applied to:
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx
🧠 Learnings (2)
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.700Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.700Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
⏰ 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). (11)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (13)
apps/deploy-web/src/pages/billing/index.tsx (3)
6-6
: Excellent use of HOC for client-side protection.The composition pattern with
RegisteredUsersOnly
HOC provides clean client-side authentication protection. This is a well-implemented security pattern for Next.js pages.
8-11
: Excellent server-side protection implementation.The server-side route protection is well-implemented with:
- Defense in depth (both client and server-side protection)
- Proper async/await usage for authentication and feature flag checks
- Route path consistency with file location
- Feature flag name alignment with PR objectives
This provides robust security for the billing functionality.
1-4
: BillingPage import path verifiedThe
BillingPage
component is correctly located atapps/deploy-web/src/components/usage/BillingPage.tsx
and exported asexport const BillingPage
. The import inapps/deploy-web/src/pages/billing/index.tsx
is valid—no changes needed.apps/deploy-web/src/components/usage/usage-tab/charts/DailyUsageBarChart.spec.tsx (2)
1-8
: LGTM! Clean import structure.The imports are well-organized and all necessary for the test implementation.
31-48
: Excellent mock implementations for dependency injection.The mocks are comprehensive and well-designed, properly supporting all test scenarios including:
- Appropriate test-ids for element queries
- Correct prop handling for className and data verification
- Proper component isolation through dependency injection
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx (4)
46-61
: LGTM! Excellent dependency injection implementation.The dependency injection pattern is well-implemented, making the component easily testable and maintainable.
89-130
: LGTM! Proper handling of loading states and accessibility.The component correctly handles loading states with spinner and disabled interactions, and includes proper accessibility attributes.
137-149
: LGTM! Clean trend indicator implementation.The trend indicator properly handles different states and provides clear visual feedback with appropriate icons.
137-149
: The TrendIndicator component logic looks correct.The component properly handles zero change by returning null, and correctly displays the trend direction with appropriate icons and text formatting.
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.spec.tsx (4)
10-56
: LGTM! Excellent adherence to coding guidelines.The test implementation perfectly follows the established coding guidelines:
- ✅ Uses
setup
function instead ofbeforeEach
- ✅ Uses
queryBy
methods instead ofgetBy
methods- ✅ Setup function at the bottom of the describe block
- ✅ No shared state in setup function
- ✅ Setup function accepts single parameter with inline type
- ✅ No return type specified for setup function
The test coverage is comprehensive, covering loading states, data rendering, and trend indicators.
57-84
: LGTM! Well-structured dependency mocking.The dependency mocking is comprehensive and includes all necessary test IDs for proper testing. The setup function correctly handles component instantiation with mocked dependencies.
10-56
: The test suite is comprehensive and follows coding guidelines correctly.The tests cover all the key functionality:
- Spinner visibility during loading
- Chart rendering with data
- Trend indicator behavior for different data scenarios
- Proper use of
queryBy
methods for assertions- Good test data setup with realistic date ranges
57-84
: The setup function is properly implemented according to coding guidelines.The setup function correctly:
- Is positioned at the bottom of the root describe block
- Accepts a single parameter with inline type definition
- Doesn't specify a return type
- Doesn't use shared state
- Uses dependency injection effectively for testing
The mock dependencies are well-structured and provide the necessary test IDs for assertions.
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/usage/usage-tab/charts/CumulativeSpendingLineChart.tsx
Outdated
Show resolved
Hide resolved
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 (13)
apps/deploy-web/src/components/usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx (4)
21-21
: UsegetBy
instead ofqueryBy
for better test debugging.According to the project's testing guidelines, use
getBy
methods instead ofqueryBy
methods when testing element presence withtoBeInTheDocument()
becausegetBy
throws an error and shows DOM state when element is not found, providing better debugging information.- expect(screen.queryByRole("status")).toBeInTheDocument(); + expect(screen.getByRole("status")).toBeInTheDocument();
28-28
: UsegetBy
instead ofqueryBy
for better test debugging.- expect(screen.queryByRole("chart-container")).toHaveClass("pointer-events-none"); + expect(screen.getByRole("chart-container")).toHaveClass("pointer-events-none");
34-34
: Remove unnecessary async/await usage.The
setup
function is not async, so awaiting it is incorrect.- const { lineChartProps } = await setup({ isFetching: false, data: sample }); + const { lineChartProps } = setup({ isFetching: false, data: sample });
36-36
: UsegetBy
instead ofqueryBy
for better test debugging.- expect(screen.queryByRole("chart-container")).not.toHaveClass("pointer-events-none"); + expect(screen.getByRole("chart-container")).not.toHaveClass("pointer-events-none");apps/deploy-web/src/components/usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx (4)
21-21
: UsegetBy
instead ofqueryBy
for better test debugging.According to the project's testing guidelines, use
getBy
methods instead ofqueryBy
methods when testing element presence withtoBeInTheDocument()
becausegetBy
throws an error and shows DOM state when element is not found, providing better debugging information.- expect(screen.queryByRole("status")).toBeInTheDocument(); + expect(screen.getByRole("status")).toBeInTheDocument();
28-28
: UsegetBy
instead ofqueryBy
for better test debugging.- expect(screen.queryByRole("chart-container")).toHaveClass("pointer-events-none"); + expect(screen.getByRole("chart-container")).toHaveClass("pointer-events-none");
36-36
: UsegetBy
instead ofqueryBy
for better test debugging.- expect(screen.queryByRole("chart-container")).not.toHaveClass("pointer-events-none"); + expect(screen.getByRole("chart-container")).not.toHaveClass("pointer-events-none");
40-52
: Fix unnecessary async/await usage.The
setup
function is marked as async but doesn't await anything internally. Therender
function is not async, so awaiting it is incorrect.- async function setup(props: { isFetching: boolean; data: Array<{ date: string; dailyUsdSpent: number }> }) { + function setup(props: { isFetching: boolean; data: Array<{ date: string; dailyUsdSpent: number }> }) { let barChartProps: React.ComponentProps<NonNullable<DailyUsageBarChartProps["dependencies"]>["BarChart"]> = {}; const DEPENDENCIES: DailyUsageBarChartProps["dependencies"] = { BarChart: props => { barChartProps = props; } }; - await render(<DailyUsageBarChart {...props} dependencies={DEPENDENCIES} />); + render(<DailyUsageBarChart {...props} dependencies={DEPENDENCIES} />); return { barChartProps }; }Also update the test calls to remove
await
:- const { barChartProps } = await setup({ isFetching: true, data: sample }); + const { barChartProps } = setup({ isFetching: true, data: sample });- const { barChartProps } = await setup({ isFetching: false, data: sample }); + const { barChartProps } = setup({ isFetching: false, data: sample });apps/deploy-web/src/components/usage/UsaveView/UsageView.spec.tsx (2)
16-16
: UsegetBy
instead ofqueryBy
for better test debugging.According to the project's testing guidelines, use
getBy
methods instead ofqueryBy
methods when testing element presence withtoBeInTheDocument()
becausegetBy
throws an error and shows DOM state when element is not found, providing better debugging information.- expect(screen.queryByText("Error loading usage stats")).toBeInTheDocument(); + expect(screen.getByText("Error loading usage stats")).toBeInTheDocument();
34-51
: UsegetBy
instead ofqueryBy
for better test debugging.- expect(screen.queryByText(usageHistoryStatsData.totalSpent)).toBeInTheDocument(); + expect(screen.getByText(usageHistoryStatsData.totalSpent)).toBeInTheDocument();- expect( - screen.queryByText((_, element) => { - return element?.textContent === "7 average per day"; - }) - ).toBeInTheDocument(); + expect( + screen.getByText((_, element) => { + return element?.textContent === "7 average per day"; + }) + ).toBeInTheDocument();- expect(screen.queryByText("3")).toBeInTheDocument(); + expect(screen.getByText("3")).toBeInTheDocument();- expect( - screen.queryByText((_, element) => { - return element?.textContent === "0.5 average per day"; - }) - ).toBeInTheDocument(); + expect( + screen.getByText((_, element) => { + return element?.textContent === "0.5 average per day"; + }) + ).toBeInTheDocument();- expect(screen.queryByText("Error loading usage data")).toBeInTheDocument(); + expect(screen.getByText("Error loading usage data")).toBeInTheDocument();apps/deploy-web/src/components/usage/DailyUsageBarChart/DailyUsageBarChart.tsx (1)
60-63
: Extract duplicated date parsing logic into a helper function.The date parsing and formatting logic is duplicated in both the tick formatter and label formatter. This creates maintenance overhead and potential inconsistencies.
+const formatDate = (value: string, formatStr: string) => { + const date = new Date(value); + return isNaN(date.getTime()) ? value : format(date, formatStr); +}; + tickFormatter={value => { - const date = new Date(value); - return isNaN(date.getTime()) ? value : format(new Date(value), "M/d"); + return formatDate(value, "M/d"); }} labelFormatter={value => { - const date = new Date(value); - return isNaN(date.getTime()) ? value : format(new Date(value), "MMM d, yyyy"); + return formatDate(value, "MMM d, yyyy"); }}Also applies to: 70-73
apps/deploy-web/src/components/usage/UsageContainer/UsageContainer.tsx (1)
37-42
: Extract default values to a constant for better maintainability.The default values for usage history stats are hardcoded inline, making them harder to maintain and potentially reuse.
+const DEFAULT_USAGE_STATS = { + totalSpent: 0, + averageSpentPerDay: 0, + totalDeployments: 0, + averageDeploymentsPerDay: 0 +}; + const { - data: usageHistoryStatsData = { - totalSpent: 0, - averageSpentPerDay: 0, - totalDeployments: 0, - averageDeploymentsPerDay: 0 - }, + data: usageHistoryStatsData = DEFAULT_USAGE_STATS, isError: isUsageHistoryStatsError, isFetching: isFetchingUsageHistoryStats } = d.useUsageStats({apps/deploy-web/src/components/usage/UsaveView/UsageView.tsx (1)
30-161
: Consider breaking down this component into smaller, focused components.The
UsageView
component has multiple responsibilities: displaying overview stats, handling CSV export, and rendering historical charts. Consider extracting the overview section and CSV export functionality into separate components.This would improve maintainability and testability:
UsageOverview
component for the stats cardsUsageExport
component or custom hook for CSV functionalityUsageHistory
component for the historical charts section
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/deploy-web/src/components/usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx
(1 hunks)apps/deploy-web/src/components/usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/DailyUsageBarChart/DailyUsageBarChart.tsx
(1 hunks)apps/deploy-web/src/components/usage/UsageContainer/UsageContainer.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/UsageContainer/UsageContainer.tsx
(1 hunks)apps/deploy-web/src/components/usage/UsagePage.tsx
(1 hunks)apps/deploy-web/src/components/usage/UsaveView/UsageView.spec.tsx
(1 hunks)apps/deploy-web/src/components/usage/UsaveView/UsageView.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/deploy-web/src/components/usage/UsagePage.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/{deploy-web,provider-console}/**/*.spec.tsx
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/query-by-in-tests.mdc
**/*.spec.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/setup-instead-of-before-each.mdc
🧠 Learnings (4)
apps/deploy-web/src/components/usage/UsageContainer/UsageContainer.spec.tsx (6)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.700Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
apps/deploy-web/src/components/usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.700Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/components/usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.700Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/components/usage/UsaveView/UsageView.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.700Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
⏰ 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). (12)
- GitHub Check: validate (apps/notifications) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (10)
apps/deploy-web/src/components/usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx (1)
40-52
: Setup function follows coding guidelines correctly.The setup function is properly positioned at the bottom of the describe block, accepts a single parameter with inline type definition, doesn't specify a return type, and avoids shared state.
apps/deploy-web/src/components/usage/UsageContainer/UsageContainer.spec.tsx (1)
59-103
: Setup function follows coding guidelines correctly.The setup function is properly positioned at the bottom of the describe block, accepts a single parameter with inline type definition, doesn't specify a return type, and avoids shared state. The mock implementations and test logic are well-structured.
apps/deploy-web/src/components/usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx (3)
55-58
: Ensure robust date handling in tick formatter.The date parsing logic correctly handles invalid dates by falling back to the original value, which is good defensive programming.
65-68
: Ensure robust date handling in label formatter.The date parsing logic in the tooltip label formatter also correctly handles invalid dates by falling back to the original value, maintaining consistency with the tick formatter.
31-87
: Well-structured component with proper accessibility and testing support.The component follows React best practices with proper TypeScript typing, accessibility attributes, and dependency injection for testing. The loading state handling with pointer-events disabling and visual feedback is well-implemented.
apps/deploy-web/src/components/usage/UsaveView/UsageView.spec.tsx (2)
57-68
: queryBy usage is appropriate for attribute checking.The use of
queryBy
methods here is appropriate since these assertions are checking attributes rather than just element presence, so the debugging benefit ofgetBy
doesn't apply.
71-101
: Setup function follows coding guidelines correctly.The setup function is properly positioned at the bottom of the describe block, accepts a single parameter with inline type definition, doesn't specify a return type, and avoids shared state. The mock implementations are well-structured.
apps/deploy-web/src/components/usage/DailyUsageBarChart/DailyUsageBarChart.tsx (1)
35-83
: Well-implemented chart component with good accessibility and loading states.The component follows React best practices with proper TypeScript typing, dependency injection for testability, and accessibility features. The loading state management and conditional styling are well-handled.
apps/deploy-web/src/components/usage/UsageContainer/UsageContainer.tsx (1)
27-61
: Excellent use of render props pattern and dependency injection.The component properly implements the render props pattern for data fetching and state management, with good dependency injection for testability. The error handling and loading states are well-managed.
apps/deploy-web/src/components/usage/UsaveView/UsageView.tsx (1)
70-161
: Excellent error handling and loading states throughout the component.The component properly handles loading and error states for both usage history and stats, with appropriate fallbacks and user feedback. The conditional rendering and accessibility considerations are well-implemented.
...oy-web/src/components/usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx
Outdated
Show resolved
Hide resolved
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 (10)
apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx (1)
40-52
: Remove unnecessary async/await usage from setup function.The
setup
function is marked asasync
and usesawait
on therender
call, but React Testing Library'srender
function is synchronous and doesn't return a Promise. This is unnecessary and should be removed.- async function setup(props: { isFetching: boolean; data: Array<{ date: string; dailyUsdSpent: number }> }) { + function setup(props: { isFetching: boolean; data: Array<{ date: string; dailyUsdSpent: number }> }) { let barChartProps: React.ComponentProps<NonNullable<DailyUsageBarChartProps["dependencies"]>["BarChart"]> = {}; const DEPENDENCIES: DailyUsageBarChartProps["dependencies"] = { BarChart: props => { barChartProps = props; } }; - await render(<DailyUsageBarChart {...props} dependencies={DEPENDENCIES} />); + render(<DailyUsageBarChart {...props} dependencies={DEPENDENCIES} />); return { barChartProps }; }Also update the test calls to remove
await
:- const { barChartProps } = await setup({ isFetching: true, data: sample }); + const { barChartProps } = setup({ isFetching: true, data: sample });- const { barChartProps } = await setup({ isFetching: false, data: sample }); + const { barChartProps } = setup({ isFetching: false, data: sample });apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.tsx (2)
12-31
: CSV utility functions are well-implemented but missing headers for usage history.The CSV escape function correctly handles special characters, and the download function properly creates and cleans up the blob URL. However, the
exportCsv
function doesn't include column headers for the usage history CSV, which would make the exported file less user-friendly.Consider adding headers to the usage history CSV:
const exportCsv = React.useCallback(() => { + const historyHeaders = Object.keys(usageHistoryData[0] || {}).join(","); - const historyCsvContent = usageHistoryData.map(row => Object.values(row).map(escapeCsvValue).join(",")).join("\n"); + const historyCsvContent = [historyHeaders, ...usageHistoryData.map(row => Object.values(row).map(escapeCsvValue).join(","))].join("\n"); downloadCsv(historyCsvContent, "usage_history.csv");
62-80
: Potential runtime error if usageHistoryData is empty.The
exportCsv
function accessesusageHistoryData.map()
without checking if the array is empty. If the array is empty, the CSV export will still work, but it might be better to handle this case explicitly.Add a check for empty data:
const exportCsv = React.useCallback(() => { + if (usageHistoryData.length === 0) { + return; // or show a toast message + } + const historyCsvContent = usageHistoryData.map(row => Object.values(row).map(escapeCsvValue).join(",")).join("\n");apps/deploy-web/src/components/billing-usage/UsageContainer/UsageContainer.tsx (2)
27-27
: Minor naming inconsistency in parameter destructuring.The parameter is destructured as
dependencies: d
but it would be more consistent to use a full descriptive name or stick with the pattern used elsewhere.-export const UsageContainer: FC<UsageContainerProps> = ({ children, dependencies: d = DEPENDENCIES }) => { +export const UsageContainer: FC<UsageContainerProps> = ({ children, dependencies = DEPENDENCIES }) => {And update the usage throughout the component:
- const { address } = d.useWallet(); + const { address } = dependencies.useWallet();
49-60
: Unnecessary React Fragment wrapping.The component returns a single child call, so the React Fragment is unnecessary and can be removed.
return ( - <> - {children({ - usageHistoryData, - usageHistoryStatsData, - isFetchingUsageHistory, - isUsageHistoryError, - isFetchingUsageHistoryStats, - isUsageHistoryStatsError - })} - </> + children({ + usageHistoryData, + usageHistoryStatsData, + isFetchingUsageHistory, + isUsageHistoryError, + isFetchingUsageHistoryStats, + isUsageHistoryStatsError + }) );apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.tsx (2)
60-63
: Potential issue with date formatting in tickFormatter.The
new Date(value)
is called twice - once for the NaN check and once for formatting. This is inefficient and could be optimized.tickFormatter={value => { const date = new Date(value); - return isNaN(date.getTime()) ? value : format(new Date(value), "M/d"); + return isNaN(date.getTime()) ? value : format(date, "M/d"); }}
70-73
: Same date formatting issue in labelFormatter.Similar to the tickFormatter, the date is created twice unnecessarily.
labelFormatter={value => { const date = new Date(value); - return isNaN(date.getTime()) ? value : format(new Date(value), "MMM d, yyyy"); + return isNaN(date.getTime()) ? value : format(date, "MMM d, yyyy"); }}apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx (3)
55-58
: Same date formatting inefficiency as in DailyUsageBarChart.The date is created twice unnecessarily in the tickFormatter.
tickFormatter={value => { const date = new Date(value); - return isNaN(date.getTime()) ? value : format(date, "M/d"); + return isNaN(date.getTime()) ? value : format(date, "M/d"); }}Wait, I made an error. The code already only creates the date once. Let me check again... Actually the code is correct as is.
65-68
: Same date formatting inefficiency in labelFormatter.The date is created twice unnecessarily.
labelFormatter={value => { const date = new Date(value); - return isNaN(date.getTime()) ? value : format(date, "MMM d, yyyy"); + return isNaN(date.getTime()) ? value : format(date, "MMM d, yyyy"); }}Actually, looking at this again, the code is already correct. The date is only created once in each formatter. I was mistaken.
1-1
: Unnecessary "use client" directive.The "use client" directive is typically used in Next.js App Router to mark client components, but this component doesn't seem to require client-side rendering specifically and could potentially be server-side rendered.
Consider removing the "use client" directive unless there's a specific reason this component needs to run on the client:
-"use client"; - import React, { type FC } from "react";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (13)
apps/deploy-web/src/components/billing-usage/BillingUsageLayout.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsageContainer/UsageContainer.spec.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsageContainer/UsageContainer.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsagePage.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.spec.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.tsx
(1 hunks)apps/deploy-web/src/components/layout/AccountMenu.tsx
(3 hunks)apps/deploy-web/src/pages/usage/index.tsx
(1 hunks)apps/deploy-web/src/utils/urlUtils.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/deploy-web/src/utils/urlUtils.ts
- apps/deploy-web/src/components/layout/AccountMenu.tsx
- apps/deploy-web/src/pages/usage/index.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/{deploy-web,provider-console}/**/*.spec.tsx
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/query-by-in-tests.mdc
**/*.spec.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/setup-instead-of-before-each.mdc
🧠 Learnings (4)
apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/components/billing-usage/UsageContainer/UsageContainer.spec.tsx (6)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.spec.tsx (6)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
🧬 Code Graph Analysis (3)
apps/deploy-web/src/components/billing-usage/UsagePage.tsx (4)
apps/provider-console/src/components/layout/Layout.tsx (1)
Layout
(22-43)apps/deploy-web/src/components/billing-usage/BillingUsageLayout.tsx (1)
BillingUsageLayout
(14-21)apps/deploy-web/src/components/billing-usage/UsageContainer/UsageContainer.tsx (1)
UsageContainer
(27-61)apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.tsx (1)
UsageView
(51-173)
apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.spec.tsx (4)
apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.tsx (2)
UsageView
(51-173)UsageViewProps
(41-49)apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.tsx (1)
DailyUsageBarChartProps
(29-33)apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx (1)
CumulativeSpendingLineChartProps
(25-29)apps/deploy-web/tests/seeders/usage.ts (2)
buildUsageHistory
(27-33)buildUsageHistoryStats
(35-43)
apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.tsx (3)
packages/ui/components/charts.tsx (1)
ChartConfig
(11-16)packages/ui/components/spinner.tsx (1)
Spinner
(10-39)apps/provider-console/src/utils/styleUtils.ts (1)
cn
(4-6)
⏰ 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). (12)
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (15)
apps/deploy-web/src/components/billing-usage/BillingUsageLayout.tsx (1)
1-21
: Clean and well-structured layout component implementation.The component provides a consistent layout structure for billing usage pages with proper error boundary handling and title display. The TypeScript typing is appropriate and the implementation follows React best practices.
apps/deploy-web/src/components/billing-usage/UsagePage.tsx (1)
9-17
: Well-structured page component with proper architecture.The component correctly orchestrates the usage UI components using render props pattern and provides proper SEO metadata and layout structure.
apps/deploy-web/src/components/billing-usage/UsageContainer/UsageContainer.spec.tsx (2)
15-57
: Comprehensive test coverage with proper test structure.The test suite covers all the key functionality of the UsageContainer component including data rendering, loading states, error states, and default values. The test cases are well-structured and provide good coverage.
59-103
: Excellent adherence to coding guidelines for test setup.The
setup
function properly follows all the coding guidelines:
- Located at the bottom of the describe block
- Accepts a single parameter with inline type definition
- Doesn't specify return type
- Creates and returns the object under test
- Avoids shared state
The mocking approach is appropriate and the dependency injection pattern is well-implemented.
apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx (2)
10-38
: Well-structured test cases with proper element querying.The test suite correctly uses
queryBy
methods for element queries as required by the coding guidelines. The test cases properly cover the component's behavior in different states (fetching/not fetching) and the ResizeObserver mock is appropriate for chart testing.
40-52
: Excellent adherence to coding guidelines for test setup.The
setup
function follows all coding guidelines:
- Located at the bottom of the describe block
- Accepts a single parameter with inline type definition
- Doesn't specify return type
- Creates and returns the object under test
- Avoids shared state
The dependency injection pattern for mocking the LineChart component is well-implemented.
apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx (1)
10-38
: Well-structured test cases with proper element querying.The test suite correctly uses
queryBy
methods for element queries as required by the coding guidelines. The test cases properly cover the component's behavior in different states and the ResizeObserver mock is appropriate for chart testing.apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.tsx (1)
33-39
: LGTM! Well-structured dependency injection pattern.The dependencies object enables easy testing and maintains good separation of concerns.
apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.spec.tsx (2)
71-101
: LGTM! Setup function follows coding guidelines correctly.The setup function is properly implemented according to the coding guidelines:
- Located at the bottom of the root describe block
- Accepts a single parameter with inline type definition
- Creates and returns the object under test
- Doesn't use shared state
- Doesn't specify return type
16-16
: Follow coding guidelines: UsegetBy
methods instead ofqueryBy
methods.Based on the coding guidelines for
apps/{deploy-web,provider-console}/**/*.spec.tsx
files, you should usegetBy
methods instead ofqueryBy
methods in test expectations.- expect(screen.queryByText("Error loading usage stats")).toBeInTheDocument(); + expect(screen.getByText("Error loading usage stats")).toBeInTheDocument();Apply this change to all similar assertions throughout the file (lines 16, 21, 34, 36, 40, 43, 51, 57, 58, 61, 67, 68).
⛔ Skipped due to learnings
Learnt from: stalniy PR: akash-network/console#1660 File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56 Timestamp: 2025-07-11T10:46:43.711Z Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
apps/deploy-web/src/components/billing-usage/UsageContainer/UsageContainer.tsx (1)
37-42
: Good default values for usage statistics.The default values provide a sensible fallback that prevents runtime errors and ensures the UI renders correctly even when data is unavailable.
apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.tsx (2)
43-51
: Good accessibility implementation.The component properly includes
accessibilityLayer
,role
attributes, and appropriate ARIA labels for screen readers.
77-77
: Effective loading state visualization.The opacity reduction during loading provides good visual feedback to users that the chart is being updated.
apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx (2)
55-68
: LGTM! Date formatting is implemented correctly.The date formatting logic properly handles invalid dates and formats them appropriately for both tick labels and tooltips.
72-81
: Well-configured line chart with proper styling.The line chart configuration includes appropriate stroke width, dot styling, and loading state opacity changes that provide good visual feedback.
apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/billing-usage/UsaveView/UsageView.spec.tsx
Outdated
Show resolved
Hide resolved
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 (2)
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.tsx (1)
12-18
: CSV escaping logic is correct but could be more robust.The CSV escaping function handles basic cases but could benefit from using a well-established CSV library for better reliability and edge case handling.
Consider using a CSV library like
csv-stringify
orpapaparse
for more robust CSV generation:+import { stringify } from 'csv-stringify/sync'; -const escapeCsvValue = (value: string | number): string => { - const stringValue = String(value); - if (stringValue.includes(",") || stringValue.includes('"') || stringValue.includes("\n")) { - return `"${stringValue.replace(/"/g, '""')}"`; - } - return stringValue; -};apps/deploy-web/src/components/billing-usage/UsageView/UsageView.spec.tsx (1)
25-47
: Test assertions could be more specific and robust.The text matching in assertions could be fragile and might break with formatting changes.
Consider using more specific selectors and data attributes:
- expect(screen.queryByText(usageHistoryStatsData.totalSpent)).toBeInTheDocument(); + expect(screen.queryByText(usageHistoryStatsData.totalSpent.toString())).toBeInTheDocument(); - expect( - screen.queryByText((_, element) => { - return element?.textContent === "7 average per day"; - }) - ).toBeInTheDocument(); + expect(screen.queryByText(/7.*average per day/)).toBeInTheDocument();Or better yet, add data-testid attributes to the component for more reliable testing:
// In UsageView.tsx - <FormattedNumber value={usageHistoryStatsData.averageSpentPerDay} style="currency" currency="USD" currencyDisplay="narrowSymbol" /> average - per day + <span data-testid="average-spent-per-day"> + <FormattedNumber value={usageHistoryStatsData.averageSpentPerDay} style="currency" currency="USD" currencyDisplay="narrowSymbol" /> average + per day + </span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/deploy-web/src/components/billing-usage/BillingUsageLayout.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsagePage.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsageView/UsageView.spec.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsageView/UsageView.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/deploy-web/src/components/billing-usage/BillingUsageLayout.tsx
- apps/deploy-web/src/components/billing-usage/UsagePage.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/{deploy-web,provider-console}/**/*.spec.tsx
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/query-by-in-tests.mdc
**/*.spec.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/setup-instead-of-before-each.mdc
🧠 Learnings (1)
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.spec.tsx (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: stalniy
PR: akash-network/console#1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
⏰ 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). (11)
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (6)
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.tsx (3)
37-43
: Good use of dependency injection for testing.The dependency injection pattern allows for easy mocking in tests while keeping the component flexible.
33-35
: Robust number validation logic.The
isValidNumber
function properly handles null, undefined, NaN, and infinite values.
116-122
: Consistent data validation and fallback handling.The component properly validates numbers before displaying them and provides appropriate fallbacks for invalid data.
Also applies to: 143-149
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.spec.tsx (3)
72-86
: Good mock implementation for dependencies.The mock dependencies are well-structured and provide the necessary functionality for testing while keeping the tests focused.
88-97
: Excellent default props handling.The default props setup allows for easy test customization while providing sensible defaults.
14-17
: Switch to getBy for presence assertions in UsageView.spec.tsx*To align with our testing guideline of using getBy* when asserting an element toBeInTheDocument (for better error output), update the two error-message tests:
– Lines 14–17:
-it("renders an error message when stats fail to load", () => { - setup({ isUsageHistoryStatsError: true }); - expect(screen.queryByText("Error loading usage stats")).toBeInTheDocument(); -}); +it("renders an error message when stats fail to load", () => { + setup({ isUsageHistoryStatsError: true }); + expect(screen.getByText("Error loading usage stats")).toBeInTheDocument(); +});– Lines 49–52:
-it("renders an error message when data fail to load", () => { - setup({ isUsageHistoryDataError: true }); - expect(screen.queryByText("Error loading usage data")).toBeInTheDocument(); -}); +it("renders an error message when data fail to load", () => { + setup({ isUsageHistoryDataError: true }); + expect(screen.getByText("Error loading usage data")).toBeInTheDocument(); +});⛔ Skipped due to learnings
Learnt from: stalniy PR: akash-network/console#1660 File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56 Timestamp: 2025-07-11T10:46:43.711Z Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-06-30T12:11:58.081Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.spec.tsx
Outdated
Show resolved
Hide resolved
...rc/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx
Outdated
Show resolved
Hide resolved
...rc/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.spec.tsx
Outdated
Show resolved
Hide resolved
|
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: 2
🧹 Nitpick comments (1)
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.tsx (1)
102-106
: Consider more specific error handling.The current error messages are quite generic and don't provide users with actionable information about what went wrong or how to resolve the issue.
Consider providing more specific error messages or recovery options:
{isUsageHistoryStatsError && ( <div className="mt-4 flex h-full items-center justify-center"> - <p className="text-red-500">Error loading usage stats</p> + <div className="text-center"> + <p className="text-red-500">Unable to load usage statistics</p> + <p className="text-sm text-gray-500 mt-1">Please try refreshing the page or contact support if the problem persists</p> + </div> </div> )}{isUsageHistoryError && ( <div className="mt-4 flex h-full items-center justify-center"> - <p className="text-red-500">Error loading usage data</p> + <div className="text-center"> + <p className="text-red-500">Unable to load usage history</p> + <p className="text-sm text-gray-500 mt-1">Please try refreshing the page or contact support if the problem persists</p> + </div> </div> )}Also applies to: 168-172
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (7)
apps/deploy-web/package.json
(1 hunks)apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsageView/UsageView.spec.tsx
(1 hunks)apps/deploy-web/src/components/billing-usage/UsageView/UsageView.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/deploy-web/package.json
- apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.spec.tsx
- apps/deploy-web/src/components/billing-usage/UsageView/UsageView.spec.tsx
- apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.spec.tsx
- apps/deploy-web/src/components/billing-usage/DailyUsageBarChart/DailyUsageBarChart.tsx
- apps/deploy-web/src/components/billing-usage/CumulativeSpendingLineChart/CumulativeSpendingLineChart.tsx
⏰ 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). (15)
- GitHub Check: validate (apps/provider-console) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Validate local packages
🔇 Additional comments (2)
apps/deploy-web/src/components/billing-usage/UsageView/UsageView.tsx (2)
12-31
: CSV utility functions are well-implemented.The
escapeCsvValue
anddownloadCsv
functions correctly handle CSV escaping and browser download functionality following standard practices.
37-43
: Excellent component architecture and type safety.The dependency injection pattern with the
COMPONENTS
object enhances testability, and the comprehensive TypeScript types ensure type safety. The component properly handles all necessary states and follows React best practices.Also applies to: 45-53, 55-65
Updates #1348
Screenhosts
Note: for the scope of this PR, the Billing page and tabs layout are now removed completely from the UI; will be added back in a follow-up PR
/billing (will be done in a separate PR)
/usage
These screenshots do not include "Export CSV" button which has been added to the right of "Overview" and exports all data visible on the page for the selected period
Loading states
With data
Error states
To do:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores
recharts
library for charting capabilities.