From 35abc4532f348997a4b5413ea9c57bebddda213d Mon Sep 17 00:00:00 2001
From: Phil Renaud <phil@riotindustries.com>
Date: Tue, 25 Jun 2024 17:01:45 -0400
Subject: [PATCH] Test and cleanup

---
 .changelog/23427.txt                  |  3 ++
 ui/app/controllers/jobs/index.js      |  8 +++--
 ui/app/routes/jobs/index.js           |  2 --
 ui/app/serializers/job.js             |  8 ++++-
 ui/app/templates/jobs/index.hbs       |  7 ++--
 ui/tests/acceptance/jobs-list-test.js | 49 +++++++++++++++++++++++++++
 6 files changed, 69 insertions(+), 8 deletions(-)
 create mode 100644 .changelog/23427.txt

diff --git a/.changelog/23427.txt b/.changelog/23427.txt
new file mode 100644
index 00000000000..ef374cce6a1
--- /dev/null
+++ b/.changelog/23427.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+ui: fix an issue where gateway timeouts would cause the jobs list to revert to null, gives users a Pause Fetch option
+```
diff --git a/ui/app/controllers/jobs/index.js b/ui/app/controllers/jobs/index.js
index 1dfe511d70f..4601eeb18cc 100644
--- a/ui/app/controllers/jobs/index.js
+++ b/ui/app/controllers/jobs/index.js
@@ -161,7 +161,6 @@ export default class JobsIndexController extends Controller {
    * In case the user wants to specifically stop polling for new jobs
    */
   @action pauseJobFetching() {
-    // this.showingCachedJobs = false;
     let notification = this.notifications.queue.find(
       (n) => n.title === 'Error fetching jobs'
     );
@@ -204,7 +203,6 @@ export default class JobsIndexController extends Controller {
    */
   notifyFetchError(e) {
     const firstError = e.errors[0];
-    console.log('firstError', firstError);
     this.notifications.add({
       title: 'Error fetching jobs',
       message: `The backend returned an error with status ${firstError.status} while fetching jobs`,
@@ -330,8 +328,14 @@ export default class JobsIndexController extends Controller {
           this.pendingJobIDs = jobIDs;
           this.pendingJobs = newJobs;
         }
+        if (Ember.testing) {
+          break;
+        }
         yield timeout(throttle);
       } else {
+        if (Ember.testing) {
+          break;
+        }
         // This returns undefined on page change / cursorAt change, resulting from the aborting of the old query.
         yield timeout(throttle);
         this.watchJobs.perform(this.jobIDs, throttle);
diff --git a/ui/app/routes/jobs/index.js b/ui/app/routes/jobs/index.js
index f4f21d5ea23..44640df6d37 100644
--- a/ui/app/routes/jobs/index.js
+++ b/ui/app/routes/jobs/index.js
@@ -81,10 +81,8 @@ export default class IndexRoute extends Route.extend(
       });
     } catch (error) {
       try {
-        console.log('route jobs index catch try 1');
         notifyForbidden(this)(error);
       } catch (secondaryError) {
-        console.log('route jobs index catch try 2');
         return this.handleErrors(error);
       }
     }
diff --git a/ui/app/serializers/job.js b/ui/app/serializers/job.js
index 940e558b82b..15d495a3f8d 100644
--- a/ui/app/serializers/job.js
+++ b/ui/app/serializers/job.js
@@ -71,7 +71,13 @@ export default class JobSerializer extends ApplicationSerializer {
     return super.normalize(typeHash, hash);
   }
 
-  normalizeQueryResponse(store, primaryModelClass, payload, id, requestType) {
+  normalizeQueryResponse(
+    store,
+    primaryModelClass,
+    payload = [],
+    id,
+    requestType
+  ) {
     // What jobs did we ask for?
     if (payload._requestBody?.jobs) {
       let requestedJobIDs = payload._requestBody.jobs;
diff --git a/ui/app/templates/jobs/index.hbs b/ui/app/templates/jobs/index.hbs
index 2fd46d6190f..57ef6f8a533 100644
--- a/ui/app/templates/jobs/index.hbs
+++ b/ui/app/templates/jobs/index.hbs
@@ -8,11 +8,12 @@
   {{#if this.showingCachedJobs}}
     <Hds::Alert @type="inline" @color="warning" id="jobs-list-cache-warning" as |A|>
       <A.Title>Error fetching jobs — shown jobs are cached</A.Title>
-      <A.Description>Jobs shown are cached and may be out of date. Often, this is due to a short timeout in proxy configurations. Consider increasing the timeout.</A.Description>
+      <A.Description>Jobs shown are cached and may be out of date. This is often due to a short timeout in proxy configurations.</A.Description>
       {{#if this.watchJobIDs.isRunning}}
-        <A.Button @text="Stop polling for job updates" @color="secondary" {{on "click" this.pauseJobFetching}} />
+        <A.Button data-test-pause-fetching @text="Stop polling for job updates" @color="secondary" {{on "click" this.pauseJobFetching}} />
       {{/if}}
-      <A.Button @text="Manually fetch jobs" @color="secondary" {{on "click" this.restartJobList}} />
+      <A.Button data-test-restart-fetching @text="Manually fetch jobs" @color="secondary" {{on "click" this.restartJobList}} />
+      <A.LinkStandalone @size="medium" @color="primary" @icon="learn-link" @iconPosition="trailing" @text="Tutorial: Configure reverse proxy for Nomad's web UI" @href="https://developer.hashicorp.com/nomad/tutorials/manage-clusters/reverse-proxy-ui#extend-connection-timeout" />
     </Hds::Alert>
   {{/if}}
 
diff --git a/ui/tests/acceptance/jobs-list-test.js b/ui/tests/acceptance/jobs-list-test.js
index e27f24f7f65..ae9f48266ab 100644
--- a/ui/tests/acceptance/jobs-list-test.js
+++ b/ui/tests/acceptance/jobs-list-test.js
@@ -250,6 +250,55 @@ module('Acceptance | jobs list', function (hooks) {
     assert.equal(currentURL(), '/settings/tokens');
   });
 
+  test('when a gateway timeout error occurs, appropriate options are shown', async function (assert) {
+    // Initial request is fine
+    await JobsList.visit();
+
+    assert.dom('#jobs-list-cache-warning').doesNotExist();
+
+    server.pretender.get('/v1/jobs/statuses', () => [
+      504,
+      {
+        errors: [
+          {
+            status: '504',
+          },
+        ],
+      },
+      null,
+    ]);
+    const controller = this.owner.lookup('controller:jobs.index');
+    let currentParams = {
+      per_page: 10,
+    };
+
+    await controller.watchJobIDs.perform(currentParams, 0);
+    // Manually set its "isRunning" attribute for testing purposes
+    // (existence of one of the buttons depends on blocking query running, which Ember testing doesnt really support)
+    controller.watchJobIDs.isRunning = true;
+    await settled();
+
+    assert.dom('#jobs-list-cache-warning').exists();
+
+    assert
+      .dom('.flash-message.alert-critical')
+      .exists('A toast error message pops up.');
+
+    await percySnapshot(assert);
+
+    await click('[data-test-pause-fetching]');
+    assert
+      .dom('.flash-message.alert-critical')
+      .doesNotExist('Error message removed when fetrching is paused');
+    assert.dom('#jobs-list-cache-warning').exists('Cache warning remains');
+
+    server.pretender.get('/v1/jobs/statuses', () => [200, {}, null]);
+    await click('[data-test-restart-fetching]');
+    assert
+      .dom('#jobs-list-cache-warning')
+      .doesNotExist('Cache warning removed when fetching is restarted');
+  });
+
   function typeForJob(job) {
     return job.periodic
       ? 'periodic'