Skip to content

Conversation

martakam
Copy link
Member

@martakam martakam commented Nov 9, 2026 •

The purpose of this PR is to check the user-provided parameters to the GraphQL queries, so we do not return unexpected results, or too many rows of data at once.

Specifically:

  • The limit parameter is capped at 100 rows of data at once
  • We no longer allow null values for the parameters fuzzy, offset, sortBy, sortDirection and other that do not make sense if they're set to null explicitly
  • The maximum number of genres the user can search for is now limited to 15
  • We do not allow searching for very long book titles (over 200 characters)

Because of these backend changes, the following has changed on the frontend:

  • The genre selector is limited to 15 genres, and an error toast is shown when the limit is reached
  • The search input field is limited to 200 characters, and an error toast is shown here too when needed

Feel free to test these features when checking the PR.

This branch should be merged after #115, since it builds upon that PR.

Closes #113

If we would leave limit as possibly null, we could have an SQL query with LIMIT NULL, which would return all rows
It does not make sense to allow these to be null.
They should either be set explicitly, or not be set by the user so they use the default.
@martakam martakam added bug Something isn't working enhancement New feature or request labels Nov 9, 2026
@martakam martakam added this to the Deliverable 4 milestone Nov 9, 2026
@martakam martakam self-assigned this Nov 9, 2026
@martakam martakam requested a review from a team as a code owner November 9, 2026 23:15
hestro
hestro previously approved these changes Nov 12, 2026
Copy link
Member

@hestro hestro left a comment

Choose a reason for hiding this comment

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

Nice! This worked as expected for me

@martakam martakam dismissed stale reviews from hestro and johahor November 12, 2026 12:11

The merge-base changed after approval.

johahor
johahor previously approved these changes Nov 12, 2026
Copy link
Member

@johahor johahor left a comment

Choose a reason for hiding this comment

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

Same, works well. Merging.

johahor
johahor previously approved these changes Nov 12, 2026
@martakam martakam dismissed johahor’s stale review November 12, 2026 12:22

The merge-base changed after approval.

@johahor
Copy link
Member

johahor commented Nov 12, 2026

For some reason i can't merge even after approval.. Can somebody else try? @martakam @hestro

@hestro
Copy link
Member

hestro commented Nov 12, 2026

For some reason i can't merge even after approval.. Can somebody else try? @martakam @hestro

Merging is blocked for me as well.....

hestro
hestro previously approved these changes Nov 12, 2026
@martakam martakam dismissed hestro’s stale review November 12, 2026 12:28

The merge-base changed after approval.

@martakam
Copy link
Member Author

martakam commented Nov 12, 2026 •

Merged main into here now, so it should be good, but let's wait for CI first.
(I did not "dismiss" the reviews, that happens automatically)

@martakam martakam merged commit 5f32d5a into main Nov 12, 2026
7 checks passed
Sign in to join this conversation on GitHub.
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: limit the max number of rows you can fetch from the API at once
3 participants