DEVPROD-11769: Improve waterfall handling for inactive versions (#8361)

graphql/query_resolver.go

@@ -1000,15 +1000,23 @@ 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 {
+	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
+		minVersionOrder = 0
 	}
 
 	// Same as above, but subtract for max order
 	maxVersionOrder := maxOrderOpt - 1
-	if maxOrderOpt == 0 {
-		// Same as above: only use the first active version if no maxOrder was specified to avoid omitting inactive versions.
+	if len(activeVersions) == 0 {
+		maxVersionOrder = 0
+	} else if maxOrderOpt == 0 && minOrderOpt == 0 {
+		// 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 {
+		// If we're paginating backwards, use the newest active version as the upper bound
 		maxVersionOrder = activeVersions[0].RevisionOrderNumber
 	}
 
@@ -1026,26 +1034,32 @@ func (r *queryResolver) Waterfall(ctx context.Context, options WaterfallOptions)
 		activeVersionIds = append(activeVersionIds, v.Id)
 	}
 
-	waterfallVersions := groupInactiveVersions(activeVersionIds, allVersions)
+	waterfallVersions := groupInactiveVersions(allVersions)
+	bv := []*model.WaterfallBuildVariant{}
 
-	buildVariants, err := model.GetWaterfallBuildVariants(ctx, activeVersionIds)
-	if err != nil {
-		return nil, InternalServerError.Send(ctx, fmt.Sprintf("getting waterfall build variants: %s", err.Error()))
-	}
+	if len(activeVersionIds) > 0 {
+		buildVariants, err := model.GetWaterfallBuildVariants(ctx, activeVersionIds)
+		if err != nil {
+			return nil, InternalServerError.Send(ctx, fmt.Sprintf("getting waterfall build variants: %s", err.Error()))
+		}
 
-	bv := []*model.WaterfallBuildVariant{}
-	for _, b := range buildVariants {
-		bCopy := b
-		bv = append(bv, &bCopy)
+		for _, b := range buildVariants {
+			bCopy := b
+			bv = append(bv, &bCopy)
+		}
 	}
 
-	// Return the min and max orders returned to be used as parameters for navigating to the next page
-	prevPageOrder := allVersions[0].RevisionOrderNumber
-	nextPageOrder := allVersions[len(allVersions)-1].RevisionOrderNumber
+	prevPageOrder := 0
+	nextPageOrder := 0
+	if len(allVersions) > 0 {
+		// Return the min and max orders returned to be used as parameters for navigating to the next page
+		prevPageOrder = allVersions[0].RevisionOrderNumber
+		nextPageOrder = allVersions[len(allVersions)-1].RevisionOrderNumber
 
-	// If loading base page, there's no prev page to navigate to regardless of max order
-	if maxOrderOpt == 0 && minOrderOpt == 0 {
-		prevPageOrder = 0
+		// If loading base page, there's no prev page to navigate to regardless of max order
+		if maxOrderOpt == 0 && minOrderOpt == 0 {
+			prevPageOrder = 0
+		}
 	}
 
 	return &Waterfall{

graphql/tests/query/waterfall/data.json

@@ -89,6 +89,36 @@
       "author": "mohamed.khelif",
       "status": "created",
       "order": 45
+    },
+    {
+      "_id": "inactive1",
+      "gitspec": "abc",
+      "identifier": "mci",
+      "r": "gitter_request",
+      "activated": false,
+      "author": "sophie.stadler",
+      "status": "inactive",
+      "order": 1
+    },
+    {
+      "_id": "inactive2",
+      "gitspec": "abc",
+      "identifier": "mci",
+      "r": "gitter_request",
+      "activated": false,
+      "author": "sophie.stadler",
+      "status": "inactive",
+      "order": 2
+    },
+    {
+      "_id": "inactive3",
+      "gitspec": "abc",
+      "identifier": "mci",
+      "r": "gitter_request",
+      "activated": false,
+      "author": "sophie.stadler",
+      "status": "inactive",
+      "order": 3
     }
   ],
   "tasks": [
@@ -169,8 +199,12 @@
       "identifier": "spruce-identifier"
     },
     {
-      "_id": "evergreen",
-      "identifier": "evergreen"
+      "_id": "mci",
+      "identifier": "mci"
+    },
+    {
+      "_id": "sandbox_project_id",
+      "identifier": "project_sandbox"
     }
   ],
   "builds": [

graphql/tests/query/waterfall/queries/all_inactive_versions.graphql

@@ -0,0 +1,20 @@
+{
+  waterfall(options: { projectIdentifier: "mci" }) {
+    nextPageOrder
+    prevPageOrder
+    versions {
+      version {
+        activated
+        author
+        id
+        order
+      }
+      inactiveVersions {
+        activated
+        author
+        id
+        order
+      }
+    }
+  }
+}

graphql/tests/query/waterfall/queries/no_versions.graphql

@@ -0,0 +1,20 @@
+{
+  waterfall(options: { projectIdentifier: "project_sandbox" }) {
+    nextPageOrder
+    prevPageOrder
+    versions {
+      version {
+        activated
+        author
+        id
+        order
+      }
+      inactiveVersions {
+        activated
+        author
+        id
+        order
+      }
+    }
+  }
+}

graphql/tests/query/waterfall/results.json

@@ -252,6 +252,54 @@
           }
         }
       }
