Skip to content

Commit

Permalink
routes/crate: Get or fetch requested version by num
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
eth3lbert committed Jan 8, 2025
1 parent 051960f commit ddfede2
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 13 deletions.
31 changes: 22 additions & 9 deletions app/routes/crate/version-dependencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,23 @@ import Route from '@ember/routing/route';
import { inject as service } from '@ember/service';

export default class VersionRoute extends Route {
@service store;
@service router;

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

let versions;
try {
versions = await crate.loadVersionsTask.perform();
} catch (error) {
let title = `${crate.name}: Failed to load version data`;
return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
}

let requestedVersion = params.version_num;
let version = versions.find(version => version.num === requestedVersion);
let version =
crate.loadedVersionsByNum.get(requestedVersion) ??
(await this.store
.queryRecord('version', {
name: crate.id,
num: requestedVersion,
})
.catch(() => {
// ignored
}));
if (!version) {
let title = `${crate.name}: Version ${requestedVersion} not found`;
return this.router.replaceWith('catch-all', { transition, title });
Expand All @@ -32,6 +34,17 @@ export default class VersionRoute extends Route {
return version;
}

async afterModel(_resolvedModel, transition) {
let crate = this.modelFor('crate');
// TODO: Resolved version without waiting for versions to be resolved
try {
await crate.loadVersionsTask.perform();
} catch (error) {
let title = `${crate.name}: Failed to load version data`;
return this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
}
}

setupController(controller, model) {
controller.set('version', model);
controller.set('crate', this.modelFor('crate'));
Expand Down
25 changes: 23 additions & 2 deletions app/routes/crate/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import { AjaxError } from '../../utils/ajax';
export default class VersionRoute extends Route {
@service router;
@service sentry;
@service store;

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

// TODO: Resolved version without waiting for versions to be resolved
let versions;
try {
versions = await crate.loadVersionsTask.perform();
Expand All @@ -25,14 +27,33 @@ export default class VersionRoute extends Route {
let version;
let requestedVersion = params.version_num;
if (requestedVersion) {
version = versions.find(version => version.num === requestedVersion);
version =
crate.loadedVersionsByNum.get(requestedVersion) ??
(await this.store
.queryRecord('version', {
name: crate.id,
num: requestedVersion,
})
.catch(() => {
// ignored
}));

if (!version) {
let title = `${crate.name}: Version ${requestedVersion} not found`;
return this.router.replaceWith('catch-all', { transition, title });
}
} else {
let { default_version } = crate;
version = versions.find(version => version.num === default_version);
version =
crate.loadedVersionsByNum.get(default_version) ??
(await this.store
.queryRecord('version', {
name: crate.id,
num: default_version,
})
.catch(() => {
// ignored
}));

if (!version) {
let versionNums = versions.map(it => it.num);
Expand Down
2 changes: 1 addition & 1 deletion e2e/acceptance/crate-dependencies.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ test.describe('Acceptance | crate dependencies page', { tag: '@acceptance' }, ()
test('shows an error page if versions fail to load', async ({ page, mirage, ember }) => {
await mirage.addHook(server => {
let crate = server.create('crate', { name: 'foo' });
server.create('version', { crate, num: '2.0.0' });
server.create('version', { crate, num: '1.0.0' });
server.get('/api/v1/crates/:crate_name/versions', {}, 500);
});

Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/crate-dependencies-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ module('Acceptance | crate dependencies page', function (hooks) {

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

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

Expand Down

0 comments on commit ddfede2

Please sign in to comment.