Skip to content

Conversation

Yoann-Abbes
Copy link
Contributor

@Yoann-Abbes Yoann-Abbes commented Dec 16, 2020

What does this PR do?

Add document:upsert API action (fix #572)

@codecov
Copy link

codecov bot commented Dec 16, 2020

Codecov Report

Merging #576 (2c018bf) into 7-dev (e1d55fc) will decrease coverage by 0.66%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##            7-dev     #576      +/-   ##
==========================================
- Coverage   88.28%   87.61%   -0.67%     
==========================================
  Files          32       32              
  Lines        1451     1462      +11     
  Branches      254      261       +7     
==========================================
  Hits         1281     1281              
- Misses        120      130      +10     
- Partials       50       51       +1     
Impacted Files Coverage Δ
src/Kuzzle.ts 86.63% <0.00%> (-0.71%) ⬇️
src/controllers/Document.ts 68.47% <50.00%> (-3.12%) ⬇️
src/controllers/Auth.ts 75.64% <0.00%> (ø)
src/controllers/Security.js 97.03% <0.00%> (ø)
src/controllers/Bulk.js
src/controllers/Bulk.ts 70.58% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1d55fc...2c018bf. Read the comment docs.

Copy link
Contributor

@Aschen Aschen left a comment

Choose a reason for hiding this comment

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

IMHO the signature should be

  upsert (index: string, collection: string, id: string, changes: JSONObject, options: { defaults?: JSONObject, retryOnConflict?: number, refresh?: 'wait_for', source?: boolean })

| `index` | <pre>string</pre> | Index name |
| `collection` | <pre>string</pre> | Collection name |
| `id` | <pre>string</pre> | Document ID |
| `body` | <pre>object</pre> | Partial content of the document to update and fields to add to the document if it gets created |
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't match method signature

* @param _id Unique document identifier
* @param body Partial changes to apply to the document and Fields to add to the document if it gets created (optional)
* @param options
* @returns {Promise<Object>}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @returns {Promise<Object>}
* @returns Information about the updated document

* @param body Partial changes to apply to the document and Fields to add to the document if it gets created (optional)
* @param options
* @returns {Promise<Object>}
* @see https://docs.kuzzle.io/sdk/js/7/controllers/document/upsert/
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be between the description and params description

collection: string,
_id: string,
body: JSONObject,
options: {refresh?: string, retryOnConflict?: boolean, source?: boolean} = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

retryOnConflict is not added to the request. Since this is a common argument, you should check for it's presence in the Kuzzle.query method and add it if present (this bug already exists in prior version of the SDK)

@Yoann-Abbes Yoann-Abbes requested a review from Aschen December 29, 2020 14:37
---
code: true
type: page
title: upsert
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing page description

@Yoann-Abbes Yoann-Abbes requested a review from Aschen December 30, 2020 13:35
Co-authored-by: Sébastien Cottinet <[email protected]>
@Aschen Aschen merged commit 89cb19f into 7-dev Jan 11, 2021
@Aschen Aschen deleted the document-upsert branch January 11, 2021 14:30
@Aschen Aschen mentioned this pull request Jan 17, 2021
Aschen added a commit that referenced this pull request Jan 17, 2021
# [7.5.0](https://github.com/kuzzleio/sdk-javascript/releases/tag/7.5.0) (2021-01-17)


#### New features

- [ [#577](#577) ] Add [auth|security]:checkRights   ([Yoann-Abbes](https://github.com/Yoann-Abbes))
- [ [#576](#576) ] Add document:upsert   ([Yoann-Abbes](https://github.com/Yoann-Abbes))
---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants