Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

[PAY-1701] Fix "Share to DMs" on mobile to go through InboxUnavailable modal #3878

Merged
merged 4 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/mobile/src/app/Drawers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ const nativeDrawersMap: { [DrawerName in Drawer]?: ComponentType } = {
BlockMessages: BlockMessagesDrawer,
DeleteChat: DeleteChatDrawer,
SupportersInfo: SupportersInfoDrawer,
InboxUnavailable: InboxUnavailableDrawer,
PremiumTrackPurchase: PremiumTrackPurchaseDrawer
}

Expand Down Expand Up @@ -158,6 +157,7 @@ export const Drawers = () => {
/>
))}
<LeavingAudiusDrawer />
<InboxUnavailableDrawer />
</>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,30 @@ import {
chatSelectors,
chatActions,
tippingActions,
cacheUsersSelectors,
ChatPermissionAction,
CHAT_BLOG_POST_URL
CHAT_BLOG_POST_URL,
accountSelectors,
makeChatId,
useInboxUnavailableModal,
cacheUsersSelectors
} from '@audius/common'
import { View } from 'react-native'
import { useDispatch, useSelector } from 'react-redux'

import IconMessageLocked from 'app/assets/images/iconMessageLocked.svg'
import IconTip from 'app/assets/images/iconTip.svg'
import { Text, Button, useLink } from 'app/components/core'
import { NativeDrawer } from 'app/components/drawer'
import { useDrawer } from 'app/hooks/useDrawer'
import Drawer from 'app/components/drawer'
import { useNavigation } from 'app/hooks/useNavigation'
import { setVisibility } from 'app/store/drawers/slice'
import { makeStyles, flexRowCentered } from 'app/styles'
import { useColor } from 'app/utils/theme'

import { UserBadges } from '../user-badges'

const { unblockUser, createChat } = chatActions
const { getCanCreateChat } = chatSelectors
const { getUser } = cacheUsersSelectors
const { beginTip } = tippingActions

const INBOX_UNAVAILABLE_MODAL_NAME = 'InboxUnavailable'

const messages = {
title: 'Inbox Unavailable',
blockee: 'You cannot send messages to users you have blocked.',
Expand Down Expand Up @@ -102,48 +100,71 @@ const useStyles = makeStyles(({ spacing, typography, palette }) => ({
}))

type DrawerContentProps = {
data: ReturnType<typeof useDrawer<'InboxUnavailable'>>['data']
data: ReturnType<typeof useInboxUnavailableModal>['data']
onClose: () => void
}

const DrawerContent = ({ data }: DrawerContentProps) => {
const DrawerContent = ({ data, onClose }: DrawerContentProps) => {
const styles = useStyles()
const dispatch = useDispatch()
const navigation = useNavigation()

const { userId, shouldOpenChat } = data
const user = useSelector((state) => getUser(state, { id: userId }))
const { canCreateChat, callToAction } = useSelector((state) =>
const { userId, presetMessage } = data
const user = useSelector((state) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use audius-query here :)

cacheUsersSelectors.getUser(state, { id: userId })
)
const { callToAction } = useSelector((state) =>
getCanCreateChat(state, { userId })
)

const closeDrawer = useCallback(() => {
dispatch(
setVisibility({
drawer: 'InboxUnavailable',
visible: false
})
)
}, [dispatch])
const currentUserId = useSelector(accountSelectors.getUserId)

const handleUnblockPress = useCallback(() => {
dispatch(unblockUser({ userId }))
if (shouldOpenChat && canCreateChat) {
dispatch(createChat({ userIds: [userId] }))
if (!userId) {
console.error(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@rickyrombo rickyrombo Aug 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

'Unexpected undefined user in InboxUnavailableDrawer unblock'
)
return
}
closeDrawer()
}, [dispatch, userId, shouldOpenChat, canCreateChat, closeDrawer])
dispatch(unblockUser({ userId }))
dispatch(createChat({ userIds: [userId], presetMessage }))
onClose()
}, [dispatch, userId, presetMessage, onClose])

const { onPress: onPressLearnMore } = useLink(CHAT_BLOG_POST_URL)
const handleLearnMorePress = useCallback(() => {
onPressLearnMore()
closeDrawer()
}, [closeDrawer, onPressLearnMore])
onClose()
}, [onClose, onPressLearnMore])

