Skip to content

Commit 246e288

Browse files
authored
Merge pull request ceph#58069 from rhcs-dashboard/multiple-alert-panel-dashboard
mgr/dashboard: fix alert broken for multiple alerts Reviewed-by: afreen23 <NOT@FOUND> Reviewed-by: ivoalmeida <NOT@FOUND>
2 parents 425f744 + b9bc49f commit 246e288

File tree

13 files changed

+193
-152
lines changed

13 files changed

+193
-152
lines changed

src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.html

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,16 @@
22
<cd-navigation>
33
<div class="container-fluid h-100"
44
[ngClass]="{'dashboard': (router.url == '/dashboard' || router.url == '/dashboard_3' || router.url == '/multi-cluster/overview'), 'rgw-dashboard': (router.url == '/rgw/overview')}">
5-
<cd-context></cd-context>
5+
<!-- ************************ -->
6+
<!-- ALERTS BANNER -->
7+
<!-- ************************ -->
8+
<div class="cd-alert-container"
9+
[ngClass]="{'ms-4 me-4': (router.url == '/dashboard' || router.url == '/dashboard_3' || router.url == '/multi-cluster/overview'), 'm-3': (router.url == '/rgw/overview')}">
10+
<cd-pwd-expiration-notification></cd-pwd-expiration-notification>
11+
<cd-telemetry-notification></cd-telemetry-notification>
12+
<cd-motd></cd-motd>
13+
</div>
14+
<cd-context></cd-context>
615
<cd-breadcrumbs></cd-breadcrumbs>
716
<router-outlet></router-outlet>
817
</div>

src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.scss

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,8 @@
1616
.rgw-dashboard {
1717
background-color: vv.$body-bg-alt;
1818
}
19+
20+
.cd-alert-container {
21+
display: flex;
22+
flex-direction: column;
23+
}

src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.spec.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,56 @@ describe('WorkbenchLayoutComponent', () => {
3232
it('should create', () => {
3333
expect(component).toBeTruthy();
3434
});
35+
36+
describe('showTopNotification', () => {
37+
const notification1 = 'notificationName1';
38+
const notification2 = 'notificationName2';
39+
40+
beforeEach(() => {
41+
component.notifications = [];
42+
});
43+
44+
it('should show notification', () => {
45+
component.showTopNotification(notification1, true);
46+
expect(component.notifications.includes(notification1)).toBeTruthy();
47+
expect(component.notifications.length).toBe(1);
48+
});
49+
50+
it('should not add a second notification if it is already shown', () => {
51+
component.showTopNotification(notification1, true);
52+
component.showTopNotification(notification1, true);
53+
expect(component.notifications.includes(notification1)).toBeTruthy();
54+
expect(component.notifications.length).toBe(1);
55+
});
56+
57+
it('should add a second notification if the first one is different', () => {
58+
component.showTopNotification(notification1, true);
59+
component.showTopNotification(notification2, true);
60+
expect(component.notifications.includes(notification1)).toBeTruthy();
61+
expect(component.notifications.includes(notification2)).toBeTruthy();
62+
expect(component.notifications.length).toBe(2);
63+
});
64+
65+
it('should hide an active notification', () => {
66+
component.showTopNotification(notification1, true);
67+
expect(component.notifications.includes(notification1)).toBeTruthy();
68+
expect(component.notifications.length).toBe(1);
69+
component.showTopNotification(notification1, false);
70+
expect(component.notifications.length).toBe(0);
71+
});
72+
73+
it('should not fail if it tries to hide an inactive notification', () => {
74+
expect(() => component.showTopNotification(notification1, false)).not.toThrow();
75+
expect(component.notifications.length).toBe(0);
76+
});
77+
78+
it('should keep other notifications if it hides one', () => {
79+
component.showTopNotification(notification1, true);
80+
component.showTopNotification(notification2, true);
81+
expect(component.notifications.length).toBe(2);
82+
component.showTopNotification(notification2, false);
83+
expect(component.notifications.length).toBe(1);
84+
expect(component.notifications.includes(notification1)).toBeTruthy();
85+
});
86+
});
3587
});

src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Component, OnDestroy, OnInit } from '@angular/core';
1+
import { Component, HostBinding, OnDestroy, OnInit } from '@angular/core';
22
import { Router } from '@angular/router';
33

44
import { Subscription } from 'rxjs';
@@ -9,6 +9,9 @@ import { AuthStorageService } from '~/app/shared/services/auth-storage.service';
99
import { FaviconService } from '~/app/shared/services/favicon.service';
1010
import { SummaryService } from '~/app/shared/services/summary.service';
1111
import { TaskManagerService } from '~/app/shared/services/task-manager.service';
12+
import { TelemetryNotificationService } from '../../../shared/services/telemetry-notification.service';
13+
import { MotdNotificationService } from '~/app/shared/services/motd-notification.service';
14+
import _ from 'lodash';
1215

