From 6fe477f19bbc9bc237a95ae68974439637ab1ea8 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 14 Jul 2020 09:04:59 -0700 Subject: [PATCH] Make sure Sandy plugins are selectable during export Summary: This diff makes sure Sandy plugins show up as well in the plugin selector when making exports (and in support form as well). Also verified that this works with the Sandy updated section plugin. Note that persisted state now didn't need any changes in the plugin code to work :) Commented / simplified the calculation of available plugins a little bit and fixed a confusing issue where two different redux stores where created in one unit test, which caused an issue in the new implementation. Reviewed By: jknoxville Differential Revision: D22434301 fbshipit-source-id: c911196bc5b105309e82204188f124f40aab487a --- .../__tests__/ExportDataPluginSheet.node.tsx | 34 +++--- .../src/utils/__tests__/pluginUtils.node.js | 1 + desktop/app/src/utils/pluginUtils.tsx | 112 +++++++++--------- .../src/__tests__/TestPlugin.tsx | 3 - desktop/flipper-plugin/src/plugin/Plugin.tsx | 4 + .../src/__tests__/seammammals.node.tsx | 19 +++ desktop/plugins/seamammals/src/index.tsx | 4 +- 7 files changed, 102 insertions(+), 75 deletions(-) diff --git a/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx b/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx index 35242da80..251d0b866 100644 --- a/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx +++ b/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx @@ -40,31 +40,22 @@ function getStore(selectedPlugins: Array) { debug: () => {}, trackTimeSince: () => {}, }; - let mockStore = configureStore([])(); const selectedDevice = new BaseDevice( 'serial', 'emulator', 'TestiPhone', 'iOS', ); - const client = new Client( - generateClientIdentifier(selectedDevice, 'app'), - {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial'}, - null, - logger, - // @ts-ignore - mockStore, - ['TestPlugin', 'TestDevicePlugin'], - ); + const clientId = generateClientIdentifier(selectedDevice, 'app'); const pluginStates: {[key: string]: any} = {}; - pluginStates[`${client.id}#TestDevicePlugin`] = { + pluginStates[`${clientId}#TestDevicePlugin`] = { msg: 'Test Device plugin', }; - pluginStates[`${client.id}#TestPlugin`] = { + pluginStates[`${clientId}#TestPlugin`] = { msg: 'Test plugin', }; - mockStore = configureStore([])({ + const mockStore = configureStore([])({ application: {share: {closeOnFinish: false, type: 'link'}}, plugins: { clientPlugins: new Map([['TestPlugin', TestPlugin]]), @@ -76,8 +67,23 @@ function getStore(selectedPlugins: Array) { }, pluginStates, pluginMessageQueue: [], - connections: {selectedApp: client.id, clients: [client]}, + connections: {selectedApp: clientId, clients: []}, }); + + const client = new Client( + clientId, + {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial'}, + null, + logger, + // @ts-ignore + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + mockStore.dispatch({ + type: 'NEW_CLIENT', + payload: client, + }); + return mockStore; } diff --git a/desktop/app/src/utils/__tests__/pluginUtils.node.js b/desktop/app/src/utils/__tests__/pluginUtils.node.js index c9178acf7..5b9d71942 100644 --- a/desktop/app/src/utils/__tests__/pluginUtils.node.js +++ b/desktop/app/src/utils/__tests__/pluginUtils.node.js @@ -95,6 +95,7 @@ function mockPluginState( function mockPluginDefinition(name: string): PluginDetails { return { name, + id: name, out: 'out', }; } diff --git a/desktop/app/src/utils/pluginUtils.tsx b/desktop/app/src/utils/pluginUtils.tsx index 6668736ca..ae0324a18 100644 --- a/desktop/app/src/utils/pluginUtils.tsx +++ b/desktop/app/src/utils/pluginUtils.tsx @@ -17,7 +17,6 @@ import { import {State as PluginStatesState} from '../reducers/pluginStates'; import {State as PluginsState} from '../reducers/plugins'; import {State as PluginMessageQueueState} from '../reducers/pluginMessageQueue'; -import {PluginDetails} from 'flipper-plugin-lib'; import {deconstructPluginKey, deconstructClientId} from './clientUtils'; type Client = import('../Client').default; @@ -27,14 +26,10 @@ export const defaultEnabledBackgroundPlugins = ['Navigation']; // The navigation export function pluginsClassMap( plugins: PluginsState, ): Map { - const pluginsMap: Map = new Map([]); - plugins.clientPlugins.forEach((val, key) => { - pluginsMap.set(key, val); - }); - plugins.devicePlugins.forEach((val, key) => { - pluginsMap.set(key, val); - }); - return pluginsMap; + return new Map([ + ...plugins.clientPlugins.entries(), + ...plugins.devicePlugins.entries(), + ]); } export function getPluginKey( @@ -141,65 +136,70 @@ export function getActivePersistentPlugins( plugins: PluginsState, selectedClient?: Client, ): {id: string; label: string}[] { - const pluginsMap: Map = pluginsClassMap(plugins); + const pluginsMap = pluginsClassMap(plugins); return getPersistentPlugins(plugins) .map((pluginName) => pluginsMap.get(pluginName)!) .sort(sortPluginsByName) - .map((plugin) => { - const keys = [ - ...new Set([ - ...Object.keys(pluginsState), - ...Object.keys(pluginsMessageQueue), - ]), - ] - .filter((k) => !selectedClient || k.includes(selectedClient.id)) - .map((key) => deconstructPluginKey(key).pluginName); - let result = plugin.id == 'DeviceLogs'; - const pluginsWithExportPersistedState = - plugin && plugin.exportPersistedState != undefined; - const pluginsWithReduxData = keys.includes(plugin.id); - if (!result && selectedClient) { - // If there is a selected client, active persistent plugin is the plugin which is active for selectedClient and also persistent. - result = - selectedClient.plugins.includes(plugin.id) && - (pluginsWithExportPersistedState || pluginsWithReduxData); - } else if (!result && !selectedClient) { - // If there is no selected client, active persistent plugin is the plugin which is just persistent. - result = - (plugin && plugin.exportPersistedState != undefined) || - keys.includes(plugin.id); + .filter((plugin) => { + if (plugin.id == 'DeviceLogs') { + return true; + } + if (selectedClient) { + const pluginKey = getPluginKey( + selectedClient.id, + {serial: selectedClient.query.device_id}, + plugin.id, + ); + // If there is a selected client, active persistent plugins are those that (can) have persisted state + return ( + selectedClient.isEnabledPlugin(plugin.id) && + // this plugin can fetch and export state + (plugin.exportPersistedState || + // this plugin has some persisted state already + pluginsState[pluginKey] || + pluginsMessageQueue[pluginKey] || + // this plugin has some persistable sandy state + selectedClient.sandyPluginStates.get(plugin.id)?.isPersistable()) + ); + } + { + // If there is no selected client, active persistent plugin is the plugin which is just persistent. + const pluginsWithReduxData = [ + ...new Set([ + ...Object.keys(pluginsState), + ...Object.keys(pluginsMessageQueue), + ]), + ].map((key) => deconstructPluginKey(key).pluginName); + return ( + (plugin && plugin.exportPersistedState != undefined) || + isSandyPlugin(plugin) || + pluginsWithReduxData.includes(plugin.id) + ); } - return (result - ? { - id: plugin.id, - label: getPluginTitle(plugin), - } - : undefined)!; }) - .filter(Boolean); + .map((plugin) => ({ + id: plugin.id, + label: getPluginTitle(plugin), + })); } +/** + * Returns all enabled plugins that are potentially exportable + * @param plugins + */ export function getPersistentPlugins(plugins: PluginsState): Array { - const pluginsMap: Map = pluginsClassMap(plugins); + const pluginsMap = pluginsClassMap(plugins); - const arr: Array = plugins.disabledPlugins.concat( - plugins.gatekeepedPlugins, - ); - arr.forEach((plugin: PluginDetails) => { - if (pluginsMap.has(plugin.name)) { + [...plugins.disabledPlugins, ...plugins.gatekeepedPlugins].forEach( + (plugin) => { pluginsMap.delete(plugin.name); - } + }, + ); + plugins.failedPlugins.forEach(([details]) => { + pluginsMap.delete(details.id); }); - plugins.failedPlugins.forEach((plugin: [PluginDetails, string]) => { - if (plugin[0] && plugin[0].name && pluginsMap.has(plugin[0].name)) { - pluginsMap.delete(plugin[0].name); - } - }); - - const activePlugins = [...pluginsMap.keys()]; - - return activePlugins.filter((plugin) => { + return Array.from(pluginsMap.keys()).filter((plugin) => { const pluginClass = pluginsMap.get(plugin); return ( plugin == 'DeviceLogs' || diff --git a/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx b/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx index db67005b1..314c0736c 100644 --- a/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx +++ b/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx @@ -35,9 +35,6 @@ export function plugin(client: FlipperClient) { }, ); - // TODO: add tests for sending and receiving data T68683442 - // including typescript assertions - client.onConnect(connectStub); client.onDisconnect(disconnectStub); client.onDestroy(destroyStub); diff --git a/desktop/flipper-plugin/src/plugin/Plugin.tsx b/desktop/flipper-plugin/src/plugin/Plugin.tsx index 945b2aa38..2f0bf274b 100644 --- a/desktop/flipper-plugin/src/plugin/Plugin.tsx +++ b/desktop/flipper-plugin/src/plugin/Plugin.tsx @@ -217,6 +217,10 @@ export class SandyPluginInstance { ); } + isPersistable(): boolean { + return Object.keys(this.rootStates).length > 0; + } + private assertNotDestroyed() { if (this.destroyed) { throw new Error('Plugin has been destroyed already'); diff --git a/desktop/plugins/seamammals/src/__tests__/seammammals.node.tsx b/desktop/plugins/seamammals/src/__tests__/seammammals.node.tsx index 41a0394a8..caf971e2d 100644 --- a/desktop/plugins/seamammals/src/__tests__/seammammals.node.tsx +++ b/desktop/plugins/seamammals/src/__tests__/seammammals.node.tsx @@ -119,4 +119,23 @@ test('It can have selection and render details', async () => { // Sidebar should be visible now expect(await renderer.findByText('Extras')).not.toBeNull(); + + // Verify export + expect(plugin.exportState()).toMatchInlineSnapshot(` + Object { + "rows": Object { + "1": Object { + "id": 1, + "title": "Dolphin", + "url": "http://dolphin.png", + }, + "2": Object { + "id": 2, + "title": "Turtle", + "url": "http://turtle.png", + }, + }, + "selection": "2", + } + `); }); diff --git a/desktop/plugins/seamammals/src/index.tsx b/desktop/plugins/seamammals/src/index.tsx index 174befdd4..65b94a046 100644 --- a/desktop/plugins/seamammals/src/index.tsx +++ b/desktop/plugins/seamammals/src/index.tsx @@ -50,8 +50,8 @@ type PersistedState = { }; export function plugin(client: FlipperClient) { - const rows = createState({}); - const selectedID = createState(null); + const rows = createState({}, {persist: 'rows'}); + const selectedID = createState(null, {persist: 'selection'}); client.onMessage('newRow', (row) => { rows.update((draft) => {