From 30ab6816d5c456d929232a1c03f31de883819a90 Mon Sep 17 00:00:00 2001 From: Steffen Kleinle Date: Thu, 17 Oct 2024 13:20:41 +0200 Subject: [PATCH 1/3] 2350: Show snackbar instead of opening settings and fix setting status if disabling --- native/src/Navigator.tsx | 7 +- native/src/components/ExportEventButton.tsx | 8 +- native/src/contexts/AppContextProvider.tsx | 9 +- .../__tests__/AppContextProvider.spec.tsx | 20 ++- native/src/hooks/useCityAppContext.ts | 4 +- native/src/routes/Settings.tsx | 42 ++----- native/src/testing/TestingAppContext.tsx | 33 ++--- native/src/utils/PushNotificationsManager.ts | 37 ++++-- .../PushNotificationsManager.spec.ts | 76 ++++++++--- .../__tests__/createSettingsSections.spec.ts | 119 +++++++----------- native/src/utils/createSettingsSections.ts | 102 ++++++--------- .../2350-pn-permission-settings.yml | 7 ++ translations/translations.json | 39 +----- 13 files changed, 247 insertions(+), 256 deletions(-) create mode 100644 release-notes/unreleased/2350-pn-permission-settings.yml diff --git a/native/src/Navigator.tsx b/native/src/Navigator.tsx index b9afa30bc9..aeb056c4c6 100644 --- a/native/src/Navigator.tsx +++ b/native/src/Navigator.tsx @@ -88,7 +88,8 @@ const Stack = createStackNavigator() const Navigator = (): ReactElement | null => { const showSnackbar = useSnackbar() - const { settings, cityCode, changeCityCode, languageCode, updateSettings } = useAppContext() + const appContext = useAppContext() + const { settings, cityCode, changeCityCode, updateSettings } = appContext const navigation = useNavigation>() const [initialRoute, setInitialRoute] = useState(null) @@ -96,8 +97,8 @@ const Navigator = (): ReactElement | null => { const { data: cities, error: citiesError, refresh: refreshCities } = useLoadCities() useEffect(() => { - initialPushNotificationRequest(cityCode, languageCode).catch(reportError) - }, [cityCode, languageCode]) + initialPushNotificationRequest(appContext).catch(reportError) + }, [appContext]) useForegroundPushNotificationListener({ showSnackbar, navigate: navigation.navigate }) diff --git a/native/src/components/ExportEventButton.tsx b/native/src/components/ExportEventButton.tsx index f09e16abbf..35ea57a4d7 100644 --- a/native/src/components/ExportEventButton.tsx +++ b/native/src/components/ExportEventButton.tsx @@ -1,9 +1,9 @@ import { DateTime } from 'luxon' import React, { ReactElement, useState } from 'react' import { useTranslation } from 'react-i18next' -import { Linking, Platform } from 'react-native' +import { Platform } from 'react-native' import RNCalendarEvents, { Calendar, CalendarEventWritable, RecurrenceFrequency } from 'react-native-calendar-events' -import { PERMISSIONS, requestMultiple } from 'react-native-permissions' +import { PERMISSIONS, openSettings, requestMultiple } from 'react-native-permissions' import { Frequency } from 'rrule' import styled from 'styled-components/native' @@ -89,8 +89,8 @@ const ExportEventButton = ({ event }: ExportEventButtonType): ReactElement => { showSnackbar({ text: 'noCalendarPermission', positiveAction: { - label: t('settings'), - onPress: Linking.openSettings, + label: t('layout:settings'), + onPress: openSettings, }, }) return diff --git a/native/src/contexts/AppContextProvider.tsx b/native/src/contexts/AppContextProvider.tsx index 14f72b140e..47cdc7ebde 100644 --- a/native/src/contexts/AppContextProvider.tsx +++ b/native/src/contexts/AppContextProvider.tsx @@ -34,6 +34,7 @@ type AppContextProviderProps = { const AppContextProvider = ({ children }: AppContextProviderProps): ReactElement | null => { const [settings, setSettings] = useState(null) + const allowPushNotifications = !!settings?.allowPushNotifications const cityCode = settings?.selectedCity const languageCode = settings?.contentLanguage const { i18n } = useTranslation() @@ -55,10 +56,10 @@ const AppContextProvider = ({ children }: AppContextProviderProps): ReactElement unsubscribeNews(cityCode, languageCode).catch(reportError) } if (languageCode && newCityCode) { - subscribeNews(newCityCode, languageCode).catch(reportError) + subscribeNews({ cityCode: newCityCode, languageCode, allowPushNotifications }).catch(reportError) } }, - [updateSettings, cityCode, languageCode], + [updateSettings, cityCode, languageCode, allowPushNotifications], ) const changeLanguageCode = useCallback( @@ -68,10 +69,10 @@ const AppContextProvider = ({ children }: AppContextProviderProps): ReactElement unsubscribeNews(cityCode, languageCode).catch(reportError) } if (cityCode) { - subscribeNews(cityCode, newLanguageCode).catch(reportError) + subscribeNews({ cityCode, languageCode: newLanguageCode, allowPushNotifications }).catch(reportError) } }, - [updateSettings, cityCode, languageCode], + [updateSettings, cityCode, languageCode, allowPushNotifications], ) useEffect(() => { diff --git a/native/src/contexts/__tests__/AppContextProvider.spec.tsx b/native/src/contexts/__tests__/AppContextProvider.spec.tsx index 04e0be5ab3..4429802558 100644 --- a/native/src/contexts/__tests__/AppContextProvider.spec.tsx +++ b/native/src/contexts/__tests__/AppContextProvider.spec.tsx @@ -135,7 +135,7 @@ describe('AppContextProvider', () => { expect(await appSettings.loadSettings()).toMatchObject({ selectedCity: 'hallo' }) expect(setSettings).toHaveBeenCalledTimes(1) expect(subscribeNews).toHaveBeenCalledTimes(1) - expect(subscribeNews).toHaveBeenCalledWith('hallo', 'de') + expect(subscribeNews).toHaveBeenCalledWith({ cityCode: 'hallo', languageCode: 'de', allowPushNotifications: true }) }) it('should select city', async () => { @@ -150,7 +150,11 @@ describe('AppContextProvider', () => { expect(await appSettings.loadSettings()).toMatchObject({ selectedCity: 'augsburg' }) expect(setSettings).toHaveBeenCalledTimes(1) expect(subscribeNews).toHaveBeenCalledTimes(1) - expect(subscribeNews).toHaveBeenCalledWith('augsburg', 'de') + expect(subscribeNews).toHaveBeenCalledWith({ + cityCode: 'augsburg', + languageCode: 'de', + allowPushNotifications: true, + }) expect(unsubscribeNews).not.toHaveBeenCalled() }) @@ -166,7 +170,11 @@ describe('AppContextProvider', () => { expect(await appSettings.loadSettings()).toMatchObject({ selectedCity: 'augsburg' }) expect(setSettings).toHaveBeenCalledTimes(1) expect(subscribeNews).toHaveBeenCalledTimes(1) - expect(subscribeNews).toHaveBeenCalledWith('augsburg', 'de') + expect(subscribeNews).toHaveBeenCalledWith({ + cityCode: 'augsburg', + languageCode: 'de', + allowPushNotifications: true, + }) expect(unsubscribeNews).toHaveBeenCalledTimes(1) expect(unsubscribeNews).toHaveBeenCalledWith('muenchen', 'de') }) @@ -198,7 +206,11 @@ describe('AppContextProvider', () => { expect(await appSettings.loadSettings()).toMatchObject({ contentLanguage: 'ar' }) expect(setSettings).toHaveBeenCalledTimes(1) expect(subscribeNews).toHaveBeenCalledTimes(1) - expect(subscribeNews).toHaveBeenCalledWith('muenchen', 'ar') + expect(subscribeNews).toHaveBeenCalledWith({ + cityCode: 'muenchen', + languageCode: 'ar', + allowPushNotifications: true, + }) expect(unsubscribeNews).toHaveBeenCalledTimes(1) expect(unsubscribeNews).toHaveBeenCalledWith('muenchen', 'de') }) diff --git a/native/src/hooks/useCityAppContext.ts b/native/src/hooks/useCityAppContext.ts index c6d9fe1763..07342b9206 100644 --- a/native/src/hooks/useCityAppContext.ts +++ b/native/src/hooks/useCityAppContext.ts @@ -2,13 +2,13 @@ import { useContext } from 'react' import { AppContext, AppContextType } from '../contexts/AppContextProvider' -type UseCityAppContextReturn = AppContextType & { +export type CityAppContext = AppContextType & { cityCode: string } export const useAppContext = (): AppContextType => useContext(AppContext) -const useCityAppContext = (): UseCityAppContextReturn => { +const useCityAppContext = (): CityAppContext => { const { cityCode, ...context } = useAppContext() if (!cityCode) { throw new Error('City code not set!') diff --git a/native/src/routes/Settings.tsx b/native/src/routes/Settings.tsx index 3a806e5296..66e176e8fb 100644 --- a/native/src/routes/Settings.tsx +++ b/native/src/routes/Settings.tsx @@ -1,4 +1,4 @@ -import React, { ReactElement, useContext } from 'react' +import React, { ReactElement } from 'react' import { useTranslation } from 'react-i18next' import { SectionList, SectionListData } from 'react-native' import styled from 'styled-components/native' @@ -10,10 +10,8 @@ import Layout from '../components/Layout' import SettingItem from '../components/SettingItem' import ItemSeparator from '../components/base/ItemSeparator' import { NavigationProps } from '../constants/NavigationTypes' -import { AppContext } from '../contexts/AppContextProvider' -import { useAppContext } from '../hooks/useCityAppContext' +import useCityAppContext from '../hooks/useCityAppContext' import useSnackbar from '../hooks/useSnackbar' -import { SettingsType } from '../utils/AppSettings' import createSettingsSections, { SettingsSectionType } from '../utils/createSettingsSections' import { log, reportError } from '../utils/sentry' @@ -31,36 +29,25 @@ const SectionHeader = styled.Text` ` const Settings = ({ navigation }: SettingsProps): ReactElement => { - const { settings, updateSettings } = useAppContext() - const { cityCode, languageCode } = useContext(AppContext) + const appContext = useCityAppContext() const showSnackbar = useSnackbar() const { t } = useTranslation('settings') + const { settings } = appContext - const setSetting = async ( - changeSetting: (settings: SettingsType) => Partial, - changeAction?: (settings: SettingsType) => Promise, - ) => { + const safeOnPress = (update: () => Promise | void) => () => { const oldSettings = settings - const newSettings = { ...oldSettings, ...changeSetting(settings) } - updateSettings(newSettings) - - try { - const successful = changeAction ? await changeAction(newSettings) : true - - if (!successful) { - updateSettings(oldSettings) - } - } catch (e) { + update()?.catch(e => { log('Failed to persist settings.', 'error') reportError(e) - updateSettings(oldSettings) - } + appContext.updateSettings(oldSettings) + showSnackbar({ text: t('error:unknownError') }) + }) } const renderItem = ({ item }: { item: SettingsSectionType }) => { - const { getSettingValue, ...otherProps } = item + const { getSettingValue, onPress, ...otherProps } = item const value = !!(getSettingValue && getSettingValue(settings)) - return + return } const renderSectionHeader = ({ section: { title } }: { section: SectionType }) => { @@ -72,13 +59,10 @@ const Settings = ({ navigation }: SettingsProps): ReactElement => { } const sections = createSettingsSections({ - setSetting, - t, - languageCode, - cityCode, + appContext, navigation, - settings, showSnackbar, + t, }) return ( diff --git a/native/src/testing/TestingAppContext.tsx b/native/src/testing/TestingAppContext.tsx index 6a991ee9d7..fc1961a187 100644 --- a/native/src/testing/TestingAppContext.tsx +++ b/native/src/testing/TestingAppContext.tsx @@ -3,27 +3,30 @@ import React, { ReactElement, ReactNode } from 'react' import { AppContext, AppContextType } from '../contexts/AppContextProvider' import { defaultSettings, SettingsType } from '../utils/AppSettings' -const TestingAppContext = ({ - children, +type TestingAppContextParams = { settings?: Partial } & Omit, 'settings'> + +export const testingAppContext = ({ settings = {}, cityCode = 'augsburg', languageCode = 'de', changeCityCode = jest.fn(), changeLanguageCode = jest.fn(), updateSettings = jest.fn(), -}: { - settings?: Partial - children: ReactNode -} & Omit, 'settings'>): ReactElement => { - const context = { - settings: { ...defaultSettings, ...settings }, - cityCode, - languageCode, - updateSettings, - changeCityCode, - changeLanguageCode, - } +}: TestingAppContextParams): AppContextType => ({ + settings: { ...defaultSettings, ...settings }, + cityCode, + languageCode, + updateSettings, + changeCityCode, + changeLanguageCode, +}) + +const TestingAppContextProvider = ({ + children, + ...props +}: { children: ReactNode } & TestingAppContextParams): ReactElement => { + const context = testingAppContext(props) return {children} } -export default TestingAppContext +export default TestingAppContextProvider diff --git a/native/src/utils/PushNotificationsManager.ts b/native/src/utils/PushNotificationsManager.ts index 24ca65232e..525e60a1d6 100644 --- a/native/src/utils/PushNotificationsManager.ts +++ b/native/src/utils/PushNotificationsManager.ts @@ -9,10 +9,12 @@ import { LOCAL_NEWS_TYPE, NEWS_ROUTE, NonNullableRouteInformationType } from 'sh import { SnackbarType } from '../components/SnackbarContainer' import { RoutesType } from '../constants/NavigationTypes' import buildConfig from '../constants/buildConfig' +import { AppContextType } from '../contexts/AppContextProvider' import urlFromRouteInformation from '../navigation/url' -import appSettings from './AppSettings' import { log, reportError } from './sentry' +type UpdateSettingsType = (settings: { allowPushNotifications: boolean }) => void + type Message = FirebaseMessagingTypes.RemoteMessage & { notification: { title: string } data: { @@ -30,7 +32,7 @@ const importFirebaseMessaging = async (): Promise<() => FirebaseMessagingTypes.M export const pushNotificationsEnabled = (): boolean => buildConfig().featureFlags.pushNotifications && !buildConfig().featureFlags.floss -export const requestPushNotificationPermission = async (): Promise => { +export const requestPushNotificationPermission = async (updateSettings: UpdateSettingsType): Promise => { if (!pushNotificationsEnabled()) { log('Push notifications disabled, no permissions requested.') return false @@ -41,7 +43,7 @@ export const requestPushNotificationPermission = async (): Promise => { if (permissionStatus !== RESULTS.GRANTED) { log(`Permission denied, disabling push notifications in settings.`) - await appSettings.setSettings({ allowPushNotifications: false }) + updateSettings({ allowPushNotifications: false }) } return permissionStatus === RESULTS.GRANTED @@ -65,15 +67,27 @@ export const unsubscribeNews = async (city: string, language: string): Promise => { + +type SubscribeNewsParams = { + cityCode: string + languageCode: string + allowPushNotifications: boolean + skipSettingsCheck?: boolean +} + +export const subscribeNews = async ({ + cityCode, + languageCode, + allowPushNotifications, + skipSettingsCheck = false, +}: SubscribeNewsParams): Promise => { try { - const { allowPushNotifications } = await appSettings.loadSettings() if (!pushNotificationsEnabled() || (!allowPushNotifications && !skipSettingsCheck)) { log('Push notifications disabled, subscription skipped.') return } - const topic = newsTopic(city, language) + const topic = newsTopic(cityCode, languageCode) const messaging = await importFirebaseMessaging() await messaging().subscribeToTopic(topic) @@ -154,7 +168,6 @@ export const backgroundAppStatePushNotificationListener = (listener: (url: strin importFirebaseMessaging() .then(messaging => { const onReceiveURL = ({ url }: { url: string }) => listener(url) - const onReceiveURLListener = Linking.addListener('url', onReceiveURL) const unsubscribeNotification = messaging().onNotificationOpenedApp(message => @@ -175,13 +188,15 @@ export const backgroundAppStatePushNotificationListener = (listener: (url: strin // Since Android 13 and iOS 17 an explicit permission request is needed, otherwise push notifications are not received. // Therefore request the permissions once if not yet granted and subscribe to the current channel if successful. // See https://github.com/digitalfabrik/integreat-app/issues/2438 and https://github.com/digitalfabrik/integreat-app/issues/2655 -export const initialPushNotificationRequest = async (cityCode: string | null, languageCode: string): Promise => { - const { allowPushNotifications } = await appSettings.loadSettings() +export const initialPushNotificationRequest = async (appContext: AppContextType): Promise => { + const { cityCode, languageCode, settings, updateSettings } = appContext + const { allowPushNotifications } = settings + const pushNotificationPermissionGranted = (await checkNotifications()).status === RESULTS.GRANTED if (!pushNotificationPermissionGranted && allowPushNotifications) { - const success = await requestPushNotificationPermission() + const success = await requestPushNotificationPermission(updateSettings) if (success && cityCode) { - await subscribeNews(cityCode, languageCode) + await subscribeNews({ cityCode, languageCode, allowPushNotifications }) } } } diff --git a/native/src/utils/__tests__/PushNotificationsManager.spec.ts b/native/src/utils/__tests__/PushNotificationsManager.spec.ts index f31691f04a..985219527a 100644 --- a/native/src/utils/__tests__/PushNotificationsManager.spec.ts +++ b/native/src/utils/__tests__/PushNotificationsManager.spec.ts @@ -1,23 +1,20 @@ -import AsyncStorage from '@react-native-async-storage/async-storage' import messaging, { FirebaseMessagingTypes } from '@react-native-firebase/messaging' import { mocked } from 'jest-mock' import { requestNotifications } from 'react-native-permissions' import buildConfig from '../../constants/buildConfig' -import appSettings from '../AppSettings' import * as PushNotificationsManager from '../PushNotificationsManager' jest.mock('@react-native-firebase/messaging', () => jest.fn(() => ({}))) describe('PushNotificationsManager', () => { - beforeEach(() => { - AsyncStorage.clear() - jest.clearAllMocks() - }) + beforeEach(jest.clearAllMocks) + const mockedFirebaseMessaging = mocked<() => FirebaseMessagingTypes.Module>(messaging) const mockedBuildConfig = mocked(buildConfig) const previousFirebaseMessaging = mockedFirebaseMessaging() const navigateToDeepLink = jest.fn() + const updateSettings = jest.fn() const mockBuildConfig = (pushNotifications: boolean, floss: boolean) => { const previous = buildConfig() @@ -57,7 +54,7 @@ describe('PushNotificationsManager', () => { return previous }) - expect(await PushNotificationsManager.requestPushNotificationPermission()).toBeFalsy() + expect(await PushNotificationsManager.requestPushNotificationPermission(updateSettings)).toBeFalsy() expect(mockRequestPermission).not.toHaveBeenCalled() }) @@ -70,7 +67,7 @@ describe('PushNotificationsManager', () => { return previous }) - expect(await PushNotificationsManager.requestPushNotificationPermission()).toBeFalsy() + expect(await PushNotificationsManager.requestPushNotificationPermission(updateSettings)).toBeFalsy() expect(mockRequestPermission).not.toHaveBeenCalled() }) @@ -78,16 +75,17 @@ describe('PushNotificationsManager', () => { mockBuildConfig(true, false) mocked(requestNotifications).mockImplementationOnce(async () => ({ status: 'blocked', settings: {} })) - expect(await PushNotificationsManager.requestPushNotificationPermission()).toBeFalsy() - expect((await appSettings.loadSettings()).allowPushNotifications).toBe(false) + expect(await PushNotificationsManager.requestPushNotificationPermission(updateSettings)).toBeFalsy() + expect(updateSettings).toHaveBeenCalledTimes(1) + expect(updateSettings).toHaveBeenCalledWith({ allowPushNotifications: false }) }) it('should request permissions and return true if granted', async () => { mockBuildConfig(true, false) mocked(requestNotifications).mockImplementationOnce(async () => ({ status: 'granted', settings: {} })) - expect(await PushNotificationsManager.requestPushNotificationPermission()).toBeTruthy() - expect((await appSettings.loadSettings()).allowPushNotifications).toBe(true) + expect(await PushNotificationsManager.requestPushNotificationPermission(updateSettings)).toBeTruthy() + expect(updateSettings).not.toHaveBeenCalled() }) }) @@ -143,7 +141,11 @@ describe('PushNotificationsManager', () => { return previous }) - await PushNotificationsManager.subscribeNews('augsburg', 'de') + await PushNotificationsManager.subscribeNews({ + cityCode: 'augsburg', + languageCode: 'de', + allowPushNotifications: true, + }) expect(mockSubscribeToTopic).not.toHaveBeenCalled() }) @@ -156,7 +158,28 @@ describe('PushNotificationsManager', () => { return previous }) - await PushNotificationsManager.subscribeNews('augsburg', 'de') + await PushNotificationsManager.subscribeNews({ + cityCode: 'augsburg', + languageCode: 'de', + allowPushNotifications: true, + }) + expect(mockSubscribeToTopic).not.toHaveBeenCalled() + }) + + it('should return and do nothing if it is disabled in settings', async () => { + mockBuildConfig(true, false) + const mockSubscribeToTopic = jest.fn() + mockedFirebaseMessaging.mockImplementation(() => { + const previous = previousFirebaseMessaging + previous.subscribeToTopic = mockSubscribeToTopic + return previous + }) + + await PushNotificationsManager.subscribeNews({ + cityCode: 'augsburg', + languageCode: 'de', + allowPushNotifications: false, + }) expect(mockSubscribeToTopic).not.toHaveBeenCalled() }) @@ -169,7 +192,30 @@ describe('PushNotificationsManager', () => { return previous }) - await PushNotificationsManager.subscribeNews('augsburg', 'de') + await PushNotificationsManager.subscribeNews({ + cityCode: 'augsburg', + languageCode: 'de', + allowPushNotifications: true, + }) + expect(mockSubscribeToTopic).toHaveBeenCalledWith('augsburg-de-news') + expect(mockSubscribeToTopic).toHaveBeenCalledTimes(1) + }) + + it('should call subscribeToTopic even if push notifications are disabled but skipSettingsCheck is true', async () => { + mockBuildConfig(true, false) + const mockSubscribeToTopic = jest.fn() + mockedFirebaseMessaging.mockImplementation(() => { + const previous = previousFirebaseMessaging + previous.subscribeToTopic = mockSubscribeToTopic + return previous + }) + + await PushNotificationsManager.subscribeNews({ + cityCode: 'augsburg', + languageCode: 'de', + allowPushNotifications: false, + skipSettingsCheck: true, + }) expect(mockSubscribeToTopic).toHaveBeenCalledWith('augsburg-de-news') expect(mockSubscribeToTopic).toHaveBeenCalledTimes(1) }) diff --git a/native/src/utils/__tests__/createSettingsSections.spec.ts b/native/src/utils/__tests__/createSettingsSections.spec.ts index 7061ffda6c..bd6eb9f7db 100644 --- a/native/src/utils/__tests__/createSettingsSections.spec.ts +++ b/native/src/utils/__tests__/createSettingsSections.spec.ts @@ -1,11 +1,11 @@ import { TFunction } from 'i18next' import { mocked } from 'jest-mock' -import { openSettings } from 'react-native-permissions' import { SettingsRouteType } from 'shared' +import { testingAppContext } from '../../testing/TestingAppContext' import createNavigationScreenPropMock from '../../testing/createNavigationPropMock' -import { defaultSettings, SettingsType } from '../AppSettings' +import { SettingsType } from '../AppSettings' import { pushNotificationsEnabled, requestPushNotificationPermission, @@ -31,40 +31,23 @@ const mockUnsubscribeNews = mocked(unsubscribeNews) const mockSubscribeNews = mocked(subscribeNews) const mockedPushNotificationsEnabled = mocked(pushNotificationsEnabled) -type changeSettingFnType = (settings: SettingsType) => Partial -type changeActionFnType = void | ((newSettings: SettingsType) => Promise) - describe('createSettingsSections', () => { - let changeSetting: changeSettingFnType - let changeAction: void | ((newSettings: SettingsType) => Promise) - - beforeEach(() => { - jest.clearAllMocks() - changeSetting = settings => settings - changeAction = async () => true - }) - - const setSetting = async (newChangeSetting: changeSettingFnType, newChangeAction: changeActionFnType) => { - changeSetting = newChangeSetting - changeAction = newChangeAction - } + beforeEach(jest.clearAllMocks) const t = ((key: string) => key) as TFunction - const languageCode = 'de' + const updateSettings = jest.fn() const cityCode = 'augsburg' + const appContext = { ...testingAppContext({}), cityCode, updateSettings } const navigation = createNavigationScreenPropMock() const showSnackbar = jest.fn() - const createSettings = () => + const createSettings = (params: Partial = {}) => createSettingsSections({ - t, - languageCode, - cityCode, + appContext: { ...appContext, settings: { ...appContext.settings, ...params } }, navigation, - settings: defaultSettings, - setSetting, showSnackbar, + t, })[0]!.data describe('allowPushNotifications', () => { @@ -75,89 +58,83 @@ describe('createSettingsSections', () => { expect(sections.find(it => it.title === 'pushNewsTitle')).toBeFalsy() }) - it('should set correct setting on press', () => { + it('should set correct setting on press', async () => { mockedPushNotificationsEnabled.mockImplementation(() => true) const sections = createSettings() - const pushNotificationSection = sections.find(it => it.title === 'pushNewsTitle') - // Initialize changeSetting and changeAction - pushNotificationSection!.onPress() - - const settings = defaultSettings - settings.allowPushNotifications = false - const changedSettings = changeSetting(settings) - expect(pushNotificationSection!.getSettingValue!(settings)).toBeFalsy() - expect(changedSettings.allowPushNotifications).toBeTruthy() - settings.allowPushNotifications = true - const changedSettings2 = changeSetting(settings) - expect(pushNotificationSection!.getSettingValue!(settings)).toBeTruthy() - expect(changedSettings2.allowPushNotifications).toBeFalsy() + const pushNotificationSection = sections.find(it => it.title === 'pushNewsTitle')! + await pushNotificationSection!.onPress() + expect(updateSettings).toHaveBeenCalledTimes(1) + expect(updateSettings).toHaveBeenCalledWith({ allowPushNotifications: false }) + + expect( + pushNotificationSection.getSettingValue!({ ...appContext.settings, allowPushNotifications: false }), + ).toBeFalsy() + expect( + pushNotificationSection!.getSettingValue!({ ...appContext.settings, allowPushNotifications: true }), + ).toBeTruthy() }) it('should unsubscribe from push notification topic', async () => { mockedPushNotificationsEnabled.mockImplementation(() => true) const sections = createSettings() - const pushNotificationSection = sections.find(it => it.title === 'pushNewsTitle') - // Initialize changeSetting and changeAction - pushNotificationSection?.onPress() - const newSettings = defaultSettings - newSettings.allowPushNotifications = false - - const assertedChangeAction = changeAction as (newSettings: SettingsType) => Promise + const pushNotificationSection = sections.find(it => it.title === 'pushNewsTitle')! expect(mockUnsubscribeNews).not.toHaveBeenCalled() - const successful = await assertedChangeAction(newSettings) - expect(successful).toBe(true) + await pushNotificationSection.onPress() + expect(mockUnsubscribeNews).toHaveBeenCalledTimes(1) - expect(mockUnsubscribeNews).toHaveBeenCalledWith(cityCode, languageCode) + expect(mockUnsubscribeNews).toHaveBeenCalledWith(cityCode, appContext.languageCode) expect(mockSubscribeNews).not.toHaveBeenCalled() expect(mockRequestPushNotificationPermission).not.toHaveBeenCalled() + + expect(updateSettings).toHaveBeenCalledTimes(1) + expect(updateSettings).toHaveBeenCalledWith({ allowPushNotifications: false }) }) it('should subscribe to push notification topic if permission is granted', async () => { mockedPushNotificationsEnabled.mockImplementation(() => true) - const sections = createSettings() - const pushNotificationSection = sections.find(it => it.title === 'pushNewsTitle') - // Initialize changeSetting and changeAction - pushNotificationSection?.onPress() - const newSettings = defaultSettings - newSettings.allowPushNotifications = true - - const assertedChangeAction = changeAction as (newSettings: SettingsType) => Promise + const sections = createSettings({ allowPushNotifications: false }) + const pushNotificationSection = sections.find(it => it.title === 'pushNewsTitle')! expect(mockRequestPushNotificationPermission).not.toHaveBeenCalled() expect(mockSubscribeNews).not.toHaveBeenCalled() mockRequestPushNotificationPermission.mockImplementationOnce(async () => true) - const successful = await assertedChangeAction(newSettings) - expect(successful).toBe(true) + await pushNotificationSection.onPress() + expect(mockRequestPushNotificationPermission).toHaveBeenCalledTimes(1) expect(mockSubscribeNews).toHaveBeenCalledTimes(1) - expect(mockSubscribeNews).toHaveBeenCalledWith(cityCode, languageCode, true) + expect(mockSubscribeNews).toHaveBeenCalledWith({ + cityCode, + languageCode: appContext.languageCode, + allowPushNotifications: true, + skipSettingsCheck: true, + }) expect(mockUnsubscribeNews).not.toHaveBeenCalled() + + expect(updateSettings).toHaveBeenCalledTimes(1) + expect(updateSettings).toHaveBeenCalledWith({ allowPushNotifications: true }) }) it('should open settings and return false if permissions not granted', async () => { mockedPushNotificationsEnabled.mockImplementation(() => true) - const sections = createSettings() - const pushNotificationSection = sections.find(it => it.title === 'pushNewsTitle') - // Initialize changeSetting and changeAction - pushNotificationSection?.onPress() - const newSettings = defaultSettings - newSettings.allowPushNotifications = true - - const assertedChangeAction = changeAction as (newSettings: SettingsType) => Promise + const sections = createSettings({ allowPushNotifications: false }) + const pushNotificationSection = sections.find(it => it.title === 'pushNewsTitle')! expect(mockRequestPushNotificationPermission).not.toHaveBeenCalled() expect(mockSubscribeNews).not.toHaveBeenCalled() mockRequestPushNotificationPermission.mockImplementationOnce(async () => false) - const successful = await assertedChangeAction(newSettings) - expect(successful).toBe(false) + await pushNotificationSection.onPress() + expect(mockRequestPushNotificationPermission).toHaveBeenCalledTimes(1) - expect(openSettings).toHaveBeenCalledTimes(1) expect(mockSubscribeNews).not.toHaveBeenCalled() expect(mockUnsubscribeNews).not.toHaveBeenCalled() + + expect(updateSettings).toHaveBeenCalledTimes(2) + expect(updateSettings).toHaveBeenLastCalledWith({ allowPushNotifications: false }) + expect(showSnackbar).toHaveBeenCalledTimes(1) }) }) }) diff --git a/native/src/utils/createSettingsSections.ts b/native/src/utils/createSettingsSections.ts index c526f88ff4..e10a547358 100644 --- a/native/src/utils/createSettingsSections.ts +++ b/native/src/utils/createSettingsSections.ts @@ -9,6 +9,7 @@ import { SnackbarType } from '../components/SnackbarContainer' import NativeConstants from '../constants/NativeConstants' import { NavigationProps } from '../constants/NavigationTypes' import buildConfig from '../constants/buildConfig' +import { CityAppContext } from '../hooks/useCityAppContext' import { SettingsType } from './AppSettings' import { pushNotificationsEnabled, @@ -19,15 +20,10 @@ import { import openExternalUrl from './openExternalUrl' import { initSentry } from './sentry' -export type SetSettingFunctionType = ( - changeSetting: (settings: SettingsType) => Partial, - changeAction?: (newSettings: SettingsType) => Promise, -) => Promise - export type SettingsSectionType = { title: string description?: string - onPress: () => void + onPress: () => Promise | void bigTitle?: boolean role?: Role hasSwitch?: boolean @@ -42,23 +38,17 @@ const volatileValues = { const TRIGGER_VERSION_TAPS = 25 type CreateSettingsSectionsProps = { - setSetting: SetSettingFunctionType - t: TFunction - languageCode: string - cityCode: string | null | undefined + appContext: CityAppContext navigation: NavigationProps - settings: SettingsType showSnackbar: (snackbar: SnackbarType) => void + t: TFunction<'error'> } const createSettingsSections = ({ - setSetting, - t, - languageCode, - cityCode, + appContext: { settings, updateSettings, cityCode, languageCode }, navigation, - settings, showSnackbar, + t, }: CreateSettingsSectionsProps): Readonly>> => [ { title: null, @@ -71,34 +61,29 @@ const createSettingsSections = ({ description: t('pushNewsDescription'), hasSwitch: true, getSettingValue: (settings: SettingsType) => settings.allowPushNotifications, - onPress: () => { - setSetting( - settings => ({ - allowPushNotifications: !settings.allowPushNotifications, - }), - async (newSettings): Promise => { - if (!cityCode) { - // No city selected so nothing to do here (should not ever happen since settings are only available from city content routes) - return true - } + onPress: async () => { + const allowPushNotifications = !settings.allowPushNotifications + updateSettings({ allowPushNotifications }) + if (!allowPushNotifications) { + await unsubscribeNews(cityCode, languageCode) + return + } - if (newSettings.allowPushNotifications) { - const status = await requestPushNotificationPermission() + const status = await requestPushNotificationPermission(updateSettings) - if (status) { - await subscribeNews(cityCode, languageCode, true) - } else { - // If the user has rejected the permission once, it can only be changed in the system settings - openSettings() - // Not successful, reset displayed setting in app - return false - } - } else { - await unsubscribeNews(cityCode, languageCode) - } - return true - }, - ) + if (status) { + await subscribeNews({ cityCode, languageCode, allowPushNotifications, skipSettingsCheck: true }) + } else { + updateSettings({ allowPushNotifications: false }) + // If the user has rejected the permission once, it can only be changed in the system settings + showSnackbar({ + text: 'noPushNotificationPermission', + positiveAction: { + label: t('layout:settings'), + onPress: openSettings, + }, + }) + } }, }, ]), @@ -109,21 +94,16 @@ const createSettingsSections = ({ }), hasSwitch: true, getSettingValue: (settings: SettingsType) => settings.errorTracking, - onPress: () => { - setSetting( - settings => ({ - errorTracking: !settings.errorTracking, - }), - async newSettings => { - const client = Sentry.getCurrentHub().getClient() - if (newSettings.errorTracking && !client) { - initSentry() - } else if (client) { - client.getOptions().enabled = !!newSettings.errorTracking - } - return true - }, - ) + onPress: async () => { + const errorTracking = !settings.errorTracking + updateSettings({ errorTracking }) + + const client = Sentry.getClient() + if (errorTracking && !client) { + initSentry() + } else if (client) { + client.getOptions().enabled = errorTracking + } }, }, { @@ -138,19 +118,19 @@ const createSettingsSections = ({ title: t('about', { appName: buildConfig().appName, }), - onPress: () => { + onPress: async () => { const { aboutUrls } = buildConfig() const aboutUrl = aboutUrls[languageCode] || aboutUrls.default - openExternalUrl(aboutUrl, showSnackbar) + await openExternalUrl(aboutUrl, showSnackbar) }, }, { role: 'link', title: t('privacyPolicy'), - onPress: () => { + onPress: async () => { const { privacyUrls } = buildConfig() const privacyUrl = privacyUrls[languageCode] || privacyUrls.default - openExternalUrl(privacyUrl, showSnackbar) + await openExternalUrl(privacyUrl, showSnackbar) }, }, { diff --git a/release-notes/unreleased/2350-pn-permission-settings.yml b/release-notes/unreleased/2350-pn-permission-settings.yml new file mode 100644 index 0000000000..b1fabb1f09 --- /dev/null +++ b/release-notes/unreleased/2350-pn-permission-settings.yml @@ -0,0 +1,7 @@ +issue_key: 2350 +show_in_stores: true +platforms: + - android + - ios +en: Add explanation if push notification permissions are missing. +de: Es wird nun eine Erklärung angezeigt, wenn die Berechtigungen für Push-Benachrichtigungen fehlen. diff --git a/translations/translations.json b/translations/translations.json index c0f8b88f24..1ea0ec636d 100644 --- a/translations/translations.json +++ b/translations/translations.json @@ -2338,6 +2338,7 @@ "notSupportedByDevice": "Diese Funktion wird auf diesem Gerät nicht unterstützt.", "languageSwitchFailedTitle": "Leider ist der Sprachwechsel fehlgeschlagen.", "languageSwitchFailedMessage": "Die ausgewählte Sprache ist nicht offline verfügbar. Eine aktive Internetverbinding ist notwendig.", + "noPushNotificatidbonPermission": "Berechtigung erforderlich", "noCalendarPermission": "Der Zugriff auf den Kalender ist nicht erlaubt.", "noCalendarFound": "Es wurde kein geeigneter Kalender gefunden." }, @@ -2618,6 +2619,7 @@ "notSupportedByDevice": "This function is not supported on this device.", "languageSwitchFailedTitle": "Unfortunately, changing the language failed.", "languageSwitchFailedMessage": "The selected language is not available if you are offline. An active internet connection is required.", + "noPushNotificationPermission": "Permission required", "noCalendarPermission": "Access to the calendar is not permitted.", "noCalendarFound": "No suitable calendar was found." }, @@ -3617,7 +3619,6 @@ "chooseCalendar": "Kalender-Auswahl", "added": "Hinzugefügt", "goToCalendar": "Zum Kalender", - "settings": "Einstellungen", "recurring": "wiederholend", "today": "heute", "todayRecurring": "heute, wiederholend", @@ -3647,7 +3648,6 @@ "chooseCalendar": "የቀን መቁጠሪያን ይምረጡ", "added": "ታክሏል", "goToCalendar": "ወደ ቀን መቁጠሪያ ይሂዱ", - "settings": "ቅንብሮች", "recurring": "ተደጋጋሚ", "today": "ዛሬ", "todayRecurring": "ዛሬ፣ ተደጋጋሚ", @@ -3677,7 +3677,6 @@ "chooseCalendar": "اختر تقويمًا", "added": "تمت الإضافة", "goToCalendar": "إلى التقويم", - "settings": "أوضاع الضبط", "recurring": "مكرر", "today": "اليوم", "todayRecurring": "اليوم، مكرر", @@ -3707,7 +3706,6 @@ "chooseCalendar": "Избери календар", "added": "Добавен", "goToCalendar": "Към календара", - "settings": "Настройки", "recurring": "повтарящ се", "today": "днес", "todayRecurring": "днес, повтарящ се", @@ -3737,7 +3735,6 @@ "chooseCalendar": "ساڵنامەیەک هەڵبژێرە", "added": "زیادکرا", "goToCalendar": "دەسپێگەیشتن بە ساڵنامە", - "settings": "ڕێکخستنەکان", "recurring": "دووپاتە", "today": "ئەمڕۆ", "todayRecurring": "ئەمڕۆ، دووپاتە", @@ -3767,7 +3764,6 @@ "chooseCalendar": "Výběr kalendáře", "added": "Přidáno", "goToCalendar": "Do kalendáře", - "settings": "Nastavení", "recurring": "opakování", "today": "dnes", "todayRecurring": "dnes, opakování", @@ -3797,7 +3793,6 @@ "chooseCalendar": "Valg af kalender", "added": "Tilføjet", "goToCalendar": "Til kalenderen", - "settings": "Indstillinger", "recurring": "gentagende", "today": "i dag", "todayRecurring": "i dag og gentagende", @@ -3827,7 +3822,6 @@ "chooseCalendar": "Επιλέξτε ένα ημερολόγιο", "added": "Προστέθηκε", "goToCalendar": "Μετάβαση στο ημερολόγιο", - "settings": "Ρυθμίσεις", "recurring": "επαναλαμβάνεται", "today": "σήμερα", "todayRecurring": "σήμερα, επαναλαμβάνεται", @@ -3857,7 +3851,6 @@ "chooseCalendar": "Calendar choice", "added": "Added", "goToCalendar": "Go to calendar", - "settings": "Settings", "recurring": "recurring", "today": "today", "todayRecurring": "today, recurring", @@ -3887,7 +3880,6 @@ "chooseCalendar": "Elije un calendario", "added": "Se añadió", "goToCalendar": "al calendario", - "settings": "Ajustes", "recurring": "repetitivo", "today": "hoy", "todayRecurring": "hoy, repetitivo", @@ -3917,7 +3909,6 @@ "chooseCalendar": "Valitse kalenteri", "added": "Lisätty", "goToCalendar": "Kalenteriin", - "settings": "Asetukset", "recurring": "toistuva", "today": "tänään", "todayRecurring": "tänään, toistuva", @@ -3947,7 +3938,6 @@ "chooseCalendar": "Choisis un calendrier", "added": "Ajouté", "goToCalendar": "Accéder au calendrier", - "settings": "Réglages", "recurring": "répétitif", "today": "aujourd’hui", "todayRecurring": "aujourd’hui, répétitif", @@ -3977,7 +3967,6 @@ "chooseCalendar": "Izaberi kalendar", "added": "Dodano", "goToCalendar": "Na kalendar", - "settings": "Postavke", "recurring": "ponavljajući", "today": "danas", "todayRecurring": "danas, ponavljajući", @@ -4007,7 +3996,6 @@ "chooseCalendar": "Válasszon ki egy naptárat", "added": "Hozzáadva", "goToCalendar": "A naptárhoz", - "settings": "Beállítások", "recurring": "ismétlődő", "today": "ma", "todayRecurring": "ma, ismétlődve", @@ -4037,7 +4025,6 @@ "chooseCalendar": "Seleziona un calendario", "added": "Aggiunto", "goToCalendar": "Al calendario", - "settings": "Impostazioni", "recurring": "ricorrente", "today": "oggi", "todayRecurring": "oggi, ricorrente", @@ -4067,7 +4054,6 @@ "chooseCalendar": "აირჩიე კალენდარი", "added": "დამატებულია", "goToCalendar": "კალენდარზე გადასვლა", - "settings": "პარამეტრები", "recurring": "განმეორებით", "today": "დღეს", "todayRecurring": "დღეს, განმეორებით", @@ -4097,7 +4083,6 @@ "chooseCalendar": "Rojhejmêrekê bibijêre", "added": "Zêdekirin", "goToCalendar": "Li rojhejmêrê", - "settings": "Eyarkirin", "recurring": "Dubarebûyî", "today": "Îro", "todayRecurring": "Îro, dubarebûyî", @@ -4127,7 +4112,6 @@ "chooseCalendar": "Избери календар", "added": "Додадено", "goToCalendar": "Кон календарот", - "settings": "Поставки", "recurring": "повторувачки", "today": "утре", "todayRecurring": "утре, повторувачки", @@ -4157,7 +4141,6 @@ "chooseCalendar": "Selecteer een kalender", "added": "Toegevoegd", "goToCalendar": "Naar de kalender", - "settings": "Instellingen", "recurring": "herhalend", "today": "vandaag", "todayRecurring": "vandaag, herhalend", @@ -4187,7 +4170,6 @@ "chooseCalendar": "Filannoo Kaalanderii", "added": "Dabalameera", "goToCalendar": "Gara kaalanderii", - "settings": "Saajoo", "recurring": "Irra deddeebii", "today": "Hara", "todayRecurring": "Irra deebii haraa", @@ -4217,7 +4199,6 @@ "chooseCalendar": "یک تقویم را انتخاب کنید", "added": "افزوده شد", "goToCalendar": "دسترسی به تقویم", - "settings": "تنظیمات", "recurring": "تکراری", "today": "امروز", "todayRecurring": "امروز، تکراری", @@ -4247,7 +4228,6 @@ "chooseCalendar": "Wybierz kalendarz", "added": "Dodano", "goToCalendar": "Do kalendarza", - "settings": "Ustawienia", "recurring": "powtarzający się", "today": "dzisiaj", "todayRecurring": "dzisiaj, powtarzający się", @@ -4277,7 +4257,6 @@ "chooseCalendar": "یک تقویم را انتخاب کنید", "added": "اضافه شد", "goToCalendar": "دسترسی به تقویم", - "settings": "تنظیمات", "recurring": "تکراری", "today": "امروز", "todayRecurring": "امروز، تکراری", @@ -4307,7 +4286,6 @@ "chooseCalendar": "يوه جنتري غوره کړئ", "added": "اضافه شو", "goToCalendar": "جنتري ته لاسرسی", - "settings": "تنظيمات", "recurring": "تکراري", "today": "نن ورځ", "todayRecurring": "نن ورځ، تکراري", @@ -4337,7 +4315,6 @@ "chooseCalendar": "Selecione um calendário", "added": "Adicionado", "goToCalendar": "Para o calendário", - "settings": "Definições", "recurring": "repetitivo", "today": "hoje", "todayRecurring": "hoje, repetitivo", @@ -4367,7 +4344,6 @@ "chooseCalendar": "Selectați un calendar", "added": "Adăugat", "goToCalendar": "La calendar", - "settings": "Setări", "recurring": "repetarea", "today": "astăzi", "todayRecurring": "astăzi, repetând", @@ -4397,7 +4373,6 @@ "chooseCalendar": "Выбери календарь", "added": "Добавлено", "goToCalendar": "К календарю", - "settings": "Настройки", "recurring": "периодически", "today": "сегодня", "todayRecurring": "сегодня, периодически", @@ -4427,7 +4402,6 @@ "chooseCalendar": "Výber kalendára", "added": "Pridané", "goToCalendar": "Ku kalendáru", - "settings": "Nastavenia", "recurring": "opakujúce sa", "today": "dnes", "todayRecurring": "dnes, opakujúce sa", @@ -4457,7 +4431,6 @@ "chooseCalendar": "Dooro jadwalka", "added": "Ku daray", "goToCalendar": "Jadwalka", - "settings": "Dejinta", "recurring": "Soo noqnoqda", "today": "Maanta", "todayRecurring": "Maanta, soo noqnoqonaysa", @@ -4487,7 +4460,6 @@ "chooseCalendar": "Zgjidh një kalendar", "added": "U shtua", "goToCalendar": "Shko te kalendari", - "settings": "Cilësimet", "recurring": "periodik", "today": "sot", "todayRecurring": "sot, periodik", @@ -4517,7 +4489,6 @@ "chooseCalendar": "Изабери календар", "added": "Додато", "goToCalendar": "На календар", - "settings": "Подешавања", "recurring": "понављајући", "today": "данас", "todayRecurring": "данас, понављајући", @@ -4547,7 +4518,6 @@ "chooseCalendar": "Izaberi kalendar", "added": "Dodato", "goToCalendar": "Na kalendar", - "settings": "Podešavanja", "recurring": "ponavljajući", "today": "danas", "todayRecurring": "danas, ponavljajući", @@ -4577,7 +4547,6 @@ "chooseCalendar": "ዓውደ ኣዋርሕ ምረጽ", "added": "ተወሰኸ", "goToCalendar": "ናብ ዓውደ ኣዋርሕ", - "settings": "ቅጥዕታት", "recurring": "ተደጋጋሚ", "today": "ሎምዓንቲ", "todayRecurring": "ሎሚ፣ ተደጋጋሚ", @@ -4607,7 +4576,6 @@ "chooseCalendar": "Bir takvim seç", "added": "Eklendi", "goToCalendar": "Takvime git", - "settings": "Ayarlar", "recurring": "tekrarlayan", "today": "bugün", "todayRecurring": "bugün, tekrarlayan", @@ -4637,7 +4605,6 @@ "chooseCalendar": "Виберіть календар", "added": "Додано", "goToCalendar": "До календаря", - "settings": "Налаштування", "recurring": "періодичні", "today": "сьогодні", "todayRecurring": "сьогодні, періодичні", @@ -4667,7 +4634,6 @@ "chooseCalendar": "ایک کیلنڈر منتخب کریں", "added": "شامل کیا گیا", "goToCalendar": "کیلنڈر پر جائیں", - "settings": "ترتیبات", "recurring": "مکرر", "today": "آج", "todayRecurring": "آج، مکرر", @@ -4697,7 +4663,6 @@ "chooseCalendar": "选择日历", "added": "添加到日历", "goToCalendar": "转到日历", - "settings": "设置", "recurring": "重复", "today": "今天", "todayRecurring": "今天，重复", From 4ba6c4ff821c64b91791b36f3469ca5fffbd3f15 Mon Sep 17 00:00:00 2001 From: Steffen Kleinle Date: Mon, 11 Nov 2024 09:49:28 +0100 Subject: [PATCH 2/3] 2350: Better naming and handle synchronous errors --- native/src/components/SettingItem.tsx | 2 +- native/src/routes/Settings.tsx | 8 +++++--- native/src/utils/createSettingsSections.ts | 23 +++++++++++++--------- translations/translations.json | 4 ++-- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/native/src/components/SettingItem.tsx b/native/src/components/SettingItem.tsx index f73dac6eba..fb47868e76 100644 --- a/native/src/components/SettingItem.tsx +++ b/native/src/components/SettingItem.tsx @@ -45,7 +45,7 @@ const Badge = styled.View<{ enabled: boolean }>` type SettingItemProps = { title: string description?: string - onPress: () => void + onPress: () => Promise bigTitle?: boolean role?: Role hasSwitch?: boolean diff --git a/native/src/routes/Settings.tsx b/native/src/routes/Settings.tsx index 66e176e8fb..f911125b85 100644 --- a/native/src/routes/Settings.tsx +++ b/native/src/routes/Settings.tsx @@ -34,14 +34,16 @@ const Settings = ({ navigation }: SettingsProps): ReactElement => { const { t } = useTranslation('settings') const { settings } = appContext - const safeOnPress = (update: () => Promise | void) => () => { + const safeOnPress = (update: () => Promise | void) => async () => { const oldSettings = settings - update()?.catch(e => { + try { + await update() + } catch (e) { log('Failed to persist settings.', 'error') reportError(e) appContext.updateSettings(oldSettings) showSnackbar({ text: t('error:unknownError') }) - }) + } } const renderItem = ({ item }: { item: SettingsSectionType }) => { diff --git a/native/src/utils/createSettingsSections.ts b/native/src/utils/createSettingsSections.ts index e10a547358..40a837519b 100644 --- a/native/src/utils/createSettingsSections.ts +++ b/native/src/utils/createSettingsSections.ts @@ -62,9 +62,9 @@ const createSettingsSections = ({ hasSwitch: true, getSettingValue: (settings: SettingsType) => settings.allowPushNotifications, onPress: async () => { - const allowPushNotifications = !settings.allowPushNotifications - updateSettings({ allowPushNotifications }) - if (!allowPushNotifications) { + const newAllowPushNotifications = !settings.allowPushNotifications + updateSettings({ allowPushNotifications: newAllowPushNotifications }) + if (!newAllowPushNotifications) { await unsubscribeNews(cityCode, languageCode) return } @@ -72,12 +72,17 @@ const createSettingsSections = ({ const status = await requestPushNotificationPermission(updateSettings) if (status) { - await subscribeNews({ cityCode, languageCode, allowPushNotifications, skipSettingsCheck: true }) + await subscribeNews({ + cityCode, + languageCode, + allowPushNotifications: newAllowPushNotifications, + skipSettingsCheck: true, + }) } else { updateSettings({ allowPushNotifications: false }) // If the user has rejected the permission once, it can only be changed in the system settings showSnackbar({ - text: 'noPushNotificationPermission', + text: 'permissionRequired', positiveAction: { label: t('layout:settings'), onPress: openSettings, @@ -95,14 +100,14 @@ const createSettingsSections = ({ hasSwitch: true, getSettingValue: (settings: SettingsType) => settings.errorTracking, onPress: async () => { - const errorTracking = !settings.errorTracking - updateSettings({ errorTracking }) + const newErrorTracking = !settings.errorTracking + updateSettings({ errorTracking: newErrorTracking }) const client = Sentry.getClient() - if (errorTracking && !client) { + if (newErrorTracking && !client) { initSentry() } else if (client) { - client.getOptions().enabled = errorTracking + client.getOptions().enabled = newErrorTracking } }, }, diff --git a/translations/translations.json b/translations/translations.json index 1ea0ec636d..2a8d010504 100644 --- a/translations/translations.json +++ b/translations/translations.json @@ -2338,7 +2338,7 @@ "notSupportedByDevice": "Diese Funktion wird auf diesem Gerät nicht unterstützt.", "languageSwitchFailedTitle": "Leider ist der Sprachwechsel fehlgeschlagen.", "languageSwitchFailedMessage": "Die ausgewählte Sprache ist nicht offline verfügbar. Eine aktive Internetverbinding ist notwendig.", - "noPushNotificatidbonPermission": "Berechtigung erforderlich", + "permissionRequired": "Berechtigung erforderlich", "noCalendarPermission": "Der Zugriff auf den Kalender ist nicht erlaubt.", "noCalendarFound": "Es wurde kein geeigneter Kalender gefunden." }, @@ -2619,7 +2619,7 @@ "notSupportedByDevice": "This function is not supported on this device.", "languageSwitchFailedTitle": "Unfortunately, changing the language failed.", "languageSwitchFailedMessage": "The selected language is not available if you are offline. An active internet connection is required.", - "noPushNotificationPermission": "Permission required", + "permissionRequired": "Permission required", "noCalendarPermission": "Access to the calendar is not permitted.", "noCalendarFound": "No suitable calendar was found." }, From a611e7f831cdcf2f22dce4e8c0436044af0fa476 Mon Sep 17 00:00:00 2001 From: Steffen Kleinle Date: Mon, 11 Nov 2024 13:32:45 +0100 Subject: [PATCH 3/3] 2350: Add custom error message --- native/src/routes/Settings.tsx | 2 +- translations/translations.json | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/native/src/routes/Settings.tsx b/native/src/routes/Settings.tsx index f911125b85..37595d1c9d 100644 --- a/native/src/routes/Settings.tsx +++ b/native/src/routes/Settings.tsx @@ -42,7 +42,7 @@ const Settings = ({ navigation }: SettingsProps): ReactElement => { log('Failed to persist settings.', 'error') reportError(e) appContext.updateSettings(oldSettings) - showSnackbar({ text: t('error:unknownError') }) + showSnackbar({ text: t('error:settingsError') }) } } diff --git a/translations/translations.json b/translations/translations.json index 2a8d010504..972a262f69 100644 --- a/translations/translations.json +++ b/translations/translations.json @@ -2340,7 +2340,8 @@ "languageSwitchFailedMessage": "Die ausgewählte Sprache ist nicht offline verfügbar. Eine aktive Internetverbinding ist notwendig.", "permissionRequired": "Berechtigung erforderlich", "noCalendarPermission": "Der Zugriff auf den Kalender ist nicht erlaubt.", - "noCalendarFound": "Es wurde kein geeigneter Kalender gefunden." + "noCalendarFound": "Es wurde kein geeigneter Kalender gefunden.", + "settingsError": "Fehler beim Anwenden der Einstellungen" }, "am": { "notFound": { @@ -2621,7 +2622,8 @@ "languageSwitchFailedMessage": "The selected language is not available if you are offline. An active internet connection is required.", "permissionRequired": "Permission required", "noCalendarPermission": "Access to the calendar is not permitted.", - "noCalendarFound": "No suitable calendar was found." + "noCalendarFound": "No suitable calendar was found.", + "settingsError": "Failed to apply settings" }, "es": { "notFound": {