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 email verification check before showing the Gravatar Quick Editor #23920

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

pinarol
Copy link
Contributor

@pinarol pinarol commented Dec 23, 2024

Fixes #

Adds an email verification check before showing the Gravatar Quick Editor because a user with an unverified email can't change their avatars due to a recent backend fix. Gravatar needs to have a strict policy that accounts who haven’t verified there email cannot use Gravatar.

The Gravatar Quick Editor handles the unauthorized errors with a session expired message so it's good to have a more accurate and descriptive alert here. In the meantime we'll work on the message on the SDK side.

To test:

You can use a temp inbox like www.emailondeck.com to create an email
Logout and create a new user
Go to Me > My Profile > Add profile photo
Observe: An alert pops saying To update your avatar, you need to verify your email address first.
Go to your inbox, verify the email
Go back to Me and repeat the steps
Observe: Gravatar Quick Editor shows

Regression Notes

  1. Potential unintended areas of impact

  2. What I did to test those areas of impact (or what existing automated tests I relied on)

  3. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@pinarol pinarol added the Gravatar Gravatar SDK related updates label Dec 23, 2024
@pinarol pinarol added this to the 25.7 milestone Dec 23, 2024
@pinarol pinarol self-assigned this Dec 23, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Dec 23, 2024

1 Warning
⚠️ This PR is assigned to the milestone 25.6 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@pinarol pinarol changed the title Add email verification check to show the Gravatar Quick Editor Add email verification check before showing the Gravatar Quick Editor Dec 23, 2024
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 23, 2024

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr23920-0e65dde
Version25.6
Bundle IDorg.wordpress.alpha
Commit0e65dde
App Center BuildWPiOS - One-Offs #11222
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 23, 2024

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr23920-0e65dde
Version25.6
Bundle IDcom.jetpack.alpha
Commit0e65dde
App Center Buildjetpack-installable-builds #10260
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@pinarol pinarol marked this pull request as ready for review December 23, 2024 11:14
@pinarol pinarol requested a review from kean December 23, 2024 11:17
@pinarol pinarol modified the milestones: 25.7, 25.6 ❄️ Dec 23, 2024
@@ -16,9 +17,24 @@ struct GravatarQuickEditorPresenter {
}
self.email = email
self.authToken = account.authToken
self.emailVerificationStatus = account.verificationStatus
Copy link
Contributor

@kean kean Dec 23, 2024

Choose a reason for hiding this comment

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

Hey, I'm not sure how accurate this field is. The usage of it was removed from the app, and I was planning to remove the field as well in favor of relying on the server to tell you which operations are not permitted without hardcoding it on the client. What happens if the client doesn't hardcode this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What happens if the client doesn't hardcode this check?

Sounds good to me but is there a way to currently achieve that on the JP side?

We want to improve our endpoints to know the reason as to why 401 unauthorized is thrown. That way we'll be able to show a better error. Unfortunately we didn't have chance to come up with a better solution on the SDK side yet(holidays).

There has been a very recent backend change on the Gravatar side. Before that, the Quick Editor worked successfully for unverified emails, but it turns out this isn't supposed to be happening so it's fixed urgently. So, a "session expired" error is displayed in the Gravatar Quick Editor for unverified emails which can be misleading and confusing. This is just an attempt to show a better message until we sort things out the SDK side.

Although I am not sure when this change would be live even if we merged this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gravatar Gravatar SDK related updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants