Integrate Gravatar Quick Editor #23729

pinarol · 2024-10-30T16:04:06Z

Fixes #

Integrates the Gravatar Quick Editor into Jetpack.
Changes are behind the remote feature flag: gravatar_quick_editor

RPReplay_Final1730303727.MP4

RPReplay_Final1730303545.MP4

Technical Details

The new structure depends on the Quick Editor's update callback and fires a notification GravatarQEAvatarUpdateNotification. The UI components that render the avatar subscribe to that notification. Once received, they fetch the latest version of the avatar from the BE. To achieve this we introduce a way to forceRefresh the avatar, which wasn't present before. To achieve forceRefresh we add a cache buster query parameter otherwise the backend cache gets in the way and the latest version of the avatar can't be fetched.

To test:

Enable the feature flag "Gravatar Quick Editor" in Me > App Settings > Debug > Feature Flags (It's enabled by default in DEBUG config)
Log out from the Jetpack app
Log in with a new email (Highly recommend getting a temp email via https://tempmail.so/us , you should go to your inbox in your mobile device since the login depends on a magic link)
Use the email magic link to continue to the app
Observe: You landed on the signup epilogue, the avatar placeholder has a + on it and tapping on the avatar opens the Quick Editor.
Upload & change the avatar via the Quick Editor
Observe: The avatar on the signup page also gets updated
Close the Quick Editor
Continue to the app

Go to Me > My Profile
Tap either on the avatar or on the "Update profile photo" button
Observe: Quick Editor opens
Upload & change the avatar via the Quick Editor
Observe: The avatars on the main app gets updated as well (the one on the profile header, and the "Me" tab bar item at the bottom)

Appearance Test
Go to Me > App Settings > Appearance
Switch the appearance
Go to Me > My Profile
Tap either on the avatar or on the "Update profile photo" button
Observe: Quick Editor opens and the appearance matched with the app.

Disable the feature flag "Gravatar Quick Editor" in Me > App Settings > Debug > Feature Flags

Repeat the tests ☝️
This time instead of the Gravatar Quick Editor you should observe that the system photo picker appears and the selected image is uploaded & set as the avatar.

Regression Notes

Potential unintended areas of impact
The existing upload flow
What I did to test those areas of impact (or what existing automated tests I relied on)
Tested with the feature flag off
What automated tests I added (or what prevented me from doing so)
x

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:

Add forceRefresh option

wpmobilebot · 2024-10-30T16:14:13Z

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr23729-157f792
	Version	25.4.2
	Bundle ID	org.wordpress.alpha
	Commit	`157f792`
	App Center Build	WPiOS - One-Offs #11057

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2024-10-30T16:15:00Z

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr23729-157f792
	Version	25.4.2
	Bundle ID	com.jetpack.alpha
	Commit	`157f792`
	App Center Build	jetpack-installable-builds #10097

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

Add email check when handling the notification `GravatarQEAvatarUpdateNotification`

# Conflicts: # WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift

# Conflicts: # WordPress/WordPress.xcodeproj/project.pbxproj

pinarol · 2024-10-31T16:19:46Z

cc: @kean @crazytonyli

pinarol

Thanks for the review @crazytonyli 🙇 This is ready for another look. (I was AFK last week so this is a bit of a late response)

WordPress/Classes/Extensions/URL+Helpers.swift

WordPressAuthenticator/Sources/Signin/LoginLinkRequestViewController.swift

...henticator/Sources/Unified Auth/View Related/Reusable Views/GravatarEmailTableViewCell.swift

pinarol · 2024-11-11T12:06:11Z

WordPress/Classes/ViewRelated/Me/My Profile/Gravatar/GravatarQuickEditorPresenter.swift

+                Task {
+                    // Purge the cache otherwise the old avatars remain around.
+                    await ImageDownloader.shared.clearURLSessionCache()
+                    await ImageDownloader.shared.clearMemoryCache()


Despite being connected to the same email, avatars scattered throughout the app have usually different URLs depending on the query parameters like size, default image option, rating etc.

And unfortunately URLCache or NSCache doesn't give us chance to iterate over the cache keys and filter based on host and path. We need to know the exact URL to remove an entry which we don't.

Alternatively, we can remove these calls altogether but this causes some avatars to remain old in different places of the app which looks inconsistent and faulty. Also, killing & reopening the app can bring in the old avatar since URLCache has a disk cache. So I recommend keeping these purge operations but let me know if you prefer otherwise.

crazytonyli · 2024-11-13T10:08:41Z

WordPress/Classes/ViewRelated/Gravatar/UIImageView+Gravatar.swift

+                    }
+                }
+            }
+


I'm not so sure about these changes. @kean Can you have a look please?

The app clears caches on onAvatarUpdated and they will be automatically re-populated later. I'm not sure it's worth adding complexity and making task getter internal for this scenario.

Normally we shouldn't need this yes. But unfortunately there's a delay at the backend that causes the old avatar to return for like 25 seconds after selecting a different avatar. So the old avatar comes back on the next request that doesn't have the cache buster parameter and looks like this:

I reported this issue to systems and following it. In the meantime I update the memory cache for the cache buster-less version of the URL so the correct image can be shown next time.

Yeah, this doesn't look great, especially since it replaced the new avatar with the old one. I don't see anything particular wrong with adding this bit of code, so I'm going to leave it up to you.

crazytonyli · 2024-11-13T10:10:37Z

@pinarol Thanks for addressing my comments. They look good to me. I'm not familiar with the image downloading implementation, and asked Alex to have to a look. ⬆️

kean

LGTM with a couple of minor comments.

Some additional feedback:

Clearing all caches isn't ideal but seems acceptable for this scenario. Ideally, it would use a separate cache and/or would rely on HTTP caching.
(UX) It's a bit odd that after you upload a new image, it doesn't change your profile picture.
There is no way to delete an avatar (pretty sure it is also the case in production)

kean · 2024-11-14T17:40:17Z

WordPress/Classes/ViewRelated/Gravatar/UIImageView+Gravatar.swift

+                    }
+                }
+            }
+


The app clears caches on onAvatarUpdated and they will be automatically re-populated later. I'm not sure it's worth adding complexity and making task getter internal for this scenario.

kean · 2024-11-14T17:48:57Z

