Skip to content

Commit 2b66b75

Browse files
authored
Fix/msal update effects (#36)
* Fix double toast behavior on user init * Fix race condition when accessing a catalog by url
1 parent 391a245 commit 2b66b75

4 files changed

Lines changed: 35 additions & 6 deletions

File tree

src/app/app.component.spec.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,15 @@ describe('AppComponent', () => {
279279
natsLiveMessage$.next({data: {}} as NatsMessage);
280280
await fixture.whenStable();
281281
expect(mockToastService.showToast).not.toHaveBeenCalled();
282+
283+
// Suppression window should short-circuit before validation and toast rendering.
284+
component['suppressLiveToastUntil'] = Date.now() + 5000;
285+
mockNatsService.isValidMessage.and.returnValue(true);
286+
natsLiveMessage$.next({data: {}} as NatsMessage);
287+
await fixture.whenStable();
288+
expect(mockToastService.showToast).not.toHaveBeenCalled();
289+
290+
component['suppressLiveToastUntil'] = 0;
282291
mockNatsService.isValidMessage.and.returnValue(true);
283292
natsLiveMessage$.next({data: {}} as NatsMessage);
284293
await fixture.whenStable();

src/app/app.component.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ export class AppComponent implements OnInit, OnDestroy {
3333
private readonly natsUrl: string | undefined;
3434
private unreadMessagesCountSubscription: Subscription | undefined;
3535
private liveMessageSubscription: Subscription | undefined;
36+
private hasShownInitialNotificationToast = false;
37+
private suppressLiveToastUntil = 0;
3638
headerVariant: string = AppShellConfig.headerVariant;
3739
applicationLogo: string = AppShellConfig.applicationLogo;
3840
applicationName: string = AppShellConfig.applicationName;
@@ -334,16 +336,20 @@ export class AppComponent implements OnInit, OnDestroy {
334336
// validBucketRe = regexp.MustCompile(^[a-zA-Z0-9_-]+$)
335337
// validKeyRe = regexp.MustCompile(^[-/_=.a-zA-Z0-9]+$)
336338
const natsUser = user.username.split('@')[0].replaceAll(/[^a-zA-Z0-9_-]/g, '_')
339+
this.hasShownInitialNotificationToast = false;
340+
this.suppressLiveToastUntil = 0;
337341
this.natsService.initializeUser(natsUser, user.projects).then(() => {
338342
setTimeout(() => {
339-
if(this.appShellNotificationsCount > 0) {
343+
if(this.appShellNotificationsCount > 0 && !this.hasShownInitialNotificationToast) {
340344
const notification = {
341345
id: Date.now().toString() + '-logged',
342346
title: `You have ${this.appShellNotificationsCount} unread notifications`,
343347
read: false,
344348
subject: 'only-toast'
345349
} as AppShellNotification;
346350
this.toastService.showToast(notification, 8000);
351+
this.hasShownInitialNotificationToast = true;
352+
this.suppressLiveToastUntil = Date.now() + 5000;
347353
}
348354
}, 1000);
349355
});
@@ -358,6 +364,9 @@ export class AppComponent implements OnInit, OnDestroy {
358364
if (!message?.data) {
359365
return;
360366
}
367+
if (Date.now() < this.suppressLiveToastUntil) {
368+
return;
369+
}
361370
try {
362371
if (this.natsService.isValidMessage(message.data)) {
363372
console.log('Received valid message:', message);
@@ -371,6 +380,7 @@ export class AppComponent implements OnInit, OnDestroy {
371380
};
372381
// If you want to show the actual notification, you can show message.data instead of notification
373382
this.toastService.showToast(notification, 8000);
383+
this.hasShownInitialNotificationToast = true;
374384
} else {
375385
console.log('Invalid message format:', message);
376386
}

src/app/services/catalog-resolver.service.spec.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ describe('CatalogResolver', () => {
1010
let catalogServiceSpy: jasmine.SpyObj<CatalogService>;
1111

1212
beforeEach(() => {
13-
catalogServiceSpy = jasmine.createSpyObj('CatalogService', ['getCatalogDescriptors', 'retrieveCatalogDescriptors']);
13+
catalogServiceSpy = jasmine.createSpyObj('CatalogService', ['getCatalogDescriptors', 'retrieveCatalogDescriptors', 'setCatalogDescriptors']);
1414

1515
TestBed.configureTestingModule({
1616
providers: [
@@ -27,16 +27,21 @@ describe('CatalogResolver', () => {
2727
});
2828

2929
it('should resolve catalog descriptors calling the service if not present in memory', () => {
30+
const descriptors = [{} as CatalogDescriptor];
3031
catalogServiceSpy.getCatalogDescriptors.and.returnValue([]);
31-
catalogServiceSpy.retrieveCatalogDescriptors.and.returnValue(of([{} as CatalogDescriptor]));
32-
service.resolve();
32+
catalogServiceSpy.retrieveCatalogDescriptors.and.returnValue(of(descriptors));
33+
34+
service.resolve().subscribe();
35+
3336
expect(catalogServiceSpy.retrieveCatalogDescriptors).toHaveBeenCalled();
37+
expect(catalogServiceSpy.setCatalogDescriptors).toHaveBeenCalledWith(descriptors);
3438
});
3539

3640
it('should resolve catalog descriptors without calling the service if present in memory', () => {
3741
catalogServiceSpy.getCatalogDescriptors.and.returnValue([{} as CatalogDescriptor]);
3842
service.resolve();
3943
expect(catalogServiceSpy.retrieveCatalogDescriptors).not.toHaveBeenCalled();
44+
expect(catalogServiceSpy.setCatalogDescriptors).not.toHaveBeenCalled();
4045
});
4146

4247
it('should handle errors from retrieveCatalogDescriptors and return empty array', (done) => {
@@ -52,6 +57,7 @@ describe('CatalogResolver', () => {
5257
service.resolve().subscribe(result => {
5358
expect(result).toEqual([]);
5459
expect(console.error).toHaveBeenCalledWith('Error retrieving catalog descriptors', error);
60+
expect(catalogServiceSpy.setCatalogDescriptors).toHaveBeenCalledWith([]);
5561
done();
5662
});
5763
});

src/app/services/catalog-resolver.service.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Injectable } from '@angular/core';
22
import { Resolve } from '@angular/router';
3-
import { catchError, Observable, of } from 'rxjs';
3+
import { catchError, Observable, of, tap } from 'rxjs';
44
import { CatalogService } from './catalog.service';
55
import { CatalogDescriptor } from '../openapi/component-catalog';
66

@@ -14,10 +14,14 @@ export class CatalogResolver implements Resolve<CatalogDescriptor[]> {
1414
const catalogDescriptors = this.catalogService.getCatalogDescriptors();
1515
if (catalogDescriptors.length > 0) {
1616
return of(catalogDescriptors);
17-
}
17+
}
1818
return this.catalogService.retrieveCatalogDescriptors().pipe(
19+
tap((descriptors) => {
20+
this.catalogService.setCatalogDescriptors(descriptors);
21+
}),
1922
catchError(error => {
2023
console.error('Error retrieving catalog descriptors', error);
24+
this.catalogService.setCatalogDescriptors([]);
2125
return of([]);
2226
})
2327
);

0 commit comments

Comments
 (0)