Skip to content

Commit 0e6d1a4

Browse files
authored
Merge pull request #9900 from Turbo87/404-pages
Show error page if team or user is missing or fails to load
2 parents 35876f8 + 9d6f105 commit 0e6d1a4

File tree

6 files changed

+122
-10
lines changed

6 files changed

+122
-10
lines changed

app/routes/team.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export default class TeamRoute extends Route {
1212
sort: { refreshModel: true },
1313
};
1414

15-
async model(params) {
15+
async model(params, transition) {
1616
const { team_id } = params;
1717

1818
try {
@@ -25,11 +25,12 @@ export default class TeamRoute extends Route {
2525
return { crates, team };
2626
} catch (error) {
2727
if (error instanceof NotFoundError) {
28-
this.notifications.error(`Team '${params.team_id}' does not exist`);
29-
return this.router.replaceWith('index');
28+
let title = `${team_id}: Team not found`;
29+
this.router.replaceWith('catch-all', { transition, error, title });
30+
} else {
31+
let title = `${team_id}: Failed to load team data`;
32+
this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
3033
}
31-
32-
throw error;
3334
}
3435
}
3536
}

app/routes/user.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export default class UserRoute extends Route {
1212
sort: { refreshModel: true },
1313
};
1414

15-
async model(params) {
15+
async model(params, transition) {
1616
const { user_id } = params;
1717
try {
1818
let user = await this.store.queryRecord('user', { user_id });
@@ -24,11 +24,12 @@ export default class UserRoute extends Route {
2424
return { crates, user };
2525
} catch (error) {
2626
if (error instanceof NotFoundError) {
27-
this.notifications.error(`User '${params.user_id}' does not exist`);
28-
return this.router.replaceWith('index');
27+
let title = `${user_id}: User not found`;
28+
this.router.replaceWith('catch-all', { transition, error, title });
29+
} else {
30+
let title = `${user_id}: Failed to load user data`;
31+
this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
2932
}
30-
31-
throw error;
3233
}
3334
}
3435
}

e2e/routes/team.spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { expect, test } from '@/e2e/helper';
2+
3+
test.describe('Route | team', { tag: '@routes' }, () => {
4+
test("shows an error message if the category can't be found", async ({ page }) => {
5+
await page.goto('/teams/foo');
6+
await expect(page).toHaveURL('/teams/foo');
7+
await expect(page.locator('[data-test-404-page]')).toBeVisible();
8+
await expect(page.locator('[data-test-title]')).toHaveText('foo: Team not found');
9+
await expect(page.locator('[data-test-go-back]')).toBeVisible();
10+
await expect(page.locator('[data-test-try-again]')).toHaveCount(0);
11+
});
12+
13+
test('server error causes the error page to be shown', async ({ page, mirage }) => {
14+
await mirage.addHook(server => {
15+
server.get('/api/v1/teams/:id', {}, 500);
16+
});
17+
18+
await page.goto('/teams/foo');
19+
await expect(page).toHaveURL('/teams/foo');
20+
await expect(page.locator('[data-test-404-page]')).toBeVisible();
21+
await expect(page.locator('[data-test-title]')).toHaveText('foo: Failed to load team data');
22+
await expect(page.locator('[data-test-go-back]')).toHaveCount(0);
23+
await expect(page.locator('[data-test-try-again]')).toBeVisible();
24+
});
25+
});

e2e/routes/user.spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { expect, test } from '@/e2e/helper';
2+
3+
test.describe('Route | user', { tag: '@routes' }, () => {
4+
test("shows an error message if the category can't be found", async ({ page }) => {
5+
await page.goto('/users/foo');
6+
await expect(page).toHaveURL('/users/foo');
7+
await expect(page.locator('[data-test-404-page]')).toBeVisible();
8+
await expect(page.locator('[data-test-title]')).toHaveText('foo: User not found');
9+
await expect(page.locator('[data-test-go-back]')).toBeVisible();
10+
await expect(page.locator('[data-test-try-again]')).toHaveCount(0);
11+
});
12+
13+
test('server error causes the error page to be shown', async ({ page, mirage }) => {
14+
await mirage.addHook(server => {
15+
server.get('/api/v1/users/:id', {}, 500);
16+
});
17+
18+
await page.goto('/users/foo');
19+
await expect(page).toHaveURL('/users/foo');
20+
await expect(page.locator('[data-test-404-page]')).toBeVisible();
21+
await expect(page.locator('[data-test-title]')).toHaveText('foo: Failed to load user data');
22+
await expect(page.locator('[data-test-go-back]')).toHaveCount(0);
23+
await expect(page.locator('[data-test-try-again]')).toBeVisible();
24+
});
25+
});

tests/routes/team-test.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { currentURL } from '@ember/test-helpers';
2+
import { module, test } from 'qunit';
3+
4+
import { setupApplicationTest } from 'crates-io/tests/helpers';
5+
6+
import { visit } from '../helpers/visit-ignoring-abort';
7+
8+
module('Route | team', function (hooks) {
9+
setupApplicationTest(hooks);
10+
11+
test("shows an error message if the user can't be found", async function (assert) {
12+
await visit('/teams/foo');
13+
assert.strictEqual(currentURL(), '/teams/foo');
14+
assert.dom('[data-test-404-page]').exists();
15+
assert.dom('[data-test-title]').hasText('foo: Team not found');
16+
assert.dom('[data-test-go-back]').exists();
17+
assert.dom('[data-test-try-again]').doesNotExist();
18+
});
19+
20+
test('server error causes the error page to be shown', async function (assert) {
21+
this.server.get('/api/v1/teams/:id', {}, 500);
22+
23+
await visit('/teams/foo');
24+
assert.strictEqual(currentURL(), '/teams/foo');
25+
assert.dom('[data-test-404-page]').exists();
26+
assert.dom('[data-test-title]').hasText('foo: Failed to load team data');
27+
assert.dom('[data-test-go-back]').doesNotExist();
28+
assert.dom('[data-test-try-again]').exists();
29+
});
30+
});

tests/routes/user-test.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { currentURL } from '@ember/test-helpers';
2+
import { module, test } from 'qunit';
3+
4+
import { setupApplicationTest } from 'crates-io/tests/helpers';
5+
6+
import { visit } from '../helpers/visit-ignoring-abort';
7+
8+
module('Route | user', function (hooks) {
9+
setupApplicationTest(hooks);
10+
11+
test("shows an error message if the user can't be found", async function (assert) {
12+
await visit('/users/foo');
13+
assert.strictEqual(currentURL(), '/users/foo');
14+
assert.dom('[data-test-404-page]').exists();
15+
assert.dom('[data-test-title]').hasText('foo: User not found');
16+
assert.dom('[data-test-go-back]').exists();
17+
assert.dom('[data-test-try-again]').doesNotExist();
18+
});
19+
20+
test('server error causes the error page to be shown', async function (assert) {
21+
this.server.get('/api/v1/users/:id', {}, 500);
22+
23+
await visit('/users/foo');
24+
assert.strictEqual(currentURL(), '/users/foo');
25+
assert.dom('[data-test-404-page]').exists();
26+
assert.dom('[data-test-title]').hasText('foo: Failed to load user data');
27+
assert.dom('[data-test-go-back]').doesNotExist();
28+
assert.dom('[data-test-try-again]').exists();
29+
});
30+
});

0 commit comments

Comments
 (0)