parse-server
parse-server copied to clipboard
parse-community
feat: add option to change the default value of the `Parse.Query.limit()` constraint
New Pull Request Checklist
- [x ] I am not disclosing a vulnerability.
- [ x] I am creating this PR in reference to an issue.
Issue Description
Change the default limit of queries' results sets on the server side
Related issue: #8149
Approach
Added defaultLimit configuration option to Parse Server
TODOs before merging
- [x ] Add tests
- [ x] Add changes to documentation (guides, repository pages, in-code descriptions)
- [ x] Add security check
- [ x] Add new Parse Error codes to Parse JS SDK
- [x] A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)
Thanks for opening this pull request!
- ❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.
![parse-github-assistant[bot] avatar](https://avatars.githubusercontent.com/in/136195?v=4 "Published by a pub.dev verified publisher"){kind=small}
Codecov Report
Base: 93.87% // Head: 84.29% // Decreases project coverage by -9.57% :warning:
Coverage data is based on head (
04239e9) compared to base (b96a4cb). Patch coverage: 90.90% of modified lines in pull request are covered.
:exclamation: Current head 04239e9 differs from pull request most recent head 283b7f0. Consider uploading reports for the commit 283b7f0 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## alpha #8152 +/- ##
==========================================
- Coverage 93.87% 84.29% -9.58%
==========================================
Files 182 182
Lines 13737 13744 +7
==========================================
- Hits 12895 11586 -1309
- Misses 842 2158 +1316
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/GraphQL/helpers/objectsQueries.js | 90.62% <ø> (ø) |
|
| src/Options/Definitions.js | 100.00% <ø> (ø) |
|
| src/Options/index.js | 100.00% <ø> (ø) |
|
| src/Config.js | 88.92% <85.71%> (-0.09%) |
:arrow_down: |
| src/Routers/AudiencesRouter.js | 100.00% <100.00%> (ø) |
|
| src/Routers/ClassesRouter.js | 97.95% <100.00%> (ø) |
|
| src/Routers/InstallationsRouter.js | 84.21% <100.00%> (ø) |
|
| ...dapters/Storage/Postgres/PostgresStorageAdapter.js | 2.48% <0.00%> (-93.23%) |
:arrow_down: |
| src/Adapters/Storage/Postgres/PostgresClient.js | 5.00% <0.00%> (-65.00%) |
:arrow_down: |
| src/batch.js | 77.19% <0.00%> (-17.55%) |
:arrow_down: |
| ... and 14 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
@dblythy , the fail was related to the change in code you suggested. I reverted this change and force pushed. I'm not a core Parse Server contributor and not even an experienced Node developer, so I followed the examples I found in the code. If there are any real problems with this PR, I'm all ears. However, if you want to do any cosmetic changes, just go ahead and do what you want, but please don't ask me for additional "styling" work. I think this PR contains an important improvement for Parse Server, so, I'll appreciate if we'll move forward and merge it (unless, again, there is a real problem you see with it). CC @mtrezza
There was just a bracket too much. That's the challenge when writing code directly on GitHub, there is no syntax check. dblythy was right to correct this, we don't use done and fail statements in tests anymore as they tend to complicate code paths and can make test less robust. All good now. Are we all ready to merge?
@vzukanov thanks for your work on this PR. We are so close, just a few minor changes and tests to fix.
I can take on this PR if you would like me to - you can invite me to your branch or I can make a new PR off it - up to you. 😊
@dblythy , thanks for your willingness to assist. I invited you as a collaborator to my fork.
EDITED
My bad, actually it's different from the maxLimit option.
ORIGINAL
Hi @dblythy @vzukanov I think this option is already supported by parse server in the config
No worries - I think we’re good to merge now.
@vzukanov, we just needed to fix some of the failing tests (npm test). You can run individual tests by changing the it("test" to fit("test
Sorry @mtrezza - I am on mobile and I just realised that this throws if maxLimit is less than or equal to 0, so !maxLimit is preferred over maxLimit == null
@dblythy, when I run npm test I get the output that you see in the attached image. I don't know what "Apple Game Center" failures are about, and then there is a failure of the test you added. Not sure why you ask me to fix it.
@vzukanov You can ignore the Apple Game Center tests, they fail unrelated to your PR.
@mtrezza, I'm also going to ignore the other failure because it's related to the additions by @dblythy. Not sure why you expect me to fix unit tests that someone else added.
This is a collaborative environment with a common goal: to get this PR ready for merging. Your contribution is voluntary, so whatever you can / want to do regarding the failing tests is more than welcome. If you cannot / don't want to look into why the tests are failing, that's fine too, you already made a great contribution with the initial PR. Others will hopefully pick this up, it may just take longer for the PR to get merged.
Hey @mtrezza, we've been using this feature on 5.3.0-alpha27 for over a month, but it looks like it didn't make it into any stable versions so far. I'm not sure what's the flow here, so could you tell in which stable version we can expect this flag?
This feature has also been released in 5.4.0-beta.1. Around Dec 1st the beta version will be released as stable. We aim for monthly cycles, so around the beginning of every month alpha becomes beta and beta becomes stable.
We've made an earlier release to free the schedule for the upcoming Parse Server 6 beta release at the end of the month.