From c9a34d3cc2b8f142fdb51f5227dd8d7db56ac0c6 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 4 Oct 2021 07:26:11 -0700 Subject: [PATCH] Fix inconsistent handling of app id and name Summary: Changelog: Improved plugin / device / app selection handing. During some refactorings I discovered that the `connetions.selectedApp` field contained sometimes an application id, and sometimes just the name. This caused inconsistent behavior especially in unit tests. I've cleaned that up, and renamed it to `selectedAppId` where applicable, to make the distinction more clear. And, in contrast, userPreferredApp now always has a name, not an id. During refactoring our existing selection update logic was quite in the way, which was overcomplicated still, since during the sandy chrome migration, the reducers needed to be able to handle both the old UI, and the new application selection UI. That logic has been simplified now, and a lot of tests were added. As a further simplification the preferredApp/Device/Plugin are now only read and used when updating selection, but not when running selectors. Reviewed By: timur-valiev Differential Revision: D31305180 fbshipit-source-id: 2dbd9f9c33950227cc63aa29cc4a98bdd0db8e7a --- desktop/app/src/NotificationsHub.tsx | 14 +- desktop/app/src/PluginContainer.tsx | 6 +- .../src/__tests__/PluginContainer.node.tsx | 49 +++- .../createMockFlipperWithPlugin.node.tsx.snap | 4 +- desktop/app/src/deeplink.tsx | 25 +- .../handleOpenPluginDeeplink.node.tsx | 14 +- desktop/app/src/dispatcher/flipperServer.tsx | 7 +- .../dispatcher/handleOpenPluginDeeplink.tsx | 4 +- desktop/app/src/dispatcher/pluginManager.tsx | 12 +- desktop/app/src/dispatcher/tracking.tsx | 18 +- desktop/app/src/plugin.tsx | 2 +- .../reducers/__tests__/connections.node.tsx | 247 +++++++++++++++- .../__tests__/sandydeviceplugins.node.tsx | 2 +- desktop/app/src/reducers/connections.tsx | 266 +++++++----------- desktop/app/src/reducers/notifications.tsx | 4 +- desktop/app/src/reducers/supportForm.tsx | 4 +- .../sandy-chrome/appinspect/AppSelector.tsx | 19 +- .../appinspect/BookmarkSection.tsx | 6 +- .../sandy-chrome/appinspect/PluginList.tsx | 8 +- .../appinspect/__tests__/PluginList.spec.tsx | 9 +- .../notification/Notification.tsx | 2 +- desktop/app/src/selectors/connections.tsx | 18 +- .../src/server/devices/metro/MetroDevice.tsx | 1 + .../createMockFlipperWithPlugin.tsx | 6 +- .../src/utils/__tests__/exportData.node.tsx | 6 +- desktop/app/src/utils/__tests__/info.node.tsx | 2 +- .../__tests__/messageQueueSandy.node.tsx | 6 +- desktop/app/src/utils/exportData.tsx | 8 +- .../src/utils/flipperLibImplementation.tsx | 2 +- desktop/app/src/utils/info.tsx | 6 +- 30 files changed, 497 insertions(+), 280 deletions(-) diff --git a/desktop/app/src/NotificationsHub.tsx b/desktop/app/src/NotificationsHub.tsx index e08acb8b8..c157e5fe7 100644 --- a/desktop/app/src/NotificationsHub.tsx +++ b/desktop/app/src/NotificationsHub.tsx @@ -52,11 +52,7 @@ type StateFromProps = { }; type DispatchFromProps = { - selectPlugin: (payload: { - selectedPlugin: string | null; - selectedApp: string | null; - deepLinkPayload: unknown; - }) => any; + selectPlugin: typeof selectPlugin; updatePluginBlocklist: (blocklist: Array) => any; updateCategoryBlocklist: (blocklist: Array) => any; }; @@ -410,11 +406,7 @@ type ItemProps = { onClear?: () => any; isSelected?: boolean; inactive?: boolean; - selectPlugin?: (payload: { - selectedPlugin: string | null; - selectedApp: string | null; - deepLinkPayload: unknown; - }) => any; + selectPlugin?: typeof selectPlugin; logger?: Logger; plugin: PluginDefinition | null | undefined; }; @@ -477,7 +469,7 @@ class NotificationItem extends Component< if (this.props.selectPlugin && notification.action) { this.props.selectPlugin({ selectedPlugin: pluginId, - selectedApp: client, + selectedAppId: client, deepLinkPayload: notification.action, }); } diff --git a/desktop/app/src/PluginContainer.tsx b/desktop/app/src/PluginContainer.tsx index 6feceb9c8..04abcf747 100644 --- a/desktop/app/src/PluginContainer.tsx +++ b/desktop/app/src/PluginContainer.tsx @@ -101,11 +101,7 @@ type StateFromProps = { }; type DispatchFromProps = { - selectPlugin: (payload: { - selectedPlugin: string | null; - selectedApp?: string | null; - deepLinkPayload: unknown; - }) => any; + selectPlugin: typeof selectPlugin; setStaticView: (payload: StaticView) => void; enablePlugin: typeof switchPlugin; loadPlugin: typeof loadPlugin; diff --git a/desktop/app/src/__tests__/PluginContainer.node.tsx b/desktop/app/src/__tests__/PluginContainer.node.tsx index 7e6ea658d..245a3ba65 100644 --- a/desktop/app/src/__tests__/PluginContainer.node.tsx +++ b/desktop/app/src/__tests__/PluginContainer.node.tsx @@ -249,6 +249,7 @@ test('PluginContainer can render Sandy plugins', async () => { act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, selectedPlugin: 'Logs', deepLinkPayload: null, }), @@ -279,6 +280,7 @@ test('PluginContainer can render Sandy plugins', async () => { act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, selectedPlugin: definition.id, deepLinkPayload: null, }), @@ -405,6 +407,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, + selectedPlugin: 'Logs', deepLinkPayload: null, }), @@ -422,6 +426,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, + selectedPlugin: definition.id, deepLinkPayload: null, }), @@ -454,6 +460,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, + selectedPlugin: 'Logs', deepLinkPayload: null, }), @@ -487,6 +495,7 @@ test('PluginContainer triggers correct lifecycles for background plugin', async act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, selectedPlugin: definition.id, deepLinkPayload: null, }), @@ -566,7 +575,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => { selectPlugin({ selectedPlugin: definition.id, deepLinkPayload: 'universe!', - selectedApp: client.query.app, + selectedAppId: client.id, }), ); }); @@ -607,7 +616,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => { selectPlugin({ selectedPlugin: definition.id, deepLinkPayload: 'universe!', - selectedApp: client.query.app, + selectedAppId: client.id, }), ); }); @@ -630,7 +639,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => { selectPlugin({ selectedPlugin: definition.id, deepLinkPayload: 'london!', - selectedApp: client.query.app, + selectedAppId: client.id, }), ); }); @@ -643,7 +652,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => { selectPlugin({ selectedPlugin: 'Logs', deepLinkPayload: 'london!', - selectedApp: client.query.app, + selectedAppId: client.id, }), ); }); @@ -652,7 +661,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => { selectPlugin({ selectedPlugin: definition.id, deepLinkPayload: 'london!', - selectedApp: client.query.app, + selectedAppId: client.id, }), ); }); @@ -783,6 +792,7 @@ test('PluginContainer can render Sandy device plugins', async () => { act(() => { store.dispatch( selectPlugin({ + selectedDevice: device, selectedPlugin: 'Logs', deepLinkPayload: null, }), @@ -801,6 +811,7 @@ test('PluginContainer can render Sandy device plugins', async () => { act(() => { store.dispatch( selectPlugin({ + selectedDevice: device, selectedPlugin: definition.id, deepLinkPayload: null, }), @@ -836,7 +847,9 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => { }, }, ); - const {renderer, act, store} = await renderMockFlipperWithPlugin(definition); + const {renderer, act, store, device} = await renderMockFlipperWithPlugin( + definition, + ); const theUniverse = { thisIs: 'theUniverse', @@ -877,9 +890,10 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => { act(() => { store.dispatch( selectPlugin({ + selectedDevice: device, selectedPlugin: definition.id, deepLinkPayload: theUniverse, - selectedApp: null, + selectedAppId: null, }), ); }); @@ -918,9 +932,10 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => { act(() => { store.dispatch( selectPlugin({ + selectedDevice: device, selectedPlugin: definition.id, deepLinkPayload: theUniverse, - selectedApp: null, + selectedAppId: null, }), ); }); @@ -941,9 +956,10 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => { act(() => { store.dispatch( selectPlugin({ + selectedDevice: device, selectedPlugin: definition.id, deepLinkPayload: 'london!', - selectedApp: null, + selectedAppId: null, }), ); }); @@ -954,18 +970,20 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => { act(() => { store.dispatch( selectPlugin({ + selectedDevice: device, selectedPlugin: 'Logs', deepLinkPayload: 'london!', - selectedApp: null, + selectedAppId: null, }), ); }); act(() => { store.dispatch( selectPlugin({ + selectedDevice: device, selectedPlugin: definition.id, deepLinkPayload: 'london!', - selectedApp: null, + selectedAppId: null, }), ); }); @@ -1203,6 +1221,7 @@ test('PluginContainer can render Sandy plugins for archived devices', async () = act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, selectedPlugin: 'Logs', deepLinkPayload: null, }), @@ -1225,6 +1244,8 @@ test('PluginContainer can render Sandy plugins for archived devices', async () = act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, + selectedPlugin: definition.id, deepLinkPayload: null, }), @@ -1315,6 +1336,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, + selectedPlugin: 'Logs', deepLinkPayload: null, }), @@ -1331,6 +1354,8 @@ test('PluginContainer triggers correct lifecycles for background plugin', async act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, + selectedPlugin: definition.id, deepLinkPayload: null, }), @@ -1346,6 +1371,7 @@ test('PluginContainer triggers correct lifecycles for background plugin', async act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, selectedPlugin: 'Logs', deepLinkPayload: null, }), @@ -1355,6 +1381,7 @@ test('PluginContainer triggers correct lifecycles for background plugin', async act(() => { store.dispatch( selectPlugin({ + selectedAppId: client.id, selectedPlugin: definition.id, deepLinkPayload: null, }), diff --git a/desktop/app/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap b/desktop/app/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap index b27a29062..65ffa5a67 100644 --- a/desktop/app/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap +++ b/desktop/app/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap @@ -36,7 +36,7 @@ Object { ], }, "flipperServer": undefined, - "selectedApp": "TestApp#Android#MockAndroidDevice#serial", + "selectedAppId": "TestApp#Android#MockAndroidDevice#serial", "selectedAppPluginListRevision": 0, "selectedDevice": Object { "deviceType": "physical", @@ -47,7 +47,7 @@ Object { "selectedPlugin": "TestPlugin", "staticView": null, "uninitializedClients": Array [], - "userPreferredApp": "TestApp#Android#MockAndroidDevice#serial", + "userPreferredApp": "TestApp", "userPreferredDevice": "MockAndroidDevice", "userPreferredPlugin": "TestPlugin", } diff --git a/desktop/app/src/deeplink.tsx b/desktop/app/src/deeplink.tsx index 99e752db2..a079a313d 100644 --- a/desktop/app/src/deeplink.tsx +++ b/desktop/app/src/deeplink.tsx @@ -112,11 +112,32 @@ export async function handleDeeplink( match[1] }&client=${match[0]}&payload=${encodeURIComponent(match[2])}' instead.`, ); + const deepLinkPayload = match[2]; + const deepLinkParams = new URLSearchParams(deepLinkPayload); + const deviceParam = deepLinkParams.get('device'); + + // if there is a device Param, find a matching device + const selectedDevice = deviceParam + ? store + .getState() + .connections.devices.find((v) => v.title === deviceParam) + : undefined; + + // if a client is specified, find it, withing the device if applicable + const selectedClient = store + .getState() + .connections.clients.find( + (c) => + c.query.app === match[0] && + (selectedDevice == null || c.device === selectedDevice), + ); + store.dispatch( selectPlugin({ - selectedApp: match[0], + selectedAppId: selectedClient?.id, + selectedDevice: selectedClient ? selectedClient.device : selectedDevice, selectedPlugin: match[1], - deepLinkPayload: match[2], + deepLinkPayload, }), ); return; diff --git a/desktop/app/src/dispatcher/__tests__/handleOpenPluginDeeplink.node.tsx b/desktop/app/src/dispatcher/__tests__/handleOpenPluginDeeplink.node.tsx index 35694d2aa..e5cc77ffd 100644 --- a/desktop/app/src/dispatcher/__tests__/handleOpenPluginDeeplink.node.tsx +++ b/desktop/app/src/dispatcher/__tests__/handleOpenPluginDeeplink.node.tsx @@ -139,11 +139,15 @@ test('triggering a deeplink without applicable device can wait for a device', as supportedDevices: [{os: 'iOS'}], }, ); - const {renderer, store, logger, createDevice} = + const {renderer, store, logger, createDevice, device} = await renderMockFlipperWithPlugin(definition); store.dispatch( - selectPlugin({selectedPlugin: 'nonexisting', deepLinkPayload: null}), + selectPlugin({ + selectedPlugin: 'nonexisting', + deepLinkPayload: null, + selectedDevice: device, + }), ); expect(renderer.baseElement).toMatchInlineSnapshot(` @@ -219,7 +223,11 @@ test('triggering a deeplink without applicable client can wait for a device', as await renderMockFlipperWithPlugin(definition); store.dispatch( - selectPlugin({selectedPlugin: 'nonexisting', deepLinkPayload: null}), + selectPlugin({ + selectedPlugin: 'nonexisting', + deepLinkPayload: null, + selectedDevice: device, + }), ); expect(renderer.baseElement).toMatchInlineSnapshot(` diff --git a/desktop/app/src/dispatcher/flipperServer.tsx b/desktop/app/src/dispatcher/flipperServer.tsx index e58ce011b..6a4ba54cb 100644 --- a/desktop/app/src/dispatcher/flipperServer.tsx +++ b/desktop/app/src/dispatcher/flipperServer.tsx @@ -173,11 +173,6 @@ export async function handleClientConnected(store: Store, client: Client) { payload: client, }); - const device = client.device; - if (device) { - store.dispatch(selectDevice(device)); - store.dispatch(selectClient(client.id)); - } - + store.dispatch(selectClient(client.id)); client.emit('plugins-change'); } diff --git a/desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx b/desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx index 36268300b..cef60673e 100644 --- a/desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx +++ b/desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx @@ -121,7 +121,7 @@ export async function handleOpenPluginDeeplink(store: Store, query: string) { store.dispatch( selectPlugin({ selectedPlugin: params.pluginId, - selectedApp: null, + selectedAppId: null, selectedDevice: device, deepLinkPayload: params.payload, }), @@ -130,7 +130,7 @@ export async function handleOpenPluginDeeplink(store: Store, query: string) { store.dispatch( selectPlugin({ selectedPlugin: params.pluginId, - selectedApp: client!.query.app, + selectedAppId: client!.id, selectedDevice: device, deepLinkPayload: params.payload, }), diff --git a/desktop/app/src/dispatcher/pluginManager.tsx b/desktop/app/src/dispatcher/pluginManager.tsx index 5cc34574a..a1ac18316 100644 --- a/desktop/app/src/dispatcher/pluginManager.tsx +++ b/desktop/app/src/dispatcher/pluginManager.tsx @@ -166,12 +166,12 @@ function updatePlugin(store: Store, payload: UpdatePluginActionPayload) { } } -function getSelectedAppId(store: Store) { +function getSelectedAppName(store: Store) { const {connections} = store.getState(); - const selectedApp = connections.selectedApp - ? deconstructClientId(connections.selectedApp).app + const selectedAppId = connections.selectedAppId + ? deconstructClientId(connections.selectedAppId).app : undefined; - return selectedApp; + return selectedAppId; } function switchPlugin( @@ -190,7 +190,7 @@ function switchClientPlugin( plugin: PluginDefinition, selectedApp: string | undefined, ) { - selectedApp = selectedApp ?? getSelectedAppId(store); + selectedApp = selectedApp ?? getSelectedAppName(store); if (!selectedApp) { return; } @@ -242,7 +242,7 @@ function updateClientPlugin( ) { const clients = store.getState().connections.clients; if (enable) { - const selectedApp = getSelectedAppId(store); + const selectedApp = getSelectedAppName(store); if (selectedApp) { store.dispatch(setPluginEnabled(plugin.id, selectedApp)); } diff --git a/desktop/app/src/dispatcher/tracking.tsx b/desktop/app/src/dispatcher/tracking.tsx index 0b7b493a0..f82dc5871 100644 --- a/desktop/app/src/dispatcher/tracking.tsx +++ b/desktop/app/src/dispatcher/tracking.tsx @@ -108,12 +108,12 @@ export default (store: Store, logger: Logger) => { timeSinceLastStartup, }); // create fresh exit data - const {selectedDevice, selectedApp, selectedPlugin} = + const {selectedDevice, selectedAppId, selectedPlugin} = store.getState().connections; persistExitData( { selectedDevice, - selectedApp, + selectedAppId, selectedPlugin, }, false, @@ -158,11 +158,11 @@ export default (store: Store, logger: Logger) => { ); return; } - const {selectedDevice, selectedPlugin, selectedApp, clients} = + const {selectedDevice, selectedPlugin, selectedAppId, clients} = state.connections; persistExitData( - {selectedDevice, selectedPlugin, selectedApp}, + {selectedDevice, selectedPlugin, selectedAppId}, args[0] === 'exit', ); @@ -224,8 +224,8 @@ export default (store: Store, logger: Logger) => { let app: string | null = null; let sdkVersion: number | null = null; - if (selectedApp) { - const client = clients.find((c: Client) => c.id === selectedApp); + if (selectedAppId) { + const client = clients.find((c: Client) => c.id === selectedAppId); if (client) { app = client.query.app; sdkVersion = client.query.sdk_version || 0; @@ -357,7 +357,7 @@ export function persistExitData( state: { selectedDevice: BaseDevice | null; selectedPlugin: string | null; - selectedApp: string | null; + selectedAppId: string | null; }, cleanExit: boolean, ) { @@ -370,7 +370,9 @@ export function persistExitData( deviceType: state.selectedDevice ? state.selectedDevice.deviceType : '', deviceTitle: state.selectedDevice ? state.selectedDevice.title : '', plugin: state.selectedPlugin || '', - app: state.selectedApp ? deconstructClientId(state.selectedApp).app : '', + app: state.selectedAppId + ? deconstructClientId(state.selectedAppId).app + : '', cleanExit, pid: remote.process.pid, }; diff --git a/desktop/app/src/plugin.tsx b/desktop/app/src/plugin.tsx index 239131c56..afdb4d4c1 100644 --- a/desktop/app/src/plugin.tsx +++ b/desktop/app/src/plugin.tsx @@ -72,7 +72,7 @@ export type Props = { deepLinkPayload: unknown; selectPlugin: (pluginID: string, deepLinkPayload: unknown) => void; isArchivedDevice: boolean; - selectedApp: string | null; + selectedApp: string | null; // name setStaticView: (payload: StaticView) => void; settingsState: Settings; }; diff --git a/desktop/app/src/reducers/__tests__/connections.node.tsx b/desktop/app/src/reducers/__tests__/connections.node.tsx index bd5a8fe6d..93fbe5039 100644 --- a/desktop/app/src/reducers/__tests__/connections.node.tsx +++ b/desktop/app/src/reducers/__tests__/connections.node.tsx @@ -7,7 +7,7 @@ * @format */ -import reducer from '../connections'; +import reducer, {selectClient, selectDevice} from '../connections'; import {State, selectPlugin} from '../connections'; import { _SandyPluginDefinition, @@ -16,7 +16,14 @@ import { MockedConsole, } from 'flipper-plugin'; import {TestDevice} from '../../test-utils/TestDevice'; -import {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin'; +import { + createMockFlipperWithPlugin, + MockFlipperResult, +} from '../../test-utils/createMockFlipperWithPlugin'; +import {Store} from '..'; +import {getActiveClient, getActiveDevice} from '../../selectors/connections'; +import BaseDevice from '../../devices/BaseDevice'; +import Client from '../../Client'; let mockedConsole: MockedConsole; beforeEach(() => { @@ -81,9 +88,23 @@ test('register, remove, re-register a metro device works correctly', () => { }); test('selectPlugin sets deepLinkPayload correctly', () => { - const state = reducer( + const device1 = new TestDevice( + 'http://localhost:8081', + 'emulator', + 'React Native', + 'Metro', + ); + let state = reducer(undefined, { + type: 'REGISTER_DEVICE', + payload: device1, + }); + state = reducer( undefined, - selectPlugin({selectedPlugin: 'myPlugin', deepLinkPayload: 'myPayload'}), + selectPlugin({ + selectedPlugin: 'myPlugin', + deepLinkPayload: 'myPayload', + selectedDevice: device1, + }), ); expect(state.deepLinkPayload).toBe('myPayload'); }); @@ -172,3 +193,221 @@ test('can handle device plugins that throw at start', async () => { `); expect(device2.sandyPluginStates.size).toBe(0); }); + +describe('selection changes', () => { + const TestPlugin1 = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails(), + { + Component() { + return null; + }, + plugin() { + return {}; + }, + }, + ); + const TestPlugin2 = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails(), + { + Component() { + return null; + }, + plugin() { + return {}; + }, + }, + ); + const DevicePlugin1 = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails({pluginType: 'device'}), + { + Component() { + return null; + }, + devicePlugin() { + return {}; + }, + }, + ); + + let device1: BaseDevice; + let device2: BaseDevice; + let metroDevice: BaseDevice; + let d1app1: Client; + let d1app2: Client; + let d2app1: Client; + let d2app2: Client; + let store: Store; + let mockFlipper: MockFlipperResult; + + beforeEach(async () => { + mockFlipper = await createMockFlipperWithPlugin(TestPlugin1, { + additionalPlugins: [TestPlugin2, DevicePlugin1], + supportedPlugins: [TestPlugin1.id, TestPlugin2.id, DevicePlugin1.id], + }); + + device1 = mockFlipper.device; + device2 = mockFlipper.createDevice({}); + metroDevice = mockFlipper.createDevice({ + os: 'Metro', + serial: 'http://localhost:8081', + }); + d1app1 = mockFlipper.client; + d1app2 = await mockFlipper.createClient(device1, 'd1app2'); + d2app1 = await mockFlipper.createClient(device2, 'd2app1'); + d2app2 = await mockFlipper.createClient(device2, 'd2app2'); + store = mockFlipper.store; + }); + + test('basic/ device selection change', async () => { + // after registering d1app2, this will have become the selection + expect(store.getState().connections).toMatchObject({ + selectedDevice: device1, + selectedPlugin: TestPlugin1.id, + selectedAppId: d1app2.id, + // no preferences changes, no explicit selection was made + userPreferredDevice: device1.title, + userPreferredPlugin: TestPlugin1.id, + userPreferredApp: d1app1.query.app, + }); + expect(getActiveClient(store.getState())).toBe(d1app2); + expect(getActiveDevice(store.getState())).toBe(device1); + + // select plugin 2 on d2app2 + store.dispatch( + selectPlugin({ + selectedPlugin: TestPlugin2.id, + selectedAppId: d2app2.id, + }), + ); + expect(store.getState().connections).toMatchObject({ + selectedDevice: device2, + selectedPlugin: TestPlugin2.id, + selectedAppId: d2app2.id, + userPreferredDevice: device2.title, + userPreferredPlugin: TestPlugin2.id, + userPreferredApp: d2app2.query.app, + }); + + // disconnect device1, and then register a new device should select it + device1.disconnect(); + const device3 = await mockFlipper.createDevice({}); + expect(store.getState().connections).toMatchObject({ + selectedDevice: device3, + selectedPlugin: TestPlugin2.id, + selectedAppId: null, + // prefs not updated + userPreferredDevice: device2.title, + userPreferredPlugin: TestPlugin2.id, + userPreferredApp: d2app2.query.app, + }); + + store.dispatch(selectDevice(device1)); + expect(store.getState().connections).toMatchObject({ + selectedDevice: device1, + selectedPlugin: TestPlugin2.id, + selectedAppId: null, + userPreferredDevice: device1.title, + // other prefs not updated + userPreferredPlugin: TestPlugin2.id, + userPreferredApp: d2app2.query.app, + }); + + // used by plugin list, to keep main device / app selection correct + expect(getActiveClient(store.getState())).toBe(null); + expect(getActiveDevice(store.getState())).toBe(device1); + }); + + test('select a metro device', async () => { + store.dispatch( + selectPlugin({ + selectedPlugin: DevicePlugin1.id, + selectedDevice: metroDevice, + selectedAppId: d2app1.id, // this app will determine the active device + }), + ); + + const state = store.getState(); + expect(state.connections).toMatchObject({ + selectedDevice: metroDevice, + selectedPlugin: DevicePlugin1.id, + selectedAppId: d2app1.id, + userPreferredDevice: metroDevice.title, + // other prefs not updated + userPreferredPlugin: DevicePlugin1.id, + userPreferredApp: d2app1.query.app, + }); + + // used by plugin list, to keep main device / app selection correct + expect(getActiveClient(state)).toBe(d2app1); + expect(getActiveDevice(state)).toBe(device2); + }); + + test('introducing new client does not select it', async () => { + await mockFlipper.createClient(device2, 'd2app3'); + expect(store.getState().connections).toMatchObject({ + selectedDevice: device1, + selectedPlugin: TestPlugin1.id, + selectedAppId: d1app2.id, + // other prefs not updated + userPreferredDevice: device1.title, + userPreferredPlugin: TestPlugin1.id, + userPreferredApp: d1app1.query.app, + }); + }); + + test('introducing new client does select it if preferred', async () => { + // pure testing evil + const client3 = await mockFlipper.createClient( + device2, + store.getState().connections.userPreferredApp!, + ); + expect(store.getState().connections).toMatchObject({ + selectedDevice: device2, + selectedPlugin: TestPlugin1.id, + selectedAppId: client3.id, + // other prefs not updated + userPreferredDevice: device1.title, + userPreferredPlugin: TestPlugin1.id, + userPreferredApp: d1app1.query.app, + }); + }); + + test('introducing new client does select it if old is offline', async () => { + d1app2.disconnect(); + const client3 = await mockFlipper.createClient(device2, 'd2app3'); + expect(store.getState().connections).toMatchObject({ + selectedDevice: device2, + selectedPlugin: TestPlugin1.id, + selectedAppId: client3.id, + // other prefs not updated + userPreferredDevice: device1.title, + userPreferredPlugin: TestPlugin1.id, + userPreferredApp: d1app1.query.app, + }); + }); + + test('select client', () => { + store.dispatch(selectClient(d2app2.id)); + expect(store.getState().connections).toMatchObject({ + selectedDevice: device2, + selectedPlugin: TestPlugin1.id, + selectedAppId: d2app2.id, + userPreferredDevice: device2.title, + userPreferredPlugin: TestPlugin1.id, + userPreferredApp: d2app2.query.app, + }); + }); + + test('select device', () => { + store.dispatch(selectDevice(metroDevice)); + expect(store.getState().connections).toMatchObject({ + selectedDevice: metroDevice, + selectedPlugin: TestPlugin1.id, + selectedAppId: null, + userPreferredDevice: metroDevice.title, + // other prefs not updated + userPreferredPlugin: TestPlugin1.id, + userPreferredApp: d1app1.query.app, + }); + }); +}); diff --git a/desktop/app/src/reducers/__tests__/sandydeviceplugins.node.tsx b/desktop/app/src/reducers/__tests__/sandydeviceplugins.node.tsx index 47658279c..6256dba20 100644 --- a/desktop/app/src/reducers/__tests__/sandydeviceplugins.node.tsx +++ b/desktop/app/src/reducers/__tests__/sandydeviceplugins.node.tsx @@ -56,7 +56,7 @@ function selectTestPlugin(store: Store) { store.dispatch( selectPlugin({ selectedPlugin: TestPlugin.id, - selectedApp: null, + selectedAppId: null, deepLinkPayload: null, selectedDevice: store.getState().connections.selectedDevice!, }), diff --git a/desktop/app/src/reducers/connections.tsx b/desktop/app/src/reducers/connections.tsx index 3dcba528d..086550faf 100644 --- a/desktop/app/src/reducers/connections.tsx +++ b/desktop/app/src/reducers/connections.tsx @@ -64,10 +64,10 @@ type StateV2 = { devices: Array; selectedDevice: null | BaseDevice; selectedPlugin: null | string; - selectedApp: null | string; + selectedAppId: null | string; // Full quantified identifier of the app userPreferredDevice: null | string; userPreferredPlugin: null | string; - userPreferredApp: null | string; + userPreferredApp: null | string; // The name of the preferred app, e.g. Facebook enabledPlugins: {[client: string]: string[]}; enabledDevicePlugins: Set; clients: Array; @@ -100,17 +100,13 @@ export type Action = | { type: 'SELECT_PLUGIN'; payload: { - selectedPlugin: null | string; - selectedApp?: null | string; - deepLinkPayload: unknown; - selectedDevice?: null | BaseDevice; + selectedPlugin: string; + selectedAppId?: null | string; // not set for device plugins + deepLinkPayload?: unknown; + selectedDevice?: BaseDevice | null; time: number; }; } - | { - type: 'SELECT_USER_PREFERRED_PLUGIN'; - payload: string; - } | { type: 'NEW_CLIENT'; payload: Client; @@ -119,10 +115,6 @@ export type Action = type: 'CLIENT_REMOVED'; payload: string; } - | { - type: 'PREFER_DEVICE'; - payload: string; - } | { type: 'START_CLIENT_SETUP'; payload: UninitializedClient; @@ -160,7 +152,7 @@ export type Action = } | { type: 'SELECT_CLIENT'; - payload: string | null; + payload: string; // App ID } | { type: 'APP_PLUGIN_LIST_CHANGED'; @@ -176,7 +168,7 @@ const DEFAULT_DEVICE_BLACKLIST: DeviceOS[] = ['MacOS', 'Metro', 'Windows']; const INITAL_STATE: State = { devices: [], selectedDevice: null, - selectedApp: null, + selectedAppId: null, selectedPlugin: DEFAULT_PLUGIN, userPreferredDevice: null, userPreferredPlugin: null, @@ -208,32 +200,31 @@ export default (state: State = INITAL_STATE, action: Actions): State => { case 'SET_STATIC_VIEW': { const {payload, deepLinkPayload} = action; - const {selectedPlugin} = state; return { ...state, staticView: payload, - selectedPlugin: payload != null ? null : selectedPlugin, deepLinkPayload: deepLinkPayload ?? null, }; } case 'RESET_SUPPORT_FORM_V2_STATE': { - return updateSelection({ + return { ...state, staticView: null, - }); + }; } case 'SELECT_DEVICE': { const {payload} = action; - return updateSelection({ + return { ...state, staticView: null, selectedDevice: payload, + selectedAppId: null, userPreferredDevice: payload ? payload.title : state.userPreferredDevice, - }); + }; } case 'REGISTER_DEVICE': { @@ -253,65 +244,66 @@ export default (state: State = INITAL_STATE, action: Actions): State => { newDevices.push(payload); } - return updateSelection({ + const selectNewDevice = + !state.selectedDevice || + !state.selectedDevice.isConnected || + state.userPreferredDevice === payload.title; + let selectedAppId = state.selectedAppId; + + if (selectNewDevice) { + // need to select a different app + selectedAppId = + state.clients.find( + (c) => + c.device === payload && c.query.app === state.userPreferredApp, + )?.id ?? null; + // nothing found, try first app if any + if (!selectedAppId) { + selectedAppId = + state.clients.find((c) => c.device === payload)?.id ?? null; + } + } + + return { ...state, devices: newDevices, - }); + selectedDevice: selectNewDevice ? payload : state.selectedDevice, + selectedAppId, + }; } case 'SELECT_PLUGIN': { - const {payload} = action; - const {selectedPlugin, selectedApp, deepLinkPayload} = payload; - let selectedDevice = payload.selectedDevice; - if (typeof deepLinkPayload === 'string') { - const deepLinkParams = new URLSearchParams(deepLinkPayload); - const deviceParam = deepLinkParams.get('device'); - if (deviceParam) { - const deviceMatch = state.devices.find( - (v) => v.title === deviceParam, - ); - if (deviceMatch) { - selectedDevice = deviceMatch; - } else { - console.warn( - `Could not find matching device "${deviceParam}" requested through deep-link.`, - ); - } - } - } - if (!selectedDevice && selectedPlugin) { - const selectedClient = state.clients.find((c) => - c.supportsPlugin(selectedPlugin), - ); - selectedDevice = state.devices.find( - (v) => v.serial === selectedClient?.query.device_id, - ); - } - if (!selectedDevice) { - console.warn('Trying to select a plugin before a device was selected!'); - } + const {selectedPlugin, selectedAppId, deepLinkPayload} = action.payload; + if (selectedPlugin) { performance.mark(`activePlugin-${selectedPlugin}`); } - return updateSelection({ + const client = state.clients.find((c) => c.id === selectedAppId); + const device = action.payload.selectedDevice ?? client?.device; + + if (!device) { + console.warn( + 'No valid device / client provided when calling SELECT_PLUGIN', + ); + return state; + } + + return { ...state, staticView: null, - selectedApp: selectedApp || null, + selectedDevice: device, + userPreferredDevice: canBeDefaultDevice(device) + ? device.title + : state.userPreferredDevice, + selectedAppId: selectedAppId ?? null, + userPreferredApp: + state.clients.find((c) => c.id === selectedAppId)?.query.app ?? + state.userPreferredApp, selectedPlugin, - userPreferredPlugin: selectedPlugin || state.userPreferredPlugin, - selectedDevice: selectedDevice!, - userPreferredDevice: - selectedDevice && canBeDefaultDevice(selectedDevice) - ? selectedDevice.title - : state.userPreferredDevice, + userPreferredPlugin: selectedPlugin, deepLinkPayload: deepLinkPayload, - }); - } - - case 'SELECT_USER_PREFERRED_PLUGIN': { - const {payload} = action; - return {...state, userPreferredPlugin: payload}; + }; } case 'NEW_CLIENT': { @@ -328,8 +320,18 @@ export default (state: State = INITAL_STATE, action: Actions): State => { }); newClients.push(payload); - return updateSelection({ + // select new client if nothing select, this one is preferred, or the old one is offline + const selectNewClient = + !state.selectedAppId || + state.userPreferredApp === payload.query.app || + state.clients + .find((c) => c.id === state.selectedAppId) + ?.connected.get() === false; + + return { ...state, + selectedAppId: selectNewClient ? payload.id : state.selectedAppId, + selectedDevice: selectNewClient ? payload.device : state.selectedDevice, clients: newClients, uninitializedClients: state.uninitializedClients.filter((c) => { return ( @@ -337,16 +339,31 @@ export default (state: State = INITAL_STATE, action: Actions): State => { c.appName !== payload.query.app ); }), - }); + }; } case 'SELECT_CLIENT': { const {payload} = action; - return updateSelection({ + const client = state.clients.find((c) => c.id === payload); + + if (!client) { + return state; + } + + return { ...state, - selectedApp: payload, - userPreferredApp: payload || state.userPreferredApp, - }); + selectedAppId: payload, + selectedDevice: client.device, + userPreferredDevice: client.device.title, + userPreferredApp: client.query.app, + selectedPlugin: + state.selectedPlugin && client.supportsPlugin(state.selectedPlugin) + ? state.selectedPlugin + : state.userPreferredPlugin && + client.supportsPlugin(state.userPreferredPlugin) + ? state.userPreferredPlugin + : null, + }; } case 'CLIENT_REMOVED': { @@ -355,16 +372,14 @@ export default (state: State = INITAL_STATE, action: Actions): State => { const newClients = state.clients.filter( (client) => client.id !== payload, ); - return updateSelection({ + return { ...state, + selectedAppId: + state.selectedAppId === payload ? null : state.selectedAppId, clients: newClients, - }); + }; } - case 'PREFER_DEVICE': { - const {payload: userPreferredDevice} = action; - return {...state, userPreferredDevice}; - } case 'START_CLIENT_SETUP': { const {payload} = action; return { @@ -457,23 +472,18 @@ export const setStaticView = ( }; }; -export const preferDevice = (payload: string): Action => ({ - type: 'PREFER_DEVICE', - payload, -}); - export const selectPlugin = (payload: { - selectedPlugin: null | string; - selectedApp?: null | string; - selectedDevice?: BaseDevice | null; - deepLinkPayload: unknown; + selectedPlugin: string; + selectedAppId?: null | string; + selectedDevice?: null | BaseDevice; + deepLinkPayload?: unknown; time?: number; }): Action => ({ type: 'SELECT_PLUGIN', payload: {...payload, time: payload.time ?? Date.now()}, }); -export const selectClient = (clientId: string | null): Action => ({ +export const selectClient = (clientId: string): Action => ({ type: 'SELECT_CLIENT', payload: clientId, }); @@ -532,20 +542,11 @@ export function getAvailableClients( .sort((a, b) => (a.query.app || '').localeCompare(b.query.app)); } -function getBestAvailableClient( - device: BaseDevice | null | undefined, +export function getClientByAppName( clients: Client[], - preferredClient: string | null, -): Client | null { - const availableClients = getAvailableClients(device, clients); - if (availableClients.length === 0) { - return null; - } - return ( - getClientById(availableClients, preferredClient) || - availableClients[0] || - null - ); + appName: string | null | undefined, +): Client | undefined { + return clients.find((client) => client.query.app === appName); } export function getClientById( @@ -559,67 +560,10 @@ export function canBeDefaultDevice(device: BaseDevice) { return !DEFAULT_DEVICE_BLACKLIST.includes(device.os); } -/** - * This function, given the current state, tries to build to build the best - * selection possible, preselection device if there is non, plugins based on preferences, etc - * @param state - */ -function updateSelection(state: Readonly): State { - if (state.staticView && state.staticView !== WelcomeScreenStaticView) { - return state; - } - - const updates: Partial = { - staticView: null, - }; - // Find the selected device if it still exists - let device: BaseDevice | null = - state.selectedDevice && state.devices.includes(state.selectedDevice) - ? state.selectedDevice - : null; - if (!device) { - device = - state.devices.find( - (device) => device.title === state.userPreferredDevice, - ) || - state.devices.find((device) => canBeDefaultDevice(device)) || - null; - } - updates.selectedDevice = device; - if (!device) { - updates.staticView = WelcomeScreenStaticView; - } - - // Select client based on device - const client = getBestAvailableClient( - device, - state.clients, - state.selectedApp || state.userPreferredApp, - ); - updates.selectedApp = client ? client.id : null; - - if ( - // Try the preferred plugin first - state.userPreferredPlugin && - state.userPreferredPlugin !== state.selectedPlugin - ) { - updates.selectedPlugin = state.userPreferredPlugin; - } else if ( - !state.selectedPlugin && - state.enabledDevicePlugins.has(DEFAULT_PLUGIN) - ) { - // currently selected plugin is not available in this state, - // fall back to the default - updates.selectedPlugin = DEFAULT_PLUGIN; - } - - return {...state, ...updates}; -} - export function getSelectedPluginKey(state: State): string | undefined { return state.selectedPlugin ? getPluginKey( - state.selectedApp, + state.selectedAppId, state.selectedDevice, state.selectedPlugin, ) diff --git a/desktop/app/src/reducers/notifications.tsx b/desktop/app/src/reducers/notifications.tsx index 3afa6512e..8506250fa 100644 --- a/desktop/app/src/reducers/notifications.tsx +++ b/desktop/app/src/reducers/notifications.tsx @@ -17,13 +17,13 @@ export const GLOBAL_NOTIFICATION_PLUGIN_ID = 'Flipper'; export type PluginNotification = { notification: Notification; pluginId: string; - client: null | string; + client: null | string; // id }; export type PluginNotificationReference = { notificationId: string; pluginId: string; - client: null | string; + client: null | string; // id }; export type State = { diff --git a/desktop/app/src/reducers/supportForm.tsx b/desktop/app/src/reducers/supportForm.tsx index 4b3220724..9d2e572a9 100644 --- a/desktop/app/src/reducers/supportForm.tsx +++ b/desktop/app/src/reducers/supportForm.tsx @@ -116,9 +116,9 @@ export class Group { store.dispatch( setStaticView(require('../fb-stubs/SupportRequestFormV2').default), ); - const selectedApp = store.getState().connections.selectedApp; + const selectedApp = store.getState().connections.selectedAppId; const selectedClient = store.getState().connections.clients.find((o) => { - return o.id === store.getState().connections.selectedApp; + return o.id === store.getState().connections.selectedAppId; }); let errorMessage: string | undefined = undefined; if (selectedApp) { diff --git a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx index f77a50557..abd22bd0f 100644 --- a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx @@ -50,8 +50,13 @@ function getOsIcon(os?: DeviceOS) { export function AppSelector() { const dispatch = useDispatch(); - const {devices, selectedDevice, clients, uninitializedClients, selectedApp} = - useStore((state) => state.connections); + const { + devices, + selectedDevice, + clients, + uninitializedClients, + selectedAppId, + } = useStore((state) => state.connections); useValue(selectedDevice?.connected, false); // subscribe to future archived state changes const onSelectDevice = useTrackedCallback( @@ -59,16 +64,14 @@ export function AppSelector() { (device: BaseDevice) => { batch(() => { dispatch(selectDevice(device)); - dispatch(selectClient(null)); }); }, [], ); const onSelectApp = useTrackedCallback( 'select-app', - (device: BaseDevice, client: Client) => { + (_device: BaseDevice, client: Client) => { batch(() => { - dispatch(selectDevice(device)); dispatch(selectClient(client.id)); }); }, @@ -82,13 +85,13 @@ export function AppSelector() { onSelectDevice, onSelectApp, ); - const client = clients.find((client) => client.id === selectedApp); + const client = clients.find((client) => client.id === selectedAppId); return ( <> {entries.length ? ( + {entries} }> diff --git a/desktop/app/src/sandy-chrome/appinspect/BookmarkSection.tsx b/desktop/app/src/sandy-chrome/appinspect/BookmarkSection.tsx index f49707270..c18ef872a 100644 --- a/desktop/app/src/sandy-chrome/appinspect/BookmarkSection.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/BookmarkSection.tsx @@ -140,9 +140,9 @@ const StyledAutoComplete = styled(AutoComplete)({ const NAVIGATION_PLUGIN_ID = 'Navigation'; function navPluginStateSelector(state: State) { - const {selectedApp, clients} = state.connections; - if (!selectedApp) return undefined; - const client = clients.find((client) => client.id === selectedApp); + const {selectedAppId, clients} = state.connections; + if (!selectedAppId) return undefined; + const client = clients.find((client) => client.id === selectedAppId); if (!client) return undefined; return client.sandyPluginStates.get(NAVIGATION_PLUGIN_ID)?.instanceApi as | undefined diff --git a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx index 365d89227..53cc50928 100644 --- a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx @@ -99,26 +99,26 @@ export const PluginList = memo(function PluginList({ dispatch( selectPlugin({ selectedPlugin: pluginId, - selectedApp: connections.selectedApp, + selectedAppId: connections.selectedAppId, deepLinkPayload: null, selectedDevice: activeDevice, }), ); }, - [dispatch, activeDevice, connections.selectedApp], + [dispatch, activeDevice, connections.selectedAppId], ); const handleMetroPluginClick = useCallback( (pluginId) => { dispatch( selectPlugin({ selectedPlugin: pluginId, - selectedApp: connections.selectedApp, + selectedAppId: connections.selectedAppId, deepLinkPayload: null, selectedDevice: metroDevice, }), ); }, - [dispatch, metroDevice, connections.selectedApp], + [dispatch, metroDevice, connections.selectedAppId], ); const handleEnablePlugin = useCallback( (id: string) => { diff --git a/desktop/app/src/sandy-chrome/appinspect/__tests__/PluginList.spec.tsx b/desktop/app/src/sandy-chrome/appinspect/__tests__/PluginList.spec.tsx index 55c6d1f60..cd5f773d0 100644 --- a/desktop/app/src/sandy-chrome/appinspect/__tests__/PluginList.spec.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/__tests__/PluginList.spec.tsx @@ -12,7 +12,6 @@ import { MockFlipperResult, } from '../../../test-utils/createMockFlipperWithPlugin'; import {FlipperPlugin} from '../../../plugin'; -import MetroDevice from '../../../server/devices/metro/MetroDevice'; import BaseDevice from '../../../devices/BaseDevice'; import {_SandyPluginDefinition} from 'flipper-plugin'; import {TestUtils} from 'flipper-plugin'; @@ -106,7 +105,7 @@ describe('basic getActiveDevice with metro present', () => { selectedPlugin: 'DeviceLogs', userPreferredDevice: 'MockAndroidDevice', userPreferredPlugin: 'DeviceLogs', - userPreferredApp: 'TestApp#Android#MockAndroidDevice#serial', + userPreferredApp: 'TestApp', }); expect(getActiveClient(state)).toBe(flipper.client); }); @@ -116,19 +115,19 @@ describe('basic getActiveDevice with metro present', () => { flipper.store.dispatch( selectPlugin({ selectedPlugin: logsPlugin.id, - selectedApp: null, + selectedAppId: flipper.client.id, selectedDevice: metro, deepLinkPayload: null, }), ); expect(flipper.store.getState().connections).toMatchObject({ devices: [testDevice, metro], - selectedApp: null, + selectedAppId: 'TestApp#Android#MockAndroidDevice#serial', selectedDevice: metro, selectedPlugin: 'DeviceLogs', userPreferredDevice: 'MockAndroidDevice', // Not metro! userPreferredPlugin: 'DeviceLogs', - userPreferredApp: 'TestApp#Android#MockAndroidDevice#serial', + userPreferredApp: 'TestApp', }); const state = flipper.store.getState(); // find best device is still metro diff --git a/desktop/app/src/sandy-chrome/notification/Notification.tsx b/desktop/app/src/sandy-chrome/notification/Notification.tsx index 024874060..3fc71a806 100644 --- a/desktop/app/src/sandy-chrome/notification/Notification.tsx +++ b/desktop/app/src/sandy-chrome/notification/Notification.tsx @@ -310,7 +310,7 @@ export function openNotification(store: Store, noti: PluginNotificationOrig) { store.dispatch( selectPlugin({ selectedPlugin: noti.pluginId, - selectedApp: noti.client, + selectedAppId: client.id, selectedDevice: client.device, deepLinkPayload: noti.notification.action, }), diff --git a/desktop/app/src/selectors/connections.tsx b/desktop/app/src/selectors/connections.tsx index 07a076d1b..c138e862f 100644 --- a/desktop/app/src/selectors/connections.tsx +++ b/desktop/app/src/selectors/connections.tsx @@ -16,17 +16,14 @@ import { import createSelector from './createSelector'; const getSelectedPluginId = (state: State) => state.connections.selectedPlugin; -const getSelectedApp = (state: State) => - state.connections.selectedApp || state.connections.userPreferredApp; +const getSelectedAppId = (state: State) => state.connections.selectedAppId; const getSelectedDevice = (state: State) => state.connections.selectedDevice; -const getUserPreferredDevice = (state: State) => - state.connections.userPreferredDevice; const getClients = (state: State) => state.connections.clients; const getDevices = (state: State) => state.connections.devices; const getPluginDownloads = (state: State) => state.pluginDownloads; export const getActiveClient = createSelector( - getSelectedApp, + getSelectedAppId, getClients, (selectedApp, clients) => { return clients.find((c) => c.id === selectedApp) || null; @@ -42,11 +39,9 @@ export const getMetroDevice = createSelector(getDevices, (devices) => { export const getActiveDevice = createSelector( getSelectedDevice, - getUserPreferredDevice, - getDevices, getActiveClient, getMetroDevice, - (selectedDevice, userPreferredDevice, devices, client, metroDevice) => { + (selectedDevice, client, metroDevice) => { // if not Metro device, use the selected device as metro device if (selectedDevice !== metroDevice) { return selectedDevice; @@ -55,13 +50,6 @@ export const getActiveDevice = createSelector( if (client) { return client.device; } - // if no active app, use the preferred device - if (userPreferredDevice) { - return ( - devices.find((device) => device.title === userPreferredDevice) ?? - selectedDevice - ); - } return selectedDevice; }, ); diff --git a/desktop/app/src/server/devices/metro/MetroDevice.tsx b/desktop/app/src/server/devices/metro/MetroDevice.tsx index 92323344b..4ff0ee2f0 100644 --- a/desktop/app/src/server/devices/metro/MetroDevice.tsx +++ b/desktop/app/src/server/devices/metro/MetroDevice.tsx @@ -152,6 +152,7 @@ export default class MetroDevice extends ServerDevice { deviceType: 'emulator', title: 'React Native', os: 'Metro', + icon: 'mobile', }); if (ws) { this.ws = ws; diff --git a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx index 53aa09271..54f005235 100644 --- a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx +++ b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx @@ -191,7 +191,7 @@ export async function createMockFlipperWithPlugin( store.dispatch( selectPlugin({ selectedPlugin: id, - selectedApp: theClient.query.app, + selectedAppId: theClient.id, deepLinkPayload, selectedDevice: theDevice, }), @@ -268,13 +268,13 @@ export async function renderMockFlipperWithPlugin( isDevicePluginDefinition(pluginClazz) ? { selectedPlugin: pluginClazz.id, - selectedApp: null, + selectedAppId: null, deepLinkPayload: null, selectedDevice: store.getState().connections.selectedDevice!, } : { selectedPlugin: pluginClazz.id, - selectedApp: client.query.app, + selectedAppId: client.id, deepLinkPayload: null, selectedDevice: store.getState().connections.selectedDevice!, }, diff --git a/desktop/app/src/utils/__tests__/exportData.node.tsx b/desktop/app/src/utils/__tests__/exportData.node.tsx index e09ed7c76..d06266adf 100644 --- a/desktop/app/src/utils/__tests__/exportData.node.tsx +++ b/desktop/app/src/utils/__tests__/exportData.node.tsx @@ -1137,7 +1137,7 @@ test('Sandy plugins are exported properly', async () => { store.dispatch( selectPlugin({ selectedPlugin: 'DeviceLogs', - selectedApp: null, + selectedAppId: null, selectedDevice: device, deepLinkPayload: null, }), @@ -1195,7 +1195,7 @@ test('Non sandy plugins are exported properly if they are still queued', async ( store.dispatch( selectPlugin({ selectedPlugin: 'DeviceLogs', - selectedApp: null, + selectedAppId: null, selectedDevice: device, deepLinkPayload: null, }), @@ -1229,7 +1229,7 @@ test('Sandy plugins with custom export are exported properly', async () => { store.dispatch( selectPlugin({ selectedPlugin: 'DeviceLogs', - selectedApp: client.id, + selectedAppId: client.id, deepLinkPayload: null, }), ); diff --git a/desktop/app/src/utils/__tests__/info.node.tsx b/desktop/app/src/utils/__tests__/info.node.tsx index 1ed89f957..18202a038 100644 --- a/desktop/app/src/utils/__tests__/info.node.tsx +++ b/desktop/app/src/utils/__tests__/info.node.tsx @@ -72,7 +72,7 @@ describe('info', () => { store.dispatch( selectPlugin({ selectedPlugin: inspectorPlugin.id, - selectedApp: client.query.app, + selectedAppId: client.id, selectedDevice: device, deepLinkPayload: null, }), diff --git a/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx b/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx index f99d24513..18578ea3c 100644 --- a/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx +++ b/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx @@ -77,7 +77,7 @@ function selectDeviceLogs(store: Store) { store.dispatch( selectPlugin({ selectedPlugin: 'DeviceLogs', - selectedApp: null, + selectedAppId: null, deepLinkPayload: null, selectedDevice: store.getState().connections.selectedDevice!, }), @@ -88,7 +88,7 @@ function selectTestPlugin(store: Store, client: Client) { store.dispatch( selectPlugin({ selectedPlugin: TestPlugin.id, - selectedApp: client.query.app, + selectedAppId: client.id, deepLinkPayload: null, selectedDevice: store.getState().connections.selectedDevice!, }), @@ -408,7 +408,7 @@ test('queue - messages that arrive during processing will be queued', async () = store.dispatch( selectPlugin({ selectedPlugin: TestPlugin.id, - selectedApp: client.id, + selectedAppId: client.id, deepLinkPayload: null, selectedDevice: device, }), diff --git a/desktop/app/src/utils/exportData.tsx b/desktop/app/src/utils/exportData.tsx index 984728cc3..ac040d71a 100644 --- a/desktop/app/src/utils/exportData.tsx +++ b/desktop/app/src/utils/exportData.tsx @@ -418,7 +418,7 @@ async function getStoreExport( fetchMetaDataErrors: {[plugin: string]: Error} | null; }> { let state = store.getState(); - const {clients, selectedApp, selectedDevice} = state.connections; + const {clients, selectedAppId, selectedDevice} = state.connections; const pluginsToProcess = determinePluginsToProcess( clients, selectedDevice, @@ -433,7 +433,7 @@ async function getStoreExport( const fetchMetaDataMarker = `${EXPORT_FLIPPER_TRACE_EVENT}:fetch-meta-data`; performance.mark(fetchMetaDataMarker); - const client = clients.find((client) => client.id === selectedApp); + const client = clients.find((client) => client.id === selectedAppId); const pluginStates2 = pluginsToProcess ? await exportSandyPluginStates(pluginsToProcess, idler, statusUpdate) @@ -487,9 +487,9 @@ export async function exportStore( exportData.supportRequestDetails = { ...state.supportForm?.supportFormV2, appName: - state.connections.selectedApp == null + state.connections.selectedAppId == null ? '' - : deconstructClientId(state.connections.selectedApp).app, + : deconstructClientId(state.connections.selectedAppId).app, }; } diff --git a/desktop/app/src/utils/flipperLibImplementation.tsx b/desktop/app/src/utils/flipperLibImplementation.tsx index 00d547c4a..4d8c18d35 100644 --- a/desktop/app/src/utils/flipperLibImplementation.tsx +++ b/desktop/app/src/utils/flipperLibImplementation.tsx @@ -41,7 +41,7 @@ export function initializeFlipperLibImplementation( payload: { selectedPlugin: pluginId, selectedDevice: device as BaseDevice, - selectedApp: client ? client.id : null, + selectedAppId: client ? client.id : null, deepLinkPayload: deeplink, time: Date.now(), }, diff --git a/desktop/app/src/utils/info.tsx b/desktop/app/src/utils/info.tsx index 843a72746..a92805061 100644 --- a/desktop/app/src/utils/info.tsx +++ b/desktop/app/src/utils/info.tsx @@ -129,14 +129,16 @@ export function stringifyInfo(info: Info): string { export function getSelectionInfo({ plugins: {clientPlugins, devicePlugins, loadedPlugins}, connections: { - selectedApp, + selectedAppId, selectedPlugin, enabledDevicePlugins, enabledPlugins, selectedDevice, }, }: State): SelectionInfo { - const clientIdParts = selectedApp ? deconstructClientId(selectedApp) : null; + const clientIdParts = selectedAppId + ? deconstructClientId(selectedAppId) + : null; const loadedPlugin = selectedPlugin ? loadedPlugins.get(selectedPlugin) : null;