+    },
+    {
+      "query_file": "all_inactive_versions.graphql",
+      "result": {
+        "data": {
+          "waterfall": {
+            "nextPageOrder": 1,
+            "prevPageOrder": 0,
+            "versions": [
+              {
+                "version": null,
+                "inactiveVersions": [
+                  {
+                    "activated": false,
+                    "author": "sophie.stadler",
+                    "id": "inactive3",
+                    "order": 3
+                  },
+                  {
+                    "activated": false,
+                    "author": "sophie.stadler",
+                    "id": "inactive2",
+                    "order": 2
+                  },
+                  {
+                    "activated": false,
+                    "author": "sophie.stadler",
+                    "id": "inactive1",
+                    "order": 1
+                  }
+                ]
+              }
+            ]
+          }
+        }
+      }
+    },
+    {
+      "query_file": "no_versions.graphql",
+      "result": {
+        "data": {
+          "waterfall": {
+            "nextPageOrder": 0,
+            "prevPageOrder": 0,
+            "versions": []
+          }
+        }
+      }
     }
   ]
 }

graphql/util.go

@@ -1320,11 +1320,11 @@ func annotationPermissionHelper(ctx context.Context, taskID string, execution *i
 }
 
 // groupInactiveVersions partitions a slice of versions into a slice where each entry is either an active version or slice of inactive versions (i.e. versions that don't match filters; they may be technically activated).
