diff --git a/docs/fixes/notification-experience-switch.md b/docs/fixes/notification-experience-switch.md new file mode 100644 index 0000000000..5ec3f9a3ac --- /dev/null +++ b/docs/fixes/notification-experience-switch.md @@ -0,0 +1,44 @@ +# Notification and real-time refresh after switching experiences + +## Problem + +`NotificationsService` is provided at the application root and replays its latest notification list. Ionic can retain the v3 page while the user visits the experience list, so returning to the dashboard does not necessarily recreate `V3Page` or rerun its initialization. Without an explicit refresh, the notification badge and list can therefore remain scoped to the previously selected project. + +## Required behavior + +- Notification state is scoped to the current user's `projectId`. +- Changing projects immediately clears the cached todo list and replayed event reminder. +- Todo items load before chat notifications because chat is appended to the freshly loaded todo list. +- Experience selection waits for this refresh before leaving the loading state. +- A notification refresh failure must not undo a successful experience switch. The new experience opens with an empty notification state rather than exposing notifications from the previous project. +- Responses from requests started under a previous project are ignored if they arrive after the active project changes. +- Pusher listeners are also experience-scoped. The application reuses one Pusher client, but disconnects its socket briefly on a scope change so pending private channels can be removed safely before reconnecting with the new channel set. +- The `login-refactor` baseline uses Pusher 8 with its bundled typings. Its pending-subscription behavior still requires the disconnect-before-removal sequence described below. + +## Refresh entry points + +- `V3Page` refreshes notifications when it is first initialized. +- `ExperiencesPage` refreshes notifications after authentication has switched to the selected experience and before navigating to the destination route. +- `ExperienceService` reinitializes web services after the selected experience's authentication response is available. `PusherService` detects scope changes using the program, project, and timeline identifiers, removes the previous listeners, refreshes authorization, and reconciles notification and chat subscriptions with the latest channel responses. +- `V3Page` also initializes web services as a fallback for authenticated login paths. App startup covers restored sessions, and direct login waits for initialization before navigating to deep links outside v3. +- Pusher initialization is single-flight. Concurrent entry points share one operation, and a scope that changes during that operation is reconciled before callers are released. +- Notification and chat discovery use independent generations. Only the latest response for the active scope may change listeners, preventing both previous-experience and same-experience request races. +- A valid empty channel response removes that listener type. Pusher leaves authorization failures in a pending state, so reconciliation disconnects before removing a pending channel; this ensures the channel is removed from Pusher's internal registry and cannot return on a later reconnect. A discovery failure preserves the last valid same-scope set, while a scope change remains empty because its previous listeners were removed before discovery. +- Pusher authorization headers are synchronized from user storage before channel subscription and connection retries. API-key rotation therefore does not require constructing another Pusher client. +- Private-channel subscription errors trigger one bounded background retry. Same-scope discovery does not cancel an outstanding retry, and simultaneous notification/chat failures are batched into one socket reconnect. Repeated failure is logged and never blocks navigation. +- Event callbacks capture their subscription scope and discard events after that scope becomes inactive. + +Both notification-refresh entry points use `NotificationsService.refreshNotifications()` so the reset and request ordering remain consistent. + +## Listener ownership and cleanup + +- `PusherService` is the only owner of Pusher channel subscriptions. Chat pages request `refreshChatChannels()` rather than subscribing to channel names directly. +- `TabsPage` remains the adapter from real-time notification, chat, and reminder events into `NotificationsService` state. +- V3, tabs, chat-list, and chat-room consumers release their event-stream subscriptions when destroyed. Chat-room typing listeners are replaced when the active room changes. +- Logout invalidates in-flight initialization and discovery work, resets generations and retry timers, disconnects the socket, removes all local channels, clears the active scope and authorization headers, and retains only the reusable application Pusher object. + +## Deferred improvements + +- Replace the string-keyed application event bus with typed real-time events. +- Add structured production telemetry for connection state, authorization errors, retries, and notification refresh failures. +- Revisit persistent capped retry backoff only if production telemetry shows the bounded retry is insufficient. diff --git a/projects/v3/src/app/app.component.spec.ts b/projects/v3/src/app/app.component.spec.ts index 5ab25d6bd8..c2a945efce 100644 --- a/projects/v3/src/app/app.component.spec.ts +++ b/projects/v3/src/app/app.component.spec.ts @@ -44,10 +44,10 @@ describe('AppComponent', () => { }, { provide: SharedService, - useValue: jasmine.createSpyObj('SharedService', [ - 'onPageLoad', - 'initWebServices', - ]), + useValue: jasmine.createSpyObj('SharedService', { + onPageLoad: undefined, + initWebServices: Promise.resolve(), + }), }, { provide: BrowserStorageService, diff --git a/projects/v3/src/app/app.component.ts b/projects/v3/src/app/app.component.ts index 41bbfdbc11..ad53116c36 100644 --- a/projects/v3/src/app/app.component.ts +++ b/projects/v3/src/app/app.component.ts @@ -213,7 +213,9 @@ export class AppComponent implements OnInit, OnDestroy { this.versionCheckService.initiateVersionCheck(); } // initialise Pusher when app loading - this.sharedService.initWebServices(); + this.sharedService.initWebServices().catch(err => { + console.error('Failed to initialise real-time services on app startup', err); + }); }); } diff --git a/projects/v3/src/app/pages/auth/auth-direct-login/auth-direct-login.component.spec.ts b/projects/v3/src/app/pages/auth/auth-direct-login/auth-direct-login.component.spec.ts index 256b66b052..e5d860edc2 100644 --- a/projects/v3/src/app/pages/auth/auth-direct-login/auth-direct-login.component.spec.ts +++ b/projects/v3/src/app/pages/auth/auth-direct-login/auth-direct-login.component.spec.ts @@ -41,7 +41,10 @@ describe('AuthDirectLoginComponent', () => { }, { provide: SharedService, - useValue: jasmine.createSpyObj('SharedService', ['onPageLoad', 'initWebServices']), + useValue: jasmine.createSpyObj('SharedService', { + onPageLoad: undefined, + initWebServices: Promise.resolve(), + }), }, { provide: AuthService, @@ -476,4 +479,3 @@ describe('AuthDirectLoginComponent', () => { }); }); }); - diff --git a/projects/v3/src/app/pages/auth/auth-direct-login/auth-direct-login.component.ts b/projects/v3/src/app/pages/auth/auth-direct-login/auth-direct-login.component.ts index 018ab6acc1..efaa49a5b1 100644 --- a/projects/v3/src/app/pages/auth/auth-direct-login/auth-direct-login.component.ts +++ b/projects/v3/src/app/pages/auth/auth-direct-login/auth-direct-login.component.ts @@ -211,10 +211,10 @@ export class AuthDirectLoginComponent implements OnInit { } } - private _saveOrRedirect(route: Array, options?: { + private async _saveOrRedirect(route: Array, options?: { save?: boolean; experience?: any; - }): void | Promise { + }): Promise { const currentLocation = window.location.href; const locale = options?.experience?.locale; if (currentLocation.indexOf('localhost') === -1 && locale && currentLocation.indexOf(locale) === -1) { @@ -237,7 +237,11 @@ export class AuthDirectLoginComponent implements OnInit { * When user use deep link to login to app. user not going through switcher service. * So pusher initialise not calling after user login using using deep link. */ - this.sharedService.initWebServices(); + try { + await this.sharedService.initWebServices(); + } catch (err) { + console.error('Failed to initialise real-time services for direct login', err); + } return this.navigate(route); } diff --git a/projects/v3/src/app/pages/chat/chat-list/chat-list.component.spec.ts b/projects/v3/src/app/pages/chat/chat-list/chat-list.component.spec.ts index 0c2f7028df..2b0f033673 100644 --- a/projects/v3/src/app/pages/chat/chat-list/chat-list.component.spec.ts +++ b/projects/v3/src/app/pages/chat/chat-list/chat-list.component.spec.ts @@ -1,5 +1,5 @@ -import { CUSTOM_ELEMENTS_SCHEMA, EventEmitter } from '@angular/core'; -import { waitForAsync, ComponentFixture, TestBed, tick, fakeAsync } from '@angular/core/testing'; +import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; +import { waitForAsync, ComponentFixture, TestBed } from '@angular/core/testing'; import { RouterTestingModule } from '@angular/router/testing'; import { ChatListComponent } from './chat-list.component'; import { ChatChannel, ChatService } from '@v3/services/chat.service'; @@ -14,19 +14,6 @@ import { FastFeedbackService } from '@v3/services/fast-feedback.service'; import { TestUtils } from '@testingv3/utils'; import { mockChats } from '@testingv3/fixtures'; -const mockPusherChannels = { - data: { - channels: [ - { - pusherChannel: 'sdb746-93r7dc-5f44eb4f' - }, - { - pusherChannel: 'kb5gt-9nfbj-5f45eb4g' - } - ] - } -}; - describe('ChatListComponent', () => { let component: ChatListComponent; let fixture: ComponentFixture; @@ -35,8 +22,6 @@ describe('ChatListComponent', () => { let storageSpy: jasmine.SpyObj; let pusherSpy: jasmine.SpyObj; let routerSpy: jasmine.SpyObj; - let routeStub: Partial; - let fastFeedbackSpy: jasmine.SpyObj; beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ @@ -52,7 +37,6 @@ describe('ChatListComponent', () => { provide: ChatService, useValue: jasmine.createSpyObj('ChatService', { 'getChatList': of(mockChats.data.channels), - 'getPusherChannels': of(true), }) }, { @@ -61,7 +45,9 @@ describe('ChatListComponent', () => { }, { provide: PusherService, - useValue: jasmine.createSpyObj('PusherService', ['subscribeChannel']) + useValue: jasmine.createSpyObj('PusherService', { + refreshChatChannels: Promise.resolve(), + }) }, { provide: Router, @@ -91,13 +77,11 @@ describe('ChatListComponent', () => { beforeEach(() => { fixture = TestBed.createComponent(ChatListComponent); component = fixture.componentInstance; - routeStub = TestBed.inject(ActivatedRoute); routerSpy = TestBed.inject(Router) as jasmine.SpyObj; chatSeviceSpy = TestBed.inject(ChatService) as jasmine.SpyObj; utils = TestBed.inject(UtilsService) as jasmine.SpyObj; storageSpy = TestBed.inject(BrowserStorageService) as jasmine.SpyObj; pusherSpy = TestBed.inject(PusherService) as jasmine.SpyObj; - fastFeedbackSpy = TestBed.inject(FastFeedbackService) as jasmine.SpyObj; }); it('should create', () => { @@ -114,18 +98,23 @@ describe('ChatListComponent', () => { utils.broadcastEvent('chat:info-update', {}); expect(chatSeviceSpy.getChatList.calls.count()).toBe(1); }); + + it('should stop handling chat events after destruction', () => { + component.ngOnDestroy(); + + utils.broadcastEvent('chat:new-message', {}); + + expect(chatSeviceSpy.getChatList).not.toHaveBeenCalled(); + }); }); describe('when testing onEnter()', () => { - it('should get correct chat list and pusher channels', () => { + it('should get the chat list and request exact Pusher reconciliation', () => { chatSeviceSpy.getChatList.and.returnValue(of(mockChats.data.channels)); - chatSeviceSpy.getPusherChannels.and.returnValue(of(mockPusherChannels.data.channels)); component.onEnter(); expect(component.chatList).toBeDefined(); expect(chatSeviceSpy.getChatList.calls.count()).toBe(1); - expect(chatSeviceSpy.getPusherChannels.calls.count()).toBe(1); - expect(pusherSpy.subscribeChannel).toHaveBeenCalledWith('chat', 'sdb746-93r7dc-5f44eb4f'); - expect(pusherSpy.subscribeChannel).toHaveBeenCalledWith('chat', 'kb5gt-9nfbj-5f45eb4g'); + expect(pusherSpy.refreshChatChannels).toHaveBeenCalled(); }); }); diff --git a/projects/v3/src/app/pages/chat/chat-list/chat-list.component.ts b/projects/v3/src/app/pages/chat/chat-list/chat-list.component.ts index 71297c636a..ebbd6f29d3 100644 --- a/projects/v3/src/app/pages/chat/chat-list/chat-list.component.ts +++ b/projects/v3/src/app/pages/chat/chat-list/chat-list.component.ts @@ -1,9 +1,11 @@ -import { ChangeDetectorRef, Component, Output, EventEmitter, NgZone, Input } from '@angular/core'; +import { ChangeDetectorRef, Component, Output, EventEmitter, NgZone, Input, OnDestroy } from '@angular/core'; import { Router, NavigationExtras } from '@angular/router'; import { BrowserStorageService } from '@v3/services/storage.service'; import { UtilsService } from '@v3/services/utils.service'; import { ChatService, ChatChannel } from '@v3/services/chat.service'; import { PusherService } from '@v3/services/pusher.service'; +import { Subject } from 'rxjs'; +import { takeUntil } from 'rxjs/operators'; /** * this is an app chat list component @@ -14,13 +16,14 @@ import { PusherService } from '@v3/services/pusher.service'; templateUrl: 'chat-list.component.html', styleUrls: ['chat-list.component.scss'] }) -export class ChatListComponent { +export class ChatListComponent implements OnDestroy { @Output() navigate = new EventEmitter(); @Output() chatListReady = new EventEmitter(); @Input() currentChat: ChatChannel; chatList: ChatChannel[]; loadingChatList = true; isMobile: boolean = false; + private readonly destroy$ = new Subject(); constructor( public utils: UtilsService, @@ -32,12 +35,22 @@ export class ChatListComponent { private pusherService: PusherService ) { this.isMobile = this.utils.isMobile(); - this.utils.getEvent('chat:new-message').subscribe(event => this._loadChatData()); - this.utils.getEvent('chat:delete-message').subscribe(event => this._loadChatData()); - this.utils.getEvent('chat:edit-message').subscribe(event => this._loadChatData()); - this.utils.getEvent('chat:info-update').subscribe(event => this._loadChatData()); + this.utils.getEvent('chat:new-message') + .pipe(takeUntil(this.destroy$)) + .subscribe(() => this._loadChatData()); + this.utils.getEvent('chat:delete-message') + .pipe(takeUntil(this.destroy$)) + .subscribe(() => this._loadChatData()); + this.utils.getEvent('chat:edit-message') + .pipe(takeUntil(this.destroy$)) + .subscribe(() => this._loadChatData()); + this.utils.getEvent('chat:info-update') + .pipe(takeUntil(this.destroy$)) + .subscribe(() => this._loadChatData()); if (!this.isMobile) { - this.utils.getEvent('chat-badge-update').subscribe(event => { + this.utils.getEvent('chat-badge-update') + .pipe(takeUntil(this.destroy$)) + .subscribe(event => { const chatIndex = this.chatList.findIndex(data => data.uuid === event.channelUuid); if (chatIndex > -1) { setTimeout(() => { @@ -60,10 +73,17 @@ export class ChatListComponent { */ onEnter() { this._initialise(); - this._checkAndSubscribePusherChannels(); + this.pusherService.refreshChatChannels().catch(err => { + console.error('Failed to refresh chat Pusher channels', err); + }); this._loadChatData(); } + ngOnDestroy(): void { + this.destroy$.next(); + this.destroy$.complete(); + } + /** * This is an _initialise method * @returns nothing @@ -78,29 +98,16 @@ export class ChatListComponent { * @returns nothing */ private _loadChatData(): void { - this.chatService.getChatList().subscribe(chats => { - this.ngZone.run(() => { - this.chatList = chats; - this.loadingChatList = false; - this.cdr.markForCheck(); + this.chatService.getChatList() + .pipe(takeUntil(this.destroy$)) + .subscribe(chats => { + this.ngZone.run(() => { + this.chatList = chats; + this.loadingChatList = false; + this.cdr.markForCheck(); + }); + this.chatListReady.emit(this.chatList); }); - this.chatListReady.emit(this.chatList); - }); - } - - /** - * This method pusher service to subscribe to chat pusher channels - * - first it call chat service to get pusher channels. - * - then it call pusher service 'subscribeChannel' method to subscribe. - * - in pusher service it chaeck if we alrady subscribe or not. - * if not it will subscribe to the pusher channel. - */ - private _checkAndSubscribePusherChannels() { - this.chatService.getPusherChannels().subscribe(pusherChannels => { - pusherChannels.forEach(channel => { - this.pusherService.subscribeChannel('chat', channel.pusherChannel); - }); - }); } goToChatRoom(chat: ChatChannel, keyboardEvent?: KeyboardEvent) { diff --git a/projects/v3/src/app/pages/chat/chat-room/chat-room.component.ts b/projects/v3/src/app/pages/chat/chat-room/chat-room.component.ts index 2b61414c3e..ffb347af59 100644 --- a/projects/v3/src/app/pages/chat/chat-room/chat-room.component.ts +++ b/projects/v3/src/app/pages/chat/chat-room/chat-room.component.ts @@ -11,7 +11,7 @@ import { ChatService, ChatChannel, Message, MessageListResult, ChannelMembers, F import { ChatPreviewComponent } from '../chat-preview/chat-preview.component'; import { ChatInfoComponent } from '../chat-info/chat-info.component'; import { EditMessagePopupComponent } from '../edit-message-popup/edit-message-popup.component'; -import { Subject, timer } from 'rxjs'; +import { Subject, Subscription, timer } from 'rxjs'; import { debounceTime, switchMap, takeUntil, tap } from 'rxjs/operators'; import { QuillModules } from 'ngx-quill'; import { UppyFileData, UppyUploaderResponse, UppyUploaderService } from '../../../components/uppy-uploader/uppy-uploader.service'; @@ -147,6 +147,7 @@ export class ChatRoomComponent implements OnInit, OnDestroy, AfterViewInit { private destroy$ = new Subject(); private scrollSubject = new Subject(); + private typingSubscription: Subscription; constructor( private chatService: ChatService, @@ -284,6 +285,7 @@ export class ChatRoomComponent implements OnInit, OnDestroy, AfterViewInit { } ngOnDestroy() { + this.typingSubscription?.unsubscribe(); this.destroy$.next(); this.destroy$.complete(); } @@ -315,8 +317,8 @@ export class ChatRoomComponent implements OnInit, OnDestroy, AfterViewInit { this.chatChannel = this.storage.getCurrentChatChannel(); } this.channelUuid = this.chatChannel.uuid; - // subscribe to typing event - this.utils + this.typingSubscription?.unsubscribe(); + this.typingSubscription = this.utils .getEvent("typing-" + this.chatChannel.pusherChannel) .pipe(takeUntil(this.destroy$)) .subscribe((event) => this._showTyping(event)); diff --git a/projects/v3/src/app/pages/experiences/experiences.page.spec.ts b/projects/v3/src/app/pages/experiences/experiences.page.spec.ts index 99e83988df..a1026e76e1 100644 --- a/projects/v3/src/app/pages/experiences/experiences.page.spec.ts +++ b/projects/v3/src/app/pages/experiences/experiences.page.spec.ts @@ -12,15 +12,15 @@ import { ExperiencesPage } from './experiences.page'; import { MockRouter } from '@testingv3/mocked.service'; import { ActivatedRouteStub } from '@testingv3/activated-route-stub'; import { TestUtils } from '@testingv3/utils'; -import { of } from 'rxjs'; +import { of, throwError } from 'rxjs'; describe('ExperiencesPage', () => { let component: ExperiencesPage; let fixture: ComponentFixture; let storageSpy: BrowserStorageService; - let experienceServiceSpy: ExperienceService; + let experienceServiceSpy: jasmine.SpyObj; let loadingCtrlSpy: LoadingController; - let notificationsSpy: NotificationsService; + let notificationsSpy: jasmine.SpyObj; beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ @@ -55,7 +55,8 @@ describe('ExperiencesPage', () => { { provide: NotificationsService, useValue: jasmine.createSpyObj('NotificationsService', { - 'alert': Promise.resolve(true) + 'alert': Promise.resolve(true), + 'refreshNotifications': of([]), }), }, { @@ -114,7 +115,8 @@ describe('ExperiencesPage', () => { let dismissLoading: any; beforeEach(() => { - experienceServiceSpy.switchProgramAndNavigate = jasmine.createSpy('switchProgramAndNavigate').and.returnValue(Promise.resolve(true)); + experienceServiceSpy.switchProgramAndNavigate.and.returnValue(Promise.resolve(['v3', 'home'])); + notificationsSpy.refreshNotifications.and.returnValue(of([])); presentLoading = jasmine.createSpy('present'); dismissLoading = jasmine.createSpy('dismiss').and.returnValue(Promise.resolve(true)); @@ -125,6 +127,20 @@ describe('ExperiencesPage', () => { }); it('should redirect user', fakeAsync(() => { + const operationOrder: string[] = []; + experienceServiceSpy.switchProgramAndNavigate.and.callFake(async () => { + operationOrder.push('switch'); + return ['v3', 'home']; + }); + notificationsSpy.refreshNotifications.and.callFake(() => { + operationOrder.push('refresh'); + return of([]); + }); + dismissLoading.and.callFake(() => { + operationOrder.push('dismiss'); + return Promise.resolve(true); + }); + component.switchProgram({ testing: true } as any); @@ -133,6 +149,8 @@ describe('ExperiencesPage', () => { expect(experienceServiceSpy.switchProgramAndNavigate).toHaveBeenCalledWith({ testing: true }); + expect(notificationsSpy.refreshNotifications).toHaveBeenCalled(); + expect(operationOrder).toEqual(['switch', 'refresh', 'dismiss']); })); it('should redirect user with keyboard event', fakeAsync(() => { @@ -165,7 +183,7 @@ describe('ExperiencesPage', () => { })); it('should throw error with alertCtrl', fakeAsync(() => { - experienceServiceSpy.switchProgramAndNavigate = jasmine.createSpy('switchProgramAndNavigate').and.throwError('SAMPLE_ERROR'); + experienceServiceSpy.switchProgramAndNavigate.and.throwError('SAMPLE_ERROR'); component.switchProgram({ testing: true @@ -173,6 +191,22 @@ describe('ExperiencesPage', () => { flushMicrotasks(); expect(notificationsSpy.alert).toHaveBeenCalled(); + expect(notificationsSpy.refreshNotifications).not.toHaveBeenCalled(); + })); + + it('should navigate with empty state when notification refresh fails', fakeAsync(() => { + const consoleErrorSpy = spyOn(console, 'error'); + notificationsSpy.refreshNotifications.and.returnValue( + throwError(() => new Error('Unable to refresh notifications')) + ); + + component.switchProgram({ testing: true } as any); + + flushMicrotasks(); + expect(consoleErrorSpy).toHaveBeenCalled(); + expect(dismissLoading).toHaveBeenCalled(); + expect(component['router'].navigate).toHaveBeenCalled(); + expect(notificationsSpy.alert).not.toHaveBeenCalled(); })); }); }); diff --git a/projects/v3/src/app/pages/experiences/experiences.page.ts b/projects/v3/src/app/pages/experiences/experiences.page.ts index a2ec72646a..3401a41108 100644 --- a/projects/v3/src/app/pages/experiences/experiences.page.ts +++ b/projects/v3/src/app/pages/experiences/experiences.page.ts @@ -8,7 +8,7 @@ import { BrowserStorageService } from '@v3/services/storage.service'; import { environment } from '@v3/environments/environment'; import { filter, takeUntil } from 'rxjs/operators'; import { UnlockIndicatorService } from '@v3/app/services/unlock-indicator.service'; -import { Subject, Observable } from 'rxjs'; +import { firstValueFrom, Observable, Subject } from 'rxjs'; @Component({ standalone: false, @@ -108,6 +108,15 @@ export class ExperiencesPage implements OnInit, OnDestroy { try { this.unlockIndicatorService.clearAllTasks(); // reset indicators const route = await this.experienceService.switchProgramAndNavigate(experience); + + try { + await firstValueFrom(this.notificationsService.refreshNotifications()); + } catch (refreshError) { + // The experience switch succeeded, so do not restore notifications + // from the previous project or block navigation when refreshing fails. + console.error('Error refreshing notifications after switching experience', refreshError); + } + await loading.dismiss(); if (environment.demo) { destination = ['v3','home']; diff --git a/projects/v3/src/app/pages/tabs/tabs.page.ts b/projects/v3/src/app/pages/tabs/tabs.page.ts index b471cdf4f7..fd9d0ab29e 100644 --- a/projects/v3/src/app/pages/tabs/tabs.page.ts +++ b/projects/v3/src/app/pages/tabs/tabs.page.ts @@ -60,9 +60,9 @@ export class TabsPage implements OnInit, OnDestroy { ngOnInit() { this.utils.setPageTitle('Practera'); - this.utils.screenStatus$.subscribe((res) => { + this.subscriptions.push(this.utils.screenStatus$.subscribe((res) => { this.hasLeftSidebar = res.leftSidebarExpanded; - }); + })); this.subscriptions.push(this.reviewService.reviews$.subscribe(res => this.reviews = res)); if (!this.storageService.getUser().chatEnabled) { // keep configuration-based value this.showMessages = false; @@ -103,7 +103,7 @@ export class TabsPage implements OnInit, OnDestroy { this.notificationsService.getReminderEvent(event).subscribe(); })); - this.notificationsService.notification$.subscribe(notifications => { + this.subscriptions.push(this.notificationsService.notification$.subscribe(notifications => { // assign notification badge to each tab this.badges.event = notifications.filter(noti => noti.type === 'event-reminder').length; this.badges.review = notifications.filter(noti => noti.type === 'review_submission').length; @@ -114,7 +114,7 @@ export class TabsPage implements OnInit, OnDestroy { } }); this.badges.chat = chat?.unreadMessages || 0; - }); + })); } ngOnDestroy(): void { diff --git a/projects/v3/src/app/pages/v3/v3.page.spec.ts b/projects/v3/src/app/pages/v3/v3.page.spec.ts index 7efd811880..a7f27edb5f 100644 --- a/projects/v3/src/app/pages/v3/v3.page.spec.ts +++ b/projects/v3/src/app/pages/v3/v3.page.spec.ts @@ -17,6 +17,7 @@ import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { HomeService } from '@v3/app/services/home.service'; import { NotificationsService } from '@v3/app/services/notifications.service'; import { UnlockIndicatorService } from '@v3/app/services/unlock-indicator.service'; +import { SharedService } from '@v3/app/services/shared.service'; describe('V3Page', () => { let component: V3Page; @@ -27,6 +28,7 @@ describe('V3Page', () => { let storageSpy: jasmine.SpyObj; let chatSpy: jasmine.SpyObj; let utilsSpy: jasmine.SpyObj; + let sharedSpy: jasmine.SpyObj; beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ @@ -84,8 +86,7 @@ describe('V3Page', () => { { provide: NotificationsService, useValue: jasmine.createSpyObj('NotificationsService', { - 'getTodoItems': of(), - 'getChatMessage': of(), + 'refreshNotifications': of(), }, { 'notification$': of(), }), @@ -96,6 +97,12 @@ describe('V3Page', () => { 'unlockedTasks$': of([]), }), }, + { + provide: SharedService, + useValue: jasmine.createSpyObj('SharedService', { + initWebServices: Promise.resolve(), + }), + }, ] }).compileComponents(); @@ -107,6 +114,7 @@ describe('V3Page', () => { storageSpy = TestBed.inject(BrowserStorageService) as jasmine.SpyObj; chatSpy = TestBed.inject(ChatService) as jasmine.SpyObj; utilsSpy = TestBed.inject(UtilsService) as jasmine.SpyObj; + sharedSpy = TestBed.inject(SharedService) as jasmine.SpyObj; component = fixture.componentInstance; fixture.detectChanges(); @@ -120,7 +128,7 @@ describe('V3Page', () => { // Prepare data and spies const getReviewsSpy = reviewSpy.getReviews; utilsSpy.moveToNewLocale.and.stub(); - const getTodoItemsSpy = notificationsSpy.getTodoItems.and.returnValue(of()); + const refreshNotificationsSpy = notificationsSpy.refreshNotifications.and.returnValue(of()); const getChatListSpy = chatSpy.getChatList.and.returnValue(of([])); storageSpy.getUser.and.returnValue({ role: 'participant', @@ -133,7 +141,8 @@ describe('V3Page', () => { // Check if the required methods are called // Note: getExperience is only called on NavigationEnd events to /v3/home, not during ngOnInit expect(getReviewsSpy).toHaveBeenCalled(); - expect(getTodoItemsSpy).toHaveBeenCalled(); + expect(refreshNotificationsSpy).toHaveBeenCalled(); + expect(sharedSpy.initWebServices).toHaveBeenCalled(); expect(getChatListSpy).toHaveBeenCalled(); // Check if component properties are set correctly diff --git a/projects/v3/src/app/pages/v3/v3.page.ts b/projects/v3/src/app/pages/v3/v3.page.ts index 9e6b621343..b6bb9997c7 100644 --- a/projects/v3/src/app/pages/v3/v3.page.ts +++ b/projects/v3/src/app/pages/v3/v3.page.ts @@ -1,4 +1,4 @@ -import { takeUntil, mergeMap } from 'rxjs/operators'; +import { takeUntil } from 'rxjs/operators'; import { Component, HostListener, isDevMode, OnDestroy, OnInit } from '@angular/core'; import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; import { MenuController, ModalController } from '@ionic/angular'; @@ -6,7 +6,7 @@ import { Review, ReviewService } from '@v3/app/services/review.service'; import { BrowserStorageService } from '@v3/app/services/storage.service'; import { AnimationsService } from '@v3/services/animations.service'; import { ChatService } from '@v3/app/services/chat.service'; -import { Subject, Subscription } from 'rxjs'; +import { Subject } from 'rxjs'; import { SettingsPage } from '../settings/settings.page'; import { UtilsService } from '@v3/app/services/utils.service'; import { animate, group, query, state, style, transition, trigger } from '@angular/animations'; @@ -14,6 +14,7 @@ import { NotificationsService } from '@v3/app/services/notifications.service'; import { HomeService } from '@v3/app/services/home.service'; import { environment } from '@v3/environments/environment'; import { UnlockIndicatorService } from '@v3/app/services/unlock-indicator.service'; +import { SharedService } from '@v3/app/services/shared.service'; @Component({ standalone: false, @@ -97,6 +98,7 @@ export class V3Page implements OnInit, OnDestroy { private readonly notificationsService: NotificationsService, private readonly homeService: HomeService, private readonly unlockIndicatorService: UnlockIndicatorService, + private readonly sharedService: SharedService, ) { } @@ -181,6 +183,9 @@ export class V3Page implements OnInit, OnDestroy { ngOnInit(): void { this.institutionLogo = this.getInstitutionLogo(); this._initMenuItems(); + this.sharedService.initWebServices().catch(err => { + console.error('Failed to initialise real-time services on v3 entry', err); + }); this.reviewService.reviews$ .pipe( @@ -194,7 +199,9 @@ export class V3Page implements OnInit, OnDestroy { } }); - this.notificationsService.notification$.subscribe(notifications => { + this.notificationsService.notification$ + .pipe(takeUntil(this.unsubscribe$)) + .subscribe(notifications => { // assign notification badge to each tab const chat = (notifications || []).find(noti => { if (noti.type === 'chat') { @@ -264,11 +271,8 @@ export class V3Page implements OnInit, OnDestroy { } this.isMenuOpen = false; - // initiate subscription v3 page level (required), so the rest independent listener can pickup the same sharedReplay - this.notificationsService.getTodoItems().pipe( - mergeMap(_generic => { - return this.notificationsService.getChatMessage(); - }), + // Initialize the shared notification stream for a fresh v3 page load. + this.notificationsService.refreshNotifications().pipe( takeUntil(this.unsubscribe$) ).subscribe(); diff --git a/projects/v3/src/app/services/auth.service.spec.ts b/projects/v3/src/app/services/auth.service.spec.ts index cabc7d17fa..26fa079e9d 100644 --- a/projects/v3/src/app/services/auth.service.spec.ts +++ b/projects/v3/src/app/services/auth.service.spec.ts @@ -74,7 +74,7 @@ describe('AuthService', () => { }, { provide: PusherService, - useValue: jasmine.createSpyObj('PusherService', ['unsubscribeChannels', 'disconnect']) + useValue: jasmine.createSpyObj('PusherService', ['reset']) }, { provide: NotificationsService, useValue: notificationsSpy }, { @@ -208,8 +208,7 @@ describe('AuthService', () => { it('should navigate to login by default', () => { storageSpy.getConfig.and.returnValue({ color: '' }); service.logout({}); - expect(pusherSpy.unsubscribeChannels.calls.count()).toBe(1); - expect(pusherSpy.disconnect.calls.count()).toBe(1); + expect(pusherSpy.reset.calls.count()).toBe(1); expect(storageSpy.clear.calls.count()).toBe(1); expect(routerSpy.navigate.calls.first().args[0]).toEqual(['/']); }); @@ -217,8 +216,7 @@ describe('AuthService', () => { it('should pass navigation data', () => { storageSpy.getConfig.and.returnValue({ color: '' }); service.logout({ data: 'data' }); - expect(pusherSpy.unsubscribeChannels.calls.count()).toBe(1); - expect(pusherSpy.disconnect.calls.count()).toBe(1); + expect(pusherSpy.reset.calls.count()).toBe(1); expect(storageSpy.clear.calls.count()).toBe(1); expect(routerSpy.navigate.calls.first().args[0]).toEqual(['/'], { data: 'data' }); }); diff --git a/projects/v3/src/app/services/auth.service.ts b/projects/v3/src/app/services/auth.service.ts index f3d55129f7..34ded98f12 100644 --- a/projects/v3/src/app/services/auth.service.ts +++ b/projects/v3/src/app/services/auth.service.ts @@ -365,8 +365,7 @@ export class AuthService { * @param redirect Whether redirect the user to login page or not */ logout(navigationParams = {}, redirect: boolean | string[] = true) { - this.pusherService.unsubscribeChannels(); - this.pusherService.disconnect(); + this.pusherService.reset(); const config = this.storage.getConfig(); this.unlockIndicatorService.clearAllTasks(); // reset indicators (cache) diff --git a/projects/v3/src/app/services/experience.service.spec.ts b/projects/v3/src/app/services/experience.service.spec.ts index 8792559e37..a4bea7cf31 100644 --- a/projects/v3/src/app/services/experience.service.spec.ts +++ b/projects/v3/src/app/services/experience.service.spec.ts @@ -12,6 +12,7 @@ import { ReviewService } from './review.service'; import { SharedService } from './shared.service'; import { BrowserStorageService } from './storage.service'; import { UtilsService } from './utils.service'; +import { of } from 'rxjs'; describe('ExperienceService', () => { let service: ExperienceService; @@ -33,11 +34,23 @@ describe('ExperienceService', () => { }, { provide: SharedService, - useValue: jasmine.createSpyObj('SharedService', ['getConfig']), + useValue: jasmine.createSpyObj('SharedService', { + getConfig: undefined, + onPageLoad: undefined, + getTeamInfo: of({}), + initWebServices: Promise.resolve(), + }), }, { provide: BrowserStorageService, - useValue: jasmine.createSpyObj('BrowserStorageService', ['get', 'set', 'getUser', 'getConfig']), + useValue: jasmine.createSpyObj('BrowserStorageService', [ + 'get', + 'set', + 'setUser', + 'remove', + 'getUser', + 'getConfig', + ]), }, { provide: RequestService, @@ -53,11 +66,19 @@ describe('ExperienceService', () => { }, { provide: HomeService, - useValue: jasmine.createSpyObj('HomeService', ['getTodoItems']), + useValue: jasmine.createSpyObj('HomeService', { + getTodoItems: undefined, + clearExperience: of([]), + }), }, { provide: AuthService, - useValue: jasmine.createSpyObj('AuthService', ['getConfig']), + useValue: jasmine.createSpyObj('AuthService', { + getConfig: undefined, + getMyInfo: of({}), + authenticate: of({}), + clearCache: Promise.resolve(), + }), }, ], }); @@ -67,4 +88,68 @@ describe('ExperienceService', () => { it('should be created', () => { expect(service).toBeTruthy(); }); + + it('should initialise Pusher only after the selected experience is authenticated', async () => { + const callOrder: string[] = []; + const sharedService = TestBed.inject(SharedService) as jasmine.SpyObj; + const authService = TestBed.inject(AuthService) as jasmine.SpyObj; + const storageService = TestBed.inject(BrowserStorageService) as jasmine.SpyObj; + const experience = { + id: 2, + uuid: 'experience-2', + projectId: 22, + timelineId: 222, + featureToggle: {}, + }; + + storageService.get.and.returnValue(null); + authService.authenticate.and.callFake(() => { + callOrder.push('authenticate'); + return of({ + data: { + auth: { + apikey: 'new-api-key', + }, + }, + } as any); + }); + sharedService.initWebServices.and.callFake(async () => { + callOrder.push('pusher'); + }); + + await service.switchProgramAndNavigate(experience as any); + + expect(callOrder).toEqual(['authenticate', 'pusher']); + expect(storageService.setUser).toHaveBeenCalledWith({ apikey: 'new-api-key' }); + expect(sharedService.initWebServices).toHaveBeenCalledTimes(1); + }); + + it('should continue the experience switch when Pusher refresh fails', async () => { + const sharedService = TestBed.inject(SharedService) as jasmine.SpyObj; + const authService = TestBed.inject(AuthService) as jasmine.SpyObj; + const storageService = TestBed.inject(BrowserStorageService) as jasmine.SpyObj; + const consoleError = spyOn(console, 'error'); + const experience = { + id: 2, + uuid: 'experience-2', + projectId: 22, + timelineId: 222, + featureToggle: {}, + }; + + storageService.get.and.returnValue(null); + authService.authenticate.and.returnValue(of({ + data: { auth: { apikey: 'new-api-key' } }, + } as any)); + sharedService.initWebServices.and.rejectWith(new Error('Pusher unavailable')); + + const route = await service.switchProgramAndNavigate(experience as any); + + expect(route).toEqual(['v3', 'home']); + expect(authService.clearCache).toHaveBeenCalled(); + expect(consoleError).toHaveBeenCalledWith( + 'Failed to refresh experience-scoped web services', + jasmine.any(Error) + ); + }); }); diff --git a/projects/v3/src/app/services/experience.service.ts b/projects/v3/src/app/services/experience.service.ts index 956a6a623d..7a1807c08d 100644 --- a/projects/v3/src/app/services/experience.service.ts +++ b/projects/v3/src/app/services/experience.service.ts @@ -306,8 +306,6 @@ export class ExperienceService { // eslint-disable-next-line rxjs/no-ignored-observable this.homeService.clearExperience(); - // initialise Pusher - this.sharedService.initWebServices(); try { const teamInfo = await this.sharedService.getTeamInfo().toPromise(); const me = await this.authService.getMyInfo().toPromise(); @@ -355,11 +353,26 @@ export class ExperienceService { } await this.switchProgram({ experience }); - await firstValueFrom(this.authService.authenticate({ + const authResponse = await firstValueFrom(this.authService.authenticate({ experienceUuid: experience.uuid, })); - // await this.pusherService.initialise({ unsubscribe: true }); + const apikey = authResponse?.data?.auth?.apikey; + if (apikey) { + this.storage.setUser({ apikey }); + } + + // Reconcile experience-scoped Pusher listeners only after the new auth + // context is available. PusherService reuses the application connection, + // removes the previous channels, and refreshes channel authorization. + try { + await this.sharedService.initWebServices(); + } catch (err) { + // Pusher availability must not block an otherwise successful experience + // switch. The old listeners have already been removed before reconnecting. + console.error('Failed to refresh experience-scoped web services', err); + } + // clear the cached data await this.authService.clearCache(); diff --git a/projects/v3/src/app/services/notifications.service.spec.ts b/projects/v3/src/app/services/notifications.service.spec.ts index d040744f32..884c6902d1 100644 --- a/projects/v3/src/app/services/notifications.service.spec.ts +++ b/projects/v3/src/app/services/notifications.service.spec.ts @@ -1,13 +1,13 @@ import { TestBed } from '@angular/core/testing'; import { ModalController, AlertController, ToastController, LoadingController } from '@ionic/angular'; import { TestUtils } from '@testingv3/utils'; -import { of } from 'rxjs'; +import { firstValueFrom, of, throwError } from 'rxjs'; import { RequestService } from 'request'; import { AchievementService } from './achievement.service'; import { ApolloService } from './apollo.service'; import { EventService } from './event.service'; -import { NotificationsService } from './notifications.service'; +import { NotificationsService, TodoItem } from './notifications.service'; import { BrowserStorageService } from './storage.service'; import { UtilsService } from './utils.service'; @@ -118,4 +118,110 @@ describe('NotificationsService', () => { expect(variables.id).toBe('456'); }); }); + + describe('refreshNotifications', () => { + let storageService: jasmine.SpyObj; + + beforeEach(() => { + storageService = TestBed.inject(BrowserStorageService) as jasmine.SpyObj; + }); + + it('should clear project-scoped state and load todo items before chat', async () => { + const notificationEmissions: TodoItem[][] = []; + const reminderEmissions: any[] = []; + const callOrder: string[] = []; + + service.notification$.subscribe(items => notificationEmissions.push(items)); + service.eventReminder$.subscribe(reminder => reminderEmissions.push(reminder)); + service.addNewNotification({ id: 1, name: 'Old notification' }); + service['_eventReminder$'].next({ id: 'old-reminder' }); + service['notificationsProjectId'] = 1; + storageService.getUser.and.returnValue({ projectId: 2 } as any); + + spyOn(service, 'getTodoItems').and.callFake(() => { + callOrder.push('todo'); + expect(notificationEmissions[notificationEmissions.length - 1]).toEqual([]); + return of([]); + }); + spyOn(service, 'getChatMessage').and.callFake(() => { + callOrder.push('chat'); + return of({}); + }); + + await firstValueFrom(service.refreshNotifications()); + + expect(callOrder).toEqual(['todo', 'chat']); + expect(notificationEmissions[notificationEmissions.length - 1]).toEqual([]); + expect(reminderEmissions[reminderEmissions.length - 1]).toEqual({}); + }); + + it('should return the combined todo and chat notification list', async () => { + const todoItem: TodoItem = { id: 2, type: 'review_submission' }; + const chatItem: TodoItem = { id: 3, type: 'chat' }; + storageService.getUser.and.returnValue({ projectId: 2 } as any); + + spyOn(service, 'getTodoItems').and.callFake(() => { + service['notifications'] = [todoItem]; + service['_notification$'].next([todoItem]); + return of([todoItem]); + }); + spyOn(service, 'getChatMessage').and.callFake(() => { + service.addNewNotification(chatItem); + return of(chatItem); + }); + + const result = await firstValueFrom(service.refreshNotifications()); + + expect(result).toEqual([todoItem, chatItem]); + }); + + it('should refresh again when the project has not changed', async () => { + storageService.getUser.and.returnValue({ projectId: 4 } as any); + const getTodoItemsSpy = spyOn(service, 'getTodoItems').and.returnValue(of([])); + const getChatMessageSpy = spyOn(service, 'getChatMessage').and.returnValue(of({})); + + await firstValueFrom(service.refreshNotifications()); + await firstValueFrom(service.refreshNotifications()); + + expect(getTodoItemsSpy).toHaveBeenCalledTimes(2); + expect(getChatMessageSpy).toHaveBeenCalledTimes(2); + }); + + it('should leave the new project empty when refresh fails', async () => { + const notificationEmissions: TodoItem[][] = []; + service.notification$.subscribe(items => notificationEmissions.push(items)); + service.addNewNotification({ id: 1, name: 'Old notification' }); + service['notificationsProjectId'] = 1; + storageService.getUser.and.returnValue({ projectId: 2 } as any); + spyOn(service, 'getTodoItems').and.returnValue( + throwError(() => new Error('Unable to refresh notifications')) + ); + spyOn(service, 'getChatMessage'); + + await expectAsync(firstValueFrom(service.refreshNotifications())).toBeRejected(); + + expect(notificationEmissions[notificationEmissions.length - 1]).toEqual([]); + expect(service.getChatMessage).not.toHaveBeenCalled(); + }); + + it('should discard a partial todo result when the new project chat refresh fails', async () => { + const notificationEmissions: TodoItem[][] = []; + const currentTodo: TodoItem = { id: 2, type: 'review_submission' }; + service.notification$.subscribe(items => notificationEmissions.push(items)); + service['notificationsProjectId'] = 1; + storageService.getUser.and.returnValue({ projectId: 2 } as any); + spyOn(service, 'getTodoItems').and.callFake(() => { + service['notifications'] = [currentTodo]; + service['_notification$'].next([currentTodo]); + return of([currentTodo]); + }); + spyOn(service, 'getChatMessage').and.returnValue( + throwError(() => new Error('Unable to refresh chat notifications')) + ); + + await expectAsync(firstValueFrom(service.refreshNotifications())).toBeRejected(); + + expect(notificationEmissions[notificationEmissions.length - 1]).toEqual([]); + }); + }); }); diff --git a/projects/v3/src/app/services/notifications.service.ts b/projects/v3/src/app/services/notifications.service.ts index d2ecf0919f..f2ba9e37e3 100644 --- a/projects/v3/src/app/services/notifications.service.ts +++ b/projects/v3/src/app/services/notifications.service.ts @@ -8,10 +8,10 @@ import { Achievement, AchievementService } from './achievement.service'; import { UtilsService } from '@v3/services/utils.service'; import { ReviewRatingComponent } from '../components/review-rating/review-rating.component'; import { LockTeamAssessmentPopUpComponent } from '../components/lock-team-assessment-pop-up/lock-team-assessment-pop-up.component'; -import { firstValueFrom, Observable, of, Subject } from 'rxjs'; +import { defer, firstValueFrom, Observable, of, Subject, throwError } from 'rxjs'; import { RequestService } from 'request'; import { BrowserStorageService } from './storage.service'; -import { map, shareReplay } from 'rxjs/operators'; +import { catchError, map, shareReplay, switchMap } from 'rxjs/operators'; import { ApolloService } from './apollo.service'; import { EventService } from './event.service'; import { NetworkService } from './network.service'; @@ -101,6 +101,7 @@ export class NotificationsService { eventReminder$ = this._eventReminder$.pipe(shareReplay(1)); private notifications: TodoItem[] = []; + private notificationsProjectId: number | null = null; private connection = { informed: false, @@ -162,6 +163,44 @@ export class NotificationsService { this._notification$.next(this.notifications); } + /** + * Refresh all notifications for the project in the current user context. + * Generic todo items must load before chat so the chat notification can be + * appended to the freshly loaded list. + */ + refreshNotifications(): Observable { + return defer(() => { + const projectId = this.storage.getUser()?.projectId ?? null; + const projectChanged = projectId !== this.notificationsProjectId; + + if (projectChanged) { + this.notificationsProjectId = projectId; + this.notifications = []; + this._notification$.next([]); + this._eventReminder$.next({}); + } + + return this.getTodoItems().pipe( + switchMap(() => this.getChatMessage()), + map(() => [...this.notifications]), + catchError(err => { + // A partially completed first load for a new project must not leave + // an ambiguous list. Same-project refresh failures retain the last + // valid state, while project switches remain explicitly empty. + if ( + projectChanged + && (this.storage.getUser()?.projectId ?? null) === projectId + ) { + this.notifications = []; + this._notification$.next([]); + this._eventReminder$.next({}); + } + return throwError(() => err); + }), + ); + }); + } + /** * @name modalConfig * @description futher customised filter @@ -482,6 +521,8 @@ export class NotificationsService { } getTodoItems(): Observable { + const projectId = this.storage.getUser()?.projectId ?? null; + return this.apolloService.graphQLFetch( `query project { project { @@ -499,6 +540,10 @@ export class NotificationsService { }` ).pipe( map((response) => { + if ((this.storage.getUser()?.projectId ?? null) !== projectId) { + return []; + } + const rawItems = response?.data?.project?.todoItems; if (rawItems) { const legacyItems = (rawItems as any[]).map(item => this._fromGqlTodoItem(item)); @@ -765,6 +810,8 @@ export class NotificationsService { } getChatMessage() { + const projectId = this.storage.getUser()?.projectId ?? null; + return this.apolloService .graphQLFetch( `query getChannels { @@ -775,6 +822,10 @@ export class NotificationsService { ) .pipe( map((response) => { + if ((this.storage.getUser()?.projectId ?? null) !== projectId) { + return {}; + } + if (response.data) { const normalized = this._normaliseChatMessage(response.data); if (!this.utils.isEmpty(normalized)) { diff --git a/projects/v3/src/app/services/pusher.service.spec.ts b/projects/v3/src/app/services/pusher.service.spec.ts index 2b741b7677..ccb26d9abf 100644 --- a/projects/v3/src/app/services/pusher.service.spec.ts +++ b/projects/v3/src/app/services/pusher.service.spec.ts @@ -1,12 +1,11 @@ -import { TestBed, fakeAsync, flushMicrotasks } from '@angular/core/testing'; -import { of } from 'rxjs'; +import { TestBed, fakeAsync, flushMicrotasks, tick } from '@angular/core/testing'; +import { of, Subject, throwError } from 'rxjs'; import { PusherService } from '@v3/services/pusher.service'; import { BrowserStorageService } from '@v3/services/storage.service'; import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; import { Router } from '@angular/router'; import { MockRouter } from '@testingv3/mocked.service'; import { UtilsService } from '@v3/services/utils.service'; -import { RequestService } from 'request'; import { environment } from '@v3/environments/environment'; import Pusher from 'pusher-js'; import { TestUtils } from '@testingv3/utils'; @@ -73,9 +72,8 @@ describe('PusherService', async () => { }; let service: PusherService; - let requestSpy: jasmine.SpyObj; let utilSpy: UtilsService; - let storageSpy: BrowserStorageService; + let storageSpy: jasmine.SpyObj; let mockBackend: HttpTestingController; let apolloSpy: jasmine.SpyObj; // let pusherLibSpy: any; @@ -127,21 +125,13 @@ describe('PusherService', async () => { graphQLFetch: of({ data: { notificationChannel: null, channels: [] } }) }), }, - { - provide: RequestService, - useValue: jasmine.createSpyObj('RequestService', { - get: of({ data: [] }), - apiResponseFormatError: 'ERROR', - }), - } ], }).compileComponents(); mockBackend = TestBed.inject(HttpTestingController); service = TestBed.inject(PusherService); - requestSpy = TestBed.inject(RequestService) as jasmine.SpyObj; utilSpy = TestBed.inject(UtilsService); - storageSpy = TestBed.inject(BrowserStorageService); + storageSpy = TestBed.inject(BrowserStorageService) as jasmine.SpyObj; apolloSpy = TestBed.inject(ApolloService) as jasmine.SpyObj; }); @@ -150,11 +140,9 @@ describe('PusherService', async () => { }); const notificationRes = { - data: [ - { - channel: 'notification-channel' - } - ] + data: { + notificationChannel: 'notification-channel', + }, }; const pusherChatChannelRes: ApolloQueryResult = { @@ -170,6 +158,8 @@ describe('PusherService', async () => { }, loading: false, networkStatus: 7, + partial: false, + dataState: 'complete', }; describe('getChannels()', async () => { @@ -190,12 +180,131 @@ describe('PusherService', async () => { service.getChatChannels().subscribe(); expect(apolloSpy.graphQLFetch.calls.count()).toBe(1); }); + + it('should ignore a notification-channel response from a previous experience', () => { + const response$ = new Subject(); + apolloSpy.graphQLFetch.and.returnValue(response$); + storageSpy.getUser = jasmine.createSpy('getUser').and.returnValue({ + apikey: 'old-key', + timelineId: 1, + }); + spyOn(service, 'subscribeChannel'); + + service.getNotificationChannel().subscribe(); + storageSpy.getUser.and.returnValue({ + apikey: 'new-key', + timelineId: 2, + }); + response$.next(notificationRes); + + expect(service.subscribeChannel).not.toHaveBeenCalled(); + }); + + it('should ignore both channel responses when the experience changes mid-refresh', async () => { + const notificationResponse$ = new Subject(); + const chatResponse$ = new Subject(); + apolloSpy.graphQLFetch.and.returnValues(notificationResponse$, chatResponse$); + storageSpy.getUser.and.returnValue({ + apikey: 'old-key', + timelineId: 1, + } as any); + spyOn(service, 'subscribeChannel'); + + const refresh = service.getChannels(); + storageSpy.getUser.and.returnValue({ + apikey: 'new-key', + timelineId: 2, + } as any); + notificationResponse$.next(notificationRes); + chatResponse$.next(pusherChatChannelRes); + await refresh; + + expect(service.subscribeChannel).not.toHaveBeenCalled(); + }); + + it('should remove chat listeners that are not in the current experience channel set', () => { + const removedSubscription = jasmine.createSpyObj('removedSubscription', ['unbind_all']); + const retainedSubscription = jasmine.createSpyObj('retainedSubscription', ['unbind_all']); + const pusher = jasmine.createSpyObj('pusher', ['unsubscribe']); + service['pusher'] = pusher; + service['channels'].chat = [ + { name: 'old-chat-channel', subscription: removedSubscription }, + { name: 'current-chat-channel', subscription: retainedSubscription }, + ]; + apolloSpy.graphQLFetch.and.returnValue(of({ + ...pusherChatChannelRes, + data: { + channels: [ + { pusherChannel: 'current-chat-channel' }, + { pusherChannel: 'new-chat-channel' }, + ], + }, + })); + spyOn(service, 'subscribeChannel'); + + service.getChatChannels().subscribe(); + + expect(removedSubscription.unbind_all).toHaveBeenCalled(); + expect(pusher.unsubscribe).toHaveBeenCalledWith('old-chat-channel'); + expect(retainedSubscription.unbind_all).not.toHaveBeenCalled(); + expect(service['channels'].chat.map(channel => channel.name)).toEqual(['current-chat-channel']); + expect(service.subscribeChannel).toHaveBeenCalledWith('chat', 'current-chat-channel'); + expect(service.subscribeChannel).toHaveBeenCalledWith('chat', 'new-chat-channel'); + }); + + it('should ignore an older same-scope chat response', () => { + const firstResponse$ = new Subject(); + const secondResponse$ = new Subject(); + apolloSpy.graphQLFetch.and.returnValues(firstResponse$, secondResponse$); + const reconcileSpy = spyOn(service, 'reconcileChatChannels'); + + service.getChatChannels().subscribe(); + service.getChatChannels().subscribe(); + secondResponse$.next({ data: { channels: [{ pusherChannel: 'latest-channel' }] } }); + firstResponse$.next({ data: { channels: [{ pusherChannel: 'stale-channel' }] } }); + + expect(reconcileSpy).toHaveBeenCalledTimes(1); + expect(reconcileSpy).toHaveBeenCalledWith(['latest-channel']); + }); + + it('should treat an empty chat response as the exact current set', () => { + const subscription = jasmine.createSpyObj('subscription', ['unbind_all']); + const pusher = jasmine.createSpyObj('pusher', ['unsubscribe']); + service['pusher'] = pusher; + service['channels'].chat = [{ name: 'removed-channel', subscription }]; + apolloSpy.graphQLFetch.and.returnValue(of({ data: { channels: [] } } as any)); + + service.getChatChannels().subscribe(); + + expect(subscription.unbind_all).toHaveBeenCalled(); + expect(pusher.unsubscribe).toHaveBeenCalledWith('removed-channel'); + expect(service['channels'].chat).toEqual([]); + }); + + it('should preserve same-scope chat listeners when discovery fails', async () => { + const existingChannels = [{ name: 'current-channel', subscription: null }]; + service['channels'].chat = existingChannels; + service['activeScope'] = { programId: null, projectId: null, timelineId: 1 }; + service['pusher'] = jasmine.createSpyObj('pusher', [], { + config: { auth: { headers: {} } }, + connection: { state: 'connected' }, + }); + apolloSpy.graphQLFetch.and.returnValue( + throwError(() => new Error('Channel discovery failed')) + ); + spyOn(console, 'error'); + + await service.refreshChatChannels(); + + expect(service['channels'].chat).toBe(existingChannels); + }); }); describe('subscribeChannel()', () => { beforeEach(() => { environment.env = 'test'; service['pusher'] = new PusherLib(); + service['activeScope'] = { programId: null, projectId: null, timelineId: 1 }; // spyOn(service, 'initialise').and.returnValue(Promise.resolve(service['pusher'])); const subscribed = []; @@ -248,15 +357,17 @@ describe('PusherService', async () => { describe('initialise()', () => { beforeEach(() => { - service['initialisePusher'] = jasmine.createSpy('initialisePusher').and.returnValue(new Promise(res => { + service['initialisePusher'] = jasmine.createSpy('initialisePusher').and.callFake(() => { const thisPusher = new PusherLib(); service['pusher'] = thisPusher; - // spyOn(service['pusher'], 'connect').and.returnValue(true); - res(thisPusher); - })); + return thisPusher; + }); service['pusher'] = undefined; - requestSpy.get.and.returnValue(of(notificationRes)); - apolloSpy.graphQLFetch.and.returnValue(of(pusherChatChannelRes)); + apolloSpy.graphQLFetch.and.callFake((query: string) => { + return query.includes('notificationChannel') + ? of(notificationRes) + : of(pusherChatChannelRes); + }); }); it('should initialise pusher', fakeAsync(() => { @@ -274,10 +385,310 @@ describe('PusherService', async () => { expect(service.unsubscribeChannels).toHaveBeenCalled(); })); + + it('should reuse the client while reconnecting and replacing channels when the experience scope changes', fakeAsync(() => { + const connection = { state: 'connected' }; + const oldPusher = jasmine.createSpyObj('oldPusher', [ + 'disconnect', + 'connect', + 'allChannels', + 'unsubscribe', + ], { + connection, + config: { auth: { headers: {} } }, + }); + oldPusher.disconnect.and.callFake(() => connection.state = 'disconnected'); + service['pusher'] = oldPusher; + service['activeScope'] = { programId: 1, projectId: 11, timelineId: 1 }; + storageSpy.getUser = jasmine.createSpy('getUser').and.returnValue({ + apikey: 'new-key', + programId: 2, + projectId: 22, + timelineId: 2, + }); + spyOn(service, 'unsubscribeChannels'); + service['initialisePusher'] = jasmine.createSpy('initialisePusher'); + spyOn(service, 'getChannels').and.returnValue(Promise.resolve()); + + service.initialise(); + flushMicrotasks(); + + expect(service.unsubscribeChannels).toHaveBeenCalled(); + expect(oldPusher.disconnect).toHaveBeenCalled(); + expect(oldPusher.connect).toHaveBeenCalled(); + expect(service['initialisePusher']).not.toHaveBeenCalled(); + expect(service['pusher']).toBe(oldPusher); + expect(oldPusher.config.auth.headers.apikey).toBe('new-key'); + expect(oldPusher.config.auth.headers.timelineid).toBe(2); + })); + + it('should update credentials without replacing the same-scope connection', fakeAsync(() => { + const currentPusher = jasmine.createSpyObj('currentPusher', [ + 'disconnect', + 'connect', + 'allChannels', + ], { + connection: { state: 'connected' }, + config: { auth: { headers: {} } }, + }); + service['pusher'] = currentPusher; + service['activeScope'] = { programId: null, projectId: null, timelineId: 1 }; + storageSpy.getUser = jasmine.createSpy('getUser').and.returnValue({ + apikey: 'rotated-key', + timelineId: 1, + }); + spyOn(service, 'unsubscribeChannels'); + service['initialisePusher'] = jasmine.createSpy('initialisePusher'); + spyOn(service, 'getChannels').and.returnValue(Promise.resolve()); + + service.initialise(); + flushMicrotasks(); + + expect(service.unsubscribeChannels).not.toHaveBeenCalled(); + expect(currentPusher.disconnect).not.toHaveBeenCalled(); + expect(service['initialisePusher']).not.toHaveBeenCalled(); + expect(currentPusher.config.auth.headers.apikey).toBe('rotated-key'); + })); + + it('should share one in-flight initialisation between concurrent callers', fakeAsync(() => { + let resolveChannels: () => void; + const channels = new Promise(resolve => resolveChannels = resolve); + const getChannelsSpy = spyOn(service, 'getChannels').and.returnValue(channels); + + const first = service.initialise(); + const second = service.initialise(); + flushMicrotasks(); + + expect(service['initialisePusher']).toHaveBeenCalledTimes(1); + expect(getChannelsSpy).toHaveBeenCalledTimes(1); + + resolveChannels(); + flushMicrotasks(); + expectAsync(Promise.all([first, second])).toBeResolved(); + })); + }); + + describe('unsubscribeChannels()', () => { + it('should unsubscribe chat channels when no notification channel exists', () => { + const chatSubscription = jasmine.createSpyObj('chatSubscription', ['unbind_all']); + const pusher = jasmine.createSpyObj('pusher', ['unsubscribe']); + service['pusher'] = pusher; + service['channels'] = { + notification: null, + chat: [{ name: 'chat-channel', subscription: chatSubscription }], + }; + + service.unsubscribeChannels(); + + expect(chatSubscription.unbind_all).toHaveBeenCalled(); + expect(pusher.unsubscribe).toHaveBeenCalledWith('chat-channel'); + expect(service['channels']).toEqual({ notification: null, chat: [] }); + }); + + it('should reset scope, retry state, auth headers, and the connection on logout', () => { + const notificationSubscription = jasmine.createSpyObj('notificationSubscription', ['unbind_all']); + const pusher = jasmine.createSpyObj('pusher', ['disconnect', 'unsubscribe'], { + config: { auth: { headers: { apikey: 'old-key', timelineid: 1 } } }, + connection: { state: 'connected' }, + }); + service['pusher'] = pusher; + service['activeScope'] = { programId: 1, projectId: 11, timelineId: 1 }; + service['channels'].notification = { + name: 'notification-channel', + subscription: notificationSubscription, + }; + service['retryAttempted'].notification = true; + service['pendingRetryTypes'].add('notification'); + service['retryTimer'] = setTimeout(() => undefined, 1000); + + service.reset(); + + expect(pusher.disconnect).toHaveBeenCalled(); + expect(pusher.unsubscribe).toHaveBeenCalledWith('notification-channel'); + expect(service['activeScope']).toBeNull(); + expect(service['retryAttempted']).toEqual({ notification: false, chat: false }); + expect(service['pendingRetryTypes'].size).toBe(0); + expect(service['retryTimer']).toBeNull(); + expect(pusher.config.auth.headers.apikey).toBe(''); + expect(pusher.config.auth.headers.timelineid).toBe(''); + }); + + it('should prevent an in-flight initialisation from restarting after reset', fakeAsync(() => { + let resolveChannels: () => void; + const channels = new Promise(resolve => resolveChannels = resolve); + spyOn(service, 'getChannels').and.returnValue(channels); + const pusher = jasmine.createSpyObj('pusher', [ + 'disconnect', + 'connect', + 'unsubscribe', + ], { + config: { auth: { headers: {} } }, + connection: { state: 'connected' }, + }); + const initialisePusherSpy = spyOn(service, 'initialisePusher').and.returnValue(pusher); + + service.initialise(); + flushMicrotasks(); + service.reset(); + resolveChannels(); + flushMicrotasks(); + + expect(initialisePusherSpy).toHaveBeenCalledTimes(1); + expect(service['activeScope']).toBeNull(); + })); + }); + + describe('subscription lifecycle', () => { + it('should retry a failed channel type only once', fakeAsync(() => { + const scope = { programId: 1, projectId: 11, timelineId: 1 }; + const pusher = jasmine.createSpyObj('pusher', ['disconnect', 'connect'], { + config: { auth: { headers: {} } }, + connection: { state: 'connected' }, + }); + service['pusher'] = pusher; + service['activeScope'] = scope; + storageSpy.getUser.and.returnValue({ + apikey: 'apikey', + ...scope, + } as any); + const refreshSpy = spyOn(service, 'refreshChatChannel').and.returnValue(Promise.resolve()); + + service['scheduleSubscriptionRetry']('chat', scope); + service['scheduleSubscriptionRetry']('chat', scope); + tick(1000); + flushMicrotasks(); + + expect(refreshSpy).toHaveBeenCalledTimes(1); + expect(pusher.disconnect).toHaveBeenCalledTimes(1); + expect(pusher.connect).toHaveBeenCalledTimes(1); + })); + + it('should batch notification and chat retries into one reconnect', fakeAsync(() => { + const scope = { programId: 1, projectId: 11, timelineId: 1 }; + const pusher = jasmine.createSpyObj('pusher', ['disconnect', 'connect'], { + config: { auth: { headers: {} } }, + connection: { state: 'connected' }, + }); + service['pusher'] = pusher; + service['activeScope'] = scope; + storageSpy.getUser.and.returnValue({ apikey: 'apikey', ...scope } as any); + const notificationRefresh = spyOn(service, 'refreshNotificationChannel') + .and.returnValue(Promise.resolve()); + const chatRefresh = spyOn(service, 'refreshChatChannel') + .and.returnValue(Promise.resolve()); + + service['scheduleSubscriptionRetry']('notification', scope); + service['scheduleSubscriptionRetry']('chat', scope); + tick(1000); + flushMicrotasks(); + + expect(pusher.disconnect).toHaveBeenCalledTimes(1); + expect(pusher.connect).toHaveBeenCalledTimes(1); + expect(notificationRefresh).toHaveBeenCalledTimes(1); + expect(chatRefresh).toHaveBeenCalledTimes(1); + })); + + it('should not cancel a pending retry during same-scope discovery', fakeAsync(() => { + const scope = { programId: 1, projectId: 11, timelineId: 1 }; + const pusher = jasmine.createSpyObj('pusher', ['disconnect', 'connect'], { + config: { auth: { headers: {} } }, + connection: { state: 'connected' }, + }); + service['pusher'] = pusher; + service['activeScope'] = scope; + storageSpy.getUser.and.returnValue({ apikey: 'apikey', ...scope } as any); + service['channels'].chat = [{ + name: 'chat-channel', + subscription: { subscriptionPending: true } as any, + }]; + apolloSpy.graphQLFetch.and.returnValue(of({ + data: { channels: [{ pusherChannel: 'chat-channel' }] }, + } as any)); + + service['scheduleSubscriptionRetry']('chat', scope); + service.refreshChatChannels(); + flushMicrotasks(); + + expect(service['retryTimer']).not.toBeNull(); + tick(1000); + flushMicrotasks(); + expect(pusher.disconnect).toHaveBeenCalledTimes(1); + expect(pusher.connect).toHaveBeenCalledTimes(1); + })); + + it('should disconnect before removing a pending Pusher channel', () => { + const connection = { state: 'connected' }; + const subscription: any = { + subscriptionPending: true, + unbind_all: jasmine.createSpy('unbind_all'), + }; + const pusher = jasmine.createSpyObj('pusher', [ + 'disconnect', + 'connect', + 'unsubscribe', + ], { + config: { auth: { headers: {} } }, + connection, + }); + pusher.disconnect.and.callFake(() => { + connection.state = 'disconnected'; + subscription.subscriptionPending = false; + }); + service['pusher'] = pusher; + service['channels'].notification = { + name: 'obsolete-notification-channel', + subscription, + }; + service['retryAttempted'].notification = true; + service['pendingRetryTypes'].add('notification'); + service['retryTimer'] = setTimeout(() => undefined, 1000); + + service['reconcileNotificationChannel'](null); + + expect(pusher.disconnect).toHaveBeenCalledBefore(pusher.unsubscribe); + expect(pusher.unsubscribe).toHaveBeenCalledWith('obsolete-notification-channel'); + expect(pusher.connect).toHaveBeenCalled(); + expect(service['channels'].notification).toBeNull(); + expect(service['retryAttempted'].notification).toBeFalse(); + expect(service['retryTimer']).toBeNull(); + }); + + it('should discard a queued notification callback from an old scope', () => { + const callbacks: Record void> = {}; + const subscription: any = { + name: 'notification-channel', + subscribed: false, + bind: jasmine.createSpy('bind').and.callFake((event, callback) => { + callbacks[event] = callback; + return subscription; + }), + unbind_all: jasmine.createSpy('unbind_all'), + }; + const pusher = jasmine.createSpyObj('pusher', ['subscribe', 'unsubscribe'], { + config: { auth: { headers: {} } }, + connection: { state: 'connected' }, + }); + pusher.subscribe.and.returnValue(subscription); + const originalScope = { programId: 1, projectId: 11, timelineId: 1 }; + service['pusher'] = pusher; + service['activeScope'] = originalScope; + storageSpy.getUser.and.returnValue({ apikey: 'apikey', ...originalScope } as any); + + service.subscribeChannel('notification', 'notification-channel'); + storageSpy.getUser.and.returnValue({ + apikey: 'new-key', + programId: 2, + projectId: 22, + timelineId: 2, + } as any); + callbacks.notification({ type: 'assessment_review_assigned' }); + + expect(utilSpy.broadcastEvent).not.toHaveBeenCalled(); + }); }); describe('initialisePusher()', () => { - it('should skip initiation if storage is empty apikey or timelineid', fakeAsync(() => { + it('should skip initiation if storage is empty apikey or timelineid', () => { service['pusher'] = undefined; storageSpy.getUser = jasmine.createSpy('getUser').and.returnValue({ @@ -285,26 +696,17 @@ describe('PusherService', async () => { timelineId: null, }); - let result; - service['initialisePusher']().then(res => { - result = res; - }); - - flushMicrotasks(); + const result = service['initialisePusher'](); expect(result).toEqual(service['pusher']); - })); + }); - it('should return instantiated pusher is there is existing one', fakeAsync(() => { + it('should create a Pusher client for an authenticated context', () => { const instantiatedpusher = new PusherLib(); - service['pusher'] = instantiatedpusher; - let result; - service['initialisePusher']().then(res => { - result = res; - }); - flushMicrotasks(); + service['initialisePusher'] = jasmine.createSpy().and.returnValue(instantiatedpusher); + const result = service['initialisePusher'](); expect(typeof result).toEqual(typeof instantiatedpusher); - })); + }); }); describe('disconnect()', () => { @@ -369,7 +771,7 @@ describe('PusherService', async () => { describe('normaliseTemplateValue() (private, tested via resolveUseTLS)', () => { it('returns empty string for unsubstituted template placeholders like ', () => { // normaliseTemplateValue is called by resolveUseTLS when pusherUseTLS is a placeholder - const originalUseTLS = environment.pusherUseTLS; + const originalUseTLS = (environment as any).pusherUseTLS; (environment as any).pusherUseTLS = ''; // resolveUseTLS will normalise the placeholder to '' and default to TLS=true const useTLS: boolean = service['resolveUseTLS'](); @@ -378,7 +780,7 @@ describe('PusherService', async () => { }); it('returns false when pusherUseTLS is explicitly set to "false"', () => { - const originalUseTLS = environment.pusherUseTLS; + const originalUseTLS = (environment as any).pusherUseTLS; (environment as any).pusherUseTLS = 'false'; const useTLS: boolean = service['resolveUseTLS'](); expect(useTLS).toBe(false); @@ -386,7 +788,7 @@ describe('PusherService', async () => { }); it('returns true when pusherUseTLS is set to "true"', () => { - const originalUseTLS = environment.pusherUseTLS; + const originalUseTLS = (environment as any).pusherUseTLS; (environment as any).pusherUseTLS = 'true'; const useTLS: boolean = service['resolveUseTLS'](); expect(useTLS).toBe(true); @@ -396,7 +798,7 @@ describe('PusherService', async () => { describe('resolvePusherPort() (private)', () => { it('returns undefined when pusherPort is not configured', () => { - const originalPort = environment.pusherPort; + const originalPort = (environment as any).pusherPort; (environment as any).pusherPort = ''; const port = service['resolvePusherPort'](true); expect(port).toBeUndefined(); @@ -404,7 +806,7 @@ describe('PusherService', async () => { }); it('returns undefined when pusherPort is an unsubstituted template', () => { - const originalPort = environment.pusherPort; + const originalPort = (environment as any).pusherPort; (environment as any).pusherPort = ''; const port = service['resolvePusherPort'](true); expect(port).toBeUndefined(); @@ -412,7 +814,7 @@ describe('PusherService', async () => { }); it('returns the numeric port when pusherPort is a valid number string', () => { - const originalPort = environment.pusherPort; + const originalPort = (environment as any).pusherPort; (environment as any).pusherPort = '6001'; const port = service['resolvePusherPort'](true); expect(port).toBe(6001); @@ -420,7 +822,7 @@ describe('PusherService', async () => { }); it('returns undefined when pusherPort is not a valid integer', () => { - const originalPort = environment.pusherPort; + const originalPort = (environment as any).pusherPort; (environment as any).pusherPort = 'not-a-number'; const port = service['resolvePusherPort'](true); expect(port).toBeUndefined(); @@ -428,4 +830,3 @@ describe('PusherService', async () => { }); }); }); - diff --git a/projects/v3/src/app/services/pusher.service.ts b/projects/v3/src/app/services/pusher.service.ts index b8335b02f5..c4d44449a5 100644 --- a/projects/v3/src/app/services/pusher.service.ts +++ b/projects/v3/src/app/services/pusher.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@angular/core'; -import { Observable, of } from 'rxjs'; -import { map, tap } from 'rxjs/operators'; +import { firstValueFrom, Observable, of } from 'rxjs'; +import { tap } from 'rxjs/operators'; import { environment } from '@v3/environments/environment'; import { UtilsService } from '@v3/services/utils.service'; import { BrowserStorageService } from '@v3/services/storage.service'; @@ -34,11 +34,22 @@ export interface DeleteMessageParam { uuid: string; } +type PusherChannelType = 'notification' | 'chat'; +type PusherSubscription = Channel & { subscriptionPending?: boolean }; + class PusherChannel { name: string; - subscription?: Channel; + subscription?: PusherSubscription; +} + +interface RealtimeScope { + programId: number | null; + projectId: number | null; + timelineId: number | null; } +const SUBSCRIPTION_RETRY_DELAY = 1000; + @Injectable({ providedIn: 'root', }) @@ -46,6 +57,18 @@ export class PusherService { private pusherKey: string; private apiurl: string; private pusher: PusherInstance; + private activeScope: RealtimeScope | null = null; + private initialisePromise: Promise | null = null; + private lifecycleGeneration = 0; + private notificationGeneration = 0; + private chatGeneration = 0; + private retryAttempted: Record = { + notification: false, + chat: false, + }; + private retryTimer: ReturnType | null = null; + private retryScope: RealtimeScope | null = null; + private pendingRetryTypes = new Set(); private channels: { notification: PusherChannel; chat: PusherChannel[]; @@ -63,39 +86,81 @@ export class PusherService { this.apiurl = environment.graphQL; } - // initialise + subscribe to channels at one go - async initialise(options?: { - unsubscribe?: boolean; - }) { + /** + * Initialise the application-scoped Pusher client and reconcile channels for + * the latest experience. Concurrent callers share the same in-flight work; + * if storage changes while that work is running, the latest scope is loaded + * before callers are released. + */ + async initialise(options?: { unsubscribe?: boolean }): Promise { if (environment.demo) { - return ; + return; } - // make sure pusher is connected - if (!this.pusher) { - this.pusher = await this.initialisePusher(); + const lifecycleGeneration = this.lifecycleGeneration; + while ( + lifecycleGeneration === this.lifecycleGeneration + && this.hasRealtimeScope(this.getCurrentScope()) + ) { + if (!this.initialisePromise) { + const requestedScope = this.getCurrentScope(); + const operation = this.performInitialise(requestedScope, options); + this.initialisePromise = operation; + + try { + await operation; + } finally { + if (this.initialisePromise === operation) { + this.initialisePromise = null; + } + } + } else { + await this.initialisePromise; + } + + // Logout/reset invalidates callers that were waiting for channel + // discovery. A later authenticated caller will start a fresh lifecycle. + if (lifecycleGeneration !== this.lifecycleGeneration) { + return; + } + + if (this.isCurrentScope(this.activeScope)) { + return; + } + } + } + + private async performInitialise( + scope: RealtimeScope, + options?: { unsubscribe?: boolean } + ): Promise { + const scopeChanged = !this.areScopesEqual(scope, this.activeScope); + + if (scopeChanged) { + this.invalidateChannelRefreshes(); + this.clearRetryState(); + this.disconnect(); + this.unsubscribeChannels(); + this.activeScope = scope; + } else if (options?.unsubscribe) { + this.disconnect(); + this.unsubscribeChannels(); } if (!this.pusher) { - return {}; + this.pusher = this.initialisePusher(); } - if (options && options.unsubscribe) { - this.unsubscribeChannels(); + if (!this.pusher) { + return; } - // handling condition at re-login without rebuilding pusher (where isInstantiated() is false) - if (this.pusher.connection.state !== 'connected') { - // reconnect pusher + this.syncAuthHeaders(); + if (this.pusher.connection.state === 'disconnected') { this.pusher.connect(); } - // subscribe to event only when pusher is available - const channels = this.getChannels(); - return { - pusher: this.pusher, - channels - }; + await this.getChannels(scope); } disconnect(): void { @@ -138,7 +203,7 @@ export class PusherService { } private resolveUseTLS(): boolean { - const raw = this.normaliseTemplateValue(environment.pusherUseTLS); + const raw = this.normaliseTemplateValue((environment as any).pusherUseTLS); if (!raw) { return true; } @@ -146,7 +211,7 @@ export class PusherService { } private resolvePusherPort(_useTLS: boolean): number | undefined { - const raw = this.normaliseTemplateValue(environment.pusherPort); + const raw = this.normaliseTemplateValue((environment as any).pusherPort); if (!raw) { return undefined; } @@ -157,18 +222,10 @@ export class PusherService { return parsed; } - private async initialisePusher(): Promise { - // during the app execution lifecycle - // never reinstantiate another instance of Pusher - const pusherHasInitiated = typeof this.pusher !== 'undefined' || !this.utils.isEmpty(this.pusher); - if (pusherHasInitiated) { - return this.pusher; - } - - // prevent pusher auth before user authenticated (skip silently) + private initialisePusher(): PusherInstance { const { apikey, timelineId } = this.storage.getUser(); - if ((!apikey || !timelineId) && !pusherHasInitiated) { - return this.pusher; + if (!apikey || !timelineId) { + return undefined; } try { @@ -190,7 +247,7 @@ export class PusherService { // If a custom host (e.g. self-hosted Soketi) is configured, use that; // otherwise fall back to Pusher Cloud's cluster-based routing. - const host = this.normaliseTemplateValue(environment.pusherHost); + const host = this.normaliseTemplateValue((environment as any).pusherHost); if (host) { config.wsHost = host; const port = this.resolvePusherPort(useTLS); @@ -206,26 +263,17 @@ export class PusherService { } else if (environment.pusherCluster) { config.cluster = environment.pusherCluster; } - const newPusherInstance = new Pusher(this.pusherKey, config) - .bind('pusher:connection_established', () => { + const newPusherInstance = new Pusher(this.pusherKey, config); + newPusherInstance.connection + .bind('connecting', () => this.syncAuthHeaders()) + .bind('state_change', state => { // eslint-disable-next-line no-console - console.log('pusher:connection_established'); + console.log('pusher:state_change', state); }) - .bind('pusher:connection_disconnected', () => { - // eslint-disable-next-line no-console - console.log('pusher:connection_disconnected'); - }) - .bind('pusher:connection_failed', () => { - // eslint-disable-next-line no-console - console.log('pusher:connection_failed'); - }) - .bind('pusher:error', (err) => { - // eslint-disable-next-line no-console - console.log('pusher:error', err); - }); + .bind('error', err => console.error('pusher:error', err)); return newPusherInstance; } catch (err) { - throw new Error(err); + throw new Error('Unable to initialise Pusher', { cause: err }); } } @@ -235,37 +283,63 @@ export class PusherService { * false: haven't subscribed */ isSubscribed(channelName: string): boolean { - return this.pusher.allChannels().some((channel: Channel) => channel.name === channelName && channel.subscribed); + return this.pusher?.allChannels().some( + (channel: Channel) => channel.name === channelName && channel.subscribed + ) || false; } /** - * get a list of channels from API request and subscribe every of them into - * connected + authorised pusher + * Refresh both experience-scoped channel types. Each request is independent: + * a transient failure preserves the same scope's last valid listener set, + * while a scope change has already removed every previous listener. */ - async getChannels() { - await this.getNotificationChannel().toPromise(); - await this.getChatChannels().toPromise(); + async getChannels(scope = this.getCurrentScope()): Promise { + await Promise.all([ + this.refreshNotificationChannel(scope), + this.refreshChatChannel(scope), + ]); + } + + async refreshChatChannels(): Promise { + const scope = this.getCurrentScope(); + if (!this.pusher || !this.isCurrentScope(scope)) { + await this.initialise(); + return; + } + await this.refreshChatChannel(scope); } - getNotificationChannel(): Observable { + getNotificationChannel( + scope = this.getCurrentScope(), + generation = ++this.notificationGeneration + ): Observable { const { apikey } = this.storage.getUser(); if (!apikey) { - return of(); + return of(undefined); } return this.apolloService.graphQLFetch( `query notificationChannel($env: String!) { notificationChannel(env: $env) }`, { variables: { env: environment.env } } - ).pipe(map(response => { - const channel = response?.data?.notificationChannel; - if (channel) { - this.subscribeChannel('notification', channel); + ).pipe(tap(response => { + if (!this.isCurrentScope(scope) || generation !== this.notificationGeneration) { + return; + } + + const channelName = response?.data?.notificationChannel; + if (channelName !== null && typeof channelName !== 'string') { + throw new Error('Pusher notification channel format error'); } + + this.reconcileNotificationChannel(channelName || null); })); } - getChatChannels(): Observable { + getChatChannels( + scope = this.getCurrentScope(), + generation = ++this.chatGeneration + ): Observable { return this.apolloService.graphQLFetch( `query getPusherChannels { channels { @@ -273,37 +347,190 @@ export class PusherService { } }` ).pipe(tap(response => { - if (response.data && response.data.channels) { - const result = JSON.parse(JSON.stringify(response.data.channels)); - result.forEach(element => { - this.subscribeChannel('chat', element.pusherChannel); - }); + if (!this.isCurrentScope(scope) || generation !== this.chatGeneration) { + return; } + + if (!Array.isArray(response?.data?.channels)) { + throw new Error('Pusher chat channel array format error'); + } + + const channelNames = response.data.channels + .map(element => element?.pusherChannel) + .filter(channelName => !!channelName); + this.reconcileChatChannels(channelNames); })); } + private async refreshNotificationChannel(scope: RealtimeScope): Promise { + const generation = ++this.notificationGeneration; + try { + await firstValueFrom(this.getNotificationChannel(scope, generation)); + } catch (err) { + console.error('Failed to refresh Pusher notification channel', err); + } + } + + private async refreshChatChannel(scope: RealtimeScope): Promise { + const generation = ++this.chatGeneration; + try { + await firstValueFrom(this.getChatChannels(scope, generation)); + } catch (err) { + console.error('Failed to refresh Pusher chat channels', err); + } + } + /** * unsubscribe all channels * (use case: after switching program) */ unsubscribeChannels(): void { + this.unsubscribeNotificationChannel(); + [...this.channels.chat].forEach(chat => this.unsubscribeChatChannel(chat)); + this.channels.chat = []; + } + + reset(): void { + this.lifecycleGeneration++; + this.invalidateChannelRefreshes(); + this.clearRetryState(); + this.disconnect(); + this.unsubscribeChannels(); + this.activeScope = null; + this.clearAuthHeaders(); + } + + private unsubscribeNotificationChannel(): void { if (!this.channels.notification) { - return ; - } - this.channels.notification.subscription.unbind_all(); - // handle issue logout at first load of program-switching view - if (this.pusher) { - this.pusher.unbind_all(); - this.pusher.unsubscribe(this.channels.notification.name); + return; } - this.channels.chat.forEach(chat => { - chat.subscription.unbind_all(); - if (this.pusher) { - this.pusher.unsubscribe(chat.name); - } - }); + + this.channels.notification.subscription?.unbind_all(); + this.pusher?.unsubscribe(this.channels.notification.name); this.channels.notification = null; - this.channels.chat = []; + } + + private unsubscribeChatChannel(channel: PusherChannel): void { + channel.subscription?.unbind_all(); + this.pusher?.unsubscribe(channel.name); + } + + private getCurrentScope(): RealtimeScope { + const { programId, projectId, timelineId } = this.storage.getUser(); + return { + programId: programId ?? null, + projectId: projectId ?? null, + timelineId: timelineId ?? null, + }; + } + + private hasRealtimeScope(scope: RealtimeScope): boolean { + const { apikey } = this.storage.getUser(); + return !!apikey && !!scope.timelineId; + } + + private isCurrentScope(scope: RealtimeScope | null): boolean { + return !!scope && this.areScopesEqual(scope, this.getCurrentScope()); + } + + private areScopesEqual(left: RealtimeScope | null, right: RealtimeScope | null): boolean { + return !!left && !!right + && left.programId === right.programId + && left.projectId === right.projectId + && left.timelineId === right.timelineId; + } + + private invalidateChannelRefreshes(): void { + this.notificationGeneration++; + this.chatGeneration++; + } + + private syncAuthHeaders(): void { + if (!this.pusher) { + return; + } + const { apikey, timelineId } = this.storage.getUser(); + this.pusher.config.auth = this.pusher.config.auth || {}; + this.pusher.config.auth.headers = { + ...(this.pusher.config.auth.headers || {}), + 'Authorization': 'pusherKey=' + this.pusherKey, + 'appkey': environment.appkey, + 'apikey': apikey, + 'timelineid': timelineId, + }; + } + + private clearAuthHeaders(): void { + if (!this.pusher?.config?.auth?.headers) { + return; + } + this.pusher.config.auth.headers.apikey = ''; + this.pusher.config.auth.headers.timelineid = ''; + } + + private reconcileNotificationChannel(channelName: string | null): void { + if (this.channels.notification?.name === channelName) { + return; + } + + // A changed exact set supersedes any retry for the previous channel. + // A failure on the replacement channel starts its own bounded retry. + this.resetRetry('notification'); + const reconnect = this.disconnectForPendingChannelRemoval( + this.channels.notification ? [this.channels.notification] : [] + ); + this.unsubscribeNotificationChannel(); + if (channelName) { + this.subscribeChannel('notification', channelName); + } + this.reconnectAfterPendingChannelRemoval(reconnect); + } + + private reconcileChatChannels(channelNames: string[]): void { + const desiredNames = [...new Set(channelNames)]; + const removedChannels = this.channels.chat + .filter(channel => !desiredNames.includes(channel.name)); + const removesPendingChannel = removedChannels.some( + channel => channel.subscription?.subscriptionPending + ); + if (desiredNames.length === 0 || removesPendingChannel) { + this.resetRetry('chat'); + } + const reconnect = this.disconnectForPendingChannelRemoval(removedChannels); + + removedChannels.forEach(channel => this.unsubscribeChatChannel(channel)); + this.channels.chat = this.channels.chat.filter(channel => desiredNames.includes(channel.name)); + + desiredNames.forEach(channelName => this.subscribeChannel('chat', channelName)); + this.reconnectAfterPendingChannelRemoval(reconnect); + } + + /** + * The Pusher client leaves a channel pending after an authorization + * error. Calling unsubscribe in that state only marks it cancelled and keeps + * it in Pusher's registry, where a later reconnect can revive it. Disconnect + * first so the client resets the pending flag and unsubscribe removes it exactly. + */ + private disconnectForPendingChannelRemoval(channels: PusherChannel[]): boolean { + const hasPendingChannel = channels.some( + channel => channel.subscription?.subscriptionPending + ); + const shouldReconnect = hasPendingChannel + && !!this.pusher + && this.pusher.connection.state !== 'disconnected'; + + if (shouldReconnect) { + this.disconnect(); + } + return shouldReconnect; + } + + private reconnectAfterPendingChannelRemoval(reconnect: boolean): void { + if (!reconnect || !this.pusher || this.pusher.connection.state !== 'disconnected') { + return; + } + this.syncAuthHeaders(); + this.pusher.connect(); } /** @@ -311,77 +538,163 @@ export class PusherService { * @param type The type of Pusher channel (notification/chat) * @param channelName The name of the Pusher channel */ - subscribeChannel(type: string, channelName: string) { - if (environment.demo) { + subscribeChannel(type: PusherChannelType, channelName: string): void | false { + if (environment.demo || !this.pusher) { return; } - if (!channelName) { return false; } - if (this.isSubscribed(channelName)) { + + if (type === 'notification' && this.channels.notification?.name === channelName) { return; } + if (type === 'chat' && this.channels.chat.some(channel => channel.name === channelName)) { + return; + } + + const scope = this.activeScope ? { ...this.activeScope } : null; + if (!scope || !this.isCurrentScope(scope)) { + return; + } + + this.syncAuthHeaders(); + const channel: PusherChannel = { + name: channelName, + subscription: this.pusher.subscribe(channelName), + }; - switch (type) { - case 'notification': - // unsubscribe previous channel - if (this.channels.notification) { - this.channels.notification.subscription.unbind_all(); + channel.subscription + .bind('pusher:subscription_succeeded', () => { + if (!this.isCurrentScope(scope)) { + return; + } + if (type === 'notification' || this.channels.chat.every(item => item.subscription?.subscribed)) { + this.resetRetry(type); } - this.channels.notification = { - name: channelName, - subscription: this.pusher.subscribe(channelName) - }; - this.channels.notification.subscription - .bind('notification', data => { + }) + .bind('pusher:subscription_error', data => { + this.handleSubscriptionError(type, channel, scope, data); + }); + + if (type === 'notification') { + this.unsubscribeNotificationChannel(); + this.channels.notification = channel; + channel.subscription + .bind('notification', data => { + if (this.isCurrentScope(scope)) { this.utils.broadcastEvent('notification', data); - }) - .bind('achievement', data => { + } + }) + .bind('achievement', data => { + if (this.isCurrentScope(scope)) { this.utils.broadcastEvent('achievement', data); - }) - .bind('event-reminder', data => { + } + }) + .bind('event-reminder', data => { + if (this.isCurrentScope(scope)) { this.utils.broadcastEvent('event-reminder', data); - }) - // .bind('pusher:subscription_succeeded', data => {}) - .bind('pusher:subscription_error', data => { - console.error(`fail to subscribe ${channelName}::`, data); - }); - break; - - case 'chat': - // don't need to subscribe again if already subscribed - if (this.channels.chat.some(c => c.name === channelName)) { - return; + } + }); + return; + } + + channel.subscription + .bind('client-chat-new-message', data => { + if (this.isCurrentScope(scope)) { + this.utils.broadcastEvent('chat:new-message', data); } - const channel = { - name: channelName, - subscription: this.pusher.subscribe(channelName) - }; - channel.subscription - .bind('client-chat-new-message', data => { - this.utils.broadcastEvent('chat:new-message', data); - }) - .bind('client-chat-delete-message', data => { - this.utils.broadcastEvent('chat:delete-message', data); - }) - .bind('client-chat-edit-message', data => { - this.utils.broadcastEvent('chat:edit-message', data); - }) - .bind('client-typing-event', data => { - this.utils.broadcastEvent('typing-' + channelName, data); - }) - // .bind('pusher:subscription_succeeded', data => {}) - .bind('pusher:subscription_error', data => { - // error handling - console.error(`fail to subscribe ${channelName}::`, data); - }); - if (!this.channels.chat) { - this.channels.chat = []; + }) + .bind('client-chat-delete-message', data => { + if (this.isCurrentScope(scope)) { + this.utils.broadcastEvent('chat:delete-message', data); } - this.channels.chat.push(channel); - break; + }) + .bind('client-chat-edit-message', data => { + if (this.isCurrentScope(scope)) { + this.utils.broadcastEvent('chat:edit-message', data); + } + }) + .bind('client-typing-event', data => { + if (this.isCurrentScope(scope)) { + this.utils.broadcastEvent('typing-' + channelName, data); + } + }); + this.channels.chat.push(channel); + } + + private handleSubscriptionError( + type: PusherChannelType, + channel: PusherChannel, + scope: RealtimeScope, + error: any + ): void { + if (!this.isCurrentScope(scope)) { + return; } + console.error(`Failed to subscribe Pusher ${type} channel ${channel.name}`, error); + this.scheduleSubscriptionRetry(type, scope); + } + + private scheduleSubscriptionRetry(type: PusherChannelType, scope: RealtimeScope): void { + if (this.retryAttempted[type]) { + console.error(`Pusher ${type} channel retry already attempted`); + return; + } + + this.retryAttempted[type] = true; + this.pendingRetryTypes.add(type); + this.retryScope = { ...scope }; + + // Notification and chat authorization commonly fail together. Batch them + // into one socket reconnect while retaining independent retry limits. + if (this.retryTimer) { + return; + } + + this.retryTimer = setTimeout(async () => { + this.retryTimer = null; + const retryScope = this.retryScope; + const retryTypes = [...this.pendingRetryTypes]; + this.retryScope = null; + this.pendingRetryTypes.clear(); + + if (!retryScope || !this.isCurrentScope(retryScope)) { + return; + } + this.syncAuthHeaders(); + this.disconnect(); + this.pusher?.connect(); + await Promise.all(retryTypes.map(retryType => { + return retryType === 'notification' + ? this.refreshNotificationChannel(retryScope) + : this.refreshChatChannel(retryScope); + })); + }, SUBSCRIPTION_RETRY_DELAY); + } + + private resetRetry(type: PusherChannelType): void { + this.retryAttempted[type] = false; + this.pendingRetryTypes.delete(type); + if (this.pendingRetryTypes.size === 0) { + this.clearRetryTimer(); + this.retryScope = null; + } + } + + private clearRetryTimer(): void { + if (this.retryTimer) { + clearTimeout(this.retryTimer); + this.retryTimer = null; + } + } + + private clearRetryState(): void { + this.clearRetryTimer(); + this.retryScope = null; + this.pendingRetryTypes.clear(); + this.retryAttempted.notification = false; + this.retryAttempted.chat = false; } /** diff --git a/projects/v3/src/app/services/shared.service.spec.ts b/projects/v3/src/app/services/shared.service.spec.ts index 547e8c5031..d7b0532030 100644 --- a/projects/v3/src/app/services/shared.service.spec.ts +++ b/projects/v3/src/app/services/shared.service.spec.ts @@ -59,7 +59,11 @@ describe('SharedService', () => { }, { provide: ApolloService, - useValue: jasmine.createSpyObj('ApolloService', ['graphQLFetch', 'graphQLWatch']), + useValue: jasmine.createSpyObj('ApolloService', [ + 'graphQLFetch', + 'graphQLWatch', + 'initiateCoreClient', + ]), }, { provide: RequestService, @@ -181,4 +185,23 @@ describe('SharedService', () => { expect(utilsSpy.changeCardBackgroundImage).toHaveBeenCalled(); }); }); + + describe('initWebServices()', () => { + it('should initialise Apollo before awaiting Pusher', async () => { + const order: string[] = []; + const pusherSpy = pusherServiceSpy as jasmine.SpyObj; + apolloSpy.initiateCoreClient.and.callFake(() => { + order.push('apollo'); + return null; + }); + pusherSpy.initialise.and.callFake(async () => { + order.push('pusher'); + }); + + await service.initWebServices(); + + expect(order).toEqual(['apollo', 'pusher']); + expect(utilsSpy.checkIsPracteraSupportEmail).toHaveBeenCalled(); + }); + }); }); diff --git a/projects/v3/src/app/services/shared.service.ts b/projects/v3/src/app/services/shared.service.ts index d0eaecde78..df58b27045 100644 --- a/projects/v3/src/app/services/shared.service.ts +++ b/projects/v3/src/app/services/shared.service.ts @@ -224,9 +224,9 @@ export class SharedService { * Initialise web services like Pusher/ apollo if there stack info in storage */ async initWebServices(): Promise { - await this.pusherService.initialise(); this.apolloService.initiateCoreClient(); this.utils.checkIsPracteraSupportEmail(); + await this.pusherService.initialise(); } /**