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

CRDCDH-1686 Base implementation for Emoji Validation #487

Merged
merged 2 commits into from
Oct 10, 2024
Merged

Conversation

amattu2
Copy link
Member

@amattu2 amattu2 commented Oct 10, 2024

Overview

This PR "fixes" a few places where emojis were implicitly allowed, it also makes some related changes to our TS project config.

Change Details (Specifics)

  • Sync ESLint and TypeScript ECMA version to ES2015 (ES6)
    • I bumped the ES version to ES6 since that was required to use a unicode RegEx. The browser support for the FE drops slightly, but it's most likely worth the trade-off
  • (Unrelated) remove explicit type injection for TS, and use defaults
  • Create a emoji validation utility function that integrates with React Hook Form (also kinda works with the Questionnaire)
  • Prevent emojis in:
    • Create Data Submission > dbGaPID
    • Create Data Submission > Submission Name
    • Submission Request > Section B > Study Name (This was only per Sofia request, but all other fields are completely out of scope of this bug)
  • Update/create related test coverage

Related Ticket(s)

CRDCDH-1686

@amattu2 amattu2 added this to the 3.1.0 (PMVP-M2) milestone Oct 10, 2024
@amattu2 amattu2 marked this pull request as ready for review October 10, 2024 16:45
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega left a comment

Choose a reason for hiding this comment

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

@amattu2 Everything LGTM, but I do notice the error message is different in the questionnaire than in the dialog. The ticket doesn't really describe the error message so I will leave it at your discretion if you would like to address this. Feel free to merge if you will leave it as-is.

@amattu2
Copy link
Member Author

amattu2 commented Oct 10, 2024

@amattu2 Everything LGTM, but I do notice the error message is different in the questionnaire than in the dialog. The ticket doesn't really describe the error message so I will leave it at your discretion if you would like to address this. Feel free to merge if you will leave it as-is.

Another unfortunate byproduct of the fact that we're using basically custom coded form management on the Submission Request form. To minimize changes to the submission request page/components, I'll leave this alone for now. Ideally it would use the same error message across the board for emojis.

@amattu2 amattu2 merged commit 42509fd into 3.1.0 Oct 10, 2024
5 checks passed
@amattu2 amattu2 deleted the CRDCDH-1686 branch October 10, 2024 20:00
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.

2 participants