From 4a25e534dc6f73a75408a5ae6b2032a4f920a61f Mon Sep 17 00:00:00 2001
From: Phil Renaud <phil@riotindustries.com>
Date: Mon, 7 Oct 2024 16:36:55 -0400
Subject: [PATCH] Separate Diffs and Versions from the /versions endpoint as
 far as Ember is concerned

---
 .changelog/24145.txt                              |  3 +++
 ui/app/components/job-version.js                  | 11 ++++++++---
 ui/app/components/job-versions-stream.js          |  7 +++++--
 ui/app/controllers/jobs/job/versions.js           | 11 +++++++++++
 ui/app/models/job.js                              |  2 +-
 ui/app/routes/jobs/job/versions.js                | 15 +--------------
 ui/app/serializers/job.js                         |  4 +++-
 ui/app/templates/components/job-version.hbs       |  5 +++--
 .../templates/components/job-versions-stream.hbs  |  2 +-
 ui/app/templates/jobs/job/versions.hbs            |  3 ++-
 10 files changed, 38 insertions(+), 25 deletions(-)
 create mode 100644 .changelog/24145.txt

diff --git a/.changelog/24145.txt b/.changelog/24145.txt
new file mode 100644
index 00000000000..15e9bd76607
--- /dev/null
+++ b/.changelog/24145.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fix an issue where new job versions would not see their diffs updated on the job versions page
+```
diff --git a/ui/app/components/job-version.js b/ui/app/components/job-version.js
index 3dbfb7f7f06..31606f9844e 100644
--- a/ui/app/components/job-version.js
+++ b/ui/app/components/job-version.js
@@ -19,6 +19,7 @@ export default class JobVersion extends Component {
   @service router;
 
   @alias('args.version') version;
+  @alias('args.diff') diff;
   @tracked isOpen = false;
   @tracked isEditing = false;
   @tracked editableTag;
@@ -29,7 +30,11 @@ export default class JobVersion extends Component {
   constructor() {
     super(...arguments);
     this.initializeEditableTag();
-    if (this.args.diffsExpanded && this.version.diff) {
+    this.versionsDidUpdate();
+  }
+
+  @action versionsDidUpdate() {
+    if (this.args.diffsExpanded && this.diff) {
       this.isOpen = true;
     }
   }
@@ -47,9 +52,9 @@ export default class JobVersion extends Component {
     this.editableTag.jobName = this.version.get('job.plainId');
   }
 
-  @computed('version.diff')
+  @computed('diff')
   get changeCount() {
-    const diff = this.version.diff;
+    const diff = this.diff;
     const taskGroups = diff.TaskGroups || [];
 
     if (!diff) {
diff --git a/ui/app/components/job-versions-stream.js b/ui/app/components/job-versions-stream.js
index 61528a70675..9b1dc03ad62 100644
--- a/ui/app/components/job-versions-stream.js
+++ b/ui/app/components/job-versions-stream.js
@@ -19,7 +19,9 @@ export default class JobVersionsStream extends Component {
   // Passes through to the job-diff component
   verbose = true;
 
-  @computed('versions.[]')
+  diffs = [];
+
+  @computed('versions.[]', 'diffs.[]')
   get annotatedVersions() {
     const versions = this.versions.sortBy('submitTime').reverse();
     return versions.map((version, index) => {
@@ -38,7 +40,8 @@ export default class JobVersionsStream extends Component {
         }
       }
 
-      return { version, meta };
+      const diff = this.diffs.objectAt(index);
+      return { version, meta, diff };
     });
   }
 }
diff --git a/ui/app/controllers/jobs/job/versions.js b/ui/app/controllers/jobs/job/versions.js
index 7a14cc434b4..b5f627b3d9b 100644
--- a/ui/app/controllers/jobs/job/versions.js
+++ b/ui/app/controllers/jobs/job/versions.js
@@ -58,6 +58,17 @@ export default class VersionsController extends Controller.extend(
     });
   }
 
+  @tracked diffs = [];
+
+  @action async versionsDidUpdate() {
+    try {
+      const diffs = await this.job.getVersions(this.diffVersion);
+      this.diffs = diffs.Diffs;
+    } catch (error) {
+      console.error('error fetching diffs', error);
+    }
+  }
+
   get diffsExpanded() {
     return this.diffVersion !== '';
   }
diff --git a/ui/app/models/job.js b/ui/app/models/job.js
index eee53d64728..b01792d9685 100644
--- a/ui/app/models/job.js
+++ b/ui/app/models/job.js
@@ -431,7 +431,7 @@ export default class Job extends Model {
 
   @attr('number') version;
 
-  @hasMany('job-versions', { async: true }) versions;
+  @hasMany('job-versions', { async: false }) versions;
   @hasMany('allocations') allocations;
   @hasMany('deployments') deployments;
   @hasMany('evaluations') evaluations;
diff --git a/ui/app/routes/jobs/job/versions.js b/ui/app/routes/jobs/job/versions.js
index 3f3a507e996..13efd2529de 100644
--- a/ui/app/routes/jobs/job/versions.js
+++ b/ui/app/routes/jobs/job/versions.js
@@ -15,21 +15,8 @@ import { inject as service } from '@ember/service';
 export default class VersionsRoute extends Route.extend(WithWatchers) {
   @service store;
 
-  queryParams = {
-    diffVersion: {
-      refreshModel: true,
-    },
-  };
-
-  async model(params) {
+  async model() {
     const job = this.modelFor('jobs.job');
-    const versions = await job.getVersions(params.diffVersion);
-
-    job.versions = job.versions.map((v, i) => {
-      const diff = versions.Diffs[i];
-      v.set('diff', diff);
-      return v;
-    });
     return job;
   }
 
diff --git a/ui/app/serializers/job.js b/ui/app/serializers/job.js
index 15d495a3f8d..e3e17f8734c 100644
--- a/ui/app/serializers/job.js
+++ b/ui/app/serializers/job.js
@@ -210,7 +210,9 @@ export default class JobSerializer extends ApplicationSerializer {
       },
       versions: {
         links: {
-          related: buildURL(`${jobURL}/versions`, { namespace, diffs: true }),
+          related: buildURL(`${jobURL}/versions`, {
+            namespace,
+          }),
         },
       },
       deployments: {
diff --git a/ui/app/templates/components/job-version.hbs b/ui/app/templates/components/job-version.hbs
index 275b9794c0f..587bb18c978 100644
--- a/ui/app/templates/components/job-version.hbs
+++ b/ui/app/templates/components/job-version.hbs
@@ -3,6 +3,7 @@
   SPDX-License-Identifier: BUSL-1.1
 ~}}
 
+{{did-update this.versionsDidUpdate this.diff}}
 <section class="job-version">
   <div class="boxed-section {{if this.version.versionTag "tagged"}}" data-test-tagged-version={{if this.version.versionTag "true" "false"}}>
     <header class="boxed-section-head is-light inline-definitions">
@@ -16,7 +17,7 @@
         <span data-test-version-submit-time class="submit-date">{{format-ts this.version.submitTime}}</span>
       </span>
       <div class="pull-right">
-        {{#if this.version.diff}}
+        {{#if this.diff}}
           <Hds::Button
             class="is-light is-small"
             @size="small"
@@ -33,7 +34,7 @@
     </header>
     {{#if this.isOpen}}
       <div class="boxed-section-body is-dark">
-        <JobDiff @diff={{this.version.diff}} @verbose={{this.verbose}} />
+        <JobDiff @diff={{this.diff}} @verbose={{this.verbose}} />
       </div>
     {{/if}}
     <footer class="boxed-section-foot {{if this.isEditing "editing"}}">
diff --git a/ui/app/templates/components/job-versions-stream.hbs b/ui/app/templates/components/job-versions-stream.hbs
index 32d8cf60912..b216ed71fd0 100644
--- a/ui/app/templates/components/job-versions-stream.hbs
+++ b/ui/app/templates/components/job-versions-stream.hbs
@@ -10,6 +10,6 @@
     </li>
   {{/if}}
   <li data-test-version class="timeline-object">
-    <JobVersion @version={{record.version}} @verbose={{this.verbose}} @handleError={{@handleError}} @diffsExpanded={{@diffsExpanded}} />
+    <JobVersion @version={{record.version}} @diff={{record.diff}} @verbose={{this.verbose}} @handleError={{@handleError}} @diffsExpanded={{@diffsExpanded}} />
   </li>
 {{/each}}
diff --git a/ui/app/templates/jobs/job/versions.hbs b/ui/app/templates/jobs/job/versions.hbs
index 1e89cc653d9..eac2e325e80 100644
--- a/ui/app/templates/jobs/job/versions.hbs
+++ b/ui/app/templates/jobs/job/versions.hbs
@@ -4,6 +4,7 @@
 ~}}
 
 {{page-title "Job " this.job.name " versions"}}
+{{did-update this.versionsDidUpdate this.job.versions}}
 <JobSubnav @job={{this.job}} />
 <section class="section">
 
@@ -56,5 +57,5 @@
     </div>
   {{/if}}
 
-  <JobVersionsStream @versions={{this.model.versions}} @verbose={{true}} @handleError={{action this.handleError}} @diffsExpanded={{this.diffsExpanded}} />
+  <JobVersionsStream @versions={{this.model.versions}} @diffs={{this.diffs}} @verbose={{true}} @handleError={{action this.handleError}} @diffsExpanded={{this.diffsExpanded}} />
 </section>