Merge pull request #2960 from digitalfabrik/2350-pn-permission-settings

2350: Show snackbar instead of opening settings

native/src/Navigator.tsx

@@ -88,16 +88,17 @@ const Stack = createStackNavigator<RoutesParamsType>()
 
 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<NavigationProps<RoutesType>>()
   const [initialRoute, setInitialRoute] = useState<InitialRouteType>(null)
 
   // Preload cities
   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 })
 

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

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<void>
   bigTitle?: boolean
   role?: Role
   hasSwitch?: boolean

native/src/contexts/AppContextProvider.tsx

@@ -34,6 +34,7 @@ type AppContextProviderProps = {
 
 const AppContextProvider = ({ children }: AppContextProviderProps): ReactElement | null => {
   const [settings, setSettings] = useState<SettingsType | null>(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(() => {

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')
   })

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!')

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,27 @@ 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<SettingsType>,
-    changeAction?: (settings: SettingsType) => Promise<boolean>,
-  ) => {
+  const safeOnPress = (update: () => Promise<void> | void) => async () => {
     const oldSettings = settings
-    const newSettings = { ...oldSettings, ...changeSetting(settings) }
-    updateSettings(newSettings)
-
     try {
-      const successful = changeAction ? await changeAction(newSettings) : true
-
-      if (!successful) {
-        updateSettings(oldSettings)
-      }
+      await update()
     } catch (e) {
       log('Failed to persist settings.', 'error')
       reportError(e)
-      updateSettings(oldSettings)
+      appContext.updateSettings(oldSettings)
+      showSnackbar({ text: t('error:settingsError') })
     }
   }
 
   const renderItem = ({ item }: { item: SettingsSectionType }) => {
-    const { getSettingValue, ...otherProps } = item
+    const { getSettingValue, onPress, ...otherProps } = item
     const value = !!(getSettingValue && getSettingValue(settings))
-    return <SettingItem value={value} key={otherProps.title} {...otherProps} />
+    return <SettingItem value={value} key={otherProps.title} onPress={safeOnPress(onPress)} {...otherProps} />
   }
 
   const renderSectionHeader = ({ section: { title } }: { section: SectionType }) => {
@@ -72,13 +61,10 @@ const Settings = ({ navigation }: SettingsProps): ReactElement => {
   }
 
   const sections = createSettingsSections({
-    setSetting,
-    t,
-    languageCode,
-    cityCode,
+    appContext,
     navigation,
-    settings,
     showSnackbar,
+    t,
   })
 
   return (

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<SettingsType> } & Omit<Partial<AppContextType>, 'settings'>
+
+export const testingAppContext = ({
   settings = {},
   cityCode = 'augsburg',
   languageCode = 'de',
   changeCityCode = jest.fn(),
   changeLanguageCode = jest.fn(),
   updateSettings = jest.fn(),
-}: {
-  settings?: Partial<SettingsType>
-  children: ReactNode
-} & Omit<Partial<AppContextType>, '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 <AppContext.Provider value={context}>{children}</AppContext.Provider>
 }
 
-export default TestingAppContext
+export default TestingAppContextProvider

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<boolean> => {
+export const requestPushNotificationPermission = async (updateSettings: UpdateSettingsType): Promise<boolean> => {
   if (!pushNotificationsEnabled()) {
     log('Push notifications disabled, no permissions requested.')
     return false
@@ -41,7 +43,7 @@ export const requestPushNotificationPermission = async (): Promise<boolean> => {
 
   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<v
   }
   log(`Unsubscribed from ${topic} topic!`)
 }
-export const subscribeNews = async (city: string, language: string, skipSettingsCheck = false): Promise<void> => {
+
+type SubscribeNewsParams = {
+  cityCode: string
+  languageCode: string
+  allowPushNotifications: boolean
+  skipSettingsCheck?: boolean
+}
+
+export const subscribeNews = async ({
+  cityCode,
+  languageCode,
+  allowPushNotifications,
+  skipSettingsCheck = false,
+}: SubscribeNewsParams): Promise<void> => {
   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<void> => {
-  const { allowPushNotifications } = await appSettings.loadSettings()
+export const initialPushNotificationRequest = async (appContext: AppContextType): Promise<void> => {
+  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 })
     }
   }
 }