Skip to content

Commit

Permalink
Remove specific hack for global aggregation (#1827)
Browse files Browse the repository at this point in the history
* Remove specific hack for global aggregation

* Fix test
  • Loading branch information
deguif authored Oct 26, 2020
1 parent 788711f commit e40a6be
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 5 deletions.
4 changes: 0 additions & 4 deletions src/Aggregation/AbstractAggregation.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,6 @@ public function toArray(): array
{
$array = parent::toArray();

if (\array_key_exists('global_aggregation', $array)) {
// compensate for class name GlobalAggregation
$array = ['global' => new \stdClass()];
}
if (\count($this->_aggs)) {
$array['aggs'] = $this->_convertArrayable($this->_aggs);
}
Expand Down
4 changes: 4 additions & 0 deletions src/Aggregation/GlobalAggregation.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,8 @@
*/
class GlobalAggregation extends AbstractAggregation
{
protected function _getBaseName()
{
return 'global';
}
}
2 changes: 1 addition & 1 deletion tests/Aggregation/GlobalAggregationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class GlobalAggregationTest extends BaseAggregationTest
public function testToArray(): void
{
$expected = [
'global' => new \stdClass(),
'global' => [],
'aggs' => [
'avg_price' => ['avg' => ['field' => 'price']],
],
Expand Down

4 comments on commit e40a6be

@harrisnl
Copy link

Choose a reason for hiding this comment

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

This commit is giving me the following error in Elastic: "Expected [START_OBJECT] under [global], but got a [START_ARRAY] in [subject]"
Using Elastic 7.6.1

@deguif
Copy link
Collaborator Author

@deguif deguif commented on e40a6be Nov 2, 2020

Choose a reason for hiding this comment

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

@harrisnl sorry about that.
Should be fixed by #1860

@deguif
Copy link
Collaborator Author

@deguif deguif commented on e40a6be Nov 3, 2020

Choose a reason for hiding this comment

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

@harrisnl it was merged in master.
Could you update, and check it works on your side?

@harrisnl
Copy link

Choose a reason for hiding this comment

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

@harrisnl it was merged in master.
Could you update, and check it works on your side?

Yes, It's working again!
Thanks for the fast response and fix @deguif

Please sign in to comment.