Fixed | Nested query with bool doesn't work when there is only one MU… #42
Conversation
|
@trandangtri next time when you do a PR please include a closing comment for a specific issue if there is one. e.g. Closing #32 |
|
@trandangtri please provide a few unit test cases to represent a problem and to make sure our fix works well. We should test:
|
| /** | ||
| * Tests toArray method. | ||
| */ | ||
| public function testToArray() | ||
| { | ||
| $missingFilterMock = $this->getMockBuilder('ONGR\ElasticsearchDSL\Filter\MissingFilter') | ||
| ->setConstructorArgs(['test_field']) | ||
| ->getMock(); | ||
| ->setConstructorArgs(['test_field']) |
There was a problem hiding this comment.
Does not look like intentional changes.
There was a problem hiding this comment.
Automatic reformatting code in my IDE. I turned off.
|
Hi guys, My fix failed in an old test case. But in my opinion, that test case is not good enough. Look at: https://github.com/ongr-io/ElasticsearchDSL/blob/master/tests/Query/BoolQueryTest.php#L75 The expected result is $expected = [
'should' => [
[
'term' => [
'key1' => 'value1',
],
],
],
'must' => [
[
'term' => [
'key2' => 'value2',
],
],
],
'must_not' => [
[
'term' => [
'key3' => 'value3',
],
],
],
];But it should be like this $expected = [
'should' => [
'term' => [
'key1' => 'value1',
],
],
'must' => [
'term' => [
'key2' => 'value2',
],
],
'must_not' => [
'term' => [
'key3' => 'value3',
],
],
];Please take on a look on the official document of ES Bool Query and their example. So I do suggest that we should update the old test case. |
|
Actually both cases are valid, because you can have either array of term queries or single query. I do not see any problems with changing old tests as building logic is changing. |
|
@murnieza, yes, both of them will work well with ES, but the output of |
|
I reverted my work. Now we will always keep the array structure of term query in the output of |
| @@ -111,7 +111,13 @@ public function toArray() | |||
| $mustContainer = $this->container[self::MUST]; | |||
| $query = array_shift($mustContainer); | |||
|
|
|||
| return [$query->getType() => $query->toArray()]; | |||
| if ($query->getType() == 'match') { | |||
There was a problem hiding this comment.
This won't fix the bug. The match query is just like an example of any query. This bug occurs on all other queries also.
|
For the test cases please write a data provider, don't need to duplicate the same check in each test case. btw, when you will use data provider please include in addition also different queries/filters, not only match. Match query was like an example of query, the bug occurs on any query or filter. |
|
fixed in #51 |
Related to this bug #32