Search by parent (#2275)

Allow to search for work items by `parent.id` and `parent.number`. See https://openshift.io/openshiftio/Openshift_io/plan/detail/450. See #2244.
fabric8-services · Sep 6, 2018 · 4909a90 · 4909a90
1 parent c0eaf33
commit 4909a90
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 6 deletions.
diff --git a/search/search_repository.go b/search/search_repository.go
@@ -578,7 +578,7 @@ func (r *GormSearchRepository) search(ctx context.Context, sqlSearchQueryParamet
 		db = db.Where(query, workItemTypes)
 	}
 
-	db = db.Select("count(*) over () as cnt2 , *").Order("execution_order desc")
+	db = db.Select("count(*) over () as cnt2 , *").Order(workitem.Column(workitem.WorkItemStorage{}.TableName(), "execution_order") + " desc")
 	db = db.Joins(", to_tsquery('english', ?) as query, ts_rank(tsv, query) as rank", sqlSearchQueryParameter)
 	if spaceID != nil {
 		db = db.Where("space_id=?", *spaceID)
@@ -717,7 +717,7 @@ func (r *GormSearchRepository) listItemsFromDB(ctx context.Context, criteria cri
 		db = db.Limit(*limit)
 	}
 
-	db = db.Select("count(*) over () as cnt2 , *").Order("execution_order desc")
+	db = db.Select("count(*) over () as cnt2 , *").Order(workitem.Column(workitem.WorkItemStorage{}.TableName(), "execution_order") + " desc")
 
 	rows, err := db.Rows()
 	defer closeable.Close(ctx, rows)

diff --git a/search/search_repository_blackbox_test.go b/search/search_repository_blackbox_test.go
@@ -256,6 +256,79 @@ func (s *searchRepositoryBlackboxTest) TestSearchBoardColumnID() {
 	})
 }
 
