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 synonyms pagination #34

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

elainenaomi
Copy link

This PR fixes a bug in the pagination of the synonyms list returned by synonyms/search endpoint.

This endpoint only returns the total number of hits instead of the number of pages

It also incorporates the changes from previous PR.

Note: The query rule is a paid feature, and since I don't have a paid personal key I also opened the PR as draft to run the test suite.

@elainenaomi elainenaomi changed the title [WIP] Fix synonyms pagination Fix synonyms pagination Mar 9, 2020
{:ok, %{"hits" => hits, "nbPages" => pages}} when page + 1 < pages ->
page_hits(hits)

{:ok, %{"hits" => hits, "nbHits" => nbHits}} when page + 1 < nbHits / hits_per_page ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this an edge case when Algolia doesn't return nbPages in their response JSON?

* - `consequence` Required at least 1 [consequence](https://www.algolia.com/doc/api-reference/api-methods/save-rule/?language=javascript#method-param-consequence-2)
* -- `params`: Additional search parameters. Any valid search parameter is allowed.
* -- `promote`: List with objects to promote as hits.
* --- `objectID`
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pretty cool. How does it look like in ex_doc's output?

@elainenaomi elainenaomi marked this pull request as ready for review March 9, 2020 21:41
@elainenaomi
Copy link
Author

Hi @sikanhe 👋 - can you help us in the review of this PR, please? 🙂

@sikanhe
Copy link
Owner

sikanhe commented Apr 14, 2020

Hi I am looking for a new owner/maintainer. I have not used Elixir in over three years

@elainenaomi
Copy link
Author

Hi @sikanhe - Thanks for your answer
Can we talk in private about it? TheRealReal has a great interest in keeping the repo updated. We already maintain a fork from this repo with new features implemented: https://github.com/TheRealReal/algolia-elixir

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.

None yet

5 participants