Skip to content

Commit f804fb0

Browse files
committed
feat(frontend): Handle moved and split features
This commit introduces client-side handling for features that have been moved (renamed) or split into multiple new features. The backend API (openapi.yaml) is updated to respond with: - 301 Moved Permanently when a feature has been moved. The response body contains the data for the new feature. - 410 Gone when a feature has been split. The response body contains a list of the new feature IDs. On the client side: - For a 301 response, the feature page performs a client-side redirect. It catches the FeatureMovedError, updates its internal state with the new feature's data, and uses history.pushState to update the URL. This avoids a full page reload and provides a smoother user experience. A notice is displayed to inform the user they have been redirected. - For a 410 response, the feature page redirects to a new "Feature Gone" page. This page explains that the feature has been split and provides links to the new features. As this is a Single Page Application (SPA), the initial page load for a feature URL always serves the main index.html. The client-side fetch determines whether a redirect or a "gone" page is necessary. This approach is a trade-off for better UX within the SPA architecture, though it differs from traditional server-side redirect handling for SEO. Unit and end-to-end tests have been added to cover these new scenarios.
1 parent cea3dee commit f804fb0

15 files changed

+604
-10
lines changed

e2e/tests/feature-page.spec.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,51 @@ test('date range changes are preserved in the URL', async ({page}) => {
169169
const endDateInputElement3 = endDateSelector3.locator('input');
170170
expect(await endDateInputElement3.inputValue()).toBe(endDate);
171171
});
172+
173+
test('redirects for a moved feature', async ({page}) => {
174+
await page.goto('http://localhost:5555/features/old-feature');
175+
176+
// Expect the URL to be updated to the new feature's URL.
177+
await expect(page).toHaveURL(
178+
'http://localhost:5555/features/new-feature?redirected_from=old-feature',
179+
);
180+
181+
// Expect the title and redirect banner to be correct.
182+
await expect(page.locator('h1')).toHaveText('New Feature');
183+
await expect(
184+
page.locator(
185+
'sl-alert:has-text("You have been redirected from an old feature ID")',
186+
),
187+
).toBeVisible();
188+
189+
// Wait for charts to load to avoid flakiness in the screenshot.
190+
await page.waitForSelector('#feature-wpt-implementation-progress-0-complete');
191+
192+
// Take a screenshot for visual verification.
193+
const pageContainer = page.locator('.page-container');
194+
await expect(pageContainer).toHaveScreenshot();
195+
});
196+
197+
test('shows gone page for a split feature', async ({page}) => {
198+
await page.goto('http://localhost:5555/features/before-split-feature');
199+
200+
// Expect to be redirected to the 'feature-gone-split' page.
201+
await expect(page).toHaveURL(
202+
'http://localhost:5555/errors-410/feature-gone-split?new_features=after-split-feature-1,after-split-feature-2',
203+
);
204+
205+
// Assert that the content of the 410 page is correct.
206+
await expect(page.locator('.new-results-header')).toContainText(
207+
'Please see the following new features',
208+
);
209+
await expect(
210+
page.locator('a[href="/features/after-split-feature-1"]'),
211+
).toBeVisible();
212+
await expect(
213+
page.locator('a[href="/features/after-split-feature-2"]'),
214+
).toBeVisible();
215+
216+
// Take a screenshot for visual verification.
217+
const pageContainer = page.locator('.container'); // Assuming a generic container for the error page.
218+
await expect(pageContainer).toHaveScreenshot();
219+
});
230 KB
Loading
332 KB
Loading
262 KB
Loading
47.7 KB
Loading
69.7 KB
Loading
58.2 KB
Loading

frontend/nginx.conf

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ http {
5353
try_files $uri /index.html;
5454
}
5555

56+
location = /errors-410/feature-gone-split {
57+
try_files $uri /index.html;
58+
}
59+
5660
# Security headers
5761
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload";
5862
add_header X-Content-Type-Options "nosniff";

frontend/src/static/js/api/client.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ import createClient, {
2121
ParseAsResponse,
2222
} from 'openapi-fetch';
2323
import {type components, type paths} from 'webstatus.dev-backend';
24-
import {createAPIError} from './errors.js';
24+
import {
25+
createAPIError,
26+
FeatureGoneSplitError,
27+
FeatureMovedError,
28+
} from './errors.js';
2529

2630
import {
2731
MediaType,
@@ -302,17 +306,37 @@ export class APIClient {
302306
const qsParams: paths['/v1/features/{feature_id}']['get']['parameters']['query'] =
303307
{};
304308
if (wptMetricView) qsParams.wpt_metric_view = wptMetricView;
305-
const {data, error} = await this.client.GET('/v1/features/{feature_id}', {
309+
const resp = await this.client.GET('/v1/features/{feature_id}', {
306310
...temporaryFetchOptions,
307311
params: {
308312
path: {feature_id: featureId},
309313
query: qsParams,
310314
},
311315
});
312-
if (error !== undefined) {
313-
throw createAPIError(error);
316+
if (resp.error !== undefined) {
317+
const data = resp.error;
318+
if (resp.response.status === 410 && 'new_features' in data) {
319+
// Type narrowing doesn't work.
320+
// https://github.com/openapi-ts/openapi-typescript/issues/1723
321+
// We have to force it.
322+
const featureGoneData =
323+
data as components['schemas']['FeatureGoneError'];
324+
throw new FeatureGoneSplitError(
325+
resp.error.message,
326+
featureGoneData.new_features.map(f => f.id),
327+
);
328+
}
329+
throw createAPIError(resp.error);
314330
}
315-
return data;
331+
if (resp.response.redirected) {
332+
const featureId = resp.response.url.split('/').pop() || '';
333+
throw new FeatureMovedError(
334+
'redirected to feature',
335+
featureId,
336+
resp.data,
337+
);
338+
}
339+
return resp.data;
316340
}
317341

318342
public async getFeatureMetadata(

frontend/src/static/js/api/errors.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import {type components} from 'webstatus.dev-backend';
2+
13
/**
24
* Copyright 2024 Google LLC
35
*
@@ -106,3 +108,27 @@ export class UnknownError extends ApiError {
106108
this.name = 'UnknownError';
107109
}
108110
}
111+
112+
export class FeatureGoneSplitError extends ApiError {
113+
newFeatureIds: Array<string>;
114+
constructor(message: string, newFeatureIds: Array<string>) {
115+
super(message, 410);
116+
this.name = 'FeatureGoneSplitError';
117+
this.newFeatureIds = newFeatureIds;
118+
}
119+
}
120+
121+
export class FeatureMovedError extends ApiError {
122+
newFeatureId: string;
123+
feature: components['schemas']['Feature'];
124+
constructor(
125+
message: string,
126+
newFeatureId: string,
127+
feature: components['schemas']['Feature'],
128+
) {
129+
super(message, 301);
130+
this.name = 'FeatureMovedError';
131+
this.newFeatureId = newFeatureId;
132+
this.feature = feature;
133+
}
134+
}

0 commit comments

Comments
 (0)