+func (s *searchRepositoryBlackboxTest) TestSearchByParent() {
+	fxt := tf.NewTestFixture(s.T(), s.DB,
+		tf.WorkItems(4, tf.SetWorkItemTitles("grandparent", "parent", "child1", "child2")),
+		tf.WorkItemLinksCustom(3,
+			func(fxt *tf.TestFixture, idx int) error {
+				l := fxt.WorkItemLinks[idx]
+				l.LinkTypeID = link.SystemWorkItemLinkTypeParentChildID
+				switch idx {
+				case 0:
+					l.SourceID = fxt.WorkItemByTitle("grandparent").ID
+					l.TargetID = fxt.WorkItemByTitle("parent").ID
+				case 1:
+					l.SourceID = fxt.WorkItemByTitle("parent").ID
+					l.TargetID = fxt.WorkItemByTitle("child1").ID
+				case 2:
+					l.SourceID = fxt.WorkItemByTitle("parent").ID
+					l.TargetID = fxt.WorkItemByTitle("child2").ID
+				}
+				return nil
+			}),
+	)
+	s.T().Run("search for children of grandparent by ID", func(t *testing.T) {
+		filter := fmt.Sprintf(`{"parent.id": "%s"}`, fxt.WorkItemByTitle("grandparent").ID)
+		res, count, _, _, err := s.searchRepo.Filter(context.Background(), filter, nil, nil, nil)
+		require.NoError(t, err)
+		require.Equal(t, 1, count)
+		require.Len(t, res, count)
+		assert.Equal(t, fxt.WorkItemByTitle("parent").ID, res[0].ID)
+	})
+	s.T().Run("search for children of parent by ID", func(t *testing.T) {
+		filter := fmt.Sprintf(`{"parent.id": "%s"}`, fxt.WorkItemByTitle("parent").ID)
+		res, count, _, _, err := s.searchRepo.Filter(context.Background(), filter, nil, nil, nil)
+		require.NoError(t, err)
+		require.Equal(t, 2, count)
+		require.Len(t, res, count)
+		assert.Equal(t, fxt.WorkItemByTitle("child1").ID, res[1].ID)
+		assert.Equal(t, fxt.WorkItemByTitle("child2").ID, res[0].ID)
+	})
+	s.T().Run("search for children of grandparent by number", func(t *testing.T) {
+		filter := fmt.Sprintf(`{"parent.number": "%d"}`, fxt.WorkItemByTitle("grandparent").Number)
+		res, count, _, _, err := s.searchRepo.Filter(context.Background(), filter, nil, nil, nil)
+		require.NoError(t, err)
+		require.Equal(t, 1, count)
+		require.Len(t, res, count)
+		assert.Equal(t, fxt.WorkItemByTitle("parent").ID, res[0].ID)
+	})
+	s.T().Run("search for children of parent by number", func(t *testing.T) {
+		filter := fmt.Sprintf(`{"parent.number": "%d"}`, fxt.WorkItemByTitle("parent").Number)
+		res, count, _, _, err := s.searchRepo.Filter(context.Background(), filter, nil, nil, nil)
+		require.NoError(t, err)
+		require.Equal(t, 2, count)
+		require.Len(t, res, count)
+		assert.Equal(t, fxt.WorkItemByTitle("child1").ID, res[1].ID)
+		assert.Equal(t, fxt.WorkItemByTitle("child2").ID, res[0].ID)
+	})
+	s.T().Run("search for children of not existing item by ID", func(t *testing.T) {
+		filter := fmt.Sprintf(`{"parent.id": "%s"}`, uuid.NewV4())
+		res, count, _, _, err := s.searchRepo.Filter(context.Background(), filter, nil, nil, nil)
+		require.NoError(t, err)
+		require.Equal(t, 0, count)
+		require.Len(t, res, count)
+		require.Empty(t, res)
+	})
+	s.T().Run("search for children of not existing item by number", func(t *testing.T) {
+		filter := fmt.Sprintf(`{"parent.number": "%d"}`, 12334)
+		res, count, _, _, err := s.searchRepo.Filter(context.Background(), filter, nil, nil, nil)
+		require.NoError(t, err)
+		require.Equal(t, 0, count)
+		require.Len(t, res, count)
+		require.Empty(t, res)
+	})
+}
+
 func (s *searchRepositoryBlackboxTest) TestSearchBoardID() {
 	s.T().Run("board", func(t *testing.T) {
 		fxt := tf.NewTestFixture(t, s.DB,

diff --git a/workitem/expression_compiler.go b/workitem/expression_compiler.go
@@ -206,6 +206,24 @@ var DefaultTableJoins = func() TableJoinMap {
 			"typegroup.": res["typegroup"],
 		},
 	}
+
+	// Filter by parent's ID or human-readable Number
+	res["parent_link"] = &TableJoin{
+		TableName:  "work_item_links",
+		TableAlias: "parent_link",
+		// importing the link package here to get the link type is currently not
+		// possible because of an import cycle
+		On: Column("parent_link", "link_type_id") + "= '25C326A7-6D03-4F5A-B23B-86A9EE4171E9' AND " + Column("parent_link", "target_id") + "=" + Column(WorkItemStorage{}.TableName(), "id"),
+	}
+	res["parent"] = &TableJoin{
+		TableName:          WorkItemStorage{}.TableName(),
+		TableAlias:         "parent",
+		On:                 Column("parent_link", "source_id") + "=" + Column("parent", "id"),
+		AllowedColumns:     []string{"id", "number"},
+		PrefixActivators:   []string{"parent."},
+		ActivateOtherJoins: []string{"parent_link"},
+	}
+
 	return res
 }
 

diff --git a/workitem/expression_compiler_blackbox_test.go b/workitem/expression_compiler_blackbox_test.go
@@ -117,6 +117,41 @@ func TestField(t *testing.T) {
 				c.Equals(c.Field("board.id"), c.Literal("c20882bd-3a70-48a4-9784-3d6735992a43")),
 				`(`+workitem.Column("boardcolumns", "id")+` = ?)`, []interface{}{"c20882bd-3a70-48a4-9784-3d6735992a43"}, []*workitem.TableJoin{&columns})
 		})
