fix: AdminOrderWatcherService reacts to auth state, not component lifecycle
Previous fix (1032891) stopped polling via AdminLayoutComponent's
DestroyRef, but logout() never navigates or destroys the component -
the watcher kept polling and toasting indefinitely after logout.
Now polling starts/stops off AdminAuthService.isAuthenticated() directly,
following the same effect() pattern already used by AdminDashboardFacade.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,10 +1,12 @@
|
|||||||
import { TestBed, fakeAsync, tick } from '@angular/core/testing';
|
import { TestBed, fakeAsync, tick } from '@angular/core/testing';
|
||||||
|
import { signal } from '@angular/core';
|
||||||
import { provideRouter } from '@angular/router';
|
import { provideRouter } from '@angular/router';
|
||||||
import { of } from 'rxjs';
|
import { of } from 'rxjs';
|
||||||
import { AdminOrderWatcherService } from './admin-order-watcher.service';
|
import { AdminOrderWatcherService } from './admin-order-watcher.service';
|
||||||
import { AdminOrdersLocalGateway } from '../../orders/services/admin-orders-local.gateway';
|
import { AdminOrdersLocalGateway } from '../../orders/services/admin-orders-local.gateway';
|
||||||
import { AdminOrder, AdminOrdersListResult } from '../../orders/models/admin-order.model';
|
import { AdminOrder, AdminOrdersListResult } from '../../orders/models/admin-order.model';
|
||||||
import { UserNotificationService } from '../../../website/user-experience/services/user-notification.service';
|
import { UserNotificationService } from '../../../website/user-experience/services/user-notification.service';
|
||||||
|
import { AdminAuthService } from '../../../../core/admin-auth/admin-auth.service';
|
||||||
|
|
||||||
function makeOrder(id: string, orderNumber: string, createdAt: string): AdminOrder {
|
function makeOrder(id: string, orderNumber: string, createdAt: string): AdminOrder {
|
||||||
return {
|
return {
|
||||||
@@ -31,6 +33,7 @@ describe('AdminOrderWatcherService', () => {
|
|||||||
let pollIndex: number;
|
let pollIndex: number;
|
||||||
let notifications: UserNotificationService;
|
let notifications: UserNotificationService;
|
||||||
let service: AdminOrderWatcherService;
|
let service: AdminOrderWatcherService;
|
||||||
|
let fakeAdminAuth: { isAuthenticated: ReturnType<typeof signal<boolean>> };
|
||||||
|
|
||||||
function fakeGateway() {
|
function fakeGateway() {
|
||||||
return {
|
return {
|
||||||
@@ -53,10 +56,16 @@ describe('AdminOrderWatcherService', () => {
|
|||||||
localStorage.removeItem('adminOrderWatcher.lastAcknowledgedOrderId.v1');
|
localStorage.removeItem('adminOrderWatcher.lastAcknowledgedOrderId.v1');
|
||||||
localStorage.removeItem('adminOrderWatcher.pollIntervalMs.v1');
|
localStorage.removeItem('adminOrderWatcher.pollIntervalMs.v1');
|
||||||
|
|
||||||
|
// Defaults to unauthenticated so the reactive effect stays a no-op and the
|
||||||
|
// pre-existing tests below (which drive polling via explicit start()/stop()
|
||||||
|
// calls) keep their original behavior unchanged.
|
||||||
|
fakeAdminAuth = { isAuthenticated: signal(false) };
|
||||||
|
|
||||||
TestBed.configureTestingModule({
|
TestBed.configureTestingModule({
|
||||||
providers: [
|
providers: [
|
||||||
provideRouter([]),
|
provideRouter([]),
|
||||||
{ provide: AdminOrdersLocalGateway, useValue: fakeGateway() as unknown as AdminOrdersLocalGateway },
|
{ provide: AdminOrdersLocalGateway, useValue: fakeGateway() as unknown as AdminOrdersLocalGateway },
|
||||||
|
{ provide: AdminAuthService, useValue: fakeAdminAuth as unknown as AdminAuthService },
|
||||||
],
|
],
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -162,4 +171,37 @@ describe('AdminOrderWatcherService', () => {
|
|||||||
|
|
||||||
expect(loadOrdersSpy.calls.count()).toBe(callsAfterStart);
|
expect(loadOrdersSpy.calls.count()).toBe(callsAfterStart);
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
it('reacts to AdminAuthService.isAuthenticated: starts, stops on logout, and resumes on re-login', fakeAsync(() => {
|
||||||
|
const gateway = TestBed.inject(AdminOrdersLocalGateway);
|
||||||
|
const loadOrdersSpy = spyOn(gateway, 'loadOrders').and.callThrough();
|
||||||
|
|
||||||
|
// Starts authenticated -> the effect should start polling on its own,
|
||||||
|
// with no explicit service.start() call from a component.
|
||||||
|
fakeAdminAuth.isAuthenticated.set(true);
|
||||||
|
TestBed.flushEffects();
|
||||||
|
tick(0);
|
||||||
|
|
||||||
|
expect(loadOrdersSpy.calls.count()).toBeGreaterThan(0);
|
||||||
|
const callsWhileAuthenticated = loadOrdersSpy.calls.count();
|
||||||
|
|
||||||
|
// Logout: flip the signal to false. Polling must actually stop - this is
|
||||||
|
// the gap the previous DestroyRef-based fix failed to close, since nothing
|
||||||
|
// destroys AdminLayoutComponent on logout.
|
||||||
|
fakeAdminAuth.isAuthenticated.set(false);
|
||||||
|
TestBed.flushEffects();
|
||||||
|
tick(0);
|
||||||
|
const callsRightAfterLogout = loadOrdersSpy.calls.count();
|
||||||
|
|
||||||
|
tick(service.intervalMs() * 3);
|
||||||
|
expect(loadOrdersSpy.calls.count()).toBe(callsRightAfterLogout);
|
||||||
|
|
||||||
|
// Re-login: polling should resume, not stay permanently stopped.
|
||||||
|
fakeAdminAuth.isAuthenticated.set(true);
|
||||||
|
TestBed.flushEffects();
|
||||||
|
tick(0);
|
||||||
|
tick(service.intervalMs());
|
||||||
|
|
||||||
|
expect(loadOrdersSpy.calls.count()).toBeGreaterThan(callsWhileAuthenticated);
|
||||||
|
}));
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,10 +1,11 @@
|
|||||||
import { Injectable, Signal, computed, inject, signal } from '@angular/core';
|
import { Injectable, Signal, computed, effect, inject, signal } from '@angular/core';
|
||||||
import { AdminOrder } from '../../orders/models/admin-order.model';
|
import { AdminOrder } from '../../orders/models/admin-order.model';
|
||||||
import { AdminOrdersLocalGateway } from '../../orders/services/admin-orders-local.gateway';
|
import { AdminOrdersLocalGateway } from '../../orders/services/admin-orders-local.gateway';
|
||||||
import { LocalStorageService } from '../../../../core/storage/local-storage.service';
|
import { LocalStorageService } from '../../../../core/storage/local-storage.service';
|
||||||
import { UserNotificationService } from '../../../website/user-experience/services/user-notification.service';
|
import { UserNotificationService } from '../../../website/user-experience/services/user-notification.service';
|
||||||
import { LanguageService } from '../../../../services/language.service';
|
import { LanguageService } from '../../../../services/language.service';
|
||||||
import { TranslateService } from '../../../../i18n/translate.service';
|
import { TranslateService } from '../../../../i18n/translate.service';
|
||||||
|
import { AdminAuthService } from '../../../../core/admin-auth/admin-auth.service';
|
||||||
|
|
||||||
const LAST_NOTIFIED_KEY = 'adminOrderWatcher.lastNotifiedOrderId.v1';
|
const LAST_NOTIFIED_KEY = 'adminOrderWatcher.lastNotifiedOrderId.v1';
|
||||||
const LAST_NOTIFIED_AT_KEY = 'adminOrderWatcher.lastNotifiedOrderCreatedAt.v1';
|
const LAST_NOTIFIED_AT_KEY = 'adminOrderWatcher.lastNotifiedOrderCreatedAt.v1';
|
||||||
@@ -22,6 +23,7 @@ export class AdminOrderWatcherService {
|
|||||||
private readonly notifications = inject(UserNotificationService);
|
private readonly notifications = inject(UserNotificationService);
|
||||||
private readonly languageService = inject(LanguageService);
|
private readonly languageService = inject(LanguageService);
|
||||||
private readonly i18n = inject(TranslateService);
|
private readonly i18n = inject(TranslateService);
|
||||||
|
private readonly adminAuth = inject(AdminAuthService);
|
||||||
|
|
||||||
private readonly recentOrdersSignal = signal<AdminOrder[]>([]);
|
private readonly recentOrdersSignal = signal<AdminOrder[]>([]);
|
||||||
readonly recentOrders: Signal<AdminOrder[]> = this.recentOrdersSignal.asReadonly();
|
readonly recentOrders: Signal<AdminOrder[]> = this.recentOrdersSignal.asReadonly();
|
||||||
@@ -49,6 +51,20 @@ export class AdminOrderWatcherService {
|
|||||||
private timerId: ReturnType<typeof setInterval> | null = null;
|
private timerId: ReturnType<typeof setInterval> | null = null;
|
||||||
private started = false;
|
private started = false;
|
||||||
|
|
||||||
|
constructor() {
|
||||||
|
// Reacts to the auth signal itself rather than component lifecycle: nothing
|
||||||
|
// reliably destroys AdminLayoutComponent on logout (no navigation happens),
|
||||||
|
// so polling must stop/start off isAuthenticated directly to avoid leaking
|
||||||
|
// admin order toasts onto the public storefront after logout.
|
||||||
|
effect(() => {
|
||||||
|
if (this.adminAuth.isAuthenticated()) {
|
||||||
|
this.start();
|
||||||
|
} else {
|
||||||
|
this.stop();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
start(): void {
|
start(): void {
|
||||||
if (this.started) {
|
if (this.started) {
|
||||||
return;
|
return;
|
||||||
|
|||||||
Reference in New Issue
Block a user