[native] Replace logging with `Alert`s in `selectFromGalleryAndUpdateUserAvatar`
ClosedPublic
Actions

Authored by atul on Apr 19 2023, 12:03 PM.

Details

Reviewers

ashoat
ginsu

Commits

rCOMM82a7ceda75f8: [native] Replace logging with `Alert`s in `selectFromGalleryAndUpdateUserAvatar`

Summary

Replace console.log with Alerts so if something goes wrong with media selection/processing/uploading we can at least give the user an indication that the operation failed.

Proper error handling would include cleaning up temporary files + creating a media mission report, but deferring that for now.

NOTE: Currently if dispatchActionPromise of updateUserAvatarCall fails it'll fail silently. One possibility could be to wrap the call like we do in siwe-login-form to introspect (see below), or we could have a useEffect that Alert.alert(...)s if loadingStatus === 'error'?

Depends on D7529

Test Plan

Flipped boolean and observed that Alert appeared as expected:

Simulator Screenshot - Fresh iPhone 14 Pro - 2023-04-19 at 14.55.31.png (2×1 px, 573 KB)

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D7530 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Apr 19 2023, 12:03 PM

Herald added a subscriber: tomek. · View Herald TranscriptApr 19 2023, 12:03 PM

atul published this revision for review.Apr 19 2023, 12:03 PM

atul edited the test plan for this revision. (Show Details)

atul edited the summary of this revision. (Show Details)

atul edited the summary of this revision. (Show Details)Apr 19 2023, 12:06 PM

atul edited the summary of this revision. (Show Details)

atul added a parent revision: D7529: [native] Show `ActivityIndicator` in `EditUserAvatar` during update.

Can we replace the ALERT_TITLES with "Alert title", to make them more human-readable?

One possibility could be to wrap the call like we do in siwe-login-form to introspect (see below)

I think that's the best option. We do that in a bunch of other places in the app, wrapping the server call with a try-catch and showing an Alert if the catch blocks activates

This revision is now accepted and ready to land.Apr 19 2023, 12:20 PM

Harbormaster completed remote builds in B18527: Diff 25385.Apr 19 2023, 12:21 PM

In D7530#223434, @ashoat wrote:

Can we replace the ALERT_TITLES with "Alert title", to make them more human-readable?

Will do

One possibility could be to wrap the call like we do in siwe-login-form to introspect (see below)

I think that's the best option. We do that in a bunch of other places in the app, wrapping the server call with a try-catch and showing an Alert if the catch blocks activates

Gotcha, and without having looked at the code will every scenario that leads to UPDATE_USER_AVATAR_FAILED result in exception being caught with a try/catch around the updateUserAvatarCall call? Was going to take a look, but just in case you know off the top of your head. (I'm guessing that's what happens internally in dispatchActionPromise but I haven't read it to verify)

This is much better than the loadingStatus approach because that'd require setting overrideKey and customKeyName to distinguish between types of avatar update request.