feat: Angular version 18 update #1146
Conversation
WalkthroughThe pull request introduces a comprehensive update to the Angular project's dependencies and testing configurations. The primary changes involve upgrading Angular-related packages from version 17.3.x to 18.2.x and modernizing the HTTP client testing setup across numerous test files. The modifications replace traditional Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🔇 Additional comments (6)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
src/app/core/services/qbd/qbd-iif-log/qbd-iif-logs.service.spec.ts (1)
Line range hint
22-26: Remove duplicate service injection.The service is being injected twice, which is unnecessary and could lead to confusion:
}); - service = TestBed.inject(QbdIifLogsService); injector = getTestBed(); service = injector.inject(QbdIifLogsService); httpMock = injector.inject(HttpTestingController);
🧹 Nitpick comments (15)
src/app/core/services/si/si-core/si-auth.service.spec.ts (1)
1-1: LGTM! Migration to new HTTP testing providers is correct.The changes properly implement Angular 18's recommended approach for HTTP client testing setup. The transition from
HttpClientTestingModuleto provider-based configuration withprovideHttpClientandprovideHttpClientTestingis correctly implemented.Consider grouping related imports together for better organization:
import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; +import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'; import { Token } from 'src/app/core/models/misc/token.model'; import { getTestBed, TestBed } from '@angular/core/testing'; import { Observable } from 'rxjs'; import { SiAuthService } from './si-auth.service'; import { environment } from 'src/environments/environment'; -import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';Also applies to: 7-7, 17-19
src/app/shared/components/helper/app-landing-page-header/app-landing-page-header.component.spec.ts (1)
18-19: Remove redundant comment.The inline comment
// Added RouterTestingModule for navigation testingis unnecessary as it's self-evident from the import.- imports: [RouterTestingModule // Added RouterTestingModule for navigation testing + imports: [RouterTestingModulesrc/app/core/services/qbd/qbd-configuration/qbd-field-mapping.service.spec.ts (1)
1-1: LGTM! Consider improving readability.The HTTP client configuration is correct, but the providers array could be more readable if split into multiple lines.
- providers: [QbdFieldMappingService, provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting()] + providers: [ + QbdFieldMappingService, + provideHttpClient(withInterceptorsFromDi()), + provideHttpClientTesting() + ]Also applies to: 7-7, 19-21
src/app/core/services/common/workspace.service.spec.ts (1)
Line range hint
40-40: Consider using more specific types in subscribe callbacksInstead of using the generic
{}type, consider defining proper interfaces for the response types to improve type safety and code maintainability.- service.postWorkspace().subscribe((value: {}) => { + service.postWorkspace().subscribe((value: WorkspaceResponse) => {- service.getWorkspace("orHVw3ikkCxJ").subscribe((value: {}) => { + service.getWorkspace("orHVw3ikkCxJ").subscribe((value: WorkspaceResponse) => {Also applies to: 65-65
src/app/core/services/qbd/qbd-configuration/qbd-advanced-setting.service.spec.ts (1)
19-21: Consider improving readability with multi-line formattingWhile the changes correctly implement Angular 18's HTTP testing setup, consider formatting the providers array across multiple lines for better readability:
- providers: [QbdAdvancedSettingService, provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting()] + providers: [ + QbdAdvancedSettingService, + provideHttpClient(withInterceptorsFromDi()), + provideHttpClientTesting() + ]src/app/core/services/travelperk/travelperk.service.spec.ts (1)
1-1: Consider documenting the new HTTP testing patternThe changes consistently implement Angular 18's new provider-based HTTP testing configuration across all test files. Consider:
- Documenting this pattern in your testing guidelines
- Creating a shared test utility to encapsulate these providers
- Updating any remaining test files to follow this pattern
This will help maintain consistency as you continue updating the codebase to Angular 18.
src/app/integrations/qbd/qbd-shared/qbd-field-mapping/qbd-field-mapping.component.spec.ts (1)
44-50: Improve TestBed configuration formatting.The providers array formatting can be improved for better readability.
declarations: [QbdFieldMappingComponent], imports: [FormsModule, ReactiveFormsModule, RouterTestingModule, SharedModule, NoopAnimationsModule], - providers: [FormBuilder, - { provide: Router, useValue: routerSpy }, - { provide: QbdFieldMappingService, useValue: service1 }, - { provide: QbdWorkspaceService, useValue: service2 }, - { provide: IntegrationsToastService, useValue: service3 }, provideHttpClient(withInterceptorsFromDi())] + providers: [ + FormBuilder, + { provide: Router, useValue: routerSpy }, + { provide: QbdFieldMappingService, useValue: service1 }, + { provide: QbdWorkspaceService, useValue: service2 }, + { provide: IntegrationsToastService, useValue: service3 }, + provideHttpClient(withInterceptorsFromDi()) + ]src/app/core/services/org/org.service.spec.ts (1)
25-31: Improve TestBed configuration formatting.The providers array formatting can be improved for better readability.
- imports: [], - providers: [ - { provide: StorageService, useValue: service1 }, - provideHttpClient(withInterceptorsFromDi()), - provideHttpClientTesting() - ] -}); + imports: [], + providers: [ + { provide: StorageService, useValue: service1 }, + provideHttpClient(withInterceptorsFromDi()), + provideHttpClientTesting() + ] + });src/app/core/services/qbd/qbd-configuration/qbd-export-setting.service.spec.ts (1)
20-22: Improve TestBed configuration formatting.The providers array should be formatted for better readability.
- imports: [], - providers: [QbdExportSettingService, provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting()] -}); + imports: [], + providers: [ + QbdExportSettingService, + provideHttpClient(withInterceptorsFromDi()), + provideHttpClientTesting() + ] + });src/app/integrations/travelperk/travelperk.component.spec.ts (1)
47-57: Improve TestBed configuration formatting.The providers array formatting can be improved for better readability.
declarations: [TravelperkComponent], imports: [], providers: [ { provide: TravelperkService, useValue: service1 }, { provide: OrgService, useValue: service2 }, { provide: EventsService, useValue: service3 }, MessageService, - provideHttpClient(withInterceptorsFromDi()), - provideHttpClientTesting() + provideHttpClient(withInterceptorsFromDi()), + provideHttpClientTesting() ] -}) + })src/app/integrations/qbd/qbd-main/qbd-mapping/qbd-generic-mapping/qbd-generic-mapping.component.spec.ts (1)
39-44: Consider reordering providers for better organization.Group related providers together for better maintainability:
- Component-specific providers (FormBuilder, MessageService)
- Service mocks (QbdMappingService)
- HTTP-related providers (provideHttpClient, provideHttpClientTesting)
declarations: [QbdGenericMappingComponent], imports: [], providers: [ + // Component providers FormBuilder, MessageService, + // Service mocks { provide: QbdMappingService, useValue: service1 }, { provide: WindowService, useValue: service2 }, { provide: IntegrationsToastService, useValue: service3 }, + // HTTP configuration provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting() ]src/app/core/interceptor/jwt.interceptor.spec.ts (1)
32-52: Consider reorganizing providers for better clarity.Group related providers together for better maintainability:
- Core services
- Interceptors
- Authentication-related providers
- HTTP configuration
imports: [], providers: [ + // Core services ApiService, + // Interceptors { provide: HTTP_INTERCEPTORS, useClass: JwtInterceptor, multi: true }, + // Authentication { provide: JWT_OPTIONS, useValue: {} }, { provide: AuthService, useValue: service1 }, JwtInterceptor, JwtHelperService, + // HTTP configuration provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting() ]src/app/integrations/bamboo-hr/bamboo-hr.component.spec.ts (1)
43-52: Consider adding error handling interceptors for tests.Add test-specific HTTP interceptors to handle and verify error responses consistently across tests.
declarations: [BambooHrComponent], imports: [], providers: [ FormBuilder, MessageService, { provide: OrgService, useValue: service1 }, { provide: BambooHrService, useValue: service2 }, + { + provide: HTTP_INTERCEPTORS, + useClass: TestErrorInterceptor, + multi: true + }, provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting() ]src/app/integrations/qbo/qbo.component.spec.ts (1)
Line range hint
36-47: Clean up window mock implementationThe window mock contains unnecessary properties that aren't used in the tests. Consider simplifying it to include only the required properties.
windowServiceMock = { get nativeWindow() { return { location: { pathname: '/integrations/qbo' - }, - // Add other required properties of Window object - clientInformation: {}, - closed: false, - customElements: {} as any, - devicePixelRatio: 1 - // Add more properties as needed + } } as Window; } };src/app/integrations/qbd/qbd-main/qbd-dashboard/qbd-dashboard.component.spec.ts (1)
1-1: HTTP client setup is correct but improve providers array formattingWhile the HTTP client setup is correctly implemented, the formatting of the providers array could be improved for better readability.
declarations: [QbdDashboardComponent], imports: [RouterTestingModule], providers: [FormBuilder, { provide: QbdIifLogsService, useValue: service1 }, { provide: QbdAdvancedSettingService, useValue: service2 }, - { provide: IntegrationsToastService, useValue: service3 }, provideHttpClient(withInterceptorsFromDi())] + { provide: IntegrationsToastService, useValue: service3 }, + provideHttpClient(withInterceptorsFromDi()) +]Also applies to: 39-45
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (52)
package.json(2 hunks)src/app/app.module.ts(2 hunks)src/app/auth/login/login.component.spec.ts(2 hunks)src/app/core/guard/auth.guard.spec.ts(2 hunks)src/app/core/interceptor/jwt.interceptor.spec.ts(2 hunks)src/app/core/interceptor/jwt.interceptor.ts(1 hunks)src/app/core/services/bamboo-hr/bamboo-hr.service.spec.ts(2 hunks)src/app/core/services/common/api.service.spec.ts(2 hunks)src/app/core/services/common/auth.service.spec.ts(2 hunks)src/app/core/services/common/clone-setting.service.spec.ts(2 hunks)src/app/core/services/common/helper.service.spec.ts(1 hunks)src/app/core/services/common/workspace.service.spec.ts(2 hunks)src/app/core/services/integration/refiner.service.spec.ts(2 hunks)src/app/core/services/misc/user.service.spec.ts(2 hunks)src/app/core/services/org/org.service.spec.ts(3 hunks)src/app/core/services/qbd/qbd-configuration/qbd-advanced-setting.service.spec.ts(2 hunks)src/app/core/services/qbd/qbd-configuration/qbd-export-setting.service.spec.ts(2 hunks)src/app/core/services/qbd/qbd-configuration/qbd-field-mapping.service.spec.ts(2 hunks)src/app/core/services/qbd/qbd-core/qbd-auth.service.spec.ts(2 hunks)src/app/core/services/qbd/qbd-core/qbd-workspace.service.spec.ts(2 hunks)src/app/core/services/qbd/qbd-iif-log/qbd-iif-logs.service.spec.ts(2 hunks)src/app/core/services/qbd/qbd-mapping/qbd-mapping.service.spec.ts(2 hunks)src/app/core/services/si/si-configuration/si-advanced-setting.service.spec.ts(1 hunks)src/app/core/services/si/si-configuration/si-export-setting.service.spec.ts(2 hunks)src/app/core/services/si/si-configuration/si-import-setting.service.spec.ts(1 hunks)src/app/core/services/si/si-core/si-auth.service.spec.ts(2 hunks)src/app/core/services/si/si-core/si-workspace.service.spec.ts(2 hunks)src/app/core/services/travelperk/travelperk.service.spec.ts(2 hunks)src/app/integrations/bamboo-hr/bamboo-hr.component.spec.ts(2 hunks)src/app/integrations/bamboo-hr/configuration/configuration.component.spec.ts(2 hunks)src/app/integrations/intacct/intacct-main/intacct-dashboard/intacct-dashboard.component.spec.ts(3 hunks)src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-export-log.component.spec.ts(2 hunks)src/app/integrations/intacct/intacct-shared/intacct-c1-import-settings/intacct-c1-import-settings.component.spec.ts(3 hunks)src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.spec.ts(3 hunks)src/app/integrations/intacct/intacct.component.spec.ts(2 hunks)src/app/integrations/integrations.component.spec.ts(2 hunks)src/app/integrations/landing/landing.component.spec.ts(2 hunks)src/app/integrations/qbd/qbd-main/qbd-dashboard/qbd-dashboard.component.spec.ts(2 hunks)src/app/integrations/qbd/qbd-main/qbd-main.component.spec.ts(2 hunks)src/app/integrations/qbd/qbd-main/qbd-mapping/qbd-generic-mapping/qbd-generic-mapping.component.spec.ts(3 hunks)src/app/integrations/qbd/qbd-shared/qbd-advanced-setting/qbd-advanced-setting.component.spec.ts(2 hunks)src/app/integrations/qbd/qbd-shared/qbd-export-setting/qbd-export-setting.component.spec.ts(2 hunks)src/app/integrations/qbd/qbd-shared/qbd-field-mapping/qbd-field-mapping.component.spec.ts(2 hunks)src/app/integrations/qbd/qbd.component.spec.ts(2 hunks)src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.spec.ts(4 hunks)src/app/integrations/qbo/qbo.component.spec.ts(2 hunks)src/app/integrations/travelperk/travelperk.component.spec.ts(2 hunks)src/app/shared/components/configuration/configuration-step-header/configuration-step-header.component.spec.ts(1 hunks)src/app/shared/components/helper/app-landing-page-header/app-landing-page-header.component.spec.ts(2 hunks)src/app/shared/components/qbd/helper/qbd-onboarding-stepper/qbd-onboarding-stepper.component.spec.ts(2 hunks)src/app/shared/components/si/core/intacct-connector/intacct-connector.component.spec.ts(2 hunks)src/app/shared/components/si/helper/skip-export/skip-export.component.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/core/interceptor/jwt.interceptor.ts
🔇 Additional comments (60)
src/app/core/services/si/si-configuration/si-import-setting.service.spec.ts (2)
4-5: LGTM! HTTP client testing setup follows Angular v18 best practices.The migration from HttpClientTestingModule to provider-based configuration is implemented correctly.
Also applies to: 12-14
7-7: Verify: Disabled test suite needs attention.The test suite is currently disabled (xdescribe). Please verify if this is intentional or if it should be enabled as part of the Angular upgrade.
src/app/core/services/si/si-configuration/si-advanced-setting.service.spec.ts (2)
4-5: LGTM! Consistent implementation of HTTP client testing setup.The changes align with the Angular v18 upgrade pattern seen across other files.
Also applies to: 12-14
7-7: Verify: Another disabled test suite found.This is another disabled test suite (xdescribe). Consider creating a tracking issue to enable all disabled tests after the Angular upgrade.
Would you like me to create a GitHub issue to track the task of enabling all disabled test suites?
src/app/core/services/misc/user.service.spec.ts (1)
3-4: LGTM! HTTP client testing setup is properly configured.The provider-based configuration is correctly implemented.
Also applies to: 13-15
src/app/shared/components/si/helper/skip-export/skip-export.component.spec.ts (2)
4-4: LGTM! Component testing setup follows Angular v18 patterns.The HTTP client and form testing configuration is correctly implemented.
Also applies to: 6-6, 14-17
8-8: Verify: Multiple levels of disabled tests.Both the test suite (xdescribe) and individual test case (xit) are disabled. This could indicate incomplete test implementation or migration issues.
Would you like me to help implement the missing test cases for this component?
Also applies to: 25-25
src/app/core/services/common/helper.service.spec.ts (2)
8-8: Test suite is disabled - needs attentionThe test suite is marked with
xdescribe, which means it's currently skipped. Please either:
- Remove the 'x' prefix if tests are working correctly with Angular 18
- Add a TODO comment explaining why tests are disabled
- Fix any failing tests that led to disabling the suite
14-20: HTTP client testing setup looks goodThe migration from
HttpClientTestingModuleto provider-based configuration is correctly implemented using:
- Empty imports array
- Proper providers setup with
provideHttpClientandprovideHttpClientTestingsrc/app/shared/components/configuration/configuration-step-header/configuration-step-header.component.spec.ts (1)
13-16: Provider setup is consistent with Angular 18 patternsThe HTTP client testing configuration follows the recommended approach for Angular 18:
- Correct usage of
provideHttpClientandprovideHttpClientTesting- Empty imports array
src/app/core/services/common/clone-setting.service.spec.ts (1)
17-19: HTTP testing configuration is correctThe setup properly includes:
- HttpTestingController for mocking HTTP requests
- Provider-based HTTP client configuration
src/app/core/services/integration/refiner.service.spec.ts (2)
17-19: Good reference implementation for Angular 18 testingThis test suite:
- Is properly enabled (not using xdescribe)
- Correctly implements the new HTTP testing providers
- Can serve as a reference for fixing other disabled test suites
1-1:⚠️ Systematic issues need attentionWhile the HTTP client testing changes for Angular 18 are consistently implemented across all files, there's a concerning pattern of disabled test suites (
xdescribe). Recommendations:
- Use the working
refiner.service.spec.tsas a reference- Document any Angular 18 breaking changes affecting tests
- Create a tracking issue for re-enabling all test suites
Would you like me to:
- Generate a detailed comparison between working and disabled tests?
- Create a GitHub issue to track the test fixes?
src/app/core/services/si/si-configuration/si-export-setting.service.spec.ts (1)
18-20: LGTM! Proper implementation of Angular v18's provider-based testing setup.The transition from HttpClientTestingModule to provideHttpClient + provideHttpClientTesting follows the recommended pattern for Angular v18.
src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-export-log.component.spec.ts (1)
16-19: LGTM! Comprehensive provider configuration for Angular v18.The setup includes all necessary providers (FormBuilder, Router, HttpClient) with proper configuration for testing.
src/app/core/guard/auth.guard.spec.ts (1)
22-28: LGTM! Proper integration of new HTTP client provider with existing test setup.Good practice maintaining RouterTestingModule while transitioning to the new HTTP client provider pattern.
src/app/auth/login/login.component.spec.ts (3)
Line range hint
11-11: Verify why tests are skipped.The test suite is marked with
xdescribe, indicating that these tests are currently skipped. Please verify if this is intentional or if the tests need to be updated for Angular v18 compatibility.
26-41: LGTM! Proper provider configuration for Angular v18 testing.The test setup correctly implements the new provider-based approach while maintaining all necessary mock providers for ActivatedRoute, AuthService, and Router.
1-1: Verify Angular v18 compatibility across the codebase.While the HTTP client provider changes look good, it would be beneficial to verify that all test suites are running successfully with Angular v18. Run the following command to check for any skipped or failing tests:
✅ Verification successful
HTTP client configuration is correctly implemented ✅
The HTTP client provider changes in the test setup are properly implemented using
provideHttpClient(withInterceptorsFromDi()). While there are skipped tests in the codebase, they are not specifically related to HTTP client configuration and appear to be pre-existing. The skipped tests focus on component creation and navigation functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all spec files and check for xdescribe echo "Checking for skipped test suites..." rg -l "xdescribe" --type ts # Note: In a real environment, you would also run the test suite # but since we can't run tests in this environment, we're just checking for patternsLength of output: 28571
Script:
#!/bin/bash # Check content of login component spec file for HTTP client related tests echo "Content of login.component.spec.ts:" cat src/app/auth/login/login.component.spec.tsLength of output: 1982
src/app/core/services/qbd/qbd-core/qbd-auth.service.spec.ts (1)
1-1: LGTM! Consistent implementation of HTTP testing providers.The changes maintain consistency with other service specs and properly implement Angular 18's HTTP testing configuration.
Also applies to: 8-8, 18-20
src/app/integrations/qbd/qbd.component.spec.ts (1)
1-2: LGTM! Component test configuration updated correctly.The changes properly implement Angular 18's HTTP testing setup while maintaining other necessary testing modules and providers.
Since this is a component test, verify that all HTTP interactions (if any) in the component work as expected with the new provider setup. Run the following to check for any HTTP-related code in the component:
Also applies to: 28-35
✅ Verification successful
HTTP client configuration is correctly set up for testing the QBD component
The test configuration properly supports the component's HTTP operations which are handled through injected services (QbdWorkspaceService). The providers
provideHttpClientandprovideHttpClientTestingare correctly configured to support these service dependencies.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for HTTP client usage in the QbdComponent ast-grep --pattern 'class QbdComponent { $$$ constructor($_) { $$$ } $$$ }'Length of output: 89
Script:
#!/bin/bash # Find the component file fd -t f qbd.component.ts # Search for HttpClient usage rg "HttpClient" -A 5 --type tsLength of output: 75556
Script:
#!/bin/bash # Get QbdComponent implementation cat src/app/integrations/qbd/qbd.component.ts # Look for service injections in constructor ast-grep --pattern 'constructor($$$) { $$$ }' src/app/integrations/qbd/qbd.component.tsLength of output: 3072
src/app/shared/components/qbd/helper/qbd-onboarding-stepper/qbd-onboarding-stepper.component.spec.ts (1)
1-1: LGTM! Proper implementation of new HTTP testing configuration.The changes correctly implement Angular 18's HTTP client testing setup while maintaining all necessary modules and providers for the component test.
Also applies to: 25-32
src/app/integrations/qbd/qbd-main/qbd-main.component.spec.ts (1)
4-4: LGTM! HTTP client configuration updated correctly.The changes properly migrate the test configuration to use the new provider-based approach introduced in Angular 18, while maintaining other necessary configurations.
Also applies to: 11-11, 30-38
src/app/integrations/integrations.component.spec.ts (2)
1-2: LGTM! HTTP client configuration updated correctly.The changes properly implement the new provider-based HTTP client configuration.
Also applies to: 24-32
Line range hint
11-11: Verify if test suite should remain disabled.The test suite is currently disabled with
xdescribe. Please verify if this was intentional or if it should be re-enabled as part of the Angular update.src/app/shared/components/helper/app-landing-page-header/app-landing-page-header.component.spec.ts (1)
1-2: LGTM! HTTP client configuration updated correctly.The changes properly implement the new provider-based HTTP client configuration while maintaining the RouterTestingModule.
Also applies to: 17-25
src/app/core/services/qbd/qbd-configuration/qbd-field-mapping.service.spec.ts (1)
Line range hint
11-11: Review disabled test suites across the codebase.Multiple test suites are currently disabled with
xdescribe. As part of the Angular 18 update, it would be beneficial to:
- Verify if these tests were disabled due to the update
- Create a plan to fix and re-enable them
- Ensure no regression issues are introduced
Also applies to: 9-9, 9-9
src/app/integrations/landing/landing.component.spec.ts (1)
6-7: HTTP client configuration updated correctly for Angular 18.The transition from HttpClientModule/HttpClientTestingModule to the new provider-based approach is implemented correctly.
Consider adding HTTP interceptor tests.
Since you're using
withInterceptorsFromDi(), consider adding test cases to verify that HTTP interceptors are properly applied.Also applies to: 30-38
src/app/app.module.ts (1)
8-8: HTTP client configuration updated correctly for Angular 18.The transition to
provideHttpClientwithwithInterceptorsFromDiis implemented correctly.Also applies to: 65-65
src/app/core/services/common/auth.service.spec.ts (1)
3-3: HTTP client testing configuration updated correctly for Angular 18.The transition to provider-based HTTP client testing is implemented correctly.
Also applies to: 8-8, 25-32
src/app/core/services/common/api.service.spec.ts (1)
3-3: HTTP client testing configuration updated correctly for Angular 18.The transition to provider-based HTTP client testing is implemented correctly.
Also applies to: 6-6, 22-28
src/app/core/services/si/si-core/si-workspace.service.spec.ts (2)
9-9: Re-enable the test suiteThe test suite is currently disabled with
xdescribe. Consider re-enabling it by changing todescribeto ensure these tests are executed as part of the test suite.
17-19: LGTM! Proper implementation of Angular v18's HTTP testing setupThe migration from
HttpClientTestingModuleto the new provider-based approach is correctly implemented usingprovideHttpClientwithwithInterceptorsFromDi()andprovideHttpClientTesting().src/app/core/services/common/workspace.service.spec.ts (2)
9-9: Re-enable the test suiteThe test suite is currently disabled with
xdescribe. Consider re-enabling it by changing todescribeto ensure these tests are executed as part of the test suite.
18-20: LGTM! Proper implementation of Angular v18's HTTP testing setupThe migration from
HttpClientTestingModuleto the new provider-based approach is correctly implemented usingprovideHttpClientwithwithInterceptorsFromDi()andprovideHttpClientTesting().src/app/core/services/qbd/qbd-core/qbd-workspace.service.spec.ts (2)
9-9: Re-enable the test suiteThe test suite is currently disabled with
xdescribe. Consider re-enabling it by changing todescribeto ensure these tests are executed as part of the test suite.
17-19: LGTM! Proper implementation of Angular v18's HTTP testing setupThe migration from
HttpClientTestingModuleto the new provider-based approach is correctly implemented usingprovideHttpClientwithwithInterceptorsFromDi()andprovideHttpClientTesting().src/app/integrations/bamboo-hr/configuration/configuration.component.spec.ts (3)
Line range hint
10-10: Re-enable the test suiteThe test suite is currently disabled with
xdescribe. Consider re-enabling it by changing todescribeto ensure these tests are executed as part of the test suite.
22-29: LGTM! Proper implementation of Angular v18's HTTP testing setupThe migration from
HttpClientModuleto the new provider-based approach is correctly implemented usingprovideHttpClientwithwithInterceptorsFromDi(). The component's testing configuration is properly set up with the necessary providers and declarations.
1-1: Verify all disabled test suites in the codebaseLet's check for any other disabled test suites that might need to be re-enabled as part of this update.
src/app/core/services/bamboo-hr/bamboo-hr.service.spec.ts (1)
22-28: LGTM! Modern HTTP testing setupThe changes correctly implement Angular 18's recommended approach for HTTP client testing by replacing the module-based configuration with the new provider-based setup.
src/app/shared/components/si/core/intacct-connector/intacct-connector.component.spec.ts (1)
29-40: LGTM! Proper component test configurationThe changes correctly implement Angular 18's HTTP client testing setup while maintaining proper component test configuration with all required dependencies.
src/app/core/services/travelperk/travelperk.service.spec.ts (1)
22-28: LGTM! Consistent HTTP testing setupThe changes correctly implement Angular 18's HTTP client testing configuration, maintaining consistency with other service tests in the codebase.
src/app/core/services/org/org.service.spec.ts (1)
12-12: Verify if test suite should be disabled.The test suite is currently disabled with
xdescribe. Please verify if this is intentional or if it should be re-enabled.src/app/core/services/qbd/qbd-configuration/qbd-export-setting.service.spec.ts (1)
10-10: Verify if test suite should be disabled.The test suite is currently disabled with
xdescribe. Please verify if this is intentional or if it should be re-enabled.src/app/integrations/travelperk/travelperk.component.spec.ts (2)
Line range hint
13-13: Verify if test suite should be disabled.The test suite is currently disabled with
xdescribe. Please verify if this is intentional or if it should be re-enabled.
1-1: Angular 18 HTTP Client Testing Migration: Overall AssessmentThe migration from HttpClientModule/HttpClientTestingModule to provideHttpClient/provideHttpClientTesting is correctly implemented across all files. However, there are a few items that need attention:
- Several test suites are disabled with
xdescribe. Please verify if this is intentional.- Consider standardizing the TestBed configuration formatting across all spec files for better maintainability.
The migration pattern is consistent and follows Angular 18's best practices.
src/app/integrations/intacct/intacct.component.spec.ts (1)
14-15: HTTP client testing setup looks good!The transition to the new provider-based approach for HTTP client testing is implemented correctly. The setup includes both
provideHttpClientwith interceptors andprovideHttpClientTesting, following Angular v18's best practices.Also applies to: 44-56
src/app/integrations/qbo/qbo.component.spec.ts (1)
14-15: HTTP client testing setup looks good!The transition to the new provider-based approach for HTTP client testing is implemented correctly, following Angular v18's best practices.
Also applies to: 51-64
src/app/core/services/qbd/qbd-mapping/qbd-mapping.service.spec.ts (2)
4-4: HTTP client testing setup looks good!The transition to the new provider-based approach for HTTP client testing is implemented correctly.
Also applies to: 9-9, 21-23
11-11: Verify if test suite should be disabledThe test suite is currently disabled with
xdescribe. Please verify if this is intentional or if it should be enabled.✅ Verification successful
Disabled test suite is part of a systematic pattern
The
xdescribein this file is part of a codebase-wide pattern where all QBD-related tests are currently disabled. This appears to be an intentional architectural decision as it consistently applies to all QBD services and components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if other QBD test suites are also disabled rg "xdescribe.*QBD" -A 1Length of output: 26
Script:
#!/bin/bash # Find all QBD-related test files echo "=== QBD Test Files ===" fd "qbd.*spec.ts$" echo -e "\n=== Disabled Tests in QBD Files ===" # Search for any disabled tests (xdescribe, xit, xtest) in QBD files fd "qbd.*spec.ts$" --exec rg -i "xdescribe|xit|xtest" {} -A 1 || true echo -e "\n=== Git History for the File ===" # Check recent history of this specific file git log -p --format="%h %s" src/app/core/services/qbd/qbd-mapping/qbd-mapping.service.spec.ts | head -n 20Length of output: 10618
src/app/integrations/intacct/intacct-main/intacct-dashboard/intacct-dashboard.component.spec.ts (1)
3-3: LGTM! Modern HTTP client testing setupThe changes correctly implement Angular's new provider-based approach for HTTP client testing, replacing the older module-based setup with
provideHttpClientandprovideHttpClientTesting.Also applies to: 17-17, 39-51
src/app/integrations/qbd/qbd-shared/qbd-advanced-setting/qbd-advanced-setting.component.spec.ts (1)
1-1: LGTM! Modern HTTP client setupThe changes correctly implement Angular's new provider-based approach for HTTP client configuration, replacing the older module-based setup with
provideHttpClient.Also applies to: 51-62
src/app/integrations/qbd/qbd-shared/qbd-export-setting/qbd-export-setting.component.spec.ts (1)
1-1: LGTM! Modern HTTP client setupThe changes correctly implement Angular's new provider-based approach for HTTP client configuration, replacing the older module-based setup with
provideHttpClient.Also applies to: 53-64
src/app/integrations/intacct/intacct-shared/intacct-c1-import-settings/intacct-c1-import-settings.component.spec.ts (1)
31-31: LGTM! Modern HTTP client testing setupThe changes correctly implement Angular's new provider-based approach for HTTP client testing, replacing the older module-based setup with
provideHttpClientandprovideHttpClientTesting.Also applies to: 36-36, 73-89
package.json (1)
53-53: Confirm zone.js compatibility.The zone.js update to ~0.14.10 is compatible with Angular 18.
src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.spec.ts (2)
20-24: Update to modern HTTP testing pattern.The test configuration has been updated to use the new provider-based approach for HTTP testing, which is the recommended pattern in Angular 18.
Line range hint
51-67: Verify test module configuration.The TestBed configuration has been properly updated to use the new HTTP testing providers. The order of providers is correct with the router providers preceding HTTP providers.
src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.spec.ts (2)
8-8: Update to modern HTTP testing pattern.The test configuration has been updated to use the new provider-based approach for HTTP testing, aligning with Angular 18 best practices.
Also applies to: 45-45
Line range hint
82-105: Verify test module configuration.The TestBed configuration has been properly updated with:
- Removal of HttpClientTestingModule
- Addition of provideHttpClient and provideHttpClientTesting
- Correct ordering of providers
| import { HttpClientTestingModule } from '@angular/common/http/testing'; | ||
| import { HttpClientModule } from '@angular/common/http'; | ||
| import { provideHttpClientTesting } from '@angular/common/http/testing'; | ||
| import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'; | ||
| import { minimalUser } from '../../interceptor/jwt.fixture'; | ||
|
|
||
| xdescribe('UserService', () => { |
There was a problem hiding this comment.
Critical: Disabled test suite contains active test cases.
This test suite contains implemented test cases for user profile functionality but is currently disabled (xdescribe). This should be prioritized for re-enabling to ensure critical user functionality remains tested.
| @@ -1,17 +1,19 @@ | |||
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | |||
|
|
|||
| import { ConfigurationStepHeaderComponent } from './configuration-step-header.component'; | |||
| import { HttpClientTestingModule } from '@angular/common/http/testing'; | |||
| import { provideHttpClientTesting } from '@angular/common/http/testing'; | |||
| import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'; | |||
|
|
|||
| xdescribe('ConfigurationStepHeaderComponent', () => { | |||
There was a problem hiding this comment.
💡 Codebase verification
Widespread test suite disablement confirmed
Found extensive evidence of disabled test suites across the codebase, with most component tests being disabled using xdescribe. This systematic disablement spans across core, integration, and shared components, confirming the pattern of disabled tests during the Angular 18 upgrade.
- Over 100 test suites are disabled using
xdescribe - Multiple individual test cases are disabled using
xit - Affected areas include QBO, Xero, Sage300, and shared components
🔗 Analysis chain
Multiple disabled test suites detected
This is another test suite marked with xdescribe. The pattern of disabled tests across multiple files suggests there might be common issues with the Angular 18 upgrade that need to be addressed.
Run this script to check for other disabled tests:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all disabled test suites in spec files
echo "Checking for disabled test suites..."
rg "xdescribe|fdescribe|xit|fit" "src/**/*.spec.ts"
Length of output: 193
Script:
#!/bin/bash
# First, verify the existence of spec files
echo "Finding spec files..."
fd "\.spec\.ts$"
# Then search for disabled tests with context
echo -e "\nChecking for disabled test suites..."
fd "\.spec\.ts$" | xargs rg "xdescribe|fdescribe|xit|fit" -A 2
Length of output: 103514
| provide: ErrorHandler, | ||
| useClass: GlobalErrorHandler | ||
| }, | ||
| { | ||
| provide: ErrorHandler, | ||
| useValue: Sentry.createErrorHandler() | ||
| }, |
There was a problem hiding this comment.
Fix duplicate ErrorHandler providers.
There are two ErrorHandler providers which could lead to conflicts:
GlobalErrorHandlerSentry.createErrorHandler()
This might cause unpredictable error handling behavior.
Consider consolidating them into a single provider that combines both handlers:
- {
- provide: ErrorHandler,
- useClass: GlobalErrorHandler
- },
- {
- provide: ErrorHandler,
- useValue: Sentry.createErrorHandler()
- },
+ {
+ provide: ErrorHandler,
+ useClass: class CombinedErrorHandler extends GlobalErrorHandler {
+ constructor() {
+ super();
+ const sentryHandler = Sentry.createErrorHandler();
+ const originalHandleError = this.handleError;
+ this.handleError = (error: any) => {
+ sentryHandler.handleError(error);
+ originalHandleError.call(this, error);
+ };
+ }
+ }
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| provide: ErrorHandler, | |
| useClass: GlobalErrorHandler | |
| }, | |
| { | |
| provide: ErrorHandler, | |
| useValue: Sentry.createErrorHandler() | |
| }, | |
| provide: ErrorHandler, | |
| useClass: class CombinedErrorHandler extends GlobalErrorHandler { | |
| constructor() { | |
| super(); | |
| const sentryHandler = Sentry.createErrorHandler(); | |
| const originalHandleError = this.handleError; | |
| this.handleError = (error: any) => { | |
| sentryHandler.handleError(error); | |
| originalHandleError.call(this, error); | |
| }; | |
| } | |
| } | |
| }, |
| import { UserService } from '../misc/user.service'; | ||
| import { environment } from 'src/environments/environment'; | ||
| import { Router } from '@angular/router'; | ||
| import { loginResponse, minimalUser, tokenResponse } from '../../interceptor/jwt.fixture'; | ||
| import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'; | ||
|
|
||
| xdescribe('AuthService', () => { |
There was a problem hiding this comment.
Re-enable test suite.
The test suite is currently disabled with xdescribe. Consider re-enabling it to ensure the tests are running as part of your test suite.
-xdescribe('AuthService', () => {
+describe('AuthService', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| xdescribe('AuthService', () => { | |
| describe('AuthService', () => { |
| import { environment } from 'src/environments/environment'; | ||
| import { ApiService } from './api.service'; | ||
| import { HttpErrorResponse, HttpEventType, HttpHeaders } from '@angular/common/http'; | ||
| import { HttpErrorResponse, HttpEventType, HttpHeaders, provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'; | ||
| import { Observable } from 'rxjs'; | ||
|
|
||
| xdescribe('ApiService', () => { |
There was a problem hiding this comment.
Re-enable test suite.
The test suite is currently disabled with xdescribe. Consider re-enabling it to ensure the tests are running as part of your test suite.
-xdescribe('ApiService', () => {
+describe('ApiService', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| xdescribe('ApiService', () => { | |
| describe('ApiService', () => { |
| provide: JWT_OPTIONS, | ||
| useValue: JWT_OPTIONS |
There was a problem hiding this comment.
Fix JWT_OPTIONS provider configuration.
The current JWT_OPTIONS provider configuration is incorrect as it's using the token itself as the value.
{
provide: JWT_OPTIONS,
- useValue: JWT_OPTIONS
+ useValue: {} // Add your JWT configuration options here
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| provide: JWT_OPTIONS, | |
| useValue: JWT_OPTIONS | |
| provide: JWT_OPTIONS, | |
| useValue: {} // Add your JWT configuration options here |
package.json
Outdated
| "@angular-devkit/build-angular": "^18.2.12", | ||
| "@angular-eslint/schematics": "^17.5.1", | ||
| "@angular/animations": "^17.3.10", | ||
| "@angular/cli": "^17.3.8", | ||
| "@angular/common": "^17.3.10", | ||
| "@angular/compiler": "^17.3.10", | ||
| "@angular/compiler-cli": "^17.3.10", | ||
| "@angular/core": "^17.3.10", | ||
| "@angular/forms": "^17.3.10", | ||
| "@angular/platform-browser": "^17.3.10", | ||
| "@angular/platform-browser-dynamic": "^17.3.10", | ||
| "@angular/router": "^17.3.10", | ||
| "@angular/animations": "^18.2.13", | ||
| "@angular/cli": "^18.2.12", | ||
| "@angular/common": "^18.2.13", | ||
| "@angular/compiler": "^18.2.13", | ||
| "@angular/compiler-cli": "^18.2.13", | ||
| "@angular/core": "^18.2.13", | ||
| "@angular/forms": "^18.2.13", | ||
| "@angular/platform-browser": "^18.2.13", | ||
| "@angular/platform-browser-dynamic": "^18.2.13", | ||
| "@angular/router": "^18.2.13", |
There was a problem hiding this comment.
💡 Codebase verification
Several packages need to be updated to be compatible with Angular 18:
- @angular-eslint/* packages are severely outdated (v14.1.2)
- @angular-eslint/schematics (v17.5.1) needs to be updated to v18
- Consider updating @auth0/angular-jwt to ensure compatibility
Please run:
ng update @angular-eslint/schematics@18
ng update @angular-eslint/builder@18🔗 Analysis chain
Verify Angular 18 migration checklist.
The Angular packages have been updated from v17.3.x to v18.2.x. Ensure that:
- The Angular update guide has been followed (https://update.angular.io/?v=17.3-18.0)
- All breaking changes have been addressed
- The application has been tested thoroughly
Run the following script to check for any missed peer dependencies or version mismatches:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify package versions and potential conflicts
npm ls @angular/core @angular/common @angular/platform-browser @angular/platform-browser-dynamic @angular/forms @angular/router @angular/compiler @angular/compiler-cli @angular/animations @angular/cli @angular-devkit/build-angular
Length of output: 691
Script:
#!/bin/bash
# Get all Angular-related dependencies and their versions
cat package.json | jq -r '.dependencies, .devDependencies | to_entries[] | select(.key | contains("@angular") or contains("angular-")) | "\(.key): \(.value)"'
# Check for zone.js version which is crucial for Angular
cat package.json | jq -r '.dependencies["zone.js"] // .devDependencies["zone.js"] // "not_found"'
# Check TypeScript version
cat package.json | jq -r '.dependencies["typescript"] // .devDependencies["typescript"] // "not_found"'
Length of output: 936
Description
feat: Angular version 18 update
Clickup
https://app.clickup.com
Summary by CodeRabbit
Dependency Updates
zone.jsto version 0.14.10Testing Framework
provideHttpClientandwithInterceptorsFromDi()for more flexible HTTP client setupHttpClientModuleandHttpClientTestingModulewith new provider methods