Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added updateByQuery endpoint #1499

Merged
merged 1 commit into from
May 28, 2018
Merged

Added updateByQuery endpoint #1499

merged 1 commit into from
May 28, 2018

Conversation

mmoreram
Copy link
Contributor

@mmoreram mmoreram commented May 25, 2018

  • Added Changelog entry as well under Added block

@mmoreram
Copy link
Contributor Author

Solves #1103

Copy link
Owner

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Changes LGTM. Left a few minor comments to the code docs.

Thanks a lot for using the elasticsearch-php functions to create this endpoint.

@@ -290,6 +291,76 @@ public function testDeleteByQueryWithQueryAndOptions()
$this->assertEquals(0, $response->count());
}

/**
* @group functional
* @group mmoreram
Copy link
Owner

Choose a reason for hiding this comment

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

What does the second group here mean and what is it for?

$response = $index->search('nicolas');
$this->assertEquals(1, $response->count());

// Delete first document
Copy link
Owner

Choose a reason for hiding this comment

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

This only renames it and does not delete it?


$index->refresh();

// Makes sure, document is deleted
Copy link
Owner

Choose a reason for hiding this comment

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

See comment above, it makes sure document is renamed.

$response = $index->search('nicolas');
$this->assertEquals(1, $response->count());

// Delete first document
Copy link
Owner

Choose a reason for hiding this comment

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

Updates all docs?


$index->refresh();

// Makes sure, document is deleted
Copy link
Owner

Choose a reason for hiding this comment

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

See above

@mmoreram
Copy link
Contributor Author

Fixed all docs. I'm sorry, my mistake I didn't check properly tests :)

@ruflin ruflin merged commit 1d169b6 into ruflin:master May 28, 2018
@ruflin
Copy link
Owner

ruflin commented May 28, 2018

@mmoreram Thanks a lot for the contribution.

@mmoreram mmoreram deleted the feature/update-by-query branch May 28, 2018 12:09
@mmoreram
Copy link
Contributor Author

@ruflin thanks to you, for this library :=)

When do you expect you will release next stable version?

@ruflin
Copy link
Owner

ruflin commented May 29, 2018

@mmoreram
Copy link
Contributor Author

@ruflin WEEEE

That deserves a beer if you come to Barcelona ! :D

Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants