From 78869e6fc3514607c921d97bde658526bd3e530d Mon Sep 17 00:00:00 2001 From: Lias Kleisa Date: Wed, 19 Jun 2024 14:36:25 +0200 Subject: [PATCH] Use second refresh subject --- frontend/cypress/e2e/checkIn.cy.ts | 16 ------- frontend/cypress/e2e/scoring.cy.ts | 1 - .../src/app/diagram/diagram.component.html | 4 +- .../src/app/diagram/diagram.component.spec.ts | 5 +- frontend/src/app/diagram/diagram.component.ts | 47 ++++++++----------- .../src/app/overview/overview.component.html | 2 +- .../app/overview/overview.component.spec.ts | 6 +-- .../src/app/overview/overview.component.ts | 24 +++------- .../shared/services/refresh-data.service.ts | 6 ++- 9 files changed, 37 insertions(+), 74 deletions(-) diff --git a/frontend/cypress/e2e/checkIn.cy.ts b/frontend/cypress/e2e/checkIn.cy.ts index 33e5dfc368..e3df4724d0 100644 --- a/frontend/cypress/e2e/checkIn.cy.ts +++ b/frontend/cypress/e2e/checkIn.cy.ts @@ -32,8 +32,6 @@ describe('OKR Check-in e2e tests', () => { checkForDialogTextMetric(); cy.fillOutCheckInMetric(30, 6, 'We bought a new house', 'We have to buy more PCs'); - cy.wait(1000); - cy.contains('30%'); cy.contains('6/10'); cy.contains('Letztes Check-in (' + getCurrentDate() + ')'); @@ -64,8 +62,6 @@ describe('OKR Check-in e2e tests', () => { checkForDialogTextMetric(); cy.fillOutCheckInMetric(30, 0, 'We bought a new house', 'We have to buy more PCs'); - cy.wait(100); - cy.contains('30%'); cy.contains('6/10'); cy.contains('Letztes Check-in (' + getCurrentDate() + ')'); @@ -96,8 +92,6 @@ describe('OKR Check-in e2e tests', () => { checkForDialogTextMetric(); cy.fillOutCheckInMetric(5, 5, null, null); - cy.wait(500); - cy.contains('5%'); cy.contains('!'); cy.contains('5/10'); @@ -130,8 +124,6 @@ describe('OKR Check-in e2e tests', () => { checkForDialogTextOrdinal(); cy.fillOutCheckInOrdinal(1, 6, 'There is a new car', 'Buy now a new pool'); - cy.wait(500); - cy.contains('6/10'); cy.contains('There is a new car'); cy.contains('Letztes Check-in (' + getCurrentDate() + ')'); @@ -162,8 +154,6 @@ describe('OKR Check-in e2e tests', () => { cy.getByTestId('add-check-in').first().click(); cy.fillOutCheckInMetric(50, 6, 'This was a good idea', 'Will be difficult'); - cy.wait(500); - cy.getByTestId('show-all-checkins').click(); cy.wait(500); @@ -201,9 +191,6 @@ describe('OKR Check-in e2e tests', () => { cy.getByTestId('add-check-in').first().click(); cy.fillOutCheckInMetric(30, 5, 'Here we are', 'A cat would be great'); - - cy.wait(500); - cy.contains('Aktuell: CHF 30.-'); cy.getByTestId('show-all-checkins').click(); @@ -249,9 +236,6 @@ describe('OKR Check-in e2e tests', () => { cy.getByTestId('keyresult').contains('For editing ordinal checkin').click(); cy.getByTestId('add-check-in').first().click(); cy.fillOutCheckInOrdinal(0, 6, 'There is a new car', 'Buy now a new pool'); - - cy.wait(500); - cy.getByTestId('show-all-checkins').click(); cy.wait(500); diff --git a/frontend/cypress/e2e/scoring.cy.ts b/frontend/cypress/e2e/scoring.cy.ts index 82d5cc6b72..1a22f91596 100644 --- a/frontend/cypress/e2e/scoring.cy.ts +++ b/frontend/cypress/e2e/scoring.cy.ts @@ -78,7 +78,6 @@ describe('Scoring component e2e tests', () => { function setupMetricKR(baseline: number, stretchgoal: number, value: number) { cy.createMetricKeyresult('Metric scoring keyresult', String(baseline), String(stretchgoal)); - cy.wait(500); cy.getByTestId('keyresult').get(':contains("Metric scoring keyresult")').last().click(); cy.getByTestId('add-check-in').click(); cy.getByTestId('check-in-metric-value').clear().type(String(value)); diff --git a/frontend/src/app/diagram/diagram.component.html b/frontend/src/app/diagram/diagram.component.html index 2491b38045..9e34e3ff55 100644 --- a/frontend/src/app/diagram/diagram.component.html +++ b/frontend/src/app/diagram/diagram.component.html @@ -1,7 +1,7 @@ -
+

Kein Alignment vorhanden

diff --git a/frontend/src/app/diagram/diagram.component.spec.ts b/frontend/src/app/diagram/diagram.component.spec.ts index 4b6a6b0285..dda3c6e3aa 100644 --- a/frontend/src/app/diagram/diagram.component.spec.ts +++ b/frontend/src/app/diagram/diagram.component.spec.ts @@ -64,7 +64,7 @@ describe('DiagramComponent', () => { jest.spyOn(component, 'prepareDiagramData'); component.ngAfterViewInit(); - component.alignmentData.next(alignmentLists); + component.alignmentData$.next(alignmentLists); expect(component.cleanUpDiagram).toHaveBeenCalled(); expect(component.prepareDiagramData).toHaveBeenCalledWith(alignmentLists); @@ -104,7 +104,6 @@ describe('DiagramComponent', () => { component.generateNodes(alignmentLists); expect(component.generateConnections).toHaveBeenCalled(); - expect(component.generateDiagram).toHaveBeenCalled(); expect(component.diagramData).toEqual(diagramElements.concat(edges)); }); @@ -120,7 +119,6 @@ describe('DiagramComponent', () => { component.generateNodes(alignmentListsKeyResult); expect(component.generateConnections).toHaveBeenCalled(); - expect(component.generateDiagram).toHaveBeenCalled(); expect(component.diagramData).toEqual(diagramData); }); @@ -136,7 +134,6 @@ describe('DiagramComponent', () => { component.generateNodes(alignmentListsKeyResult); expect(component.generateConnections).toHaveBeenCalled(); - expect(component.generateDiagram).toHaveBeenCalled(); expect(component.diagramData).toEqual(diagramData); }); diff --git a/frontend/src/app/diagram/diagram.component.ts b/frontend/src/app/diagram/diagram.component.ts index 4883cb248b..6d35f71bad 100644 --- a/frontend/src/app/diagram/diagram.component.ts +++ b/frontend/src/app/diagram/diagram.component.ts @@ -21,6 +21,7 @@ import { AlignmentObject } from '../shared/types/model/AlignmentObject'; import { AlignmentConnection } from '../shared/types/model/AlignmentConnection'; import { Zone } from '../shared/types/enums/Zone'; import { ObjectiveState } from '../shared/types/enums/ObjectiveState'; +import { RefreshDataService } from '../shared/services/refresh-data.service'; @Component({ selector: 'app-diagram', @@ -28,39 +29,27 @@ import { ObjectiveState } from '../shared/types/enums/ObjectiveState'; styleUrl: './diagram.component.scss', }) export class DiagramComponent implements AfterViewInit, OnDestroy { - private alignmentData$: Subject = new Subject(); + @Input() + public alignmentData$: Subject = new Subject(); cy!: cytoscape.Core; diagramData: any[] = []; alignmentDataCache: AlignmentLists | null = null; + reloadRequired: boolean | null | undefined = false; constructor( private keyResultService: KeyresultService, + private refreshDataService: RefreshDataService, private router: Router, ) {} - @Input() - get alignmentData(): Subject { - return this.alignmentData$; - } - - set alignmentData(alignmentData: AlignmentLists) { - this.alignmentData$.next(alignmentData); - } - ngAfterViewInit(): void { - this.alignmentData.subscribe((alignmentData: AlignmentLists): void => { - let lastAlignmentItem: AlignmentObject = - alignmentData.alignmentObjectDtoList[alignmentData.alignmentObjectDtoList.length - 1]; - - const diagramReloadRequired: boolean = - lastAlignmentItem?.objectTitle === 'reload' - ? lastAlignmentItem?.objectType === 'true' - : JSON.stringify(this.alignmentDataCache) !== JSON.stringify(alignmentData); + this.refreshDataService.reloadAlignmentSubject.subscribe((value: boolean | null | undefined): void => { + this.reloadRequired = value; + }); - if (diagramReloadRequired) { - if (lastAlignmentItem?.objectTitle === 'reload') { - alignmentData.alignmentObjectDtoList.pop(); - } + this.alignmentData$.subscribe((alignmentData: AlignmentLists): void => { + if (this.reloadRequired == true || JSON.stringify(this.alignmentDataCache) !== JSON.stringify(alignmentData)) { + this.reloadRequired = undefined; this.alignmentDataCache = alignmentData; this.diagramData = []; this.cleanUpDiagram(); @@ -71,7 +60,8 @@ export class DiagramComponent implements AfterViewInit, OnDestroy { ngOnDestroy(): void { this.cleanUpDiagram(); - this.alignmentData.unsubscribe(); + this.alignmentData$.unsubscribe(); + this.refreshDataService.reloadAlignmentSubject.unsubscribe(); } generateDiagram(): void { @@ -187,8 +177,8 @@ export class DiagramComponent implements AfterViewInit, OnDestroy { } }); - zip(observableArray).subscribe(() => { - this.generateConnections(alignmentData, diagramElements); + zip(observableArray).subscribe(async () => { + await this.generateConnections(alignmentData, diagramElements); }); } @@ -223,7 +213,7 @@ export class DiagramComponent implements AfterViewInit, OnDestroy { }; } - generateConnections(alignmentData: AlignmentLists, diagramElements: any[]): void { + async generateConnections(alignmentData: AlignmentLists, diagramElements: any[]) { let edges: any[] = []; alignmentData.alignmentConnectionDtoList.forEach((alignmentConnection: AlignmentConnection) => { let edge = { @@ -238,7 +228,10 @@ export class DiagramComponent implements AfterViewInit, OnDestroy { edges.push(edge); }); this.diagramData = diagramElements.concat(edges); - this.generateDiagram(); + + // Sometimes the DOM Element #cy is not ready when cytoscape tries to generate the diagram + // To avoid this, we use here a setTimeout() + setTimeout(() => this.generateDiagram(), 0); } generateObjectiveSVG(title: string, teamName: string, state: string): string { diff --git a/frontend/src/app/overview/overview.component.html b/frontend/src/app/overview/overview.component.html index f4929da899..5c46bf9c35 100644 --- a/frontend/src/app/overview/overview.component.html +++ b/frontend/src/app/overview/overview.component.html @@ -46,7 +46,7 @@
- +
diff --git a/frontend/src/app/overview/overview.component.spec.ts b/frontend/src/app/overview/overview.component.spec.ts index c047057d24..6f9e20117e 100644 --- a/frontend/src/app/overview/overview.component.spec.ts +++ b/frontend/src/app/overview/overview.component.spec.ts @@ -121,7 +121,7 @@ describe('OverviewComponent', () => { routerHarness.detectChanges(); component.loadOverviewWithParams(); expect(overviewService.getOverview).toHaveBeenCalledWith(quarterParam, teamsParam, objectiveQueryParam); - expect(component.loadOverview).toHaveBeenCalledWith(quarterParam, teamsParam, objectiveQueryParam, undefined); + expect(component.loadOverview).toHaveBeenCalledWith(quarterParam, teamsParam, objectiveQueryParam); }, ); @@ -145,7 +145,7 @@ describe('OverviewComponent', () => { jest.spyOn(overviewService, 'getOverview'); component.isOverview = true; - component.loadOverview(3, [5, 6], '', null); + component.loadOverview(3, [5, 6], ''); expect(overviewService.getOverview).toHaveBeenCalled(); }); @@ -154,7 +154,7 @@ describe('OverviewComponent', () => { component.isOverview = false; fixture.detectChanges(); - component.loadOverview(3, [5, 6], '', null); + component.loadOverview(3, [5, 6], ''); expect(alignmentService.getAlignmentByFilter).toHaveBeenCalled(); }); diff --git a/frontend/src/app/overview/overview.component.ts b/frontend/src/app/overview/overview.component.ts index cc97822235..43116692ce 100644 --- a/frontend/src/app/overview/overview.component.ts +++ b/frontend/src/app/overview/overview.component.ts @@ -7,7 +7,6 @@ import { RefreshDataService } from '../shared/services/refresh-data.service'; import { getQueryString, getValueFromQuery, isMobileDevice, trackByFn } from '../shared/common'; import { AlignmentService } from '../shared/services/alignment.service'; import { AlignmentLists } from '../shared/types/model/AlignmentLists'; -import { AlignmentObject } from '../shared/types/model/AlignmentObject'; @Component({ selector: 'app-overview', @@ -18,7 +17,6 @@ import { AlignmentObject } from '../shared/types/model/AlignmentObject'; export class OverviewComponent implements OnInit, OnDestroy { overviewEntities$: Subject = new Subject(); alignmentLists$: Subject = new Subject(); - emptyAlignmentList: AlignmentLists = { alignmentObjectDtoList: [], alignmentConnectionDtoList: [] } as AlignmentLists; protected readonly trackByFn = trackByFn; private destroyed$: ReplaySubject = new ReplaySubject(1); hasAdminAccess: ReplaySubject = new ReplaySubject(1); @@ -34,7 +32,7 @@ export class OverviewComponent implements OnInit, OnDestroy { ) { this.refreshDataService.reloadOverviewSubject .pipe(takeUntil(this.destroyed$)) - .subscribe((reload: boolean | null | undefined) => this.loadOverviewWithParams(reload)); + .subscribe(() => this.loadOverviewWithParams()); combineLatest([ refreshDataService.teamFilterReady.asObservable(), @@ -58,7 +56,7 @@ export class OverviewComponent implements OnInit, OnDestroy { } } - loadOverviewWithParams(reload?: boolean | null) { + loadOverviewWithParams() { const quarterQuery = this.activatedRoute.snapshot.queryParams['quarter']; const teamQuery = this.activatedRoute.snapshot.queryParams['teams']; const objectiveQuery = this.activatedRoute.snapshot.queryParams['objectiveQuery']; @@ -66,14 +64,14 @@ export class OverviewComponent implements OnInit, OnDestroy { const teamIds = getValueFromQuery(teamQuery); const quarterId = getValueFromQuery(quarterQuery)[0]; const objectiveQueryString = getQueryString(objectiveQuery); - this.loadOverview(quarterId, teamIds, objectiveQueryString, reload); + this.loadOverview(quarterId, teamIds, objectiveQueryString); } - loadOverview(quarterId?: number, teamIds?: number[], objectiveQuery?: string, reload?: boolean | null): void { + loadOverview(quarterId?: number, teamIds?: number[], objectiveQuery?: string): void { if (this.isOverview) { this.loadOverviewData(quarterId, teamIds, objectiveQuery); } else { - this.loadAlignmentData(quarterId, teamIds, objectiveQuery, reload); + this.loadAlignmentData(quarterId, teamIds, objectiveQuery); } } @@ -92,7 +90,7 @@ export class OverviewComponent implements OnInit, OnDestroy { }); } - loadAlignmentData(quarterId?: number, teamIds?: number[], objectiveQuery?: string, reload?: boolean | null): void { + loadAlignmentData(quarterId?: number, teamIds?: number[], objectiveQuery?: string): void { this.alignmentService .getAlignmentByFilter(quarterId, teamIds, objectiveQuery) .pipe( @@ -102,16 +100,6 @@ export class OverviewComponent implements OnInit, OnDestroy { }), ) .subscribe((alignmentLists: AlignmentLists) => { - if (reload != null) { - let alignmentObjectReload: AlignmentObject = { - objectId: 0, - objectTitle: 'reload', - objectType: reload.toString(), - objectTeamName: '', - objectState: null, - }; - alignmentLists.alignmentObjectDtoList.push(alignmentObjectReload); - } this.alignmentLists$.next(alignmentLists); }); } diff --git a/frontend/src/app/shared/services/refresh-data.service.ts b/frontend/src/app/shared/services/refresh-data.service.ts index e04f27a711..637eb78ba8 100644 --- a/frontend/src/app/shared/services/refresh-data.service.ts +++ b/frontend/src/app/shared/services/refresh-data.service.ts @@ -6,7 +6,8 @@ import { DEFAULT_HEADER_HEIGHT_PX } from '../constantLibary'; providedIn: 'root', }) export class RefreshDataService { - public reloadOverviewSubject: Subject = new Subject(); + public reloadOverviewSubject: Subject = new Subject(); + public reloadAlignmentSubject: Subject = new Subject(); public quarterFilterReady: Subject = new Subject(); public teamFilterReady: Subject = new Subject(); @@ -14,6 +15,7 @@ export class RefreshDataService { public okrBannerHeightSubject: BehaviorSubject = new BehaviorSubject(DEFAULT_HEADER_HEIGHT_PX); markDataRefresh(reload?: boolean | null) { - this.reloadOverviewSubject.next(reload); + this.reloadOverviewSubject.next(); + this.reloadAlignmentSubject.next(reload); } }