Implement real-time password validation for the registration form

[jimchamp]

Problem

Whenever we removed the second password field from the account registration page, we forgot to remove the code that validates that field's input. The validation function is triggered when the existing password field is blurred, and causes errors when attempting to get the value of the missing field.

Evidence / Screenshot

Relevant URL(s)

https://openlibrary.org/account/create

Reproducing the bug

1. Go to the account registration page
2. Open your browser's dev tools and click the "console" tab
3. Click on the password field, then click on a different field

• Expected behavior: Password validation occurs without error messages in the console
• Actual behavior: There are error messages in the console

Context

• Browser (Chrome, Safari, Firefox, etc): Any
• OS (Windows, Mac, etc): Any
• Logged in (Y/N): N
• Environment (prod, dev, local): prod

Notes from this Issue's Lead

Proposal & constraints

Remove references to the #password2 element from the client-side real-time password validation function. Update any existing unit tests for this function.

Related files

Password validation function:

openlibrary/openlibrary/plugins/openlibrary/js/realtime_account_validation.js

Lines 68 to 88 in 2827e41

	function validatePasswords() {
	var value = document.getElementById('password').value;
	var value2 = document.getElementById('password2').value;
	if (value && value2) {
	if (value2 === value) {
	$('#password2Message').removeClass().addClass('darkgreen').text('');
	$('label[for="password2"]').removeClass();
	$(document.getElementById('password2')).removeClass().addClass('required');
	}
	else {
	$(document.getElementById('password2')).removeClass().addClass('required invalid');
	$('label[for="password2"]').removeClass().addClass('invalid');
	$('#password2Message').removeClass().addClass('invalid').text('Passwords didnt match');
	}
	}
	else {
	$('label[for="password2"]').removeClass();
	$(document.getElementById('password2')).removeClass().addClass('required');
	$('#password2Message').removeClass().text('');
	}
	}

Real-time validation unit tests:
https://github.com/internetarchive/openlibrary/blob/2827e41998825d4ef99fbdc1bc1f3751358102d1/tests/unit/js/realtime_account_validation.test.js

Be sure to update the function name to validatePassword, now that only one password is being validated.

Stakeholders

[devesh-2002]

May I take this issue?

[jimchamp]

Thanks, @devesh-2002. Let me know if you have any questions.

[jimchamp]

Today I learned that this issue is related to #2055, which is currently being worked on by @rebecca-shoptaw. In addition to removing the #password2 references, she'll also be adding some additional requirements sometime in the next few days.

@devesh-2002, can you hold off on creating a pull request for this issue until the new requirements are added here? Sorry for the late changes... If this is too much of a hassle, feel free to look for other good first issues.

[devesh-2002]

Today I learned that this issue is related to #2055, which is currently being worked on by @rebecca-shoptaw. In addition to removing the #password2 references, she'll also be adding some additional requirements sometime in the next few days.

@devesh-2002, can you hold off on creating a pull request for this issue until the new requirements are added here? Sorry for the late changes... If this is too much of a hassle, feel free to look for other good first issues.

Thanks for the update, @jimchamp. I'll hold off on creating the pull request for now and await the additional requirements from @rebecca-shoptaw.

[rebecca-shoptaw]

Note: Another PR may be coming in to handle the i18n process, if you could hold off for now that would be great!

Hi @devesh-2002, thanks so much for taking this on! As part of #2055, we'll need to do a true re-write of this function, rather than simply removing the #password2 references.

To make this a little easier, I've just done a quick PR (#9190) to clean up and streamline the jQuery functions required and fix a bug in the error rendering methods, so you'll want to wait on doing this PR until that one is merged (thus State: Blocked).

Once that's merged, this is what we'll want from the rewrite:

• On blur (i.e. click away), a message that says "Password must be between 3 and 20 characters" should appear next to the password if it is < 3 or > 20 characters long
• The message should disappear (on blur) if the password is the correct length
• The JavaScript tests should check for this behavior instead of matching passwords

