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

Add HTML validation to registration form #9245

Conversation

rebecca-shoptaw
Copy link
Collaborator

@rebecca-shoptaw rebecca-shoptaw commented May 9, 2024

Closes #9244.

Feature. Adds HTML attributes to registration form inputs to prevent submission with improperly formatted data.

Technical

Implementation involved removing validate class to prevent duplicated realtime validation, and adding appropriate attributes to inputs in forms.py. Also involved triggering the HTML validation manually before the grecaptcha process, a strategy that will likely be re-used/expanded for part 2 of #9205.

Testing

  1. Log out (if logged in)
  2. Hit "Sign Up" or go to /account/create
  3. Try entering an improperly formatted password, username or email
  4. Ignore the on-screen warning and submit the form
  5. Submission should be prevented and you should see an HTML built-in error message pop up next to the first badly formatted input

Screenshot

Incorrect email format Incorrect username format Incorrect password length

Stakeholders

@cdrini

@rebecca-shoptaw rebecca-shoptaw marked this pull request as ready for review May 9, 2024 15:38
@rebecca-shoptaw rebecca-shoptaw marked this pull request as draft May 9, 2024 15:44
@rebecca-shoptaw rebecca-shoptaw marked this pull request as ready for review May 9, 2024 17:08
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Code looks good, a few opportunities to DRY things. Putting up on testing now for a whirl

openlibrary/plugins/upstream/forms.py Outdated Show resolved Hide resolved
openlibrary/plugins/upstream/forms.py Outdated Show resolved Hide resolved
@cdrini cdrini added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label May 13, 2024
@cdrini
Copy link
Collaborator

cdrini commented May 13, 2024

tested on testing.openlibrary.org and I can't seem to get the email html error to appear 🤔

@rebecca-shoptaw rebecca-shoptaw force-pushed the 9244/feature/add-html-validation-to-registration-form branch 2 times, most recently from cc55b39 to b00336e Compare May 13, 2024 19:30
@mekarpeles mekarpeles assigned jimchamp and cdrini and unassigned jimchamp May 13, 2024
@rebecca-shoptaw rebecca-shoptaw force-pushed the 9244/feature/add-html-validation-to-registration-form branch 2 times, most recently from 3844f63 to 732820a Compare May 13, 2024 20:16
@rebecca-shoptaw
Copy link
Collaborator Author

rebecca-shoptaw commented May 13, 2024

tested on testing.openlibrary.org and I can't seem to get the email html error to appear 🤔

Update: Fixed! Was a result of grecaptcha blocking the validation. 🙂

@rebecca-shoptaw rebecca-shoptaw force-pushed the 9244/feature/add-html-validation-to-registration-form branch from 732820a to ec98583 Compare May 13, 2024 21:03
@rebecca-shoptaw rebecca-shoptaw added Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. State: Blocked Work has stopped, waiting for something (Info, Dependent fix, etc. See comments). [managed] and removed Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. labels May 14, 2024
@rebecca-shoptaw rebecca-shoptaw force-pushed the 9244/feature/add-html-validation-to-registration-form branch from ec98583 to 749b4c0 Compare May 16, 2024 19:21
@rebecca-shoptaw rebecca-shoptaw removed the State: Blocked Work has stopped, waiting for something (Info, Dependent fix, etc. See comments). [managed] label May 16, 2024
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Code lgtm ; we noticed we were having trouble creating an account on testing.openlibrary.org and staging.openlibrary.org ! I didn't test doing a full registration flow on the realtime validation PR that preceded this one, so perhaps something is up there.

Otherwise I tested on FF and the errors were showing up in the right spots! 👍

@cdrini cdrini added Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] Needs: Testing labels May 20, 2024
@rebecca-shoptaw rebecca-shoptaw force-pushed the 9244/feature/add-html-validation-to-registration-form branch from 749b4c0 to 91b86cd Compare May 20, 2024 21:42
@github-actions github-actions bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label May 20, 2024
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Ok! I played around with it a bit more, and it seems like the + emails no longer work! They're treated as already registered by IA. I found a workaround for testing it though, and was able to fully create an account on testing 👍

But it seems like our realtime email validation does not correctly detect this. Would you mind creating an issue to fix this?

@cdrini cdrini merged commit c7e4e2b into internetarchive:master May 23, 2024
5 checks passed
@rebecca-shoptaw
Copy link
Collaborator Author

@cdrini sure thing!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Testing On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add HTML validation and remove jQuery validation from registration form
3 participants