-func groupInactiveVersions(activeVersionIds []string, versions []model.Version) []*WaterfallVersion {
+func groupInactiveVersions(versions []model.Version) []*WaterfallVersion {
 	waterfallVersions := []*WaterfallVersion{}
 	i := 0
 	for i < len(versions) {
-		if utility.StringSliceContains(activeVersionIds, versions[i].Id) {
+		if utility.FromBoolPtr(versions[i].Activated) {
 			apiVersion := restModel.APIVersion{}
 			apiVersion.BuildFromService(versions[i])
 			waterfallVersions = append(waterfallVersions, &WaterfallVersion{
@@ -1334,7 +1334,7 @@ func groupInactiveVersions(activeVersionIds []string, versions []model.Version)
 			i++
 		} else {
 			inactiveGroup := []*restModel.APIVersion{}
-			for i < len(versions) && !utility.StringSliceContains(activeVersionIds, versions[i].Id) {
+			for i < len(versions) && !utility.FromBoolPtr(versions[i].Activated) {
 				apiVersion := restModel.APIVersion{}
 				apiVersion.BuildFromService(versions[i])
 				inactiveGroup = append(inactiveGroup, &apiVersion)

graphql/util_test.go

@@ -515,22 +515,14 @@ func TestHasAnnotationPermission(t *testing.T) {
 }
 
 func TestGroupInactiveVersions(t *testing.T) {
-	assert.NoError(t, db.ClearCollections(model.VersionCollection))
-
-	v0 := model.Version{Id: "0"}
-	v1 := model.Version{Id: "1"}
-	assert.NoError(t, v1.Insert())
-	v2 := model.Version{Id: "2"}
-	assert.NoError(t, v2.Insert())
-	v3 := model.Version{Id: "3"}
-	assert.NoError(t, v3.Insert())
-	v4 := model.Version{Id: "4"}
-	assert.NoError(t, v4.Insert())
-	v5 := model.Version{Id: "5"}
-	assert.NoError(t, v5.Insert())
-
-	activeVersionIds := []string{v2.Id, v3.Id, v5.Id}
-	waterfallVersions := groupInactiveVersions(activeVersionIds, []model.Version{v0, v1, v2, v3, v4, v5})
+	v0 := model.Version{Id: "0", Activated: utility.ToBoolPtr(false)}
+	v1 := model.Version{Id: "1", Activated: utility.ToBoolPtr(false)}
+	v2 := model.Version{Id: "2", Activated: utility.ToBoolPtr(true)}
+	v3 := model.Version{Id: "3", Activated: utility.ToBoolPtr(true)}
+	v4 := model.Version{Id: "4", Activated: utility.ToBoolPtr(false)}
+	v5 := model.Version{Id: "5", Activated: utility.ToBoolPtr(true)}
+
+	waterfallVersions := groupInactiveVersions([]model.Version{v0, v1, v2, v3, v4, v5})
 	require.Len(t, waterfallVersions, 5)
 
 	assert.Nil(t, waterfallVersions[0].Version)

model/waterfall.go

@@ -103,19 +103,26 @@ func GetActiveWaterfallVersions(ctx context.Context, projectId string, opts Wate
 
 // GetAllWaterfallVersions returns all of a project's versions within an inclusive range of orders.
 func GetAllWaterfallVersions(ctx context.Context, projectId string, minOrder int, maxOrder int) ([]Version, error) {
-	if minOrder >= maxOrder {
+	if minOrder != 0 && maxOrder != 0 && minOrder >= maxOrder {
 		return nil, errors.New("minOrder must be less than maxOrder")
 	}
-
 	match := bson.M{
 		VersionIdentifierKey: projectId,
 		VersionRequesterKey: bson.M{
 			"$in": evergreen.SystemVersionRequesterTypes,
 		},
-		VersionRevisionOrderNumberKey: bson.M{
-			"$gte": minOrder,
-			"$lte": maxOrder,
-		},
+	}
+
+	hasOrderFilters := maxOrder != 0 || minOrder != 0
+	if hasOrderFilters {
+		revisionFilter := bson.M{}
+		if minOrder != 0 {
+			revisionFilter["$gte"] = minOrder
+		}
+		if maxOrder != 0 {
+			revisionFilter["$lte"] = maxOrder
+		}
+		match[VersionRevisionOrderNumberKey] = revisionFilter
 	}
 
 	pipeline := []bson.M{{"$match": match}}

model/waterfall_test.go

@@ -191,6 +191,10 @@ func TestGetAllWaterfallVersions(t *testing.T) {
 	assert.NoError(t, err)
 	require.Len(t, versions, 1)
 	assert.EqualValues(t, "v_1", versions[0].Id)
+
+	versions, err = GetAllWaterfallVersions(ctx, p.Id, 0, 0)
+	assert.NoError(t, err)
+	require.Len(t, versions, 5)
 }
 
 func TestGetWaterfallBuildVariants(t *testing.T) {