Fix circular dependency in JobModel/PushModel (#4929)

tests/ui/unit/models/jobs_test.js

@@ -7,8 +7,6 @@ import paginatedJobListFixtureOne from '../../mock/job_list/pagination/page_1';
 import paginatedJobListFixtureTwo from '../../mock/job_list/pagination/page_2';
 
 describe('JobModel', () => {
-  const repoName = 'mozilla-inbound';
-
   afterEach(() => {
     fetchMock.reset();
   });
@@ -19,7 +17,7 @@ describe('JobModel', () => {
     });
 
     test('should return a promise', () => {
-      const result = JobModel.getList('mozilla-inbound');
+      const result = JobModel.getList();
       expect(result.then).toBeDefined();
     });
   });
@@ -37,17 +35,13 @@ describe('JobModel', () => {
     });
 
     test('should return a page of results by default', async () => {
-      const jobList = await JobModel.getList(repoName, { count: 2 });
+      const jobList = await JobModel.getList({ count: 2 });
 
       expect(jobList).toHaveLength(2);
     });
 
     test('should return all the pages when fetch_all==true', async () => {
-      const jobList = await JobModel.getList(
-        repoName,
-        { count: 2 },
-        { fetch_all: true },
-      );
+      const jobList = await JobModel.getList({ count: 2 }, { fetch_all: true });
 
       expect(jobList).toHaveLength(3);
       expect(jobList[2].id).toBe(3);

ui/job-view/context/Pushes.jsx

@@ -373,7 +373,13 @@ export class PushesClass extends React.Component {
     }
     const pushIds = pushList.map(push => push.id);
     const lastModified = this.getLastModifiedJobTime();
-    const jobList = await PushModel.getJobs(pushIds, { lastModified });
+    const jobList = await JobModel.getList(
+      {
+        push_id__in: pushIds.join(','),
+        last_modified__gt: lastModified.toISOString().replace('Z', ''),
+      },
+      { fetch_all: true },
+    );
     // break the jobs up per push
     const jobs = jobList.reduce((acc, job) => {
       const pushJobs = acc[job.push_id] ? [...acc[job.push_id], job] : [job];

ui/job-view/details/tabs/SimilarJobsTab.jsx

@@ -57,7 +57,6 @@ class SimilarJobsTab extends React.Component {
     });
 
     const newSimilarJobs = await JobModel.getSimilarJobs(
-      repoName,
       selectedJob.id,
       options,
     );

ui/job-view/pushes/Push.jsx

@@ -10,7 +10,7 @@ import {
 import { withPushes } from '../context/Pushes';
 import { getGroupMapKey } from '../../helpers/aggregateId';
 import { getAllUrlParams, getUrlParam } from '../../helpers/location';
-import PushModel from '../../models/push';
+import JobModel from '../../models/job';
 import RunnableJobModel from '../../models/runnableJob';
 import { withNotifications } from '../../shared/context/Notifications';
 import { getRevisionTitle } from '../../helpers/revision';
@@ -139,7 +139,14 @@ class Push extends React.Component {
 
   fetchJobs = async () => {
     const { push } = this.props;
-    const jobs = await PushModel.getJobs(push.id);
+    const jobs = await JobModel.getList(
+      {
+        push_id: push.id,
+        count: 2000,
+        return_type: 'list',
+      },
+      { fetch_all: true },
+    );
 
     this.mapPushJobs(jobs);
   };

ui/models/job.js

@@ -45,7 +45,7 @@ export default class JobModel {
       .join(' ');
   }
 
-  static getList(repoName, options, config) {
+  static getList(options, config) {
     // a static method to retrieve a list of JobModel
     config = config || {};
     const fetch_all = config.fetch_all || false;
@@ -67,11 +67,7 @@ export default class JobModel {
             const offset = parseInt(data.meta.offset, 10) + count;
             const newOptions = { ...options, offset, count };
 
-            nextPagesJobs = await JobModel.getList(
-              repoName,
-              newOptions,
-              config,
-            );
+            nextPagesJobs = await JobModel.getList(newOptions, config);
           }
           if ('job_property_names' in data) {
             // the results came as list of fields
@@ -108,12 +104,12 @@ export default class JobModel {
     });
   }
 
-  static getSimilarJobs(repoName, pk, options, config) {
+  static getSimilarJobs(pk, options, config) {
     config = config || {};
     // The similar jobs endpoints returns the same type of objects as
     // the job list endpoint, so let's reuse the getList method logic.
     config.uri = `${uri}${pk}/similar_jobs/`;
-    return JobModel.getList(repoName, options, config);
+    return JobModel.getList(options, config);
   }
 
   static async retrigger(jobs, repoName, notify) {

ui/models/push.js

@@ -5,7 +5,6 @@ import { getData } from '../helpers/http';
 import { getProjectUrl, getUrlParam } from '../helpers/location';
 import { createQueryParams, pushEndpoint } from '../helpers/url';
 
-import JobModel from './job';
 import TaskclusterModel from './taskcluster';
 
 const convertDates = function convertDates(locationParams) {
@@ -60,33 +59,6 @@ export default class PushModel {
     return fetch(getProjectUrl(`${pushEndpoint}${pk}/`, repoName));
   }
 
-  static getJobs(pushIds, options = {}) {
-    const { lastModified, repo } = options;
-    delete options.lastModified;
-    delete options.repo;
-    const params = {
-      return_type: 'list',
-      count: 2000,
-      ...options,
-    };
-
-    if (!Array.isArray(pushIds)) {
-      params.push_id = pushIds;
-    } else {
-      params.push_id__in = pushIds.join(',');
-    }
-    if (lastModified) {
-      // XXX: should never happen, but maybe sometimes does? see bug 1287501
-      if (!(lastModified instanceof Date)) {
-        throw Error(
-          `Invalid parameter passed to get job updates: ${lastModified}.  Please reload treeherder`,
-        );
-      }
-      params.last_modified__gt = lastModified.toISOString().replace('Z', '');
-    }
-    return JobModel.getList(repo, params, { fetch_all: true });
-  }
-
   static triggerMissingJobs(decisionTaskId) {
     return TaskclusterModel.load(decisionTaskId).then(results => {
       const actionTaskId = slugid();