From bfe8d72a836769f764969487a27484d9170e80a1 Mon Sep 17 00:00:00 2001
From: eth3lbert <eth3lbert+dev@gmail.com>
Date: Wed, 8 Jan 2025 20:23:18 +0800
Subject: [PATCH] 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.
---
 app/routes/crate/version-dependencies.js    | 31 +++++++++++++++------
 app/routes/crate/version.js                 | 25 +++++++++++++++--
 e2e/acceptance/crate-dependencies.spec.ts   |  2 +-
 tests/acceptance/crate-dependencies-test.js |  2 +-
 4 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/app/routes/crate/version-dependencies.js b/app/routes/crate/version-dependencies.js
index 75068fd7af4..94f5999f262 100644
--- a/app/routes/crate/version-dependencies.js
+++ b/app/routes/crate/version-dependencies.js
@@ -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 });
@@ -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'));
diff --git a/app/routes/crate/version.js b/app/routes/crate/version.js
index 683fbf8f3fd..d463978bea6 100644
--- a/app/routes/crate/version.js
+++ b/app/routes/crate/version.js
@@ -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();
@@ -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);
diff --git a/e2e/acceptance/crate-dependencies.spec.ts b/e2e/acceptance/crate-dependencies.spec.ts
index eb020ffa03f..65dd9870805 100644
--- a/e2e/acceptance/crate-dependencies.spec.ts
+++ b/e2e/acceptance/crate-dependencies.spec.ts
@@ -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);
     });
 
diff --git a/tests/acceptance/crate-dependencies-test.js b/tests/acceptance/crate-dependencies-test.js
index c009c89c223..859a5dc8bf4 100644
--- a/tests/acceptance/crate-dependencies-test.js
+++ b/tests/acceptance/crate-dependencies-test.js
@@ -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);