Skip to content

Commit fbe6234

Browse files
committed
Fix non-canonical crate name pages
Before the Ember Data 5 update it was possible to visit `/crates/foo-bar` when the crate was actually called `foo_bar` and vice-versa. The Ember Data 5 update broke this behavior, and this commit is restoring it, by using `queryRecord()` instead of `findRecord()`, which can no longer deal with requesting a certain `id` and getting a different one returned from the server.
1 parent 94abc3f commit fbe6234

File tree

4 files changed

+54
-7
lines changed

4 files changed

+54
-7
lines changed

app/adapters/crate.js

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,23 @@ export default class CrateAdapter extends ApplicationAdapter {
66
coalesceFindRequests = true;
77

88
findRecord(store, type, id, snapshot) {
9-
let { include } = snapshot;
10-
// This ensures `crate.versions` are always fetched from another request.
11-
if (include === undefined) {
12-
snapshot.include = 'keywords,categories,downloads';
9+
return super.findRecord(store, type, id, setDefaultInclude(snapshot));
10+
}
11+
12+
queryRecord(store, type, query, adapterOptions) {
13+
return super.queryRecord(store, type, setDefaultInclude(query), adapterOptions);
14+
}
15+
16+
/** Removes the `name` query parameter and turns it into a path parameter instead */
17+
urlForQueryRecord(query) {
18+
let baseUrl = super.urlForQueryRecord(...arguments);
19+
if (!query.name) {
20+
return baseUrl;
1321
}
14-
return super.findRecord(store, type, id, snapshot);
22+
23+
let crateName = query.name;
24+
delete query.name;
25+
return `${baseUrl}/${crateName}`;
1526
}
1627

1728
groupRecordsForFindMany(store, snapshots) {
@@ -22,3 +33,12 @@ export default class CrateAdapter extends ApplicationAdapter {
2233
return result;
2334
}
2435
}
36+
37+
function setDefaultInclude(query) {
38+
if (query.include === undefined) {
39+
// This ensures `crate.versions` are always fetched from another request.
40+
query.include = 'keywords,categories,downloads';
41+
}
42+
43+
return query;
44+
}

app/routes/crate.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export default class CrateRoute extends Route {
1212
let crateName = params.crate_id;
1313

1414
try {
15-
return await this.store.findRecord('crate', crateName);
15+
return this.store.peekRecord('crate', crateName) || (await this.store.queryRecord('crate', { name: crateName }));
1616
} catch (error) {
1717
if (error instanceof NotFoundError) {
1818
let title = `${crateName}: Crate not found`;

e2e/acceptance/crate.spec.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { test, expect } from '@/e2e/helper';
1+
import { expect, test } from '@/e2e/helper';
22

33
test.describe('Acceptance | crate page', { tag: '@acceptance' }, () => {
44
test('visiting a crate page from the front page', async ({ page, mirage }) => {
@@ -139,6 +139,20 @@ test.describe('Acceptance | crate page', { tag: '@acceptance' }, () => {
139139
await expect(page.locator('[data-test-try-again]')).toBeVisible();
140140
});
141141

142+
test('works for non-canonical names', async ({ page, mirage }) => {
143+
await mirage.addHook(server => {
144+
let crate = server.create('crate', { name: 'foo-bar' });
145+
server.create('version', { crate });
146+
});
147+
148+
await page.goto('/crates/foo_bar');
149+
150+
await expect(page).toHaveURL('/crates/foo_bar');
151+
await expect(page).toHaveTitle('foo-bar - crates.io: Rust Package Registry');
152+
153+
await expect(page.locator('[data-test-heading] [data-test-crate-name]')).toHaveText('foo-bar');
154+
});
155+
142156
test('navigating to the all versions page', async ({ page, mirage }) => {
143157
await mirage.addHook(server => {
144158
server.loadFixtures();

tests/acceptance/crate-test.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,19 @@ module('Acceptance | crate page', function (hooks) {
131131
assert.dom('[data-test-try-again]').exists();
132132
});
133133

134+
test('works for non-canonical names', async function (assert) {
135+
let crate = this.server.create('crate', { name: 'foo-bar' });
136+
this.server.create('version', { crate });
137+
138+
await visit('/crates/foo_bar');
139+
140+
assert.strictEqual(currentURL(), '/crates/foo_bar');
141+
assert.strictEqual(currentRouteName(), 'crate.index');
142+
assert.strictEqual(getPageTitle(), 'foo-bar - crates.io: Rust Package Registry');
143+
144+
assert.dom('[data-test-heading] [data-test-crate-name]').hasText('foo-bar');
145+
});
146+
134147
test('navigating to the all versions page', async function (assert) {
135148
this.server.loadFixtures();
136149

0 commit comments

Comments
 (0)