Skip to content

Commit 6b9a56b

Browse files
lkleisalkleisa
committed
Use second refresh subject
1 parent 89de0e4 commit 6b9a56b

9 files changed

+37
-74
lines changed

‎frontend/cypress/e2e/checkIn.cy.ts

-16
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ describe('OKR Check-in e2e tests', () => {
3232
checkForDialogTextMetric();
3333
cy.fillOutCheckInMetric(30, 6, 'We bought a new house', 'We have to buy more PCs');
3434

35-
cy.wait(1000);
36-
3735
cy.contains('30%');
3836
cy.contains('6/10');
3937
cy.contains('Letztes Check-in (' + getCurrentDate() + ')');
@@ -64,8 +62,6 @@ describe('OKR Check-in e2e tests', () => {
6462
checkForDialogTextMetric();
6563
cy.fillOutCheckInMetric(30, 0, 'We bought a new house', 'We have to buy more PCs');
6664

67-
cy.wait(100);
68-
6965
cy.contains('30%');
7066
cy.contains('6/10');
7167
cy.contains('Letztes Check-in (' + getCurrentDate() + ')');
@@ -96,8 +92,6 @@ describe('OKR Check-in e2e tests', () => {
9692
checkForDialogTextMetric();
9793
cy.fillOutCheckInMetric(5, 5, null, null);
9894

99-
cy.wait(500);
100-
10195
cy.contains('5%');
10296
cy.contains('!');
10397
cy.contains('5/10');
@@ -130,8 +124,6 @@ describe('OKR Check-in e2e tests', () => {
130124
checkForDialogTextOrdinal();
131125
cy.fillOutCheckInOrdinal(1, 6, 'There is a new car', 'Buy now a new pool');
132126

133-
cy.wait(500);
134-
135127
cy.contains('6/10');
136128
cy.contains('There is a new car');
137129
cy.contains('Letztes Check-in (' + getCurrentDate() + ')');
@@ -162,8 +154,6 @@ describe('OKR Check-in e2e tests', () => {
162154
cy.getByTestId('add-check-in').first().click();
163155
cy.fillOutCheckInMetric(50, 6, 'This was a good idea', 'Will be difficult');
164156

165-
cy.wait(500);
166-
167157
cy.getByTestId('show-all-checkins').click();
168158

169159
cy.wait(500);
@@ -201,9 +191,6 @@ describe('OKR Check-in e2e tests', () => {
201191

202192
cy.getByTestId('add-check-in').first().click();
203193
cy.fillOutCheckInMetric(30, 5, 'Here we are', 'A cat would be great');
204-
205-
cy.wait(500);
206-
207194
cy.contains('Aktuell: CHF 30.-');
208195
cy.getByTestId('show-all-checkins').click();
209196

@@ -249,9 +236,6 @@ describe('OKR Check-in e2e tests', () => {
249236
cy.getByTestId('keyresult').contains('For editing ordinal checkin').click();
250237
cy.getByTestId('add-check-in').first().click();
251238
cy.fillOutCheckInOrdinal(0, 6, 'There is a new car', 'Buy now a new pool');
252-
253-
cy.wait(500);
254-
255239
cy.getByTestId('show-all-checkins').click();
256240

257241
cy.wait(500);

‎frontend/cypress/e2e/scoring.cy.ts

-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ describe('Scoring component e2e tests', () => {
7878

7979
function setupMetricKR(baseline: number, stretchgoal: number, value: number) {
8080
cy.createMetricKeyresult('Metric scoring keyresult', String(baseline), String(stretchgoal));
81-
cy.wait(500);
8281
cy.getByTestId('keyresult').get(':contains("Metric scoring keyresult")').last().click();
8382
cy.getByTestId('add-check-in').click();
8483
cy.getByTestId('check-in-metric-value').clear().type(String(value));

‎frontend/src/app/diagram/diagram.component.html

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
<div *ngIf="(alignmentDataCache?.alignmentObjectDtoList)!.length != 0" id="cy"></div>
1+
<div *ngIf="((alignmentData$ | async)?.alignmentObjectDtoList)!.length != 0" id="cy"></div>
22

33
<div
4-
*ngIf="(alignmentDataCache?.alignmentObjectDtoList)!.length == 0"
4+
*ngIf="((alignmentData$ | async)?.alignmentObjectDtoList)!.length == 0"
55
class="d-flex align-items-center flex-column pt-5 gap-5"
66
>
77
<p>Kein Alignment vorhanden</p>

‎frontend/src/app/diagram/diagram.component.spec.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ describe('DiagramComponent', () => {
6464
jest.spyOn(component, 'prepareDiagramData');
6565

6666
component.ngAfterViewInit();
67-
component.alignmentData.next(alignmentLists);
67+
component.alignmentData$.next(alignmentLists);
6868

6969
expect(component.cleanUpDiagram).toHaveBeenCalled();
7070
expect(component.prepareDiagramData).toHaveBeenCalledWith(alignmentLists);
@@ -104,7 +104,6 @@ describe('DiagramComponent', () => {
104104
component.generateNodes(alignmentLists);
105105

106106
expect(component.generateConnections).toHaveBeenCalled();
107-
expect(component.generateDiagram).toHaveBeenCalled();
108107
expect(component.diagramData).toEqual(diagramElements.concat(edges));
109108
});
110109

@@ -120,7 +119,6 @@ describe('DiagramComponent', () => {
120119
component.generateNodes(alignmentListsKeyResult);
121120

122121
expect(component.generateConnections).toHaveBeenCalled();
123-
expect(component.generateDiagram).toHaveBeenCalled();
124122
expect(component.diagramData).toEqual(diagramData);
125123
});
126124

@@ -136,7 +134,6 @@ describe('DiagramComponent', () => {
136134
component.generateNodes(alignmentListsKeyResult);
137135

138136
expect(component.generateConnections).toHaveBeenCalled();
139-
expect(component.generateDiagram).toHaveBeenCalled();
140137
expect(component.diagramData).toEqual(diagramData);
141138
});
142139

‎frontend/src/app/diagram/diagram.component.ts

+20-27
Original file line numberDiff line numberDiff line change
@@ -21,46 +21,35 @@ import { AlignmentObject } from '../shared/types/model/AlignmentObject';
2121
import { AlignmentConnection } from '../shared/types/model/AlignmentConnection';
2222
import { Zone } from '../shared/types/enums/Zone';
2323
import { ObjectiveState } from '../shared/types/enums/ObjectiveState';
24+
import { RefreshDataService } from '../shared/services/refresh-data.service';
2425

2526
@Component({
2627
selector: 'app-diagram',
2728
templateUrl: './diagram.component.html',
2829
styleUrl: './diagram.component.scss',
2930
})
3031
export class DiagramComponent implements AfterViewInit, OnDestroy {
31-
private alignmentData$: Subject<AlignmentLists> = new Subject<AlignmentLists>();
32+
@Input()
33+
public alignmentData$: Subject<AlignmentLists> = new Subject<AlignmentLists>();
3234
cy!: cytoscape.Core;
3335
diagramData: any[] = [];
3436
alignmentDataCache: AlignmentLists | null = null;
37+
reloadRequired: boolean | null | undefined = false;
3538

3639
constructor(
3740
private keyResultService: KeyresultService,
41+
private refreshDataService: RefreshDataService,
3842
private router: Router,
3943
) {}
4044

41-
@Input()
42-
get alignmentData(): Subject<AlignmentLists> {
43-
return this.alignmentData$;
44-
}
45-
46-
set alignmentData(alignmentData: AlignmentLists) {
47-
this.alignmentData$.next(alignmentData);
48-
}
49-
5045
ngAfterViewInit(): void {
51-
this.alignmentData.subscribe((alignmentData: AlignmentLists): void => {
52-
let lastAlignmentItem: AlignmentObject =
53-
alignmentData.alignmentObjectDtoList[alignmentData.alignmentObjectDtoList.length - 1];
54-
55-
const diagramReloadRequired: boolean =
56-
lastAlignmentItem?.objectTitle === 'reload'
57-
? lastAlignmentItem?.objectType === 'true'
58-
: JSON.stringify(this.alignmentDataCache) !== JSON.stringify(alignmentData);
46+
this.refreshDataService.reloadAlignmentSubject.subscribe((value: boolean | null | undefined): void => {
47+
this.reloadRequired = value;
48+
});
5949

60-
if (diagramReloadRequired) {
61-
if (lastAlignmentItem?.objectTitle === 'reload') {
62-
alignmentData.alignmentObjectDtoList.pop();
63-
}
50+
this.alignmentData$.subscribe((alignmentData: AlignmentLists): void => {
51+
if (this.reloadRequired == true || JSON.stringify(this.alignmentDataCache) !== JSON.stringify(alignmentData)) {
52+
this.reloadRequired = undefined;
6453
this.alignmentDataCache = alignmentData;
6554
this.diagramData = [];
6655
this.cleanUpDiagram();
@@ -71,7 +60,8 @@ export class DiagramComponent implements AfterViewInit, OnDestroy {
7160

7261
ngOnDestroy(): void {
7362
this.cleanUpDiagram();
74-
this.alignmentData.unsubscribe();
63+
this.alignmentData$.unsubscribe();
64+
this.refreshDataService.reloadAlignmentSubject.unsubscribe();
7565
}
7666

7767
generateDiagram(): void {
@@ -187,8 +177,8 @@ export class DiagramComponent implements AfterViewInit, OnDestroy {
187177
}
188178
});
189179

190-
zip(observableArray).subscribe(() => {
191-
this.generateConnections(alignmentData, diagramElements);
180+
zip(observableArray).subscribe(async () => {
181+
await this.generateConnections(alignmentData, diagramElements);
192182
});
193183
}
194184

@@ -223,7 +213,7 @@ export class DiagramComponent implements AfterViewInit, OnDestroy {
223213
};
224214
}
225215

226-
generateConnections(alignmentData: AlignmentLists, diagramElements: any[]): void {
216+
async generateConnections(alignmentData: AlignmentLists, diagramElements: any[]) {
227217
let edges: any[] = [];
228218
alignmentData.alignmentConnectionDtoList.forEach((alignmentConnection: AlignmentConnection) => {
229219
let edge = {
@@ -238,7 +228,10 @@ export class DiagramComponent implements AfterViewInit, OnDestroy {
238228
edges.push(edge);
239229
});
240230
this.diagramData = diagramElements.concat(edges);
241-
this.generateDiagram();
231+
232+
// Sometimes the DOM Element #cy is not ready when cytoscape tries to generate the diagram
233+
// To avoid this, we use here a setTimeout()
234+
setTimeout(() => this.generateDiagram(), 0);
242235
}
243236

244237
generateObjectiveSVG(title: string, teamName: string, state: string): string {

‎frontend/src/app/overview/overview.component.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
</div>
4747

4848
<div [class.hidden]="isOverview">
49-
<app-diagram [alignmentData]="(alignmentLists$ | async) || emptyAlignmentList"></app-diagram>
49+
<app-diagram [alignmentData$]="alignmentLists$"></app-diagram>
5050
</div>
5151
</div>
5252
</div>

‎frontend/src/app/overview/overview.component.spec.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ describe('OverviewComponent', () => {
121121
routerHarness.detectChanges();
122122
component.loadOverviewWithParams();
123123
expect(overviewService.getOverview).toHaveBeenCalledWith(quarterParam, teamsParam, objectiveQueryParam);
124-
expect(component.loadOverview).toHaveBeenCalledWith(quarterParam, teamsParam, objectiveQueryParam, undefined);
124+
expect(component.loadOverview).toHaveBeenCalledWith(quarterParam, teamsParam, objectiveQueryParam);
125125
},
126126
);
127127

@@ -145,7 +145,7 @@ describe('OverviewComponent', () => {
145145
jest.spyOn(overviewService, 'getOverview');
146146
component.isOverview = true;
147147

148-
component.loadOverview(3, [5, 6], '', null);
148+
component.loadOverview(3, [5, 6], '');
149149
expect(overviewService.getOverview).toHaveBeenCalled();
150150
});
151151

@@ -154,7 +154,7 @@ describe('OverviewComponent', () => {
154154
component.isOverview = false;
155155
fixture.detectChanges();
156156

157-
component.loadOverview(3, [5, 6], '', null);
157+
component.loadOverview(3, [5, 6], '');
158158
expect(alignmentService.getAlignmentByFilter).toHaveBeenCalled();
159159
});
160160

‎frontend/src/app/overview/overview.component.ts

+6-18
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { RefreshDataService } from '../shared/services/refresh-data.service';
77
import { getQueryString, getValueFromQuery, isMobileDevice, trackByFn } from '../shared/common';
88
import { AlignmentService } from '../shared/services/alignment.service';
99
import { AlignmentLists } from '../shared/types/model/AlignmentLists';
10-
import { AlignmentObject } from '../shared/types/model/AlignmentObject';
1110

1211
@Component({
1312
selector: 'app-overview',
@@ -18,7 +17,6 @@ import { AlignmentObject } from '../shared/types/model/AlignmentObject';
1817
export class OverviewComponent implements OnInit, OnDestroy {
1918
overviewEntities$: Subject<OverviewEntity[]> = new Subject<OverviewEntity[]>();
2019
alignmentLists$: Subject<AlignmentLists> = new Subject<AlignmentLists>();
21-
emptyAlignmentList: AlignmentLists = { alignmentObjectDtoList: [], alignmentConnectionDtoList: [] } as AlignmentLists;
2220
protected readonly trackByFn = trackByFn;
2321
private destroyed$: ReplaySubject<boolean> = new ReplaySubject(1);
2422
hasAdminAccess: ReplaySubject<boolean> = new ReplaySubject<boolean>(1);
@@ -34,7 +32,7 @@ export class OverviewComponent implements OnInit, OnDestroy {
3432
) {
3533
this.refreshDataService.reloadOverviewSubject
3634
.pipe(takeUntil(this.destroyed$))
37-
.subscribe((reload: boolean | null | undefined) => this.loadOverviewWithParams(reload));
35+
.subscribe(() => this.loadOverviewWithParams());
3836

3937
combineLatest([
4038
refreshDataService.teamFilterReady.asObservable(),
@@ -58,22 +56,22 @@ export class OverviewComponent implements OnInit, OnDestroy {
5856
}
5957
}
6058

61-
loadOverviewWithParams(reload?: boolean | null) {
59+
loadOverviewWithParams() {
6260
const quarterQuery = this.activatedRoute.snapshot.queryParams['quarter'];
6361
const teamQuery = this.activatedRoute.snapshot.queryParams['teams'];
6462
const objectiveQuery = this.activatedRoute.snapshot.queryParams['objectiveQuery'];
6563

6664
const teamIds = getValueFromQuery(teamQuery);
6765
const quarterId = getValueFromQuery(quarterQuery)[0];
6866
const objectiveQueryString = getQueryString(objectiveQuery);
69-
this.loadOverview(quarterId, teamIds, objectiveQueryString, reload);
67+
this.loadOverview(quarterId, teamIds, objectiveQueryString);
7068
}
7169

72-
loadOverview(quarterId?: number, teamIds?: number[], objectiveQuery?: string, reload?: boolean | null): void {
70+
loadOverview(quarterId?: number, teamIds?: number[], objectiveQuery?: string): void {
7371
if (this.isOverview) {
7472
this.loadOverviewData(quarterId, teamIds, objectiveQuery);
7573
} else {
76-
this.loadAlignmentData(quarterId, teamIds, objectiveQuery, reload);
74+
this.loadAlignmentData(quarterId, teamIds, objectiveQuery);
7775
}
7876
}
7977

@@ -92,7 +90,7 @@ export class OverviewComponent implements OnInit, OnDestroy {
9290
});
9391
}
9492

95-
loadAlignmentData(quarterId?: number, teamIds?: number[], objectiveQuery?: string, reload?: boolean | null): void {
93+
loadAlignmentData(quarterId?: number, teamIds?: number[], objectiveQuery?: string): void {
9694
this.alignmentService
9795
.getAlignmentByFilter(quarterId, teamIds, objectiveQuery)
9896
.pipe(
@@ -102,16 +100,6 @@ export class OverviewComponent implements OnInit, OnDestroy {
102100
}),
103101
)
104102
.subscribe((alignmentLists: AlignmentLists) => {
105-
if (reload != null) {
106-
let alignmentObjectReload: AlignmentObject = {
107-
objectId: 0,
108-
objectTitle: 'reload',
109-
objectType: reload.toString(),
110-
objectTeamName: '',
111-
objectState: null,
112-
};
113-
alignmentLists.alignmentObjectDtoList.push(alignmentObjectReload);
114-
}
115103
this.alignmentLists$.next(alignmentLists);
116104
});
117105
}

‎frontend/src/app/shared/services/refresh-data.service.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,16 @@ import { DEFAULT_HEADER_HEIGHT_PX } from '../constantLibary';
66
providedIn: 'root',
77
})
88
export class RefreshDataService {
9-
public reloadOverviewSubject: Subject<boolean | null | undefined> = new Subject();
9+
public reloadOverviewSubject: Subject<void> = new Subject();
10+
public reloadAlignmentSubject: Subject<boolean | null | undefined> = new Subject();
1011

1112
public quarterFilterReady: Subject<void> = new Subject<void>();
1213
public teamFilterReady: Subject<void> = new Subject<void>();
1314

1415
public okrBannerHeightSubject: BehaviorSubject<number> = new BehaviorSubject<number>(DEFAULT_HEADER_HEIGHT_PX);
1516

1617
markDataRefresh(reload?: boolean | null) {
17-
this.reloadOverviewSubject.next(reload);
18+
this.reloadOverviewSubject.next();
19+
this.reloadAlignmentSubject.next(reload);
1820
}
1921
}

0 commit comments

Comments
 (0)