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

Additional foorm survey validation on names #40557

Merged
merged 5 commits into from May 21, 2021
Merged

Conversation

tim-dot-org
Copy link
Contributor

@tim-dot-org tim-dot-org commented May 12, 2021

Adds some additional linting validation to the foorm form json editor to prevent "name" and "value" keys from having values outside of alphanumeric and underscores. This stems from an issue where the keys have extra characters stripped away when uploaded to the server.
Functionally, this change hooks into the getAnnotations step of the codeMirror linter, runs the default lint step, and adds additional error messages when needed.

see: code mirror docs

Links

Testing story

Added some unit tests, and tested manually on localhost.

@tim-dot-org tim-dot-org requested a review from a team May 12, 2021 22:08
@breville
Copy link
Member

This is very cool!

nameEntries.forEach(match => {

let match;
while ((match = nameKeyRegex.exec(text)) !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For my understanding, why a while loop here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exec stores some state about where it last found a match, and each time you call it, it finds the next match in the string. when it doesn't find any more matches, it'll return null, and we break out of the loop

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh got it, and I see a similar example in the docs. Thanks!

These values get turned into keys in the query params that get sent to the server
and so we want them to be simple strings
@tim-dot-org
Copy link
Contributor Author

just a rebase on staging to re-trigger tests

@tim-dot-org
Copy link
Contributor Author

the fix i showed you when i was screensharing turned out to actually work, I must have had the saucelabs stuff set up weird

@tim-dot-org tim-dot-org merged commit 0eecaf7 into staging May 21, 2021
@tim-dot-org tim-dot-org deleted the survey-validation branch May 21, 2021 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants