[PAY-1701] Fix "Share to DMs" on mobile to go through InboxUnavailable modal #3878
Conversation
rickyrombo
requested review from
a team and
sddioulde
and removed request for
a team
| if (shouldOpenChat && canCreateChat) { | ||
| dispatch(createChat({ userIds: [userId] })) | ||
| if (!userId) { | ||
| console.error( |
There was a problem hiding this comment.
Just double-checking this is sufficient handling of the error. Does the user see anything? Will we get a message in sentry or some other type of analytics. It reads as a silent failure as is.
There was a problem hiding this comment.
Yeah this is a good q. Sentry does capture our console.error messages, but I was torn about whether to throw here and crash the app, or just log an error, or do nothing. In theory, this should never happen - the drawer should always be opened with a userId - but typecheck makes some of these checks necessary and it felt like I should at least log them if they do occur. Wdyt?
There was a problem hiding this comment.
Makes sense. I just wanted to make sure that we have actionable data if this does happen, as it would probably be in a context where something else has gone wrong in an unexpected way.
but typecheck makes some of these checks necessary
This is a pattern worth thinking about. There's a way to design where values are not typed as potentially null or undefined. That would essentially mean that we throw at a much lower layer (validating values as they come across the wire), and we don't even render a component that would consume a value in cases where the value doesn't exist.
It's kind of a fundamental shift, though. And not a trivial amount of effort, so I'm not pushing it 😅 . This is a reasonable approach given that we can't change the type to be required without doing a lot of other work.
| dispatch(beginTip({ user, source: 'inboxUnavailableModal' })) | ||
| if (!currentUserId || !user) { | ||
| console.error( | ||
| 'Unexpected undefined currentUserId or user when starting tip' |
There was a problem hiding this comment.
Same question here about error propagation.
| chatId: makeChatId([account.user_id, recipient.user_id]) | ||
| }) | ||
| ) | ||
| if (onSuccessActions && account?.user_id && recipient?.user_id) { |
There was a problem hiding this comment.
This is an interesting pattern. Are there any implications of order of actions, failure of an action blocking dispatch of the rest, etc?
There was a problem hiding this comment.
Yeah if an action fails/crashes the app, the next one won't be processed (not that that should be happening - ideally we handle our saga exceptions internally unless fatal). I'm not in love with this pattern, it was the best idea I had though - open to alternatives!
| @@ -24,24 +23,17 @@ const useStyles = makeStyles(({ palette, spacing }) => ({ | |||
| })) | |||
|
|
|||
| type MessageLockedButtonProps = { | |||
| profile: Pick<User, 'user_id'> | |||
| userId: ID | |||
| const user = useSelector((state) => getUser(state, { id: userId })) | ||
| const { canCreateChat, callToAction } = useSelector((state) => | ||
| const { userId, presetMessage } = data | ||
| const user = useSelector((state) => |
There was a problem hiding this comment.
could use audius-query here :)
…e-page * origin/main: [PAY-1720] Implements PlainButton (#3897) Fix minor bugs for multi-track upload demo (#3854) Limit lines in Leaving Audius Modal (#3896) [C-2681, C-2682, C-2683] Add new upload finish page (#3890) [C-2914] USDC purchase options for new upload UI (web) (#3888) Minor UI fixes for leaving audius modal (#3895) Fix OAuth login page width (#3894) Fix playlist form from crashing after double save (#3893) Update seo h1 to be accessibly hidden vs visually hidden (#3892) Move setCollectionPermalink within fetchCollectionSucceeded action (#3867) [plat-1055] revert legacy playlist route formatting in embed player to use permalink (#3824) [PAY-1717] Make sign in/sign up page overlap banner (#3886) [PAY-1658] Artist pick, hidden track tile tags moved to mid-left (#3889) [C-2957] Add h1 tag for SEO (#3887) [C-2685 C-2686] Implement collection upload form (#3870) [PAY-1702] Use existing chats as default user list when sharing to DMs (#3877) [PAY-1701] Fix "Share to DMs" on mobile to go through InboxUnavailable modal (#3878) [PAY-1700] Replace navigation if coming from ChatUserListScreen (#3879) [PAY-1588] Use existing balance in purchase flow on mobile (#3885)
[bd3fdc7] [C-2972] Fix feed, trending track-page seo (#3907) Dylan Jeffers [45279fe] [C-2969] Fix related artist images not loading (#3905) Andrew Mendelsohn [f51e6ac] [C-2961] Fix LeftNav SEO (#3906) Dylan Jeffers [6256792] Fix lint in useUserProfilePicture (#3903) Dylan Jeffers [f7dc55b] [C-2971] Add Avatar (#3902) Dylan Jeffers [ce441c1] [C-2970] Add Link, Improve Text (#3901) Dylan Jeffers [72fcc88] [PAY-1631] Implements post-purchase content on web (#3898) Randy Schott [0f28578] [C-2964] Cannonical URL should be uri encoded (#3899) Raymond Jacobson [3fca3c9] [C-2684 C-2955] Improve upload component hierarchy, state, and validation (#3891) Dylan Jeffers [3138da3] [PAY-1720] Implements PlainButton (#3897) Randy Schott [b353f94] Fix minor bugs for multi-track upload demo (#3854) Andrew Mendelsohn [22d4be5] Limit lines in Leaving Audius Modal (#3896) Marcus Pasell [b73b2f6] [C-2681, C-2682, C-2683] Add new upload finish page (#3890) Kyle Shanks [5d94b1f] [C-2914] USDC purchase options for new upload UI (web) (#3888) Andrew Mendelsohn [30cc8f1] Minor UI fixes for leaving audius modal (#3895) Marcus Pasell [970f411] Fix OAuth login page width (#3894) nicoback2 [deff919] Fix playlist form from crashing after double save (#3893) sabrina-kiam [18227f3] Update seo h1 to be accessibly hidden vs visually hidden (#3892) Dylan Jeffers [594b5c4] Move setCollectionPermalink within fetchCollectionSucceeded action (#3867) sabrina-kiam [9165c76] [plat-1055] revert legacy playlist route formatting in embed player to use permalink (#3824) sabrina-kiam [eb747cf] [PAY-1717] Make sign in/sign up page overlap banner (#3886) Marcus Pasell [2887342] [PAY-1658] Artist pick, hidden track tile tags moved to mid-left (#3889) Reed [2c80443] [C-2957] Add h1 tag for SEO (#3887) Raymond Jacobson [bae86c3] [C-2685 C-2686] Implement collection upload form (#3870) Dylan Jeffers [1c7ec6f] [PAY-1702] Use existing chats as default user list when sharing to DMs (#3877) Marcus Pasell [bf09344] [PAY-1701] Fix "Share to DMs" on mobile to go through InboxUnavailable modal (#3878) Marcus Pasell [9c9d7f4] [PAY-1700] Replace navigation if coming from ChatUserListScreen (#3879) Marcus Pasell [eaef4e6] [PAY-1588] Use existing balance in purchase flow on mobile (#3885) Reed [49d1d05] Add fb share page (#3876) Raymond Jacobson [46c33eb] Change nullish check to falsey check for collection track times (#3884) Kyle Shanks [b773045] Update twitter icon on mobile (#3880) Reed [dfc6c60] [PAY-1707] Implements usage of existing balance during content purchases (#3883) Randy Schott [5b4242f] [PAY-1592] Wire up USDC purchase flow on mobile (#3881) Reed [41cc4af] [PAY-1629] Purchase flow cleanup (#3873) Randy Schott
Description
Follow up for #3874 but for mobile
Dragons
Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.
iOS sim
How will this change be monitored?
For features that are critical or could fail silently please describe the monitoring/alerting being added.
Feature Flags
Are all new features properly feature flagged? Describe added feature flags.