Skip to content

Commit

Permalink
fix: initialization error page params passing via url fragment (#443)
Browse files Browse the repository at this point in the history
* fix: using fragment instead of query params to pass error details

* feat: increased coverage and fixed code smells
  • Loading branch information
markuczy authored Dec 19, 2024
1 parent fb5663b commit 51fa292
Show file tree
Hide file tree
Showing 6 changed files with 203 additions and 38 deletions.
16 changes: 16 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
"jest-preset-angular": "14.2.4",
"jest-sonar-reporter": "^2.0.0",
"jsonc-eslint-parser": "^2.4.0",
"ngx-translate-testing": "^7.0.0",
"nx": "19.8.2",
"prettier": "^3.3.3",
"ts-jest": "^29.2.5",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { TestBed, ComponentFixture } from '@angular/core/testing'
import { ActivatedRoute } from '@angular/router'
import { of } from 'rxjs'
import { TranslateTestingModule } from 'ngx-translate-testing'
import { InitializationErrorPageComponent } from './initialization-error-page.component'

describe('InitializationErrorPageComponent', () => {
let component: InitializationErrorPageComponent
let fixture: ComponentFixture<InitializationErrorPageComponent>
let route: ActivatedRoute

const fragmentParams = {
message: 'Test Error',
requestedUrl: 'http://example.com',
detail: 'Detail message',
errorCode: '404',
invalidParams: '[param1: Invalid]',
params: '[key1: value1]'
}

beforeEach(() => {
route = {
fragment: of(new URLSearchParams(fragmentParams).toString())
} as any

TestBed.configureTestingModule({
declarations: [InitializationErrorPageComponent],
imports: [
TranslateTestingModule.withTranslations({
en: require('../../../../assets/i18n/en.json')
}).withDefaultLanguage('en')
],
providers: [{ provide: ActivatedRoute, useValue: route }]
}).compileComponents()

fixture = TestBed.createComponent(InitializationErrorPageComponent)
component = fixture.componentInstance
fixture.detectChanges()
})

it('should create', () => {
expect(component).toBeTruthy()
})

it('should display the error details in the template', () => {
fixture.detectChanges()
const compiled = fixture.nativeElement as HTMLElement

expect(compiled.querySelector('#onecxInitializationErrorMessage')?.textContent).toContain('Test Error')
expect(compiled.querySelector('#onecxInitializationErrorRequestedUrl')?.textContent).toContain('http://example.com')
expect(compiled.querySelector('#onecxInitializationErrorDetail')?.textContent).toContain('Detail message')
expect(compiled.querySelector('#onecxInitializationErrorErrorCode')?.textContent).toContain('404')
expect(compiled.querySelector('#onecxInitializationErrorInvalidParams')?.textContent).toContain('[param1: Invalid]')
expect(compiled.querySelector('#onecxInitializationErrorParams')?.textContent).toContain('[key1: value1]')
})
})
Original file line number Diff line number Diff line change
@@ -1,43 +1,52 @@
import { Component } from '@angular/core'
import { ActivatedRoute } from '@angular/router'
import { SerializedInitializationError } from '../../utils/initialization-error-handler.utils'
import { Observable, map } from 'rxjs'

interface InitializationError {
message: string
requestedUrl: string
detail: string | null
errorCode: string | null
params: string | null
invalidParams: string | null
}

@Component({
template: `<div class="p-4">
<h1 class="md:text-2xl text-lg mb-1">{{ 'INITIALIZATION_ERROR_PAGE.TITLE' | translate }}</h1>
<p class="md:text-lg text-base">{{ 'INITIALIZATION_ERROR_PAGE.SUBTITLE' | translate }}</p>
<div class="mt-3 flex flex-column row-gap-2">
<div *ngIf="error.message">
<div *ngIf="error$ | async as error" class="mt-3 flex flex-column row-gap-2">
<div id="onecxInitializationErrorMessage" *ngIf="error.message">
<div class="md:text-base text-sm">
{{ 'INITIALIZATION_ERROR_PAGE.DETAILS.MESSAGE' | translate }}
</div>
<i>{{ error.message }}</i>
</div>
<div *ngIf="error.requestedUrl">
<div id="onecxInitializationErrorRequestedUrl" *ngIf="error.requestedUrl">
<div class="md:text-base text-sm">
{{ 'INITIALIZATION_ERROR_PAGE.DETAILS.REQUESTED_URL' | translate }}
</div>
<i>{{ error.requestedUrl }}</i>
</div>
<div *ngIf="error.detail">
<div id="onecxInitializationErrorDetail" *ngIf="error.detail">
<div class="md:text-base text-sm">
{{ 'INITIALIZATION_ERROR_PAGE.DETAILS.DETAILS' | translate }}
</div>
<i>{{ error.detail }}</i>
</div>
<div *ngIf="error.errorCode">
<div id="onecxInitializationErrorErrorCode" *ngIf="error.errorCode">
<div class="md:text-base text-sm">
{{ 'INITIALIZATION_ERROR_PAGE.DETAILS.ERRORCODE' | translate }}
</div>
<i>{{ error.errorCode }}</i>
</div>
<div *ngIf="error.invalidParams">
<div id="onecxInitializationErrorInvalidParams" *ngIf="error.invalidParams">
<div class="md:text-base text-sm">
{{ 'INITIALIZATION_ERROR_PAGE.DETAILS.INVALID_PARAMS' | translate }}
</div>
<i>{{ error.invalidParams }}</i>
</div>
<div *ngIf="error.params">
<div id="onecxInitializationErrorParams" *ngIf="error.params">
<div class="md:text-base text-sm">
{{ 'INITIALIZATION_ERROR_PAGE.DETAILS.PARAMS' | translate }}
</div>
Expand All @@ -47,15 +56,21 @@ import { SerializedInitializationError } from '../../utils/initialization-error-
</div> `
})
export class InitializationErrorPageComponent {
error: SerializedInitializationError
error$: Observable<InitializationError>

constructor(private route: ActivatedRoute) {
this.error = {
message: this.route.snapshot.paramMap.get('message') ?? '',
requestedUrl: this.route.snapshot.paramMap.get('requestedUrl') ?? '',
detail: this.route.snapshot.paramMap.get('detail') ?? '',
errorCode: this.route.snapshot.paramMap.get('errorCode') ?? '',
params: this.route.snapshot.paramMap.get('params') ?? '',
invalidParams: this.route.snapshot.paramMap.get('invalidParams') ?? ''
}
this.error$ = this.route.fragment.pipe(
map((fragment) => {
const params = new URLSearchParams(fragment ?? '')
return {
message: params.get('message') ?? '',
requestedUrl: params.get('requestedUrl') ?? '',
detail: params.get('detail'),
errorCode: params.get('errorCode'),
params: params.get('params'),
invalidParams: params.get('invalidParams')
}
})
)
}
}
81 changes: 81 additions & 0 deletions src/app/shell/utils/initialization-error-handler.utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { TestBed } from '@angular/core/testing'
import { Router } from '@angular/router'
import { initializationErrorHandler } from './initialization-error-handler.utils'
import { HttpErrorResponse } from '@angular/common/http'

describe('initializationErrorHandler', () => {
let router: Router

beforeEach(() => {
TestBed.configureTestingModule({
providers: [{ provide: Router, useValue: { navigate: jest.fn() } }]
})

router = TestBed.inject(Router)
})

it('should log the error and navigate to the error page with fragment parameters for ErrorEvent', () => {
const errorEvent = new ErrorEvent('Network error', { error: { message: 'Network error occurred' } })
const consoleSpy = jest.spyOn(console, 'error')

initializationErrorHandler(errorEvent, router)

expect(consoleSpy).toHaveBeenCalledWith(errorEvent)
expect(router.navigate).toHaveBeenCalledWith(['portal-initialization-error-page'], {
fragment: expect.stringContaining('message=Network+error+occurred')
})

consoleSpy.mockRestore()
})

it('should log the error and navigate to the error page with fragment parameters for HttpErrorResponse', () => {
const httpErrorResponse = new HttpErrorResponse({
error: {
detail: 'Detail message',
errorCode: '404',
invalidParams: [{ name: 'param1', message: 'Invalid' }],
params: [{ key: 'key1', value: 'value1' }]
},
status: 404,
statusText: 'Not Found',
url: 'http://example.com'
})
;(httpErrorResponse as any)['message'] = 'HTTP error occurred'
const consoleSpy = jest.spyOn(console, 'error')

initializationErrorHandler(httpErrorResponse, router)

expect(consoleSpy).toHaveBeenCalledWith(httpErrorResponse)
expect(router.navigate).toHaveBeenCalledWith(['portal-initialization-error-page'], {
fragment: expect.stringContaining('message=HTTP+error+occurred')
})
expect(router.navigate).toHaveBeenCalledWith(['portal-initialization-error-page'], {
fragment: expect.stringContaining('detail=Detail+message')
})
expect(router.navigate).toHaveBeenCalledWith(['portal-initialization-error-page'], {
fragment: expect.stringContaining('errorCode=404')
})
expect(router.navigate).toHaveBeenCalledWith(['portal-initialization-error-page'], {
fragment: expect.stringContaining('invalidParams=%5Bparam1%3A+Invalid%5D')
})
expect(router.navigate).toHaveBeenCalledWith(['portal-initialization-error-page'], {
fragment: expect.stringContaining('params=%5Bkey1%3A+value1%5D')
})

consoleSpy.mockRestore()
})

it('should handle unknown error types gracefully', () => {
const unknownError = { message: 'Unknown error' }
const consoleSpy = jest.spyOn(console, 'error')

initializationErrorHandler(unknownError, router)

expect(consoleSpy).toHaveBeenCalledWith(unknownError)
expect(router.navigate).toHaveBeenCalledWith(['portal-initialization-error-page'], {
fragment: expect.stringContaining('message=')
})

consoleSpy.mockRestore()
})
})
38 changes: 17 additions & 21 deletions src/app/shell/utils/initialization-error-handler.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,6 @@ interface InitializationError {
details?: InitializationErrorDetails
}

export interface SerializedInitializationError {
message: string
requestedUrl: string
detail?: string
errorCode?: string
params?: string
invalidParams?: string
}

export function initializationErrorHandler(error: any, router: Router): Observable<any> {
console.error(error)
const initError: InitializationError = { message: '' }
Expand All @@ -29,19 +20,24 @@ export function initializationErrorHandler(error: any, router: Router): Observab
initError.message = error.message
}

const params: SerializedInitializationError = {
message: initError.message,
requestedUrl: window.location.href,
detail: initError.details?.detail ?? '',
errorCode: initError.details?.errorCode ?? '',
invalidParams: initError.details?.invalidParams
? `[${initError.details.invalidParams.map((invalidParam) => `${invalidParam.name}: ${invalidParam.message}`)}]`
: '',
params: initError.details?.params
? `[${initError.details.params.map((param) => `${param.key}: ${param.value}`)}]`
const params = new URLSearchParams()
params.set('message', initError.message)
params.set('requestedUrl', window.location.href)
params.set('detail', initError.details?.detail ?? '')
params.set('errorCode', initError.details?.errorCode ?? '')
params.set(
'invalidParams',
initError.details?.invalidParams
? '['.concat(initError.details.invalidParams.map((p) => `${p.name}: ${p.message}`).join(',')).concat(']')
: ''
}
)
params.set(
'params',
initError.details?.params
? '['.concat(initError.details.params.map((p) => `${p.key}: ${p.value}`).join(',')).concat(']')
: ''
)

router.navigate(['portal-initialization-error-page', params])
router.navigate(['portal-initialization-error-page'], { fragment: params.toString() })
return of(undefined)
}

0 comments on commit 51fa292

Please sign in to comment.