DEVPROD-13211: Fix bug with trailing inactive commits

graphql/query_resolver.go

@@ -989,12 +989,22 @@ func (r *queryResolver) Waterfall(ctx context.Context, options WaterfallOptions)
 
 	// Since GetAllWaterfallVersions uses an inclusive order range ($gte instead of $gt), add 1 to our minimum range
 	minVersionOrder := minOrderOpt + 1
-	if minOrderOpt == 0 && len(activeVersions) != 0 {
-		// Only use the last active version order number if no minOrder was provided. Using the activeVersions bounds may omit inactive versions between the min and the last active version found.
-		minVersionOrder = activeVersions[len(activeVersions)-1].RevisionOrderNumber
-	} else if len(activeVersions) == 0 {
-		// If there are no active versions, use 0 to fetch all inactive versions
+	if len(activeVersions) == 0 {
 		minVersionOrder = 0
+	} else if minOrderOpt == 0 {
+		// Find an older version that is activated. If it doesn't exist, that means there are trailing inactive
+		// versions on the waterfall and that we should not place a lower bound.
+		// If it does exist, set the min order to one more than its order. This is guaranteed to either be
+		// the last version in activeVersions or the last inactive version within its collapsed group.
+		prevActiveVersion, err := model.GetNextOlderActiveWaterfallVersion(ctx, projectId, activeVersions[len(activeVersions)-1].RevisionOrderNumber)
+		if err != nil {
+			return nil, InternalServerError.Send(ctx, fmt.Sprintf("fetching previous active waterfall version: %s", err.Error()))
+		}
+		if prevActiveVersion == nil {
+			minVersionOrder = 0
+		} else {
+			minVersionOrder = prevActiveVersion.RevisionOrderNumber + 1
+		}
 	}
 
 	// Same as above, but subtract for max order
@@ -1005,10 +1015,10 @@ func (r *queryResolver) Waterfall(ctx context.Context, options WaterfallOptions)
 		// If no order options were specified, we're on the first page and should not put a limit on the first version returned so that we don't omit inactive versions
 		maxVersionOrder = 0
 	} else if maxOrderOpt == 0 {
-		// Find the next recent active version. If it doesn't exist, that means there are leading inactive versions
-		// on the waterfall and we should reset to the first page.
-		// If it does exist, we should set the max order to one less than its order. This is guaranteed to either be
-		// the 0th version in activeVersions or the most recent inactive version within its collapsed group.
+		// Find the next recent active version. If it doesn't exist, that means there are leading inactive
+		// versions on the waterfall and we should reset to the first page.
+		// If it does exist, set the max order to one less than its order. This is guaranteed to either be
+		// the 0th version in activeVersions or the first inactive version within its collapsed group.
 		nextActiveVersion, err := model.GetNextRecentActiveWaterfallVersion(ctx, projectId, activeVersions[0].RevisionOrderNumber)
 		if err != nil {
 			return nil, InternalServerError.Send(ctx, fmt.Sprintf("fetching next active waterfall version: %s", err.Error()))
@@ -1060,6 +1070,10 @@ func (r *queryResolver) Waterfall(ctx context.Context, options WaterfallOptions)
 		if mostRecentWaterfallVersion.RevisionOrderNumber <= prevPageOrder {
 			prevPageOrder = 0
 		}
+		// The first order of any project is 1, so there's no next page if we've reached that.
+		if nextPageOrder == 1 {
+			nextPageOrder = 0
+		}
 	}
 
 	flattenedVersions := []*restModel.APIVersion{}

model/waterfall.go

@@ -266,3 +266,38 @@ func GetNextRecentActiveWaterfallVersion(ctx context.Context, projectId string,
 	}
 	return &res[0], nil
 }
