Skip to content

Commit ddfede2

Browse files
committed
routes/crate: Get or fetch requested version by num
Previously, we fetched all versions and then found the requested version within them. This commit changes the approach to either retrieve the version from loaded versions or fetch it from the endpoint. This change allows us to migrate to paginated versions in the future.
1 parent 051960f commit ddfede2

File tree

4 files changed

+47
-13
lines changed

4 files changed

+47
-13
lines changed

app/routes/crate/version-dependencies.js

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,23 @@ import Route from '@ember/routing/route';
22
import { inject as service } from '@ember/service';
33

44
export default class VersionRoute extends Route {
5+
@service store;
56
@service router;
67

78
async model(params, transition) {
89
let crate = this.modelFor('crate');
910

10-
let versions;
11-
try {
12-
versions = await crate.loadVersionsTask.perform();
13-
} catch (error) {
14-
let title = `${crate.name}: Failed to load version data`;
15-
return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
16-
}
17-
1811
let requestedVersion = params.version_num;
19-
let version = versions.find(version => version.num === requestedVersion);
12+
let version =
13+
crate.loadedVersionsByNum.get(requestedVersion) ??
14+
(await this.store
15+
.queryRecord('version', {
16+
name: crate.id,
17+
num: requestedVersion,
18+
})
19+
.catch(() => {
20+
// ignored
21+
}));
2022
if (!version) {
2123
let title = `${crate.name}: Version ${requestedVersion} not found`;
2224
return this.router.replaceWith('catch-all', { transition, title });
@@ -32,6 +34,17 @@ export default class VersionRoute extends Route {
3234
return version;
3335
}
3436

37+
async afterModel(_resolvedModel, transition) {
38+
let crate = this.modelFor('crate');
39+
// TODO: Resolved version without waiting for versions to be resolved
40+
try {
41+
await crate.loadVersionsTask.perform();
42+
} catch (error) {
43+
let title = `${crate.name}: Failed to load version data`;
44+
return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
45+
}
46+
}
47+
3548
setupController(controller, model) {
3649
controller.set('version', model);
3750
controller.set('crate', this.modelFor('crate'));

app/routes/crate/version.js

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ import { AjaxError } from '../../utils/ajax';
1010
export default class VersionRoute extends Route {
1111
@service router;
1212
@service sentry;
13+
@service store;
1314

1415
async model(params, transition) {
1516
let crate = this.modelFor('crate');
1617

18+
// TODO: Resolved version without waiting for versions to be resolved
1719
let versions;
1820
try {
1921
versions = await crate.loadVersionsTask.perform();
@@ -25,14 +27,33 @@ export default class VersionRoute extends Route {
2527
let version;
2628
let requestedVersion = params.version_num;
2729
if (requestedVersion) {
28-
version = versions.find(version => version.num === requestedVersion);
30+
version =
31+
crate.loadedVersionsByNum.get(requestedVersion) ??
32+
(await this.store
33+
.queryRecord('version', {
34+
name: crate.id,
35+
num: requestedVersion,
36+
})
37+
.catch(() => {
38+
// ignored
39+
}));
40+
2941
if (!version) {
3042
let title = `${crate.name}: Version ${requestedVersion} not found`;
3143
return this.router.replaceWith('catch-all', { transition, title });
3244
}
3345
} else {
3446
let { default_version } = crate;
35-
version = versions.find(version => version.num === default_version);
47+
version =
48+
crate.loadedVersionsByNum.get(default_version) ??
49+
(await this.store
50+
.queryRecord('version', {
51+
name: crate.id,
52+
num: default_version,
53+
})
54+
.catch(() => {
55+
// ignored
56+
}));
3657

3758
if (!version) {
3859
let versionNums = versions.map(it => it.num);

e2e/acceptance/crate-dependencies.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ test.describe('Acceptance | crate dependencies page', { tag: '@acceptance' }, ()
7171
test('shows an error page if versions fail to load', async ({ page, mirage, ember }) => {
7272
await mirage.addHook(server => {
7373
let crate = server.create('crate', { name: 'foo' });
74-
server.create('version', { crate, num: '2.0.0' });
74+
server.create('version', { crate, num: '1.0.0' });
7575
server.get('/api/v1/crates/:crate_name/versions', {}, 500);
7676
});
7777

tests/acceptance/crate-dependencies-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ module('Acceptance | crate dependencies page', function (hooks) {
7474

7575
test('shows an error page if versions fail to load', async function (assert) {
7676
let crate = this.server.create('crate', { name: 'foo' });
77-
this.server.create('version', { crate, num: '2.0.0' });
77+
this.server.create('version', { crate, num: '1.0.0' });
7878

7979
this.server.get('/api/v1/crates/:crate_name/versions', {}, 500);
8080

0 commit comments

Comments
 (0)