Move registrationMode-related logic from `updateImageUserAvatar` to `nativeUpdateUserImageAvatar`
ClosedPublic
Actions

Authored by atul on Jun 27 2023, 5:59 PM.

Details

Reviewers

ashoat
ginsu
rohan

Commits

rCOMMf78ae44c3ece: Move registrationMode-related logic from `updateImageUserAvatar` to…

Summary

Effectively D8343, but for updateImageUserAvatar instead of setUserAvatar.

The registrationMode-related code is native-specific and was pretty interwoven with the logic in setUserAvatar/updateImageUserAvatar.

This diff decouples native-specific from updateImageUserAvatar and moves it to nativeUpdateImageUserAvatar.

As of this diff, updateImageUserAvatar is NOT yet platform-agnostic. We still need to pull out uploadSelectedMedia and want the function to take ImageAvatarDBContent instead of the native-specific NativeMediaSelection. The next diff will address this.

Depends on D8344

Test Plan

Continue to be able to set image avatars on native as expected.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Jun 27 2023, 5:59 PM

Herald added a subscriber: tomek. · View Herald TranscriptJun 27 2023, 5:59 PM

atul published this revision for review.Jun 27 2023, 6:00 PM

atul edited the test plan for this revision. (Show Details)Jun 27 2023, 6:15 PM

atul added a child revision: D8346: Move `native`-specific `uploadSelectedMedia` from `updateImageUserAvatar` to `nativeUpdateUserImageAvatar`.Jun 27 2023, 6:16 PM

atul edited the test plan for this revision. (Show Details)Jun 27 2023, 6:16 PM

Harbormaster completed remote builds in B20563: Diff 28198.Jun 27 2023, 6:17 PM

rohan accepted this revision.Jun 28 2023, 10:10 AM

This revision is now accepted and ready to land.Jun 28 2023, 10:10 AM

Same feedback as in D8343

This revision now requires changes to proceed.Jun 28 2023, 6:20 PM

updateImageUserAvatar previously took (selection: NativeMediaSelection) as an argument to satisfy the mediaSelection: NativeMediaSelection field of the object passed to registrationModeRef.current.successCallback when needsUpload is true.

As of D8346 the argument is changed to (imageAvatarUpdateRequest: ImageAvatarDBContent) so updateUserImageAvatar can be used by both web and native. It becomes the responsibility of platform-specific avatar-hooks to provide platform-agnostic ImageAvatarDBContent to *EditUserAvatarProvider.

Because I was moving the registrationMode stuff out of updateImageUserAvatar, I decided to do the same for setUserAvatar for consistency/symmetry. I thought it'd be best if the registrationMode stuff for image and non-image avatars were handled in the same file in nativeUpdateUserImageAvatar(...) and nativeSetUserAvatar(...) instead of in nativeUpdateUserImageAvatar(...) in avatar-hooks and setUserAvatar(...) in *EditUserAvatarProvider.

registrationMode is only relevant on native because there's no registration flow on web (and based on my understanding there won't be in the foreseeable future). We are leaving setRegistrationMode, getRegistrationModeEnabled, etc here because it doesn't affect anything on web and I wanted to limit the amount of refactor work.