const handleTipPress = useCallback(() => {
dispatch(beginTip({ user, source: 'inboxUnavailableModal' }))
if (!currentUserId || !user) {
console.error(
'Unexpected undefined currentUserId or user when starting tip'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here about error propagation.

)
return
}
const chatId = makeChatId([currentUserId, user.user_id])
dispatch(
beginTip({
user,
source: 'inboxUnavailableModal',
onSuccessActions: [
chatActions.goToChat({
chatId,
presetMessage
})
],
onSuccessConfirmedActions: [
chatActions.createChat({
userIds: [user.user_id],
skipNavigation: true
})
]
})
)
navigation.navigate('TipArtist')
closeDrawer()
}, [closeDrawer, dispatch, navigation, user])
onClose()
}, [onClose, currentUserId, dispatch, navigation, user, presetMessage])

switch (callToAction) {
case ChatPermissionAction.NONE:
Expand Down Expand Up @@ -219,7 +240,7 @@ const DrawerContent = ({ data }: DrawerContentProps) => {
<Button
key={messages.cancel}
title={messages.cancel}
onPress={closeDrawer}
onPress={onClose}
variant={'common'}
styles={{
root: styles.button,
Expand All @@ -237,22 +258,18 @@ const DrawerContent = ({ data }: DrawerContentProps) => {
export const InboxUnavailableDrawer = () => {
const styles = useStyles()
const neutralLight2 = useColor('neutralLight2')

// Select data outside of drawer and use it to conditionally render content,
// preventing a race condition with the store clears before drawer
// is dismissed
const { data } = useDrawer('InboxUnavailable')
const { isOpen, onClose, onClosed, data } = useInboxUnavailableModal()

return (
<NativeDrawer drawerName={INBOX_UNAVAILABLE_MODAL_NAME}>
<Drawer isOpen={isOpen} onClose={onClose} onClosed={onClosed}>
<View style={styles.drawer}>
<View style={styles.titleContainer}>
<IconMessageLocked fill={neutralLight2} />
<Text style={styles.title}>{messages.title}</Text>
</View>
<View style={styles.border} />
{data ? <DrawerContent data={data} /> : null}
<DrawerContent data={data} onClose={onClose} />
</View>
</NativeDrawer>
</Drawer>
)
}
14 changes: 5 additions & 9 deletions packages/mobile/src/screens/chat-screen/ChatUserListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {
chatSelectors,
ChatPermissionAction,
cacheUsersSelectors,
formatCount
formatCount,
useInboxUnavailableModal
} from '@audius/common'
import { useSelector } from 'audius-client/src/common/hooks/useSelector'
import { View, TouchableOpacity, Keyboard } from 'react-native'
Expand Down Expand Up @@ -158,6 +159,7 @@ export const ChatUserListItem = ({
const { callToAction, canCreateChat } = useSelector((state) =>
getCanCreateChat(state, { userId: user?.user_id })
)
const { onOpen: openInboxUnavailableDrawer } = useInboxUnavailableModal()

const handlePress = useCallback(() => {
if (user?.user_id) {
Expand All @@ -169,15 +171,9 @@ export const ChatUserListItem = ({
const handleNotPermittedPress = useCallback(() => {
if (user?.user_id) {
Keyboard.dismiss()
dispatch(
setVisibility({
drawer: 'InboxUnavailable',
visible: true,
data: { userId: user.user_id, shouldOpenChat: true }
})
)
openInboxUnavailableDrawer({ userId: user.user_id, presetMessage })
}
}, [dispatch, user?.user_id])
}, [openInboxUnavailableDrawer, user, presetMessage])

const handleKebabPress = useCallback(() => {
if (user?.user_id) {
Expand Down
22 changes: 7 additions & 15 deletions packages/mobile/src/screens/profile-screen/MessageLockedButton.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { useCallback } from 'react'

import type { User } from '@audius/common'
import { useDispatch } from 'react-redux'
import type { ID } from '@audius/common'
import { useInboxUnavailableModal } from '@audius/common'

import IconMessageLocked from 'app/assets/images/iconMessageLocked.svg'
import { Button } from 'app/components/core'
import { setVisibility } from 'app/store/drawers/slice'
import { makeStyles } from 'app/styles'

const messages = {
Expand All @@ -24,24 +23,17 @@ const useStyles = makeStyles(({ palette, spacing }) => ({
}))

type MessageLockedButtonProps = {
profile: Pick<User, 'user_id'>
userId: ID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ty for cleanup

}

export const MessageLockedButton = (props: MessageLockedButtonProps) => {
const styles = useStyles()
const { profile } = props
const { user_id: userId } = profile
const dispatch = useDispatch()
const { userId } = props
const { onOpen: openInboxUnavailableDrawer } = useInboxUnavailableModal()

const handlePress = useCallback(() => {
dispatch(
setVisibility({
drawer: 'InboxUnavailable',
visible: true,
data: { userId, shouldOpenChat: true }
})
)
}, [dispatch, userId])
openInboxUnavailableDrawer({ userId })
}, [userId, openInboxUnavailableDrawer])

return (
<Button
Expand Down
2 changes: 1 addition & 1 deletion packages/mobile/src/screens/profile-screen/ProfileInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export const ProfileInfo = (props: ProfileInfoProps) => {
canCreateChat ? (
<MessageButton profile={profile} />
) : (
<MessageLockedButton profile={profile} />
<MessageLockedButton userId={profile.user_id} />
)
) : null}
{does_current_user_follow ? (
Expand Down
27 changes: 13 additions & 14 deletions packages/mobile/src/screens/tip-artist-screen/TipSentScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import { useCallback } from 'react'

import type { SolanaWalletAddress } from '@audius/common'
import {
makeChatId,
chatActions,
formatNumberCommas,
accountSelectors,
tippingSelectors
Expand Down Expand Up @@ -55,7 +53,8 @@ export const TipSentScreen = () => {
const {
user: recipient,
amount: sendAmount,
source
source,
onSuccessActions
} = useSelector(getSendTipData)
const styles = useStyles()
const navigation = useNavigation()
Expand Down Expand Up @@ -84,20 +83,20 @@ export const TipSentScreen = () => {
// After success + close, take the user to the chat they were
// attempting to make if they were unlocking DMs by tipping.
// The saga will create the chat once the tip is confirmed
if (
source === 'inboxUnavailableModal' &&
account?.user_id &&
recipient?.user_id
) {
dispatch(
chatActions.goToChat({
chatId: makeChatId([account.user_id, recipient.user_id])
})
)
if (onSuccessActions && account?.user_id && recipient?.user_id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting pattern. Are there any implications of order of actions, failure of an action blocking dispatch of the rest, etc?

Copy link
Contributor Author

@rickyrombo rickyrombo Aug 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

for (const action of onSuccessActions) {
dispatch(action)
}
} else {
navigation.getParent()?.goBack()
}
}, [account?.user_id, dispatch, navigation, recipient?.user_id, source])
}, [
account?.user_id,
dispatch,
onSuccessActions,
navigation,
recipient?.user_id
])

return (
<TipScreen
Expand Down
2 changes: 0 additions & 2 deletions packages/mobile/src/store/drawers/slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export type Drawer =
| 'BlockMessages'
| 'DeleteChat'
| 'SupportersInfo'
| 'InboxUnavailable'
| 'PremiumTrackPurchase'

export type DrawerData = {
Expand Down Expand Up @@ -96,7 +95,6 @@ const initialState: DrawersState = {
BlockMessages: false,
DeleteChat: false,
SupportersInfo: false,
InboxUnavailable: false,
PremiumTrackPurchase: false,
data: {}
}
Expand Down