From 3309fec90e8c1f4db75e33f04948d8cba03108a0 Mon Sep 17 00:00:00 2001 From: Jannik Pulfer Date: Fri, 20 Dec 2024 11:37:27 +0100 Subject: [PATCH] Refactor interceptors and duplicate objective cypress tests --- .../cypress/e2e/duplicate-objective.cy.ts | 20 +++--- frontend/src/app/app.module.ts | 6 +- .../interceptors/error.interceptor.spec.ts | 64 ++++++++++--------- ...ceptor.service.ts => error.interceptor.ts} | 4 +- ...tor.spec.ts => o-auth.interceptor.spec.ts} | 6 +- ...h.interceptor.ts => o-auth.interceptor.ts} | 10 +-- frontend/src/app/shared/constantLibary.ts | 15 ++--- 7 files changed, 65 insertions(+), 60 deletions(-) rename frontend/src/app/interceptors/{error-interceptor.service.ts => error.interceptor.ts} (96%) rename frontend/src/app/interceptors/{oauth.interceptor.spec.ts => o-auth.interceptor.spec.ts} (77%) rename frontend/src/app/interceptors/{oauth.interceptor.ts => o-auth.interceptor.ts} (78%) diff --git a/frontend/cypress/e2e/duplicate-objective.cy.ts b/frontend/cypress/e2e/duplicate-objective.cy.ts index 3b78331bea..9f494c11e4 100644 --- a/frontend/cypress/e2e/duplicate-objective.cy.ts +++ b/frontend/cypress/e2e/duplicate-objective.cy.ts @@ -10,12 +10,12 @@ beforeEach(() => { cy.loginAsUser(users.gl); }); -describe('Functionality of duplicating objectives and their belonging keyResults', () => { +describe('functionality of duplicating objectives and their belonging key-results', () => { const firstKeyResultName = 'New structure that rewards funny guys and innovation before the end of Q1.'; const secondKeyResultName = 'Monthly town halls between our people and leadership teams over the next four months.'; const thirdKeyResultName = 'High employee satisfaction scores (80%+) throughout the year.'; - it('Should be able to duplicate a objective into this quarter, including all keyResults', () => { + it('should be able to duplicate a objective into this quarter, including all keyResults', () => { const duplicatedTitle = 'This is a duplicated objective with all keyResults'; overviewPage @@ -29,7 +29,7 @@ describe('Functionality of duplicating objectives and their belonging keyResults overviewPage.getKeyResultOfObjective(duplicatedTitle, thirdKeyResultName).should('exist'); }); - it('Should be able to duplicate a objective into this quarter, only including one keyResult', () => { + it('should be able to duplicate a objective into this quarter, only including one key-result', () => { const duplicatedTitle = 'This is a duplicated objective with one keyResult'; overviewPage @@ -46,8 +46,8 @@ describe('Functionality of duplicating objectives and their belonging keyResults .should('not.contain', thirdKeyResultName); }); - it('Should not show option to select keyResults when objective with no keyResults is being duplicated', () => { - const duplicatedTitle = 'This is a duplicated objective without any keyResults'; + it('should not show option to select key-results when objective with no key-results is being duplicated', () => { + const duplicatedTitle = 'This is a duplicated objective without any key-results'; overviewPage.duplicateObjective( 'should not appear on staging, no sea takimata sanctus est Lorem ipsum dolor sit amet.', @@ -58,8 +58,8 @@ describe('Functionality of duplicating objectives and their belonging keyResults overviewPage.getObjectiveByName(duplicatedTitle).should('exist'); }); - it('Should be able to duplicate a objective into the next quarter, including all keyResults', () => { - const duplicatedTitle = 'This is a default objective with all keyResults in quarter 3!'; + it('should be able to duplicate a objective into the next quarter, including all key-results', () => { + const duplicatedTitle = 'This is a default objective with all key-results in quarter 3!'; overviewPage .duplicateObjective('Build a company culture that kills the competition.') @@ -75,7 +75,7 @@ describe('Functionality of duplicating objectives and their belonging keyResults overviewPage.getKeyResultOfObjective(duplicatedTitle, thirdKeyResultName).should('exist'); }); - it('Should not duplicate objective when cancel button is clicked', () => { + it('should not duplicate objective when cancel button is clicked', () => { const duplicatedTitle = 'This is a never existing objective'; overviewPage @@ -88,10 +88,10 @@ describe('Functionality of duplicating objectives and their belonging keyResults }); }); -describe('Verify functionality of scoring adjustment on duplicated objectives', () => { +describe('verify functionality of scoring adjustment on duplicated objectives', () => { let keyResultDetailPage = new KeyResultDetailPage(); - it('Duplicate ordinal checkin and validate value of scoring component', () => { + it('should not duplicate check-ins with objective', () => { overviewPage .addKeyResult('Puzzle ITC', 'Wir wollen die Kundenzufriedenheit steigern') .fillKeyResultTitle('stretch keyresult for testing') diff --git a/frontend/src/app/app.module.ts b/frontend/src/app/app.module.ts index bc48077897..b6b815a73b 100644 --- a/frontend/src/app/app.module.ts +++ b/frontend/src/app/app.module.ts @@ -56,8 +56,8 @@ import { ObjectiveFilterComponent } from './components/objective-filter/objectiv import { ActionPlanComponent } from './components/action-plan/action-plan.component'; import { CdkDrag, CdkDragHandle, CdkDropList } from '@angular/cdk/drag-drop'; import { SharedModule } from './shared/shared.module'; -import { OauthInterceptor } from './interceptors/oauth.interceptor'; -import { ErrorInterceptor } from './interceptors/error-interceptor.service'; +import { OAuthInterceptor } from './interceptors/o-auth.interceptor'; +import { ErrorInterceptor } from './interceptors/error.interceptor'; import { CustomRouter } from './shared/customRouter'; import { KeyResultFormComponent } from './components/key-result-form/key-result-form.component'; import { KeyResultDialogComponent } from './components/key-result-dialog/key-result-dialog.component'; @@ -179,7 +179,7 @@ export const MY_FORMATS = { deps: [MAT_DATE_LOCALE], }, { provide: MAT_DATE_FORMATS, useValue: MY_FORMATS }, - { provide: HTTP_INTERCEPTORS, useClass: OauthInterceptor, multi: true }, + { provide: HTTP_INTERCEPTORS, useClass: OAuthInterceptor, multi: true }, { provide: HTTP_INTERCEPTORS, useClass: ErrorInterceptor, multi: true }, { provide: OAuthStorage, useFactory: storageFactory }, { diff --git a/frontend/src/app/interceptors/error.interceptor.spec.ts b/frontend/src/app/interceptors/error.interceptor.spec.ts index 14477ec29b..c0a6987886 100644 --- a/frontend/src/app/interceptors/error.interceptor.spec.ts +++ b/frontend/src/app/interceptors/error.interceptor.spec.ts @@ -1,5 +1,5 @@ import { TestBed } from '@angular/core/testing'; -import { ErrorInterceptor } from './error-interceptor.service'; +import { ErrorInterceptor } from './error.interceptor'; import { ToasterService } from '../services/toaster.service'; import { TranslateModule, TranslateService } from '@ngx-translate/core'; import { Router } from '@angular/router'; @@ -30,10 +30,10 @@ describe('ErrorInterceptor', () => { }); it.each([ - ['test', 0], - ['objective', 1], - ['keyresult', 1], - ])('handleDrawerError on route %p should be called %p times', (url: string, isCalledTimes: number) => { + [0, 'test'], + [1, 'objective'], + [1, 'keyresult'], + ])('should call handleDrawerError %p times when routing to %p', (isCalledTimes: number, url: string) => { const requestMock = { url: url }; jest.spyOn(router, 'navigate'); @@ -45,7 +45,7 @@ describe('ErrorInterceptor', () => { it.each([ ['NOT_AUTHORIZED_TO_READ', ['Objective']], ['NOT_AUTHORIZED_TO_WRITE', ['Check-in']], - ])('handleErrorToaster should show correct toaster', (key: string, params: string[]) => { + ])('should show correct error toaster when calling handleErrorToaster()', (key: string, params: string[]) => { const ERROR_PREFIX = 'ERROR.'; jest.spyOn(translator, 'instant'); jest.spyOn(toaster, 'showError'); @@ -72,7 +72,7 @@ describe('ErrorInterceptor', () => { ['/keyresult/1', 200, HttpType.PUT, 'KEYRESULT', ToasterType.SUCCESS, 'Keyresult wurde aktualisiert'], ['/keyresult/1', 200, HttpType.DELETE, 'KEYRESULT', ToasterType.SUCCESS, 'Keyresult wurde gelöscht'], ])( - 'handleSuccessToaster should show toaster ', + 'should show correct success toaster when calling handleSuccessToaster()', (url: string, code: number, method: HttpType, key: string, toasterType: ToasterType, message: string) => { const SUCCESS_PREFIX = 'SUCCESS.'; jest.spyOn(translator, 'instant').mockReturnValue(message); @@ -95,22 +95,25 @@ describe('ErrorInterceptor', () => { ['/objective/1', 200, HttpType.GET, 'OBJECTIVE'], ['/keyresult/1', 200, HttpType.GET, 'KEYRESULT'], ['/keyresult/1', 200, HttpType.GET, 'KEYRESULT'], - ])('handleSuccessToaster should not show toaster ', (url: string, code: number, method: HttpType) => { - jest.spyOn(translator, 'instant'); - jest.spyOn(toaster, 'showCustomToaster'); - jest.spyOn(interceptor, 'getSuccessMessageKey').mockReturnValue(undefined); + ])( + 'should not show toaster if handleSuccessToaster() returns undefined', + (url: string, code: number, method: HttpType) => { + jest.spyOn(translator, 'instant'); + jest.spyOn(toaster, 'showCustomToaster'); + jest.spyOn(interceptor, 'getSuccessMessageKey').mockReturnValue(undefined); - const requestMock = { - url: url, - status: code, - }; + const requestMock = { + url: url, + status: code, + }; - interceptor.handleSuccessToaster(requestMock, method); - expect(interceptor.getSuccessMessageKey).toBeCalledWith(url, code, method); + interceptor.handleSuccessToaster(requestMock, method); + expect(interceptor.getSuccessMessageKey).toBeCalledWith(url, code, method); - expect(translator.instant).toBeCalledTimes(0); - expect(toaster.showCustomToaster).toBeCalledTimes(0); - }); + expect(translator.instant).toBeCalledTimes(0); + expect(toaster.showCustomToaster).toBeCalledTimes(0); + }, + ); it.each([ ['/teams/1', 200, HttpType.GET, undefined], @@ -130,21 +133,24 @@ describe('ErrorInterceptor', () => { ['/keyresults/1', 200, HttpType.DELETE, { key: 'KEY_RESULT.DELETE', toasterType: ToasterType.SUCCESS }], ['/keyresults/1', 226, HttpType.PUT, { key: 'KEY_RESULT.IM_USED', toasterType: ToasterType.WARN }], - ['/checkIns/1', 200, HttpType.GET, undefined], - ['/checkIns/1', 200, HttpType.PUT, { key: 'CHECK_IN.PUT', toasterType: ToasterType.SUCCESS }], - ['/checkIns/1', 200, HttpType.POST, { key: 'CHECK_IN.POST', toasterType: ToasterType.SUCCESS }], - ['/checkIns/1', 200, HttpType.DELETE, undefined], - ])('getSuccessMessageKey should work', (url: string, code: number, method: HttpType, result: any) => { - const successMessageKey = interceptor.getSuccessMessageKey(url, code, method); - expect(successMessageKey).toStrictEqual(result); - }); + ['/checkins/1', 200, HttpType.GET, undefined], + ['/checkins/1', 200, HttpType.PUT, { key: 'CHECK_IN.PUT', toasterType: ToasterType.SUCCESS }], + ['/checkins/1', 200, HttpType.POST, { key: 'CHECK_IN.POST', toasterType: ToasterType.SUCCESS }], + ['/checkins/1', 200, HttpType.DELETE, undefined], + ])( + 'should get correct success message object when calling getSuccessMessageKey()', + (url: string, code: number, method: HttpType, result: any) => { + const successMessageKey = interceptor.getSuccessMessageKey(url, code, method); + expect(successMessageKey).toStrictEqual(result); + }, + ); it.each([ ['http://localhost:4200/', 'http://localhost:4200/api/objecive/1', true], ['http://localhost:4200/', 'http://habasch:4200/api/objecive/1', false], ['http://localhost:4200/', 'http://habasch:4200/objecive/1', false], ])( - 'checkIfSuccessToasterIsShown should work as intended', + 'should correctly determine if toaster is shown with checkIfSuccessToasterIsShown()', (currentURL: string, requestURL: string, result: boolean) => { const requestMock = { url: requestURL }; window.location.assign(currentURL); diff --git a/frontend/src/app/interceptors/error-interceptor.service.ts b/frontend/src/app/interceptors/error.interceptor.ts similarity index 96% rename from frontend/src/app/interceptors/error-interceptor.service.ts rename to frontend/src/app/interceptors/error.interceptor.ts index c5841d0990..6556d4c462 100644 --- a/frontend/src/app/interceptors/error-interceptor.service.ts +++ b/frontend/src/app/interceptors/error.interceptor.ts @@ -77,11 +77,11 @@ export class ErrorInterceptor implements HttpInterceptor { if (toasterMessage.method == method) { for (let codeKey of toasterMessage.keysForCode || []) { if (codeKey.code == statusCode) { - const messageKey = value.KEY + '.' + codeKey.key; + const messageKey = value.key + '.' + codeKey.key; return { key: messageKey, toasterType: codeKey.toaster }; } } - const messageKey = value.KEY + '.' + method; + const messageKey = value.key + '.' + method; return { key: messageKey, toasterType: ToasterType.SUCCESS }; } } diff --git a/frontend/src/app/interceptors/oauth.interceptor.spec.ts b/frontend/src/app/interceptors/o-auth.interceptor.spec.ts similarity index 77% rename from frontend/src/app/interceptors/oauth.interceptor.spec.ts rename to frontend/src/app/interceptors/o-auth.interceptor.spec.ts index 8650d459c9..2cdb9408d2 100644 --- a/frontend/src/app/interceptors/oauth.interceptor.spec.ts +++ b/frontend/src/app/interceptors/o-auth.interceptor.spec.ts @@ -1,10 +1,10 @@ import { TestBed } from '@angular/core/testing'; -import { OauthInterceptor } from './oauth.interceptor'; +import { OAuthInterceptor } from './o-auth.interceptor'; import { HttpClientTestingModule } from '@angular/common/http/testing'; import { DateTimeProvider, OAuthLogger, OAuthModule, OAuthService, UrlHelperService } from 'angular-oauth2-oidc'; -describe('OauthInterceptor', () => { +describe('OAuthInterceptor', () => { beforeEach(() => TestBed.configureTestingModule({ imports: [HttpClientTestingModule], @@ -13,7 +13,7 @@ describe('OauthInterceptor', () => { ); it('should be created', () => { - const interceptor: OauthInterceptor = TestBed.inject(OauthInterceptor); + const interceptor: OAuthInterceptor = TestBed.inject(OAuthInterceptor); expect(interceptor).toBeTruthy(); }); }); diff --git a/frontend/src/app/interceptors/oauth.interceptor.ts b/frontend/src/app/interceptors/o-auth.interceptor.ts similarity index 78% rename from frontend/src/app/interceptors/oauth.interceptor.ts rename to frontend/src/app/interceptors/o-auth.interceptor.ts index 0b96fa28b6..2a36c65d79 100644 --- a/frontend/src/app/interceptors/oauth.interceptor.ts +++ b/frontend/src/app/interceptors/o-auth.interceptor.ts @@ -6,19 +6,19 @@ import { OAuthService } from 'angular-oauth2-oidc'; @Injectable({ providedIn: 'root', }) -export class OauthInterceptor implements HttpInterceptor { - constructor(private oauthService: OAuthService) {} +export class OAuthInterceptor implements HttpInterceptor { + constructor(private oAuthService: OAuthService) {} intercept(req: HttpRequest, next: HttpHandler): Observable> { if (!req.url.match(/^(\/)?api/)) { return next.handle(req); } return merge( - of(this.oauthService.getAccessToken()).pipe(filter((token) => !!token)), - this.oauthService.events.pipe( + of(this.oAuthService.getAccessToken()).pipe(filter((token) => !!token)), + this.oAuthService.events.pipe( filter((e) => e.type === 'token_received'), timeout(500), - map((_) => this.oauthService.getAccessToken()), + map((_) => this.oAuthService.getAccessToken()), ), ).pipe( take(1), diff --git a/frontend/src/app/shared/constantLibary.ts b/frontend/src/app/shared/constantLibary.ts index aa0e28ccf6..7ce226acfb 100644 --- a/frontend/src/app/shared/constantLibary.ts +++ b/frontend/src/app/shared/constantLibary.ts @@ -4,7 +4,7 @@ import { HttpStatusCode } from '@angular/common/http'; interface MessageKeyMap { [key: string]: { - KEY: string; + key: string; methods: { method: HttpType; keysForCode?: { @@ -23,18 +23,17 @@ export const SUCCESS_MESSAGE_KEY_PREFIX = 'SUCCESS.'; export const ERROR_MESSAGE_KEY_PREFIX = 'ERROR.'; export const DATE_FORMAT = 'dd.MM.yyyy'; -export const CONFIRM_DIALOG_WIDTH: string = '450px'; export const DRAWER_ROUTES = ['objective', 'keyresult']; export const GJ_REGEX_PATTERN = /^GJ \d{2}\/\d{2}-Q\d$/; export const SUCCESS_MESSAGE_MAP: MessageKeyMap = { teams: { - KEY: 'TEAM', + key: 'TEAM', methods: [{ method: HttpType.POST }, { method: HttpType.PUT }, { method: HttpType.DELETE }], }, objectives: { - KEY: 'OBJECTIVE', + key: 'OBJECTIVE', methods: [ { method: HttpType.POST }, { method: HttpType.DELETE }, @@ -51,7 +50,7 @@ export const SUCCESS_MESSAGE_MAP: MessageKeyMap = { ], }, keyresults: { - KEY: 'KEY_RESULT', + key: 'KEY_RESULT', methods: [ { method: HttpType.POST }, { method: HttpType.DELETE }, @@ -67,12 +66,12 @@ export const SUCCESS_MESSAGE_MAP: MessageKeyMap = { }, ], }, - checkIns: { - KEY: 'CHECK_IN', + checkins: { + key: 'CHECK_IN', methods: [{ method: HttpType.POST }, { method: HttpType.PUT }], }, user: { - KEY: 'USERS', + key: 'USERS', methods: [{ method: HttpType.PUT }, { method: HttpType.POST }, { method: HttpType.DELETE }], }, };