From e295a34914cb82f45b7e6dfcb9813c664e01c8d9 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 17 Jan 2024 22:29:46 -0800 Subject: [PATCH] notifs: Add snoozable banner for notifs-soon-to-expire --- src/__tests__/lib/exampleData.js | 4 + src/account/__tests__/accountsReducer-test.js | 78 +++++++++++++++++++ src/account/accountActions.js | 10 +++ src/account/accountsReducer.js | 17 ++++ src/actionConstants.js | 2 + src/actionTypes.js | 8 ++ src/common/ServerNotifsExpiringBanner.js | 76 ++++++++++++++++++ src/main/HomeScreen.js | 2 + src/storage/__tests__/migrations-test.js | 27 ++++++- src/storage/migrations.js | 6 ++ src/types.js | 19 +++-- 11 files changed, 241 insertions(+), 8 deletions(-) create mode 100644 src/common/ServerNotifsExpiringBanner.js diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js index 768ad8f268..c43a45ec43 100644 --- a/src/__tests__/lib/exampleData.js +++ b/src/__tests__/lib/exampleData.js @@ -225,6 +225,7 @@ export const makeAccount = ( zulipVersion?: ZulipVersion | null, ackedPushToken?: string | null, lastDismissedServerPushSetupNotice?: Date | null, + lastDismissedServerNotifsExpiringBanner?: Date | null, |} = Object.freeze({}), ): Account => { const { @@ -237,6 +238,7 @@ export const makeAccount = ( zulipVersion: zulipVersionInner = recentZulipVersion, ackedPushToken = null, lastDismissedServerPushSetupNotice = null, + lastDismissedServerNotifsExpiringBanner = null, } = args; return deepFreeze({ realm: realmInner, @@ -247,6 +249,7 @@ export const makeAccount = ( zulipVersion: zulipVersionInner, ackedPushToken, lastDismissedServerPushSetupNotice, + lastDismissedServerNotifsExpiringBanner, silenceServerPushSetupWarnings: false, }); }; @@ -648,6 +651,7 @@ export const plusReduxState: GlobalState & PerAccountState = reduxState({ zulipVersion: recentZulipVersion, zulipFeatureLevel: recentZulipFeatureLevel, lastDismissedServerPushSetupNotice: null, + lastDismissedServerNotifsExpiringBanner: null, silenceServerPushSetupWarnings: false, }, ], diff --git a/src/account/__tests__/accountsReducer-test.js b/src/account/__tests__/accountsReducer-test.js index 782b0040ed..63d46de370 100644 --- a/src/account/__tests__/accountsReducer-test.js +++ b/src/account/__tests__/accountsReducer-test.js @@ -73,6 +73,31 @@ describe('accountsReducer', () => { ); expect(actualState).toEqual([account]); }); + + test('when realm_push_notifications_enabled_end_timestamp is null, clears lastDismissedServerNotifsExpiringBanner', () => { + const account = { ...eg.selfAccount, lastDismissedServerNotifsExpiringBanner: new Date() }; + const actualState = accountsReducer( + [account], + eg.mkActionRegisterComplete({ realm_push_notifications_enabled_end_timestamp: null }), + ); + expect(actualState).toEqual([{ ...account, lastDismissedServerNotifsExpiringBanner: null }]); + }); + + test('when realm_push_notifications_enabled_end_timestamp is not null, preserves lastDismissedServerNotifsExpiringBanner', () => { + const account = { ...eg.selfAccount, lastDismissedServerNotifsExpiringBanner: new Date() }; + const actualState = accountsReducer( + [account], + eg.mkActionRegisterComplete({ realm_push_notifications_enabled_end_timestamp: 1705616035 }), + ); + expect(actualState).toEqual([account]); + }); + + // TODO(server-8.0) + test('legacy: when realm_push_notifications_enabled_end_timestamp missing, preserves lastDismissedServerNotifsExpiringBanner: null', () => { + const account = { ...eg.selfAccount, lastDismissedServerNotifsExpiringBanner: null }; + const actualState = accountsReducer([account], eg.mkActionRegisterComplete({})); + expect(actualState).toEqual([account]); + }); }); describe('ACCOUNT_SWITCH', () => { @@ -296,5 +321,58 @@ describe('accountsReducer', () => { expect(actualState).toEqual(stateWithoutDismissedNotice); }); }); + + describe('lastDismissedServerNotifsExpiringBanner', () => { + const stateWithDismissedBanner = [ + { ...eg.plusReduxState.accounts[0], lastDismissedServerNotifsExpiringBanner: new Date() }, + ]; + const stateWithoutDismissedBanner = [ + { ...eg.plusReduxState.accounts[0], lastDismissedServerNotifsExpiringBanner: null }, + ]; + + const someTimestamp = 1705616035; + + test('data.push_notifications_enabled_end_timestamp is null, on state with dismissed banner', () => { + const actualState = accountsReducer( + stateWithDismissedBanner, + eventWith({ push_notifications_enabled_end_timestamp: null }), + ); + expect(actualState).toEqual(stateWithoutDismissedBanner); + }); + + test('data.push_notifications_enabled_end_timestamp is null, on state without dismissed banner', () => { + const actualState = accountsReducer( + stateWithoutDismissedBanner, + eventWith({ push_notifications_enabled_end_timestamp: null }), + ); + expect(actualState).toEqual(stateWithoutDismissedBanner); + }); + + test('data.push_notifications_enabled_end_timestamp is non-null, on state with dismissed banner', () => { + const actualState = accountsReducer( + stateWithDismissedBanner, + eventWith({ push_notifications_enabled_end_timestamp: someTimestamp }), + ); + expect(actualState).toEqual(stateWithDismissedBanner); + }); + + test('data.push_notifications_enabled_end_timestamp is non-null, on state without dismissed banner', () => { + const actualState = accountsReducer( + stateWithoutDismissedBanner, + eventWith({ push_notifications_enabled_end_timestamp: someTimestamp }), + ); + expect(actualState).toEqual(stateWithoutDismissedBanner); + }); + + test('data.push_notifications_enabled_end_timestamp is absent, on state with dismissed banner', () => { + const actualState = accountsReducer(stateWithDismissedBanner, eventWith({})); + expect(actualState).toEqual(stateWithDismissedBanner); + }); + + test('data.push_notifications_enabled_end_timestamp is absent, on state without dismissed banner', () => { + const actualState = accountsReducer(stateWithoutDismissedBanner, eventWith({})); + expect(actualState).toEqual(stateWithoutDismissedBanner); + }); + }); }); }); diff --git a/src/account/accountActions.js b/src/account/accountActions.js index b88d32f43a..604945977a 100644 --- a/src/account/accountActions.js +++ b/src/account/accountActions.js @@ -7,6 +7,7 @@ import { LOGIN_SUCCESS, DISMISS_SERVER_PUSH_SETUP_NOTICE, SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, + DISMISS_SERVER_NOTIFS_EXPIRING_BANNER, } from '../actionConstants'; import { registerAndStartPolling } from '../events/eventActions'; import { resetToMainTabs } from '../nav/navActions'; @@ -23,6 +24,15 @@ export const dismissServerPushSetupNotice = (): PerAccountAction => ({ date: new Date(), }); +export const dismissServerNotifsExpiringBanner = (): PerAccountAction => ({ + type: DISMISS_SERVER_NOTIFS_EXPIRING_BANNER, + + // We don't compute this in a reducer function. Those should be pure + // functions of their params: + // https://redux.js.org/tutorials/fundamentals/part-3-state-actions-reducers#rules-of-reducers + date: new Date(), +}); + export const setSilenceServerPushSetupWarnings = (value: boolean): PerAccountAction => ({ type: SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, value, diff --git a/src/account/accountsReducer.js b/src/account/accountsReducer.js index 3957ffe1f8..d1db546ea4 100644 --- a/src/account/accountsReducer.js +++ b/src/account/accountsReducer.js @@ -12,6 +12,7 @@ import { DISMISS_SERVER_PUSH_SETUP_NOTICE, ACCOUNT_REMOVE, SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, + DISMISS_SERVER_NOTIFS_EXPIRING_BANNER, } from '../actionConstants'; import { EventTypes } from '../api/eventTypes'; import type { AccountsState, Identity, Action, Account } from '../types'; @@ -51,6 +52,10 @@ export default (state: AccountsState = initialState, action: Action): AccountsSt lastDismissedServerPushSetupNotice: action.data.realm_push_notifications_enabled ? null : current.lastDismissedServerPushSetupNotice, + lastDismissedServerNotifsExpiringBanner: + action.data.realm_push_notifications_enabled_end_timestamp == null + ? null + : current.lastDismissedServerNotifsExpiringBanner, })); case ACCOUNT_SWITCH: { @@ -79,6 +84,7 @@ export default (state: AccountsState = initialState, action: Action): AccountsSt zulipVersion: null, zulipFeatureLevel: null, lastDismissedServerPushSetupNotice: null, + lastDismissedServerNotifsExpiringBanner: null, silenceServerPushSetupWarnings: false, }, ...state, @@ -136,6 +142,13 @@ export default (state: AccountsState = initialState, action: Action): AccountsSt })); } + case DISMISS_SERVER_NOTIFS_EXPIRING_BANNER: { + return updateActiveAccount(state, current => ({ + ...current, + lastDismissedServerNotifsExpiringBanner: action.date, + })); + } + case SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS: { return updateActiveAccount(state, current => ({ ...current, @@ -178,6 +191,10 @@ export default (state: AccountsState = initialState, action: Action): AccountsSt event.data.push_notifications_enabled === true ? null : current.lastDismissedServerPushSetupNotice, + lastDismissedServerNotifsExpiringBanner: + event.data.push_notifications_enabled_end_timestamp === null + ? null + : current.lastDismissedServerNotifsExpiringBanner, })); } diff --git a/src/actionConstants.js b/src/actionConstants.js index 5c354fcd6c..06a7e3dc38 100644 --- a/src/actionConstants.js +++ b/src/actionConstants.js @@ -21,6 +21,8 @@ export const REFRESH_SERVER_EMOJI_DATA: 'REFRESH_SERVER_EMOJI_DATA' = 'REFRESH_S export const DISMISS_SERVER_PUSH_SETUP_NOTICE: 'DISMISS_SERVER_PUSH_SETUP_NOTICE' = 'DISMISS_SERVER_PUSH_SETUP_NOTICE'; +export const DISMISS_SERVER_NOTIFS_EXPIRING_BANNER: 'DISMISS_SERVER_NOTIFS_EXPIRING_BANNER' = + 'DISMISS_SERVER_NOTIFS_EXPIRING_BANNER'; export const SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS: 'SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS' = 'SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS'; diff --git a/src/actionTypes.js b/src/actionTypes.js index ca767b492a..e86378dc70 100644 --- a/src/actionTypes.js +++ b/src/actionTypes.js @@ -61,6 +61,7 @@ import { REGISTER_PUSH_TOKEN_START, REGISTER_PUSH_TOKEN_END, SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, + DISMISS_SERVER_NOTIFS_EXPIRING_BANNER, } from './actionConstants'; import type { UserMessageFlag } from './api/modelTypes'; @@ -177,6 +178,11 @@ type DismissServerPushSetupNoticeAction = $ReadOnly<{| date: Date, |}>; +type DismissServerNotifsExpiringBannerAction = $ReadOnly<{| + type: typeof DISMISS_SERVER_NOTIFS_EXPIRING_BANNER, + date: Date, +|}>; + type SetSilenceServerPushSetupWarningsAction = $ReadOnly<{| type: typeof SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, value: boolean, @@ -677,6 +683,7 @@ export type PerAccountAction = // state.session | DismissServerCompatNoticeAction | DismissServerPushSetupNoticeAction + | DismissServerNotifsExpiringBannerAction | SetSilenceServerPushSetupWarningsAction | ToggleOutboxSendingAction ; @@ -804,6 +811,7 @@ export function isPerAccountApplicableAction(action: Action): boolean { case CLEAR_TYPING: case DISMISS_SERVER_COMPAT_NOTICE: case DISMISS_SERVER_PUSH_SETUP_NOTICE: + case DISMISS_SERVER_NOTIFS_EXPIRING_BANNER: case SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS: case TOGGLE_OUTBOX_SENDING: (action: PerAccountAction); diff --git a/src/common/ServerNotifsExpiringBanner.js b/src/common/ServerNotifsExpiringBanner.js new file mode 100644 index 0000000000..0b945aa066 --- /dev/null +++ b/src/common/ServerNotifsExpiringBanner.js @@ -0,0 +1,76 @@ +/* @flow strict-local */ + +import React from 'react'; +import type { Node } from 'react'; +import subWeeks from 'date-fns/subWeeks'; + +import ZulipBanner from './ZulipBanner'; +import { useSelector, useDispatch, useGlobalSelector } from '../react-redux'; +import { getAccount, getSilenceServerPushSetupWarnings } from '../account/accountsSelectors'; +import { dismissServerNotifsExpiringBanner } from '../account/accountActions'; +import { + kPushNotificationsEnabledEndDoc, + pushNotificationsEnabledEndTimestampWarning, +} from '../settings/NotifTroubleshootingScreen'; +import { useDateRefreshedAtInterval } from '../reactUtils'; +import { openLinkWithUserPreference } from '../utils/openLink'; +import { getGlobalSettings } from '../directSelectors'; + +type Props = $ReadOnly<{||}>; + +/** + * A "nag banner" saying the server will soon disable notifications, if so. + * + * Offers a dismiss button. If this notice is dismissed, it sleeps for a + * week, then reappears if the warning still applies. + */ +export default function ServerNotifsExpiringBanner(props: Props): Node { + const dispatch = useDispatch(); + + const globalSettings = useGlobalSelector(getGlobalSettings); + + const lastDismissedServerNotifsExpiringBanner = useSelector( + state => getAccount(state).lastDismissedServerNotifsExpiringBanner, + ); + + const perAccountState = useSelector(state => state); + const dateNow = useDateRefreshedAtInterval(60_000); + + const expiryWarning = pushNotificationsEnabledEndTimestampWarning(perAccountState, dateNow); + + const silenceServerPushSetupWarnings = useSelector(getSilenceServerPushSetupWarnings); + + let visible = false; + let text = ''; + if (expiryWarning == null) { + // don't show + } else if (silenceServerPushSetupWarnings) { + // don't show + } else if ( + lastDismissedServerNotifsExpiringBanner !== null + && lastDismissedServerNotifsExpiringBanner >= subWeeks(dateNow, 1) + ) { + // don't show + } else { + visible = true; + text = expiryWarning.reactText; + } + + const buttons = []; + buttons.push({ + id: 'dismiss', + label: 'Dismiss', + onPress: () => { + dispatch(dismissServerNotifsExpiringBanner()); + }, + }); + buttons.push({ + id: 'learn-more', + label: 'Learn more', + onPress: () => { + openLinkWithUserPreference(kPushNotificationsEnabledEndDoc, globalSettings); + }, + }); + + return ; +} diff --git a/src/main/HomeScreen.js b/src/main/HomeScreen.js index e5dde7aa72..0872f6fce5 100644 --- a/src/main/HomeScreen.js +++ b/src/main/HomeScreen.js @@ -18,6 +18,7 @@ import LoadingBanner from '../common/LoadingBanner'; import ServerCompatBanner from '../common/ServerCompatBanner'; import ServerNotifsDisabledBanner from '../common/ServerNotifsDisabledBanner'; import { OfflineNoticePlaceholder } from '../boot/OfflineNoticeProvider'; +import ServerNotifsExpiringBanner from '../common/ServerNotifsExpiringBanner'; const styles = createStyleSheet({ wrapper: { @@ -71,6 +72,7 @@ export default function HomeScreen(props: Props): Node { + diff --git a/src/storage/__tests__/migrations-test.js b/src/storage/__tests__/migrations-test.js index 5e545092c8..086708f508 100644 --- a/src/storage/__tests__/migrations-test.js +++ b/src/storage/__tests__/migrations-test.js @@ -6,6 +6,7 @@ import { storeKeys } from '../../boot/store'; import { createMigrationFunction } from '../../redux-persist-migrate'; import { ZulipVersion } from '../../utils/zulipVersion'; import * as eg from '../../__tests__/lib/exampleData'; +import type { UserId } from '../../api/idTypes'; describe('historicalStoreKeys', () => { test('equals current storeKeys', () => { @@ -106,13 +107,33 @@ describe('migrations', () => { const base62 = { ...base52, migrations: { version: 62 }, - accounts: base52.accounts.map(a => ({ ...a, silenceServerPushSetupWarnings: false })), + accounts: (base52.accounts.map(a => ({ + ...a, + silenceServerPushSetupWarnings: false, + })): $ReadOnlyArray<{| + +email: string, + +apiKey: string, + +realm: URL, + +ackedPushToken: string | null, + +zulipFeatureLevel: number | null, + +zulipVersion: ZulipVersion | null, + +lastDismissedServerPushSetupNotice: Date | null, + +userId: UserId | null, + +silenceServerPushSetupWarnings: boolean, + |}>), + }; + + // What `base` becomes after migrations up through 66. + const base66 = { + ...base62, + migrations: { version: 66 }, + accounts: base62.accounts.map(a => ({ ...a, lastDismissedServerNotifsExpiringBanner: null })), }; // What `base` becomes after all migrations. const endBase = { - ...base62, - migrations: { version: 65 }, + ...base66, + migrations: { version: 66 }, }; for (const [desc, before, after] of [ diff --git a/src/storage/migrations.js b/src/storage/migrations.js index cec93ff111..915796280c 100644 --- a/src/storage/migrations.js +++ b/src/storage/migrations.js @@ -535,6 +535,12 @@ const migrationsInner: {| [string]: (LessPartialState) => LessPartialState |} = // Add pushNotificationsEnabledEndTimestamp to state.realm '65': dropCache, + // Add `accounts[].lastDismissedServerNotifsExpiringBanner`, as Date | null. + '66': state => ({ + ...state, + accounts: state.accounts.map(a => ({ ...a, lastDismissedServerNotifsExpiringBanner: null })), + }), + // TIP: When adding a migration, consider just using `dropCache`. // (See its jsdoc for guidance on when that's the right answer.) }; diff --git a/src/types.js b/src/types.js index 80a9aae43f..94dcd37d20 100644 --- a/src/types.js +++ b/src/types.js @@ -143,17 +143,26 @@ export type Account = $ReadOnly<{| */ lastDismissedServerPushSetupNotice: Date | null, + /** + * When the user last dismissed the server-notifs-soon-to-expire notice. + * + * `null` when the user hasn't dismissed this notice. + */ + lastDismissedServerNotifsExpiringBanner: Date | null, + /** * The setting to silence prominent warnings about disabled notifications. * * ("Disabled" meaning in particular that the realm hasn't enabled - * notifications, i.e., RealmState.pushNotificationsEnabled is false.) + * notifications, i.e., RealmState.pushNotificationsEnabled is false; + * or that the realm is expected to disable them soon, i.e., + * RealmState.pushNotificationsEnabledEndTimestamp is approaching.) * * Users will set this if they want something more permanent than the - * ServerNotifsDisabledBanner's "Dismiss" button. That button only snoozes the - * banner (for two weeks, as of writing), but this setting makes the - * banner never appear. (The banner's information will still be available - * on the "Notifications" screen.) + * "Dismiss" button on the ServerNotifsDisabledBanner or the + * ServerNotifsExpiringBanner. That button only snoozes the banner, but + * this setting makes the banner never appear. (The banner's information + * will still be available on the "Notifications" screen.) * * Defaults to off / `false`. */