From 05f3bf2d09e1011d3e26509f26712e77c64f5ae0 Mon Sep 17 00:00:00 2001 From: Michael Lange 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 | 23 +----- ui/tests/acceptance/task-fs-test.js | 47 +++++------- 5 files changed, 55 insertions(+), 106 deletions(-) diff --git a/ui/app/routes/allocations/allocation/fs.js b/ui/app/routes/allocations/allocation/fs.js index ae1c67fe79a..c988b7e05ba 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 7fcbc8734f9..8ba69555c0b 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 67e67c76500..5eebf798c58 100644 --- a/ui/app/templates/components/fs/browser.hbs +++ b/ui/app/templates/components/fs/browser.hbs @@ -1,47 +1,38 @@
- {{#if this.model.isRunning}} - {{#if this.isFile}} - + {{#if this.isFile}} + + + + {{else}} +
+
- - {{else}} -
-
- -
- {{#if this.directoryEntries}} - - - Name - File Size - Last Modified - - - - - - {{else}} -
-
-

No Files

-

- Directory is currently empty. -

-
-
- {{/if}}
- {{/if}} - {{else}} -
-

{{capitalize this.type}} is not Running

-

- Cannot access files of a{{if this.allocation 'n'}} {{this.type}} that is not running. -

+ {{#if this.directoryEntries}} + + + Name + File Size + Last Modified + + + + + + {{else}} +
+
+

No Files

+

+ Directory is currently empty. +

+
+
+ {{/if}}
{{/if}}
diff --git a/ui/tests/acceptance/allocation-fs-test.js b/ui/tests/acceptance/allocation-fs-test.js index 7e35a8b11c7..2712a325f32 100644 --- a/ui/tests/acceptance/allocation-fs-test.js +++ b/ui/tests/acceptance/allocation-fs-test.js @@ -1,14 +1,11 @@ /* eslint-disable ember-a11y-testing/a11y-audit-called */ // Covered in behaviours/fs -import { module, test } from 'qunit'; +import { module } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; -import Response from 'ember-cli-mirage/response'; import browseFilesystem from './behaviors/fs'; -import FS from 'nomad-ui/tests/pages/allocations/fs'; - let allocation; let files; @@ -48,24 +45,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 5a3bd4765f6..9737e8060d2 100644 --- a/ui/tests/acceptance/task-fs-test.js +++ b/ui/tests/acceptance/task-fs-test.js @@ -1,14 +1,11 @@ /* eslint-disable ember-a11y-testing/a11y-audit-called */ // Covered in behaviours/fs -import { module, test } from 'qunit'; +import { module } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; -import Response from 'ember-cli-mirage/response'; import browseFilesystem from './behaviors/fs'; -import FS from 'nomad-ui/tests/pages/allocations/fs'; - let allocation; let task; let files, taskDirectory, directory, nestedDirectory; @@ -37,10 +34,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 +56,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 +67,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',