1316
@Component({
1417
selector: 'cd-workbench-layout',
@@ -17,16 +20,22 @@ import { TaskManagerService } from '~/app/shared/services/task-manager.service';
1720
providers: [FaviconService]
1821
})
1922
export class WorkbenchLayoutComponent implements OnInit, OnDestroy {
23+
notifications: string[] = [];
2024
private subs = new Subscription();
2125
permissions: Permissions;
26+
@HostBinding('class') get class(): string {
27+
return 'top-notification-' + this.notifications.length;
28+
}
2229

2330
constructor(
2431
public router: Router,
2532
private summaryService: SummaryService,
2633
private taskManagerService: TaskManagerService,
2734
private multiClusterService: MultiClusterService,
2835
private faviconService: FaviconService,
29-
private authStorageService: AuthStorageService
36+
private authStorageService: AuthStorageService,
37+
private telemetryNotificationService: TelemetryNotificationService,
38+
private motdNotificationService: MotdNotificationService
3039
) {
3140
this.permissions = this.authStorageService.getPermissions();
3241
}
@@ -38,8 +47,36 @@ export class WorkbenchLayoutComponent implements OnInit, OnDestroy {
3847
}
3948
this.subs.add(this.summaryService.startPolling());
4049
this.subs.add(this.taskManagerService.init(this.summaryService));
50+
51+
this.subs.add(
52+
this.authStorageService.isPwdDisplayed$.subscribe((isDisplayed) => {
53+
this.showTopNotification('isPwdDisplayed', isDisplayed);
54+
})
55+
);
56+
this.subs.add(
57+
this.telemetryNotificationService.update.subscribe((visible: boolean) => {
58+
this.showTopNotification('telemetryNotificationEnabled', visible);
59+
})
60+
);
61+
this.subs.add(
62+
this.motdNotificationService.motd$.subscribe((motd: any) => {
63+
this.showTopNotification('motdNotificationEnabled', _.isPlainObject(motd));
64+
})
65+
);
4166
this.faviconService.init();
4267
}
68+
showTopNotification(name: string, isDisplayed: boolean) {
69+
if (isDisplayed) {
70+
if (!this.notifications.includes(name)) {
71+
this.notifications.push(name);
72+
}
73+
} else {
74+
const index = this.notifications.indexOf(name);
75+
if (index >= 0) {
76+
this.notifications.splice(index, 1);
77+
}
78+
}
79+
}
4380

4481
ngOnDestroy() {
4582
this.subs.unsubscribe();

src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.html

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
<div class="cd-navbar-main">
22
<!-- ************************ -->
3-
<!-- NOTIFICATIONS/ALERTS -->
3+
<!-- NOTIFICATIONS -->
44
<!-- ************************ -->
5-
<cd-pwd-expiration-notification></cd-pwd-expiration-notification>
6-
<cd-telemetry-notification></cd-telemetry-notification>
7-
<cd-motd></cd-motd>
85
<cd-notifications-sidebar></cd-notifications-sidebar>
96
<!-- ************************ -->
107
<!-- HEADER -->

src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.spec.ts

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -214,56 +214,4 @@ describe('NavigationComponent', () => {
214214
});
215215
}
216216
});
217-
218-
describe('showTopNotification', () => {
219-
const notification1 = 'notificationName1';
220-
const notification2 = 'notificationName2';
221-
222-
beforeEach(() => {
223-
component.notifications = [];
224-
});
225-
226-
it('should show notification', () => {
227-
component.showTopNotification(notification1, true);
228-
expect(component.notifications.includes(notification1)).toBeTruthy();
229-
expect(component.notifications.length).toBe(1);
230-
});
231-
232-
it('should not add a second notification if it is already shown', () => {
233-
component.showTopNotification(notification1, true);
234-
component.showTopNotification(notification1, true);
235-
expect(component.notifications.includes(notification1)).toBeTruthy();
236-
expect(component.notifications.length).toBe(1);
237-
});
238-
239-
it('should add a second notification if the first one is different', () => {
240-
component.showTopNotification(notification1, true);
241-
component.showTopNotification(notification2, true);
242-
expect(component.notifications.includes(notification1)).toBeTruthy();
243-
expect(component.notifications.includes(notification2)).toBeTruthy();
244-
expect(component.notifications.length).toBe(2);
245-
});
246-
247-
it('should hide an active notification', () => {
248-
component.showTopNotification(notification1, true);
249-
expect(component.notifications.includes(notification1)).toBeTruthy();
250-
expect(component.notifications.length).toBe(1);
251-
component.showTopNotification(notification1, false);
252-
expect(component.notifications.length).toBe(0);
253-
});
254-
255-
it('should not fail if it tries to hide an inactive notification', () => {
256-
expect(() => component.showTopNotification(notification1, false)).not.toThrow();
257-
expect(component.notifications.length).toBe(0);
258-
});
259-
260-
it('should keep other notifications if it hides one', () => {
261-
component.showTopNotification(notification1, true);
262-
component.showTopNotification(notification2, true);
263-
expect(component.notifications.length).toBe(2);
264-
component.showTopNotification(notification2, false);
265-
expect(component.notifications.length).toBe(1);
266-
expect(component.notifications.includes(notification1)).toBeTruthy();
267-
});
268-
});
269217
});

