From 313d0e42b81b69b57aa81a760465a414e6afd7f7 Mon Sep 17 00:00:00 2001 From: Olivier Freyssinet Date: Wed, 7 Aug 2024 16:46:32 +0200 Subject: [PATCH] fix(MyLedger): filtering of apps depending on feature flag --- .changeset/khaki-melons-exist.md | 7 + .../DeviceDashboard/AppsList/AppActions.tsx | 6 +- .../manager/DeviceDashboard/AppsList/Item.tsx | 22 ++- .../DeviceDashboard/AppsList/index.tsx | 21 +-- .../src/screens/MyLedgerDevice/AppsScreen.tsx | 17 +-- .../src/apps/filtering.test.ts | 130 +++++++++++++++++- libs/ledger-live-common/src/apps/filtering.ts | 70 ++++++---- libs/ledger-live-common/src/apps/react.ts | 13 +- 8 files changed, 208 insertions(+), 78 deletions(-) create mode 100644 .changeset/khaki-melons-exist.md diff --git a/.changeset/khaki-melons-exist.md b/.changeset/khaki-melons-exist.md new file mode 100644 index 000000000000..a93ab75f857f --- /dev/null +++ b/.changeset/khaki-melons-exist.md @@ -0,0 +1,7 @@ +--- +"ledger-live-desktop": patch +"live-mobile": patch +"@ledgerhq/live-common": patch +--- + +My Ledger: fix & refactor logic of "is app supported by Ledger Live" depending on feature flags, and associated filters in apps catalog. Now this logic is in one single place and fully unit tested. diff --git a/apps/ledger-live-desktop/src/renderer/screens/manager/DeviceDashboard/AppsList/AppActions.tsx b/apps/ledger-live-desktop/src/renderer/screens/manager/DeviceDashboard/AppsList/AppActions.tsx index c7a670ad54d4..c2f78021480e 100644 --- a/apps/ledger-live-desktop/src/renderer/screens/manager/DeviceDashboard/AppsList/AppActions.tsx +++ b/apps/ledger-live-desktop/src/renderer/screens/manager/DeviceDashboard/AppsList/AppActions.tsx @@ -61,7 +61,6 @@ type Props = { setAppUninstallDep?: (a: { dependents: App[]; app: App }) => void; isLiveSupported: boolean; addAccount?: () => void; - featureFlagActivated: boolean; }; // eslint-disable-next-line react/display-name @@ -80,7 +79,6 @@ const AppActions = React.memo( setAppUninstallDep, isLiveSupported, addAccount, - featureFlagActivated, }: Props) => { const { name, type } = app; @@ -141,7 +139,7 @@ const AppActions = React.memo( // FIXME: given the heavy use of conditions in the rendering logic below, // it would be better to derive this value from the same rendering logic... - const hasTwoItems = showLearnMore || (hasSpecificAction && installed && featureFlagActivated); + const hasTwoItems = showLearnMore || (hasSpecificAction && installed); return ( @@ -182,7 +180,7 @@ const AppActions = React.memo( /> ) : showActions ? ( <> - {installed && featureFlagActivated ? ( + {installed ? ( isCurrencyApp && isLiveSupported ? ( { - const { name, type, currencyId, authorName } = app; + const { name, authorName } = app; const { deviceModel, deviceInfo } = state; const notEnoughMemoryToInstall = useNotEnoughMemoryToInstall(optimisticState, name); const currency = useMemo( @@ -109,13 +109,8 @@ const Item = ({ [app.name, state.apps], ); - // FIXME No explicit mapping between Currency.id and FeatureId - const flag = useFeature(camelCase(`currency_${currencyId}`) as FeatureId); - - // when the flag doesn't exist it's equivalent to being enabled - const currencyFlagEnabled = !flag || flag.enabled; - const currencySupported = !!currency && isCurrencySupported(currency) && currencyFlagEnabled; - const isLiveSupported = currencySupported || ["swap", "plugin"].includes(type); + const { getFeature, isFeature } = useFeatureFlags(); + const isLiveSupported = isAppAssociatedCurrencySupported({ app, isFeature, getFeature }); const bytes = useMemo( () => @@ -169,7 +164,7 @@ const Item = ({ - {isLiveSupported && currencyFlagEnabled ? ( + {isLiveSupported ? ( <> @@ -204,7 +199,6 @@ const Item = ({ setAppUninstallDep={setAppUninstallDep} isLiveSupported={isLiveSupported} addAccount={onAddAccount} - featureFlagActivated={currencyFlagEnabled} /> ); diff --git a/apps/ledger-live-desktop/src/renderer/screens/manager/DeviceDashboard/AppsList/index.tsx b/apps/ledger-live-desktop/src/renderer/screens/manager/DeviceDashboard/AppsList/index.tsx index d9e34497b2e6..363aa7ef1ff8 100644 --- a/apps/ledger-live-desktop/src/renderer/screens/manager/DeviceDashboard/AppsList/index.tsx +++ b/apps/ledger-live-desktop/src/renderer/screens/manager/DeviceDashboard/AppsList/index.tsx @@ -1,4 +1,4 @@ -import React, { useState, memo, useCallback, useEffect, useRef, useMemo } from "react"; +import React, { useState, memo, useCallback, useEffect, useRef } from "react"; import { useLocation, useHistory } from "react-router-dom"; import { useDispatch, useSelector } from "react-redux"; import styled from "styled-components"; @@ -21,12 +21,10 @@ import { openModal } from "~/renderer/actions/modals"; import debounce from "lodash/debounce"; import InstallSuccessBanner from "./InstallSuccessBanner"; import SearchBox from "../../../accounts/AccountList/SearchBox"; -import { App, FeatureId } from "@ledgerhq/types-live"; +import { App } from "@ledgerhq/types-live"; import { AppType, SortOptions } from "@ledgerhq/live-common/apps/filtering"; import NoResults from "~/renderer/icons/NoResults"; import { CryptoOrTokenCurrency } from "@ledgerhq/types-cryptoassets"; -import { useFeatureFlags } from "@ledgerhq/live-common/featureFlags/index"; -import camelCase from "lodash/camelCase"; import { getEnv } from "@ledgerhq/live-env"; // sticky top bar with extra width to cover card boxshadow underneath @@ -128,17 +126,6 @@ const AppsList = ({ const displayedAppList = isDeviceTab ? device : catalog; - const { getFeature } = useFeatureFlags(); - const enabledAppList = useMemo( - () => - displayedAppList.filter(({ currencyId }: App) => { - if (!currencyId) return true; - const currencyFeatureKey = camelCase(`currency_${currencyId}`) as FeatureId; - return getFeature(currencyFeatureKey)?.enabled ?? true; - }), - [displayedAppList, getFeature], - ); - const mapApp = useCallback( (app: App, appStoreView: boolean, onlyUpdate?: boolean, showActions?: boolean) => { return ( @@ -238,8 +225,8 @@ const AppsList = ({ /> )} - {enabledAppList.length ? ( - enabledAppList.map((app: App) => mapApp(app, !isDeviceTab)) + {displayedAppList.length ? ( + displayedAppList.map((app: App) => mapApp(app, !isDeviceTab)) ) : ( - catalog.filter(({ currencyId }: App) => { - if (!currencyId) return true; - const currencyFeatureKey = camelCase(`currency_${currencyId}`) as FeatureId; - return isFeature(currencyFeatureKey) ? getFeature(currencyFeatureKey)?.enabled : true; - }), - [catalog, getFeature, isFeature], - ); - return ( diff --git a/libs/ledger-live-common/src/apps/filtering.test.ts b/libs/ledger-live-common/src/apps/filtering.test.ts index 55f7d83a1237..446bfe2fc223 100644 --- a/libs/ledger-live-common/src/apps/filtering.test.ts +++ b/libs/ledger-live-common/src/apps/filtering.test.ts @@ -1,13 +1,19 @@ import { initState } from "./logic"; import { deviceInfo155, mockListAppsResult } from "./mock"; import type { FilterOptions, SortOptions } from "./filtering"; -import { sortFilterApps } from "./filtering"; +import { sortFilterApps, isAppAssociatedCurrencySupported } from "./filtering"; import { setSupportedCurrencies } from "../currencies/index"; import { WALLET_API_VERSION } from "../wallet-api/constants"; import { setWalletAPIVersion } from "../wallet-api/version"; +import { App } from "@ledgerhq/types-live"; setWalletAPIVersion(WALLET_API_VERSION); +const mockedFeatureFlags = { + isFeature: () => false, + getFeature: () => null, +}; + type FilteringScenario = { name: string; apps: string; @@ -38,6 +44,7 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { installedApps: [], type: [], + ...mockedFeatureFlags, }, expectedApps: "Bitcoin, Ethereum, Litecoin, Dogecoin, HODL, Password Manager, Ethereum Classic, XRP, Stellar", @@ -53,6 +60,7 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { installedApps: [], type: [], + ...mockedFeatureFlags, }, expectedApps: "Bitcoin, Ethereum, XRP, Litecoin, Stellar, Ethereum Classic, Dogecoin, Password Manager, HODL", @@ -68,6 +76,7 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { installedApps: [], type: [], + ...mockedFeatureFlags, }, expectedApps: "Dogecoin, Ethereum Classic, Stellar, Litecoin, XRP, Ethereum, Bitcoin, HODL, Password Manager", @@ -83,6 +92,7 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { installedApps: [], type: [], + ...mockedFeatureFlags, }, expectedApps: "XRP, Stellar, Password Manager, Litecoin, HODL, Ethereum Classic, Ethereum, Dogecoin, Bitcoin", @@ -98,6 +108,7 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { installedApps: [], type: [], + ...mockedFeatureFlags, }, expectedApps: "Bitcoin, Dogecoin, Ethereum, Ethereum Classic, HODL, Litecoin, Password Manager, Stellar, XRP", @@ -113,9 +124,32 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { type: ["supported"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "Bitcoin, Dogecoin, Ethereum, Ethereum Classic, Litecoin, XRP", }, + { + name: "Catalog - Only apps supported by Live, feature flagged currencies", + apps, + installed: "", + _sortOptions: { + type: "name", + order: "asc", + }, + _filterOptions: { + type: ["supported"], + installedApps: [], + isFeature() { + return true; + }, + getFeature(key: string) { + if (key === "currencyLitecoin") return { enabled: true }; + if (key === "currencyDogecoin") return { enabled: false }; + return null; + }, + }, + expectedApps: "Bitcoin, Ethereum, Ethereum Classic, Litecoin, XRP", + }, { name: "Catalog - Only apps not supported by Live", apps, @@ -127,6 +161,7 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { type: ["not_supported"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "HODL, Password Manager, Stellar", }, @@ -141,6 +176,7 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { type: ["updatable"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "Bitcoin, Dogecoin", }, @@ -155,6 +191,7 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { type: ["installed", "not_supported"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "HODL, Stellar", }, @@ -169,6 +206,7 @@ const scenarios: FilteringScenario[] = [ _filterOptions: { type: ["installed"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "Stellar, Ethereum, Bitcoin", }, @@ -181,6 +219,7 @@ const scenarios: FilteringScenario[] = [ query: "IMNOTHERE", type: ["installed"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "", }, @@ -193,6 +232,7 @@ const scenarios: FilteringScenario[] = [ type: ["installed"], installQueue: ["XRP", "Dogecoin"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "XRP, Stellar, Ethereum, Dogecoin, Bitcoin", }, @@ -208,6 +248,7 @@ const scenarios: FilteringScenario[] = [ query: "ethereum", type: ["not_installed"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "Ethereum, Ethereum Classic", }, @@ -223,6 +264,7 @@ const scenarios: FilteringScenario[] = [ query: "", type: ["installed", "supported"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "Bitcoin, Ethereum", }, @@ -238,6 +280,7 @@ const scenarios: FilteringScenario[] = [ query: "", type: ["not_installed", "installed"], installedApps: [], + ...mockedFeatureFlags, }, expectedApps: "", }, @@ -257,3 +300,88 @@ scenarios.forEach(scenario => { ); }); }); + +type AppSupportedScenario = { + name: string; + expected: boolean; + params: Parameters[0]; +}; + +const appSupportedScenarios: Array = [ + { + name: "App is a plugin", + expected: true, + params: { app: { type: "plugin" } as App, ...mockedFeatureFlags }, + }, + { + name: "App is swap", + expected: true, + params: { app: { type: "swap" } as App, ...mockedFeatureFlags }, + }, + { + name: "App has no currencyId", + expected: false, + params: { app: {} as App, ...mockedFeatureFlags }, + }, + { + name: "App's associated currency is not supported", + expected: false, + params: { + app: { currencyId: "stellar" } as App, + ...mockedFeatureFlags, + }, + }, + { + name: "App's associated currency has no feature flag", + expected: true, + params: { + app: { currencyId: "bitcoin" } as App, + ...mockedFeatureFlags, + isFeature: () => false, + }, + }, + { + name: "App's associated currency has a feature flag that disables it", + expected: false, + params: { + app: { currencyId: "bitcoin" } as App, + isFeature: () => true, + getFeature: (key: string) => { + if (key === "currencyBitcoin") return { enabled: false }; + throw new Error("wrong featureId key generated: " + key); + }, + }, + }, + { + name: "App's associated currency has a feature flag that enables it", + expected: true, + params: { + app: { currencyId: "bitcoin" } as App, + isFeature: () => true, + getFeature: (key: string) => { + if (key === "currencyBitcoin") return { enabled: true }; + throw new Error("wrong featureId key generate: " + key); + }, + }, + }, + { + name: "App's associated currency has a feature flag that is null (will never happen but typewise it can happen)", + expected: true, + params: { + app: { currencyId: "bitcoin" } as App, + isFeature: () => true, + getFeature: (key: string) => { + if (key === "currencyBitcoin") return null; + throw new Error("wrong featureId key generate: " + key); + }, + }, + }, +]; + +describe("isAppAssociatedCurrencySupported", () => { + appSupportedScenarios.forEach(scenario => { + test(`Scenario: ${scenario.name} (expect ${scenario.expected})`, () => { + expect(isAppAssociatedCurrencySupported(scenario.params)).toBe(scenario.expected); + }); + }); +}); diff --git a/libs/ledger-live-common/src/apps/filtering.ts b/libs/ledger-live-common/src/apps/filtering.ts index 30a7aa442785..6ecc4bde9af4 100644 --- a/libs/ledger-live-common/src/apps/filtering.ts +++ b/libs/ledger-live-common/src/apps/filtering.ts @@ -1,7 +1,8 @@ +import camelCase from "lodash/fp/camelCase"; import type { InstalledItem } from "./types"; import { getCryptoCurrencyById, isCurrencySupported } from "../currencies"; import { useMemo } from "react"; -import type { App } from "@ledgerhq/types-live"; +import type { App, Feature, FeatureId } from "@ledgerhq/types-live"; export type SortOptions = { type?: "name" | "marketcap" | "default"; @@ -21,6 +22,8 @@ export type FilterOptions = { installQueue?: string[]; installedApps: InstalledItem[]; type: AppType[]; + isFeature: (key: string) => boolean; + getFeature: (key: FeatureId) => Feature | null; }; type UpdateAwareInstalledApps = Record; @@ -39,13 +42,40 @@ const searchFilter = return queries.some(query => terms.toLowerCase().includes(query)); }; +/** + * Checks if the app's associated currency is supported in Ledger Live. + * + * It boils down to: if the app has a currencyId, check if the currency is supported. + * The currency can be unsupported if there is a feature flag that disables it. + */ +export function isAppAssociatedCurrencySupported({ + app, + isFeature, + getFeature, +}: { + app: App; + isFeature: (key: string) => boolean; + getFeature: (key: FeatureId) => Feature | null; +}): boolean { + if (["swap", "plugin"].includes(app.type)) return true; + if (!app.currencyId) return false; + if (!isCurrencySupported(getCryptoCurrencyById(app.currencyId))) return false; + + const currencyFeatureKey = camelCase(`currency_${app.currencyId}`) as FeatureId; + if (!isFeature(currencyFeatureKey)) return true; // no associated feature flag, the currency is supported + if (getFeature(currencyFeatureKey)?.enabled === false) return false; + return true; +} + function typeFilter( filters: AppType[] = ["all"], updateAwareInstalledApps: UpdateAwareInstalledApps, installQueue: string[] = [], + isFeature: (key: string) => boolean, + getFeature: (key: FeatureId) => Feature | null, ) { - return (app: App): boolean => { - return filters.every(filter => { + return (app: App): boolean => + filters.every(filter => { switch (filter) { case "installed": return installQueue.includes(app.name) || app.name in updateAwareInstalledApps; @@ -54,14 +84,13 @@ function typeFilter( case "updatable": return app.name in updateAwareInstalledApps && !updateAwareInstalledApps[app.name]; case "supported": - return app.currencyId && isCurrencySupported(getCryptoCurrencyById(app.currencyId)); + return isAppAssociatedCurrencySupported({ app, isFeature, getFeature }); case "not_supported": - return !app.currencyId || !isCurrencySupported(getCryptoCurrencyById(app.currencyId)); + return !isAppAssociatedCurrencySupported({ app, isFeature, getFeature }); default: return true; } }); - }; } export const sortApps = (apps: App[], _options: SortOptions): App[] => { @@ -90,7 +119,15 @@ export const filterApps = (apps: App[], _options: FilterOptions): App[] => { return apps .filter(searchFilter(query)) - .filter(typeFilter(type, updateAwareInstalledApps, installQueue)); + .filter( + typeFilter( + type, + updateAwareInstalledApps, + installQueue, + _options.isFeature, + _options.getFeature, + ), + ); }; export const sortFilterApps = ( @@ -106,23 +143,8 @@ export const useSortedFilteredApps = ( _filterOptions: FilterOptions, _sortOptions: SortOptions, ): App[] => { - const { query, installedApps, type: filterType, installQueue } = _filterOptions; - const { type: sortType, order } = _sortOptions; return useMemo( - () => - sortFilterApps( - apps, - { - query, - installedApps, - type: filterType, - installQueue, - }, - { - type: sortType, - order, - }, - ), - [apps, query, installedApps, filterType, installQueue, sortType, order], + () => sortFilterApps(apps, _filterOptions, _sortOptions), + [apps, _filterOptions, _sortOptions], ); }; diff --git a/libs/ledger-live-common/src/apps/react.ts b/libs/ledger-live-common/src/apps/react.ts index 2aa139a491a6..d65e02f5e110 100644 --- a/libs/ledger-live-common/src/apps/react.ts +++ b/libs/ledger-live-common/src/apps/react.ts @@ -1,6 +1,6 @@ import { useState, useReducer, useEffect, useMemo } from "react"; import type { Exec, State, Action, ListAppsResult } from "./types"; -import type { AppType, SortOptions } from "./filtering"; +import type { AppType, FilterOptions, SortOptions } from "./filtering"; import { useSortedFilteredApps } from "./filtering"; import { reducer, @@ -11,6 +11,7 @@ import { } from "./logic"; import { runAppOp } from "./runner"; import { App } from "@ledgerhq/types-live"; +import { useFeatureFlags } from "../featureFlags"; type UseAppsRunnerResult = [State, (arg0: Action) => void]; // use for React apps. support dynamic change of the state. @@ -85,17 +86,21 @@ export function useAppsSections(state: State, opts: AppsSectionsOpts): AppsSecti () => apps.filter(({ name }) => installed.some(i => i.name === name && !i.updated)), [apps, installed], ); + const { getFeature, isFeature } = useFeatureFlags(); const update = appsUpdating.length > 0 ? appsUpdating : updatableAppList; - const filterParam = useMemo( + const filterParam: FilterOptions = useMemo( () => ({ query: opts.query, installedApps: installed, type: [opts.appFilter], + getFeature, + isFeature, }), - [installed, opts.appFilter, opts.query], + [getFeature, installed, isFeature, opts.appFilter, opts.query], ); const catalog = useSortedFilteredApps(apps, filterParam, opts.sort); + const installedAppList = useSortedFilteredApps( apps, { @@ -103,6 +108,8 @@ export function useAppsSections(state: State, opts: AppsSectionsOpts): AppsSecti installedApps: installed, installQueue, type: ["installed"], + getFeature, + isFeature, }, { type: "default",