askorama / orama

🌌 Fast, dependency-free, full-text and vector search engine with typo tolerance, filters, facets, stemming, and more. Works with any JavaScript runtime, browser, server, service!
https://docs.askorama.ai
Other
8.27k stars 273 forks source link

fix: prevent double pagination and bad count value #734

Closed nicolastoulemont closed 2 weeks ago

nicolastoulemont commented 2 weeks ago

Fixes https://github.com/askorama/orama/issues/730 by removing the double pagination which upperbounded the count value and caused some missed documents in the documents fetching later in the execution path.

vercel[bot] commented 2 weeks ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
orama-docs ❌ Failed (Inspect) Jun 10, 2024 8:36am
nicolastoulemont commented 2 weeks ago

@micheleriva Ready for review :) If the PR is good for you, I'm not claiming anything bug bounty fyi

rajeshj11 commented 2 weeks ago

@nicolastoulemont Hey, It appears that the changes you made are identical to the ones I implemented. Could you please clarify the differences or provide additional insights? Thank you.

rajeshj11 commented 2 weeks ago

@micheleriva Ready for review :) If the PR is good for you, I'm not claiming anything bug bounty FYI

this is my first commit. I have missed the test cases. which I have later added. https://github.com/askorama/orama/pull/731/commits/2200f15934babc3b09a6ecdd66622be7063510a0

nicolastoulemont commented 2 weeks ago

@micheleriva Ready for review :) If the PR is good for you, I'm not claiming anything bug bounty FYI

this is my first commit. I have missed the test cases. which I have later added. 2200f15

TLDR: Closing in favor of https://github.com/askorama/orama/pull/731 after updates

For the record, I didn't take your work as much as I implemented the changes I suggested on slack since your PR tests weren't ready, hoping for a quicker merge :)

Happy to close my PR in favor of your tho, especially since you participate in the BB program which I have no interests in myself and fixed your tests. I will add another comment on your PR to help you finish it

rajeshj11 commented 2 weeks ago

@micheleriva Ready for review :) If the PR is good for you, I'm not claiming anything bug bounty FYI

this is my first commit. I have missed the test cases. which I have later added. 2200f15

TLDR: Closing in favor of #731 after updates

For the record, I didn't take your work as much as I implemented the changes I suggested on slack since your PR tests weren't ready, hoping for a quicker merge :)

Happy to close my PR in favor of your tho, especially since you participate in the BB program which I have no interests in myself and fixed your tests. I will add another comment on your PR to help you finish it

I apologize if my previous comment was unclear. I did not mean that I just want to understand what my PR lacked and am awaiting feedback. I have some questions that need clarification. I've made the required changes and pushed them.