Bug 1489507 - Fix not loading older pushes more than once

ui/helpers/location.js

@@ -1,4 +1,5 @@
 import { thDefaultRepo } from '../js/constants';
+import { createQueryParams } from './url';
 
 export const getQueryString = function getQueryString() {
   return location.hash.split('?')[1];
@@ -16,13 +17,17 @@ export const getRepo = function getRepo() {
   return getUrlParam('repo') || thDefaultRepo;
 };
 
+export const setLocation = function setLocation(params, hashPrefix = '/jobs') {
+  location.hash = `#${hashPrefix}${createQueryParams(params)}`;
+};
+
 export const setUrlParam = function setUrlParam(field, value, hashPrefix = '/jobs') {
   const params = getAllUrlParams();
+
   if (value) {
     params.set(field, value);
   } else {
     params.delete(field);
   }
-
-  location.hash = `#${hashPrefix}?${params.toString()}`;
+  setLocation(params, hashPrefix);
 };

ui/job-view/pushes/PushList.jsx

@@ -10,11 +10,16 @@ import {
   scrollToElement,
 } from '../../helpers/job';
 import PushLoadErrors from './PushLoadErrors';
-import { thEvents } from '../../js/constants';
+import { thEvents, thMaxPushFetchSize } from '../../js/constants';
 import JobModel from '../../models/job';
 import PushModel from '../../models/push';
 import ErrorBoundary from '../../shared/ErrorBoundary';
-import { getUrlParam, setUrlParam } from '../../helpers/location';
+import {
+  getAllUrlParams,
+  getUrlParam,
+  setLocation,
+  setUrlParam,
+} from '../../helpers/location';
 
 export default class PushList extends React.Component {
 
@@ -40,7 +45,11 @@ export default class PushList extends React.Component {
     };
 
     // get our first set of resultsets
-    this.ThResultSetStore.fetchPushes(this.ThResultSetStore.defaultPushCount);
+    const fromchange = getUrlParam('fromchange');
+    // If we have a ``fromchange`` url param.  We don't want to limit ourselves
+    // to the default of 10 pushes on the first load.
+    this.ThResultSetStore.fetchPushes(
+      fromchange ? thMaxPushFetchSize : this.ThResultSetStore.defaultPushCount);
   }
 
   componentWillMount() {
@@ -108,12 +117,14 @@ export default class PushList extends React.Component {
   }
 
   getNextPushes(count) {
-    this.setState({ loadingPushes: true });
     const revision = getUrlParam('revision');
+    this.setState({ loadingPushes: true });
     if (revision) {
       this.$rootScope.skipNextPageReload = true;
-      setUrlParam('revision', null);
-      setUrlParam('tochange', revision);
+      const params = getAllUrlParams();
+      params.delete('revision');
+      params.set('tochange', revision);
+      setLocation(params);
     }
     this.ThResultSetStore.fetchPushes(count)
       .then(this.updateUrlFromchange);

ui/js/models/resultsets_store.js

@@ -4,6 +4,7 @@
 
 import _ from 'lodash';
 import max from 'lodash/max';
+import pick from 'lodash/pick';
 
 import { thPlatformMap, thOptionOrder, thEvents } from '../constants';
 import { escapeId, getGroupMapKey } from '../../helpers/aggregateId';
@@ -52,6 +53,7 @@ treeherder.factory('ThResultSetStore', [
         // Keys that, if present on the url, must be passed into the push
         // polling endpoint
         var rsPollingKeys = ['tochange', 'enddate', 'revision', 'author'];
+        const rsFetchKeys = [...rsPollingKeys, 'fromchange', 'startdate'];
 
         // changes to the url for any of these fields should reload the page
         // because it changes the query to the db
@@ -794,9 +796,17 @@ treeherder.factory('ThResultSetStore', [
              */
             repoData.loadingStatus.appending = true;
             const isAppend = (repoData.pushes.length > 0);
-            const options = { ...parseQueryParams(getQueryString()), count };
+            // Only pass supported query string params to this endpoint.
+            const options = {
+              ...pick(parseQueryParams(getQueryString()), rsFetchKeys),
+              count,
+            };
 
             if (repoData.rsMapOldestTimestamp) {
+              // If we have an oldestTimestamp, then this isn't our first fetch,
+              // we're fetching more pushes.  We don't want to limit this fetch
+              // by the current ``fromchange`` value.
+              delete options.fromchange;
               options.push_timestamp__lte = repoData.rsMapOldestTimestamp;
             }
             return PushModel.getList(options).then(async (resp) => {