From 4909a90aef93ad7efc600faa43e7f0037a04de54 Mon Sep 17 00:00:00 2001 From: Konrad Kleine <193408+kwk@users.noreply.github.com> Date: Thu, 6 Sep 2018 13:17:10 +0200 Subject: [PATCH] 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 https://github.com/fabric8-services/fabric8-wit/issues/2244. --- search/search_repository.go | 4 +- search/search_repository_blackbox_test.go | 73 +++++++++++++++++++ workitem/expression_compiler.go | 18 +++++ workitem/expression_compiler_blackbox_test.go | 42 ++++++++++- workitem/table_join.go | 6 +- 5 files changed, 137 insertions(+), 6 deletions(-) diff --git a/search/search_repository.go b/search/search_repository.go index 7c551d7ef7..7e765b1e05 100644 --- 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 index b188ff9b95..59b6b45d63 100644 --- 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 index 5755bd0f97..9ec98c358f 100644 --- 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 index 8eca8124d7..4f3d534d60 100644 --- 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 index 98925e4163..900a2d0c5e 100644 --- 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