From 579691ac772971c754d3b15704a2c9e3efc7db0d Mon Sep 17 00:00:00 2001
From: Michael Lange <dingoeatingfuzz@gmail.com>
Date: Sun, 25 Oct 2020 22:17:41 -0700
Subject: [PATCH] Always show the file browser for allocations and tasks.

Before, we'd show a helpful error message when a task isn't running
instead of erroring in a generic way. Turns out when an alloc is
terminal but reachable, the filesystem is left behind so we were hiding
it.

Now it is always shown and in the event that something errors, it'll
either be generic, or--more commonly--a 404 of the allocation.
---
 ui/app/routes/allocations/allocation/fs.js    |  7 +-
 .../routes/allocations/allocation/task/fs.js  | 11 +--
 ui/app/templates/components/fs/browser.hbs    | 73 ++++++++-----------
 ui/tests/acceptance/allocation-fs-test.js     | 18 -----
 ui/tests/acceptance/task-fs-test.js           | 42 +++++------
 5 files changed, 53 insertions(+), 98 deletions(-)

diff --git a/ui/app/routes/allocations/allocation/fs.js b/ui/app/routes/allocations/allocation/fs.js
index ae1c67fe79a1..c988b7e05ba4 100644
--- a/ui/app/routes/allocations/allocation/fs.js
+++ b/ui/app/routes/allocations/allocation/fs.js
@@ -7,12 +7,7 @@ export default class FsRoute extends Route {
     const decodedPath = decodeURIComponent(path);
     const allocation = this.modelFor('allocations.allocation');
 
-    if (!allocation.isRunning) {
-      return {
-        path: decodedPath,
-        allocation,
-      };
-    }
+    if (!allocation) return;
 
     return RSVP.all([allocation.stat(decodedPath), allocation.get('node')])
       .then(([statJson]) => {
diff --git a/ui/app/routes/allocations/allocation/task/fs.js b/ui/app/routes/allocations/allocation/task/fs.js
index 7fcbc8734f9d..8ba69555c0b5 100644
--- a/ui/app/routes/allocations/allocation/task/fs.js
+++ b/ui/app/routes/allocations/allocation/task/fs.js
@@ -6,19 +6,14 @@ export default class FsRoute extends Route {
   model({ path = '/' }) {
     const decodedPath = decodeURIComponent(path);
     const taskState = this.modelFor('allocations.allocation.task');
-    const allocation = taskState.allocation;
 
+    if (!taskState || !taskState.allocation) return;
+
+    const allocation = taskState.allocation;
     const pathWithTaskName = `${taskState.name}${
       decodedPath.startsWith('/') ? '' : '/'
     }${decodedPath}`;
 
-    if (!taskState.isRunning) {
-      return {
-        path: decodedPath,
-        taskState,
-      };
-    }
-
     return RSVP.all([allocation.stat(pathWithTaskName), taskState.get('allocation.node')])
       .then(([statJson]) => {
         if (statJson.IsDir) {
diff --git a/ui/app/templates/components/fs/browser.hbs b/ui/app/templates/components/fs/browser.hbs
index 67e67c765003..5eebf798c58a 100644
--- a/ui/app/templates/components/fs/browser.hbs
+++ b/ui/app/templates/components/fs/browser.hbs
@@ -1,47 +1,38 @@
 <section class="section is-closer {{if this.isFile "is-full-width"}}">
-  {{#if this.model.isRunning}}
-    {{#if this.isFile}}
-      <Fs::File @allocation={{this.allocation}} @taskState={{this.taskState}} @file={{this.path}} @stat={{this.stat}} @class="fs-explorer">
+  {{#if this.isFile}}
+    <Fs::File @allocation={{this.allocation}} @taskState={{this.taskState}} @file={{this.path}} @stat={{this.stat}} @class="fs-explorer">
+      <Fs::Breadcrumbs @allocation={{this.allocation}} @taskState={{this.taskState}} @path={{this.path}} />
+    </Fs::File>
+  {{else}}
+    <div class="fs-explorer boxed-section">
+      <div class="boxed-section-head">
         <Fs::Breadcrumbs @allocation={{this.allocation}} @taskState={{this.taskState}} @path={{this.path}} />
-      </Fs::File>
-    {{else}}
-      <div class="fs-explorer boxed-section">
-        <div class="boxed-section-head">
-          <Fs::Breadcrumbs @allocation={{this.allocation}} @taskState={{this.taskState}} @path={{this.path}} />
-        </div>
-        {{#if this.directoryEntries}}
-          <ListTable
-            @source={{this.sortedDirectoryEntries}}
-            @sortProperty={{this.sortProperty}}
-            @sortDescending={{this.sortDescending}}
-            @class="boxed-section-body is-full-bleed is-compact" as |t|>
-            <t.head>
-              <t.sort-by @prop="Name" @class="is-two-thirds">Name</t.sort-by>
-              <t.sort-by @prop="Size" @class="has-text-right">File Size</t.sort-by>
-              <t.sort-by @prop="ModTime" @class="has-text-right">Last Modified</t.sort-by>
-            </t.head>
-            <t.body as |row|>
-              <Fs::DirectoryEntry @path={{this.path}} @allocation={{this.allocation}} @taskState={{this.taskState}} @entry={{row.model}} />
-            </t.body>
-          </ListTable>
-        {{else}}
-          <div class="boxed-section-body">
-            <div data-test-empty-directory class="empty-message">
-              <h3 data-test-empty-directory-headline class="empty-message-headline">No Files</h3>
-              <p data-test-empty-directory-body class="empty-message-body">
-                Directory is currently empty.
-              </p>
-            </div>
-          </div>
-        {{/if}}
       </div>
-    {{/if}}
-  {{else}}
-    <div data-test-not-running class="empty-message">
-      <h3 data-test-not-running-headline class="empty-message-headline">{{capitalize this.type}} is not Running</h3>
-      <p data-test-not-running-body class="empty-message-body">
-        Cannot access files of a{{if this.allocation 'n'}} {{this.type}} that is not running.
-      </p>
+      {{#if this.directoryEntries}}
+        <ListTable
+          @source={{this.sortedDirectoryEntries}}
+          @sortProperty={{this.sortProperty}}
+          @sortDescending={{this.sortDescending}}
+          @class="boxed-section-body is-full-bleed is-compact" as |t|>
+          <t.head>
+            <t.sort-by @prop="Name" @class="is-two-thirds">Name</t.sort-by>
+            <t.sort-by @prop="Size" @class="has-text-right">File Size</t.sort-by>
+            <t.sort-by @prop="ModTime" @class="has-text-right">Last Modified</t.sort-by>
+          </t.head>
+          <t.body as |row|>
+            <Fs::DirectoryEntry @path={{this.path}} @allocation={{this.allocation}} @taskState={{this.taskState}} @entry={{row.model}} />
+          </t.body>
+        </ListTable>
+      {{else}}
+        <div class="boxed-section-body">
+          <div data-test-empty-directory class="empty-message">
+            <h3 data-test-empty-directory-headline class="empty-message-headline">No Files</h3>
+            <p data-test-empty-directory-body class="empty-message-body">
+              Directory is currently empty.
+            </p>
+          </div>
+        </div>
+      {{/if}}
     </div>
   {{/if}}
 </section>
diff --git a/ui/tests/acceptance/allocation-fs-test.js b/ui/tests/acceptance/allocation-fs-test.js
index 7e35a8b11c72..0ad26e5b3af9 100644
--- a/ui/tests/acceptance/allocation-fs-test.js
+++ b/ui/tests/acceptance/allocation-fs-test.js
@@ -48,24 +48,6 @@ module('Acceptance | allocation fs', function(hooks) {
     this.nestedDirectory = files[1];
   });
 
-  test('when the allocation is not running, an empty state is shown', async function(assert) {
-    // The API 500s on stat when not running
-    this.server.get('/client/fs/stat/:allocation_id', () => {
-      return new Response(500, {}, 'no such file or directory');
-    });
-
-    allocation.update({
-      clientStatus: 'complete',
-    });
-
-    await FS.visitAllocation({ id: allocation.id });
-    assert.ok(FS.hasEmptyState, 'Non-running allocation has no files');
-    assert.ok(
-      FS.emptyState.headline.includes('Allocation is not Running'),
-      'Empty state explains the condition'
-    );
-  });
-
   browseFilesystem({
     visitSegments: ({ allocation }) => ({ id: allocation.id }),
     getExpectedPathBase: ({ allocation }) => `/allocations/${allocation.id}/fs/`,
diff --git a/ui/tests/acceptance/task-fs-test.js b/ui/tests/acceptance/task-fs-test.js
index 5a3bd4765f67..6aa82d0f131d 100644
--- a/ui/tests/acceptance/task-fs-test.js
+++ b/ui/tests/acceptance/task-fs-test.js
@@ -37,10 +37,18 @@ module('Acceptance | task fs', function(hooks) {
     files.push(taskDirectory);
 
     // Nested files
-    directory = server.create('allocFile', { isDir: true, name: 'directory', parent: taskDirectory });
+    directory = server.create('allocFile', {
+      isDir: true,
+      name: 'directory',
+      parent: taskDirectory,
+    });
     files.push(directory);
 
-    nestedDirectory = server.create('allocFile', { isDir: true, name: 'another', parent: directory });
+    nestedDirectory = server.create('allocFile', {
+      isDir: true,
+      name: 'another',
+      parent: directory,
+    });
     files.push(nestedDirectory);
 
     files.push(
@@ -51,7 +59,9 @@ module('Acceptance | task fs', function(hooks) {
       })
     );
 
-    files.push(server.create('allocFile', { isDir: true, name: 'empty-directory', parent: taskDirectory }));
+    files.push(
+      server.create('allocFile', { isDir: true, name: 'empty-directory', parent: taskDirectory })
+    );
     files.push(server.create('allocFile', 'file', { fileType: 'txt', parent: taskDirectory }));
     files.push(server.create('allocFile', 'file', { fileType: 'txt', parent: taskDirectory }));
 
@@ -60,29 +70,11 @@ module('Acceptance | task fs', function(hooks) {
     this.nestedDirectory = nestedDirectory;
   });
 
-  test('when the task is not running, an empty state is shown', async function(assert) {
-    // The API 500s on stat when not running
-    this.server.get('/client/fs/stat/:allocation_id', () => {
-      return new Response(500, {}, 'no such file or directory');
-    });
-
-    task.update({
-      finishedAt: new Date(),
-    });
-
-    await FS.visitTask({ id: allocation.id, name: task.name });
-    assert.ok(FS.hasEmptyState, 'Non-running task has no files');
-    assert.ok(
-      FS.emptyState.headline.includes('Task is not Running'),
-      'Empty state explains the condition'
-    );
-  });
-
   browseFilesystem({
-    visitSegments: ({allocation,task}) => ({ id: allocation.id, name: task.name }),
-    getExpectedPathBase: ({allocation,task}) => `/allocations/${allocation.id}/${task.name}/fs/`,
-    getTitleComponent: ({task}) => `Task ${task.name} filesystem`,
-    getBreadcrumbComponent: ({task}) => task.name,
+    visitSegments: ({ allocation, task }) => ({ id: allocation.id, name: task.name }),
+    getExpectedPathBase: ({ allocation, task }) => `/allocations/${allocation.id}/${task.name}/fs/`,
+    getTitleComponent: ({ task }) => `Task ${task.name} filesystem`,
+    getBreadcrumbComponent: ({ task }) => task.name,
     getFilesystemRoot: ({ task }) => task.name,
     pageObjectVisitFunctionName: 'visitTask',
     pageObjectVisitPathFunctionName: 'visitTaskPath',