Auto merge of #4476 - Turbo87:crate-range-errors, r=Turbo87

crate.range: Show error page if data fails to load

Similar to the crate and version pages. This ensures that the URL stays the same so it can be fixed if there was a typo.

<img width="753" alt="Bildschirmfoto 2022-01-22 um 21 21 09" src="https://user-images.githubusercontent.com/141300/150654244-0825e7b5-f844-451b-bbf1-2b53d3708d22.png">

Author: bors

app/routes/crate/range.js

@@ -11,21 +11,26 @@ export default class VersionRoute extends Route {
   @service notifications;
   @service router;
 
-  async model({ range }) {
+  async model({ range }, transition) {
     let crate = this.modelFor('crate');
 
-    let versions = await crate.get('versions');
-    let allVersionNums = versions.map(it => it.num);
-    let unyankedVersionNums = versions.filter(it => !it.yanked).map(it => it.num);
+    try {
+      let versions = await crate.hasMany('versions').load();
+      let allVersionNums = versions.map(it => it.num);
+      let unyankedVersionNums = versions.filter(it => !it.yanked).map(it => it.num);
 
-    let npmRange = cargoRangeToNpm(range);
-    // find a version that matches the specified range
-    let versionNum = maxSatisfying(unyankedVersionNums, npmRange) ?? maxSatisfying(allVersionNums, npmRange);
-    if (!versionNum) {
-      this.notifications.error(`No matching version of crate '${crate.name}' found for: ${range}`);
-      this.router.replaceWith('crate.index');
+      let npmRange = cargoRangeToNpm(range);
+      // find a version that matches the specified range
+      let versionNum = maxSatisfying(unyankedVersionNums, npmRange) ?? maxSatisfying(allVersionNums, npmRange);
+      if (versionNum) {
+        this.router.replaceWith('crate.version', versionNum);
+      } else {
+        let title = `${crate.name}: No matching version found for ${range}`;
+        this.router.replaceWith('catch-all', { transition, title });
+      }
+    } catch (error) {
+      let title = `${crate.name}: Failed to load version data`;
+      this.router.replaceWith('catch-all', { transition, error, title, tryAgain: true });
     }
-
-    this.router.replaceWith('crate.version', versionNum);
   }
 }

tests/routes/crate/range-test.js

@@ -1,8 +1,10 @@
-import { currentURL, visit } from '@ember/test-helpers';
+import { currentURL } from '@ember/test-helpers';
 import { module, test } from 'qunit';
 
 import { setupApplicationTest } from 'cargo/tests/helpers';
 
+import { visit } from '../../helpers/visit-ignoring-abort';
+
 module('Route | crate.range', function (hooks) {
   setupApplicationTest(hooks);
 
@@ -76,17 +78,58 @@ module('Route | crate.range', function (hooks) {
     assert.dom('[data-test-notification-message]').doesNotExist();
   });
 
-  test('redirects to main crate page if no match found', async function (assert) {
+  test('shows an error page if crate not found', async function (assert) {
+    await visit('/crates/foo/range/^3');
+    assert.equal(currentURL(), '/crates/foo/range/%5E3');
+    assert.dom('[data-test-404-page]').exists();
+    assert.dom('[data-test-title]').hasText('Crate not found');
+    assert.dom('[data-test-go-back]').exists();
+    assert.dom('[data-test-try-again]').doesNotExist();
+  });
+
+  test('shows an error page if crate fails to load', async function (assert) {
+    this.server.get('/api/v1/crates/:crate_name', {}, 500);
+
+    await visit('/crates/foo/range/^3');
+    assert.equal(currentURL(), '/crates/foo/range/%5E3');
+    assert.dom('[data-test-404-page]').exists();
+    assert.dom('[data-test-title]').hasText('Crate failed to load');
+    assert.dom('[data-test-go-back]').doesNotExist();
+    assert.dom('[data-test-try-again]').exists();
+  });
+
+  test('shows an error page if no match found', async function (assert) {
     let crate = this.server.create('crate', { name: 'foo' });
     this.server.create('version', { crate, num: '1.0.0' });
     this.server.create('version', { crate, num: '1.1.0' });
     this.server.create('version', { crate, num: '1.1.1' });
     this.server.create('version', { crate, num: '2.0.0' });
 
     await visit('/crates/foo/range/^3');
-    assert.equal(currentURL(), `/crates/foo`);
-    assert.dom('[data-test-crate-name]').hasText('foo');
-    assert.dom('[data-test-crate-version]').hasText('2.0.0');
-    assert.dom('[data-test-notification-message="error"]').hasText("No matching version of crate 'foo' found for: ^3");
+    assert.equal(currentURL(), '/crates/foo/range/%5E3');
+    assert.dom('[data-test-404-page]').exists();
+    assert.dom('[data-test-title]').hasText('foo: No matching version found for ^3');
+    assert.dom('[data-test-go-back]').exists();
+    assert.dom('[data-test-try-again]').doesNotExist();
+  });
+
+  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: '3.2.1' });
+
+    this.server.get('/api/v1/crates/:crate_name/versions', {}, 500);
+
+    // Load `crate` and then explicitly unload the side-loaded `versions`.
+    let store = this.owner.lookup('service:store');
+    let crateRecord = await store.findRecord('crate', 'foo');
+    let versions = crateRecord.hasMany('versions').value();
+    versions.forEach(record => record.unloadRecord());
+
+    await visit('/crates/foo/range/^3');
+    assert.equal(currentURL(), '/crates/foo/range/%5E3');
+    assert.dom('[data-test-404-page]').exists();
+    assert.dom('[data-test-title]').hasText('foo: Failed to load version data');
+    assert.dom('[data-test-go-back]').doesNotExist();
+    assert.dom('[data-test-try-again]').exists();
   });
 });