Skip to content

Conversation

scottinet
Copy link
Contributor

Description

The SearchResult class sends a search_after request to Kuzzle upon calling its next method, if:

  • no scroll_id is available
  • a size option is provided
  • the search query contains a sort modifier

The current implementation considers that sort must be at the root of the API request, while Kuzzle expects it inside the body part of it.
Same thing goes for the search_after argument, which must be put in the request body, not at root level.

@codecov-io
Copy link

codecov-io commented Apr 8, 2019

Codecov Report

Merging #384 into 6-dev will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            6-dev     #384      +/-   ##
==========================================
+ Coverage   96.73%   96.73%   +<.01%     
==========================================
  Files          30       30              
  Lines        1438     1439       +1     
==========================================
+ Hits         1391     1392       +1     
  Misses         47       47
Impacted Files Coverage Δ
src/controllers/searchResult/base.js 94.44% <100%> (+0.1%) ⬆️

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 a61a23d...2c7a4e3. Read the comment docs.

const key = typeof sort === 'string'
? sort
: Object.keys(sort)[0];
const value = key === '_uid'
Copy link
Contributor

Choose a reason for hiding this comment

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

(nitpicking) maybe adding parentheses make it more readable

Suggested change
const value = key === '_uid'
const value = (key === '_uid')

request.body.search_after = [];

for (const sort of this._request.body.sort) {
const key = typeof sort === 'string'
Copy link
Contributor

@Yoann-Abbes Yoann-Abbes Apr 8, 2019

Choose a reason for hiding this comment

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

(nitpicking) maybe adding parentheses here make it more readable

Suggested change
const key = typeof sort === 'string'
const key = (typeof sort === 'string')

@benoitvidis benoitvidis merged commit 6934c0a into 6-dev Apr 8, 2019
This was referenced Apr 29, 2019
Aschen pushed a commit that referenced this pull request Apr 29, 2019
# [6.1.1](https://github.com/kuzzleio/sdk-javascript/releases/tag/6.1.1) (2019-04-29)


#### Bug fixes

- [ [#384](#384) ] [fix] search API: "sort" and "search_after" must be in the requests body   ([scottinet](https://github.com/scottinet))

#### Enhancements

- [ [#388](#388) ] Use BaseController class for controllers   ([Aschen](https://github.com/Aschen))
- [ [#385](#385) ] Add Realtime.createRestrictedUser method   ([Aschen](https://github.com/Aschen))

#### Others

- [ [#387](#387) ] SearchResult.next returns a new instance   ([Aschen](https://github.com/Aschen))
---
@Aschen Aschen mentioned this pull request Jun 14, 2019
Aschen added a commit that referenced this pull request Jun 14, 2019
Release 6.1.2

Bug fixes

    [ #398 ] Fix bulk return (Aschen)
    [ #394 ] Add default values for from/size to document:search (Aschen)
    [ #384 ] Fix search API: "sort" and "search_after" must be in the requests body (scottinet)

Enhancements

    [ #390 ] Add authenticated property on Kuzzle object (Aschen)
    [ #395 ] Proxify kuzzle to avoid mistyping error (thomasarbona)
    [ #389 ] Remove usage of _meta (Aschen)
    [ #391 ] Add isConnected (Aschen)
    [ #388 ] Use BaseController class for controllers (Aschen)
    [ #385 ] Add Security.createRestrictedUser method (Aschen)

Others

    [ #400 ] Fix large document search using scroll (stafyniaksacha)
    [ #387 ] SearchResult.next returns a new instance (Aschen)
@Aschen Aschen deleted the fix-search_after branch September 25, 2019 07:31
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.

5 participants