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

[zackcl]

Resolves#3129

Opening a detail page (experiment, feature flag, or segment) with an ID that can't be loaded currently leaves the user on an endless spinner. This PR replaces that with a proper error screen on all three pages.

Case	What the user sees now
Entity doesn't exist — wrong link, malformed ID, or deleted by someone else	"Experiment not found" + Back to Experiments button (same for flags/segments)
Unexpected failure (5xx, network error)	"Something went wrong" + Try Again and Back buttons
Detail URL without an ID (/home/detail/)	Redirects to the list page

Changes

• Added a shared CommonPageErrorComponent (not-found / load-failed variants) to the standalone component lib, plus i18n strings. All three detail pages use it.
• Added detailsPageError to the experiments / feature-flags / segments stores: the fetch-by-id failure actions now carry the failed ID and error type, new route-scoped selectors expose it, and the experiments effect got its missing catchError — the root cause of the endless spinner.
• The three detail-page content components render the error screen, and it takes priority over cached list data, so an entity deleted elsewhere shows "not found" instead of a stale details page. Once a route-scoped error is established, it remains visible during retries until a fetch for the same entity succeeds, so cached editable content and stale breadcrumb names do not reappear.
• Non-canonical IDs show not-found without firing a request. The check (isCanonicalEntityId) is the lowercase-only subset of the backend's @IsUUID() rule: the backend rejects malformed IDs with 400 (not 404), and the app only generates lowercase URLs.
• The "Network call failed" toast is skipped for 404s on the three details-page fetches only (opt-in HttpContextToken set in the data services) — the page communicates the error itself now; preview-user and all other non-details requests keep their existing toast behavior.
• Experiment detail fetches cancel an older contextual detail request when the route changes or the user retries, while non-details requests for different experiment IDs remain concurrent. Only contextual failures can write detailsPageError, so preview-user and other background failures cannot overwrite the details page's error state.
• Added focused tests for each fetch effect's malformed / 404 / 5xx paths (plus the experiment stats-only-failure case), the route-scoped error selectors, the interceptor's scoped 404 suppression, and the canonical-ID check.
• Fallout: updated the affected specs, added the new state field to the local-storage initial states, and added the missing @shared-component-lib alias to jest.config.js.
• Added explicit ID-less detail redirects, plus requireRouteParam for Angular's trailing-empty-segment case.
• Added regression tests for the shared error component, accessibility announcement, retry-time error retention, request cancellation/error ownership, data-service HttpContext opt-in, and slash/no-slash routing behavior.

Screenshots

[Copilot]

Pull request overview

Adds explicit not-found and load-failure states to experiment, feature-flag, and segment detail pages.

Changes:

• Introduces a reusable localized page-error component.
• Tracks route-specific loading errors in NgRx stores and supports retries.
• Adds ID validation, ID-less route handling, and scoped 404-toast suppression.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/frontend/projects/upgrade/src/assets/i18n/en.json	Adds error-page translations.
packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/index.ts	Exports the error component.
.../common-page-error/common-page-error.model.ts	Defines error models and ID validation.
.../common-page-error/common-page-error.model.spec.ts	Tests ID validation.
.../common-page-error/common-page-error.component.ts	Implements shared error UI logic.
.../common-page-error/common-page-error.component.scss	Styles the error screen.
.../common-page-error/common-page-error.component.html	Renders error actions and messaging.
.../segment-details-page-content.component.ts	Connects segment errors and retry.
.../segment-details-page-content.component.html	Prioritizes segment error rendering.
.../require-route-param.guard.ts	Redirects ID-less detail routes.
.../feature-flag-details-page-content.component.ts	Connects flag errors and retry.
.../feature-flag-details-page-content.component.html	Prioritizes flag error rendering.
.../experiment-details-page-content.component.ts	Connects experiment errors and retry.
.../experiment-details-page-content.component.html	Prioritizes experiment error rendering.
.../dashboard-routing.module.ts	Applies detail-route parameter guards.
.../segments/store/segments.selectors.ts	Selects route-scoped segment errors.
.../segments/store/segments.selectors.spec.ts	Tests segment error selection.
.../segments/store/segments.reducer.ts	Stores segment detail failures.
.../segments/store/segments.reducer.spec.ts	Tests segment error transitions.
.../segments/store/segments.model.ts	Extends segment state.
.../segments/store/segments.effects.ts	Classifies segment fetch failures.
.../segments/store/segments.effects.spec.ts	Tests segment failure branches.
.../segments/store/segments.actions.ts	Adds segment failure metadata.
.../segments/segments.service.ts	Exposes segment detail errors.
.../segments/segments.data.service.ts	Enables contextual 404 handling.
.../local-storage/local-storage.service.ts	Initializes new error state.
.../local-storage/local-storage.service.spec.ts	Updates persisted-state expectations.
.../http-interceptors/http-error.interceptor.ts	Suppresses opted-in 404 notifications.
.../http-interceptors/http-error.interceptor.spec.ts	Tests notification suppression.
.../http-interceptors/http-context-tokens.ts	Defines the 404 context token.
.../feature-flags/store/feature-flags.selectors.ts	Selects route-scoped flag errors.
.../feature-flags/store/feature-flags.selectors.spec.ts	Tests flag error selection.
.../feature-flags/store/feature-flags.reducer.ts	Stores flag detail failures.
.../feature-flags/store/feature-flags.model.ts	Extends feature-flag state.
.../feature-flags/store/feature-flags.effects.ts	Handles malformed, empty, and failed fetches.
.../feature-flags/store/feature-flags.effects.spec.ts	Tests flag fetch branches.
.../feature-flags/store/feature-flags.actions.ts	Adds flag failure metadata.
.../feature-flags/feature-flags.service.ts	Exposes flag detail errors.
.../feature-flags/feature-flags.data.service.ts	Enables contextual 404 handling.
.../experiments/store/experiments.selectors.ts	Selects route-scoped experiment errors.
.../experiments/store/experiments.selector.spec.ts	Tests experiment error selection.
.../experiments/store/experiments.reducer.ts	Stores experiment detail failures.
.../experiments/store/experiments.reducer.spec.ts	Tests experiment error transitions.
.../experiments/store/experiments.model.ts	Extends experiment state.
.../experiments/store/experiments.effects.ts	Handles fetch and auxiliary-stat failures.
.../experiments/store/experiments.effects.spec.ts	Tests experiment fetch branches.
.../experiments/store/experiments.actions.ts	Adds experiment failure metadata.
.../experiments/experiments.service.ts	Exposes experiment detail errors.
.../experiments/experiments.data.service.ts	Enables contextual 404 handling.
.../experiments/experiments.data.service.spec.ts	Tests the request context token.
packages/frontend/jest.config.js	Maps shared-component aliases in Jest.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:18

• This guard only runs after segments/detail/:segmentId matches. /segments/detail has no parameter segment, so it bypasses the guard and reaches the global ** redirect to /home instead of the Segments list. Add an explicit ID-less route before the parameterized route; keep this guard for the trailing-empty-segment case.
  packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:91
• This guard cannot handle /featureflags/detail because that URL does not match a route requiring :flagId. It falls through to the global wildcard and redirects to /home, contrary to the intended Feature Flags list destination. Add an explicit ID-less redirect before this parameterized route.
  packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts:89
• getExperimentById is also used by ExperimentService.selectExperimentById for the Preview User editor (preview-user.component.ts:156), where no page-level 404 UI exists. Setting this token unconditionally suppresses the only 404 feedback in that flow. Pass contextual handling from the details-page request (or split the details fetch) so non-details callers retain the generic notification.

 return this.http.get(url, { context: new HttpContext().set(HANDLES_404_CONTEXTUALLY, true) });

packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-page-error/common-page-error.component.html:1

• This error state replaces the spinner asynchronously, but the new content has no live-region semantics or focus change, so screen-reader users are not notified that loading finished with an error. Mark the inserted error container as an alert (or implement equivalent focus management) so the title and recovery actions are announced.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• detailsPageError is a single value, but this effect intentionally keeps requests for different IDs alive. After navigating from experiment A to B, if B fails and then A's older request fails, A overwrites B's error; the route-scoped selector hides A's error and B returns to the spinner this PR is meant to remove. Preserve concurrent preview-user requests without letting stale detail requests share this slot—for example, cancel any prior contextual details fetch when a newer contextual fetch starts, and ensure non-contextual/background failures do not write details-page error state (or key errors by entity ID).

 takeUntil(
this.actions$.pipe(
ofType(experimentAction.actionGetExperimentById),
filter((newerAction) => newerAction !== action && newerAction.experimentId === experimentId)
)

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[danoswaltCL]

seems we might as well make the GET flags return a 404 instead of a 204 like the other ones, I don't think there should be a reason it's different.

[danoswaltCL]

@zackcl the PR to update feature-flags GET call to 404 has been merged into dev

[zackcl]

@bcb37@danoswaltCL Addressed the latest feedback: consolidated the duplicate error translations, clarified the not-found message with the current environment, and aligned feature flag handling with the backend’s 404 response. Feel free to take another look!

[zackcl]

Resolves#3129

Opening a detail page (experiment, feature flag, or segment) with an ID that can't be loaded currently leaves the user on an endless spinner. This PR replaces that with a proper error screen on all three pages.

Case	What the user sees now
Entity doesn't exist — wrong link, malformed ID, or deleted by someone else	"Experiment not found" + Back to Experiments button (same for flags/segments)
Unexpected failure (5xx, network error)	"Something went wrong" + Try Again and Back buttons
Detail URL without an ID (/home/detail/)	Redirects to the list page

Changes

• Added a shared CommonPageErrorComponent (not-found / load-failed variants) to the standalone component lib, plus i18n strings. All three detail pages use it.
• Added detailsPageError to the experiments / feature-flags / segments stores: the fetch-by-id failure actions now carry the failed ID and error type, new route-scoped selectors expose it, and the experiments effect got its missing catchError — the root cause of the endless spinner.
• The three detail-page content components render the error screen, and it takes priority over cached list data, so an entity deleted elsewhere shows "not found" instead of a stale details page. Once a route-scoped error is established, it remains visible during retries until a fetch for the same entity succeeds, so cached editable content and stale breadcrumb names do not reappear.
• Non-canonical IDs show not-found without firing a request. The check (isCanonicalEntityId) is the lowercase-only subset of the backend's @IsUUID() rule: the backend rejects malformed IDs with 400 (not 404), and the app only generates lowercase URLs.
• The "Network call failed" toast is skipped for 404s on the three details-page fetches only (opt-in HttpContextToken set in the data services) — the page communicates the error itself now; preview-user and all other non-details requests keep their existing toast behavior.
• Experiment detail fetches cancel an older contextual detail request when the route changes or the user retries, while non-details requests for different experiment IDs remain concurrent. Only contextual failures can write detailsPageError, so preview-user and other background failures cannot overwrite the details page's error state.
• Added focused tests for each fetch effect's malformed / 404 / 5xx paths (plus the experiment stats-only-failure case), the route-scoped error selectors, the interceptor's scoped 404 suppression, and the canonical-ID check.
• Fallout: updated the affected specs, added the new state field to the local-storage initial states, and added the missing @shared-component-lib alias to jest.config.js.
• Added explicit ID-less detail redirects, plus requireRouteParam for Angular's trailing-empty-segment case.
• Added regression tests for the shared error component, accessibility announcement, retry-time error retention, request cancellation/error ownership, data-service HttpContext opt-in, and slash/no-slash routing behavior.

Screenshots

[Copilot]

Pull request overview

Adds explicit not-found and load-failure states to experiment, feature-flag, and segment detail pages.

Changes:

• Introduces a reusable localized page-error component.
• Tracks route-specific loading errors in NgRx stores and supports retries.
• Adds ID validation, ID-less route handling, and scoped 404-toast suppression.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/frontend/projects/upgrade/src/assets/i18n/en.json	Adds error-page translations.
packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/index.ts	Exports the error component.
.../common-page-error/common-page-error.model.ts	Defines error models and ID validation.
.../common-page-error/common-page-error.model.spec.ts	Tests ID validation.
.../common-page-error/common-page-error.component.ts	Implements shared error UI logic.
.../common-page-error/common-page-error.component.scss	Styles the error screen.
.../common-page-error/common-page-error.component.html	Renders error actions and messaging.
.../segment-details-page-content.component.ts	Connects segment errors and retry.
.../segment-details-page-content.component.html	Prioritizes segment error rendering.
.../require-route-param.guard.ts	Redirects ID-less detail routes.
.../feature-flag-details-page-content.component.ts	Connects flag errors and retry.
.../feature-flag-details-page-content.component.html	Prioritizes flag error rendering.
.../experiment-details-page-content.component.ts	Connects experiment errors and retry.
.../experiment-details-page-content.component.html	Prioritizes experiment error rendering.
.../dashboard-routing.module.ts	Applies detail-route parameter guards.
.../segments/store/segments.selectors.ts	Selects route-scoped segment errors.
.../segments/store/segments.selectors.spec.ts	Tests segment error selection.
.../segments/store/segments.reducer.ts	Stores segment detail failures.
.../segments/store/segments.reducer.spec.ts	Tests segment error transitions.
.../segments/store/segments.model.ts	Extends segment state.
.../segments/store/segments.effects.ts	Classifies segment fetch failures.
.../segments/store/segments.effects.spec.ts	Tests segment failure branches.
.../segments/store/segments.actions.ts	Adds segment failure metadata.
.../segments/segments.service.ts	Exposes segment detail errors.
.../segments/segments.data.service.ts	Enables contextual 404 handling.
.../local-storage/local-storage.service.ts	Initializes new error state.
.../local-storage/local-storage.service.spec.ts	Updates persisted-state expectations.
.../http-interceptors/http-error.interceptor.ts	Suppresses opted-in 404 notifications.
.../http-interceptors/http-error.interceptor.spec.ts	Tests notification suppression.
.../http-interceptors/http-context-tokens.ts	Defines the 404 context token.
.../feature-flags/store/feature-flags.selectors.ts	Selects route-scoped flag errors.
.../feature-flags/store/feature-flags.selectors.spec.ts	Tests flag error selection.
.../feature-flags/store/feature-flags.reducer.ts	Stores flag detail failures.
.../feature-flags/store/feature-flags.model.ts	Extends feature-flag state.
.../feature-flags/store/feature-flags.effects.ts	Handles malformed, empty, and failed fetches.
.../feature-flags/store/feature-flags.effects.spec.ts	Tests flag fetch branches.
.../feature-flags/store/feature-flags.actions.ts	Adds flag failure metadata.
.../feature-flags/feature-flags.service.ts	Exposes flag detail errors.
.../feature-flags/feature-flags.data.service.ts	Enables contextual 404 handling.
.../experiments/store/experiments.selectors.ts	Selects route-scoped experiment errors.
.../experiments/store/experiments.selector.spec.ts	Tests experiment error selection.
.../experiments/store/experiments.reducer.ts	Stores experiment detail failures.
.../experiments/store/experiments.reducer.spec.ts	Tests experiment error transitions.
.../experiments/store/experiments.model.ts	Extends experiment state.
.../experiments/store/experiments.effects.ts	Handles fetch and auxiliary-stat failures.
.../experiments/store/experiments.effects.spec.ts	Tests experiment fetch branches.
.../experiments/store/experiments.actions.ts	Adds experiment failure metadata.
.../experiments/experiments.service.ts	Exposes experiment detail errors.
.../experiments/experiments.data.service.ts	Enables contextual 404 handling.
.../experiments/experiments.data.service.spec.ts	Tests the request context token.
packages/frontend/jest.config.js	Maps shared-component aliases in Jest.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:18

• This guard only runs after segments/detail/:segmentId matches. /segments/detail has no parameter segment, so it bypasses the guard and reaches the global ** redirect to /home instead of the Segments list. Add an explicit ID-less route before the parameterized route; keep this guard for the trailing-empty-segment case.
  packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:91
• This guard cannot handle /featureflags/detail because that URL does not match a route requiring :flagId. It falls through to the global wildcard and redirects to /home, contrary to the intended Feature Flags list destination. Add an explicit ID-less redirect before this parameterized route.
  packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts:89
• getExperimentById is also used by ExperimentService.selectExperimentById for the Preview User editor (preview-user.component.ts:156), where no page-level 404 UI exists. Setting this token unconditionally suppresses the only 404 feedback in that flow. Pass contextual handling from the details-page request (or split the details fetch) so non-details callers retain the generic notification.

 return this.http.get(url, { context: new HttpContext().set(HANDLES_404_CONTEXTUALLY, true) });

packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-page-error/common-page-error.component.html:1

• This error state replaces the spinner asynchronously, but the new content has no live-region semantics or focus change, so screen-reader users are not notified that loading finished with an error. Mark the inserted error container as an alert (or implement equivalent focus management) so the title and recovery actions are announced.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• detailsPageError is a single value, but this effect intentionally keeps requests for different IDs alive. After navigating from experiment A to B, if B fails and then A's older request fails, A overwrites B's error; the route-scoped selector hides A's error and B returns to the spinner this PR is meant to remove. Preserve concurrent preview-user requests without letting stale detail requests share this slot—for example, cancel any prior contextual details fetch when a newer contextual fetch starts, and ensure non-contextual/background failures do not write details-page error state (or key errors by entity ID).

 takeUntil(
this.actions$.pipe(
ofType(experimentAction.actionGetExperimentById),
filter((newerAction) => newerAction !== action && newerAction.experimentId === experimentId)
)

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[danoswaltCL]

seems we might as well make the GET flags return a 404 instead of a 204 like the other ones, I don't think there should be a reason it's different.

[danoswaltCL]

@zackcl the PR to update feature-flags GET call to 404 has been merged into dev

[zackcl]

@bcb37@danoswaltCL Addressed the latest feedback: consolidated the duplicate error translations, clarified the not-found message with the current environment, and aligned feature flag handling with the backend’s 404 response. Feel free to take another look!

[zackcl]

Resolves#3129

Opening a detail page (experiment, feature flag, or segment) with an ID that can't be loaded currently leaves the user on an endless spinner. This PR replaces that with a proper error screen on all three pages.

Case	What the user sees now
Entity doesn't exist — wrong link, malformed ID, or deleted by someone else	"Experiment not found" + Back to Experiments button (same for flags/segments)
Unexpected failure (5xx, network error)	"Something went wrong" + Try Again and Back buttons
Detail URL without an ID (/home/detail/)	Redirects to the list page

Changes

• Added a shared CommonPageErrorComponent (not-found / load-failed variants) to the standalone component lib, plus i18n strings. All three detail pages use it.
• Added detailsPageError to the experiments / feature-flags / segments stores: the fetch-by-id failure actions now carry the failed ID and error type, new route-scoped selectors expose it, and the experiments effect got its missing catchError — the root cause of the endless spinner.
• The three detail-page content components render the error screen, and it takes priority over cached list data, so an entity deleted elsewhere shows "not found" instead of a stale details page. Once a route-scoped error is established, it remains visible during retries until a fetch for the same entity succeeds, so cached editable content and stale breadcrumb names do not reappear.
• Non-canonical IDs show not-found without firing a request. The check (isCanonicalEntityId) is the lowercase-only subset of the backend's @IsUUID() rule: the backend rejects malformed IDs with 400 (not 404), and the app only generates lowercase URLs.
• The "Network call failed" toast is skipped for 404s on the three details-page fetches only (opt-in HttpContextToken set in the data services) — the page communicates the error itself now; preview-user and all other non-details requests keep their existing toast behavior.
• Experiment detail fetches cancel an older contextual detail request when the route changes or the user retries, while non-details requests for different experiment IDs remain concurrent. Only contextual failures can write detailsPageError, so preview-user and other background failures cannot overwrite the details page's error state.
• Added focused tests for each fetch effect's malformed / 404 / 5xx paths (plus the experiment stats-only-failure case), the route-scoped error selectors, the interceptor's scoped 404 suppression, and the canonical-ID check.
• Fallout: updated the affected specs, added the new state field to the local-storage initial states, and added the missing @shared-component-lib alias to jest.config.js.
• Added explicit ID-less detail redirects, plus requireRouteParam for Angular's trailing-empty-segment case.
• Added regression tests for the shared error component, accessibility announcement, retry-time error retention, request cancellation/error ownership, data-service HttpContext opt-in, and slash/no-slash routing behavior.

Screenshots

[Copilot]

Pull request overview

Adds explicit not-found and load-failure states to experiment, feature-flag, and segment detail pages.

Changes:

• Introduces a reusable localized page-error component.
• Tracks route-specific loading errors in NgRx stores and supports retries.
• Adds ID validation, ID-less route handling, and scoped 404-toast suppression.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/frontend/projects/upgrade/src/assets/i18n/en.json	Adds error-page translations.
packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/index.ts	Exports the error component.
.../common-page-error/common-page-error.model.ts	Defines error models and ID validation.
.../common-page-error/common-page-error.model.spec.ts	Tests ID validation.
.../common-page-error/common-page-error.component.ts	Implements shared error UI logic.
.../common-page-error/common-page-error.component.scss	Styles the error screen.
.../common-page-error/common-page-error.component.html	Renders error actions and messaging.
.../segment-details-page-content.component.ts	Connects segment errors and retry.
.../segment-details-page-content.component.html	Prioritizes segment error rendering.
.../require-route-param.guard.ts	Redirects ID-less detail routes.
.../feature-flag-details-page-content.component.ts	Connects flag errors and retry.
.../feature-flag-details-page-content.component.html	Prioritizes flag error rendering.
.../experiment-details-page-content.component.ts	Connects experiment errors and retry.
.../experiment-details-page-content.component.html	Prioritizes experiment error rendering.
.../dashboard-routing.module.ts	Applies detail-route parameter guards.
.../segments/store/segments.selectors.ts	Selects route-scoped segment errors.
.../segments/store/segments.selectors.spec.ts	Tests segment error selection.
.../segments/store/segments.reducer.ts	Stores segment detail failures.
.../segments/store/segments.reducer.spec.ts	Tests segment error transitions.
.../segments/store/segments.model.ts	Extends segment state.
.../segments/store/segments.effects.ts	Classifies segment fetch failures.
.../segments/store/segments.effects.spec.ts	Tests segment failure branches.
.../segments/store/segments.actions.ts	Adds segment failure metadata.
.../segments/segments.service.ts	Exposes segment detail errors.
.../segments/segments.data.service.ts	Enables contextual 404 handling.
.../local-storage/local-storage.service.ts	Initializes new error state.
.../local-storage/local-storage.service.spec.ts	Updates persisted-state expectations.
.../http-interceptors/http-error.interceptor.ts	Suppresses opted-in 404 notifications.
.../http-interceptors/http-error.interceptor.spec.ts	Tests notification suppression.
.../http-interceptors/http-context-tokens.ts	Defines the 404 context token.
.../feature-flags/store/feature-flags.selectors.ts	Selects route-scoped flag errors.
.../feature-flags/store/feature-flags.selectors.spec.ts	Tests flag error selection.
.../feature-flags/store/feature-flags.reducer.ts	Stores flag detail failures.
.../feature-flags/store/feature-flags.model.ts	Extends feature-flag state.
.../feature-flags/store/feature-flags.effects.ts	Handles malformed, empty, and failed fetches.
.../feature-flags/store/feature-flags.effects.spec.ts	Tests flag fetch branches.
.../feature-flags/store/feature-flags.actions.ts	Adds flag failure metadata.
.../feature-flags/feature-flags.service.ts	Exposes flag detail errors.
.../feature-flags/feature-flags.data.service.ts	Enables contextual 404 handling.
.../experiments/store/experiments.selectors.ts	Selects route-scoped experiment errors.
.../experiments/store/experiments.selector.spec.ts	Tests experiment error selection.
.../experiments/store/experiments.reducer.ts	Stores experiment detail failures.
.../experiments/store/experiments.reducer.spec.ts	Tests experiment error transitions.
.../experiments/store/experiments.model.ts	Extends experiment state.
.../experiments/store/experiments.effects.ts	Handles fetch and auxiliary-stat failures.
.../experiments/store/experiments.effects.spec.ts	Tests experiment fetch branches.
.../experiments/store/experiments.actions.ts	Adds experiment failure metadata.
.../experiments/experiments.service.ts	Exposes experiment detail errors.
.../experiments/experiments.data.service.ts	Enables contextual 404 handling.
.../experiments/experiments.data.service.spec.ts	Tests the request context token.
packages/frontend/jest.config.js	Maps shared-component aliases in Jest.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:18

• This guard only runs after segments/detail/:segmentId matches. /segments/detail has no parameter segment, so it bypasses the guard and reaches the global ** redirect to /home instead of the Segments list. Add an explicit ID-less route before the parameterized route; keep this guard for the trailing-empty-segment case.
  packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:91
• This guard cannot handle /featureflags/detail because that URL does not match a route requiring :flagId. It falls through to the global wildcard and redirects to /home, contrary to the intended Feature Flags list destination. Add an explicit ID-less redirect before this parameterized route.
  packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts:89
• getExperimentById is also used by ExperimentService.selectExperimentById for the Preview User editor (preview-user.component.ts:156), where no page-level 404 UI exists. Setting this token unconditionally suppresses the only 404 feedback in that flow. Pass contextual handling from the details-page request (or split the details fetch) so non-details callers retain the generic notification.

 return this.http.get(url, { context: new HttpContext().set(HANDLES_404_CONTEXTUALLY, true) });

packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-page-error/common-page-error.component.html:1

• This error state replaces the spinner asynchronously, but the new content has no live-region semantics or focus change, so screen-reader users are not notified that loading finished with an error. Mark the inserted error container as an alert (or implement equivalent focus management) so the title and recovery actions are announced.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• detailsPageError is a single value, but this effect intentionally keeps requests for different IDs alive. After navigating from experiment A to B, if B fails and then A's older request fails, A overwrites B's error; the route-scoped selector hides A's error and B returns to the spinner this PR is meant to remove. Preserve concurrent preview-user requests without letting stale detail requests share this slot—for example, cancel any prior contextual details fetch when a newer contextual fetch starts, and ensure non-contextual/background failures do not write details-page error state (or key errors by entity ID).

 takeUntil(
this.actions$.pipe(
ofType(experimentAction.actionGetExperimentById),
filter((newerAction) => newerAction !== action && newerAction.experimentId === experimentId)
)

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[danoswaltCL]

seems we might as well make the GET flags return a 404 instead of a 204 like the other ones, I don't think there should be a reason it's different.

[danoswaltCL]

@zackcl the PR to update feature-flags GET call to 404 has been merged into dev

[zackcl]

@bcb37@danoswaltCL Addressed the latest feedback: consolidated the duplicate error translations, clarified the not-found message with the current environment, and aligned feature flag handling with the backend’s 404 response. Feel free to take another look!

[zackcl]

Resolves#3129

Opening a detail page (experiment, feature flag, or segment) with an ID that can't be loaded currently leaves the user on an endless spinner. This PR replaces that with a proper error screen on all three pages.

Case	What the user sees now
Entity doesn't exist — wrong link, malformed ID, or deleted by someone else	"Experiment not found" + Back to Experiments button (same for flags/segments)
Unexpected failure (5xx, network error)	"Something went wrong" + Try Again and Back buttons
Detail URL without an ID (/home/detail/)	Redirects to the list page

Changes

• Added a shared CommonPageErrorComponent (not-found / load-failed variants) to the standalone component lib, plus i18n strings. All three detail pages use it.
• Added detailsPageError to the experiments / feature-flags / segments stores: the fetch-by-id failure actions now carry the failed ID and error type, new route-scoped selectors expose it, and the experiments effect got its missing catchError — the root cause of the endless spinner.
• The three detail-page content components render the error screen, and it takes priority over cached list data, so an entity deleted elsewhere shows "not found" instead of a stale details page. Once a route-scoped error is established, it remains visible during retries until a fetch for the same entity succeeds, so cached editable content and stale breadcrumb names do not reappear.
• Non-canonical IDs show not-found without firing a request. The check (isCanonicalEntityId) is the lowercase-only subset of the backend's @IsUUID() rule: the backend rejects malformed IDs with 400 (not 404), and the app only generates lowercase URLs.
• The "Network call failed" toast is skipped for 404s on the three details-page fetches only (opt-in HttpContextToken set in the data services) — the page communicates the error itself now; preview-user and all other non-details requests keep their existing toast behavior.
• Experiment detail fetches cancel an older contextual detail request when the route changes or the user retries, while non-details requests for different experiment IDs remain concurrent. Only contextual failures can write detailsPageError, so preview-user and other background failures cannot overwrite the details page's error state.
• Added focused tests for each fetch effect's malformed / 404 / 5xx paths (plus the experiment stats-only-failure case), the route-scoped error selectors, the interceptor's scoped 404 suppression, and the canonical-ID check.
• Fallout: updated the affected specs, added the new state field to the local-storage initial states, and added the missing @shared-component-lib alias to jest.config.js.
• Added explicit ID-less detail redirects, plus requireRouteParam for Angular's trailing-empty-segment case.
• Added regression tests for the shared error component, accessibility announcement, retry-time error retention, request cancellation/error ownership, data-service HttpContext opt-in, and slash/no-slash routing behavior.

Screenshots

[Copilot]

Pull request overview

Adds explicit not-found and load-failure states to experiment, feature-flag, and segment detail pages.

Changes:

• Introduces a reusable localized page-error component.
• Tracks route-specific loading errors in NgRx stores and supports retries.
• Adds ID validation, ID-less route handling, and scoped 404-toast suppression.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/frontend/projects/upgrade/src/assets/i18n/en.json	Adds error-page translations.
packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/index.ts	Exports the error component.
.../common-page-error/common-page-error.model.ts	Defines error models and ID validation.
.../common-page-error/common-page-error.model.spec.ts	Tests ID validation.
.../common-page-error/common-page-error.component.ts	Implements shared error UI logic.
.../common-page-error/common-page-error.component.scss	Styles the error screen.
.../common-page-error/common-page-error.component.html	Renders error actions and messaging.
.../segment-details-page-content.component.ts	Connects segment errors and retry.
.../segment-details-page-content.component.html	Prioritizes segment error rendering.
.../require-route-param.guard.ts	Redirects ID-less detail routes.
.../feature-flag-details-page-content.component.ts	Connects flag errors and retry.
.../feature-flag-details-page-content.component.html	Prioritizes flag error rendering.
.../experiment-details-page-content.component.ts	Connects experiment errors and retry.
.../experiment-details-page-content.component.html	Prioritizes experiment error rendering.
.../dashboard-routing.module.ts	Applies detail-route parameter guards.
.../segments/store/segments.selectors.ts	Selects route-scoped segment errors.
.../segments/store/segments.selectors.spec.ts	Tests segment error selection.
.../segments/store/segments.reducer.ts	Stores segment detail failures.
.../segments/store/segments.reducer.spec.ts	Tests segment error transitions.
.../segments/store/segments.model.ts	Extends segment state.
.../segments/store/segments.effects.ts	Classifies segment fetch failures.
.../segments/store/segments.effects.spec.ts	Tests segment failure branches.
.../segments/store/segments.actions.ts	Adds segment failure metadata.
.../segments/segments.service.ts	Exposes segment detail errors.
.../segments/segments.data.service.ts	Enables contextual 404 handling.
.../local-storage/local-storage.service.ts	Initializes new error state.
.../local-storage/local-storage.service.spec.ts	Updates persisted-state expectations.
.../http-interceptors/http-error.interceptor.ts	Suppresses opted-in 404 notifications.
.../http-interceptors/http-error.interceptor.spec.ts	Tests notification suppression.
.../http-interceptors/http-context-tokens.ts	Defines the 404 context token.
.../feature-flags/store/feature-flags.selectors.ts	Selects route-scoped flag errors.
.../feature-flags/store/feature-flags.selectors.spec.ts	Tests flag error selection.
.../feature-flags/store/feature-flags.reducer.ts	Stores flag detail failures.
.../feature-flags/store/feature-flags.model.ts	Extends feature-flag state.
.../feature-flags/store/feature-flags.effects.ts	Handles malformed, empty, and failed fetches.
.../feature-flags/store/feature-flags.effects.spec.ts	Tests flag fetch branches.
.../feature-flags/store/feature-flags.actions.ts	Adds flag failure metadata.
.../feature-flags/feature-flags.service.ts	Exposes flag detail errors.
.../feature-flags/feature-flags.data.service.ts	Enables contextual 404 handling.
.../experiments/store/experiments.selectors.ts	Selects route-scoped experiment errors.
.../experiments/store/experiments.selector.spec.ts	Tests experiment error selection.
.../experiments/store/experiments.reducer.ts	Stores experiment detail failures.
.../experiments/store/experiments.reducer.spec.ts	Tests experiment error transitions.
.../experiments/store/experiments.model.ts	Extends experiment state.
.../experiments/store/experiments.effects.ts	Handles fetch and auxiliary-stat failures.
.../experiments/store/experiments.effects.spec.ts	Tests experiment fetch branches.
.../experiments/store/experiments.actions.ts	Adds experiment failure metadata.
.../experiments/experiments.service.ts	Exposes experiment detail errors.
.../experiments/experiments.data.service.ts	Enables contextual 404 handling.
.../experiments/experiments.data.service.spec.ts	Tests the request context token.
packages/frontend/jest.config.js	Maps shared-component aliases in Jest.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:18

• This guard only runs after segments/detail/:segmentId matches. /segments/detail has no parameter segment, so it bypasses the guard and reaches the global ** redirect to /home instead of the Segments list. Add an explicit ID-less route before the parameterized route; keep this guard for the trailing-empty-segment case.
  packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:91
• This guard cannot handle /featureflags/detail because that URL does not match a route requiring :flagId. It falls through to the global wildcard and redirects to /home, contrary to the intended Feature Flags list destination. Add an explicit ID-less redirect before this parameterized route.
  packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts:89
• getExperimentById is also used by ExperimentService.selectExperimentById for the Preview User editor (preview-user.component.ts:156), where no page-level 404 UI exists. Setting this token unconditionally suppresses the only 404 feedback in that flow. Pass contextual handling from the details-page request (or split the details fetch) so non-details callers retain the generic notification.

 return this.http.get(url, { context: new HttpContext().set(HANDLES_404_CONTEXTUALLY, true) });

packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-page-error/common-page-error.component.html:1

• This error state replaces the spinner asynchronously, but the new content has no live-region semantics or focus change, so screen-reader users are not notified that loading finished with an error. Mark the inserted error container as an alert (or implement equivalent focus management) so the title and recovery actions are announced.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• detailsPageError is a single value, but this effect intentionally keeps requests for different IDs alive. After navigating from experiment A to B, if B fails and then A's older request fails, A overwrites B's error; the route-scoped selector hides A's error and B returns to the spinner this PR is meant to remove. Preserve concurrent preview-user requests without letting stale detail requests share this slot—for example, cancel any prior contextual details fetch when a newer contextual fetch starts, and ensure non-contextual/background failures do not write details-page error state (or key errors by entity ID).

 takeUntil(
this.actions$.pipe(
ofType(experimentAction.actionGetExperimentById),
filter((newerAction) => newerAction !== action && newerAction.experimentId === experimentId)
)

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[danoswaltCL]

seems we might as well make the GET flags return a 404 instead of a 204 like the other ones, I don't think there should be a reason it's different.

[danoswaltCL]

@zackcl the PR to update feature-flags GET call to 404 has been merged into dev

[zackcl]

@bcb37@danoswaltCL Addressed the latest feedback: consolidated the duplicate error translations, clarified the not-found message with the current environment, and aligned feature flag handling with the backend’s 404 response. Feel free to take another look!

[zackcl]

Resolves#3129

Opening a detail page (experiment, feature flag, or segment) with an ID that can't be loaded currently leaves the user on an endless spinner. This PR replaces that with a proper error screen on all three pages.

Case	What the user sees now
Entity doesn't exist — wrong link, malformed ID, or deleted by someone else	"Experiment not found" + Back to Experiments button (same for flags/segments)
Unexpected failure (5xx, network error)	"Something went wrong" + Try Again and Back buttons
Detail URL without an ID (/home/detail/)	Redirects to the list page

Changes

• Added a shared CommonPageErrorComponent (not-found / load-failed variants) to the standalone component lib, plus i18n strings. All three detail pages use it.
• Added detailsPageError to the experiments / feature-flags / segments stores: the fetch-by-id failure actions now carry the failed ID and error type, new route-scoped selectors expose it, and the experiments effect got its missing catchError — the root cause of the endless spinner.
• The three detail-page content components render the error screen, and it takes priority over cached list data, so an entity deleted elsewhere shows "not found" instead of a stale details page. Once a route-scoped error is established, it remains visible during retries until a fetch for the same entity succeeds, so cached editable content and stale breadcrumb names do not reappear.
• Non-canonical IDs show not-found without firing a request. The check (isCanonicalEntityId) is the lowercase-only subset of the backend's @IsUUID() rule: the backend rejects malformed IDs with 400 (not 404), and the app only generates lowercase URLs.
• The "Network call failed" toast is skipped for 404s on the three details-page fetches only (opt-in HttpContextToken set in the data services) — the page communicates the error itself now; preview-user and all other non-details requests keep their existing toast behavior.
• Experiment detail fetches cancel an older contextual detail request when the route changes or the user retries, while non-details requests for different experiment IDs remain concurrent. Only contextual failures can write detailsPageError, so preview-user and other background failures cannot overwrite the details page's error state.
• Added focused tests for each fetch effect's malformed / 404 / 5xx paths (plus the experiment stats-only-failure case), the route-scoped error selectors, the interceptor's scoped 404 suppression, and the canonical-ID check.
• Fallout: updated the affected specs, added the new state field to the local-storage initial states, and added the missing @shared-component-lib alias to jest.config.js.
• Added explicit ID-less detail redirects, plus requireRouteParam for Angular's trailing-empty-segment case.
• Added regression tests for the shared error component, accessibility announcement, retry-time error retention, request cancellation/error ownership, data-service HttpContext opt-in, and slash/no-slash routing behavior.

Screenshots

[Copilot]

Pull request overview

Adds explicit not-found and load-failure states to experiment, feature-flag, and segment detail pages.

Changes:

• Introduces a reusable localized page-error component.
• Tracks route-specific loading errors in NgRx stores and supports retries.
• Adds ID validation, ID-less route handling, and scoped 404-toast suppression.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/frontend/projects/upgrade/src/assets/i18n/en.json	Adds error-page translations.
packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/index.ts	Exports the error component.
.../common-page-error/common-page-error.model.ts	Defines error models and ID validation.
.../common-page-error/common-page-error.model.spec.ts	Tests ID validation.
.../common-page-error/common-page-error.component.ts	Implements shared error UI logic.
.../common-page-error/common-page-error.component.scss	Styles the error screen.
.../common-page-error/common-page-error.component.html	Renders error actions and messaging.
.../segment-details-page-content.component.ts	Connects segment errors and retry.
.../segment-details-page-content.component.html	Prioritizes segment error rendering.
.../require-route-param.guard.ts	Redirects ID-less detail routes.
.../feature-flag-details-page-content.component.ts	Connects flag errors and retry.
.../feature-flag-details-page-content.component.html	Prioritizes flag error rendering.
.../experiment-details-page-content.component.ts	Connects experiment errors and retry.
.../experiment-details-page-content.component.html	Prioritizes experiment error rendering.
.../dashboard-routing.module.ts	Applies detail-route parameter guards.
.../segments/store/segments.selectors.ts	Selects route-scoped segment errors.
.../segments/store/segments.selectors.spec.ts	Tests segment error selection.
.../segments/store/segments.reducer.ts	Stores segment detail failures.
.../segments/store/segments.reducer.spec.ts	Tests segment error transitions.
.../segments/store/segments.model.ts	Extends segment state.
.../segments/store/segments.effects.ts	Classifies segment fetch failures.
.../segments/store/segments.effects.spec.ts	Tests segment failure branches.
.../segments/store/segments.actions.ts	Adds segment failure metadata.
.../segments/segments.service.ts	Exposes segment detail errors.
.../segments/segments.data.service.ts	Enables contextual 404 handling.
.../local-storage/local-storage.service.ts	Initializes new error state.
.../local-storage/local-storage.service.spec.ts	Updates persisted-state expectations.
.../http-interceptors/http-error.interceptor.ts	Suppresses opted-in 404 notifications.
.../http-interceptors/http-error.interceptor.spec.ts	Tests notification suppression.
.../http-interceptors/http-context-tokens.ts	Defines the 404 context token.
.../feature-flags/store/feature-flags.selectors.ts	Selects route-scoped flag errors.
.../feature-flags/store/feature-flags.selectors.spec.ts	Tests flag error selection.
.../feature-flags/store/feature-flags.reducer.ts	Stores flag detail failures.
.../feature-flags/store/feature-flags.model.ts	Extends feature-flag state.
.../feature-flags/store/feature-flags.effects.ts	Handles malformed, empty, and failed fetches.
.../feature-flags/store/feature-flags.effects.spec.ts	Tests flag fetch branches.
.../feature-flags/store/feature-flags.actions.ts	Adds flag failure metadata.
.../feature-flags/feature-flags.service.ts	Exposes flag detail errors.
.../feature-flags/feature-flags.data.service.ts	Enables contextual 404 handling.
.../experiments/store/experiments.selectors.ts	Selects route-scoped experiment errors.
.../experiments/store/experiments.selector.spec.ts	Tests experiment error selection.
.../experiments/store/experiments.reducer.ts	Stores experiment detail failures.
.../experiments/store/experiments.reducer.spec.ts	Tests experiment error transitions.
.../experiments/store/experiments.model.ts	Extends experiment state.
.../experiments/store/experiments.effects.ts	Handles fetch and auxiliary-stat failures.
.../experiments/store/experiments.effects.spec.ts	Tests experiment fetch branches.
.../experiments/store/experiments.actions.ts	Adds experiment failure metadata.
.../experiments/experiments.service.ts	Exposes experiment detail errors.
.../experiments/experiments.data.service.ts	Enables contextual 404 handling.
.../experiments/experiments.data.service.spec.ts	Tests the request context token.
packages/frontend/jest.config.js	Maps shared-component aliases in Jest.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:18

• This guard only runs after segments/detail/:segmentId matches. /segments/detail has no parameter segment, so it bypasses the guard and reaches the global ** redirect to /home instead of the Segments list. Add an explicit ID-less route before the parameterized route; keep this guard for the trailing-empty-segment case.
  packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:91
• This guard cannot handle /featureflags/detail because that URL does not match a route requiring :flagId. It falls through to the global wildcard and redirects to /home, contrary to the intended Feature Flags list destination. Add an explicit ID-less redirect before this parameterized route.
  packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts:89
• getExperimentById is also used by ExperimentService.selectExperimentById for the Preview User editor (preview-user.component.ts:156), where no page-level 404 UI exists. Setting this token unconditionally suppresses the only 404 feedback in that flow. Pass contextual handling from the details-page request (or split the details fetch) so non-details callers retain the generic notification.

 return this.http.get(url, { context: new HttpContext().set(HANDLES_404_CONTEXTUALLY, true) });

packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-page-error/common-page-error.component.html:1

• This error state replaces the spinner asynchronously, but the new content has no live-region semantics or focus change, so screen-reader users are not notified that loading finished with an error. Mark the inserted error container as an alert (or implement equivalent focus management) so the title and recovery actions are announced.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• detailsPageError is a single value, but this effect intentionally keeps requests for different IDs alive. After navigating from experiment A to B, if B fails and then A's older request fails, A overwrites B's error; the route-scoped selector hides A's error and B returns to the spinner this PR is meant to remove. Preserve concurrent preview-user requests without letting stale detail requests share this slot—for example, cancel any prior contextual details fetch when a newer contextual fetch starts, and ensure non-contextual/background failures do not write details-page error state (or key errors by entity ID).

 takeUntil(
this.actions$.pipe(
ofType(experimentAction.actionGetExperimentById),
filter((newerAction) => newerAction !== action && newerAction.experimentId === experimentId)
)

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[danoswaltCL]

seems we might as well make the GET flags return a 404 instead of a 204 like the other ones, I don't think there should be a reason it's different.

[danoswaltCL]

@zackcl the PR to update feature-flags GET call to 404 has been merged into dev

[zackcl]

@bcb37@danoswaltCL Addressed the latest feedback: consolidated the duplicate error translations, clarified the not-found message with the current environment, and aligned feature flag handling with the backend’s 404 response. Feel free to take another look!

[zackcl]

Resolves#3129

Opening a detail page (experiment, feature flag, or segment) with an ID that can't be loaded currently leaves the user on an endless spinner. This PR replaces that with a proper error screen on all three pages.

Case	What the user sees now
Entity doesn't exist — wrong link, malformed ID, or deleted by someone else	"Experiment not found" + Back to Experiments button (same for flags/segments)
Unexpected failure (5xx, network error)	"Something went wrong" + Try Again and Back buttons
Detail URL without an ID (/home/detail/)	Redirects to the list page

Changes

• Added a shared CommonPageErrorComponent (not-found / load-failed variants) to the standalone component lib, plus i18n strings. All three detail pages use it.
• Added detailsPageError to the experiments / feature-flags / segments stores: the fetch-by-id failure actions now carry the failed ID and error type, new route-scoped selectors expose it, and the experiments effect got its missing catchError — the root cause of the endless spinner.
• The three detail-page content components render the error screen, and it takes priority over cached list data, so an entity deleted elsewhere shows "not found" instead of a stale details page. Once a route-scoped error is established, it remains visible during retries until a fetch for the same entity succeeds, so cached editable content and stale breadcrumb names do not reappear.
• Non-canonical IDs show not-found without firing a request. The check (isCanonicalEntityId) is the lowercase-only subset of the backend's @IsUUID() rule: the backend rejects malformed IDs with 400 (not 404), and the app only generates lowercase URLs.
• The "Network call failed" toast is skipped for 404s on the three details-page fetches only (opt-in HttpContextToken set in the data services) — the page communicates the error itself now; preview-user and all other non-details requests keep their existing toast behavior.
• Experiment detail fetches cancel an older contextual detail request when the route changes or the user retries, while non-details requests for different experiment IDs remain concurrent. Only contextual failures can write detailsPageError, so preview-user and other background failures cannot overwrite the details page's error state.
• Added focused tests for each fetch effect's malformed / 404 / 5xx paths (plus the experiment stats-only-failure case), the route-scoped error selectors, the interceptor's scoped 404 suppression, and the canonical-ID check.
• Fallout: updated the affected specs, added the new state field to the local-storage initial states, and added the missing @shared-component-lib alias to jest.config.js.
• Added explicit ID-less detail redirects, plus requireRouteParam for Angular's trailing-empty-segment case.
• Added regression tests for the shared error component, accessibility announcement, retry-time error retention, request cancellation/error ownership, data-service HttpContext opt-in, and slash/no-slash routing behavior.

Screenshots

[Copilot]

Pull request overview

Adds explicit not-found and load-failure states to experiment, feature-flag, and segment detail pages.

Changes:

• Introduces a reusable localized page-error component.
• Tracks route-specific loading errors in NgRx stores and supports retries.
• Adds ID validation, ID-less route handling, and scoped 404-toast suppression.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/frontend/projects/upgrade/src/assets/i18n/en.json	Adds error-page translations.
packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/index.ts	Exports the error component.
.../common-page-error/common-page-error.model.ts	Defines error models and ID validation.
.../common-page-error/common-page-error.model.spec.ts	Tests ID validation.
.../common-page-error/common-page-error.component.ts	Implements shared error UI logic.
.../common-page-error/common-page-error.component.scss	Styles the error screen.
.../common-page-error/common-page-error.component.html	Renders error actions and messaging.
.../segment-details-page-content.component.ts	Connects segment errors and retry.
.../segment-details-page-content.component.html	Prioritizes segment error rendering.
.../require-route-param.guard.ts	Redirects ID-less detail routes.
.../feature-flag-details-page-content.component.ts	Connects flag errors and retry.
.../feature-flag-details-page-content.component.html	Prioritizes flag error rendering.
.../experiment-details-page-content.component.ts	Connects experiment errors and retry.
.../experiment-details-page-content.component.html	Prioritizes experiment error rendering.
.../dashboard-routing.module.ts	Applies detail-route parameter guards.
.../segments/store/segments.selectors.ts	Selects route-scoped segment errors.
.../segments/store/segments.selectors.spec.ts	Tests segment error selection.
.../segments/store/segments.reducer.ts	Stores segment detail failures.
.../segments/store/segments.reducer.spec.ts	Tests segment error transitions.
.../segments/store/segments.model.ts	Extends segment state.
.../segments/store/segments.effects.ts	Classifies segment fetch failures.
.../segments/store/segments.effects.spec.ts	Tests segment failure branches.
.../segments/store/segments.actions.ts	Adds segment failure metadata.
.../segments/segments.service.ts	Exposes segment detail errors.
.../segments/segments.data.service.ts	Enables contextual 404 handling.
.../local-storage/local-storage.service.ts	Initializes new error state.
.../local-storage/local-storage.service.spec.ts	Updates persisted-state expectations.
.../http-interceptors/http-error.interceptor.ts	Suppresses opted-in 404 notifications.
.../http-interceptors/http-error.interceptor.spec.ts	Tests notification suppression.
.../http-interceptors/http-context-tokens.ts	Defines the 404 context token.
.../feature-flags/store/feature-flags.selectors.ts	Selects route-scoped flag errors.
.../feature-flags/store/feature-flags.selectors.spec.ts	Tests flag error selection.
.../feature-flags/store/feature-flags.reducer.ts	Stores flag detail failures.
.../feature-flags/store/feature-flags.model.ts	Extends feature-flag state.
.../feature-flags/store/feature-flags.effects.ts	Handles malformed, empty, and failed fetches.
.../feature-flags/store/feature-flags.effects.spec.ts	Tests flag fetch branches.
.../feature-flags/store/feature-flags.actions.ts	Adds flag failure metadata.
.../feature-flags/feature-flags.service.ts	Exposes flag detail errors.
.../feature-flags/feature-flags.data.service.ts	Enables contextual 404 handling.
.../experiments/store/experiments.selectors.ts	Selects route-scoped experiment errors.
.../experiments/store/experiments.selector.spec.ts	Tests experiment error selection.
.../experiments/store/experiments.reducer.ts	Stores experiment detail failures.
.../experiments/store/experiments.reducer.spec.ts	Tests experiment error transitions.
.../experiments/store/experiments.model.ts	Extends experiment state.
.../experiments/store/experiments.effects.ts	Handles fetch and auxiliary-stat failures.
.../experiments/store/experiments.effects.spec.ts	Tests experiment fetch branches.
.../experiments/store/experiments.actions.ts	Adds experiment failure metadata.
.../experiments/experiments.service.ts	Exposes experiment detail errors.
.../experiments/experiments.data.service.ts	Enables contextual 404 handling.
.../experiments/experiments.data.service.spec.ts	Tests the request context token.
packages/frontend/jest.config.js	Maps shared-component aliases in Jest.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:18

• This guard only runs after segments/detail/:segmentId matches. /segments/detail has no parameter segment, so it bypasses the guard and reaches the global ** redirect to /home instead of the Segments list. Add an explicit ID-less route before the parameterized route; keep this guard for the trailing-empty-segment case.
  packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:91
• This guard cannot handle /featureflags/detail because that URL does not match a route requiring :flagId. It falls through to the global wildcard and redirects to /home, contrary to the intended Feature Flags list destination. Add an explicit ID-less redirect before this parameterized route.
  packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts:89
• getExperimentById is also used by ExperimentService.selectExperimentById for the Preview User editor (preview-user.component.ts:156), where no page-level 404 UI exists. Setting this token unconditionally suppresses the only 404 feedback in that flow. Pass contextual handling from the details-page request (or split the details fetch) so non-details callers retain the generic notification.

 return this.http.get(url, { context: new HttpContext().set(HANDLES_404_CONTEXTUALLY, true) });

packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-page-error/common-page-error.component.html:1

• This error state replaces the spinner asynchronously, but the new content has no live-region semantics or focus change, so screen-reader users are not notified that loading finished with an error. Mark the inserted error container as an alert (or implement equivalent focus management) so the title and recovery actions are announced.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• detailsPageError is a single value, but this effect intentionally keeps requests for different IDs alive. After navigating from experiment A to B, if B fails and then A's older request fails, A overwrites B's error; the route-scoped selector hides A's error and B returns to the spinner this PR is meant to remove. Preserve concurrent preview-user requests without letting stale detail requests share this slot—for example, cancel any prior contextual details fetch when a newer contextual fetch starts, and ensure non-contextual/background failures do not write details-page error state (or key errors by entity ID).

 takeUntil(
this.actions$.pipe(
ofType(experimentAction.actionGetExperimentById),
filter((newerAction) => newerAction !== action && newerAction.experimentId === experimentId)
)

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[danoswaltCL]

seems we might as well make the GET flags return a 404 instead of a 204 like the other ones, I don't think there should be a reason it's different.

[danoswaltCL]

@zackcl the PR to update feature-flags GET call to 404 has been merged into dev

[zackcl]

@bcb37@danoswaltCL Addressed the latest feedback: consolidated the duplicate error translations, clarified the not-found message with the current environment, and aligned feature flag handling with the backend’s 404 response. Feel free to take another look!

[zackcl]

Resolves#3129

Opening a detail page (experiment, feature flag, or segment) with an ID that can't be loaded currently leaves the user on an endless spinner. This PR replaces that with a proper error screen on all three pages.

Case	What the user sees now
Entity doesn't exist — wrong link, malformed ID, or deleted by someone else	"Experiment not found" + Back to Experiments button (same for flags/segments)
Unexpected failure (5xx, network error)	"Something went wrong" + Try Again and Back buttons
Detail URL without an ID (/home/detail/)	Redirects to the list page

Changes

• Added a shared CommonPageErrorComponent (not-found / load-failed variants) to the standalone component lib, plus i18n strings. All three detail pages use it.
• Added detailsPageError to the experiments / feature-flags / segments stores: the fetch-by-id failure actions now carry the failed ID and error type, new route-scoped selectors expose it, and the experiments effect got its missing catchError — the root cause of the endless spinner.
• The three detail-page content components render the error screen, and it takes priority over cached list data, so an entity deleted elsewhere shows "not found" instead of a stale details page. Once a route-scoped error is established, it remains visible during retries until a fetch for the same entity succeeds, so cached editable content and stale breadcrumb names do not reappear.
• Non-canonical IDs show not-found without firing a request. The check (isCanonicalEntityId) is the lowercase-only subset of the backend's @IsUUID() rule: the backend rejects malformed IDs with 400 (not 404), and the app only generates lowercase URLs.
• The "Network call failed" toast is skipped for 404s on the three details-page fetches only (opt-in HttpContextToken set in the data services) — the page communicates the error itself now; preview-user and all other non-details requests keep their existing toast behavior.
• Experiment detail fetches cancel an older contextual detail request when the route changes or the user retries, while non-details requests for different experiment IDs remain concurrent. Only contextual failures can write detailsPageError, so preview-user and other background failures cannot overwrite the details page's error state.
• Added focused tests for each fetch effect's malformed / 404 / 5xx paths (plus the experiment stats-only-failure case), the route-scoped error selectors, the interceptor's scoped 404 suppression, and the canonical-ID check.
• Fallout: updated the affected specs, added the new state field to the local-storage initial states, and added the missing @shared-component-lib alias to jest.config.js.
• Added explicit ID-less detail redirects, plus requireRouteParam for Angular's trailing-empty-segment case.
• Added regression tests for the shared error component, accessibility announcement, retry-time error retention, request cancellation/error ownership, data-service HttpContext opt-in, and slash/no-slash routing behavior.

Screenshots

[Copilot]

Pull request overview

Adds explicit not-found and load-failure states to experiment, feature-flag, and segment detail pages.

Changes:

• Introduces a reusable localized page-error component.
• Tracks route-specific loading errors in NgRx stores and supports retries.
• Adds ID validation, ID-less route handling, and scoped 404-toast suppression.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/frontend/projects/upgrade/src/assets/i18n/en.json	Adds error-page translations.
packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/index.ts	Exports the error component.
.../common-page-error/common-page-error.model.ts	Defines error models and ID validation.
.../common-page-error/common-page-error.model.spec.ts	Tests ID validation.
.../common-page-error/common-page-error.component.ts	Implements shared error UI logic.
.../common-page-error/common-page-error.component.scss	Styles the error screen.
.../common-page-error/common-page-error.component.html	Renders error actions and messaging.
.../segment-details-page-content.component.ts	Connects segment errors and retry.
.../segment-details-page-content.component.html	Prioritizes segment error rendering.
.../require-route-param.guard.ts	Redirects ID-less detail routes.
.../feature-flag-details-page-content.component.ts	Connects flag errors and retry.
.../feature-flag-details-page-content.component.html	Prioritizes flag error rendering.
.../experiment-details-page-content.component.ts	Connects experiment errors and retry.
.../experiment-details-page-content.component.html	Prioritizes experiment error rendering.
.../dashboard-routing.module.ts	Applies detail-route parameter guards.
.../segments/store/segments.selectors.ts	Selects route-scoped segment errors.
.../segments/store/segments.selectors.spec.ts	Tests segment error selection.
.../segments/store/segments.reducer.ts	Stores segment detail failures.
.../segments/store/segments.reducer.spec.ts	Tests segment error transitions.
.../segments/store/segments.model.ts	Extends segment state.
.../segments/store/segments.effects.ts	Classifies segment fetch failures.
.../segments/store/segments.effects.spec.ts	Tests segment failure branches.
.../segments/store/segments.actions.ts	Adds segment failure metadata.
.../segments/segments.service.ts	Exposes segment detail errors.
.../segments/segments.data.service.ts	Enables contextual 404 handling.
.../local-storage/local-storage.service.ts	Initializes new error state.
.../local-storage/local-storage.service.spec.ts	Updates persisted-state expectations.
.../http-interceptors/http-error.interceptor.ts	Suppresses opted-in 404 notifications.
.../http-interceptors/http-error.interceptor.spec.ts	Tests notification suppression.
.../http-interceptors/http-context-tokens.ts	Defines the 404 context token.
.../feature-flags/store/feature-flags.selectors.ts	Selects route-scoped flag errors.
.../feature-flags/store/feature-flags.selectors.spec.ts	Tests flag error selection.
.../feature-flags/store/feature-flags.reducer.ts	Stores flag detail failures.
.../feature-flags/store/feature-flags.model.ts	Extends feature-flag state.
.../feature-flags/store/feature-flags.effects.ts	Handles malformed, empty, and failed fetches.
.../feature-flags/store/feature-flags.effects.spec.ts	Tests flag fetch branches.
.../feature-flags/store/feature-flags.actions.ts	Adds flag failure metadata.
.../feature-flags/feature-flags.service.ts	Exposes flag detail errors.
.../feature-flags/feature-flags.data.service.ts	Enables contextual 404 handling.
.../experiments/store/experiments.selectors.ts	Selects route-scoped experiment errors.
.../experiments/store/experiments.selector.spec.ts	Tests experiment error selection.
.../experiments/store/experiments.reducer.ts	Stores experiment detail failures.
.../experiments/store/experiments.reducer.spec.ts	Tests experiment error transitions.
.../experiments/store/experiments.model.ts	Extends experiment state.
.../experiments/store/experiments.effects.ts	Handles fetch and auxiliary-stat failures.
.../experiments/store/experiments.effects.spec.ts	Tests experiment fetch branches.
.../experiments/store/experiments.actions.ts	Adds experiment failure metadata.
.../experiments/experiments.service.ts	Exposes experiment detail errors.
.../experiments/experiments.data.service.ts	Enables contextual 404 handling.
.../experiments/experiments.data.service.spec.ts	Tests the request context token.
packages/frontend/jest.config.js	Maps shared-component aliases in Jest.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:18

• This guard only runs after segments/detail/:segmentId matches. /segments/detail has no parameter segment, so it bypasses the guard and reaches the global ** redirect to /home instead of the Segments list. Add an explicit ID-less route before the parameterized route; keep this guard for the trailing-empty-segment case.
  packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:91
• This guard cannot handle /featureflags/detail because that URL does not match a route requiring :flagId. It falls through to the global wildcard and redirects to /home, contrary to the intended Feature Flags list destination. Add an explicit ID-less redirect before this parameterized route.
  packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts:89
• getExperimentById is also used by ExperimentService.selectExperimentById for the Preview User editor (preview-user.component.ts:156), where no page-level 404 UI exists. Setting this token unconditionally suppresses the only 404 feedback in that flow. Pass contextual handling from the details-page request (or split the details fetch) so non-details callers retain the generic notification.

 return this.http.get(url, { context: new HttpContext().set(HANDLES_404_CONTEXTUALLY, true) });

packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-page-error/common-page-error.component.html:1

• This error state replaces the spinner asynchronously, but the new content has no live-region semantics or focus change, so screen-reader users are not notified that loading finished with an error. Mark the inserted error container as an alert (or implement equivalent focus management) so the title and recovery actions are announced.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• detailsPageError is a single value, but this effect intentionally keeps requests for different IDs alive. After navigating from experiment A to B, if B fails and then A's older request fails, A overwrites B's error; the route-scoped selector hides A's error and B returns to the spinner this PR is meant to remove. Preserve concurrent preview-user requests without letting stale detail requests share this slot—for example, cancel any prior contextual details fetch when a newer contextual fetch starts, and ensure non-contextual/background failures do not write details-page error state (or key errors by entity ID).

 takeUntil(
this.actions$.pipe(
ofType(experimentAction.actionGetExperimentById),
filter((newerAction) => newerAction !== action && newerAction.experimentId === experimentId)
)

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[danoswaltCL]

seems we might as well make the GET flags return a 404 instead of a 204 like the other ones, I don't think there should be a reason it's different.

[danoswaltCL]

@zackcl the PR to update feature-flags GET call to 404 has been merged into dev

[zackcl]

@bcb37@danoswaltCL Addressed the latest feedback: consolidated the duplicate error translations, clarified the not-found message with the current environment, and aligned feature flag handling with the backend’s 404 response. Feel free to take another look!

[zackcl]

Resolves#3129

Opening a detail page (experiment, feature flag, or segment) with an ID that can't be loaded currently leaves the user on an endless spinner. This PR replaces that with a proper error screen on all three pages.

Case	What the user sees now
Entity doesn't exist — wrong link, malformed ID, or deleted by someone else	"Experiment not found" + Back to Experiments button (same for flags/segments)
Unexpected failure (5xx, network error)	"Something went wrong" + Try Again and Back buttons
Detail URL without an ID (/home/detail/)	Redirects to the list page

Changes

• Added a shared CommonPageErrorComponent (not-found / load-failed variants) to the standalone component lib, plus i18n strings. All three detail pages use it.
• Added detailsPageError to the experiments / feature-flags / segments stores: the fetch-by-id failure actions now carry the failed ID and error type, new route-scoped selectors expose it, and the experiments effect got its missing catchError — the root cause of the endless spinner.
• The three detail-page content components render the error screen, and it takes priority over cached list data, so an entity deleted elsewhere shows "not found" instead of a stale details page. Once a route-scoped error is established, it remains visible during retries until a fetch for the same entity succeeds, so cached editable content and stale breadcrumb names do not reappear.
• Non-canonical IDs show not-found without firing a request. The check (isCanonicalEntityId) is the lowercase-only subset of the backend's @IsUUID() rule: the backend rejects malformed IDs with 400 (not 404), and the app only generates lowercase URLs.
• The "Network call failed" toast is skipped for 404s on the three details-page fetches only (opt-in HttpContextToken set in the data services) — the page communicates the error itself now; preview-user and all other non-details requests keep their existing toast behavior.
• Experiment detail fetches cancel an older contextual detail request when the route changes or the user retries, while non-details requests for different experiment IDs remain concurrent. Only contextual failures can write detailsPageError, so preview-user and other background failures cannot overwrite the details page's error state.
• Added focused tests for each fetch effect's malformed / 404 / 5xx paths (plus the experiment stats-only-failure case), the route-scoped error selectors, the interceptor's scoped 404 suppression, and the canonical-ID check.
• Fallout: updated the affected specs, added the new state field to the local-storage initial states, and added the missing @shared-component-lib alias to jest.config.js.
• Added explicit ID-less detail redirects, plus requireRouteParam for Angular's trailing-empty-segment case.
• Added regression tests for the shared error component, accessibility announcement, retry-time error retention, request cancellation/error ownership, data-service HttpContext opt-in, and slash/no-slash routing behavior.

Screenshots

[Copilot]

Pull request overview

Adds explicit not-found and load-failure states to experiment, feature-flag, and segment detail pages.

Changes:

• Introduces a reusable localized page-error component.
• Tracks route-specific loading errors in NgRx stores and supports retries.
• Adds ID validation, ID-less route handling, and scoped 404-toast suppression.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/frontend/projects/upgrade/src/assets/i18n/en.json	Adds error-page translations.
packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/index.ts	Exports the error component.
.../common-page-error/common-page-error.model.ts	Defines error models and ID validation.
.../common-page-error/common-page-error.model.spec.ts	Tests ID validation.
.../common-page-error/common-page-error.component.ts	Implements shared error UI logic.
.../common-page-error/common-page-error.component.scss	Styles the error screen.
.../common-page-error/common-page-error.component.html	Renders error actions and messaging.
.../segment-details-page-content.component.ts	Connects segment errors and retry.
.../segment-details-page-content.component.html	Prioritizes segment error rendering.
.../require-route-param.guard.ts	Redirects ID-less detail routes.
.../feature-flag-details-page-content.component.ts	Connects flag errors and retry.
.../feature-flag-details-page-content.component.html	Prioritizes flag error rendering.
.../experiment-details-page-content.component.ts	Connects experiment errors and retry.
.../experiment-details-page-content.component.html	Prioritizes experiment error rendering.
.../dashboard-routing.module.ts	Applies detail-route parameter guards.
.../segments/store/segments.selectors.ts	Selects route-scoped segment errors.
.../segments/store/segments.selectors.spec.ts	Tests segment error selection.
.../segments/store/segments.reducer.ts	Stores segment detail failures.
.../segments/store/segments.reducer.spec.ts	Tests segment error transitions.
.../segments/store/segments.model.ts	Extends segment state.
.../segments/store/segments.effects.ts	Classifies segment fetch failures.
.../segments/store/segments.effects.spec.ts	Tests segment failure branches.
.../segments/store/segments.actions.ts	Adds segment failure metadata.
.../segments/segments.service.ts	Exposes segment detail errors.
.../segments/segments.data.service.ts	Enables contextual 404 handling.
.../local-storage/local-storage.service.ts	Initializes new error state.
.../local-storage/local-storage.service.spec.ts	Updates persisted-state expectations.
.../http-interceptors/http-error.interceptor.ts	Suppresses opted-in 404 notifications.
.../http-interceptors/http-error.interceptor.spec.ts	Tests notification suppression.
.../http-interceptors/http-context-tokens.ts	Defines the 404 context token.
.../feature-flags/store/feature-flags.selectors.ts	Selects route-scoped flag errors.
.../feature-flags/store/feature-flags.selectors.spec.ts	Tests flag error selection.
.../feature-flags/store/feature-flags.reducer.ts	Stores flag detail failures.
.../feature-flags/store/feature-flags.model.ts	Extends feature-flag state.
.../feature-flags/store/feature-flags.effects.ts	Handles malformed, empty, and failed fetches.
.../feature-flags/store/feature-flags.effects.spec.ts	Tests flag fetch branches.
.../feature-flags/store/feature-flags.actions.ts	Adds flag failure metadata.
.../feature-flags/feature-flags.service.ts	Exposes flag detail errors.
.../feature-flags/feature-flags.data.service.ts	Enables contextual 404 handling.
.../experiments/store/experiments.selectors.ts	Selects route-scoped experiment errors.
.../experiments/store/experiments.selector.spec.ts	Tests experiment error selection.
.../experiments/store/experiments.reducer.ts	Stores experiment detail failures.
.../experiments/store/experiments.reducer.spec.ts	Tests experiment error transitions.
.../experiments/store/experiments.model.ts	Extends experiment state.
.../experiments/store/experiments.effects.ts	Handles fetch and auxiliary-stat failures.
.../experiments/store/experiments.effects.spec.ts	Tests experiment fetch branches.
.../experiments/store/experiments.actions.ts	Adds experiment failure metadata.
.../experiments/experiments.service.ts	Exposes experiment detail errors.
.../experiments/experiments.data.service.ts	Enables contextual 404 handling.
.../experiments/experiments.data.service.spec.ts	Tests the request context token.
packages/frontend/jest.config.js	Maps shared-component aliases in Jest.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:18

• This guard only runs after segments/detail/:segmentId matches. /segments/detail has no parameter segment, so it bypasses the guard and reaches the global ** redirect to /home instead of the Segments list. Add an explicit ID-less route before the parameterized route; keep this guard for the trailing-empty-segment case.
  packages/frontend/projects/upgrade/src/app/features/dashboard/dashboard-routing.module.ts:91
• This guard cannot handle /featureflags/detail because that URL does not match a route requiring :flagId. It falls through to the global wildcard and redirects to /home, contrary to the intended Feature Flags list destination. Add an explicit ID-less redirect before this parameterized route.
  packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts:89
• getExperimentById is also used by ExperimentService.selectExperimentById for the Preview User editor (preview-user.component.ts:156), where no page-level 404 UI exists. Setting this token unconditionally suppresses the only 404 feedback in that flow. Pass contextual handling from the details-page request (or split the details fetch) so non-details callers retain the generic notification.

 return this.http.get(url, { context: new HttpContext().set(HANDLES_404_CONTEXTUALLY, true) });

packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-page-error/common-page-error.component.html:1

• This error state replaces the spinner asynchronously, but the new content has no live-region semantics or focus change, so screen-reader users are not notified that loading finished with an error. Mark the inserted error container as an alert (or implement equivalent focus management) so the title and recovery actions are announced.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• detailsPageError is a single value, but this effect intentionally keeps requests for different IDs alive. After navigating from experiment A to B, if B fails and then A's older request fails, A overwrites B's error; the route-scoped selector hides A's error and B returns to the spinner this PR is meant to remove. Preserve concurrent preview-user requests without letting stale detail requests share this slot—for example, cancel any prior contextual details fetch when a newer contextual fetch starts, and ensure non-contextual/background failures do not write details-page error state (or key errors by entity ID).

 takeUntil(
this.actions$.pipe(
ofType(experimentAction.actionGetExperimentById),
filter((newerAction) => newerAction !== action && newerAction.experimentId === experimentId)
)

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

[danoswaltCL]

seems we might as well make the GET flags return a 404 instead of a 204 like the other ones, I don't think there should be a reason it's different.

[danoswaltCL]

@zackcl the PR to update feature-flags GET call to 404 has been merged into dev

[zackcl]

@bcb37@danoswaltCL Addressed the latest feedback: consolidated the duplicate error translations, clarified the not-found message with the current environment, and aligned feature flag handling with the backend’s 404 response. Feel free to take another look!