diff --git a/desktop/app/src/__tests__/deeplink.node.tsx b/desktop/app/src/__tests__/deeplink.node.tsx index 3df509039..3104b6a69 100644 --- a/desktop/app/src/__tests__/deeplink.node.tsx +++ b/desktop/app/src/__tests__/deeplink.node.tsx @@ -99,11 +99,16 @@ test('Will throw error on invalid deeplinks', async () => { ).rejects.toThrowErrorMatchingInlineSnapshot(`"Unknown deeplink"`); expect(logger.track).toHaveBeenCalledTimes(2); - expect(logger.track).toHaveBeenLastCalledWith('usage', 'deeplink', { - query: 'flipper://test', - state: 'ERROR', - errorMessage: 'Unknown deeplink', - }); + expect(logger.track).toHaveBeenLastCalledWith( + 'usage', + 'deeplink', + { + query: 'flipper://test', + state: 'ERROR', + errorMessage: 'Unknown deeplink', + }, + undefined, + ); }); test('Will throw error on invalid protocol', async () => { @@ -116,11 +121,16 @@ test('Will throw error on invalid protocol', async () => { ).rejects.toThrowErrorMatchingInlineSnapshot(`"Unknown deeplink"`); expect(logger.track).toHaveBeenCalledTimes(2); - expect(logger.track).toHaveBeenLastCalledWith('usage', 'deeplink', { - query: 'notflipper://test', - state: 'ERROR', - errorMessage: 'Unknown deeplink', - }); + expect(logger.track).toHaveBeenLastCalledWith( + 'usage', + 'deeplink', + { + query: 'notflipper://test', + state: 'ERROR', + errorMessage: 'Unknown deeplink', + }, + undefined, + ); }); test('Will track deeplinks', async () => { @@ -142,9 +152,14 @@ test('Will track deeplinks', async () => { 'flipper://open-plugin?plugin-id=TestPlugin&client=TestApp&payload=universe', ); - expect(logger.track).toHaveBeenCalledWith('usage', 'deeplink', { - query: - 'flipper://open-plugin?plugin-id=TestPlugin&client=TestApp&payload=universe', - state: 'INIT', - }); + expect(logger.track).toHaveBeenCalledWith( + 'usage', + 'deeplink', + { + query: + 'flipper://open-plugin?plugin-id=TestPlugin&client=TestApp&payload=universe', + state: 'INIT', + }, + undefined, + ); }); diff --git a/desktop/app/src/deeplink.tsx b/desktop/app/src/deeplink.tsx index 209deb2b4..09de7b29f 100644 --- a/desktop/app/src/deeplink.tsx +++ b/desktop/app/src/deeplink.tsx @@ -20,22 +20,7 @@ import {selectPlugin, getAllClients} from './reducers/connections'; import {Dialog} from 'flipper-plugin'; import {handleOpenPluginDeeplink} from './dispatcher/handleOpenPluginDeeplink'; import {message} from 'antd'; - -type DeeplinkInteraction = { - state: 'INIT' | 'ERROR'; - errorMessage?: string; -}; - -function track( - logger: Logger, - query: string, - interaction: DeeplinkInteraction, -) { - logger.track('usage', 'deeplink', { - ...interaction, - query, - }); -} +import {track} from './deeplinkTracking'; const UNKNOWN = 'Unknown deeplink'; /** @@ -63,7 +48,7 @@ export async function handleDeeplink( throw unknownError(); } if (uri.href.startsWith('flipper://open-plugin')) { - return handleOpenPluginDeeplink(store, query); + return handleOpenPluginDeeplink(store, query, trackInteraction); } if (uri.pathname.match(/^\/*import\/*$/)) { const url = uri.searchParams.get('url'); diff --git a/desktop/app/src/deeplinkTracking.tsx b/desktop/app/src/deeplinkTracking.tsx new file mode 100644 index 000000000..da4165f20 --- /dev/null +++ b/desktop/app/src/deeplinkTracking.tsx @@ -0,0 +1,48 @@ +/** + * 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 {Logger} from './fb-interfaces/Logger'; + +export type OpenPluginParams = { + pluginId: string; + client: string | undefined; + devices: string[]; + payload: string | undefined; +}; + +export type DeeplinkInteraction = { + state: + | 'INIT' + | 'ERROR' + | 'PLUGIN_LIGHTHOUSE_BAIL' + | 'PLUGIN_STATUS_BAIL' + | 'PLUGIN_DEVICE_BAIL' + | 'PLUGIN_DEVICE_UNSUPPORTED' + | 'PLUGIN_CLIENT_UNSUPPORTED' + | 'PLUGIN_OPEN_SUCCESS'; + errorMessage?: string; + plugin?: OpenPluginParams; + extra?: object; +}; + +export function track( + logger: Logger, + query: string, + interaction: DeeplinkInteraction, +) { + logger.track( + 'usage', + 'deeplink', + { + ...interaction, + query, + }, + interaction.plugin?.pluginId, + ); +} diff --git a/desktop/app/src/dispatcher/__tests__/handleOpenPluginDeeplink.node.tsx b/desktop/app/src/dispatcher/__tests__/handleOpenPluginDeeplink.node.tsx index e5cc77ffd..6ac5c541b 100644 --- a/desktop/app/src/dispatcher/__tests__/handleOpenPluginDeeplink.node.tsx +++ b/desktop/app/src/dispatcher/__tests__/handleOpenPluginDeeplink.node.tsx @@ -19,11 +19,25 @@ import { createState, useValue, DevicePluginClient, + Dialog, } from 'flipper-plugin'; import {parseOpenPluginParams} from '../handleOpenPluginDeeplink'; import {handleDeeplink} from '../../deeplink'; import {selectPlugin} from '../../reducers/connections'; +let origAlertImpl: any; +let origConfirmImpl: any; + +beforeEach(() => { + origAlertImpl = Dialog.alert; + origConfirmImpl = Dialog.confirm; +}); + +afterEach(() => { + Dialog.alert = origAlertImpl; + Dialog.confirm = origConfirmImpl; +}); + test('open-plugin deeplink parsing', () => { const testpayload = 'http://www.google/?test=c o%20o+l'; const testLink = @@ -83,6 +97,7 @@ test('Triggering a deeplink will work', async () => { const {renderer, client, store, logger} = await renderMockFlipperWithPlugin( definition, ); + logger.track = jest.fn(); expect(linksSeen).toEqual([]); @@ -120,6 +135,33 @@ test('Triggering a deeplink will work', async () => { `); + expect(logger.track).toHaveBeenCalledTimes(2); + expect(logger.track).toHaveBeenCalledWith( + 'usage', + 'deeplink', + { + query: + 'flipper://open-plugin?plugin-id=TestPlugin&client=TestApp&payload=universe', + state: 'INIT', + }, + undefined, + ); + expect(logger.track).toHaveBeenCalledWith( + 'usage', + 'deeplink', + { + query: + 'flipper://open-plugin?plugin-id=TestPlugin&client=TestApp&payload=universe', + state: 'PLUGIN_OPEN_SUCCESS', + plugin: { + client: 'TestApp', + devices: [], + payload: 'universe', + pluginId: 'TestPlugin', + }, + }, + 'TestPlugin', + ); }); test('triggering a deeplink without applicable device can wait for a device', async () => { @@ -283,3 +325,63 @@ test('triggering a deeplink without applicable client can wait for a device', as `); }); + +test('triggering a deeplink with incompatible device will cause bail', async () => { + const definition = TestUtils.createTestDevicePlugin( + { + Component() { + return

Hello

; + }, + devicePlugin() { + return {}; + }, + }, + { + id: 'DevicePlugin', + supportedDevices: [{os: 'iOS'}], + }, + ); + const {store, logger, createDevice} = await renderMockFlipperWithPlugin( + definition, + ); + logger.track = jest.fn(); + + // Skipping user interactions. + Dialog.alert = (async () => {}) as any; + Dialog.confirm = (async () => {}) as any; + + store.dispatch( + selectPlugin({selectedPlugin: 'nonexisting', deepLinkPayload: null}), + ); + + const handlePromise = handleDeeplink( + store, + logger, + `flipper://open-plugin?plugin-id=${definition.id}&devices=iOS`, + ); + + jest.runAllTimers(); + + // create a new device that doesn't match spec + createDevice({serial: 'device2', os: 'Android'}); + + // wait for dialogues + await handlePromise; + + expect(logger.track).toHaveBeenCalledTimes(2); + expect(logger.track).toHaveBeenCalledWith( + 'usage', + 'deeplink', + { + plugin: { + client: undefined, + devices: ['iOS'], + payload: undefined, + pluginId: 'DevicePlugin', + }, + query: 'flipper://open-plugin?plugin-id=DevicePlugin&devices=iOS', + state: 'PLUGIN_DEVICE_BAIL', + }, + 'DevicePlugin', + ); +}); diff --git a/desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx b/desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx index 8714ad61a..94b69d3f1 100644 --- a/desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx +++ b/desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx @@ -18,7 +18,7 @@ import {UserNotSignedInError} from '../utils/errors'; import {selectPlugin, setPluginEnabled} from '../reducers/connections'; import {getUpdateAvailableMessage} from '../chrome/UpdateIndicator'; import {Typography} from 'antd'; -import {getPluginStatus} from '../utils/pluginUtils'; +import {getPluginStatus, PluginStatus} from '../utils/pluginUtils'; import {loadPluginsFromMarketplace} from './fb-stubs/pluginMarketplace'; import {loadPlugin, switchPlugin} from '../reducers/pluginManager'; import {startPluginDownload} from '../reducers/pluginDownloads'; @@ -29,13 +29,7 @@ import Client from '../Client'; import {RocketOutlined} from '@ant-design/icons'; import {showEmulatorLauncher} from '../sandy-chrome/appinspect/LaunchEmulator'; import {getAllClients} from '../reducers/connections'; - -type OpenPluginParams = { - pluginId: string; - client: string | undefined; - devices: string[]; - payload: string | undefined; -}; +import {DeeplinkInteraction, OpenPluginParams} from '../deeplinkTracking'; export function parseOpenPluginParams(query: string): OpenPluginParams { // 'flipper://open-plugin?plugin-id=graphql&client=facebook&devices=android,ios&chrome=1&payload=' @@ -54,15 +48,33 @@ export function parseOpenPluginParams(query: string): OpenPluginParams { }; } -export async function handleOpenPluginDeeplink(store: Store, query: string) { +export async function handleOpenPluginDeeplink( + store: Store, + query: string, + trackInteraction: (interaction: DeeplinkInteraction) => void, +) { const params = parseOpenPluginParams(query); const title = `Opening plugin ${params.pluginId}…`; if (!(await verifyLighthouseAndUserLoggedIn(store, title))) { + trackInteraction({ + state: 'PLUGIN_LIGHTHOUSE_BAIL', + plugin: params, + }); return; } await verifyFlipperIsUpToDate(title); - if (!(await verifyPluginStatus(store, params.pluginId, title))) { + const [pluginStatusResult, pluginStatus] = await verifyPluginStatus( + store, + params.pluginId, + title, + ); + if (!pluginStatusResult) { + trackInteraction({ + state: 'PLUGIN_STATUS_BAIL', + plugin: params, + extra: {pluginStatus}, + }); return; } @@ -79,6 +91,10 @@ export async function handleOpenPluginDeeplink(store: Store, query: string) { isDevicePlugin, ); if (deviceOrClient === false) { + trackInteraction({ + state: 'PLUGIN_DEVICE_BAIL', + plugin: params, + }); return; } const client: Client | undefined = isDevicePlugin @@ -95,6 +111,11 @@ export async function handleOpenPluginDeeplink(store: Store, query: string) { type: 'error', message: `This plugin is not supported by device ${device.displayTitle()}`, }); + trackInteraction({ + state: 'PLUGIN_DEVICE_UNSUPPORTED', + plugin: params, + extra: {device: device.displayTitle()}, + }); return; } if (!isDevicePlugin && !client!.plugins.has(params.pluginId)) { @@ -103,6 +124,12 @@ export async function handleOpenPluginDeeplink(store: Store, query: string) { type: 'error', message: `This plugin is not supported by client ${client!.query.app}`, }); + trackInteraction({ + state: 'PLUGIN_CLIENT_UNSUPPORTED', + plugin: params, + extra: {client: client!.query.app}, + }); + return; } // verify plugin enabled @@ -137,6 +164,10 @@ export async function handleOpenPluginDeeplink(store: Store, query: string) { }), ); } + trackInteraction({ + state: 'PLUGIN_OPEN_SUCCESS', + plugin: params, + }); } // check if user is connected to VPN and logged in. Returns true if OK, or false if aborted @@ -269,7 +300,7 @@ async function verifyPluginStatus( store: Store, pluginId: string, title: string, -): Promise { +): Promise<[boolean, PluginStatus]> { // make sure we have marketplace plugin data present if (!isTest() && !store.getState().plugins.marketplacePlugins.length) { // plugins not yet fetched @@ -281,21 +312,21 @@ async function verifyPluginStatus( const [status, reason] = getPluginStatus(store, pluginId); switch (status) { case 'ready': - return true; + return [true, status]; case 'unknown': await Dialog.alert({ type: 'warning', title, message: `No plugin with id '${pluginId}' is known to Flipper. Please correct the deeplink, or install the plugin from NPM using the plugin manager.`, }); - return false; + return [false, status]; case 'failed': await Dialog.alert({ type: 'error', title, message: `We found plugin '${pluginId}', but failed to load it: ${reason}. Please check the logs for more details`, }); - return false; + return [false, status]; case 'gatekeeped': if ( !(await Dialog.confirm({ @@ -318,7 +349,7 @@ async function verifyPluginStatus( }, })) ) { - return false; + return [false, status]; } break; case 'bundle_installable': { @@ -328,7 +359,7 @@ async function verifyPluginStatus( } case 'marketplace_installable': { if (!(await installMarketPlacePlugin(store, pluginId, title))) { - return false; + return [false, status]; } break; } diff --git a/desktop/app/src/utils/pluginUtils.tsx b/desktop/app/src/utils/pluginUtils.tsx index 7d7fb1189..4d6212147 100644 --- a/desktop/app/src/utils/pluginUtils.tsx +++ b/desktop/app/src/utils/pluginUtils.tsx @@ -402,19 +402,18 @@ export function computeActivePluginList({ return pluginList; } +export type PluginStatus = + | 'ready' + | 'unknown' + | 'failed' + | 'gatekeeped' + | 'bundle_installable' + | 'marketplace_installable'; + export function getPluginStatus( store: Store, id: string, -): [ - state: - | 'ready' - | 'unknown' - | 'failed' - | 'gatekeeped' - | 'bundle_installable' - | 'marketplace_installable', - reason?: string, -] { +): [state: PluginStatus, reason?: string] { const state: PluginsState = store.getState().plugins; if (state.devicePlugins.has(id) || state.clientPlugins.has(id)) { return ['ready'];