WordPress/Classes/ViewRelated/Me/My Profile/MyProfileHeaderView.swift

        didSet {
            if let email = gravatarEmail {
-                gravatarImageView.downloadGravatar(for: email, gravatarRating: .x)
+                downloadAvatar()


It produces a new Xcode warning (email unused).

Oh I missed this. Fixed. 👍

pinarol · 2024-11-15T10:37:57Z

Thanks for the review @kean !

(UX) It's a bit odd that after you upload a new image, it doesn't change your profile picture.

Yeah this is the same with web's Avatar Quick Editor as well as in gravatar.com, and a consistent UX was targeted. If no avatar is selected though, the uploaded avatar will be set immediately. But if there's already a selected avatar, the user needs to deliberately change the selection. A kind of precaution to prevent unintended changes to the user's global avatar.

There is no way to delete an avatar (pretty sure it is also the case in production)

This is coming in a couple of months. For now, avatars can be deleted via gravatar.com only. We are waiting for the corresponding REST endpoint and we'll be adding this feature to the QE along with some other improvements.

This reverts commit f0b3eb9.

dangermattic · 2024-11-15T11:42:34Z

	1 Warning
⚠️	This PR is assigned to the milestone 25.6. This milestone is due in less than 4 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

kean · 2024-11-15T12:40:06Z

But if there's already a selected avatar, the user needs to deliberately change the selection. A kind of precaution to prevent unintended changes to the user's global avatar.

It just it appeared broken to me. You can change it back is a single click, so does it need a precaution? If it's not a precaution, it's not clear what user scenario it solves by making a two-step process.

For now, avatars can be deleted via gravatar.com only.

It's be nice if it also includes removing your current avatar as I couldn't find a way to do it.

pinarol · 2024-11-15T13:03:23Z

It just it appeared broken to me. You can change it back is a single click, so does it need a precaution? If it's not a precaution, it's not clear what user scenario it solves by making a two-step process.

Let me take this feedback to our designer. I know that we want a unified experience between web/iOS/Android so I can't change this for iOS only but I am sure we can consider and discuss this.

It's be nice if it also includes removing your current avatar as I couldn't find a way to do it.

You mean on the web? https://gravatar.com/profile/avatars
Because I can do it by removing the selected avatar:

kean · 2024-11-15T16:02:04Z

You mean on the web? https://gravatar.com/profile/avatars. Because I can do it by removing the selected avatar:

Yeah, it would be nice to add soon too, but I think it needs a separate way to "remove current avatar" though without actually deleting it.

Overall, the new experience looks pretty great.

pinarol added 12 commits October 30, 2024 13:31

Add remote FF gravatar_quick_editor

280d66f

Add forceRefresh option for image download methods

2c5b3ae

Force refresh on gravatar update notification

d6dee63

Add GravatarQuickEditorPresenter

d25747d

Display QuickEditor instead of the avatar upload menu

9737971

Move listenForGravatarChanges to upper level

9fd8df8

Add forceRefresh option

Add one more FF check

52a32cd

Adjust the obj-c call

16cc08a

Listen to changes on the MeHeaderView

2694718

Await the image download to update the cached image

7989b14

Listen to gravatar changes on the signup epilogue

e33654f

Add one more FF check

54bb02f

pinarol added this to the 25.6 milestone Oct 30, 2024

pinarol self-assigned this Oct 30, 2024

pinarol added 3 commits October 31, 2024 14:03

Separate the notification name for the QE

9c5f288

Add email check when handling the notification `GravatarQEAvatarUpdateNotification`

Merge branch 'trunk' into wppinar/integrate-gravatar-quick-editor

f3daef7

# Conflicts: # WordPress/Classes/Utility/BuildInformation/RemoteFeatureFlag.swift

Add a release note

74a9dd7

pinarol added the Gravatar Gravatar SDK related updates label Oct 31, 2024

pinarol added 3 commits October 31, 2024 15:04

Revert whitespace change

8d191d4

Merge branch 'trunk' into wppinar/integrate-gravatar-quick-editor

d4d8fd4

# Conflicts: # WordPress/WordPress.xcodeproj/project.pbxproj

Use overrideImageCache`

2fcba8d

pinarol marked this pull request as ready for review October 31, 2024 16:19

pinarol requested review from andrewdmontgomery and etoledom October 31, 2024 16:19

pinarol added 3 commits November 1, 2024 10:51

Fix the old avatar issue

e85a655

Merge branch 'trunk' into wppinar/integrate-gravatar-quick-editor

6b6e745

Update unit test

9247b27

pinarol added this to the 25.7 milestone Nov 11, 2024

pinarol commented Nov 11, 2024

View reviewed changes

crazytonyli reviewed Nov 13, 2024

View reviewed changes

pinarol requested a review from kean November 14, 2024 10:01

kean approved these changes Nov 14, 2024

View reviewed changes

pinarol added 2 commits November 15, 2024 13:02

Fix "unused var" warning

4b7b4fc

Merge branch 'trunk' into wppinar/integrate-gravatar-quick-editor

239f45c

Update release notes

f0b3eb9

pinarol modified the milestones: 25.7, 25.6 Nov 15, 2024

Revert "Update release notes"

157f792

This reverts commit f0b3eb9.

pinarol modified the milestones: 25.6, 25.7 Nov 15, 2024

etoledom approved these changes Nov 15, 2024

View reviewed changes

pinarol added this pull request to the merge queue Nov 15, 2024

Merged via the queue into trunk with commit 41b8062 Nov 15, 2024
26 of 28 checks passed

pinarol deleted the wppinar/integrate-gravatar-quick-editor branch November 15, 2024 13:50

Integrate Gravatar Quick Editor #23729

Integrate Gravatar Quick Editor #23729

Uh oh!

Conversation

pinarol commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Regression Notes

Uh oh!

wpmobilebot commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pinarol commented Oct 31, 2024

Uh oh!

pinarol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crazytonyli commented Nov 13, 2024

Uh oh!

kean left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinarol commented Nov 15, 2024

Uh oh!

dangermattic commented Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kean commented Nov 15, 2024

Uh oh!

pinarol commented Nov 15, 2024

Uh oh!

Uh oh!

kean commented Nov 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

pinarol commented Oct 30, 2024 •

edited

Loading

wpmobilebot commented Oct 30, 2024 •

edited

Loading

wpmobilebot commented Oct 30, 2024 •

edited

Loading

dangermattic commented Nov 15, 2024 •

edited

Loading