Show a proper 404 / error screen on detail pages instead of an infinite spinner

packages/frontend/jest.config.js

@@ -11,6 +11,8 @@ module.exports = {
     '^zone.js/testing$': '<rootDir>/../../node_modules/zone.js/bundles/zone-testing.umd.js',
     // Keep other path mappings from tsconfig
     '^upgrade_types(.*)$': '<rootDir>/../../../types$1',
+    '^@shared-component-lib$': '<rootDir>/src/app/shared-standalone-component-lib/components',
+    '^@shared-component-lib/(.*)$': '<rootDir>/src/app/shared-standalone-component-lib/components/$1',
   },
 
   // Updated transform configuration
@@ -34,7 +36,8 @@ module.exports = {
     '<rootDir>/dist/',
     '<rootDir>/e2e/',
     '<rootDir>/src/environments/',
-    '<rootDir>/src/app/features/dashboard/',
+    // Dashboard specs are excluded except the routing spec, which guards subtle redirect behavior
+    '<rootDir>/src/app/features/dashboard/(?!dashboard-routing\\.spec)',
     '<rootDir>/src/app/shared/',
   ],
 

packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.spec.ts

@@ -1,4 +1,5 @@
-import { HttpClient, HttpParams } from '@angular/common/http';
+import { HttpClient, HttpContext, HttpParams } from '@angular/common/http';
+import { HANDLES_404_CONTEXTUALLY } from '../http-interceptors/http-context-tokens';
 import { of } from 'rxjs';
 import {
   ASSIGNMENT_ALGORITHM,
@@ -240,13 +241,26 @@ describe('ExperimentDataService', () => {
   });
 
   describe('#getExperimentById', () => {
-    it('should get the getExperimentById http observable', () => {
+    it('should get the getExperimentById http observable with contextual 404 handling when requested', () => {
+      const experimentId = mockExperimentId;
+      const expectedUrl = `${API_ENDPOINTS.getExperimentById}/${experimentId}`;
+
+      service.getExperimentById(experimentId, true);
+
+      expect(mockHttpClient.get).toHaveBeenCalledWith(expectedUrl, { context: expect.any(HttpContext) });
+      const context: HttpContext = (mockHttpClient.get as jest.Mock).mock.calls[0][1].context;
+      expect(context.get(HANDLES_404_CONTEXTUALLY)).toBe(true);
+    });
+
+    it('should keep the generic 404 notification by default (e.g. for preview-user callers)', () => {
       const experimentId = mockExperimentId;
       const expectedUrl = `${API_ENDPOINTS.getExperimentById}/${experimentId}`;
 
       service.getExperimentById(experimentId);
 
-      expect(mockHttpClient.get).toHaveBeenCalledWith(expectedUrl);
+      expect(mockHttpClient.get).toHaveBeenCalledWith(expectedUrl, { context: expect.any(HttpContext) });
+      const context: HttpContext = (mockHttpClient.get as jest.Mock).mock.calls[0][1].context;
+      expect(context.get(HANDLES_404_CONTEXTUALLY)).toBe(false);
     });
   });
 

packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts

@@ -10,7 +10,8 @@ import {
   ExperimentSegmentListResponse,
   UpdateExperimentConditionsRequest,
 } from './store/experiments.model';
-import { HttpClient, HttpParams } from '@angular/common/http';
+import { HttpClient, HttpContext, HttpParams } from '@angular/common/http';
+import { HANDLES_404_CONTEXTUALLY } from '../http-interceptors/http-context-tokens';
 import { API_ENDPOINTS } from '../api-endpoints.constants';
 import { Observable } from 'rxjs';
 import { ExperimentSegmentListRequest, SegmentFile } from '../segments/store/segments.model';
@@ -82,9 +83,11 @@ export class ExperimentDataService {
     return this.http.delete(url);
   }
 
-  getExperimentById(experimentId: string) {
+  getExperimentById(experimentId: string, handles404Contextually = false) {
     const url = `${API_ENDPOINTS.getExperimentById}/${experimentId}`;
-    return this.http.get(url);
+    // Details-page callers render their own not-found state, so they skip the generic 404
+    // notification; other callers (e.g. preview-user) keep it
+    return this.http.get(url, { context: new HttpContext().set(HANDLES_404_CONTEXTUALLY, handles404Contextually) });
   }
 
   fetchAllPartitions() {

packages/frontend/projects/upgrade/src/app/core/experiments/experiments.service.spec.ts

@@ -347,14 +347,28 @@ describe('ExperimentService', () => {
   });
 
   describe('#fetchExperimentById', () => {
-    it('should dispatch actionGetExperimentById with the given input', () => {
+    it('should dispatch actionGetExperimentById without contextual 404 handling by default', () => {
       const experimentId = 'abc123';
 
       service.fetchExperimentById(experimentId);
 
       expect(mockStore.dispatch).toHaveBeenCalledWith(
         actionGetExperimentById({
           experimentId,
+          handles404Contextually: false,
+        })
+      );
+    });
+
+    it('should dispatch actionGetExperimentById with contextual 404 handling when requested', () => {
+      const experimentId = 'abc123';
+
+      service.fetchExperimentById(experimentId, true);
+
+      expect(mockStore.dispatch).toHaveBeenCalledWith(
+        actionGetExperimentById({
+          experimentId,
+          handles404Contextually: true,
         })
       );
     });

packages/frontend/projects/upgrade/src/app/core/experiments/experiments.service.ts

@@ -25,6 +25,7 @@ import {
   selectIsLoadingExperiment,
   selectIsLoadingUpsertPrivateSegmentList,
   selectSelectedExperiment,
+  selectExperimentDetailsPageError,
   selectExperimentOverviewDetails,
   selectSearchExperimentParams,
   selectRootTableState,
@@ -74,6 +75,7 @@ export class ExperimentService {
   isLoadingExperiment$ = this.store$.pipe(select(selectIsLoadingExperiment));
   isLoadingUpsertPrivateSegmentList$ = this.store$.pipe(select(selectIsLoadingUpsertPrivateSegmentList));
   selectedExperiment$ = this.store$.pipe(select(selectSelectedExperiment));
+  experimentDetailsPageError$ = this.store$.pipe(select(selectExperimentDetailsPageError));
   selectedExperimentOverviewDetails$ = this.store$.pipe(select(selectExperimentOverviewDetails));
   searchParams$ = this.store$.pipe(select(selectSearchExperimentParams));
   selectRootTableState$ = this.store$.pipe(select(selectRootTableState));
@@ -153,14 +155,15 @@ export class ExperimentService {
     );
   }
 
-  fetchExperimentById(experimentId: string) {
-    this.store$.dispatch(experimentAction.actionGetExperimentById({ experimentId }));
+  fetchExperimentById(experimentId: string, handles404Contextually = false) {
+    this.store$.dispatch(experimentAction.actionGetExperimentById({ experimentId, handles404Contextually }));
   }
 
   refetchCurrentSelectedExperiment() {
     this.selectedExperiment$.pipe(take(1)).subscribe((experiment) => {
       if (experiment) {
-        this.fetchExperimentById(experiment.id);
+        // selectedExperiment$ only resolves on the details page, which renders its own error state
+        this.fetchExperimentById(experiment.id, true);
       }
     });
   }

packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.actions.ts

@@ -1,4 +1,5 @@
 import { createAction, props } from '@ngrx/store';
+import { PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model';
 import {
   Experiment,
   UpsertExperimentType,
@@ -48,15 +49,21 @@ export const actionRemoveExperimentStat = createAction(
 
 export const actionGetExperimentById = createAction(
   '[Experiment] Get Experiment By Id',
-  props<{ experimentId: string }>()
+  // handles404Contextually: set by details-page callers that render their own not-found state,
+  // so the generic 404 notification is suppressed for them only (e.g. preview-user keeps it)
+  props<{ experimentId: string; handles404Contextually?: boolean }>()
 );
 
 export const actionGetExperimentByIdSuccess = createAction(
   '[Experiment] Get Experiment By Id Success',
   props<{ experiment: Experiment }>()
 );
 
-export const actionGetExperimentByIdFailure = createAction('[Experiment] Get Experiment By Id Failure');
+export const actionGetExperimentByIdFailure = createAction(
+  '[Experiment] Get Experiment By Id Failure',
+  // Only failures of details-page fetches (handles404Contextually) may write detailsPageError
+  props<{ experimentId: string; errorType: PAGE_ERROR_TYPE; handles404Contextually?: boolean }>()
+);
 
 export const actionUpsertExperiment = createAction(
   '[Experiment] Upsert Experiment',

packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.effects.spec.ts

@@ -1,7 +1,7 @@
 import { fakeAsync, tick } from '@angular/core/testing';
 import { ActionsSubject } from '@ngrx/store';
-import { BehaviorSubject, of, throwError } from 'rxjs';
-import { last, pairwise, scan, take } from 'rxjs/operators';
+import { BehaviorSubject, of, throwError, timer } from 'rxjs';
+import { delay, last, mergeMap, pairwise, scan, take } from 'rxjs/operators';
 import {
   actionDeleteExperimentSuccess,
   actionFetchAllExperimentNames,
@@ -22,6 +22,7 @@ import {
   actionDeleteExperimentFailure,
   actionGetExperimentById,
   actionGetExperimentByIdSuccess,
+  actionGetExperimentByIdFailure,
   actionFetchExperimentDetailStatSuccess,
   actionFetchExperimentDetailStat,
   actionGetExperimentsSuccess,
@@ -67,6 +68,7 @@ import { actionExecuteQuery, actionFetchMetrics } from '../../analysis/store/ana
 import { selectCurrentUser } from '../../auth/store/auth.selectors';
 import { UserRole } from '../../users/store/users.model';
 import { Environment } from '../../../../environments/environment-types';
+import { PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model';
 
 describe('ExperimentEffects', () => {
   let service: ExperimentEffects;
@@ -596,7 +598,8 @@ describe('ExperimentEffects', () => {
   });
 
   describe('#getExperimentById$', () => {
-    const experimentId = 'testId';
+    // Must be a canonical (lowercase) UUID - the effect short-circuits non-canonical ids to a not-found failure
+    const experimentId = '11111111-2222-4333-8444-555555555555';
     const experiment = {
       id: 'test1',
       stat: [
@@ -642,6 +645,130 @@ describe('ExperimentEffects', () => {
 
       actions$.next(actionGetExperimentById({ experimentId }));
     }));
+
+    it('should dispatch a not-found failure without calling the API when the id is not a canonical UUID', fakeAsync(() => {
+      experimentDataService.getExperimentById = jest.fn();
+      Selectors.selectExperimentStats.setResult({});
+      let result: any;
+      service.getExperimentById$.subscribe((action: any) => (result = action));
+
+      actions$.next(actionGetExperimentById({ experimentId: 'not-a-uuid' }));
+      tick(0);
+
+      expect(result).toEqual(
+        actionGetExperimentByIdFailure({ experimentId: 'not-a-uuid', errorType: PAGE_ERROR_TYPE.NOT_FOUND })
+      );
+      expect(experimentDataService.getExperimentById).not.toHaveBeenCalled();
+    }));
+
+    it('should dispatch a not-found failure when the fetch fails with 404', fakeAsync(() => {
+      experimentDataService.getExperimentById = jest.fn().mockReturnValue(throwError(() => ({ status: 404 })));
+      Selectors.selectExperimentStats.setResult({});
+      let result: any;
+      service.getExperimentById$.subscribe((action: any) => (result = action));
+
+      actions$.next(actionGetExperimentById({ experimentId }));
+      tick(0);
+
+      expect(result).toEqual(actionGetExperimentByIdFailure({ experimentId, errorType: PAGE_ERROR_TYPE.NOT_FOUND }));
+    }));
+
+    it('should dispatch a load-failed failure when the fetch fails with an unexpected error', fakeAsync(() => {
+      experimentDataService.getExperimentById = jest.fn().mockReturnValue(throwError(() => ({ status: 500 })));
+      Selectors.selectExperimentStats.setResult({});
+      let result: any;
+      service.getExperimentById$.subscribe((action: any) => (result = action));
+
+      actions$.next(actionGetExperimentById({ experimentId }));
+      tick(0);
+
+      expect(result).toEqual(actionGetExperimentByIdFailure({ experimentId, errorType: PAGE_ERROR_TYPE.LOAD_FAILED }));
+    }));
+
+    it('should still dispatch success without stats when only the stats call fails', fakeAsync(() => {
+      experimentDataService.getExperimentById = jest.fn().mockReturnValue(of(experiment));
+      experimentDataService.getAllExperimentsStats = jest.fn().mockReturnValue(throwError(() => ({ status: 500 })));
+      Selectors.selectExperimentStats.setResult({});
+      let result: any;
+      service.getExperimentById$.subscribe((action: any) => (result = action));
+
+      actions$.next(actionGetExperimentById({ experimentId }));
+      tick(0);
+
+      expect(result).toEqual(actionGetExperimentByIdSuccess({ experiment }));
+    }));
+
+    it('should cancel a stale request when a newer fetch for the same id is dispatched', fakeAsync(() => {
+      // The first (stale) request fails slowly; the second succeeds immediately.
+      // The stale failure must not surface after the newer request has succeeded.
+      experimentDataService.getExperimentById = jest
+        .fn()
+        .mockReturnValueOnce(timer(100).pipe(mergeMap(() => throwError(() => ({ status: 500 })))))
+        .mockReturnValueOnce(of(experiment));
+      experimentDataService.getAllExperimentsStats = jest.fn().mockReturnValue(of(stats));
+      Selectors.selectExperimentStats.setResult({});
+      const results: any[] = [];
+      service.getExperimentById$.subscribe((action: any) => results.push(action));
+
+      actions$.next(actionGetExperimentById({ experimentId }));
+      actions$.next(actionGetExperimentById({ experimentId }));
+      tick(200);
+
+      expect(results).toContainEqual(actionGetExperimentByIdSuccess({ experiment }));
+      expect(results).not.toContainEqual(
+        actionGetExperimentByIdFailure({ experimentId, errorType: PAGE_ERROR_TYPE.LOAD_FAILED })
+      );
+    }));
+
+    it('should cancel a previous details-page fetch when a newer details-page fetch for another id starts', fakeAsync(() => {
+      // Simulates navigating from details page A to details page B: A's slow failure must not
+      // surface, or it would overwrite B's detailsPageError and bring the spinner back on B.
+      const otherExperimentId = '22222222-3333-4333-8444-555555555555';
+      const otherExperiment = { ...experiment, id: 'test2' } as any;
+      experimentDataService.getExperimentById = jest
+        .fn()
+        .mockReturnValueOnce(timer(100).pipe(mergeMap(() => throwError(() => ({ status: 500 })))))
+        .mockReturnValueOnce(of(otherExperiment));
+      experimentDataService.getAllExperimentsStats = jest.fn().mockReturnValue(of(stats));
+      Selectors.selectExperimentStats.setResult({});
+      const results: any[] = [];
+      service.getExperimentById$.subscribe((action: any) => results.push(action));
+
+      actions$.next(actionGetExperimentById({ experimentId, handles404Contextually: true }));
+      actions$.next(actionGetExperimentById({ experimentId: otherExperimentId, handles404Contextually: true }));
+      tick(200);
+
+      expect(results).toContainEqual(actionGetExperimentByIdSuccess({ experiment: otherExperiment }));
+      expect(results).not.toContainEqual(
+        actionGetExperimentByIdFailure({
+          experimentId,
+          errorType: PAGE_ERROR_TYPE.LOAD_FAILED,
+          handles404Contextually: true,
+        })
+      );
+    }));
+
+    it('should keep concurrent background fetches for different experiment ids', fakeAsync(() => {
+      // preview-user loads several different experiments at once (without contextual 404 handling) -
+      // one fetch must not cancel another
+      const otherExperimentId = '22222222-3333-4333-8444-555555555555';
+      const otherExperiment = { ...experiment, id: 'test2' } as any;
+      experimentDataService.getExperimentById = jest
+        .fn()
+        .mockReturnValueOnce(of(experiment).pipe(delay(50)))
+        .mockReturnValueOnce(of(otherExperiment));
+      experimentDataService.getAllExperimentsStats = jest.fn().mockReturnValue(of(stats));
+      Selectors.selectExperimentStats.setResult({});
+      const results: any[] = [];
+      service.getExperimentById$.subscribe((action: any) => results.push(action));
+
+      actions$.next(actionGetExperimentById({ experimentId }));
+      actions$.next(actionGetExperimentById({ experimentId: otherExperimentId }));
+      tick(100);
+
+      expect(results).toContainEqual(actionGetExperimentByIdSuccess({ experiment }));
+      expect(results).toContainEqual(actionGetExperimentByIdSuccess({ experiment: otherExperiment }));
+    }));
   });
 
   describe('#getExperimentDetailStat', () => {