feat(billing): add composable guard for usage page#1710
feat(billing): add composable guard for usage page#1710baktun14 merged 5 commits intoakash-network:mainfrom
Conversation
WalkthroughThe changes introduce a new higher-order component (HOC) system for access control, featuring a generic Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UsagePage
participant BillingPage
participant GuardHOC
participant useIsManagedWalletUser
participant useIsRegisteredUser
User->>GuardHOC: Request UsagePage or BillingPage
GuardHOC->>useIsManagedWalletUser: Check managed wallet
GuardHOC->>useIsRegisteredUser: Check registered user
alt Both checks pass
GuardHOC->>UsagePage: Render UsagePage
GuardHOC->>BillingPage: Render BillingPage
else Any check fails
GuardHOC->>User: Render fallback (404)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Files:
🧠 Learnings (2)📓 Common learningsapps/deploy-web/src/pages/billing/index.tsx (1)Learnt from: CR 🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/deploy-web/src/hoc/guard/guard.hoc.tsx (1)
21-23: Consider adding input validation for the composeGuards utility.The implementation is clean but could benefit from runtime validation to ensure all guards are functions.
export const composeGuards = (...guards: (() => boolean)[]) => { + if (guards.some(guard => typeof guard !== 'function')) { + throw new Error('All guards must be functions'); + } return () => guards.every(guard => guard()); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/deploy-web/src/components/layout/AccountMenu.tsx(3 hunks)apps/deploy-web/src/hoc/guard/guard.hoc.tsx(1 hunks)apps/deploy-web/src/pages/usage/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/deploy-web/src/components/layout/AccountMenu.tsx (1)
Learnt from: baktun14
PR: akash-network/console#1432
File: apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentCloseAlert.tsx:38-38
Timestamp: 2025-06-05T21:07:51.985Z
Learning: The ContactPointSelect component in apps/deploy-web/src/components/alerts/ContactPointSelectForm/ContactPointSelect.tsx uses the useFormContext hook internally to connect to React Hook Form, so it doesn't need to be wrapped in a FormField component.
🧬 Code Graph Analysis (2)
apps/deploy-web/src/components/layout/AccountMenu.tsx (1)
apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (1)
useWallet(352-354)
apps/deploy-web/src/pages/usage/index.tsx (2)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(8-24)apps/deploy-web/src/hoc/guard/guard.hoc.tsx (2)
Guard(3-19)composeGuards(21-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (8)
apps/deploy-web/src/hoc/guard/guard.hoc.tsx (1)
3-19: Well-implemented Guard HOC with proper React patterns.The Guard HOC implementation follows React best practices with proper generic typing, display name preservation, and clean conditional rendering logic.
apps/deploy-web/src/components/layout/AccountMenu.tsx (3)
17-17: LGTM: Proper import of useWallet hook.The import is correctly placed and follows the existing import pattern.
29-29: LGTM: Clean wallet context usage.The wallet context is properly initialized and follows the existing pattern.
95-99: Excellent consistency with page-level access controls.The menu item gating logic now perfectly aligns with the composable guards used in the usage page, ensuring users only see navigation options they can actually access.
apps/deploy-web/src/pages/usage/index.tsx (4)
2-4: LGTM: Clean imports for the new guard system.All necessary dependencies are properly imported for the composable guard implementation.
8-11: LGTM: Well-implemented managed wallet check.The hook correctly extracts and returns the
isManagedproperty from the wallet context.
13-16: LGTM: Proper registered user validation.The hook correctly checks for the presence of
userIdto determine if a user is registered, using proper boolean coercion.
18-18: Excellent composable guard implementation.The migration from a single guard to a composable system is well-executed. The two guards (
useIsManagedWalletUseranduseIsRegisteredUser) are properly composed and will only allow access when both conditions are met.
ygrishajev
left a comment
ygrishajev
left a comment
There was a problem hiding this comment.
I think it wouldn't take much effort to achieve required level of coverage for this PR. Please add some
domhhv
force-pushed
the
feat/billing-usage-page-guard
branch
from
669e177 to
359b7dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/deploy-web/src/hooks/useUser.spec.tsx(1 hunks)apps/deploy-web/tests/seeders/user.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/tests/seeders/user.tsapps/deploy-web/src/hooks/useUser.spec.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/tests/seeders/user.tsapps/deploy-web/src/hooks/useUser.spec.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/deploy-web/src/hooks/useUser.spec.tsx
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/hooks/useUser.spec.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: baktun14
PR: akash-network/console#1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
apps/deploy-web/tests/seeders/user.ts (4)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
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-07-21T08:25:07.474Z
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-07-21T08:24:27.953Z
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-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : setup function should accept a single parameter with inline type definition
apps/deploy-web/src/hooks/useUser.spec.tsx (9)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
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/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use jest.mock() to mock dependencies in test files. Instead, use jest-mock-extended to create mocks and pass mocks as dependencies to the service under test.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
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-07-21T08:24:27.953Z
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-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in setup function
Learnt from: stalniy
PR: #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-07-21T08:25:07.474Z
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-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : setup function must be at the bottom of the root describe block in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of setup function
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (3)
apps/deploy-web/src/hooks/useUser.spec.tsx (2)
55-70: LGTM - Well-structured test cases.The test cases appropriately cover both scenarios for the
useIsRegisteredUserhook - when userId is present and when it's falsy. The test logic and assertions are correct.
22-32: Fix incorrect mock implementation.The mock calls using
mock<typeof moduleType>()don't actually mock the modules. This syntax creates mock objects but doesn't replace the actual module imports.You need to properly mock the hooks. Consider using
jest.doMock()or restructure the mocking approach:- mock<typeof useCustomUserModule.useCustomUser>(() => ({ - user: customUser || buildUser(), - isLoading: false, - error: undefined, - checkSession: mock() - })); - - mock<typeof useStoredAnonymousUserModule.useStoredAnonymousUser>(() => ({ - user: anonymousUser || buildAnonymousUser(), - isLoading: false - })); + jest.doMock("@src/hooks/useCustomUser", () => ({ + useCustomUser: jest.fn(() => ({ + user: customUser || buildUser(), + isLoading: false, + error: undefined, + checkSession: jest.fn() + })) + })); + + jest.doMock("@src/hooks/useStoredAnonymousUser", () => ({ + useStoredAnonymousUser: jest.fn(() => ({ + user: anonymousUser || buildAnonymousUser(), + isLoading: false + })) + }));⛔ Skipped due to learnings
Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/no-jest-mock.mdc:0-0 Timestamp: 2025-07-21T08:24:24.269Z Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.Learnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test filesLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` functionLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns itLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block in test filesLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` functionLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/setup-instead-of-before-each.mdc:0-0 Timestamp: 2025-07-21T08:25:07.474Z Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definitionLearnt from: CR PR: akash-network/console#0 File: .cursor/rules/query-by-in-tests.mdc:0-0 Timestamp: 2025-07-21T08:24:27.953Z Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` filesapps/deploy-web/tests/seeders/user.ts (1)
7-40: Excellent seeder implementation.The factory functions are well-structured with:
- Proper TypeScript typing with partial overrides support
- Consistent use of faker for realistic test data generation
- Good separation between registered and anonymous user data
- Clean, reusable design that will serve the testing needs well
| planCode: "COMMUNITY", | ||
| plan: plans.find(plan => plan.code === "COMMUNITY"), | ||
| ...overrides |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential undefined plan resolution.
The plans.find() call could return undefined if no plan with code "COMMUNITY" exists, which might not match the expected type for CustomUserProfile.plan.
Consider adding a fallback or assertion to ensure the plan is found:
- planCode: "COMMUNITY",
- plan: plans.find(plan => plan.code === "COMMUNITY"),
+ planCode: "COMMUNITY",
+ plan: plans.find(plan => plan.code === "COMMUNITY") || plans[0],Or use a more explicit approach:
- planCode: "COMMUNITY",
- plan: plans.find(plan => plan.code === "COMMUNITY"),
+ planCode: "COMMUNITY",
+ plan: (() => {
+ const plan = plans.find(p => p.code === "COMMUNITY");
+ if (!plan) throw new Error("COMMUNITY plan not found in test setup");
+ return plan;
+ })(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| planCode: "COMMUNITY", | |
| plan: plans.find(plan => plan.code === "COMMUNITY"), | |
| ...overrides | |
| planCode: "COMMUNITY", | |
| plan: plans.find(plan => plan.code === "COMMUNITY") || plans[0], | |
| ...overrides |
| planCode: "COMMUNITY", | |
| plan: plans.find(plan => plan.code === "COMMUNITY"), | |
| ...overrides | |
| planCode: "COMMUNITY", | |
| plan: (() => { | |
| const plan = plans.find(p => p.code === "COMMUNITY"); | |
| if (!plan) throw new Error("COMMUNITY plan not found in test setup"); | |
| return plan; | |
| })(), | |
| ...overrides |
🤖 Prompt for AI Agents
In apps/deploy-web/tests/seeders/user.ts around lines 21 to 23, the plans.find
call may return undefined if no plan with code "COMMUNITY" exists, which can
cause type errors or runtime issues. To fix this, add a fallback value or an
assertion to ensure the plan variable is never undefined before assigning it to
the plan property. For example, throw an error if the plan is not found or
provide a default plan object to guarantee the plan property is always valid.
domhhv
left a comment
domhhv
left a comment
There was a problem hiding this comment.
Sure, added the tests
* feat(billing): add composable guard for usage page * refactor: move custom hooks * test: cover useIsRegisteredUser hook * feat: guard billing route
3 participants
Updates #1348
Summary by CodeRabbit
New Features
Refactor
Tests