Skip to content
This repository has been archived by the owner on Nov 10, 2017. It is now read-only.

Commit

Permalink
bug 1082966 - could miss resultsets with fetching next 10
Browse files Browse the repository at this point in the history 
This also fixes the ability to load 1000 rs at one time due to
“tochange” param bug.
  • Loading branch information
Cameron Dawson committed Oct 15, 2014
1 parent 220bde7 commit 4322c28
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 30 deletions.
14 changes: 7 additions & 7 deletions webapp/app/js/controllers/jobs.js
View file
Expand Up @@ -14,8 +14,8 @@ treeherder.controller('JobsCtrl', [

// load our initial set of resultsets
// scope needs this function so it can be called directly by the user, too.
$scope.fetchResultSets = function(count) {
ThResultSetModel.fetchResultSets($scope.repoName, count);
$scope.fetchResultSets = function(count, keepFilters) {
ThResultSetModel.fetchResultSets($scope.repoName, count, keepFilters);
};

// set the default repo to mozilla-central if not specified
Expand All @@ -41,15 +41,15 @@ treeherder.controller('JobsCtrl', [

// determine how many resultsets to fetch. default to 10.
var count = ThResultSetModel.defaultResultSetCount;
if ((_.has($scope.searchParams, "startdate") || _.has($scope.searchParams, "fromchange") &&
(_.has($scope.searchParams, "enddate")) || _.has($scope.searchParams, "tochange"))) {
// just fetch all (up to 1000) the resultsets if an upper AND lower range is specified
if ((_.has($scope.searchParams, "startdate") || _.has($scope.searchParams, "fromchange")) &&
(_.has($scope.searchParams, "enddate") || _.has($scope.searchParams, "tochange"))) {
// just fetch all (up to 100) the resultsets if an upper AND lower range is specified

count = 1000;
count = 100;
}
if(ThResultSetModel.isNotLoaded($scope.repoName)){
// get our first set of resultsets
ThResultSetModel.fetchResultSets($scope.repoName, count);
ThResultSetModel.fetchResultSets($scope.repoName, count, true);
}

$rootScope.$on(
Expand Down
31 changes: 14 additions & 17 deletions webapp/app/js/models/resultsets.js
View file
Expand Up @@ -190,10 +190,6 @@ treeherder.factory('ThResultSetModel', [

name:repoName,

//This is set to the id of the last resultset loaded
//and used as the offset in paging
rsOffsetId:0,

lastJobElSelected:{},
lastJobObjSelected:{},

Expand All @@ -202,6 +198,7 @@ treeherder.factory('ThResultSetModel', [
jobMap:{},
unclassifiedFailureMap: {},
jobMapOldestId:null,
//used as the offset in paging
rsMapOldestTimestamp:null,
resultSets:[],

Expand Down Expand Up @@ -290,13 +287,6 @@ treeherder.factory('ThResultSetModel', [
repositories[repoName].rsMapOldestTimestamp = rs_obj.push_timestamp;
}

//Keep track of the last resultset id for paging
var resultsetId = parseInt(rs_obj.id, 10);
if( (resultsetId < repositories[repoName].rsOffsetId) ||
(repositories[repoName].rsOffsetId === 0) ){
repositories[repoName].rsOffsetId = resultsetId;
}

// platforms
if(rs_obj.platforms !== undefined){
mapPlatforms(repoName, rs_obj);
Expand Down Expand Up @@ -691,10 +681,15 @@ treeherder.factory('ThResultSetModel', [
if(data.results.length > 0){

$log.debug("appendResultSets", data.results);
var rsIds = _.map(repositories[repoName].resultSets, function(rs){
return rs.id;
});

Array.prototype.push.apply(
repositories[repoName].resultSets, data.results
);
_.each(data.results, function(rs) {
if (!_.contains(rsIds, rs.id)) {
repositories[repoName].resultSets.push(rs);
}
});

mapResultSets(repoName, data.results);

Expand Down Expand Up @@ -780,7 +775,7 @@ treeherder.factory('ThResultSetModel', [
return _.isEmpty(repositories[repoName].rsMap);
};

var fetchResultSets = function(repoName, count){
var fetchResultSets = function(repoName, count, keepFilters){
/**
* Get the next batch of resultsets based on our current offset.
* @param count How many to fetch
Expand All @@ -789,10 +784,12 @@ treeherder.factory('ThResultSetModel', [
var resultsets;
var loadRepositories = ThRepositoryModel.load(repoName);
var loadResultsets = thResultSets.getResultSets(repoName,
repositories[repoName].rsOffsetId,
repositories[repoName].rsMapOldestTimestamp,
count,
undefined,
false).
false,
true,
keepFilters).
then(function(data) {
resultsets = data.data;
});
Expand Down
16 changes: 11 additions & 5 deletions webapp/app/js/services/resultsets.js
View file
Expand Up @@ -63,12 +63,12 @@ treeherder.factory('thResultSets', [
}
);
},
getResultSets: function(repoName, rsOffsetId, count, resultsetlist, with_jobs, full, keep_filters) {
rsOffsetId = typeof rsOffsetId === 'undefined'? 0: rsOffsetId;
getResultSets: function(repoName, rsOffsetTimestamp, count, resultsetlist, with_jobs, full, keep_filters) {
rsOffsetTimestamp = typeof rsOffsetTimestamp === 'undefined'? 0: rsOffsetTimestamp;
count = typeof count === 'undefined'? 10: count;
with_jobs = _.isUndefined(with_jobs) ? true: with_jobs;
full = _.isUndefined(full) ? true: full;
keep_filters = _.isUndefined(keep_filters) ? true : false;
keep_filters = _.isUndefined(keep_filters) ? true : keep_filters;

var params = {
full: full,
Expand All @@ -80,8 +80,14 @@ treeherder.factory('thResultSets', [
params.count = count;
}

if(rsOffsetId > 0){
params.id__lt = rsOffsetId;
if(rsOffsetTimestamp > 0){
params.push_timestamp__lte = rsOffsetTimestamp;
// we will likely re-fetch the oldest we already have, but
// that's not guaranteed. There COULD be two resultsets
// with the same timestamp, theoretically.
if (params.count) {
params.count++;
}
}

if(keep_filters){
Expand Down
2 changes: 1 addition & 1 deletion webapp/app/partials/main/jobs.html
View file
Expand Up @@ -88,7 +88,7 @@
get next:
<div class="btn-group">
<div class="btn btn-default btn-sm"
ng-click="fetchResultSets(count)"
ng-click="fetchResultSets(count, false)"
ng-repeat="count in [10, 20, 50]">{{count}}</div>
</div>
</div>
Expand Down

0 comments on commit 4322c28

Please sign in to comment.