From c0633652e8fd04fcdbc06d9be6bc3a0e5d4d4d91 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 4 May 2021 13:49:11 -0700 Subject: [PATCH] Fix notification handling Summary: Changelog: Fixed application crash notifications not opening the crash log Crashlog notifications were quite blatantly broken: 1. Because I force all GK's to true locally during testing, and the feature gate is negative, I never had any crash reports in the first place. The GK was empty so removed it. 2. The notifications overview would always cause a NPE when the notification came from a device plugin (it assumed a client to be present). This made the button do nothing and the device name not show up near the notification 3. The OS level notifications would link to the notification screen, which was a static screen in the classic UI and would always render to an empty page in Sandy. Removed that screen from the code base. Instead the click the notification will now trigger the notification action. Reviewed By: passy Differential Revision: D28119719 fbshipit-source-id: 5b28dd854260fd479d09e3ee6206301cc669ab40 --- desktop/app/src/chrome/NotificationScreen.tsx | 104 ------------------ desktop/app/src/dispatcher/notifications.tsx | 16 +-- .../notification/Notification.tsx | 81 +++++++++----- 3 files changed, 53 insertions(+), 148 deletions(-) delete mode 100644 desktop/app/src/chrome/NotificationScreen.tsx diff --git a/desktop/app/src/chrome/NotificationScreen.tsx b/desktop/app/src/chrome/NotificationScreen.tsx deleted file mode 100644 index b93db5a21..000000000 --- a/desktop/app/src/chrome/NotificationScreen.tsx +++ /dev/null @@ -1,104 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @format - */ - -import {PureComponent, Fragment} from 'react'; -import {Logger} from '../fb-interfaces/Logger'; -import {connect} from 'react-redux'; -import {State as Store} from '../reducers/index'; -import {ConnectedNotificationsTable} from '../NotificationsHub'; -import {Button, colors, styled, FlexColumn} from '../ui'; -import {clearAllNotifications} from '../reducers/notifications'; -import {selectPlugin} from '../reducers/connections'; -import React from 'react'; - -type StateFromProps = { - deepLinkPayload: unknown; - blocklistedPlugins: Array; - blocklistedCategories: Array; -}; - -type DispatchFromProps = { - clearAllNotifications: () => void; - selectPlugin: (payload: { - selectedPlugin: string | null; - selectedApp: string | null | undefined; - deepLinkPayload: unknown; - }) => any; -}; - -type OwnProps = { - logger: Logger; -}; - -type Props = StateFromProps & DispatchFromProps & OwnProps; - -type State = {}; - -const Container = styled(FlexColumn)({ - width: 0, - flexGrow: 1, - flexShrink: 1, - backgroundColor: colors.white, -}); - -class Notifications extends PureComponent { - render() { - const { - blocklistedPlugins, - blocklistedCategories, - deepLinkPayload, - logger, - clearAllNotifications, - selectPlugin, - } = this.props; - return ( - - - ({ - value, - type: 'exclude', - key: 'plugin', - })), - ...blocklistedCategories.map((value) => ({ - value, - type: 'exclude', - key: 'category', - })), - ]} - actions={ - - - - } - /> - - - ); - } -} - -export default connect( - ({ - connections: {deepLinkPayload}, - notifications: {blocklistedPlugins, blocklistedCategories}, - }) => ({ - deepLinkPayload, - blocklistedPlugins, - blocklistedCategories, - }), - {clearAllNotifications, selectPlugin}, -)(Notifications); diff --git a/desktop/app/src/dispatcher/notifications.tsx b/desktop/app/src/dispatcher/notifications.tsx index a536b2253..3f5be4d05 100644 --- a/desktop/app/src/dispatcher/notifications.tsx +++ b/desktop/app/src/dispatcher/notifications.tsx @@ -11,7 +11,6 @@ import {Store} from '../reducers/index'; import {Logger} from '../fb-interfaces/Logger'; import {PluginNotification} from '../reducers/notifications'; import {PluginDefinition} from '../plugin'; -import {setStaticView} from '../reducers/connections'; import {ipcRenderer, IpcRendererEvent} from 'electron'; import { setActiveNotifications, @@ -19,20 +18,15 @@ import { updateCategoryBlocklist, } from '../reducers/notifications'; import {textContent} from '../utils/index'; -import GK from '../fb-stubs/GK'; import {deconstructPluginKey} from '../utils/clientUtils'; -import NotificationScreen from '../chrome/NotificationScreen'; import {getPluginTitle, isSandyPlugin} from '../utils/pluginUtils'; import {sideEffect} from '../utils/sideEffect'; +import {openNotification} from '../sandy-chrome/notification/Notification'; type NotificationEvents = 'show' | 'click' | 'close' | 'reply' | 'action'; const NOTIFICATION_THROTTLE = 5 * 1000; // in milliseconds export default (store: Store, logger: Logger) => { - if (GK.get('flipper_disable_notifications')) { - return; - } - const knownNotifications: Set = new Set(); const knownPluginStates: Map = new Map(); const lastNotificationTime: Map = new Map(); @@ -46,12 +40,7 @@ export default (store: Store, logger: Logger) => { arg: null | string | number, ) => { if (eventName === 'click' || (eventName === 'action' && arg === 0)) { - store.dispatch( - setStaticView( - NotificationScreen, - pluginNotification.notification.action ?? null, - ), - ); + openNotification(store, pluginNotification); } else if (eventName === 'action') { if (arg === 1 && pluginNotification.notification.category) { // Hide similar (category) @@ -114,7 +103,6 @@ export default (store: Store, logger: Logger) => { const persistingPlugin: undefined | PluginDefinition = getPlugin( pluginName, ); - // TODO: add support for Sandy plugins T68683442 if ( persistingPlugin && !isSandyPlugin(persistingPlugin) && diff --git a/desktop/app/src/sandy-chrome/notification/Notification.tsx b/desktop/app/src/sandy-chrome/notification/Notification.tsx index 03e8e79c5..9cbe10444 100644 --- a/desktop/app/src/sandy-chrome/notification/Notification.tsx +++ b/desktop/app/src/sandy-chrome/notification/Notification.tsx @@ -20,18 +20,18 @@ import { EllipsisOutlined, } from '@ant-design/icons'; import {LeftSidebar, SidebarTitle} from '../LeftSidebar'; -import {useStore, useDispatch} from '../../utils/useStore'; -import {ClientQuery} from '../../Client'; -import {deconstructClientId} from '../../utils/clientUtils'; +import {useDispatch, useStore} from '../../utils/useStore'; import {selectPlugin} from '../../reducers/connections'; import { clearAllNotifications, + PluginNotification as PluginNotificationOrig, updateCategoryBlocklist, updatePluginBlocklist, } from '../../reducers/notifications'; import {filterNotifications} from './notificationUtils'; import {useMemoize} from 'flipper-plugin'; import BlocklistSettingButton from './BlocklistSettingButton'; +import {Store} from '../../reducers'; type NotificationExtra = { onOpen: () => void; @@ -188,23 +188,11 @@ function NotificationList({ } export function Notification() { + const store = useStore(); const dispatch = useDispatch(); const [searchString, setSearchString] = useState(''); - const clients = useStore((state) => state.connections.clients); - const getClientQuery = useCallback( - (id: string | null) => - id !== null - ? clients.reduce( - (query: ClientQuery | null, client) => - client.id === id ? client.query : query, - null, - ) ?? deconstructClientId(id) - : null, - [clients], - ); - const clientPlugins = useStore((state) => state.plugins.clientPlugins); const devicePlugins = useStore((state) => state.plugins.devicePlugins); const getPlugin = useCallback( @@ -222,21 +210,19 @@ export function Notification() { const displayedNotifications: Array = useMemo( () => activeNotifications.map((noti) => { + const client = getClientById(store, noti.client); + const device = client + ? client.deviceSync + : getDeviceById(store, noti.client); const plugin = getPlugin(noti.pluginId); - const client = getClientQuery(noti.client); return { ...noti.notification, - onOpen: () => - dispatch( - selectPlugin({ - selectedPlugin: noti.pluginId, - selectedApp: noti.client, - deepLinkPayload: noti.notification.action, - }), - ), + onOpen: () => { + openNotification(store, noti); + }, onHideSimilar: noti.notification.category ? () => - dispatch( + store.dispatch( updateCategoryBlocklist([ ...notifications.blocklistedCategories, noti.notification.category!, @@ -244,19 +230,19 @@ export function Notification() { ) : null, onHidePlugin: () => - dispatch( + store.dispatch( updatePluginBlocklist([ ...notifications.blocklistedPlugins, noti.pluginId, ]), ), - clientName: client?.device_id, - appName: client?.app, + clientName: client?.query.device_id ?? device?.displayTitle(), + appName: client?.query.app, pluginName: plugin?.title ?? noti.pluginId, iconName: plugin?.icon, }; }), - [activeNotifications, notifications, getPlugin, getClientQuery, dispatch], + [activeNotifications, notifications, getPlugin, store], ); const actions = ( @@ -312,3 +298,38 @@ export function Notification() { ); } + +export function openNotification(store: Store, noti: PluginNotificationOrig) { + const client = getClientById(store, noti.client); + if (client) { + store.dispatch( + selectPlugin({ + selectedPlugin: noti.pluginId, + selectedApp: noti.client, + selectedDevice: client.deviceSync, + deepLinkPayload: noti.notification.action, + }), + ); + } else { + const device = getDeviceById(store, noti.client); + if (device) { + store.dispatch( + selectPlugin({ + selectedPlugin: noti.pluginId, + selectedDevice: device, + deepLinkPayload: noti.notification.action, + }), + ); + } + } +} + +function getClientById(store: Store, identifier: string | null) { + return store.getState().connections.clients.find((c) => c.id === identifier); +} + +function getDeviceById(store: Store, identifier: string | null) { + return store + .getState() + .connections.devices.find((c) => c.serial === identifier); +}