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(rest-api-client): prevent infinite loop when condition contains OR clause #726

Merged
merged 3 commits into from
Mar 15, 2021

Conversation

zaki-yama
Copy link
Contributor

@zaki-yama zaki-yama commented Mar 1, 2021

Why

Fixes #716 .

What

Enclose condition parameter of getAllRecords() and getAllRecordsWithId() with parenthesis (()). (See e634fb4)
NOTE: I didn't enclose the conditionit parameter of getAllRecordsWithOffset().

How to test

  1. Create a kintone app, insert more than 500 records
  2. Call getAllRecords() with condition parameter that meets the following conditions:
    • condition contains or clause (e.g. 'Customer != "foo" or Status in ("Completed")' )
    • the number of filtered records also exceeds 500 records
  3. Confirm that all expected records are returned, and the process finishes correctly

You can test it easily using getAllRecords() demo script with modified condition parameter.

Checklist

  • Read CONTRIBUTING.md
  • Updated documentation if it is required.
  • Added tests if it is required. => See 0c810bc
  • Passed yarn lint and yarn test on the root directory.

Sorry, something went wrong.

@zaki-yama zaki-yama marked this pull request as ready for review March 1, 2021 04:48
@zaki-yama zaki-yama requested review from a team, shisama and chick-p and removed request for a team March 1, 2021 04:48
@zaki-yama
Copy link
Contributor Author

I refactored a little in RecordClient.test (1f765e2).
The actual change is 0c810bc.

Copy link
Contributor

@chick-p chick-p left a comment

Choose a reason for hiding this comment

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

@zaki-yama
Thank you for fixing!

Copy link
Contributor

@shisama shisama left a comment

Choose a reason for hiding this comment

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

@zaki-yama Thank you for your work. Code looks good to me. However, does this modification occur a breaking change because the result will change by changing the condition?

@shisama shisama merged commit 6afdd84 into master Mar 15, 2021
@shisama shisama deleted the fix-infinite-loop-by-condition-or branch March 15, 2021 08:11
@tasshi-me tasshi-me added the pkg: rest-api-client @kintone/rest-api-client label Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: rest-api-client @kintone/rest-api-client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Continuously send requests when use getAllRecordsWithId
4 participants