Conversation
|
Warning Rate limit exceeded@stalniy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 57 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (16)
WalkthroughThis change refactors the codebase to centralize the instantiation and access of API service classes (GitHub, Bitbucket, GitLab, and API URL services) via a dependency injection context ( Changes
Sequence Diagram(s)sequenceDiagram
participant Component/Hook
participant ServicesContext
participant ServiceInstance
Component/Hook->>ServicesContext: useServices()
ServicesContext-->>Component/Hook: { githubService, bitbucketService, gitlabService, apiUrlService }
Component/Hook->>ServiceInstance: Call service method (e.g., fetchUserProfile)
ServiceInstance-->>Component/Hook: Response
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1665 +/- ##
==========================================
- Coverage 72.28% 72.02% -0.27%
==========================================
Files 590 591 +1
Lines 13507 13660 +153
Branches 2218 2293 +75
==========================================
+ Hits 9764 9839 +75
- Misses 3424 3601 +177
+ Partials 319 220 -99
*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: 0
🧹 Nitpick comments (4)
apps/deploy-web/src/queries/useGithubQuery.spec.tsx (1)
11-180: Consider usingsetupfunction for better test structureAccording to the coding guidelines, test files should use a
setupfunction instead ofbeforeEach. While the currentsetupQueryhelper works, consider restructuring to use asetupfunction at the bottom of the rootdescribeblock that creates the object under test and returns it.apps/deploy-web/src/services/remote-deploy/github-http.service.ts (1)
82-84: Consider consistency in axios usageThe
fetchAccessTokenmethod still uses the externalaxiosdirectly instead of the privateaxiosInstance. While this might be intentional since it calls a different endpoint (/api/github/authenticate), consider whether this should also use the private instance for consistency, or if it needs different configuration.apps/deploy-web/src/queries/useGitlabQuery.spec.tsx (1)
21-246: Consider adding a setup function for consistency with coding guidelines.According to the coding guidelines, test files should use a
setupfunction instead ofbeforeEach. While these tests don't usebeforeEach, they also don't follow the prescribedsetupfunction pattern. The currentsetupQueryapproach works well for this specific use case, but consider if asetupfunction would be more appropriate for consistency.The
setupfunction should:
- Be at the bottom of the root
describeblock- Create an object under test and return it
- Accept a single parameter with inline type definition
- Not use shared state
- Not specify return type
apps/deploy-web/src/queries/useBitBucketQuery.spec.tsx (1)
21-281: Consider adding a setup function for consistency with coding guidelines.According to the coding guidelines, test files should use a
setupfunction instead ofbeforeEach. While these tests don't usebeforeEach, they also don't follow the prescribedsetupfunction pattern. The currentsetupQueryapproach works well for this specific use case, but consider if asetupfunction would be more appropriate for consistency.The
setupfunction should:
- Be at the bottom of the root
describeblock- Create an object under test and return it
- Accept a single parameter with inline type definition
- Not use shared state
- Not specify return type
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/deploy-web/src/components/remote-deploy/RemoteRepositoryDeployManager.tsx(5 hunks)apps/deploy-web/src/pages/profile/[username]/index.tsx(1 hunks)apps/deploy-web/src/pages/template/[id]/index.tsx(1 hunks)apps/deploy-web/src/queries/useBitBucketQuery.spec.tsx(10 hunks)apps/deploy-web/src/queries/useBitBucketQuery.ts(9 hunks)apps/deploy-web/src/queries/useGithubQuery.spec.tsx(7 hunks)apps/deploy-web/src/queries/useGithubQuery.ts(7 hunks)apps/deploy-web/src/queries/useGitlabQuery.spec.tsx(10 hunks)apps/deploy-web/src/queries/useGitlabQuery.ts(5 hunks)apps/deploy-web/src/services/api-url/browser-api-url.service.ts(0 hunks)apps/deploy-web/src/services/api-url/server-api-url.service.ts(0 hunks)apps/deploy-web/src/services/app-di-container/app-di-container.ts(2 hunks)apps/deploy-web/src/services/http/http-browser.service.ts(1 hunks)apps/deploy-web/src/services/http/http-server.service.ts(2 hunks)apps/deploy-web/src/services/remote-deploy/github-http.service.ts(7 hunks)apps/deploy-web/src/utils/apiUtils.ts(2 hunks)
💤 Files with no reviewable changes (2)
- apps/deploy-web/src/services/api-url/browser-api-url.service.ts
- apps/deploy-web/src/services/api-url/server-api-url.service.ts
🧰 Additional context used
📓 Path-based instructions (2)
`apps/{deploy-web,provider-console}/**/*.spec.tsx`: Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
apps/{deploy-web,provider-console}/**/*.spec.tsx: UsequeryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
📄 Source: CodeRabbit Inference Engine (.cursor/rules/query-by-in-tests.mdc)
List of files the instruction was applied to:
apps/deploy-web/src/queries/useGithubQuery.spec.tsxapps/deploy-web/src/queries/useGitlabQuery.spec.tsxapps/deploy-web/src/queries/useBitBucketQuery.spec.tsx
`**/*.spec.{ts,tsx}`: Use `setup` function instead of `beforeEach` in test files...
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock
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
📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)
List of files the instruction was applied to:
apps/deploy-web/src/queries/useGithubQuery.spec.tsxapps/deploy-web/src/queries/useGitlabQuery.spec.tsxapps/deploy-web/src/queries/useBitBucketQuery.spec.tsx
🧠 Learnings (11)
apps/deploy-web/src/pages/profile/[username]/index.tsx (2)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: ygrishajev
PR: akash-network/console#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.
apps/deploy-web/src/pages/template/[id]/index.tsx (2)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: ygrishajev
PR: akash-network/console#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.
apps/deploy-web/src/queries/useGitlabQuery.ts (2)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
apps/deploy-web/src/queries/useBitBucketQuery.ts (2)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
apps/deploy-web/src/services/remote-deploy/github-http.service.ts (2)
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
apps/deploy-web/src/utils/apiUtils.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
apps/deploy-web/src/queries/useGithubQuery.ts (1)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
apps/deploy-web/src/components/remote-deploy/RemoteRepositoryDeployManager.tsx (1)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
apps/deploy-web/src/queries/useGithubQuery.spec.tsx (8)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/queries/useGitlabQuery.spec.tsx (6)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/queries/useBitBucketQuery.spec.tsx (5)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
🧬 Code Graph Analysis (7)
apps/deploy-web/src/pages/profile/[username]/index.tsx (1)
apps/deploy-web/src/services/http/http-server.service.ts (1)
services(22-31)
apps/deploy-web/src/queries/useGitlabQuery.ts (5)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(30-32)apps/deploy-web/src/store/remoteDeployStore.ts (1)
tokens(3-13)apps/deploy-web/src/types/remoteProfile.ts (1)
GitLabProfile(33-47)apps/deploy-web/src/queries/queryKeys.ts (1)
QueryKeys(1-84)apps/deploy-web/src/types/remotedeploy.ts (1)
PackageJson(14-18)
apps/deploy-web/src/queries/useBitBucketQuery.ts (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(30-32)
apps/deploy-web/src/services/remote-deploy/github-http.service.ts (3)
apps/deploy-web/src/types/remoteProfile.ts (1)
GitHubProfile(2-2)apps/deploy-web/src/types/remotedeploy.ts (1)
GithubRepository(2-2)apps/deploy-web/src/types/remoteCommits.ts (1)
GitCommit(2-2)
apps/deploy-web/src/services/app-di-container/app-di-container.ts (1)
apps/deploy-web/src/utils/apiUtils.ts (1)
ApiUrlService(7-126)
apps/deploy-web/src/queries/useGithubQuery.ts (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(30-32)
apps/deploy-web/src/components/remote-deploy/RemoteRepositoryDeployManager.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(30-32)
⏰ 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 (22)
apps/deploy-web/src/pages/template/[id]/index.tsx (1)
37-37: Excellent refactoring to use dynamic service-based API URL retrieval.This change properly encapsulates API URL logic within the service layer and enables better testability by removing direct dependency on static configuration.
apps/deploy-web/src/utils/apiUtils.ts (2)
3-3: Correct update to use centralized services container.This change properly replaces the deprecated
browserApiUrlServicewith the new centralized services approach.
124-124: Well-executed migration to service-based API URL retrieval.The static getter now correctly uses the services container while maintaining the same interface for consumers.
apps/deploy-web/src/pages/profile/[username]/index.tsx (1)
27-27: Consistent refactoring pattern applied correctly.This change mirrors the template page refactoring, maintaining consistency across server-side rendered pages by using dynamic service-based API URL retrieval.
apps/deploy-web/src/services/http/http-server.service.ts (2)
8-8: Correct import update for DI refactoring.Replacing the deprecated singleton service import with the class import supports the new factory-based instantiation pattern.
19-19: Excellent implementation of factory-based service instantiation.This change properly implements dependency injection by using a factory function instead of a static singleton, enabling better testability and lazy initialization.
apps/deploy-web/src/queries/useGithubQuery.ts (2)
5-5: Proper context import for DI pattern.Adding the
useServicesimport enables context-based service retrieval, supporting the dependency injection refactoring.
12-12: Excellent consistent refactoring across all GitHub hooks.The consistent pattern of using
const { githubService } = useServices()properly implements dependency injection while maintaining the same hook interfaces. This refactoring improves:
- Testability: Services can now be easily mocked in tests
- Centralized management: Service configuration is centralized in the DI container
- Lazy initialization: Services are created only when needed
The hook logic remains unchanged, ensuring no functional regressions while achieving architectural improvements.
Also applies to: 23-23, 33-33, 44-44, 55-55, 81-81, 100-100
apps/deploy-web/src/components/remote-deploy/RemoteRepositoryDeployManager.tsx (2)
10-10: LGTM: Proper dependency injection implementationThe refactoring correctly implements the dependency injection pattern by importing and using the
useServiceshook to obtain service instances from the React context instead of creating them locally.Also applies to: 42-42
160-160: LGTM: Consistent service usage patternAll service method calls correctly use the context-provided instances (
bitbucketService,gitlabService,githubService) instead of locally instantiated services. This achieves the PR objectives of centralizing service management and improving testability.Also applies to: 170-170, 181-181, 183-183
apps/deploy-web/src/queries/useGitlabQuery.ts (1)
6-6: LGTM: Consistent dependency injection pattern across all hooksExcellent refactoring that consistently applies the dependency injection pattern across all GitLab-related hooks. Each hook properly retrieves the
gitlabServiceinstance from the React context usinguseServices(), maintaining the same functionality while improving testability and service management.Also applies to: 14-14, 32-32, 48-48, 68-68, 78-78, 88-88, 98-98, 108-108, 133-133
apps/deploy-web/src/queries/useBitBucketQuery.ts (1)
6-6: LGTM: Consistent dependency injection implementationThe refactoring successfully applies the dependency injection pattern consistently across all Bitbucket-related hooks. Each hook properly obtains the
bitbucketServiceinstance from the React context, maintaining functional consistency while improving the service architecture.Also applies to: 15-15, 29-29, 46-46, 66-66, 77-77, 88-88, 99-99, 110-110, 129-129
apps/deploy-web/src/queries/useGithubQuery.spec.tsx (1)
2-2: LGTM: Improved test isolation with per-test mocksThe refactoring successfully removes global mocks in favor of per-test mock instances injected via the
servicesoption. This improves test isolation and aligns with the dependency injection pattern. The use ofjest-mock-extendedprovides better type safety for mocks.Also applies to: 4-4, 15-17, 20-24, 42-44, 46-50, 63-65, 67-71, 85-87, 89-93, 112-114, 116-120, 140-142, 144-148, 163-165, 167-171
apps/deploy-web/src/services/remote-deploy/github-http.service.ts (1)
11-17: LGTM: Proper encapsulation of axios instanceExcellent refactoring that encapsulates the axios instance as a private class member. This improves encapsulation and makes the service more self-contained. All GitHub API calls consistently use the private instance with proper configuration.
Also applies to: 28-28, 37-37, 46-46, 55-55, 64-64, 73-73
apps/deploy-web/src/services/app-di-container/app-di-container.ts (2)
76-76: LGTM! Consistent DI pattern implementation.The change correctly updates the container to use the factory function directly instead of wrapping it, which aligns with the new DI pattern where
apiUrlServiceis expected to be a factory function in the config.
119-119: LGTM! Interface correctly updated for factory function pattern.The interface change properly reflects that
apiUrlServiceshould now be a factory function returning anApiUrlServiceinstance, which is consistent with the DI container pattern.apps/deploy-web/src/queries/useGitlabQuery.spec.tsx (2)
3-3: LGTM! Good use of jest-mock-extended for type-safe mocking.The import of
jest-mock-extendedenables better type safety and per-test isolation compared to global mocks.
25-34: LGTM! Excellent per-test mocking strategy.The use of
jest-mock-extendedwith per-test mock instances provides better test isolation and type safety. The service injection throughsetupQueryaligns well with the new DI pattern.apps/deploy-web/src/services/http/http-browser.service.ts (2)
15-15: LGTM! Factory function pattern correctly implemented.The change from using a singleton import to creating an instance via factory function aligns perfectly with the DI pattern and enables better testability and control.
20-22: LGTM! Remote deployment services properly added to DI container.The addition of GitHub, Bitbucket, and GitLab services to the container enables centralized service access through the
useServiceshook, which supports the architectural goal of this refactoring.apps/deploy-web/src/queries/useBitBucketQuery.spec.tsx (2)
3-3: LGTM! Good use of jest-mock-extended for type-safe mocking.The import of
jest-mock-extendedenables better type safety and per-test isolation compared to global mocks.
25-34: LGTM! Excellent per-test mocking strategy.The use of
jest-mock-extendedwith per-test mock instances provides better test isolation and type safety. The service injection throughsetupQueryaligns well with the new DI pattern.
stalniy
force-pushed
the
refactor/services
branch
from
6357c64 to
49aad97
Compare
2 participants
Why
For better unit testing, lazy creation and central control over axios based http requests
Summary by CodeRabbit
Refactor
Bug Fixes
Tests
Chores