Skip to content

Commit e0d253d

Browse files
authored
fix(ssr): updated toast error for ssr (#1085)
- Ticket: https://openscience.atlassian.net/browse/ENG-12432 - Feature flag: n/a ## Summary of Changes 1. Updated toast logic for SSR. 2. Added condition for some actions. 3. Updated unit tests.
1 parent 1f440e2 commit e0d253d

8 files changed

Lines changed: 59 additions & 28 deletions

File tree

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,13 @@ describe('AppComponent', () => {
173173
expect(fixture.nativeElement.querySelector('.maintenance-overlay')).toBeFalsy();
174174
});
175175

176-
it('should not subscribe to router events on server', () => {
176+
it('should not dispatch user session or subscribe to router events on server', () => {
177177
setup({ isBrowser: false });
178178
routerBuilder.emit(new ResolveStart(1, '/next', '/next', {} as any));
179179
routerBuilder.emit(new NavigationEnd(5, '/x', '/x'));
180180
fixture.detectChanges();
181+
expect(store.dispatch).not.toHaveBeenCalled();
182+
expect(maintenanceModeServiceMock.checkOnce).not.toHaveBeenCalled();
181183
expect(loaderServiceMock.show).not.toHaveBeenCalled();
182184
expect(loaderServiceMock.hide).not.toHaveBeenCalled();
183185
expect(gtmServiceMock.pushTag).not.toHaveBeenCalled();

‎src/app/app.component.ts‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,15 @@ export class AppComponent implements OnInit {
5353
}
5454

5555
ngOnInit(): void {
56-
this.actions
57-
.getCurrentUser()
58-
.pipe(
59-
takeUntilDestroyed(this.destroyRef),
60-
switchMap(() => this.actions.getEmails())
61-
)
62-
.subscribe();
63-
6456
if (this.isBrowser) {
65-
// `GetCurrentUser` short-circuits on a cached user, so it cannot be relied on to surface an outage.
57+
this.actions
58+
.getCurrentUser()
59+
.pipe(
60+
takeUntilDestroyed(this.destroyRef),
61+
switchMap(() => this.actions.getEmails())
62+
)
63+
.subscribe();
64+
6665
this.maintenanceModeService.checkOnce();
6766

6867
this.router.events.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((event) => {

‎src/app/core/components/osf-banners/maintenance-banner/maintenance-banner.component.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ describe('MaintenanceBannerComponent', () => {
7373
expect(component.maintenance()).toEqual(activeMaintenance);
7474
});
7575

76-
it('should fetch maintenance on server without cookie check', () => {
76+
it('should not check cookie or fetch maintenance on server', () => {
7777
setup({ isBrowser: false, maintenance: activeMaintenance });
7878
expect(cookieServiceMock.check).not.toHaveBeenCalled();
79-
expect(maintenanceServiceMock.fetchMaintenanceStatus).toHaveBeenCalledTimes(1);
80-
expect(component.maintenance()).toEqual(activeMaintenance);
79+
expect(maintenanceServiceMock.fetchMaintenanceStatus).not.toHaveBeenCalled();
80+
expect(component.maintenance()).toBeNull();
8181
});
8282

8383
it('should dismiss and persist cookie in browser', () => {
@@ -89,7 +89,7 @@ describe('MaintenanceBannerComponent', () => {
8989
});
9090

9191
it('should dismiss without setting cookie on server', () => {
92-
setup({ isBrowser: false, maintenance: activeMaintenance });
92+
setup({ isBrowser: false });
9393
component.dismiss();
9494
expect(cookieServiceMock.set).not.toHaveBeenCalled();
9595
expect(component.dismissed()).toBe(true);

‎src/app/core/components/osf-banners/maintenance-banner/maintenance-banner.component.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,12 @@ export class MaintenanceBannerComponent implements OnInit {
6565
* - If not dismissed, triggers a fetch of current maintenance status
6666
*/
6767
ngOnInit(): void {
68-
if (isPlatformBrowser(this.platformId)) {
69-
this.dismissed.set(this.cookies.check(this.cookieName));
68+
if (!isPlatformBrowser(this.platformId)) {
69+
return;
7070
}
7171

72+
this.dismissed.set(this.cookies.check(this.cookieName));
73+
7274
if (!this.dismissed()) {
7375
this.fetchMaintenanceStatus();
7476
}

‎src/app/core/components/osf-banners/scheduled-banner/scheduled-banner.component.spec.ts‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { MockProvider } from 'ng-mocks';
44

55
import { BehaviorSubject } from 'rxjs';
66

7-
import { signal, WritableSignal } from '@angular/core';
7+
import { PLATFORM_ID, signal, WritableSignal } from '@angular/core';
88
import { ComponentFixture, TestBed } from '@angular/core/testing';
99

1010
import { BannerModel } from '@core/components/osf-banners/models/banner.model';
@@ -24,43 +24,51 @@ describe('Component: Scheduled Banner', () => {
2424
const currentBannerSignal: WritableSignal<BannerModel | null> = signal<BannerModel | null>(null);
2525
let isMobile$: BehaviorSubject<boolean>;
2626

27-
beforeEach(() => {
27+
function setup(overrides: { isBrowser?: boolean } = {}) {
2828
isMobile$ = new BehaviorSubject<boolean>(false);
29+
currentBannerSignal.set(null);
2930

3031
TestBed.configureTestingModule({
3132
imports: [ScheduledBannerComponent],
3233
providers: [
3334
provideOSFCore(),
3435
MockProvider(IS_XSMALL, isMobile$),
36+
MockProvider(PLATFORM_ID, overrides.isBrowser === false ? 'server' : 'browser'),
3537
provideMockStore({
3638
signals: [{ selector: BannersSelector.getCurrentBanner, value: currentBannerSignal }],
3739
}),
3840
],
3941
});
4042

43+
store = TestBed.inject(Store);
4144
fixture = TestBed.createComponent(ScheduledBannerComponent);
4245
component = fixture.componentInstance;
4346
fixture.detectChanges();
44-
45-
store = TestBed.inject(Store);
46-
});
47+
}
4748

4849
it('should create the component', () => {
50+
setup();
4951
expect(component).toBeInstanceOf(ScheduledBannerComponent);
5052
});
5153

52-
it('should dispatch FetchCurrentScheduledBanner on init', () => {
53-
const dispatchSpy = vi.spyOn(store, 'dispatch');
54-
component.ngOnInit();
55-
expect(dispatchSpy).toHaveBeenCalledWith(new GetCurrentScheduledBanner());
54+
it('should dispatch GetCurrentScheduledBanner on init in browser', () => {
55+
setup();
56+
expect(store.dispatch).toHaveBeenCalledWith(new GetCurrentScheduledBanner());
57+
});
58+
59+
it('should not dispatch GetCurrentScheduledBanner on server', () => {
60+
setup({ isBrowser: false });
61+
expect(store.dispatch).not.toHaveBeenCalled();
5662
});
5763

5864
it('should return false if no banner is set', () => {
65+
setup();
5966
currentBannerSignal.set(null);
6067
expect(component.shouldShowBanner()).toBe(false);
6168
});
6269

6370
it('should return true if current time is within banner window', () => {
71+
setup();
6472
const showBanner = component.shouldShowBanner;
6573
expect(showBanner()).toBeFalsy();
6674

@@ -87,6 +95,7 @@ describe('Component: Scheduled Banner', () => {
8795
});
8896

8997
it('should return false if current time is outside the banner window', () => {
98+
setup();
9099
const now = new Date();
91100
currentBannerSignal.set(
92101
Object({
@@ -104,6 +113,7 @@ describe('Component: Scheduled Banner', () => {
104113
});
105114

106115
it('should reflect the isMobile signal', () => {
116+
setup();
107117
const now = new Date();
108118
currentBannerSignal.set(
109119
Object({

‎src/app/core/components/osf-banners/scheduled-banner/scheduled-banner.component.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Store } from '@ngxs/store';
22

3-
import { ChangeDetectionStrategy, Component, computed, inject, OnInit } from '@angular/core';
3+
import { isPlatformBrowser } from '@angular/common';
4+
import { ChangeDetectionStrategy, Component, computed, inject, OnInit, PLATFORM_ID } from '@angular/core';
45
import { toSignal } from '@angular/core/rxjs-interop';
56

67
import { IS_XSMALL } from '@osf/shared/helpers/breakpoints.tokens';
@@ -27,6 +28,7 @@ export class ScheduledBannerComponent implements OnInit {
2728
* Injected NGXS Store instance used to dispatch actions and select state.
2829
*/
2930
private readonly store = inject(Store);
31+
private readonly platformId = inject(PLATFORM_ID);
3032

3133
/**
3234
* Reactive signal of the currently scheduled banner fetched from the NGXS store.
@@ -44,7 +46,9 @@ export class ScheduledBannerComponent implements OnInit {
4446
* when the component initializes.
4547
*/
4648
ngOnInit(): void {
47-
this.store.dispatch(new GetCurrentScheduledBanner());
49+
if (isPlatformBrowser(this.platformId)) {
50+
this.store.dispatch(new GetCurrentScheduledBanner());
51+
}
4852
}
4953

5054
/**

‎src/app/core/interceptors/error.interceptor.spec.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,18 @@ describe('errorInterceptor', () => {
113113
expect(toastServiceMock.showError).toHaveBeenCalledWith('server failed');
114114
});
115115

116+
it('should hide loader without toast on server platform', async () => {
117+
setup('server');
118+
const request = createRequest('/api/v2/nodes/abc');
119+
const error = new HttpErrorResponse({ status: 500, error: { message: 'server failed' }, url: request.url });
120+
121+
const caught = await runInterceptor(request, error);
122+
123+
expect(caught?.status).toBe(500);
124+
expect(loaderServiceMock.hide).toHaveBeenCalled();
125+
expect(toastServiceMock.showError).not.toHaveBeenCalled();
126+
});
127+
116128
it('should rethrow 409 without toast or loader handling', async () => {
117129
setup();
118130
const request = createRequest('/api/v2/nodes/abc');

‎src/app/core/interceptors/error.interceptor.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,9 @@ export const errorInterceptor: HttpInterceptorFn = (req, next) => {
105105

106106
loaderService.hide();
107107

108-
toastService.showError(errorMessage);
108+
if (isPlatformBrowser(platformId)) {
109+
toastService.showError(errorMessage);
110+
}
109111

110112
return throwError(() => error);
111113
})

0 commit comments

Comments
 (0)