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

fix(batch): add missing BatchActionType #1149

Merged
merged 1 commit into from
May 12, 2020

Conversation

@bodinsamuel bodinsamuel self-assigned this May 4, 2020
@Haroenv Haroenv requested a review from nunomaduro May 4, 2020 11:16
@Haroenv
Copy link
Contributor

Haroenv commented May 4, 2020

Seems to be inconsistent with the docs: https://www.algolia.com/doc/api-reference/api-methods/batch/#method-param-action

Do the docs also need to be updated?

@@ -4,11 +4,15 @@ export const BatchActionEnum: Readonly<Record<string, BatchActionType>> = {
PartialUpdateObject: 'partialUpdateObject',
PartialUpdateObjectNoCreate: 'partialUpdateObjectNoCreate',
DeleteObject: 'deleteObject',
DeleteIndex: 'delete',

Choose a reason for hiding this comment

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

@Ant-hem @aseure can you quick check those in your clients?

Copy link

Choose a reason for hiding this comment

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

What do we need to check? If batch actions do accept delete? Could you open issues with polyglot with the exact context?

Choose a reason for hiding this comment

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

@aseure Correct. I will do it.

Copy link

Choose a reason for hiding this comment

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

@nunomaduro I've just checked and it's available in Go, Scala, Java, C#.

Copy link
Author

Choose a reason for hiding this comment

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

can we merge this one as everybody seems to agree ? ☺️

@bodinsamuel
Copy link
Author

For the ref, we are currently using that in the crawler so it works. Probably need a doc update indeed.

Example:

var algoliasearch = require("algoliasearch");

const client = algoliasearch('<app>', '<key>');
await client.multipleBatch([
    {
      indexName: 'test-multiple-delete',
      action: 'addObject',
      body: { objectID: 'sample-objectID' },
    },
    {
      indexName: 'test-multiple-delete',
      action: 'clear',
      body: {},
    },
    {
      indexName: 'test-multiple-delete',
      action: 'delete',
      body: {},
    },
  ]);

@bodinsamuel
Copy link
Author

bodinsamuel commented May 12, 2020

@Haroenv can you review it during the absence of nuno ? ☺️

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

the code itself is fine, but let's make sure we update the docs after

@Haroenv Haroenv merged commit 5bde85f into master May 12, 2020
@Haroenv Haroenv deleted the fix/add-missing-batch-action-type branch May 12, 2020 14:38
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.

4 participants