Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Query language support for child iteration & area #2182

Merged
merged 56 commits into from
Oct 3, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
e06b5c6
Query language support for child iteration & area
baijum Jul 25, 2018
375ee86
remove comment
baijum Jul 25, 2018
4fc0296
more test cases
baijum Jul 26, 2018
7047576
child is true by default for area
baijum Jul 26, 2018
fe3528a
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Jul 30, 2018
31b3511
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Jul 31, 2018
0b46360
update query include parent iter
baijum Aug 1, 2018
b53c8b9
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Aug 1, 2018
d19f69f
fix query
baijum Aug 2, 2018
949cb11
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Aug 2, 2018
c43fd0a
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Aug 2, 2018
2db9f35
avoid unwanted select
baijum Aug 2, 2018
09b67fd
do not override child enablement
baijum Aug 2, 2018
10ae3d8
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Aug 2, 2018
5588a87
YAGNI
baijum Aug 3, 2018
9c02141
better description
baijum Aug 3, 2018
924ef7a
no return value
baijum Aug 3, 2018
9afe79c
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Aug 3, 2018
bdf423e
test child expression
baijum Aug 3, 2018
1a6bf6a
Use value based syntax to fetch
baijum Aug 14, 2018
826c6cc
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Aug 14, 2018
8450f41
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Sep 10, 2018
4dc2094
New SQL query based on
baijum Sep 10, 2018
c77e254
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Sep 10, 2018
0c39c44
Update query based on new path representation
baijum Sep 10, 2018
bb8f8a7
Fix tests
baijum Sep 10, 2018
c8b23b5
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Sep 11, 2018
33d6069
Set false option explicitly
baijum Sep 11, 2018
d35ccb4
typo
baijum Sep 11, 2018
a84a234
Revert "Set false option explicitly"
baijum Sep 11, 2018
88e26d1
better test description
baijum Sep 11, 2018
51ef43e
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Sep 11, 2018
f3c1463
typo
baijum Sep 11, 2018
761d3dc
Use apropriate table alias
baijum Sep 11, 2018
28909d0
better comment
baijum Sep 11, 2018
dcbad2b
Avoid unnecessary helper function
baijum Sep 11, 2018
3ef0e22
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Sep 11, 2018
387c959
fix comment
baijum Sep 11, 2018
0c6e586
Merge branch 'master' into wi-2450
baijum Sep 11, 2018
0ed33f8
Fix key used for joins map
baijum Sep 11, 2018
366f81b
remove unrelated changes
baijum Sep 13, 2018
be44bd2
remove ambiguous comment
baijum Sep 13, 2018
80f7103
use space id, better comment, better message
baijum Sep 14, 2018
65e483a
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Sep 18, 2018
1184566
child: true is no more supported
baijum Sep 18, 2018
10b021e
better word
baijum Sep 18, 2018
7e29206
test a error case
baijum Sep 18, 2018
d524455
use compact syntax
baijum Sep 21, 2018
1488af0
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Sep 21, 2018
251645a
Merge branch 'master' into wi-2450
baijum Sep 21, 2018
20a5d4f
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Oct 1, 2018
3e69d32
Use attribute instead of value based syntax
baijum Oct 1, 2018
46f2e98
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Oct 1, 2018
b8392a4
Merge remote-tracking branch 'upstream/master' into wi-2450
baijum Oct 3, 2018
42d62d5
dont' override already set value
baijum Oct 3, 2018
3dbf8fe
test against default
baijum Oct 3, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
213 changes: 213 additions & 0 deletions controller/search_blackbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,219 @@ func (s *searchControllerTestSuite) TestSearchByJoinedData() {
})
}

// TestSearchWorkItemsWithChildIterationsOption verifies the Included list of parents
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please fix the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 387c959

func (s *searchControllerTestSuite) TestSearchWorkItemsWithChildIterationsOption() {
s.T().Run("iterations", func(t *testing.T) {
fxt := tf.NewTestFixture(t, s.DB,
tf.Iterations(3, func(fxt *tf.TestFixture, idx int) error {
i := fxt.Iterations[idx]
switch idx {
case 0:
i.Name = "Top level iteration"
case 1:
i.Name = "Level 1 iteration"
i.MakeChildOf(*fxt.Iterations[idx-1])
case 2:
i.Name = "Level 2 iteration"
i.MakeChildOf(*fxt.Iterations[idx-1])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setup is all good, but if you're searching for something easier to set up please consider using this:

fxt := tf.NewTestFixture(t, s.DB,
  tf.Iterations(3,
    tf.SetIterationNames("Top level iteration", "Level 1 iteration", "Level 2 iteration"),
    func(fxt *tf.TestFixture, idx int) error {
      if idx > 0 {
        fxt.Iterations[idx].MakeChildOf(*fxt.Iterations[idx-1])
      }
    },
  ),
  //...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. d524455

}
return nil
}),

tf.WorkItems(10, func(fxt *tf.TestFixture, idx int) error {
switch idx {
case 0, 1, 2:
fxt.WorkItems[idx].Fields[workitem.SystemIteration] = fxt.Iterations[0].ID.String()
case 3, 4:
fxt.WorkItems[idx].Fields[workitem.SystemIteration] = fxt.Iterations[1].ID.String()
case 5, 6, 7, 8:
fxt.WorkItems[idx].Fields[workitem.SystemIteration] = fxt.Iterations[2].ID.String()
}
return nil
}),
)

spaceIDStr := fxt.Spaces[0].ID.String()

t.Run("without child iteration", func(t *testing.T) {
filter := fmt.Sprintf(`{"iteration": "%s", "child": true}`, fxt.Iterations[2].ID)
_, result := test.ShowSearchOK(t, nil, nil, s.controller, &filter, nil, nil, nil, nil, &spaceIDStr)
require.NotEmpty(t, result.Data)
assert.Len(t, result.Data, 4)
toBeFound := id.MapFromSlice(id.Slice{
fxt.WorkItems[5].ID,
fxt.WorkItems[6].ID,
fxt.WorkItems[7].ID,
fxt.WorkItems[8].ID,
})
for _, wi := range result.Data {
_, ok := toBeFound[*wi.ID]
require.True(t, ok, "unknown work item found: %s", *wi.ID)
delete(toBeFound, *wi.ID)
}
require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound)
jarifibrahim marked this conversation as resolved.
Show resolved Hide resolved
})
t.Run("with one child iteration", func(t *testing.T) {
filter := fmt.Sprintf(`{"iteration": "%s", "child": true}`, fxt.Iterations[1].ID)
_, result := test.ShowSearchOK(t, nil, nil, s.controller, &filter, nil, nil, nil, nil, &spaceIDStr)
require.NotEmpty(t, result.Data)
assert.Len(t, result.Data, 6)
toBeFound := id.MapFromSlice(id.Slice{
fxt.WorkItems[3].ID,
fxt.WorkItems[4].ID,
fxt.WorkItems[5].ID,
fxt.WorkItems[6].ID,
fxt.WorkItems[7].ID,
fxt.WorkItems[8].ID,
})
for _, wi := range result.Data {
_, ok := toBeFound[*wi.ID]
require.True(t, ok, "unknown work item found: %s", *wi.ID)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should say: "unexpected work item found"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 10b021e

delete(toBeFound, *wi.ID)
}
require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound)
jarifibrahim marked this conversation as resolved.
Show resolved Hide resolved
})
t.Run("with two child iteration", func(t *testing.T) {
filter := fmt.Sprintf(`{"iteration": "%s", "child": true}`, fxt.Iterations[0].ID)
_, result := test.ShowSearchOK(t, nil, nil, s.controller, &filter, nil, nil, nil, nil, &spaceIDStr)
require.NotEmpty(t, result.Data)
assert.Len(t, result.Data, 9)
toBeFound := id.MapFromSlice(id.Slice{
fxt.WorkItems[0].ID,
fxt.WorkItems[1].ID,
fxt.WorkItems[2].ID,
fxt.WorkItems[3].ID,
fxt.WorkItems[4].ID,
fxt.WorkItems[5].ID,
fxt.WorkItems[6].ID,
fxt.WorkItems[7].ID,
fxt.WorkItems[8].ID,
})
for _, wi := range result.Data {
_, ok := toBeFound[*wi.ID]
require.True(t, ok, "unknown work item found: %s", *wi.ID)
delete(toBeFound, *wi.ID)
}
require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound)
jarifibrahim marked this conversation as resolved.
Show resolved Hide resolved
})

t.Run("without child iteration - implicit", func(t *testing.T) {
filter := fmt.Sprintf(`{"iteration": "%s"}`, fxt.Iterations[2].ID)
_, result := test.ShowSearchOK(t, nil, nil, s.controller, &filter, nil, nil, nil, nil, &spaceIDStr)
require.NotEmpty(t, result.Data)
assert.Len(t, result.Data, 4)
toBeFound := id.MapFromSlice(id.Slice{
fxt.WorkItems[5].ID,
fxt.WorkItems[6].ID,
fxt.WorkItems[7].ID,
fxt.WorkItems[8].ID,
})
for _, wi := range result.Data {
_, ok := toBeFound[*wi.ID]
require.True(t, ok, "unknown work item found: %s", *wi.ID)
delete(toBeFound, *wi.ID)
}
require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound)
})
t.Run("with one child iteration - implicit", func(t *testing.T) {
filter := fmt.Sprintf(`{"iteration": "%s"}`, fxt.Iterations[1].ID)
_, result := test.ShowSearchOK(t, nil, nil, s.controller, &filter, nil, nil, nil, nil, &spaceIDStr)
require.NotEmpty(t, result.Data)
assert.Len(t, result.Data, 6)
toBeFound := id.MapFromSlice(id.Slice{
fxt.WorkItems[3].ID,
fxt.WorkItems[4].ID,
fxt.WorkItems[5].ID,
fxt.WorkItems[6].ID,
fxt.WorkItems[7].ID,
fxt.WorkItems[8].ID,
})
for _, wi := range result.Data {
_, ok := toBeFound[*wi.ID]
require.True(t, ok, "unknown work item found: %s", *wi.ID)
delete(toBeFound, *wi.ID)
}
require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound)
})
t.Run("with two child iteration - implicit", func(t *testing.T) {
filter := fmt.Sprintf(`{"iteration": "%s"}`, fxt.Iterations[0].ID)
_, result := test.ShowSearchOK(t, nil, nil, s.controller, &filter, nil, nil, nil, nil, &spaceIDStr)
require.NotEmpty(t, result.Data)
assert.Len(t, result.Data, 9)
toBeFound := id.MapFromSlice(id.Slice{
fxt.WorkItems[0].ID,
fxt.WorkItems[1].ID,
fxt.WorkItems[2].ID,
fxt.WorkItems[3].ID,
fxt.WorkItems[4].ID,
fxt.WorkItems[5].ID,
fxt.WorkItems[6].ID,
fxt.WorkItems[7].ID,
fxt.WorkItems[8].ID,
})
for _, wi := range result.Data {
_, ok := toBeFound[*wi.ID]
require.True(t, ok, "unknown work item found: %s", *wi.ID)
delete(toBeFound, *wi.ID)
}
require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound)
})

t.Run("without child iteration - false option", func(t *testing.T) {
filter := fmt.Sprintf(`{"iteration": "%s", "child": false}`, fxt.Iterations[2].ID)
_, result := test.ShowSearchOK(t, nil, nil, s.controller, &filter, nil, nil, nil, nil, &spaceIDStr)
require.NotEmpty(t, result.Data)
assert.Len(t, result.Data, 4)
toBeFound := id.MapFromSlice(id.Slice{
fxt.WorkItems[5].ID,
fxt.WorkItems[6].ID,
fxt.WorkItems[7].ID,
fxt.WorkItems[8].ID,
})
for _, wi := range result.Data {
_, ok := toBeFound[*wi.ID]
require.True(t, ok, "unknown work item found: %s", *wi.ID)
delete(toBeFound, *wi.ID)
}
require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound)
})
t.Run("with one child iteration - false option", func(t *testing.T) {
filter := fmt.Sprintf(`{"iteration": "%s", "child": false}`, fxt.Iterations[1].ID)
_, result := test.ShowSearchOK(t, nil, nil, s.controller, &filter, nil, nil, nil, nil, &spaceIDStr)
require.NotEmpty(t, result.Data)
assert.Len(t, result.Data, 2)
toBeFound := id.MapFromSlice(id.Slice{
fxt.WorkItems[3].ID,
fxt.WorkItems[4].ID,
})
for _, wi := range result.Data {
_, ok := toBeFound[*wi.ID]
require.True(t, ok, "unknown work item found: %s", *wi.ID)
delete(toBeFound, *wi.ID)
}
require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound)
})
t.Run("with two child iteration - false option", func(t *testing.T) {
filter := fmt.Sprintf(`{"iteration": "%s", "child": false}`, fxt.Iterations[0].ID)
_, result := test.ShowSearchOK(t, nil, nil, s.controller, &filter, nil, nil, nil, nil, &spaceIDStr)
require.NotEmpty(t, result.Data)
assert.Len(t, result.Data, 3)
toBeFound := id.MapFromSlice(id.Slice{
fxt.WorkItems[0].ID,
fxt.WorkItems[1].ID,
fxt.WorkItems[2].ID,
})
for _, wi := range result.Data {
_, ok := toBeFound[*wi.ID]
require.True(t, ok, "unknown work item found: %s", *wi.ID)
delete(toBeFound, *wi.ID)
}
require.Empty(t, toBeFound, "failed to found all work items: %+s", toBeFound)
jarifibrahim marked this conversation as resolved.
Show resolved Hide resolved
})
})

}