+
+// GetNextOlderActiveWaterfallVersion returns the next older active version on the waterfall, i.e. an older
+// activated version than the version with the given maxOrder.
+func GetNextOlderActiveWaterfallVersion(ctx context.Context, projectId string, maxOrder int) (*Version, error) {
+	match := bson.M{
+		VersionIdentifierKey: projectId,
+		VersionRequesterKey: bson.M{
+			"$in": evergreen.SystemVersionRequesterTypes,
+		},
+		VersionRevisionOrderNumberKey: bson.M{
+			"$lt": maxOrder,
+		},
+		VersionActivatedKey: true,
+	}
+	pipeline := []bson.M{
+		{"$match": match},
+		{"$sort": bson.M{VersionRevisionOrderNumberKey: -1}},
+		{"$limit": 1},
+	}
+
+	res := []Version{}
+	env := evergreen.GetEnvironment()
+	cursor, err := env.DB().Collection(VersionCollection).Aggregate(ctx, pipeline)
+	if err != nil {
+		return nil, errors.Wrap(err, "aggregating versions")
+	}
+	err = cursor.All(ctx, &res)
+	if err != nil {
+		return nil, err
+	}
+	if len(res) == 0 {
+		return nil, nil
+	}
+	return &res[0], nil
+}

model/waterfall_test.go

@@ -790,11 +790,20 @@ func TestGetNextRecentActiveWaterfallVersion(t *testing.T) {
 	assert.NoError(t, p.Insert())
 
 	v := Version{
+		Id:                  "v_0",
+		Identifier:          "a_project",
+		Requester:           evergreen.RepotrackerVersionRequester,
+		RevisionOrderNumber: 11,
+		CreateTime:          start,
+		Activated:           utility.TruePtr(),
+	}
+	assert.NoError(t, v.Insert())
+	v = Version{
 		Id:                  "v_1",
 		Identifier:          "a_project",
 		Requester:           evergreen.RepotrackerVersionRequester,
 		RevisionOrderNumber: 10,
-		CreateTime:          start,
+		CreateTime:          start.Add(-2 * time.Minute),
 		Activated:           utility.TruePtr(),
 	}
 	assert.NoError(t, v.Insert())
@@ -831,3 +840,67 @@ func TestGetNextRecentActiveWaterfallVersion(t *testing.T) {
 	require.NotNil(t, version)
 	assert.Equal(t, "v_1", version.Id)
 }
+
+func TestGetNextOlderActiveWaterfallVersion(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	assert.NoError(t, db.ClearCollections(VersionCollection, ProjectRefCollection))
+	start := time.Now()
+	p := ProjectRef{
+		Id:         "a_project",
+		Identifier: "a_project_identifier",
+	}
+	assert.NoError(t, p.Insert())
+
+	v := Version{
+		Id:                  "v_1",
+		Identifier:          "a_project",
+		Requester:           evergreen.RepotrackerVersionRequester,
+		RevisionOrderNumber: 10,
+		CreateTime:          start,
+		Activated:           utility.TruePtr(),
+	}
+	assert.NoError(t, v.Insert())
+	v = Version{
+		Id:                  "v_2",
+		Identifier:          "a_project",
+		Requester:           evergreen.RepotrackerVersionRequester,
+		RevisionOrderNumber: 9,
+		CreateTime:          start.Add(-2 * time.Minute),
+		Activated:           utility.FalsePtr(),
+	}
+	assert.NoError(t, v.Insert())
+	v = Version{
+		Id:                  "v_3",
+		Identifier:          "a_project",
+		Requester:           evergreen.RepotrackerVersionRequester,
+		RevisionOrderNumber: 8,
+		CreateTime:          start.Add(-2 * time.Minute),
+		Activated:           utility.FalsePtr(),
+	}
+	assert.NoError(t, v.Insert())
+	v = Version{
+		Id:                  "v_4",
+		Identifier:          "a_project",
+		Requester:           evergreen.RepotrackerVersionRequester,
+		RevisionOrderNumber: 7,
+		CreateTime:          start.Add(-2 * time.Minute),
+		Activated:           utility.TruePtr(),
+	}
+	assert.NoError(t, v.Insert())
+	v = Version{
+		Id:                  "v_5",
+		Identifier:          "a_project",
+		Requester:           evergreen.RepotrackerVersionRequester,
+		RevisionOrderNumber: 6,
+		CreateTime:          start.Add(-2 * time.Minute),
+		Activated:           utility.TruePtr(),
+	}
+	assert.NoError(t, v.Insert())
+
+	version, err := GetNextOlderActiveWaterfallVersion(ctx, p.Id, 10)
+	assert.NoError(t, err)
+	require.NotNil(t, version)
+	assert.Equal(t, "v_4", version.Id)
+}