From 1734c807de36413513c6b0b57c60b1d45434840a Mon Sep 17 00:00:00 2001 From: iamacook Date: Wed, 4 Oct 2023 13:00:26 +0200 Subject: [PATCH 1/4] fix: enable notifications per chain --- .../PushNotificationsBanner.test.ts | 49 ++++++------------ .../PushNotificationsBanner/index.tsx | 50 ++++++++----------- .../analytics/events/push-notifications.ts | 2 +- 3 files changed, 39 insertions(+), 62 deletions(-) diff --git a/src/components/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.ts b/src/components/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.ts index ca17984eb3..e091df0af6 100644 --- a/src/components/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.ts +++ b/src/components/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.ts @@ -1,39 +1,28 @@ import { _getSafesToRegister } from '.' -import type { AddedSafesState } from '@/store/addedSafesSlice' +import type { AddedSafesOnChain } from '@/store/addedSafesSlice' import type { PushNotificationPreferences } from '@/services/push-notifications/preferences' describe('PushNotificationsBanner', () => { describe('getSafesToRegister', () => { it('should return all added safes if no preferences exist', () => { - const addedSafes = { - '1': { - '0x123': {}, - '0x456': {}, - }, - '4': { - '0x789': {}, - }, - } as unknown as AddedSafesState + const addedSafesOnChain = { + '0x123': {}, + '0x456': {}, + } as unknown as AddedSafesOnChain const allPreferences = undefined - const result = _getSafesToRegister(addedSafes, allPreferences) + const result = _getSafesToRegister('1', addedSafesOnChain, allPreferences) expect(result).toEqual({ '1': ['0x123', '0x456'], - '4': ['0x789'], }) }) it('should return only newly added safes if preferences exist', () => { - const addedSafes = { - '1': { - '0x123': {}, - '0x456': {}, - }, - '4': { - '0x789': {}, - }, - } as unknown as AddedSafesState + const addedSafesOnChain = { + '0x123': {}, + '0x456': {}, + } as unknown as AddedSafesOnChain const allPreferences = { '1:0x123': { safeAddress: '0x123', @@ -45,7 +34,7 @@ describe('PushNotificationsBanner', () => { }, } as unknown as PushNotificationPreferences - const result = _getSafesToRegister(addedSafes, allPreferences) + const result = _getSafesToRegister('1', addedSafesOnChain, allPreferences) expect(result).toEqual({ '1': ['0x456'], @@ -53,15 +42,10 @@ describe('PushNotificationsBanner', () => { }) it('should return all added safes if no preferences match', () => { - const addedSafes = { - '1': { - '0x123': {}, - '0x456': {}, - }, - '4': { - '0x789': {}, - }, - } as unknown as AddedSafesState + const addedSafesOnChain = { + '0x123': {}, + '0x456': {}, + } as unknown as AddedSafesOnChain const allPreferences = { '1:0x111': { safeAddress: '0x111', @@ -73,11 +57,10 @@ describe('PushNotificationsBanner', () => { }, } as unknown as PushNotificationPreferences - const result = _getSafesToRegister(addedSafes, allPreferences) + const result = _getSafesToRegister('1', addedSafesOnChain, allPreferences) expect(result).toEqual({ '1': ['0x123', '0x456'], - '4': ['0x789'], }) }) }) diff --git a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx index f1fbfb8082..e58e024d34 100644 --- a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx +++ b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx @@ -7,11 +7,10 @@ import type { ReactElement } from 'react' import { CustomTooltip } from '@/components/common/CustomTooltip' import { AppRoutes } from '@/config/routes' import { useAppSelector } from '@/store' -import { selectAllAddedSafes, selectTotalAdded } from '@/store/addedSafesSlice' +import { selectAddedSafes, selectAllAddedSafes, selectTotalAdded } from '@/store/addedSafesSlice' import PushNotificationIcon from '@/public/images/notifications/push-notification.svg' import useLocalStorage from '@/services/local-storage/useLocalStorage' import { useNotificationRegistrations } from '../hooks/useNotificationRegistrations' -import { transformAddedSafes } from '../GlobalPushNotifications' import { PUSH_NOTIFICATION_EVENTS } from '@/services/analytics/events/push-notifications' import { trackEvent } from '@/services/analytics' import useSafeInfo from '@/hooks/useSafeInfo' @@ -21,9 +20,9 @@ import { useNotificationPreferences } from '../hooks/useNotificationPreferences' import { sameAddress } from '@/utils/addresses' import useOnboard from '@/hooks/wallets/useOnboard' import { assertWalletChain } from '@/services/tx/tx-sender/sdk' -import { useHasFeature } from '@/hooks/useChains' +import { useCurrentChain, useHasFeature } from '@/hooks/useChains' import { FEATURES } from '@/utils/chains' -import type { AddedSafesState } from '@/store/addedSafesSlice' +import type { AddedSafesOnChain } from '@/store/addedSafesSlice' import type { PushNotificationPreferences } from '@/services/push-notifications/preferences' import type { NotifiableSafes } from '../logic' @@ -66,44 +65,39 @@ export const useDismissPushNotificationsBanner = () => { } export const _getSafesToRegister = ( - addedSafes: AddedSafesState, + chainId: string, + addedSafesOnChain: AddedSafesOnChain, allPreferences: PushNotificationPreferences | undefined, -) => { - // Regiser all added Safes +): NotifiableSafes => { + const addedSafeAddressesOnChain = Object.keys(addedSafesOnChain) + if (!allPreferences) { - return transformAddedSafes(addedSafes) + return { [chainId]: addedSafeAddressesOnChain } } - // Only register Safes that are not already registered - return Object.entries(addedSafes).reduce((acc, [chainId, addedSafesOnChain]) => { - const addedSafeAddressesOnChain = Object.keys(addedSafesOnChain) - const notificationRegistrations = Object.values(allPreferences) - - const newlyAddedSafes = addedSafeAddressesOnChain.filter((safeAddress) => { - return !notificationRegistrations.some( - (registration) => chainId === registration.chainId && sameAddress(registration.safeAddress, safeAddress), - ) - }) + const notificationRegistrations = Object.values(allPreferences) - if (newlyAddedSafes.length > 0) { - acc[chainId] = newlyAddedSafes - } + const newlyAddedSafes = addedSafeAddressesOnChain.filter((safeAddress) => { + return !notificationRegistrations.some( + (registration) => chainId === registration.chainId && sameAddress(registration.safeAddress, safeAddress), + ) + }) - return acc - }, {}) + return { [chainId]: newlyAddedSafes } } export const PushNotificationsBanner = ({ children }: { children: ReactElement }): ReactElement => { const isNotificationsEnabled = useHasFeature(FEATURES.PUSH_NOTIFICATIONS) - const addedSafes = useAppSelector(selectAllAddedSafes) + const chain = useCurrentChain() const totalAddedSafes = useAppSelector(selectTotalAdded) const { safe, safeAddress } = useSafeInfo() + const addedSafesOnChain = useAppSelector((state) => selectAddedSafes(state, safe.chainId)) const { query } = useRouter() const onboard = useOnboard() const { dismissPushNotificationBanner, isPushNotificationBannerDismissed } = useDismissPushNotificationsBanner() - const isSafeAdded = !!addedSafes?.[safe.chainId]?.[safeAddress] + const isSafeAdded = !!addedSafesOnChain?.[safeAddress] const shouldShowBanner = isNotificationsEnabled && !isPushNotificationBannerDismissed && isSafeAdded const { registerNotifications } = useNotificationRegistrations() @@ -121,14 +115,14 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement } }, [dismissBanner, shouldShowBanner]) const onEnableAll = async () => { - if (!onboard) { + if (!onboard || !addedSafesOnChain) { return } trackEvent(PUSH_NOTIFICATION_EVENTS.ENABLE_ALL) const allPreferences = getAllPreferences() - const safesToRegister = _getSafesToRegister(addedSafes, allPreferences) + const safesToRegister = _getSafesToRegister(safe.chainId, addedSafesOnChain, allPreferences) try { await assertWalletChain(onboard, safe.chainId) @@ -183,7 +177,7 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement } onClick={onEnableAll} disabled={!isOk || !onboard} > - Enable all + Enable on {chain?.chainName} )} {safe && ( diff --git a/src/services/analytics/events/push-notifications.ts b/src/services/analytics/events/push-notifications.ts index 08e6a89a3b..9f41ebe70a 100644 --- a/src/services/analytics/events/push-notifications.ts +++ b/src/services/analytics/events/push-notifications.ts @@ -38,7 +38,7 @@ export const PUSH_NOTIFICATION_EVENTS = { }, // User enabled all notifications from banner ENABLE_ALL: { - action: 'Enable all notifications', + action: 'Enable all notifications on chain', category, }, // User opened Safe notification settings from banner From 32681dedf2c3420c2e7fbf7bb746554492aea4ed Mon Sep 17 00:00:00 2001 From: iamacook Date: Wed, 4 Oct 2023 13:15:29 +0200 Subject: [PATCH 2/4] fix: adjust text --- .../PushNotifications/PushNotificationsBanner/index.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx index e58e024d34..63fbb29434 100644 --- a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx +++ b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx @@ -162,8 +162,9 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement } - Get notified about pending signatures, incoming and outgoing transactions and more when Safe{`{Wallet}`}{' '} - is in the background or closed. + Get notified about pending signatures, incoming and outgoing transactions for all Safes on{' '} + {chain?.chainName} when Safe + {`{Wallet}`} is in the background or closed. {/* Cannot wrap singular button as it causes style inconsistencies */} @@ -177,7 +178,7 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement } onClick={onEnableAll} disabled={!isOk || !onboard} > - Enable on {chain?.chainName} + Enable all )} {safe && ( From da4a219fb7edc5ef33a16310b7fe40be9ae5d2ae Mon Sep 17 00:00:00 2001 From: iamacook Date: Wed, 4 Oct 2023 13:22:51 +0200 Subject: [PATCH 3/4] fix: "Safes" -> "Safe Accounts" --- .../PushNotifications/PushNotificationsBanner/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx index 63fbb29434..96332f1d2b 100644 --- a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx +++ b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx @@ -162,7 +162,7 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement } - Get notified about pending signatures, incoming and outgoing transactions for all Safes on{' '} + Get notified about pending signatures, incoming and outgoing transactions for all Safe Accounts on{' '} {chain?.chainName} when Safe {`{Wallet}`} is in the background or closed. From 762caeb2f55a92e7a0709bfa96607ff66152fca4 Mon Sep 17 00:00:00 2001 From: iamacook Date: Wed, 4 Oct 2023 15:55:28 +0200 Subject: [PATCH 4/4] fix: remove Safes on unsupported chains --- .../GlobalPushNotifications.tsx | 26 ++++++++++++++++--- .../__tests__/GlobalPushNotifications.test.ts | 26 ++++++++++++++++--- .../analytics/events/push-notifications.ts | 2 +- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/components/settings/PushNotifications/GlobalPushNotifications.tsx b/src/components/settings/PushNotifications/GlobalPushNotifications.tsx index 55feb8c9c7..88b789fda9 100644 --- a/src/components/settings/PushNotifications/GlobalPushNotifications.tsx +++ b/src/components/settings/PushNotifications/GlobalPushNotifications.tsx @@ -15,6 +15,7 @@ import { } from '@mui/material' import { Fragment, useEffect, useMemo, useState } from 'react' import type { ReactElement } from 'react' +import type { ChainInfo } from '@safe-global/safe-gateway-typescript-sdk' import EthHashInfo from '@/components/common/EthHashInfo' import { sameAddress } from '@/utils/addresses' @@ -37,7 +38,7 @@ import css from './styles.module.css' // UI logic // Convert data structure of added Safes -export const transformAddedSafes = (addedSafes: AddedSafesState): NotifiableSafes => { +export const _transformAddedSafes = (addedSafes: AddedSafesState): NotifiableSafes => { return Object.entries(addedSafes).reduce((acc, [chainId, addedSafesOnChain]) => { acc[chainId] = Object.keys(addedSafesOnChain) return acc @@ -62,12 +63,28 @@ export const _transformCurrentSubscribedSafes = ( }, {}) } +// Remove Safes that are not on a supported chain +export const _sanitizeNotifiableSafes = ( + chains: Array, + notifiableSafes: NotifiableSafes, +): NotifiableSafes => { + return Object.entries(notifiableSafes).reduce((acc, [chainId, safeAddresses]) => { + const chain = chains.find((chain) => chain.chainId === chainId) + + if (chain) { + acc[chainId] = safeAddresses + } + + return acc + }, {}) +} + // Merges added Safes and currently notified Safes into a single data structure without duplicates export const _mergeNotifiableSafes = ( addedSafes: AddedSafesState, currentSubscriptions?: NotifiableSafes, ): NotifiableSafes => { - const notifiableSafes = transformAddedSafes(addedSafes) + const notifiableSafes = _transformAddedSafes(addedSafes) if (!currentSubscriptions) { return notifiableSafes @@ -249,8 +266,9 @@ export const GlobalPushNotifications = (): ReactElement | null => { // Merged added Safes and `currentNotifiedSafes` (in case subscriptions aren't added) const notifiableSafes = useMemo(() => { - return _mergeNotifiableSafes(addedSafes, currentNotifiedSafes) - }, [addedSafes, currentNotifiedSafes]) + const safes = _mergeNotifiableSafes(addedSafes, currentNotifiedSafes) + return _sanitizeNotifiableSafes(chains.configs, safes) + }, [chains.configs, addedSafes, currentNotifiedSafes]) const totalNotifiableSafes = useMemo(() => { return _getTotalNotifiableSafes(notifiableSafes) diff --git a/src/components/settings/PushNotifications/__tests__/GlobalPushNotifications.test.ts b/src/components/settings/PushNotifications/__tests__/GlobalPushNotifications.test.ts index b6f590b5ee..42d1f5a6e4 100644 --- a/src/components/settings/PushNotifications/__tests__/GlobalPushNotifications.test.ts +++ b/src/components/settings/PushNotifications/__tests__/GlobalPushNotifications.test.ts @@ -1,5 +1,7 @@ +import type { ChainInfo } from '@safe-global/safe-gateway-typescript-sdk' + import { - transformAddedSafes, + _transformAddedSafes, _mergeNotifiableSafes, _transformCurrentSubscribedSafes, _getTotalNotifiableSafes, @@ -10,6 +12,7 @@ import { _getSafesToRegister, _getSafesToUnregister, _shouldUnregisterDevice, + _sanitizeNotifiableSafes, } from '../GlobalPushNotifications' import type { AddedSafesState } from '@/store/addedSafesSlice' @@ -31,7 +34,7 @@ describe('GlobalPushNotifications', () => { '4': ['0x789'], } - expect(transformAddedSafes(addedSafes)).toEqual(expectedNotifiableSafes) + expect(_transformAddedSafes(addedSafes)).toEqual(expectedNotifiableSafes) }) }) @@ -71,7 +74,24 @@ describe('GlobalPushNotifications', () => { }, } as unknown as AddedSafesState - expect(_mergeNotifiableSafes(addedSafes)).toEqual(transformAddedSafes(addedSafes)) + expect(_mergeNotifiableSafes(addedSafes)).toEqual(_transformAddedSafes(addedSafes)) + }) + }) + + describe('sanitizeNotifiableSafes', () => { + it('should remove Safes that are not on a supported chain', () => { + const chains = [{ chainId: '1', name: 'Mainnet' }] as unknown as Array + + const notifiableSafes = { + '1': ['0x123', '0x456'], + '4': ['0xabc'], + } + + const expected = { + '1': ['0x123', '0x456'], + } + + expect(_sanitizeNotifiableSafes(chains, notifiableSafes)).toEqual(expected) }) }) diff --git a/src/services/analytics/events/push-notifications.ts b/src/services/analytics/events/push-notifications.ts index 9f41ebe70a..08e6a89a3b 100644 --- a/src/services/analytics/events/push-notifications.ts +++ b/src/services/analytics/events/push-notifications.ts @@ -38,7 +38,7 @@ export const PUSH_NOTIFICATION_EVENTS = { }, // User enabled all notifications from banner ENABLE_ALL: { - action: 'Enable all notifications on chain', + action: 'Enable all notifications', category, }, // User opened Safe notification settings from banner