// TestIncludedParents verifies the Included list of parents
func (s *searchControllerTestSuite) TestIncludedParents() {

Expand Down
20 changes: 20 additions & 0 deletions criteria/expression_child.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package criteria

// ChildExpression represents the negation operator
jarifibrahim marked this conversation as resolved.
Show resolved Hide resolved
type ChildExpression struct {
binaryExpression
}

// Ensure ChildExpression implements the Expression interface
var _ Expression = &ChildExpression{}
var _ Expression = (*ChildExpression)(nil)

// Accept implements ExpressionVisitor
func (t *ChildExpression) Accept(visitor ExpressionVisitor) interface{} {
return visitor.Child(t)
}

// Child constructs a ChildExpression
func Child(left Expression, right Expression) Expression {
return reparent(&ChildExpression{binaryExpression{expression{}, left, right}})
}
1 change: 1 addition & 0 deletions criteria/expression_visitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ type ExpressionVisitor interface {
Parameter(v *ParameterExpression) interface{}
Literal(c *LiteralExpression) interface{}
Not(e *NotExpression) interface{}
Child(e *ChildExpression) interface{}
IsNull(e *IsNullExpression) interface{}
}
4 changes: 4 additions & 0 deletions criteria/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ func (i *postOrderIterator) Not(exp *NotExpression) interface{} {
return i.binary(exp)
}

func (i *postOrderIterator) Child(exp *ChildExpression) interface{} {
return i.binary(exp)
}

func (i *postOrderIterator) IsNull(exp *IsNullExpression) interface{} {
return i.visit(exp)
}
Expand Down
6 changes: 3 additions & 3 deletions search/query_language_whitebox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func TestParseMap(t *testing.T) {
expected := &Query{Name: OR, Children: []Query{
{Name: AND, Children: []Query{
{Name: "space", Value: &openshiftio},
{Name: "area", Value: &area}}},
{Name: "area", Value: &area, Child: true}}},
{Name: AND, Children: []Query{
{Name: "space", Value: &rhel}}},
}}
Expand Down Expand Up @@ -260,7 +260,7 @@ func TestParseMap(t *testing.T) {
expected := &Query{Name: OR, Children: []Query{
{Name: AND, Children: []Query{
{Name: "space", Value: &openshiftio},
{Name: "area", Value: &area}}},
{Name: "area", Value: &area, Child: true}}},
{Name: AND, Children: []Query{
{Name: "space", Value: &rhel, Negate: true}}},
}}
Expand Down Expand Up @@ -288,7 +288,7 @@ func TestParseMap(t *testing.T) {
expected := &Query{Name: OR, Children: []Query{
{Name: AND, Children: []Query{
{Name: "space", Value: &openshiftio},
{Name: "area", Value: &area}}},
{Name: "area", Value: &area, Child: true}}},
{Name: AND, Children: []Query{
{Name: "space", Value: &rhel, Negate: true}}},
}}
Expand Down
23 changes: 20 additions & 3 deletions search/search_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,16 @@ func parseMap(queryMap map[string]interface{}, q *Query) {
q.Name = key
s := string(concreteVal)
q.Value = &s
if q.Name == "iteration" || q.Name == "area" {
q.Child = true
}
case bool:
s := concreteVal
q.Negate = s
if key == "negate" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this fixing? Should this maybe be moved out into its own PR? It seems like it is solving some other problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 366f81b

q.Negate = s
} else if key == "child" {
q.Child = s
}
case nil:
q.Name = key
q.Value = nil
Expand Down Expand Up @@ -378,6 +385,8 @@ type Query struct {
Children []Query
// The Options represent the query options provided by the user.
Options *QueryOptions
// Consider child iteration/area
Child bool
kwk marked this conversation as resolved.
Show resolved Hide resolved
}

func isOperator(str string) bool {
Expand Down Expand Up @@ -436,7 +445,11 @@ func (q Query) generateExpression() (criteria.Expression, error) {
if q.Substring {
myexpr = append(myexpr, criteria.Substring(left, right))
} else {
myexpr = append(myexpr, criteria.Equals(left, right))
if q.Child {
myexpr = append(myexpr, criteria.Child(left, right))
} else {
myexpr = append(myexpr, criteria.Equals(left, right))
}
}
}
} else {
Expand Down Expand Up @@ -477,7 +490,11 @@ func (q Query) generateExpression() (criteria.Expression, error) {
if child.Substring {
myexpr = append(myexpr, criteria.Substring(left, right))
} else {
myexpr = append(myexpr, criteria.Equals(left, right))
if child.Child {
myexpr = append(myexpr, criteria.Child(left, right))
} else {
myexpr = append(myexpr, criteria.Equals(left, right))
}
}
}
} else {
Expand Down
Loading