Skip to content

Commit e77ae2a

Browse files
committed
feat(frontend): Implement handling for moved and split features
This commit introduces the frontend implementation to handle API responses for features that have been moved (301) or split (410). This builds upon previous backend changes that enabled the API to return these status codes. On the client side: - For a 301 response, the feature page now performs a client-side redirect. It catches a `FeatureMovedError`, updates its state with the new feature's data from the response body, and uses `history.pushState` to update the URL. This provides a smooth user experience without a full page reload. A notice is also displayed to inform the user of the redirect. - For a 410 response, the feature page redirects to a dedicated "Feature Gone" page, which explains that the feature was split and provides the user with links to the new features. As this is a Single Page Application (SPA), 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. Unit and end-to-end tests have been added to cover these new scenarios.
1 parent cea3dee commit e77ae2a

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)