Skip to content

Commit 1644694

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 529f8a3 commit 1644694

File tree

4 files changed

+39
-56
lines changed

4 files changed

+39
-56
lines changed

app/routes/crate/version-dependencies.js

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,31 @@
1+
import { NotFoundError } from '@ember-data/adapter/error';
12
import Route from '@ember/routing/route';
23
import { inject as service } from '@ember/service';
34

45
export default class VersionRoute extends Route {
6+
@service store;
57
@service router;
68

79
async model(params, transition) {
810
let crate = this.modelFor('crate');
911

10-
let versions;
12+
let requestedVersion = params.version_num;
13+
let version;
1114
try {
12-
versions = await crate.loadVersionsTask.perform();
15+
version =
16+
crate.loadedVersionsByNum.get(requestedVersion) ??
17+
(await this.store.queryRecord('version', {
18+
name: crate.id,
19+
num: requestedVersion,
20+
}));
1321
} 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-
18-
let requestedVersion = params.version_num;
19-
let version = versions.find(version => version.num === requestedVersion);
20-
if (!version) {
21-
let title = `${crate.name}: Version ${requestedVersion} not found`;
22-
return this.router.replaceWith('catch-all', { transition, title });
22+
if (error instanceof NotFoundError) {
23+
let title = `${crate.name}: Version ${requestedVersion} not found`;
24+
return this.router.replaceWith('catch-all', { transition, title });
25+
} else {
26+
let title = `${crate.name}: Failed to load version data`;
27+
return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
28+
}
2329
}
2430

2531
try {

app/routes/crate/version.js

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,49 @@
1+
import { NotFoundError } from '@ember-data/adapter/error';
12
import Route from '@ember/routing/route';
23
import { inject as service } from '@ember/service';
34
import { waitForPromise } from '@ember/test-waiters';
45

56
import { didCancel } from 'ember-concurrency';
6-
import semverSort from 'semver/functions/rsort';
77

88
import { AjaxError } from '../../utils/ajax';
99

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

17-
let versions;
18+
// TODO: Resolved version without waiting for versions to be resolved
1819
try {
19-
versions = await crate.loadVersionsTask.perform();
20+
await crate.loadVersionsTask.perform();
2021
} catch (error) {
2122
let title = `${crate.name}: Failed to load version data`;
2223
return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
2324
}
2425

2526
let version;
2627
let requestedVersion = params.version_num;
27-
if (requestedVersion) {
28-
version = versions.find(version => version.num === requestedVersion);
29-
if (!version) {
30-
let title = `${crate.name}: Version ${requestedVersion} not found`;
31-
return this.router.replaceWith('catch-all', { transition, title });
32-
}
33-
} else {
34-
let { default_version } = crate;
35-
version = versions.find(version => version.num === default_version);
36-
37-
if (!version) {
38-
let versionNums = versions.map(it => it.num);
39-
semverSort(versionNums, { loose: true });
28+
let num = requestedVersion || crate.default_version;
4029

41-
version = versions.find(version => version.num === versionNums[0]);
30+
try {
31+
version =
32+
crate.loadedVersionsByNum.get(num) ??
33+
(await crate.store.queryRecord('version', {
34+
name: crate.id,
35+
num,
36+
}));
37+
} catch (error) {
38+
if (error instanceof NotFoundError) {
39+
let title =
40+
requestedVersion == null
41+
? `${crate.name}: Failed to find default version`
42+
: `${crate.name}: Version ${requestedVersion} not found`;
43+
return this.router.replaceWith('catch-all', { transition, title });
44+
} else {
45+
let title = `${crate.name}: Failed to load version data`;
46+
return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
4247
}
4348
}
4449

e2e/acceptance/crate-dependencies.spec.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,20 +62,6 @@ test.describe('Acceptance | crate dependencies page', { tag: '@acceptance' }, ()
6262
await expect(page.locator('[data-test-try-again]')).toHaveCount(0);
6363
});
6464

65-
test('shows an error page if versions fail to load', async ({ page, msw, ember }) => {
66-
let crate = msw.db.crate.create({ name: 'foo' });
67-
msw.db.version.create({ crate, num: '2.0.0' });
68-
await msw.worker.use(http.get('/api/v1/crates/:crate_name/versions', () => HttpResponse.json({}, { status: 500 })));
69-
70-
await page.goto('/crates/foo/1.0.0/dependencies');
71-
72-
await expect(page).toHaveURL('/crates/foo/1.0.0/dependencies');
73-
await expect(page.locator('[data-test-404-page]')).toBeVisible();
74-
await expect(page.locator('[data-test-title]')).toHaveText('foo: Failed to load version data');
75-
await expect(page.locator('[data-test-go-back]')).toHaveCount(0);
76-
await expect(page.locator('[data-test-try-again]')).toBeVisible();
77-
});
78-
7965
test('shows error message if loading of dependencies fails', async ({ page, msw }) => {
8066
let crate = msw.db.crate.create({ name: 'foo' });
8167
msw.db.version.create({ crate, num: '1.0.0' });

tests/acceptance/crate-dependencies-test.js

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -74,20 +74,6 @@ module('Acceptance | crate dependencies page', function (hooks) {
7474
assert.dom('[data-test-try-again]').doesNotExist();
7575
});
7676

77-
test('shows an error page if versions fail to load', async function (assert) {
78-
let crate = this.db.crate.create({ name: 'foo' });
79-
this.db.version.create({ crate, num: '2.0.0' });
80-
81-
this.worker.use(http.get('/api/v1/crates/:crate_name/versions', () => HttpResponse.json({}, { status: 500 })));
82-
83-
await visit('/crates/foo/1.0.0/dependencies');
84-
assert.strictEqual(currentURL(), '/crates/foo/1.0.0/dependencies');
85-
assert.dom('[data-test-404-page]').exists();
86-
assert.dom('[data-test-title]').hasText('foo: Failed to load version data');
87-
assert.dom('[data-test-go-back]').doesNotExist();
88-
assert.dom('[data-test-try-again]').exists();
89-
});
90-
9177
test('shows error message if loading of dependencies fails', async function (assert) {
9278
let crate = this.db.crate.create({ name: 'foo' });
9379
this.db.version.create({ crate, num: '1.0.0' });

0 commit comments

Comments
 (0)