From a78b23aa5a7337135378a4fca29248231aebc536 Mon Sep 17 00:00:00 2001 From: Finnian Jacobson-Schulte <140328381+finnian0826@users.noreply.github.com> Date: Fri, 1 Mar 2024 15:22:39 -0700 Subject: [PATCH] refactor(contacts): Use react-native-permissions for contacts (#4979) ### Description Replace custom contact permissions checking with react-native-permissions ### Test plan Unit tests updated Ran the wallet locally and logged the `contactPermissionStatus` for each place it was added. Started with contacts permissions turned off and verified that it logged as something other than `granted`, then turned permissions to allow and verified that it logged as `granted` in each place. ### Related issues - Fixes ACT-1077 ### Backwards compatibility Yes --- jest_setup.ts | 3 ++ src/analytics/ValoraAnalytics.test.ts | 1 - src/home/WalletHome.tsx | 6 +-- src/identity/contactMapping.test.ts | 13 +++--- src/identity/contactMapping.ts | 10 ++--- src/navigator/QRNavigator.test.tsx | 2 - src/send/SelectRecipientButtons.test.tsx | 1 - src/send/SelectRecipientButtons.tsx | 5 +-- src/send/SendSelectRecipient.test.tsx | 2 - src/utils/contacts.ts | 18 +++++++-- src/utils/permissions.android.ts | 51 ------------------------ src/utils/permissions.d.ts | 18 --------- src/utils/permissions.ios.ts | 31 -------------- 13 files changed, 32 insertions(+), 129 deletions(-) delete mode 100644 src/utils/permissions.android.ts delete mode 100644 src/utils/permissions.d.ts delete mode 100644 src/utils/permissions.ios.ts diff --git a/jest_setup.ts b/jest_setup.ts index 6871d8d3d2f..57a6674c9f4 100644 --- a/jest_setup.ts +++ b/jest_setup.ts @@ -60,5 +60,8 @@ jest.mock('@react-native-clipboard/clipboard', () => ({ hasString: jest.fn(), })) +// this mock defaults to granting all permissions +jest.mock('react-native-permissions', () => require('react-native-permissions/mock')) + // @ts-ignore global.__reanimatedWorkletInit = jest.fn() diff --git a/src/analytics/ValoraAnalytics.test.ts b/src/analytics/ValoraAnalytics.test.ts index 40c578eeae6..18658b84c40 100644 --- a/src/analytics/ValoraAnalytics.test.ts +++ b/src/analytics/ValoraAnalytics.test.ts @@ -23,7 +23,6 @@ jest.mock('@segment/analytics-react-native') jest.mock('@segment/analytics-react-native-plugin-adjust') jest.mock('@segment/analytics-react-native-plugin-clevertap') jest.mock('@segment/analytics-react-native-plugin-firebase') -jest.mock('react-native-permissions', () => ({})) jest.mock('@sentry/react-native', () => ({ init: jest.fn() })) jest.mock('src/redux/store', () => ({ store: { getState: jest.fn() } })) jest.mock('src/config', () => ({ diff --git a/src/home/WalletHome.tsx b/src/home/WalletHome.tsx index 1149651da90..642ad13640d 100644 --- a/src/home/WalletHome.tsx +++ b/src/home/WalletHome.tsx @@ -45,8 +45,8 @@ import colors from 'src/styles/colors' import { Spacing } from 'src/styles/styles' import { celoAddressSelector, coreTokensSelector } from 'src/tokens/selectors' import TransactionFeed from 'src/transactions/feed/TransactionFeed' +import { hasGrantedContactsPermission } from 'src/utils/contacts' import { userInSanctionedCountrySelector } from 'src/utils/countryFeatures' -import { checkContactsPermission } from 'src/utils/permissions' const AnimatedSectionList = Animated.createAnimatedComponent(SectionList) @@ -96,8 +96,8 @@ function WalletHome() { return } - const hasGivenContactPermission = await checkContactsPermission() - if (hasGivenContactPermission) { + const contactPermissionStatusGranted = await hasGrantedContactsPermission() + if (contactPermissionStatusGranted) { dispatch(importContacts()) } } diff --git a/src/identity/contactMapping.test.ts b/src/identity/contactMapping.test.ts index fe14727e13a..37e7e911555 100644 --- a/src/identity/contactMapping.test.ts +++ b/src/identity/contactMapping.test.ts @@ -37,8 +37,7 @@ import { contactsToRecipients } from 'src/recipients/recipient' import { phoneRecipientCacheSelector, setPhoneRecipientCache } from 'src/recipients/reducer' import { getFeatureGate } from 'src/statsig' import Logger from 'src/utils/Logger' -import { getAllContacts } from 'src/utils/contacts' -import { checkContactsPermission } from 'src/utils/permissions' +import { getAllContacts, hasGrantedContactsPermission } from 'src/utils/contacts' import networkConfig from 'src/web3/networkConfig' import { getConnectedAccount } from 'src/web3/saga' import { walletAddressSelector } from 'src/web3/selectors' @@ -249,7 +248,7 @@ describe('saveContacts', () => { await expectSaga(saveContacts) .provide([ [select(phoneNumberVerifiedSelector), true], - [call(checkContactsPermission), true], + [call(hasGrantedContactsPermission), true], [select(phoneRecipientCacheSelector), mockPhoneRecipientCache], [select(e164NumberSelector), mockE164Number], [select(lastSavedContactsHashSelector), null], @@ -282,7 +281,7 @@ describe('saveContacts', () => { await expectSaga(saveContacts) .provide([ [select(phoneNumberVerifiedSelector), true], - [call(checkContactsPermission), true], + [call(hasGrantedContactsPermission), true], [ select(phoneRecipientCacheSelector), { ...mockPhoneRecipientCache, [mockE164Number2]: {} }, @@ -320,7 +319,7 @@ describe('saveContacts', () => { await expectSaga(saveContacts) .provide([ [select(phoneNumberVerifiedSelector), true], - [call(checkContactsPermission), true], + [call(hasGrantedContactsPermission), true], [select(phoneRecipientCacheSelector), mockPhoneRecipientCache], [select(e164NumberSelector), mockE164Number], [ @@ -347,7 +346,7 @@ describe('saveContacts', () => { await expectSaga(saveContacts) .provide([ [select(phoneNumberVerifiedSelector), phoneVerified], - [call(checkContactsPermission), contactsEnabled], + [call(hasGrantedContactsPermission), contactsEnabled], ]) .not.select(phoneRecipientCacheSelector) .not.select(e164NumberSelector) @@ -362,7 +361,7 @@ describe('saveContacts', () => { await expectSaga(saveContacts) .provide([ [select(phoneNumberVerifiedSelector), true], - [call(checkContactsPermission), true], + [call(hasGrantedContactsPermission), true], [select(phoneRecipientCacheSelector), mockPhoneRecipientCache], [select(e164NumberSelector), mockE164Number], [select(lastSavedContactsHashSelector), undefined], diff --git a/src/identity/contactMapping.ts b/src/identity/contactMapping.ts index 20f71eb4bab..30fa588bcdd 100644 --- a/src/identity/contactMapping.ts +++ b/src/identity/contactMapping.ts @@ -46,10 +46,9 @@ import { SentryTransaction } from 'src/sentry/SentryTransactions' import { getFeatureGate } from 'src/statsig' import { StatsigFeatureGates } from 'src/statsig/types' import Logger from 'src/utils/Logger' -import { getAllContacts } from 'src/utils/contacts' +import { getAllContacts, hasGrantedContactsPermission } from 'src/utils/contacts' import { ensureError } from 'src/utils/ensureError' import { fetchWithTimeout } from 'src/utils/fetchWithTimeout' -import { checkContactsPermission } from 'src/utils/permissions' import { calculateSha256Hash } from 'src/utils/random' import { getContractKit } from 'src/web3/contracts' import networkConfig from 'src/web3/networkConfig' @@ -92,8 +91,8 @@ export function* doImportContactsWrapper() { } function* doImportContacts() { - const hasGivenContactPermission: boolean = yield* call(checkContactsPermission) - if (!hasGivenContactPermission) { + const contactPermissionStatusGranted = yield* call(hasGrantedContactsPermission) + if (!contactPermissionStatusGranted) { Logger.warn(TAG, 'Contact permissions denied. Skipping import.') ValoraAnalytics.track(IdentityEvents.contacts_import_permission_denied) return true @@ -424,8 +423,7 @@ export function* saveContacts() { try { const saveContactsGate = getFeatureGate(StatsigFeatureGates.SAVE_CONTACTS) const phoneVerified = yield* select(phoneNumberVerifiedSelector) - // TODO(satish): use rn permissions - const contactsEnabled: boolean = yield* call(checkContactsPermission) + const contactsEnabled = yield* call(hasGrantedContactsPermission) if (!saveContactsGate || !phoneVerified || !contactsEnabled) { Logger.debug(`${TAG}/saveContacts`, "Skipping because pre conditions aren't met", { diff --git a/src/navigator/QRNavigator.test.tsx b/src/navigator/QRNavigator.test.tsx index b0cd01a4644..09ae435baba 100644 --- a/src/navigator/QRNavigator.test.tsx +++ b/src/navigator/QRNavigator.test.tsx @@ -5,8 +5,6 @@ import QRNavigator, { QRCodePicker, QRCodeProps } from 'src/navigator/QRNavigato import MockedNavigator from 'test/MockedNavigator' import { createMockStore } from 'test/utils' -jest.mock('react-native-permissions', () => jest.fn()) - jest.mock('src/qrcode/StyledQRGen', () => jest.fn().mockReturnValue('')) jest.mock('src/qrcode/QRGen', () => jest.fn().mockReturnValue('')) diff --git a/src/send/SelectRecipientButtons.test.tsx b/src/send/SelectRecipientButtons.test.tsx index 93d76dbd2f6..c1a82b79c0f 100644 --- a/src/send/SelectRecipientButtons.test.tsx +++ b/src/send/SelectRecipientButtons.test.tsx @@ -13,7 +13,6 @@ import { StatsigFeatureGates } from 'src/statsig/types' import { navigateToPhoneSettings } from 'src/utils/linking' import { createMockStore } from 'test/utils' -jest.mock('react-native-permissions', () => require('react-native-permissions/mock')) jest.mock('src/statsig') const renderComponent = (phoneNumberVerified = false) => { diff --git a/src/send/SelectRecipientButtons.tsx b/src/send/SelectRecipientButtons.tsx index d57a3e67d64..fc759c48523 100644 --- a/src/send/SelectRecipientButtons.tsx +++ b/src/send/SelectRecipientButtons.tsx @@ -3,7 +3,6 @@ import { useAsync } from 'react-async-hook' import { useTranslation } from 'react-i18next' import { Platform } from 'react-native' import { - PERMISSIONS, RESULTS as PERMISSION_RESULTS, PermissionStatus, check as checkPermission, @@ -21,11 +20,9 @@ import { navigate } from 'src/navigator/NavigationService' import { Screens } from 'src/navigator/Screens' import useSelector from 'src/redux/useSelector' import Logger from 'src/utils/Logger' +import { CONTACTS_PERMISSION } from 'src/utils/contacts' import { navigateToPhoneSettings } from 'src/utils/linking' -const CONTACTS_PERMISSION = - Platform.OS === 'ios' ? PERMISSIONS.IOS.CONTACTS : PERMISSIONS.ANDROID.READ_CONTACTS - type Props = { onContactsPermissionGranted: () => void } diff --git a/src/send/SendSelectRecipient.test.tsx b/src/send/SendSelectRecipient.test.tsx index cabeacb57ee..a98b9c8e9db 100644 --- a/src/send/SendSelectRecipient.test.tsx +++ b/src/send/SendSelectRecipient.test.tsx @@ -35,8 +35,6 @@ jest.mock('src/recipients/recipient', () => ({ })) jest.mock('react-native-device-info', () => ({ getFontScaleSync: () => 1 })) -// this mock defaults to granting all permissions -jest.mock('react-native-permissions', () => require('react-native-permissions/mock')) const mockScreenProps = ({ defaultTokenIdOverride, diff --git a/src/utils/contacts.ts b/src/utils/contacts.ts index cb8172fa321..0da13b7bfd0 100644 --- a/src/utils/contacts.ts +++ b/src/utils/contacts.ts @@ -1,10 +1,22 @@ import { Platform } from 'react-native' import { getAll, getMinimal, MinimalContact } from 'react-native-contacts' +import { + check as checkPermission, + RESULTS as PERMISSION_RESULTS, + PERMISSIONS, +} from 'react-native-permissions' import Logger from 'src/utils/Logger' -import { checkContactsPermission } from 'src/utils/permissions' const TAG = 'utils/contacts' +export const CONTACTS_PERMISSION = + Platform.OS === 'ios' ? PERMISSIONS.IOS.CONTACTS : PERMISSIONS.ANDROID.READ_CONTACTS + +export async function hasGrantedContactsPermission() { + const contactPermissionStatus = await checkPermission(CONTACTS_PERMISSION) + return contactPermissionStatus === PERMISSION_RESULTS.GRANTED +} + // Stop gap solution since getMinimal is not yet implement on iOS function customGetAll(callback: (error: any, contacts: MinimalContact[]) => void) { getAll((error, fullContacts) => { @@ -38,8 +50,8 @@ function customGetAll(callback: (error: any, contacts: MinimalContact[]) => void } export async function getAllContacts(): Promise { - const contactPermissionsGiven = await checkContactsPermission() - if (!contactPermissionsGiven) { + const contactPermissionStatusGranted = await hasGrantedContactsPermission() + if (!contactPermissionStatusGranted) { Logger.warn(TAG, 'Permissions not given for retrieving contacts') return null } diff --git a/src/utils/permissions.android.ts b/src/utils/permissions.android.ts deleted file mode 100644 index fab488dfe86..00000000000 --- a/src/utils/permissions.android.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { Permission, PermissionsAndroid } from 'react-native' -import { PRIVACY_LINK } from 'src/brandingConfig' -import i18n from 'src/i18n' -import Logger from 'src/utils/Logger' - -const TAG = 'utils/permissions.android' - -/** - * @deprecated use react-native-permissions - */ -export async function requestContactsPermission() { - return requestPermission( - PermissionsAndroid.PERMISSIONS.READ_CONTACTS, - i18n.t('accessContacts.disclosure.title') ?? undefined, - i18n.t('accessContacts.disclosure.body', { privacyLink: PRIVACY_LINK }) ?? undefined - ) -} - -/** - * @deprecated use react-native-permissions - */ -export async function checkContactsPermission() { - return PermissionsAndroid.check(PermissionsAndroid.PERMISSIONS.READ_CONTACTS) -} - -async function requestPermission(permission: Permission, title?: string, message?: string) { - try { - const granted = await PermissionsAndroid.request( - permission, - title && message - ? { - title, - message, - buttonPositive: i18n.t('continue') ?? undefined, - buttonNegative: i18n.t('notNow') ?? undefined, - } - : undefined - ) - - if (granted === PermissionsAndroid.RESULTS.GRANTED) { - Logger.debug(TAG + '@requestPermission', 'Permission granted for: ' + permission) - return true - } else { - Logger.debug(TAG + '@requestPermission', 'Permission denied for: ' + permission) - return false - } - } catch (err) { - Logger.showError('Error requesting permisison: ' + permission) - return false - } -} diff --git a/src/utils/permissions.d.ts b/src/utils/permissions.d.ts deleted file mode 100644 index 1d4d4d86853..00000000000 --- a/src/utils/permissions.d.ts +++ /dev/null @@ -1,18 +0,0 @@ -// This file exists for two purposes: -// 1. Ensure that both ios and android files present identical types to importers. -// 2. Allow consumers to import the module as if typescript understood react-native suffixes. - -import DefaultAndroid, * as android from './permissions.android' -import DefaultIos, * as ios from './permissions.ios' - -// eslint-disable-next-line no-var -declare var _test: typeof ios -// eslint-disable-next-line no-var -declare var _test: typeof android - -// eslint-disable-next-line no-var -declare var _testDefault: typeof DefaultIos -// eslint-disable-next-line no-var -declare var _testDefault: typeof DefaultAndroid - -export * from './permissions.ios' diff --git a/src/utils/permissions.ios.ts b/src/utils/permissions.ios.ts deleted file mode 100644 index bc4a2cf5aa9..00000000000 --- a/src/utils/permissions.ios.ts +++ /dev/null @@ -1,31 +0,0 @@ -import Contacts from 'react-native-contacts' - -/** - * @deprecated use react-native-permissions - */ -export async function requestContactsPermission(): Promise { - return new Promise((resolve, reject) => { - Contacts.requestPermission((err, permission) => { - if (err) { - reject(err) - } else { - resolve(permission === 'authorized') - } - }) - }) -} - -/** - * @deprecated use react-native-permissions - */ -export async function checkContactsPermission(): Promise { - return new Promise((resolve, reject) => { - Contacts.checkPermission((err, permission) => { - if (err) { - reject(err) - } else { - resolve(permission === 'authorized') - } - }) - }) -}