refactor: moves analyticsService creation to DI container #2418
Conversation
📝 WalkthroughWalkthroughRewires components to obtain analytics/url/stripe services from the DI container via useServices(), introduces publicConfig in the DI container, updates AnalyticsService and StripeService constructor shapes, removes the deprecated analytics singleton, and adapts tests and DI wiring accordingly. No UI/control-flow behavior changes. Changes
Sequence Diagram(s)(omitted — refactor/DI wiring without new multi-component control-flow features) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
stalniy
force-pushed
the
refactor/analytics-service
branch
from
6cecef4 to
feac91b
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2418 +/- ##
==========================================
- Coverage 51.29% 50.94% -0.36%
==========================================
Files 1071 1061 -10
Lines 29328 29005 -323
Branches 6474 6436 -38
==========================================
- Hits 15044 14776 -268
+ Misses 13873 13818 -55
Partials 411 411
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/deploy-web/src/components/liquidity-modal/index.tsx (1)
133-146: MissinganalyticsServiceinuseMemodependency array.The
tabsConfigmemoization usesanalyticsServiceinsideonTxnComplete, but it's not listed in the dependency array. This could lead to stale closures if the service reference ever changes.🔎 Proposed fix
- }, [refreshBalances]); + }, [refreshBalances, analyticsService]);
🧹 Nitpick comments (5)
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx (1)
58-58: UsequeryByTextinstead ofgetByTextin test expectations.The test uses
getByTextin an expectation, but the coding guidelines specify thatqueryBymethods should be used instead ofgetBymethods in test expectations for this path pattern.🔎 Proposed fix
- expect(getByText(`${provider.ipRegionCode}, ${provider.ipCountryCode}`)).toBeInTheDocument(); + const { queryByText } = setup({ + bid, + provider, + components + }); + expect(queryByText(`${provider.ipRegionCode}, ${provider.ipCountryCode}`)).toBeInTheDocument();Note: You'll also need to update the destructuring at line 38 to use
queryByTextinstead ofgetByText.As per coding guidelines, use
queryBymethods in test expectations.apps/deploy-web/src/components/settings/ExportCertificate.tsx (1)
13-23: Consider addinganalyticsServiceto the dependency array.With
analyticsServicenow coming fromuseServices(), it's technically a dependency of this effect. However, since the service instance is stable (coming from context), the current behavior is safe. The existing eslint-disable comment suggests this was intentional.Also, the
init()wrapper function isn't needed sinceanalyticsService.track()isn't async:🔎 Optional simplification
useEffect(() => { - async function init() { - analyticsService.track("export_certificate", { - category: "certificates", - label: "Export certificate" - }); - } - - init(); + analyticsService.track("export_certificate", { + category: "certificates", + label: "Export certificate" + }); // eslint-disable-next-line react-hooks/exhaustive-deps }, []);apps/deploy-web/src/services/stripe/stripe.service.ts (1)
26-30: Type and runtime check are inconsistent.The type declares
publishableKey: string(always present), yet the runtime guard checks for a falsy value. This inconsistency exists because the container uses a non-null assertion. Consider aligning the type with runtime expectations:🔎 Option: Reflect optional nature in type
export interface StripeServiceDependencies { loadStripe?: typeof loadStripe; config: { - publishableKey: string; + publishableKey: string | undefined; }; }This would make the runtime guard type-correct and clarify to callers that an undefined key is acceptable.
apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
7-7: UseStripeService.namefor the root describe block.As per coding guidelines, use
StripeService.nameinstead of the hardcoded string to enable automated refactoring tools to find all references.🔎 Suggested fix
-describe("StripeService", () => { +describe(StripeService.name, () => {apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx (1)
378-378: Double type assertion suggests type mismatch.The
as unknown as typeof TransactionMessageDatapattern works but indicates a type compatibility issue between the mock and the actual type. Consider usingjest-mock-extended'smock<typeof TransactionMessageData>()for cleaner typing, though the current approach is functional.🔎 Optional: Cleaner mock typing approach
Consider this alternative at the mock definition (around line 344):
- const mockTransactionMessageData = { - prototype: {}, - getRevokeCertificateMsg: jest.fn(), - getCreateCertificateMsg: jest.fn(), - getCreateLeaseMsg: jest.fn(), - getCreateDeploymentMsg: jest.fn(), - getUpdateDeploymentMsg: jest.fn(), - getDepositDeploymentMsg: jest.fn(), - getCloseDeploymentMsg: jest.fn(), - getSendTokensMsg: jest.fn(), - getGrantMsg: jest.fn(), - getRevokeDepositMsg: jest.fn(), - getGrantBasicAllowanceMsg: jest.fn(), - getRevokeAllowanceMsg: jest.fn(), - getUpdateProviderMsg: jest.fn() - }; + const mockTransactionMessageData = { + getRevokeCertificateMsg: jest.fn(), + getCreateCertificateMsg: jest.fn(), + getCreateLeaseMsg: jest.fn(), + getCreateDeploymentMsg: jest.fn(), + getUpdateDeploymentMsg: jest.fn(), + getDepositDeploymentMsg: jest.fn(), + getCloseDeploymentMsg: jest.fn(), + getSendTokensMsg: jest.fn(), + getGrantMsg: jest.fn(), + getRevokeDepositMsg: jest.fn(), + getGrantBasicAllowanceMsg: jest.fn(), + getRevokeAllowanceMsg: jest.fn(), + getUpdateProviderMsg: jest.fn() + } as typeof TransactionMessageData;Then simplify line 378:
- TransactionMessageData: mockTransactionMessageData as unknown as typeof TransactionMessageData + TransactionMessageData: mockTransactionMessageData
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
apps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsxapps/deploy-web/src/components/authorizations/AllowanceModal.tsxapps/deploy-web/src/components/authorizations/GrantModal.tsxapps/deploy-web/src/components/deployments/DeploymentDepositModal.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/deployments/DeploymentSubHeader.tsxapps/deploy-web/src/components/deployments/ShellDownloadModal.tsxapps/deploy-web/src/components/liquidity-modal/index.tsxapps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit.tsxapps/deploy-web/src/components/new-deployment/TemplateList.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/deploy-web/src/components/sdl/ImportSdlModal.tsxapps/deploy-web/src/components/sdl/RentGpusForm.tsxapps/deploy-web/src/components/sdl/SaveTemplateModal.tsxapps/deploy-web/src/components/settings/ExportCertificate.tsxapps/deploy-web/src/components/templates/UserTemplate.tsxapps/deploy-web/src/components/user/AddFundsLink.tsxapps/deploy-web/src/components/user/UserFavorites.tsxapps/deploy-web/src/components/user/UserProfile.tsxapps/deploy-web/src/components/user/UserProfileLayout.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/context/WalletProvider/WalletProvider.tsxapps/deploy-web/src/hooks/useStoredAnonymousUser.tsapps/deploy-web/src/hooks/useTrialBalance.tsapps/deploy-web/src/pages/user/api-keys/index.tsxapps/deploy-web/src/services/analytics/analytics.service.tsapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/services/app-di-container/browser-di-container.tsapps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/services/stripe/stripe.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsxapps/deploy-web/src/hooks/useStoredAnonymousUser.tsapps/deploy-web/src/components/user/UserFavorites.tsxapps/deploy-web/src/components/user/AddFundsLink.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/deployments/ShellDownloadModal.tsxapps/deploy-web/src/hooks/useTrialBalance.tsapps/deploy-web/src/components/user/UserProfileLayout.tsxapps/deploy-web/src/components/liquidity-modal/index.tsxapps/deploy-web/src/pages/user/api-keys/index.tsxapps/deploy-web/src/components/user/UserProfile.tsxapps/deploy-web/src/services/app-di-container/browser-di-container.tsapps/deploy-web/src/components/deployments/DeploymentSubHeader.tsxapps/deploy-web/src/components/settings/ExportCertificate.tsxapps/deploy-web/src/components/templates/UserTemplate.tsxapps/deploy-web/src/components/authorizations/AllowanceModal.tsxapps/deploy-web/src/services/stripe/stripe.service.tsapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/components/authorizations/GrantModal.tsxapps/deploy-web/src/context/WalletProvider/WalletProvider.tsxapps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/components/new-deployment/TemplateList.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/sdl/ImportSdlModal.tsxapps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/components/sdl/RentGpusForm.tsxapps/deploy-web/src/components/deployments/DeploymentDepositModal.tsxapps/deploy-web/src/components/sdl/SaveTemplateModal.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsxapps/deploy-web/src/services/analytics/analytics.service.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsxapps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations
Files:
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: stalniy
Repo: akash-network/console PR: 2255
File: apps/api/src/middlewares/privateMiddleware.ts:5-9
Timestamp: 2025-11-19T16:13:43.249Z
Learning: In the Akash Console API (apps/api), avoid resolving DI container dependencies at module scope (module initialization time) to prevent side effects. Instead, resolve dependencies inside functions/methods where they are actually used, even if this means resolving on every invocation, to maintain explicit control over when side effects occur and improve testability.
📚 Learning: 2025-06-19T16:00:05.428Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 1512
File: apps/deploy-web/src/components/deployments/DeploymentBalanceAlert/DeploymentBalanceAlert.tsx:47-68
Timestamp: 2025-06-19T16:00:05.428Z
Learning: In React Hook Form setups with zod validation, child components using useFormContext() can rely on parent form validation rather than implementing local input validation. Invalid inputs like NaN from parseFloat() are handled by the parent schema validation, eliminating the need for additional local validation in child components.
Applied to files:
apps/deploy-web/src/components/deployments/ShellDownloadModal.tsx
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/deploy-web/src/services/app-di-container/browser-di-container.tsapps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx
📚 Learning: 2025-05-28T20:42:58.200Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 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.
Applied to files:
apps/deploy-web/src/services/app-di-container/browser-di-container.ts
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
Applied to files:
apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsxapps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
Applied to files:
apps/deploy-web/src/services/stripe/stripe.service.spec.ts
📚 Learning: 2025-06-05T21:07:51.985Z
Learnt from: baktun14
Repo: akash-network/console PR: 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.
Applied to files:
apps/deploy-web/src/components/sdl/RentGpusForm.tsx
🧬 Code graph analysis (30)
apps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/hooks/useStoredAnonymousUser.ts (2)
apps/api/src/billing/config/index.ts (1)
appConfig(3-5)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/user/UserFavorites.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/user/AddFundsLink.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/deployments/DeploymentListRow.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/deployments/ShellDownloadModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/hooks/useTrialBalance.ts (2)
apps/api/src/billing/config/index.ts (1)
appConfig(3-5)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/user/UserProfileLayout.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/liquidity-modal/index.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/pages/user/api-keys/index.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/user/UserProfile.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/services/app-di-container/browser-di-container.ts (1)
apps/deploy-web/src/services/app-di-container/server-di-container.service.ts (1)
services(24-54)
apps/deploy-web/src/components/deployments/DeploymentSubHeader.tsx (2)
apps/api/src/billing/config/index.ts (1)
appConfig(3-5)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/settings/ExportCertificate.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/templates/UserTemplate.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/authorizations/AllowanceModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx (2)
apps/api/src/billing/config/index.ts (1)
appConfig(3-5)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/authorizations/GrantModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
apps/api/src/billing/config/index.ts (1)
appConfig(3-5)
apps/deploy-web/src/services/app-di-container/app-di-container.ts (3)
apps/deploy-web/src/config/browser-env.config.ts (1)
browserEnvConfig(4-41)apps/deploy-web/src/services/stripe/stripe.service.ts (1)
StripeService(10-44)apps/deploy-web/src/services/analytics/analytics.service.ts (1)
AnalyticsService(142-271)
apps/deploy-web/src/components/new-deployment/TemplateList.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/sdl/ImportSdlModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
apps/deploy-web/src/services/stripe/stripe.service.ts (2)
StripeServiceDependencies(3-8)StripeService(10-44)
apps/deploy-web/src/components/deployments/DeploymentDepositModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/sdl/SaveTemplateModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/new-deployment/ManifestEdit.tsx (2)
apps/api/src/billing/config/index.ts (1)
appConfig(3-5)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx (1)
apps/deploy-web/src/utils/TransactionMessageData.ts (1)
TransactionMessageData(12-234)
apps/deploy-web/src/services/analytics/analytics.service.ts (2)
apps/api/src/core/providers/amplitude.provider.ts (1)
Amplitude(16-20)apps/deploy-web/src/components/layout/CustomGoogleAnalytics.tsx (1)
GoogleAnalytics(7-17)
🪛 Biome (2.1.2)
apps/deploy-web/src/services/analytics/analytics.service.ts
[error] 155-155: This parameter is used before its declaration.
The parameter is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ 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 (40)
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx (2)
2-2: LGTM! Proper use of jest-mock-extended for analytics service mocking.The imports correctly use
jest-mock-extendedfor creating mocks and importAnalyticsServiceas a type-only import, which aligns with TypeScript best practices and the coding guidelines.Also applies to: 6-6
114-114: LGTM! Correct implementation of analyticsService mock injection.The addition of
analyticsService: () => mock<AnalyticsService>()to the TestContainerProvider services properly implements the DI pattern for analytics service in tests. This follows the established pattern used for other services and ensures test isolation by creating a fresh mock for each test.apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx (2)
15-15: LGTM! Correct implementation of DI pattern.The change correctly moves
appConfigresolution from module scope into the component, accessing it viauseServices()from the DI container. The aliaspublicConfig: appConfigmaintains compatibility with the existing code below.Based on learnings, this approach avoids uncontrolled side effects at module initialization time and improves testability.
31-47: LGTM! Services correctly injected via DI container.The
UserTrackercomponent properly retrievesanalyticsServiceanduserTrackerfrom the DI container and includes them in theuseEffectdependency array (line 44). This ensures the effect re-runs when services change and aligns with the PR's objective of movinganalyticsServiceto the DI container.apps/deploy-web/src/components/user/AddFundsLink.tsx (1)
6-6: LGTM! Clean migration to context-provided analytics service.The change correctly moves analyticsService resolution from module scope to inside the component using
useServices(), which aligns with the DI best practices. This improves testability and reduces uncontrolled side effects.Based on learnings, resolving dependencies inside components rather than at module scope maintains explicit control over side effects.
Also applies to: 17-17, 28-28
apps/deploy-web/src/components/sdl/RentGpusForm.tsx (1)
49-49: LGTM! Configuration source updated correctly.The change from
appConfigtopublicConfig: appConfigaligns with the container refactoring. The aliasing maintains compatibility with existing code while centralizing configuration access through the DI container.apps/deploy-web/src/hooks/useStoredAnonymousUser.ts (1)
19-19: LGTM! Hook correctly updated to use publicConfig.The change maintains the same functionality while sourcing configuration from the centralized DI container. The aliasing preserves compatibility with existing usage.
apps/deploy-web/src/hooks/useTrialBalance.ts (1)
20-20: LGTM! Configuration access properly centralized.The hook correctly accesses trial configuration through the DI container's
publicConfig, maintaining existing functionality while following the refactored pattern.apps/deploy-web/src/components/deployments/ShellDownloadModal.tsx (1)
8-8: LGTM! Analytics service properly injected via context.The migration correctly moves analytics tracking to use the context-provided service, improving testability and reducing module-level side effects.
Also applies to: 33-33, 51-54
apps/deploy-web/src/components/deployments/DeploymentDepositModal.tsx (1)
12-12: LGTM! Multiple analytics tracking points successfully migrated.All analytics tracking calls now use the context-provided service. The refactoring maintains existing behavior while properly centralizing service access through the DI container.
Also applies to: 51-51, 70-70, 87-87
apps/deploy-web/src/components/user/UserFavorites.tsx (1)
6-6: LGTM! Analytics tracking correctly refactored.The component properly retrieves analyticsService from the DI container and uses it for user profile interaction tracking, following the established pattern.
Also applies to: 12-12, 38-41
apps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsx (1)
10-10: LGTM! API key creation analytics properly migrated.The modal correctly uses the context-provided analytics service for tracking API key creation, completing the consistent pattern of DI-based service access across the codebase.
Also applies to: 30-30, 89-92
apps/deploy-web/src/components/new-deployment/TemplateList.tsx (1)
11-11: LGTM!The refactor correctly moves
analyticsServicefrom a static module import to the DI container viauseServices(). This aligns with the retrieved learning about avoiding module-scope DI resolution to prevent uncontrolled side effects and improve testability.Also applies to: 47-47
apps/deploy-web/src/components/deployments/DeploymentListRow.tsx (1)
24-24: LGTM!The
analyticsServiceis now correctly obtained from the DI container viauseServices(). The analytics tracking calls remain unchanged, and the refactor properly aligns with the pattern of resolving dependencies at runtime rather than module initialization.Also applies to: 63-63
apps/deploy-web/src/components/authorizations/AllowanceModal.tsx (1)
13-13: LGTM!The refactor correctly obtains
analyticsServicefrom the DI container. The tracking call inonSubmitremains unchanged in behavior.Also applies to: 35-35
apps/deploy-web/src/components/authorizations/GrantModal.tsx (1)
26-26: LGTM!The refactor correctly obtains
analyticsServicefrom the DI container viauseServices().Also applies to: 55-55
apps/deploy-web/src/services/analytics/analytics.service.ts (1)
153-164: Constructor refactor improves testability.The defaulted public parameters pattern is a good approach for DI - it allows the service to work out-of-the-box while enabling test mocks to be injected. The
isBrowserguards forwindow.gtagandlocalStorageare appropriate for SSR compatibility.apps/deploy-web/src/components/templates/UserTemplate.tsx (1)
15-15: LGTM!The refactor correctly obtains
analyticsServicefrom the DI container. All analytics tracking calls throughout the component maintain their existing behavior.Also applies to: 34-34
apps/deploy-web/src/components/new-deployment/ManifestEdit.tsx (1)
75-75: LGTM! Service dependencies correctly injected via context.The refactoring moves
analyticsServiceand configuration resolution from module scope to component scope viauseServices(), aligning with best practices for dependency injection. ThepublicConfigaliasing toappConfigmaintains backward compatibility within the component.Based on learnings, this approach avoids module-level side effects and improves testability by making dependencies explicit.
apps/deploy-web/src/components/sdl/ImportSdlModal.tsx (1)
12-12: LGTM! Analytics service correctly injected via DI container.The refactoring replaces the direct module import with context-provided
analyticsServiceviauseServices(), improving testability and avoiding uncontrolled side effects at module scope.Based on learnings, resolving dependencies inside components rather than at module initialization time provides better control over side effects.
Also applies to: 24-24
apps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsx (1)
18-18: LGTM! Service dependency properly injected.The change moves
analyticsServiceresolution from module scope to component scope via the DI container, improving testability and explicit dependency management. All usages occur within event handlers with no dependency array concerns.Also applies to: 51-51
apps/deploy-web/src/pages/user/api-keys/index.tsx (1)
9-9: LGTM! Analytics service correctly provided via context.The refactoring consistently applies the DI pattern, moving
analyticsServicefrom a static import to context-based injection, which enhances testability.Also applies to: 13-13
apps/deploy-web/src/components/sdl/SaveTemplateModal.tsx (1)
13-13: LGTM! Dependency injection applied correctly.The component now obtains
analyticsServicefrom the DI container viauseServices(), maintaining consistency with the broader refactoring effort and improving testability.Also applies to: 45-45
apps/deploy-web/src/components/deployments/DeploymentSubHeader.tsx (1)
40-40: LGTM! Configuration sourced from publicConfig.The change updates the configuration source from
appConfigtopublicConfig(aliased asappConfig) to align with the centralized DI container pattern. This maintains backward compatibility while adopting the new configuration structure.apps/deploy-web/src/components/user/UserProfileLayout.tsx (2)
7-7: LGTM! Multiple services correctly injected via DI container.The refactoring moves both
analyticsServiceandurlServicefrom static imports to context-provided instances viauseServices(). This improves testability by allowing easy mocking of both services and eliminates module-scope side effects.Also applies to: 21-21
31-31: LGTM! URL service calls correctly updated.All
UrlService.*static method calls have been replaced withurlService.*instance method calls, consistent with the DI container pattern. The routing logic remains unchanged.Also applies to: 34-34, 37-37
apps/deploy-web/src/services/app-di-container/browser-di-container.ts (1)
39-39: LGTM! Configuration reference updated to publicConfig.The DI container correctly references
services.publicConfiginstead ofservices.appConfigfor the base API URL configuration. This aligns with the broader refactoring to standardize configuration access throughpublicConfigacross the application.apps/deploy-web/src/services/app-di-container/app-di-container.ts (4)
19-20: LGTM!Clean imports for the DI container setup. The
AnalyticsServiceis now created within the container rather than imported as a singleton, aligning with the goal of reducing uncontrolled side effects.
36-37: LGTM!Exposing
publicConfigas a factory aligns with the DI pattern and provides a centralized configuration source for other services in the container.
71-76: Non-null assertion may cause silent failures.The
!operator onNEXT_PUBLIC_STRIPE_PUBLISHABLE_KEYsuppresses TypeScript checks, but if the env var is missing, theStripeServicewill receive an empty/undefined value despite the type claiming it's astring. WhilegetStripe()has a runtime guard, the type contract becomes misleading.Consider removing the assertion and letting the config type reflect reality (
string | undefined), or validate at container initialization.🔎 Suggested fix
stripeService: () => new StripeService({ config: { - publishableKey: container.publicConfig.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY! + publishableKey: container.publicConfig.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY ?? "" } }),Alternatively, update
StripeServiceDependenciesto acceptpublishableKey: string | undefinedif empty keys are expected.
137-148: LGTM!The
AnalyticsServiceinstantiation properly uses thepublicConfigvalues and follows the DI pattern. This improves testability by making the configuration explicit rather than relying on module-level singletons. Based on learnings, this aligns with the guidance to resolve dependencies inside functions rather than at module scope.apps/deploy-web/src/components/user/UserProfile.tsx (2)
8-8: LGTM!Clean migration to the DI pattern. Using
useServices()to obtainanalyticsServiceandurlServiceimproves testability and aligns with the broader refactor across the codebase.Also applies to: 20-20
41-47: LGTM!Correctly replaced the static
UrlService.sdlBuilder()call with the instance method from the context-providedurlService.apps/deploy-web/src/services/stripe/stripe.service.ts (1)
3-8: LGTM!Clean dependency interface that explicitly requires a
configobject withpublishableKey, while makingloadStripeoptional to allow injection of mocks during testing. This improves testability significantly.apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
75-91: LGTM!The
setupfunction correctly:
- Sits at the bottom of the root
describeblock- Creates the subject under test with explicit dependencies
- Accepts a single parameter with inline type definition
- Avoids shared state
- Has no specified return type
The test structure aligns with the updated
StripeServiceDependenciesinterface.apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (2)
369-369: LGTM! Consistent refactoring pattern.The same aliasing approach is correctly applied in the nested component, maintaining consistency with the WalletProvider changes.
76-76: Aliasing pattern correctly applied and consistently used throughout codebase.The
publicConfig: appConfigaliasing at line 76 maintains backward compatibility within the component. All destructured services (analyticsService, txHttpService, appConfig, urlService, windowLocation) are actively used. Verification confirms this refactoring pattern has been applied consistently across the codebase with no remaining unaliased appConfig destructurings from useServices().apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (2)
292-292: LGTM!Correctly uses
urlServicefrom the DI container instead of a static import. This change aligns with the PR's refactoring objectives.
75-84: DI pattern refactoring is correctly implemented.The destructuring of services from
useServices()properly aligns with the PR objective. ThepublicConfigincludes the requiredNEXT_PUBLIC_DEFAULT_INITIAL_DEPOSITproperty (defined in browser-env.config.ts and validated with a default of 500000), so the aliasing toappConfigis safe and follows the established pattern across the codebase.apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx (1)
285-295: LGTM!The test mock correctly reflects the production code change where
useServices()returnspublicConfig(aliased toappConfigin the component). The mock structure properly maintains test coverage.
stalniy
force-pushed
the
refactor/analytics-service
branch
from
feac91b to
68e37c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (2)
98-122: Add missing dependencies to useEffect dependency array.The effect uses
windowLocation(line 107) andwindowHistory(line 120) but doesn't include them in the dependency array on line 122. This violates React's exhaustive-deps rule and could lead to stale closures.🔎 Proposed fix
- }, [analyticsService, d.localStorage]); + }, [analyticsService, d.localStorage, windowLocation, windowHistory]);
201-316: Add missingurlServiceto useCallback dependency array.The callback uses
urlServiceon line 292 (router.push(urlService.newDeployment(...))) but doesn't include it in the dependency array at lines 298-315. This violates React's exhaustive-deps rule and could lead to stale closures.🔎 Proposed fix
[ d, router, + urlService, connectManagedWallet, templates,
🧹 Nitpick comments (3)
apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
71-76: Non-null assertion on environment variable.The
!assertion onNEXT_PUBLIC_STRIPE_PUBLISHABLE_KEYsilently assumes the value is always defined. WhileStripeService.getStripe()handles missing keys gracefully at runtime, consider using an empty string fallback to make the optionality explicit:🔎 Suggested change
stripeService: () => new StripeService({ config: { - publishableKey: container.publicConfig.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY! + publishableKey: container.publicConfig.NEXT_PUBLIC_STRIPE_PUBLISHABLE_KEY ?? "" } }),apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
8-52: Consider adding a test for missing publishableKey.The test suite covers the happy path and error handling well. Consider adding a test case for when
publishableKeyis an empty string to verify the warning behavior ingetStripe().🔎 Example test case
it("returns null and warns when publishable key is empty", async () => { const { stripeService, mockLoadStripe } = setup({ publishableKey: "" }); const consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(); const result = await stripeService.getStripe(); expect(result).toBeNull(); expect(mockLoadStripe).not.toHaveBeenCalled(); expect(consoleWarnSpy).toHaveBeenCalledWith("Stripe publishable key is not configured"); consoleWarnSpy.mockRestore(); });apps/deploy-web/src/hooks/useTrialBalance.ts (1)
22-27: Consider adding defensive defaults for config values.If
publicConfiglacksNEXT_PUBLIC_TRIAL_CREDITS_AMOUNTor it's zero, line 27's division could produceInfinityorNaN, breaking the percentage calculation.🔎 Optional: Add fallback defaults
- const TRIAL_TOTAL = appConfig.NEXT_PUBLIC_TRIAL_CREDITS_AMOUNT; - const TRIAL_DURATION_DAYS = appConfig.NEXT_PUBLIC_TRIAL_DURATION_DAYS; + const TRIAL_TOTAL = appConfig.NEXT_PUBLIC_TRIAL_CREDITS_AMOUNT || 0; + const TRIAL_DURATION_DAYS = appConfig.NEXT_PUBLIC_TRIAL_DURATION_DAYS || 0;Or validate that these values are positive numbers when initializing
publicConfigin the DI container.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
apps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsxapps/deploy-web/src/components/authorizations/AllowanceModal.tsxapps/deploy-web/src/components/authorizations/GrantModal.tsxapps/deploy-web/src/components/deployments/DeploymentDepositModal.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/deployments/DeploymentSubHeader.tsxapps/deploy-web/src/components/deployments/ShellDownloadModal.tsxapps/deploy-web/src/components/liquidity-modal/index.tsxapps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit.tsxapps/deploy-web/src/components/new-deployment/TemplateList.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/deploy-web/src/components/sdl/ImportSdlModal.tsxapps/deploy-web/src/components/sdl/RentGpusForm.tsxapps/deploy-web/src/components/sdl/SaveTemplateModal.tsxapps/deploy-web/src/components/settings/ExportCertificate.tsxapps/deploy-web/src/components/templates/UserTemplate.tsxapps/deploy-web/src/components/user/AddFundsLink.tsxapps/deploy-web/src/components/user/UserFavorites.tsxapps/deploy-web/src/components/user/UserProfile.tsxapps/deploy-web/src/components/user/UserProfileLayout.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/context/WalletProvider/WalletProvider.tsxapps/deploy-web/src/hooks/useStoredAnonymousUser.tsapps/deploy-web/src/hooks/useTrialBalance.tsapps/deploy-web/src/pages/user/api-keys/index.tsxapps/deploy-web/src/services/analytics/analytics.service.tsapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/services/app-di-container/browser-di-container.tsapps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/services/stripe/stripe.service.ts
🚧 Files skipped from review as they are similar to previous changes (20)
- apps/deploy-web/src/components/sdl/RentGpusForm.tsx
- apps/deploy-web/src/components/liquidity-modal/index.tsx
- apps/deploy-web/src/components/authorizations/AllowanceModal.tsx
- apps/deploy-web/src/components/user/UserProfileLayout.tsx
- apps/deploy-web/src/components/settings/ExportCertificate.tsx
- apps/deploy-web/src/components/authorizations/GrantModal.tsx
- apps/deploy-web/src/components/sdl/SaveTemplateModal.tsx
- apps/deploy-web/src/components/deployments/DeploymentDepositModal.tsx
- apps/deploy-web/src/services/analytics/analytics.service.ts
- apps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsx
- apps/deploy-web/src/components/deployments/DeploymentSubHeader.tsx
- apps/deploy-web/src/components/user/UserFavorites.tsx
- apps/deploy-web/src/components/new-deployment/TemplateList.tsx
- apps/deploy-web/src/services/app-di-container/browser-di-container.ts
- apps/deploy-web/src/hooks/useStoredAnonymousUser.ts
- apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx
- apps/deploy-web/src/components/new-deployment/ManifestEdit.tsx
- apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
- apps/deploy-web/src/components/deployments/DeploymentListRow.tsx
- apps/deploy-web/src/pages/user/api-keys/index.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/components/sdl/ImportSdlModal.tsxapps/deploy-web/src/hooks/useTrialBalance.tsapps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsxapps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/components/user/AddFundsLink.tsxapps/deploy-web/src/services/stripe/stripe.service.tsapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsxapps/deploy-web/src/components/user/UserProfile.tsxapps/deploy-web/src/components/templates/UserTemplate.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/components/deployments/ShellDownloadModal.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations
Files:
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx
🧠 Learnings (7)
📓 Common learnings
Learnt from: stalniy
Repo: akash-network/console PR: 2255
File: apps/api/src/middlewares/privateMiddleware.ts:5-9
Timestamp: 2025-11-19T16:13:43.249Z
Learning: In the Akash Console API (apps/api), avoid resolving DI container dependencies at module scope (module initialization time) to prevent side effects. Instead, resolve dependencies inside functions/methods where they are actually used, even if this means resolving on every invocation, to maintain explicit control over when side effects occur and improve testability.
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/deploy-web/src/services/stripe/stripe.service.spec.ts
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
apps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
Applied to files:
apps/deploy-web/src/services/stripe/stripe.service.spec.ts
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx
📚 Learning: 2025-06-19T16:00:05.428Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 1512
File: apps/deploy-web/src/components/deployments/DeploymentBalanceAlert/DeploymentBalanceAlert.tsx:47-68
Timestamp: 2025-06-19T16:00:05.428Z
Learning: In React Hook Form setups with zod validation, child components using useFormContext() can rely on parent form validation rather than implementing local input validation. Invalid inputs like NaN from parseFloat() are handled by the parent schema validation, eliminating the need for additional local validation in child components.
Applied to files:
apps/deploy-web/src/components/deployments/ShellDownloadModal.tsx
🧬 Code graph analysis (9)
apps/deploy-web/src/components/sdl/ImportSdlModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
apps/deploy-web/src/services/stripe/stripe.service.ts (2)
StripeServiceDependencies(3-8)StripeService(10-44)
apps/deploy-web/src/components/user/AddFundsLink.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
apps/api/src/billing/config/index.ts (1)
appConfig(3-5)
apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx (2)
apps/deploy-web/tests/unit/TestContainerProvider.tsx (1)
TestContainerProvider(9-32)apps/deploy-web/src/services/analytics/analytics.service.ts (1)
AnalyticsService(144-273)
apps/deploy-web/src/components/user/UserProfile.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/templates/UserTemplate.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/deployments/ShellDownloadModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
⏰ 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)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (26)
apps/deploy-web/src/components/sdl/ImportSdlModal.tsx (1)
12-12: LGTM! Clean refactor to DI-based service access.The change correctly replaces the direct
analyticsServiceimport with context-based access viauseServices(). The hook is properly called at the component's top level, following React rules, and the DI resolution occurs inside the component function rather than at module scope, which aligns with the learnings.Also applies to: 24-24
apps/deploy-web/src/components/user/UserProfile.tsx (2)
8-8: LGTM! Excellent refactor aligning with DI best practices.The migration to context-based service resolution is well-executed:
- Services are resolved inside the component function rather than at module scope, avoiding initialization-time side effects.
- Follows React hooks rules by calling
useServices()at the top level.- Maintains existing functionality while improving testability.
Based on learnings, this approach correctly defers DI container resolution to function execution time rather than module initialization.
Also applies to: 20-20
41-41: Service instance correctly used.The shift from static
UrlService.sdlBuilder()to instanceurlService.sdlBuilder()is consistent with the DI container pattern.apps/deploy-web/src/components/templates/UserTemplate.tsx (2)
15-15: LGTM! DI resolution follows project guidelines.The refactoring correctly moves analyticsService from a static import to DI-provided service via
useServices(). The resolution happens inside the component function rather than at module scope, which improves testability and explicit control over side effects.Based on learnings, this approach aligns with the project's preference to resolve DI container dependencies inside functions where they're actually used rather than at module initialization time.
Also applies to: 34-34
54-57: LGTM! Analytics tracking usage is consistent.All
analyticsService.track()calls maintain consistent usage patterns with appropriate event names, categories, and labels for each user action. The refactoring doesn't alter the tracking logic, only the source of the service instance.Also applies to: 70-73, 114-117, 127-130, 153-156, 166-169, 172-175
apps/deploy-web/src/components/deployments/ShellDownloadModal.tsx (3)
8-8: LGTM!Clean import addition that enables access to the DI container services.
33-33: LGTM! Follows DI best practices.The
analyticsServiceis correctly resolved inside the component function rather than at module scope, which maintains explicit control over side effects and improves testability as intended by this refactor.Based on learnings, this approach properly avoids resolving DI container dependencies at module initialization time.
48-57: LGTM! Analytics tracking properly integrated.The analytics tracking is well-placed in the
onSubmithandler and correctly uses the DI-providedanalyticsService. The fire-and-forget pattern (noawait) is appropriate for analytics, as it tracks user intent without blocking the UI flow.apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (4)
47-64: LGTM! DEPENDENCIES object correctly updated.The removal of
UrlServicefrom theDEPENDENCIESobject is consistent with the refactoring to access it via the DI container throughuseServices().
75-84: LGTM! Services correctly accessed via DI container.The destructuring of services from
useServices()follows the DI pattern and aligns with the PR objectives. The aliasing ofpublicConfigasappConfigmaintains internal consistency within the component.Based on learnings, this correctly resolves DI dependencies inside the component rather than at module scope.
181-181: LGTM! Correctly using urlService from DI container.The usage of
urlServicefrom the DI container (instead of the previousd.UrlService) is correct and consistent with the refactoring objectives. The dependency array at line 183 properly includesurlService.
292-292: LGTM! Correctly using urlService from DI container.The usage of
urlServicefrom the DI container (instead of the previousd.UrlService) is correct and consistent with the refactoring objectives.apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx (2)
2-2: LGTM: Correct use of jest-mock-extended for analytics mocking.The imports follow the project's testing guidelines by using
jest-mock-extendedinstead ofjest.mock(), and the type-only import ofAnalyticsServiceis appropriately scoped.Also applies to: 6-6
114-114: LGTM: Analytics service correctly injected via DI.The mocked
analyticsServiceis properly provided through the test container using the factory pattern, aligning with the PR's objective to move analytics service creation to the DI container.Note: If future tests need to verify analytics event tracking, consider storing the mock in a variable to enable assertions:
const analyticsServiceMock = mock<AnalyticsService>(); // ... later in services analyticsService: () => analyticsServiceMockapps/deploy-web/src/services/stripe/stripe.service.ts (3)
3-8: LGTM!The interface redesign cleanly separates the optional dependency (
loadStripe) from the required configuration (config.publishableKey), making the service more testable and explicit about its configuration requirements.
10-19: LGTM!The constructor correctly provides a default for
loadStripewhile allowing injection for testing. The spread order ensures provided dependencies override defaults.
21-39: LGTM!The method correctly retrieves the publishable key from the new config path while maintaining defensive checks for missing configuration.
apps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsx (1)
10-10: LGTM!Migrating
analyticsServicefrom a direct import touseServices()properly aligns with the DI container pattern, improving testability by making the dependency injectable. Based on retrieved learnings, this avoids resolving dependencies at module scope.Also applies to: 29-30
apps/deploy-web/src/services/app-di-container/app-di-container.ts (2)
36-36: LGTM!Exposing
browserEnvConfigthrough the DI container aspublicConfigcentralizes configuration access and maintains the lazy resolution pattern.
137-148: LGTM!The
AnalyticsServicefactory properly centralizes configuration and follows the lazy resolution pattern. This aligns with the PR objective to move analytics service creation into the DI container, reducing uncontrolled side effects and improving testability. Based on retrieved learnings, resolving dependencies inside factory functions is the correct approach.apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
75-91: LGTM!The setup function correctly mirrors the updated
StripeServiceconstructor signature, passing theconfigobject withpublishableKey. The type annotation update toRequired<StripeServiceDependencies>["loadStripe"]accurately reflects thatloadStripebecomes required after the service is constructed.apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx (2)
15-15: LGTM! Clean aliasing pattern.The destructuring correctly aliases
publicConfigtoappConfigfor local use, maintaining consistency within the component while adapting to the updated DI container structure.
31-47: Service stability is properly managed through memoization.The DI container is memoized in
ServicesProvider(line 25 viauseMemo) with dependencies on settings changes, ensuringanalyticsServiceanduserTrackerare stable references across renders. The effect dependency array correctly includes these services, which will only change when settings change, making the implementation safe and aligned with best practices.The
UserTrackercomponent correctly resolves dependencies at component scope (not module scope), improving testability and avoiding unintended side effects.apps/deploy-web/src/hooks/useTrialBalance.ts (1)
20-20: Good refactor to use DI-provided config.This change correctly sources configuration from the DI container's
publicConfig, aligning with the PR's objective. The aliasing pattern maintains backward compatibility with downstream code. The dependency is resolved inside the hook function rather than at module scope, which follows best practices for DI container usage.apps/deploy-web/src/components/user/AddFundsLink.tsx (2)
6-6: LGTM! Proper DI container integration.The migration from direct analyticsService import to context-provided service via
useServices()is correctly implemented. The hook is invoked at the component level (not module scope), which is the appropriate pattern for React components and aligns with the PR's objective to improve testability.Also applies to: 17-17
28-28: Thetrackmethod already uses proper TypeScript typing with theAnalyticsEventunion type, not a generic string type. The event"add_funds_btn_clk"is a valid member of that union. No type safety issues exist, and the code follows all coding guidelines.
stalniy
force-pushed
the
refactor/analytics-service
branch
from
68e37c6 to
b3f66bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (2)
98-122: Add missing dependencies to useEffectThe
useEffectuseswindowLocation(lines 107-108, 118) andwindowHistory(line 120) but they are not included in the dependency array at line 122. This violates React's exhaustive-deps rule and could lead to stale closure bugs.🔎 Proposed fix
- }, [analyticsService, d.localStorage]); + }, [analyticsService, d.localStorage, windowLocation, windowHistory]);
201-316: Add missingurlServicedependency to useCallbackThe
handleCompletecallback usesurlServiceon line 292 but it's not included in the dependency array (lines 298-315). This violates React's exhaustive-deps rule and creates an inconsistency with how other services likeanalyticsService,chainApiHttpClient, anddeploymentLocalStorageare properly included.🔎 Proposed fix
[ d, router, connectManagedWallet, templates, chainApiHttpClient, address, appConfig, depositParams, genNewCertificateIfLocalIsInvalid, signAndBroadcastTx, updateSelectedCertificate, deploymentLocalStorage, analyticsService, enqueueSnackbar, errorHandler, - managedDenom + managedDenom, + urlService ]apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
7-7: Use class name reference instead of hardcoded string.The root describe should use
StripeService.nameinstead of the hardcoded string"StripeService"to enable automated refactoring tools to find all references.As per coding guidelines for test files.
🔎 Proposed fix
-describe("StripeService", () => { +describe(StripeService.name, () => {apps/deploy-web/src/services/analytics/analytics.service.ts (1)
210-218: Potential race condition in ensureAmplitudeFor.Multiple concurrent calls to
identify()could all pass thetypeof this.isAmplitudeEnabled === "undefined"check before any of them sets the value, potentially resulting inamplitudeClient.init()being called multiple times with the same API key.While calling
init()multiple times may not cause data corruption, it could lead to unexpected behavior or duplicate event tracking. Consider using a promise-based initialization pattern or ensuring single initialization.Based on learnings about race conditions in async initialization patterns.
🔎 Potential fix using promise caching
export class AnalyticsService { private readonly STORAGE_KEY = "analytics_values_cache"; private readonly valuesCache: Map<string, string> = this.loadSwitchValuesFromStorage(); private isAmplitudeEnabled: boolean | undefined; + private amplitudeInitPromise: Promise<void> | undefined; private get gtag() { return this.getGtag(); } private ensureAmplitudeFor(user: AnalyticsUser) { if (typeof this.isAmplitudeEnabled === "undefined" && user.id) { + if (!this.amplitudeInitPromise) { + this.amplitudeInitPromise = this.initializeAmplitude(user.id); + } + return this.amplitudeInitPromise; + } + } + + private async initializeAmplitude(userId: string): Promise<void> { - this.isAmplitudeEnabled = this.shouldSampleUser(user.id); + this.isAmplitudeEnabled = this.shouldSampleUser(userId); if (this.isAmplitudeEnabled) { this.amplitudeClient.init(this.options.amplitude.apiKey); } - } }Note: This would require updating
identify()to handle the promise if you want to await initialization.
🧹 Nitpick comments (2)
apps/deploy-web/src/components/settings/ExportCertificate.tsx (1)
13-23: Consider simplifying the effect.The
async init()wrapper function doesn't useawait, making it unnecessary. You could simplify this to directly call the tracking method.🔎 Proposed simplification
useEffect(() => { - async function init() { - analyticsService.track("export_certificate", { - category: "certificates", - label: "Export certificate" - }); - } - - init(); + analyticsService.track("export_certificate", { + category: "certificates", + label: "Export certificate" + }); // eslint-disable-next-line react-hooks/exhaustive-deps }, []);apps/deploy-web/src/services/stripe/stripe.service.ts (1)
26-30: Consider aligning type with runtime check.The runtime check on line 27 handles falsy
publishableKeyvalues, but the interface type declares it as a requiredstring. For type consistency, consider either:
- Make the type optional:
publishableKey?: string(if it can genuinely be absent)- Or keep the defensive check as-is (valid defensive coding even with strong types)
🔎 Optional type refinement
export interface StripeServiceDependencies { loadStripe?: typeof loadStripe; config: { - publishableKey: string; + publishableKey?: string; }; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
apps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsxapps/deploy-web/src/components/authorizations/AllowanceModal.tsxapps/deploy-web/src/components/authorizations/GrantModal.tsxapps/deploy-web/src/components/deployments/DeploymentDepositModal.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/deployments/DeploymentListRow.tsxapps/deploy-web/src/components/deployments/DeploymentSubHeader.tsxapps/deploy-web/src/components/deployments/ShellDownloadModal.tsxapps/deploy-web/src/components/liquidity-modal/index.tsxapps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsxapps/deploy-web/src/components/new-deployment/ManifestEdit.tsxapps/deploy-web/src/components/new-deployment/TemplateList.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/deploy-web/src/components/sdl/ImportSdlModal.tsxapps/deploy-web/src/components/sdl/RentGpusForm.tsxapps/deploy-web/src/components/sdl/SaveTemplateModal.tsxapps/deploy-web/src/components/settings/ExportCertificate.tsxapps/deploy-web/src/components/templates/UserTemplate.tsxapps/deploy-web/src/components/user/AddFundsLink.tsxapps/deploy-web/src/components/user/UserFavorites.tsxapps/deploy-web/src/components/user/UserProfile.tsxapps/deploy-web/src/components/user/UserProfileLayout.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/context/WalletProvider/WalletProvider.tsxapps/deploy-web/src/hooks/useStoredAnonymousUser.tsapps/deploy-web/src/hooks/useTrialBalance.tsapps/deploy-web/src/pages/user/api-keys/index.tsxapps/deploy-web/src/services/analytics/analytics.service.tsapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/services/app-di-container/browser-di-container.tsapps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/services/stripe/stripe.service.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- apps/deploy-web/src/components/user/UserFavorites.tsx
- apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx
- apps/deploy-web/src/components/user/AddFundsLink.tsx
- apps/deploy-web/src/components/new-deployment/ManifestEdit.tsx
- apps/deploy-web/src/components/new-deployment/BidRow/BidRow.spec.tsx
- apps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsx
- apps/deploy-web/src/components/deployments/DeploymentListRow.tsx
- apps/deploy-web/src/components/liquidity-modal/index.tsx
- apps/deploy-web/src/components/deployments/DeploymentSubHeader.tsx
- apps/deploy-web/src/components/user/UserProfile.tsx
- apps/deploy-web/src/hooks/useTrialBalance.ts
- apps/deploy-web/src/components/user/UserProfileLayout.tsx
- apps/deploy-web/src/components/sdl/ImportSdlModal.tsx
- apps/deploy-web/src/components/authorizations/GrantModal.tsx
- apps/deploy-web/src/components/authorizations/AllowanceModal.tsx
- apps/deploy-web/src/components/sdl/SaveTemplateModal.tsx
- apps/deploy-web/src/components/api-keys/CreateApiKeyModal.tsx
- apps/deploy-web/src/hooks/useStoredAnonymousUser.ts
- apps/deploy-web/src/components/deployments/ShellDownloadModal.tsx
- apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx
- apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/components/settings/ExportCertificate.tsxapps/deploy-web/src/services/app-di-container/browser-di-container.tsapps/deploy-web/src/services/stripe/stripe.service.spec.tsapps/deploy-web/src/components/deployments/DeploymentDepositModal.tsxapps/deploy-web/src/components/new-deployment/TemplateList.tsxapps/deploy-web/src/components/sdl/RentGpusForm.tsxapps/deploy-web/src/services/analytics/analytics.service.tsapps/deploy-web/src/pages/user/api-keys/index.tsxapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/services/stripe/stripe.service.tsapps/deploy-web/src/components/templates/UserTemplate.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/deploy-web/src/services/stripe/stripe.service.spec.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: stalniy
Repo: akash-network/console PR: 2255
File: apps/api/src/middlewares/privateMiddleware.ts:5-9
Timestamp: 2025-11-19T16:13:43.249Z
Learning: In the Akash Console API (apps/api), avoid resolving DI container dependencies at module scope (module initialization time) to prevent side effects. Instead, resolve dependencies inside functions/methods where they are actually used, even if this means resolving on every invocation, to maintain explicit control over when side effects occur and improve testability.
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/deploy-web/src/services/app-di-container/browser-di-container.ts
📚 Learning: 2025-05-28T20:42:58.200Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 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.
Applied to files:
apps/deploy-web/src/services/app-di-container/browser-di-container.ts
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
apps/deploy-web/src/services/stripe/stripe.service.spec.ts
📚 Learning: 2025-11-25T17:45:58.258Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/test-descriptions.mdc:0-0
Timestamp: 2025-11-25T17:45:58.258Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `<Subject>.name` in the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Applied to files:
apps/deploy-web/src/services/stripe/stripe.service.spec.ts
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
Applied to files:
apps/deploy-web/src/services/stripe/stripe.service.spec.ts
📚 Learning: 2025-06-05T21:07:51.985Z
Learnt from: baktun14
Repo: akash-network/console PR: 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.
Applied to files:
apps/deploy-web/src/components/sdl/RentGpusForm.tsx
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use deprecated methods from libraries.
Applied to files:
apps/deploy-web/src/services/analytics/analytics.service.ts
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/deploy-web/src/services/analytics/analytics.service.ts
🧬 Code graph analysis (10)
apps/deploy-web/src/components/settings/ExportCertificate.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/services/app-di-container/browser-di-container.ts (1)
apps/deploy-web/src/services/app-di-container/server-di-container.service.ts (1)
services(24-54)
apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
apps/deploy-web/src/services/stripe/stripe.service.ts (2)
StripeServiceDependencies(3-8)StripeService(10-44)
apps/deploy-web/src/components/deployments/DeploymentDepositModal.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/new-deployment/TemplateList.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/sdl/RentGpusForm.tsx (2)
apps/api/src/billing/config/index.ts (1)
appConfig(3-5)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/pages/user/api-keys/index.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/services/app-di-container/app-di-container.ts (4)
apps/deploy-web/src/config/browser-env.config.ts (1)
browserEnvConfig(4-41)apps/deploy-web/src/services/stripe/stripe.service.ts (1)
StripeService(10-44)packages/http-sdk/src/stripe/stripe.service.ts (1)
StripeService(19-103)apps/deploy-web/src/services/analytics/analytics.service.ts (1)
AnalyticsService(144-273)
apps/deploy-web/src/components/templates/UserTemplate.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
apps/api/src/billing/config/index.ts (1)
appConfig(3-5)
⏰ 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)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (17)
apps/deploy-web/src/components/sdl/RentGpusForm.tsx (1)
49-49: LGTM! Config now sourced from DI container.The change correctly retrieves
publicConfigfrom the DI container and aliases it toappConfigfor use within the component. This aligns with the PR objective to centralize service creation in the DI container and improves testability.apps/deploy-web/src/components/settings/ExportCertificate.tsx (1)
6-6: LGTM! Clean DI container integration.The change correctly sources
analyticsServicefrom the DI container viauseServices(), following React hooks rules and improving testability as intended by the PR.Also applies to: 11-11
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (2)
47-64: LGTM - Correct refactoring to use DI containerThe removal of
UrlServicefrom DEPENDENCIES and obtaining it viauseServices()hook aligns with the PR objective to move service creation to the DI container. This improves testability by allowing services to be mocked via thedependenciesprop.
75-84: LGTM - Services correctly obtained from DI containerThe destructuring of services from
useServices()is correctly implemented. The renaming ofpublicConfigtoappConfigimproves local readability while maintaining clarity.apps/deploy-web/src/services/app-di-container/browser-di-container.ts (1)
39-39: LGTM - Config source correctly migrated to publicConfig.The change from
appConfigtopublicConfigaligns with the DI container refactoring that exposes configuration viapublicConfig. The access remains properly scoped within the factory function.apps/deploy-web/src/components/templates/UserTemplate.tsx (1)
15-15: LGTM - Correctly migrated to context-based service access.The component now properly retrieves
analyticsServicefrom the DI container viauseServices(), eliminating the direct module import. This improves testability and follows the project's DI pattern.Also applies to: 34-34
apps/deploy-web/src/components/deployments/DeploymentDepositModal.tsx (1)
12-12: LGTM - Service access properly migrated to useServices().The modal correctly retrieves
analyticsServicefrom the DI container viauseServices(), consistent with the refactoring pattern across the codebase.Also applies to: 51-51
apps/deploy-web/src/components/new-deployment/TemplateList.tsx (1)
11-11: LGTM - Consistent with DI container refactoring.The component correctly obtains
analyticsServiceviauseServices(), maintaining consistency with the broader refactoring effort.Also applies to: 47-47
apps/deploy-web/src/pages/user/api-keys/index.tsx (1)
9-9: LGTM - Service access correctly refactored.The page correctly retrieves
analyticsServicefrom the context viauseServices(), following the established DI pattern.Also applies to: 13-13
apps/deploy-web/src/services/app-di-container/app-di-container.ts (3)
36-36: LGTM - publicConfig properly exposed in the DI container.The
publicConfiggetter correctly exposesbrowserEnvConfigthrough the container, enabling other services to access configuration in a testable manner.
71-76: LGTM - StripeService correctly configured with publicConfig.The service is properly instantiated with the publishable key sourced from
publicConfig, improving testability and aligning with the DI refactoring goals.
137-148: LGTM - AnalyticsService correctly instantiated with configuration from publicConfig.The service is properly created within a factory function with amplitude and GA settings sourced from
publicConfig. This eliminates the singleton pattern and improves testability.apps/deploy-web/src/services/stripe/stripe.service.spec.ts (1)
75-91: LGTM - Test correctly updated for new constructor signature.The setup function properly reflects the new
StripeServiceconstructor that accepts a config object withpublishableKey. The test pattern follows the project guidelines with the setup function at the bottom and usingjest-mock-extended.apps/deploy-web/src/services/analytics/analytics.service.ts (1)
1-1: LGTM - Service correctly refactored for DI container usage.The service now:
- Uses proper value imports (not type-only) for amplitude and murmurhash
- Accepts dependencies via constructor with appropriate defaults
- Removes the singleton pattern, enabling proper DI
- Includes "use client" directive for client-side execution
These changes align with the PR objective to improve testability and reduce side effects.
Also applies to: 3-5, 157-161
apps/deploy-web/src/services/stripe/stripe.service.ts (3)
3-8: LGTM! Clean dependency injection interface.The interface properly separates configuration (
config) from optional implementation dependencies (loadStripe), enabling both testability and production use.
12-12: LGTM! Correct use of Required utility type.Using
Required<StripeServiceDependencies>accurately reflects that the constructor ensures all dependencies are present after applying defaults.
14-19: LGTM! Constructor follows DI best practices.The constructor correctly accepts dependencies as a parameter and applies defaults for optional dependencies. The spread order allows test overrides while providing production defaults.
Why
to reduce amount of uncontrolled side effects and improve testability of the app
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.