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

[vtadmin-web] Do not parse numbers/booleans in URL query parameters by default #8100

Merged

Conversation

doeg
Copy link
Contributor

@doeg doeg commented May 11, 2021

Signed-off-by: Sara Bee 855595+doeg@users.noreply.github.com

Description

Currently, VTAdmin by default parses numeric URL query parameters as numbers. This resulted in unintended behaviour with shard filtering, where filtering by a string like "000" (to, say, find shard 0001-0002) would be parsed as 0 (no leading zeroes) and would also evaluate as falsey, which caused a host of other unfun side-effects.

In this screenshot, I tried to filter by "00". Note the ?filter=0 in the URL and the empty text input:

Screen Shot 2021-05-11 at 1 48 50 PM

It turns out that not parsing booleans/numbers is the more common VTAdmin use case. The one exception so far is pagination, were we do want page=1 to parse as an integer.

Here's what it looks like with the fix:

Screen Shot 2021-05-11 at 1 48 42 PM

Related Issue(s)

N/A

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

N/A

…y default

Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
@doeg doeg changed the title [vtadmin-web] Do not parse numbers/booleans in URL query parameters b… [vtadmin-web] Do not parse numbers/booleans in URL query parameters by default May 11, 2021
@doeg doeg force-pushed the sarabee-vtadmin-filtering-defaults branch from df6e0ed to 1e74570 Compare May 11, 2021 18:07
@doeg doeg marked this pull request as ready for review May 11, 2021 18:08
@doeg doeg requested a review from ajm188 as a code owner May 11, 2021 18:08
Copy link
Contributor

@ajm188 ajm188 left a comment

Choose a reason for hiding this comment

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

nice find, and great writeup!

@ajm188 ajm188 merged commit aa7eaab into vitessio:master May 12, 2021
@doeg doeg deleted the sarabee-vtadmin-filtering-defaults branch November 1, 2021 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants