From b4151ce37b8b5b624fcf5ca953c14dc54322f77c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 13 Dec 2023 21:33:13 -0800 Subject: [PATCH] notifs: Add "silence warnings" setting for server-setup banner Fixes: #5785 --- src/__tests__/lib/exampleData.js | 2 ++ src/account/AccountItem.js | 8 +++++-- src/account/AccountPickScreen.js | 5 ++++- src/account/accountActions.js | 6 +++++ src/account/accountsReducer.js | 9 ++++++++ src/account/accountsSelectors.js | 3 +++ src/actionConstants.js | 2 ++ src/actionTypes.js | 8 +++++++ src/common/ServerPushSetupBanner.js | 5 ++++- src/settings/NotifTroubleshootingScreen.js | 12 ++++++++-- .../PerAccountNotificationSettingsGroup.js | 21 ++++++++++++++++-- src/storage/__tests__/migrations-test.js | 8 ++++--- src/storage/migrations.js | 6 +++++ src/types.js | 22 +++++++++++++++++++ static/translations/messages_en.json | 1 + 15 files changed, 107 insertions(+), 11 deletions(-) diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js index abc22401009..675c3392cef 100644 --- a/src/__tests__/lib/exampleData.js +++ b/src/__tests__/lib/exampleData.js @@ -248,6 +248,7 @@ export const makeAccount = ( zulipVersion: zulipVersionInner, ackedPushToken, lastDismissedServerPushSetupNotice, + silenceServerPushSetupWarnings: null, }); }; @@ -648,6 +649,7 @@ export const plusReduxState: GlobalState & PerAccountState = reduxState({ zulipVersion: recentZulipVersion, zulipFeatureLevel: recentZulipFeatureLevel, lastDismissedServerPushSetupNotice: null, + silenceServerPushSetupWarnings: false, }, ], realm: { diff --git a/src/account/AccountItem.js b/src/account/AccountItem.js index f41cf378be1..26173a6d269 100644 --- a/src/account/AccountItem.js +++ b/src/account/AccountItem.js @@ -62,7 +62,8 @@ type Props = $ReadOnly<{| |}>; export default function AccountItem(props: Props): Node { - const { email, realm, isLoggedIn, notificationReport } = props.account; + const { email, realm, isLoggedIn, notificationReport, silenceServerPushSetupWarnings } = + props.account; const _ = React.useContext(TranslationContext); const navigation = useNavigation(); @@ -91,7 +92,10 @@ export default function AccountItem(props: Props): Node { isActiveAccount && activeAccountState != null && getHaveServerData(activeAccountState) ? getRealmName(activeAccountState) : '(unknown organization name)'; - const singleNotifProblem = chooseNotifProblemForShortText({ report: notificationReport }); + const singleNotifProblem = chooseNotifProblemForShortText({ + report: notificationReport, + ignoreServerHasNotEnabled: silenceServerPushSetupWarnings ?? false, + }); const handlePressNotificationWarning = React.useCallback(() => { if (singleNotifProblem == null) { diff --git a/src/account/AccountPickScreen.js b/src/account/AccountPickScreen.js index 86da36f4cc9..adee196ce67 100644 --- a/src/account/AccountPickScreen.js +++ b/src/account/AccountPickScreen.js @@ -34,6 +34,8 @@ export type AccountStatus = {| // list of problems that are likely to prevent notifications from working. // We'll use it to warn on each account item that has problems. +notificationReport: { +problems: NotificationReport['problems'], ... }, + + +silenceServerPushSetupWarnings: boolean | null, |}; /** @@ -45,7 +47,7 @@ function useAccountStatuses(): $ReadOnlyArray { const accounts = useGlobalSelector(getAccounts); const notificationReportsByIdentityKey = useNotificationReportsByIdentityKey(); - return accounts.map(({ realm, email, apiKey }) => { + return accounts.map(({ realm, email, apiKey, silenceServerPushSetupWarnings }) => { const notificationReport = notificationReportsByIdentityKey.get( keyOfIdentity({ realm, email }), ); @@ -56,6 +58,7 @@ function useAccountStatuses(): $ReadOnlyArray { email, isLoggedIn: apiKey !== '', notificationReport, + silenceServerPushSetupWarnings, }; }); } diff --git a/src/account/accountActions.js b/src/account/accountActions.js index 5a88c3cdf2f..b88d32f43ad 100644 --- a/src/account/accountActions.js +++ b/src/account/accountActions.js @@ -6,6 +6,7 @@ import { ACCOUNT_REMOVE, LOGIN_SUCCESS, DISMISS_SERVER_PUSH_SETUP_NOTICE, + SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, } from '../actionConstants'; import { registerAndStartPolling } from '../events/eventActions'; import { resetToMainTabs } from '../nav/navActions'; @@ -22,6 +23,11 @@ export const dismissServerPushSetupNotice = (): PerAccountAction => ({ date: new Date(), }); +export const setSilenceServerPushSetupWarnings = (value: boolean): PerAccountAction => ({ + type: SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, + value, +}); + const accountSwitchPlain = (identity: Identity): AllAccountsAction => ({ type: ACCOUNT_SWITCH, identity, diff --git a/src/account/accountsReducer.js b/src/account/accountsReducer.js index 7feeb42a491..dd47275a1e5 100644 --- a/src/account/accountsReducer.js +++ b/src/account/accountsReducer.js @@ -11,6 +11,7 @@ import { LOGOUT, DISMISS_SERVER_PUSH_SETUP_NOTICE, ACCOUNT_REMOVE, + SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, } from '../actionConstants'; import { EventTypes } from '../api/eventTypes'; import type { AccountsState, Identity, Action, Account } from '../types'; @@ -49,6 +50,9 @@ export default (state: AccountsState = initialState, action: Action): AccountsSt lastDismissedServerPushSetupNotice: action.data.realm_push_notifications_enabled ? null : state[0].lastDismissedServerPushSetupNotice, + silenceServerPushSetupWarnings: action.data.realm_push_notifications_enabled + ? null + : state[0].silenceServerPushSetupWarnings ?? false, }); case ACCOUNT_SWITCH: { @@ -77,6 +81,7 @@ export default (state: AccountsState = initialState, action: Action): AccountsSt zulipVersion: null, zulipFeatureLevel: null, lastDismissedServerPushSetupNotice: null, + silenceServerPushSetupWarnings: null, }, ...state, ]; @@ -126,6 +131,10 @@ export default (state: AccountsState = initialState, action: Action): AccountsSt return updateActiveAccount(state, { lastDismissedServerPushSetupNotice: action.date }); } + case SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS: { + return updateActiveAccount(state, { silenceServerPushSetupWarnings: action.value }); + } + case ACCOUNT_REMOVE: { const shouldRemove = a => keyOfIdentity(identityOfAccount(a)) === keyOfIdentity(action.identity); diff --git a/src/account/accountsSelectors.js b/src/account/accountsSelectors.js index 186a0cb45ad..f3be04ab3f6 100644 --- a/src/account/accountsSelectors.js +++ b/src/account/accountsSelectors.js @@ -159,6 +159,9 @@ export const getZulipFeatureLevel = (state: PerAccountState): number => { return zulipFeatureLevel; }; +export const getSilenceServerPushSetupWarnings = (state: PerAccountState): boolean | null => + getAccount(state).silenceServerPushSetupWarnings; + /** * The auth object for this account, if logged in; else undefined. * diff --git a/src/actionConstants.js b/src/actionConstants.js index 58631ce7204..5c354fcd6cf 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 SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS: 'SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS' = + 'SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS'; export const INIT_TOPICS: 'INIT_TOPICS' = 'INIT_TOPICS'; diff --git a/src/actionTypes.js b/src/actionTypes.js index fb59b663f0e..ca767b492ac 100644 --- a/src/actionTypes.js +++ b/src/actionTypes.js @@ -60,6 +60,7 @@ import { DISMISS_SERVER_COMPAT_NOTICE, REGISTER_PUSH_TOKEN_START, REGISTER_PUSH_TOKEN_END, + SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, } from './actionConstants'; import type { UserMessageFlag } from './api/modelTypes'; @@ -176,6 +177,11 @@ type DismissServerPushSetupNoticeAction = $ReadOnly<{| date: Date, |}>; +type SetSilenceServerPushSetupWarningsAction = $ReadOnly<{| + type: typeof SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS, + value: boolean, +|}>; + /** We learned the device token from the system. See `SessionState`. */ type GotPushTokenAction = $ReadOnly<{| type: typeof GOT_PUSH_TOKEN, @@ -671,6 +677,7 @@ export type PerAccountAction = // state.session | DismissServerCompatNoticeAction | DismissServerPushSetupNoticeAction + | SetSilenceServerPushSetupWarningsAction | ToggleOutboxSendingAction ; @@ -797,6 +804,7 @@ export function isPerAccountApplicableAction(action: Action): boolean { case CLEAR_TYPING: case DISMISS_SERVER_COMPAT_NOTICE: case DISMISS_SERVER_PUSH_SETUP_NOTICE: + case SET_SILENCE_SERVER_PUSH_SETUP_WARNINGS: case TOGGLE_OUTBOX_SENDING: (action: PerAccountAction); (action: PerAccountApplicableAction); diff --git a/src/common/ServerPushSetupBanner.js b/src/common/ServerPushSetupBanner.js index ddae8f36653..5e3c466ce99 100644 --- a/src/common/ServerPushSetupBanner.js +++ b/src/common/ServerPushSetupBanner.js @@ -6,7 +6,7 @@ import subWeeks from 'date-fns/subWeeks'; import ZulipBanner from './ZulipBanner'; import { useSelector, useDispatch } from '../react-redux'; -import { getAccount } from '../account/accountsSelectors'; +import { getAccount, getSilenceServerPushSetupWarnings } from '../account/accountsSelectors'; import { getRealm } from '../directSelectors'; import { getRealmName } from '../selectors'; import { dismissServerPushSetupNotice } from '../account/accountActions'; @@ -38,12 +38,15 @@ export default function ServerPushSetupBanner(props: Props): Node { state => getAccount(state).lastDismissedServerPushSetupNotice, ); const pushNotificationsEnabled = useSelector(state => getRealm(state).pushNotificationsEnabled); + const silenceServerPushSetupWarnings = useSelector(getSilenceServerPushSetupWarnings); const realmName = useSelector(getRealmName); let visible = false; let text = ''; if (pushNotificationsEnabled) { // don't show + } else if (silenceServerPushSetupWarnings === true) { + // don't show } else if ( lastDismissedServerPushSetupNotice !== null // TODO: Could rerender this component on an interval, to give an diff --git a/src/settings/NotifTroubleshootingScreen.js b/src/settings/NotifTroubleshootingScreen.js index 9111fdafe9e..f09585b1025 100644 --- a/src/settings/NotifTroubleshootingScreen.js +++ b/src/settings/NotifTroubleshootingScreen.js @@ -160,9 +160,17 @@ invariant( export const chooseNotifProblemForShortText = (args: {| // eslint-disable-next-line no-use-before-define report: { +problems: NotificationReport['problems'], ... }, + ignoreServerHasNotEnabled?: boolean, |}): NotificationProblem | null => { - const { report } = args; - return notifProblemsHighToLowShortTextPrecedence.find(p => report.problems.includes(p)) ?? null; + const { report, ignoreServerHasNotEnabled = false } = args; + const result = notifProblemsHighToLowShortTextPrecedence.find(p => { + if (p === NotificationProblem.ServerHasNotEnabled && ignoreServerHasNotEnabled) { + return false; + } + return report.problems.includes(p); + }); + + return result ?? null; }; /** diff --git a/src/settings/PerAccountNotificationSettingsGroup.js b/src/settings/PerAccountNotificationSettingsGroup.js index cd06cd933ab..7ae0f2e7075 100644 --- a/src/settings/PerAccountNotificationSettingsGroup.js +++ b/src/settings/PerAccountNotificationSettingsGroup.js @@ -5,7 +5,7 @@ import type { Node } from 'react'; import invariant from 'invariant'; import type { AppNavigationMethods } from '../nav/AppNavigator'; -import { useGlobalSelector, useSelector } from '../react-redux'; +import { useDispatch, useGlobalSelector, useSelector } from '../react-redux'; import { getAuth, getSettings } from '../selectors'; import SwitchRow from '../common/SwitchRow'; import * as api from '../api'; @@ -13,7 +13,11 @@ import NavRow from '../common/NavRow'; import TextRow from '../common/TextRow'; import { IconAlertTriangle } from '../common/Icons'; import { kWarningColor } from '../styles/constants'; -import { getIdentity, getZulipFeatureLevel } from '../account/accountsSelectors'; +import { + getIdentity, + getSilenceServerPushSetupWarnings, + getZulipFeatureLevel, +} from '../account/accountsSelectors'; import { getGlobalSession, getGlobalSettings, getRealm, getRealmName } from '../directSelectors'; import ZulipText from '../common/ZulipText'; import RowGroup from '../common/RowGroup'; @@ -29,6 +33,7 @@ import { ApiError } from '../api/apiErrors'; import { showErrorAlert } from '../utils/info'; import * as logging from '../utils/logging'; import { TranslationContext } from '../boot/TranslationProvider'; +import { setSilenceServerPushSetupWarnings } from '../account/accountActions'; type Props = $ReadOnly<{| navigation: AppNavigationMethods, @@ -40,6 +45,7 @@ type Props = $ReadOnly<{| export default function PerAccountNotificationSettingsGroup(props: Props): Node { const { navigation } = props; + const dispatch = useDispatch(); const _ = React.useContext(TranslationContext); const auth = useSelector(getAuth); @@ -53,6 +59,7 @@ export default function PerAccountNotificationSettingsGroup(props: Props): Node const realmName = useSelector(getRealmName); const zulipFeatureLevel = useSelector(getZulipFeatureLevel); const pushNotificationsEnabled = useSelector(state => getRealm(state).pushNotificationsEnabled); + const silenceServerPushSetupWarnings = useSelector(getSilenceServerPushSetupWarnings); const offlineNotification = useSelector(state => getSettings(state).offlineNotification); const onlineNotification = useSelector(state => getSettings(state).onlineNotification); const streamNotification = useSelector(state => getSettings(state).streamNotification); @@ -61,6 +68,10 @@ export default function PerAccountNotificationSettingsGroup(props: Props): Node const pushToken = useGlobalSelector(state => getGlobalSession(state).pushToken); + const handleSilenceWarningsChange = React.useCallback(() => { + dispatch(setSilenceServerPushSetupWarnings(!silenceServerPushSetupWarnings)); + }, [dispatch, silenceServerPushSetupWarnings]); + // Helper variable so that we can refer to the button correctly in // other UI text. const troubleshootingPageTitle = 'Troubleshooting'; @@ -207,6 +218,12 @@ export default function PerAccountNotificationSettingsGroup(props: Props): Node ); }} />, + , ]; return ( diff --git a/src/storage/__tests__/migrations-test.js b/src/storage/__tests__/migrations-test.js index dc8dd963eff..917cd95ae5e 100644 --- a/src/storage/__tests__/migrations-test.js +++ b/src/storage/__tests__/migrations-test.js @@ -105,7 +105,8 @@ describe('migrations', () => { // What `base` becomes after all migrations. const endBase = { ...base52, - migrations: { version: 60 }, + migrations: { version: 61 }, + accounts: base52.accounts.map(a => ({ ...a, silenceServerPushSetupWarnings: null })), }; for (const [desc, before, after] of [ @@ -282,7 +283,7 @@ describe('migrations', () => { [ 'check 57 with an `undefined` in state.accounts', { ...base52, migrations: { version: 56 }, accounts: [...base37.accounts, undefined] }, - { ...endBase, accounts: [...base37.accounts] }, + { ...endBase, accounts: [...endBase.accounts] }, ], [ 'check 58 with a malformed Account in state.accounts', @@ -299,8 +300,9 @@ describe('migrations', () => { ...base37.accounts, ], }, - { ...endBase, accounts: [...base37.accounts] }, + { ...endBase, accounts: [...endBase.accounts] }, ], + // 61 covered by whole ]) { test(desc, async () => { // $FlowIgnore[incompatible-exact] diff --git a/src/storage/migrations.js b/src/storage/migrations.js index c0af14ff851..05fbf82387c 100644 --- a/src/storage/migrations.js +++ b/src/storage/migrations.js @@ -517,6 +517,12 @@ const migrationsInner: {| [string]: (LessPartialState) => LessPartialState |} = // Discard invalid enum values from `state.mute`. '60': dropCache, + // Add silenceServerPushSetupWarnings to accounts. + '61': state => ({ + ...state, + accounts: state.accounts.map(a => ({ ...a, silenceServerPushSetupWarnings: 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 8c08f70d151..a7e886a2921 100644 --- a/src/types.js +++ b/src/types.js @@ -142,6 +142,28 @@ export type Account = $ReadOnly<{| * apply. */ lastDismissedServerPushSetupNotice: 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.) + * + * Users will set this if they want something more permanent than the + * ServerPushSetupBanner'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.) + * + * Will be `null` when notifications are enabled for the realm, or when + * we've never had server data for this account. Otherwise false, unless + * the user has switched it to true, which they can do until the realm + * enables notifications. (If true when the realm enables notifications, + * we intentionally forget the `true` by setting this to `null`. That's to + * make sure it's false if notifications are disabled later, because that + * disabling might be a surprise that the user wants to know about.) + */ + silenceServerPushSetupWarnings: boolean | null, |}>; /** diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index b5a8362a891..12d2b46d406 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -255,6 +255,7 @@ "Recipients": "Recipients", "Delete message": "Delete message", "System settings for Zulip": "System settings for Zulip", + "Silence warnings about disabled mobile push notifications": "Silence warnings about disabled mobile push notifications", "Troubleshooting": "Troubleshooting", "Send a test notification": "Send a test notification", "Failed to send test notification": "Failed to send test notification",