Most of this logic is already written, so this PR will just involve rewriting the validatePassword function to check password length and render/clear errors using the new renderError and clearError helper functions (once #9190 is merged in).

The only slightly finicky element of it is the JavaScript internationalization process which is necessary for making the error message accessible to translators, but I've spelled out the process in detail below, and you can find many examples of it throughout the codebase. 🙂

And here's the process you'll want to follow:

1. For the JavaScript internationalization process, you can put the error message at the top of create.html inside an i18n_strings set, like so:
   

   openlibrary/openlibrary/templates/books/add.html

   Lines 5 to 11 in d6dcacc

   	$ i18n_strings = {
   	$ "invalid_checksum": _("Invalid ISBN checksum digit"),
   	$ "invalid_isbn10": _("ID must be exactly 10 characters [0-9 or X]. For example 0-19-853453-1 or 0198534531"),
   	$ "invalid_isbn13": _("ID must be exactly 13 characters [0-9]. For example 978-3-16-148410-0 or 9783161484100"),
   	$ "invalid_lccn": _("Invalid LC Control Number format"),
   	$ "empty_id": _("Please enter a value for the selected identifier")
   	$ }

2. If you run into conflicts with the existing i18n_strings set (unlikely), you can rename the set something like i18n_strings_registration for now
3. Add a json_encode(i18n_strings) -- or json_encode(i18n_strings_registration) if needed -- to a data-i18n attribute in the main form element in create.html, like so:
   

   openlibrary/openlibrary/templates/books/add.html

   Line 64 in d6dcacc

   <div id="id-errors" class="note hidden" data-i18n="$json_encode(i18n_strings)"></div>

4. Now that the i18n is done, once my jQuery cleanup is all merged, it should be quite simple to modify the password function in realtime_account_validation.js so it works more or less like the email one:
   

   openlibrary/openlibrary/plugins/openlibrary/js/realtime_account_validation.js

   Lines 61 to 79 in 8d50dbe

   	function validateEmail() {
   	var value_email = $(this).val();
   	if (value_email !== '') {
   	$.ajax({
   	url: `/account/validate?email=${encodeURIComponent(value_email)}`,
   	type: 'GET',
   	success: function(errors) {
   	if (errors.email) {
   	renderError('emailAddr', 'emailAddrMessage', errors.email);
   	} else {
   	clearError('emailAddr', 'emailAddrMessage');
   	}
   	}
   	});
   	}
   	else {
   	clearError('emailAddr', 'emailAddrMessage');
   	}
   	}

   • Of course, you don't need the ajax call, just to renderError or clearError depending on whether the password is the correct length
   • For the calls to renderError and clearError, you'll need the input ID (password), the error div ID (passwordMessage), and of course that message stored in data-i18n, which you can access like so, either adding an ID to the form or querying it with form[name="submit"]:
     

     openlibrary/openlibrary/plugins/openlibrary/js/edit.js

     Lines 395 to 398 in d6dcacc

     	const i18nStrings = JSON.parse(document.querySelector('#excerpts-errors').dataset.i18n);
     	if (!data.excerpt) {
     	return error('#excerpts-errors', '#excerpts-excerpt', i18nStrings['empty_excerpt']);

5. You'll then have to do a quick rewrite of the call to the validatePassword function, so that it reflects the new name and only refers to one password element
6. You'll then want to rewrite the tests in realtime_account_validation.test.js so that they only refer to a single password input, and check for password length rather than matching

That's it!! You're welcome to get started on the internationalization before the jQuery PR is done, and you can let me know if you have any questions here or on Slack! Thanks again!!

[cdrini]

@devesh-2002 we've been looking at the landscape of the registration form and we've decided that this one might not be as "good first issue" as we initially thought, and it might be best for @rebecca-shoptaw to tackle this as part of her larger registration page work.

You can find some other good first issues here: https://github.com/internetarchive/openlibrary/issues?q=is%3Aissue+is%3Aopen+label%3A%22Good+First+Issue%22 . Perhaps this one #9141 ? Apologies for any confusion!