src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Component, HostBinding, OnDestroy, OnInit } from '@angular/core';
1+
import { Component, OnDestroy, OnInit } from '@angular/core';
22
import { Router } from '@angular/router';
33

44
import * as _ from 'lodash';
@@ -14,22 +14,16 @@ import {
1414
FeatureTogglesMap$,
1515
FeatureTogglesService
1616
} from '~/app/shared/services/feature-toggles.service';
17-
import { MotdNotificationService } from '~/app/shared/services/motd-notification.service';
1817
import { PrometheusAlertService } from '~/app/shared/services/prometheus-alert.service';
1918
import { SummaryService } from '~/app/shared/services/summary.service';
20-
import { TelemetryNotificationService } from '~/app/shared/services/telemetry-notification.service';
2119

2220
@Component({
2321
selector: 'cd-navigation',
2422
templateUrl: './navigation.component.html',
2523
styleUrls: ['./navigation.component.scss']
2624
})
2725
export class NavigationComponent implements OnInit, OnDestroy {
28-
notifications: string[] = [];
2926
clusterDetails: any[] = [];
30-
@HostBinding('class') get class(): string {
31-
return 'top-notification-' + this.notifications.length;
32-
}
3327

3428
permissions: Permissions;
3529
enabledFeature$: FeatureTogglesMap$;
@@ -54,9 +48,7 @@ export class NavigationComponent implements OnInit, OnDestroy {
5448
private router: Router,
5549
private summaryService: SummaryService,
5650
private featureToggles: FeatureTogglesService,
57-
private telemetryNotificationService: TelemetryNotificationService,
5851
public prometheusAlertService: PrometheusAlertService,
59-
private motdNotificationService: MotdNotificationService,
6052
private cookieService: CookiesService,
6153
private settingsService: SettingsService
6254
) {
@@ -91,26 +83,6 @@ export class NavigationComponent implements OnInit, OnDestroy {
9183
this.summaryData = summary;
9284
})
9385
);
94-
/*
95-
Note: If you're going to add more top notifications please do not forget to increase
96-
the number of generated css-classes in section topNotification settings in the scss
97-
file.
98-
*/
99-
this.subs.add(
100-
this.authStorageService.isPwdDisplayed$.subscribe((isDisplayed) => {
101-
this.showTopNotification('isPwdDisplayed', isDisplayed);
102-
})
103-
);
104-
this.subs.add(
105-
this.telemetryNotificationService.update.subscribe((visible: boolean) => {
106-
this.showTopNotification('telemetryNotificationEnabled', visible);
107-
})
108-
);
109-
this.subs.add(
110-
this.motdNotificationService.motd$.subscribe((motd: any) => {
111-
this.showTopNotification('motdNotificationEnabled', _.isPlainObject(motd));
112-
})
113-
);
11486
this.subs.add(
11587
this.multiClusterService.subscribeClusterTokenStatus((resp: object) => {
11688
this.clusterTokenStatus = resp;
@@ -165,19 +137,6 @@ export class NavigationComponent implements OnInit, OnDestroy {
165137
this.rightSidebarOpen = !this.rightSidebarOpen;
166138
}
167139

168-
showTopNotification(name: string, isDisplayed: boolean) {
169-
if (isDisplayed) {
170-
if (!this.notifications.includes(name)) {
171-
this.notifications.push(name);
172-
}
173-
} else {
174-
const index = this.notifications.indexOf(name);
175-
if (index >= 0) {
176-
this.notifications.splice(index, 1);
177-
}
178-
}
179-
}
180-
181140
onClusterSelection(value: object) {
182141
this.multiClusterService.setCluster(value).subscribe(
183142
(resp: any) => {

0 commit comments

Comments
 (0)