Skip to content

Commit 52f086f

Browse files
committed
crate.version: Use defaultVersion to simplify the model() hook
1 parent 9816e49 commit 52f086f

File tree

2 files changed

+16
-53
lines changed

2 files changed

+16
-53
lines changed

app/routes/crate/version.js

+12-47
Original file line numberDiff line numberDiff line change
@@ -2,65 +2,30 @@ import Route from '@ember/routing/route';
22
import { inject as service } from '@ember/service';
33

44
import * as Sentry from '@sentry/browser';
5-
import prerelease from 'semver/functions/prerelease';
65

76
import { AjaxError } from '../../utils/ajax';
87

9-
function isUnstableVersion(version) {
10-
return !!prerelease(version);
11-
}
12-
138
export default class VersionRoute extends Route {
149
@service notifications;
1510

1611
async model(params) {
17-
const requestedVersion = params.version_num;
18-
const crate = this.modelFor('crate');
19-
const maxVersion = crate.max_version;
20-
12+
let crate = this.modelFor('crate');
2113
let versions = await crate.get('versions');
2214

23-
// Fallback to the crate's last stable version
24-
// If `max_version` is `0.0.0` then all versions have been yanked
25-
if (!params.version_num && maxVersion !== '0.0.0') {
26-
if (isUnstableVersion(maxVersion)) {
27-
// Find the latest version that is stable AND not-yanked.
28-
const latestStableVersion = versions.find(version => !isUnstableVersion(version.num) && !version.yanked);
29-
30-
if (latestStableVersion == null) {
31-
// Cannot find any version that is stable AND not-yanked.
32-
// The fact that "maxVersion" itself cannot be found means that
33-
// we have to fall back to the latest one that is unstable....
34-
35-
// Find the latest version that not yanked.
36-
const latestUnyankedVersion = versions.find(version => !version.yanked);
37-
38-
if (latestUnyankedVersion == null) {
39-
// There's not even any unyanked version...
40-
params.version_num = maxVersion;
41-
} else {
42-
params.version_num = latestUnyankedVersion.num;
43-
}
44-
} else {
45-
params.version_num = latestStableVersion.num;
46-
}
47-
} else {
48-
params.version_num = maxVersion;
15+
let version;
16+
let requestedVersion = params.version_num;
17+
if (requestedVersion) {
18+
version = versions.find(version => version.num === requestedVersion);
19+
if (!version) {
20+
this.notifications.error(`Version '${requestedVersion}' of crate '${crate.name}' does not exist`);
21+
this.replaceWith('crate.index');
4922
}
23+
} else {
24+
let { defaultVersion } = crate;
25+
version = versions.find(version => version.num === defaultVersion) ?? versions.lastObject;
5026
}
5127

52-
const version = versions.find(version => version.num === params.version_num);
53-
if (params.version_num && !version) {
54-
this.notifications.error(`Version '${params.version_num}' of crate '${crate.name}' does not exist`);
55-
this.replaceWith('crate.index');
56-
return;
57-
}
58-
59-
return {
60-
crate,
61-
requestedVersion,
62-
version: version || versions.find(version => version.num === maxVersion) || versions.objectAt(0),
63-
};
28+
return { crate, requestedVersion, version };
6429
}
6530

6631
setupController(controller, model) {

tests/routes/crate/version/model-test.js

+4-6
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ module('Route | crate.version | model() hook', function (hooks) {
6464
assert.dom('[data-test-notification-message]').doesNotExist();
6565
});
6666

67-
test('defaults to the first not-yanked version', async function (assert) {
67+
test('defaults to the highest not-yanked version', async function (assert) {
6868
let crate = this.server.create('crate', { name: 'foo' });
6969
this.server.create('version', { crate, num: '1.0.0', yanked: true });
7070
this.server.create('version', { crate, num: '1.2.3', yanked: true });
@@ -75,12 +75,11 @@ module('Route | crate.version | model() hook', function (hooks) {
7575
await visit('/crates/foo');
7676
assert.equal(currentURL(), `/crates/foo`);
7777
assert.dom('[data-test-crate-name]').hasText('foo');
78-
// TODO this should default to the highest not-yanked version instead
79-
assert.dom('[data-test-crate-version]').hasText('2.0.0-beta.1');
78+
assert.dom('[data-test-crate-version]').hasText('2.0.0-beta.2');
8079
assert.dom('[data-test-notification-message]').doesNotExist();
8180
});
8281

83-
test('if there are only yanked versions, it defaults to the first version', async function (assert) {
82+
test('if there are only yanked versions, it defaults to the latest version', async function (assert) {
8483
let crate = this.server.create('crate', { name: 'foo' });
8584
this.server.create('version', { crate, num: '1.0.0', yanked: true });
8685
this.server.create('version', { crate, num: '1.2.3', yanked: true });
@@ -89,8 +88,7 @@ module('Route | crate.version | model() hook', function (hooks) {
8988
await visit('/crates/foo');
9089
assert.equal(currentURL(), `/crates/foo`);
9190
assert.dom('[data-test-crate-name]').hasText('foo');
92-
// TODO we should probably default to the highest version instead
93-
assert.dom('[data-test-crate-version]').hasText('1.0.0');
91+
assert.dom('[data-test-crate-version]').hasText('2.0.0-beta.1');
9492
assert.dom('[data-test-notification-message]').doesNotExist();
9593
});
9694
});

0 commit comments

Comments
 (0)