Page MenuHomePhabricator

[native] Set user avatar after registering in registration flow
ClosedPublic

Authored by ashoat on Jun 8 2023, 6:57 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 2, 9:19 PM
Unknown Object (File)
Oct 27 2024, 2:48 AM
Unknown Object (File)
Oct 26 2024, 6:25 AM
Unknown Object (File)
Oct 2 2024, 6:33 AM
Unknown Object (File)
Oct 2 2024, 6:33 AM
Unknown Object (File)
Oct 2 2024, 6:33 AM
Unknown Object (File)
Oct 2 2024, 6:28 AM
Unknown Object (File)
Oct 2 2024, 3:22 AM
Subscribers

Details

Summary

I initially considered passing the avatar directly to the registration endpoint, but realized that gets complicated since we need a logged-in user in order to do a media upload.

Instead, we handle the user avatar after the registration call, but we block the UI from appearing logged-in until after the avatar upload concludes.

If the avatar upload fails, we'll log the user in anyways, and display an error Alert. The user can then go and change their avatar using the normal flow. I opted for this approach because I figured trying to "undo" the registration would be more complicated, and leaving the registration in a half-complete state would be confusing to the user.

Note that I had to implement this in multiple "steps", with subsequent steps handled by a useEffect. The reason for this is that if everything was done in one async function, the Redux-bound values in the callback wouldn't be rebound after registration. See the new code comment at the top of the file for more context.

Depends on D8151

Test Plan

The whole stack was tested as follows:

  1. Try creating an account with the default avatar
  2. Try creating an account with an emoji avatar
  3. Try creating an account with an image avatar (that needs to be uploaded)
  4. Try creating an Ethereum account with an ENS avatar

I made sure of the following:

  1. Loading spinner appeared on the button, not on the avatar, and was present throughout
  2. After the RegistrationNavigator is dismissed, the avatar that appears for the user's PRIVATE chat in the ChatThreadList is correct
  3. Subsequent delete-account-and-then-register-again worked correctly
  4. Various error cases are triggered correctly

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/account/registration/registration-server-call.js
132 ↗(On Diff #27560)

This ESLint rule listed two reasons for its existence:

If an async executor function throws an error, the error will be lost and won’t cause the newly-constructed Promise to reject. This could make it difficult to debug and handle some errors.

This is addressed by wrapping the whole thing in a try-catch.

If a Promise executor function is using await, this is usually a sign that it is not actually necessary to use the new Promise constructor, or the scope of the new Promise constructor can be reduced.

This is not the case for us. We need direct access to the resolve and reject functions so we can pass them to the next step, but we want to use an async function because there are some steps we want to await beforehand.

207 ↗(On Diff #27560)

The reason for always resolveing (even in an error case) are addressed in the diff description.

ashoat requested review of this revision.Jun 8 2023, 7:27 AM

If the avatar upload fails, we'll log the user in anyways, and display an error Alert. The user can then go and change their avatar using the normal flow. I opted for this approach because I figured trying to "undo" the registration would be more complicated, and leaving the registration in a half-complete state would be confusing to the user.

Thanks for explaining, makes sense

This revision is now accepted and ready to land.Jun 9 2023, 12:25 PM