+		t.Run("parent", func(t *testing.T) {
+			t.Run("by id", func(t *testing.T) {
+				parent := *defJoins["parent"]
+				parent.Active = true
+				parent.HandledFields = []string{"id"}
+				parent_link := *defJoins["parent_link"]
+				parent_link.Active = true
+				expect(t,
+					c.Equals(c.Field("parent.id"), c.Literal("c20882bd-3a70-48a4-9784-3d6735992a43")),
+					`(`+workitem.Column("parent", "id")+` = ?)`, []interface{}{"c20882bd-3a70-48a4-9784-3d6735992a43"}, []*workitem.TableJoin{&parent_link, &parent})
+			})
+			t.Run("by number", func(t *testing.T) {
+				parent := *defJoins["parent"]
+				parent.Active = true
+				parent.HandledFields = []string{"number"}
+				parent_link := *defJoins["parent_link"]
+				parent_link.Active = true
+				expect(t,
+					c.Equals(c.Field("parent.number"), c.Literal("1234")),
+					`(`+workitem.Column("parent", "number")+` = ?)`, []interface{}{"1234"}, []*workitem.TableJoin{&parent_link, &parent})
+			})
+			t.Run("by number or id", func(t *testing.T) {
+				parent := *defJoins["parent"]
+				parent.Active = true
+				parent.HandledFields = []string{"number", "id"}
+				parent_link := *defJoins["parent_link"]
+				parent_link.Active = true
+				expect(t,
+					c.Or(
+						c.Equals(c.Field("parent.number"), c.Literal("1234")),
+						c.Equals(c.Field("parent.id"), c.Literal("5feea506-b0ab-4913-a08b-fe6a5234fa69")),
+					),
+					`((`+workitem.Column("parent", "number")+` = ?) OR (`+workitem.Column("parent", "id")+` = ?))`, []interface{}{"1234", "5feea506-b0ab-4913-a08b-fe6a5234fa69"}, []*workitem.TableJoin{&parent_link, &parent})
+			})
+		})
 	})
 	t.Run("test illegal field name", func(t *testing.T) {
 		t.Run("double quote", func(t *testing.T) {
@@ -168,7 +203,12 @@ func expect(t *testing.T, expr c.Expression, expectedClause string, expectedPara
 		require.Equal(t, expectedParameters, parameters, "parameters mismatch")
 	})
 	t.Run("check joins", func(t *testing.T) {
-		require.Equal(t, expectedJoins, joins, "joins mismatch")
+		// We could just use `require.Equal` on the two join array but that is
+		// much harder to debug, therefore we do it manually.
+		require.Len(t, joins, len(expectedJoins), "number of joins not matching the expected number of joins")
+		for i, expected := range expectedJoins {
+			require.Equal(t, expected, joins[i], "join at index #%d is not matching", i)
+		}
 	})
 }
 

diff --git a/workitem/table_join.go b/workitem/table_join.go
@@ -43,12 +43,12 @@ type TableJoin struct {
 	// object to be activated.
 	PrefixActivators []string // e.g. []string{"iteration."}
 
-	// disallowedColumns specified all fields that are allowed to be queried
-	// from the foreign table. When empty all columns are allowed.
+	// AllowedColumns specified all fields that are allowed to be queried from
+	// the foreign table. When empty, all columns are allowed.
 	AllowedColumns []string // e.g. ["name"].
 
 	// DisallowedColumns specified all fields that are not allowed to be queried
-	// from the foreign table. When empty all columns are allowed.
+	// from the foreign table. When empty, all columns are allowed.
 	DisallowedColumns []string // e.g. ["created_at"].
 
 	// HandledFields contains those fields that were found to reference this