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

Added 'Verified Teacher' string and label added in account settings... #46888

Merged
merged 3 commits into from
Jun 16, 2022

Conversation

mgc1194
Copy link
Contributor

@mgc1194 mgc1194 commented Jun 16, 2022

… for verified teachers.

Our Customer Support manager has asked if we can add some sort of explicit indicator on the Account Settings page, clearly stating "Verified! " or similar, to reduce confusion. Sometimes we verify folks, but their provided email address (under their Code.org account) isn’t a real working accessible inbox, so they’re verified, but they don’t receive our email letting them know. In general, there’s no indicator on the site stating “you are verified.”

Acceptance criteria

Add “Verified teacher” to the i18n sync

  • String added in en.yml file.

A green checkmark icon (not an interactive component) and the string “Verified teacher” appear below the “Edit Account Details” title on http://studio.code.org/users/edit if the User has an authorized_teacher UserPermission.

  • Verified Teacher green label appears in the account settings page for teacher with authorized_teacher permission.

Links

Testing story

Test Performed
Account settings has been tested locally for the following scenarios:

  • Account with no Permissions.

Captura de Pantalla 2022-06-16 a la(s) 10 28 22 a m

Captura de Pantalla 2022-06-16 a la(s) 10 28 34 a m

  • Account with authorized_teacher permission.

Captura de Pantalla 2022-06-16 a la(s) 10 27 26 a m

Captura de Pantalla 2022-06-16 a la(s) 1 26 58 p m

  • Account with other than authorized_teacher permissions.

Captura de Pantalla 2022-06-16 a la(s) 10 28 50 a m

Captura de Pantalla 2022-06-16 a la(s) 10 29 01 a m

  • Account with multiple permissions included authorized_teacher.

Captura de Pantalla 2022-06-16 a la(s) 10 30 29 a m

Captura de Pantalla 2022-06-16 a la(s) 1 26 58 p m

All tests were successful.

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@mgc1194 mgc1194 requested a review from a team June 16, 2022 16:55
@mgc1194 mgc1194 requested a review from a team as a code owner June 16, 2022 16:55
@dju90
Copy link
Contributor

dju90 commented Jun 16, 2022

Can we capitalize the "v" in "verified"?

@mgc1194 mgc1194 changed the title 'Added Verified' teacher string and label added in account settings for… Added 'Verified Teacher' string and label added in account settings... Jun 16, 2022
@@ -15,6 +15,9 @@
- email_mismatch = current_user.errors.delete(:email_mismatch)
- current_user.reload if email_mismatch.present?
= render "devise/shared/error_messages", resource: resource
- if current_user.permission?(UserPermission::AUTHORIZED_TEACHER)
.field
= f.label '✔ '+t('activerecord.attributes.user.verified_teacher'), class: 'label-bold', style: 'color:green; text-transform: capitalize';
Copy link
Member

Choose a reason for hiding this comment

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

Usually it can be buggy to concatinate two strings together because the behaviour with RTL languages is hard to predict. Please verify this still looks good in a RTL language (Arabic / ar-SA)

Copy link
Contributor

@smusoke smusoke left a comment

Choose a reason for hiding this comment

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

🎉 🎉 🎉

Copy link
Contributor

@KatieShipley KatieShipley left a comment

Choose a reason for hiding this comment

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

@mgc1194 mgc1194 merged commit b772038 into staging Jun 16, 2022
@mgc1194 mgc1194 deleted the fnd-2045-add-verified-status-on-acc-settings branch June 16, 2022 21:09
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

5 participants