ocadotechnology / codeforlife-portal

The portal code for Code for Life
https://www.codeforlife.education/
Other
51 stars 79 forks source link

fix: Add password visibility toggle and strength checker to teacher invitation registration form #2372

Closed besque closed 1 month ago

besque commented 1 month ago

This pr fixes issue #2304

Description

Fixed missing JavaScript functionality in teacher invitation registration form. Added prefix to match main registration form's field IDs, which restored:

Root cause:

How Has This Been Tested?

  1. Logged in as a school admin
  2. Invited a teacher to join the school using an unused email address
  3. Followed the registration link
  4. Verified that:
    • Password reveal (eye) icon appears and functions correctly
    • Password strength checker shows and updates while typing
    • Behavior matches the main registration form exactly
    • Both password and confirm password fields work as expected

Checklist:


This change is Reviewable

besque commented 1 month ago

Ah, I suppose the tests are failing because of the prefix, the test would be expecting the field name without the prefix. I've modified the InvitedTeacherForm in teach.py to keep the original field names while still getting the ID's we want.

Hope this fixes it, really sorry πŸ˜…

Please check and let me know, Thank you!

faucomte97 commented 1 month ago

@besque It looks like you're getting a different error now - seems like name isn't an attribute of the fields

besque commented 1 month ago

My apologies, I'm looking into this

faucomte97 commented 1 month ago

No worries! The test logs should hopefully have clear error messages but if you have any questions feel free to ask πŸ˜„

besque commented 1 month ago

Ahh, I was trying to acess the field's name from the field object, I've updated the code to acess the field name directly from the dictionary key instead.

This should do the job

faucomte97 commented 1 month ago

Hi @besque, I might be wrong but I think you just need to update the failing test so that the correct ID is used - similarly to how this test uses the full ID with the prefix too

besque commented 1 month ago

hey @faucomte97,

You're right, I've updated it so that even the prefix is included so it shouldn't give that error anymore.

Sorry for the delay πŸ˜