Skip to content

Commit

Permalink
Prevent crashing on remote cards code (#5924)
Browse files Browse the repository at this point in the history 
* Return undefined from cards store if data is missing

* deal with undefined card obj

---------

Co-authored-by: Christian Baroni <[email protected]>
  • Loading branch information
@brunobar79 @christianbaroni @greg-schrammel
2 people authored and greg-schrammel committed Jul 19, 2024
1 parent 2ff817a commit c5b3340
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 33 deletions.
46 changes: 22 additions & 24 deletions src/components/cards/remote-cards/RemoteCard.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import LinearGradient from 'react-native-linear-gradient';
import { analyticsV2 } from '@/analytics';
import { FlashList } from '@shopify/flash-list';
import { ButtonPressAnimationTouchEvent } from '@/components/animations/ButtonPressAnimation/types';
import { TrimmedCard } from '@/resources/cards/cardCollectionQuery';
import { remoteCardsStore } from '@/state/remoteCards/remoteCards';

const ICON_SIZE = 36;
Expand Down Expand Up @@ -68,49 +67,48 @@ export const RemoteCard: React.FC<RemoteCardProps> = ({ id, gutterSize, carousel
const { navigate } = useNavigation();
const { language } = useAccountSettings();
const { width } = useDimensions();
const card = remoteCardsStore(state => state.getCard(id)) ?? ({} as TrimmedCard);
const { cardKey, accentColor, backgroundColor, primaryButton, imageIcon } = card;
const card = remoteCardsStore(state => state.getCard(id));

const accent = useForegroundColor(getColorFromString(accentColor));
const accent = useForegroundColor(getColorFromString(card?.accentColor || undefined));

const onPress = useCallback(() => {
analyticsV2.track(analyticsV2.event.remoteCardPrimaryButtonPressed, {
cardKey: cardKey ?? 'unknown-backend-driven-card',
action: primaryButton.url || primaryButton.route,
props: JSON.stringify(primaryButton.props),
cardKey: card?.cardKey ?? 'unknown-backend-driven-card',
action: card?.primaryButton.url || card?.primaryButton.route,
props: JSON.stringify(card?.primaryButton.props),
});
if (primaryButton && primaryButton.url) {
Linking.openURL(primaryButton.url);
} else if (primaryButton && primaryButton.route) {
navigate(primaryButton.route, primaryButton.props);
if (card?.primaryButton && card?.primaryButton.url) {
Linking.openURL(card?.primaryButton.url);
} else if (card?.primaryButton && card?.primaryButton.route) {
navigate(card?.primaryButton.route, card?.primaryButton.props);
}
}, [navigate, primaryButton, cardKey]);
}, [navigate, card?.primaryButton, card?.cardKey]);

const onDismiss = useCallback(
(e: ButtonPressAnimationTouchEvent) => {
if (e && 'stopPropagation' in e) {
e.stopPropagation();
}
analyticsV2.track(analyticsV2.event.remoteCardDismissed, {
cardKey: cardKey ?? card.sys.id ?? 'unknown-backend-driven-card',
cardKey: card?.cardKey ?? card?.sys.id ?? 'unknown-backend-driven-card',
});

const { cards } = remoteCardsStore.getState();

const isLastCard = cards.size === 1;

remoteCardsStore.getState().dismissCard(card.sys.id);
card?.sys.id && remoteCardsStore.getState().dismissCard(card.sys.id);
if (carouselRef?.current) {
// check if this is the last card and don't scroll if so
if (isLastCard) return;

carouselRef.current.scrollToIndex({
index: Array.from(cards.values()).findIndex(c => c.sys.id === card.sys.id),
index: Array.from(cards.values()).findIndex(c => c.sys.id === card?.sys.id),
animated: true,
});
}
},
[carouselRef, cardKey, card.sys.id]
[card?.cardKey, card?.sys.id, carouselRef]
);

const imageForPlatform = () => {
Expand Down Expand Up @@ -149,24 +147,24 @@ export const RemoteCard: React.FC<RemoteCardProps> = ({ id, gutterSize, carousel
const contentWidth = width - gutterSize - 16 * 2 - ICON_SIZE - 12;
return (
<ConditionalWrap
condition={primaryButton.route || primaryButton.url}
condition={card?.primaryButton.route || card?.primaryButton.url}
wrap={children => (
<ButtonPressAnimation hapticType="impactHeavy" onPress={onPress} disabled={IS_ANDROID} scaleTo={0.94} disallowInterruption>
{children}
</ButtonPressAnimation>
)}
>
<Box
testID={`remote-card-${cardKey}`}
testID={`remote-card-${card?.cardKey}`}
width={{ custom: width - gutterSize }}
overflow="visible"
justifyContent="center"
height={'full'}
borderRadius={CARD_BORDER_RADIUS}
padding="16px"
shadow="12px"
background={(backgroundColor as BackgroundColor) ?? 'surfaceSecondaryElevated'}
style={backgroundColor || !isDarkMode ? {} : { backgroundColor: '#191A1C' }}
background={(card?.backgroundColor as BackgroundColor) ?? 'surfaceSecondaryElevated'}
style={card?.backgroundColor || !isDarkMode ? {} : { backgroundColor: '#191A1C' }}
>
<Box flexDirection="row" width={{ custom: width - gutterSize - 16 * 2 }} gap={12}>
<Box
Expand All @@ -188,7 +186,7 @@ export const RemoteCard: React.FC<RemoteCardProps> = ({ id, gutterSize, carousel
<Box
height="full"
style={
!imageIcon && imageUri
!card?.imageIcon && imageUri
? {
shadowColor: isDarkMode ? colors.shadowBlack : accent,
shadowOffset: { width: 0, height: 2 },
Expand All @@ -200,15 +198,15 @@ export const RemoteCard: React.FC<RemoteCardProps> = ({ id, gutterSize, carousel
width="full"
>
<Cover alignHorizontal="center" alignVertical="center">
{imageIcon && (
{card?.imageIcon && (
<TextShadow blur={12}>
<Text align="center" color={{ custom: accent }} size="icon 17px" weight="heavy">
{imageIcon}
{card?.imageIcon}
</Text>
</TextShadow>
)}

{!imageIcon && imageUri && (
{!card?.imageIcon && imageUri && (
<Box
as={ImgixImage}
enableFasterImage
Expand Down
39 changes: 30 additions & 9 deletions src/state/remoteCards/remoteCards.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface RemoteCardsState {
setCards: (cards: TrimmedCards) => void;

getCard: (id: string) => TrimmedCard | undefined;
getCardPlacement: (id: string) => TrimmedCard['placement'];
getCardPlacement: (id: string) => TrimmedCard['placement'] | undefined;
dismissCard: (id: string) => void;

getCardIdsForScreen: (screen: keyof typeof Routes) => string[];
Expand All @@ -28,10 +28,18 @@ type RemoteCardsStateWithTransforms = Omit<Partial<RemoteCardsState>, 'cards' |

function serializeState(state: Partial<RemoteCardsState>, version?: number) {
try {
const validCards = Array.from(state.cards?.entries() ?? []).filter(([, card]) => card && card.sys && card.sys.id);

if (state.cards && validCards.length < state.cards.size) {
logger.error(new RainbowError('remoteCardsStore: filtered cards without sys.id during serialization'), {
filteredCount: state.cards.size - validCards.length,
});
}

const transformedStateToPersist: RemoteCardsStateWithTransforms = {
...state,
cardsById: state.cardsById ? Array.from(state.cardsById) : [],
cards: state.cards ? Array.from(state.cards.entries()) : [],
cards: validCards,
};

return JSON.stringify({
Expand All @@ -58,7 +66,7 @@ function deserializeState(serializedState: string) {
let cardsByIdData = new Set<string>();
try {
if (state.cardsById.length) {
cardsByIdData = new Set(state.cardsById);
cardsByIdData = new Set(state.cardsById.filter(id => typeof id === 'string' && id.length > 0));
}
} catch (error) {
logger.error(new RainbowError('Failed to convert cardsById from remote cards storage'), { error });
Expand All @@ -68,7 +76,15 @@ function deserializeState(serializedState: string) {
let cardsData: Map<string, TrimmedCard> = new Map();
try {
if (state.cards.length) {
cardsData = new Map(state.cards);
const validCards = state.cards.filter(([, card]) => card && card.sys && typeof card.sys.id === 'string');

if (validCards.length < state.cards.length) {
logger.error(new RainbowError('Filtered out cards without sys.id during deserialization'), {
filteredCount: state.cards.length - validCards.length,
});
}

cardsData = new Map(validCards);
}
} catch (error) {
logger.error(new RainbowError('Failed to convert cards from remote cards storage'), { error });
Expand Down Expand Up @@ -109,10 +125,14 @@ export const remoteCardsStore = createRainbowStore<RemoteCardsState>(
});
},

getCard: (id: string) => get().cards.get(id),
getCard: (id: string) => {
const card = get().cards.get(id);
return card && card.sys.id ? card : undefined;
},

getCardPlacement: (id: string) => {
const card = get().getCard(id);
if (!card || !card.placement) {
if (!card || !card.sys.id || !card.placement) {
return undefined;
}

Expand Down Expand Up @@ -145,17 +165,18 @@ export const remoteCardsStore = createRainbowStore<RemoteCardsState>(
cards: new Map(state.cards.set(id, newCard)),
};
}),

getCardIdsForScreen: (screen: keyof typeof Routes) => {
return Array.from(get().cards.values())
.filter(card => get().getCardPlacement(card.sys.id) === screen)
.filter(card => !card.dismissed)
.filter(card => card.sys.id && get().getCardPlacement(card.sys.id) === screen && !card.dismissed)
.sort((a, b) => {
if (a.index === b.index) return 0;
if (a.index === undefined || a.index === null) return 1;
if (b.index === undefined || b.index === null) return -1;
return a.index - b.index;
})
.map(card => card.sys.id);
.map(card => card.sys.id)
.filter((id): id is string => id !== undefined);
},
}),
{
Expand Down

0 comments on commit c5b3340

Please sign in to comment.