From 876959ebab8f5209b8c982650c658e00559b0a9b Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 18 Oct 2021 22:19:17 +0100 Subject: [PATCH 1/9] adds a search box to the toolbar featureflag list --- frontend/src/toolbar/flags/FeatureFlags.tsx | 154 ++++++++++-------- frontend/src/toolbar/flags/featureFlags.scss | 4 + .../src/toolbar/flags/featureFlagsLogic.ts | 32 +++- 3 files changed, 117 insertions(+), 73 deletions(-) diff --git a/frontend/src/toolbar/flags/FeatureFlags.tsx b/frontend/src/toolbar/flags/FeatureFlags.tsx index b0daf9e32bc0c..5e11bd8c1889d 100644 --- a/frontend/src/toolbar/flags/FeatureFlags.tsx +++ b/frontend/src/toolbar/flags/FeatureFlags.tsx @@ -3,16 +3,18 @@ import './featureFlags.scss' import React from 'react' import { useActions, useValues } from 'kea' import { featureFlagsLogic } from '~/toolbar/flags/featureFlagsLogic' -import { Radio, Switch, Row, Typography, List, Button } from 'antd' +import { Radio, Switch, Row, Typography, List, Button, Input } from 'antd' import { AnimatedCollapsible } from './AnimatedCollapsible' import { PostHog } from 'posthog-js' import { toolbarLogic } from '~/toolbar/toolbarLogic' import { urls } from 'scenes/urls' import { IconExternalLinkBold } from 'lib/components/icons' +import clsx from 'clsx' export function FeatureFlags(): JSX.Element { - const { userFlagsWithCalculatedInfo, showLocalFeatureFlagWarning } = useValues(featureFlagsLogic) - const { setOverriddenUserFlag, deleteOverriddenUserFlag, setShowLocalFeatureFlagWarning } = + const { userFlagsWithCalculatedInfo, showLocalFeatureFlagWarning, searchTerm, isHiddenBySearch } = + useValues(featureFlagsLogic) + const { setOverriddenUserFlag, deleteOverriddenUserFlag, setShowLocalFeatureFlagWarning, setSearchTerm } = useActions(featureFlagsLogic) const { apiURL } = useValues(toolbarLogic) @@ -40,79 +42,99 @@ export function FeatureFlags(): JSX.Element { ) : ( - { - return ( -
- + setSearchTerm(e.target.value)} + /> + { + return ( +
- - {feature_flag.key} - - - - - { - const newValue = - hasVariants && checked - ? (feature_flag.filters?.multivariate?.variants[0]?.key as string) - : checked - if (newValue === value_for_user_without_override && override) { - deleteOverriddenUserFlag(override.id as number) - } else { - setOverriddenUserFlag(feature_flag.id as number, newValue) - } - }} - /> - - - - { - const newValue = event.target.value + + {feature_flag.key} + + + + + { + const newValue = + hasVariants && checked + ? (feature_flag.filters?.multivariate?.variants[0] + ?.key as string) + : checked if (newValue === value_for_user_without_override && override) { deleteOverriddenUserFlag(override.id as number) } else { setOverriddenUserFlag(feature_flag.id as number, newValue) } }} - > - {feature_flag.filters?.multivariate?.variants.map((variant) => ( - - {`${variant.key} - ${variant.name} (${variant.rollout_percentage}%)`} - - ))} - + /> - -
- ) - }} - /> + + + + { + const newValue = event.target.value + if (newValue === value_for_user_without_override && override) { + deleteOverriddenUserFlag(override.id as number) + } else { + setOverriddenUserFlag(feature_flag.id as number, newValue) + } + }} + > + {feature_flag.filters?.multivariate?.variants.map((variant) => ( + + {`${variant.key} - ${variant.name} (${variant.rollout_percentage}%)`} + + ))} + + + +
+ ) + }} + /> + )} ) diff --git a/frontend/src/toolbar/flags/featureFlags.scss b/frontend/src/toolbar/flags/featureFlags.scss index 5b9047233d4bc..f39f9ba1d1602 100644 --- a/frontend/src/toolbar/flags/featureFlags.scss +++ b/frontend/src/toolbar/flags/featureFlags.scss @@ -9,6 +9,10 @@ margin-bottom: 5px; padding: 0 8px 0 8px; + &.hidden { + display: none; + } + .feature-flag-row-header { background-color: #fafafa; border-radius: 4px; diff --git a/frontend/src/toolbar/flags/featureFlagsLogic.ts b/frontend/src/toolbar/flags/featureFlagsLogic.ts index bc4d785258df7..4e556d617f685 100644 --- a/frontend/src/toolbar/flags/featureFlagsLogic.ts +++ b/frontend/src/toolbar/flags/featureFlagsLogic.ts @@ -4,6 +4,7 @@ import { featureFlagsLogicType } from './featureFlagsLogicType' import { PostHog } from 'posthog-js' import { toolbarFetch } from '~/toolbar/utils' import { toolbarLogic } from '~/toolbar/toolbarLogic' +import Fuse from 'fuse.js' export const featureFlagsLogic = kea({ actions: { @@ -11,6 +12,7 @@ export const featureFlagsLogic = kea({ setOverriddenUserFlag: (flagId: number, overrideValue: string | boolean) => ({ flagId, overrideValue }), deleteOverriddenUserFlag: (overrideId: number) => ({ overrideId }), setShowLocalFeatureFlagWarning: (showWarning: boolean) => ({ showWarning }), + setSearchTerm: (searchTerm: string) => ({ searchTerm }), }, loaders: ({ values }) => ({ @@ -23,8 +25,7 @@ export const featureFlagsLogic = kea({ if (!response.ok) { return [] } - const results = await response.json() - return results + return await response.json() }, setOverriddenUserFlag: async ({ flagId, overrideValue }, breakpoint) => { const response = await toolbarFetch( @@ -67,6 +68,12 @@ export const featureFlagsLogic = kea({ ], }), reducers: { + searchTerm: [ + '', + { + setSearchTerm: (_, { searchTerm }) => searchTerm, + }, + ], showLocalFeatureFlagWarning: [ false, { @@ -92,12 +99,23 @@ export const featureFlagsLogic = kea({ }) }, ], - countFlagsOverridden: [ - (s) => [s.userFlags], - (userFlags) => { - return userFlags.filter((flag) => !!flag.override).length - }, + flagNames: [(s) => [s.userFlags], (userFlags) => userFlags.map((uf) => uf.feature_flag.name)], + searchMatches: [ + (s) => [s.searchTerm, s.flagNames], + (searchTerm, flagNames) => + searchTerm + ? new Fuse(flagNames, { + threshold: 0.3, + }) + .search(searchTerm) + .map(({ item }) => item) + : flagNames, + ], + isHiddenBySearch: [ + (s) => [s.searchMatches], + (searchMatches) => (featureFlagName: string) => !searchMatches.includes(featureFlagName), ], + countFlagsOverridden: [(s) => [s.userFlags], (userFlags) => userFlags.filter((flag) => !!flag.override).length], }, events: ({ actions }) => ({ afterMount: () => { From 720322953186ea8840902be3212d014d3435f162 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 19 Oct 2021 08:17:02 +0100 Subject: [PATCH 2/9] test flakes because react is re-rendering between the get and the click. add an assertion to try and slow cypress down to avoid this --- cypress/integration/invitesMembers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/integration/invitesMembers.js b/cypress/integration/invitesMembers.js index da0328f47f320..6540ee212edca 100644 --- a/cypress/integration/invitesMembers.js +++ b/cypress/integration/invitesMembers.js @@ -61,7 +61,7 @@ describe('Invite Signup', () => { cy.get('[data-attr=invite-teammate-button]').first().click() // Enter invite the user cy.get('[data-attr=invite-email-input]').type(`fake+${Math.floor(Math.random() * 10000)}@posthog.com`) - cy.get('[data-attr=invite-team-member-submit]').click() + cy.get('[data-attr=invite-team-member-submit]').should('not.be.disabled').click() // Log in as invited user cy.get('[data-attr=invite-link]') From ed57718546f64780e43eb28dead93f63e078b5ae Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Tue, 19 Oct 2021 11:44:35 +0100 Subject: [PATCH 3/9] move filtering toolbar feature flags to a Set and out of CSS --- frontend/src/toolbar/flags/FeatureFlags.tsx | 13 +++---------- frontend/src/toolbar/flags/featureFlags.scss | 4 ---- frontend/src/toolbar/flags/featureFlagsLogic.ts | 15 +++++++++------ 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/frontend/src/toolbar/flags/FeatureFlags.tsx b/frontend/src/toolbar/flags/FeatureFlags.tsx index 5e11bd8c1889d..3fe31eb6055ab 100644 --- a/frontend/src/toolbar/flags/FeatureFlags.tsx +++ b/frontend/src/toolbar/flags/FeatureFlags.tsx @@ -9,11 +9,9 @@ import { PostHog } from 'posthog-js' import { toolbarLogic } from '~/toolbar/toolbarLogic' import { urls } from 'scenes/urls' import { IconExternalLinkBold } from 'lib/components/icons' -import clsx from 'clsx' export function FeatureFlags(): JSX.Element { - const { userFlagsWithCalculatedInfo, showLocalFeatureFlagWarning, searchTerm, isHiddenBySearch } = - useValues(featureFlagsLogic) + const { showLocalFeatureFlagWarning, searchTerm, filteredFlags } = useValues(featureFlagsLogic) const { setOverriddenUserFlag, deleteOverriddenUserFlag, setShowLocalFeatureFlagWarning, setSearchTerm } = useActions(featureFlagsLogic) const { apiURL } = useValues(toolbarLogic) @@ -52,7 +50,7 @@ export function FeatureFlags(): JSX.Element { onChange={(e) => setSearchTerm(e.target.value)} /> { return ( -
+
({ flagNames: [(s) => [s.userFlags], (userFlags) => userFlags.map((uf) => uf.feature_flag.name)], searchMatches: [ (s) => [s.searchTerm, s.flagNames], - (searchTerm, flagNames) => - searchTerm + (searchTerm, flagNames) => { + const filteredNames = searchTerm ? new Fuse(flagNames, { threshold: 0.3, }) .search(searchTerm) .map(({ item }) => item) - : flagNames, + : flagNames + return new Set(filteredNames) + }, ], - isHiddenBySearch: [ - (s) => [s.searchMatches], - (searchMatches) => (featureFlagName: string) => !searchMatches.includes(featureFlagName), + filteredFlags: [ + (s) => [s.searchMatches, s.userFlagsWithCalculatedInfo], + (searchMatches, userFlagsWithCalculatedInfo) => + userFlagsWithCalculatedInfo.filter((uf) => searchMatches.has(uf.feature_flag.name)), ], countFlagsOverridden: [(s) => [s.userFlags], (userFlags) => userFlags.filter((flag) => !!flag.override).length], }, From 634ded7be70b0ed76f2a9bbf82fca15ead0a1585 Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Wed, 20 Oct 2021 10:16:13 +0200 Subject: [PATCH 4/9] support initKeaTestLogic() with no args --- frontend/src/test/init.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/frontend/src/test/init.ts b/frontend/src/test/init.ts index 6c306b093d483..6f2f362048c42 100644 --- a/frontend/src/test/init.ts +++ b/frontend/src/test/init.ts @@ -9,10 +9,10 @@ export function initKeaTestLogic({ props, onLogic, }: { - logic: LogicWrapper + logic?: LogicWrapper props?: LogicWrapper['props'] onLogic?: (l: BuiltLogic) => any -}): void { +} = {}): void { let builtLogic: BuiltLogic let unmount: () => void @@ -33,13 +33,17 @@ export function initKeaTestLogic({ ;(history as any).pushState = history.push ;(history as any).replaceState = history.replace initKea({ beforePlugins: [testUtilsPlugin], routerLocation: history.location, routerHistory: history }) - builtLogic = logic.build({ ...props }) - await onLogic?.(builtLogic) - unmount = builtLogic.mount() + if (logic) { + builtLogic = logic.build({ ...props }) + await onLogic?.(builtLogic) + unmount = builtLogic.mount() + } }) afterEach(async () => { - unmount() - await expectLogic(logic).toFinishAllListeners() + if (logic) { + unmount() + await expectLogic(logic).toFinishAllListeners() + } }) } From 992a4aa6f6b55a871342c0aab3d31b9638703e39 Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Wed, 20 Oct 2021 10:16:35 +0200 Subject: [PATCH 5/9] fix query-selector-all-deep jest bug --- jest.config.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/jest.config.ts b/jest.config.ts index 7a92e889429c8..0fcf4031741fe 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -182,10 +182,7 @@ export default { // transform: undefined, // An array of regexp pattern strings that are matched against all source file paths, matched files will skip transformation - // transformIgnorePatterns: [ - // "/node_modules/", - // "\\.pnp\\.[^\\/]+$" - // ], + transformIgnorePatterns: ['node_modules/(?!(query-selector-shadow-dom)/)'], // An array of regexp pattern strings that are matched against all modules before the module loader will automatically return a mock for them // unmockedModulePathPatterns: undefined, From 7e93b41bb7600192b58eb25f05943c046ad28639 Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Wed, 20 Oct 2021 10:16:47 +0200 Subject: [PATCH 6/9] add simple test case for feature flags logic --- .../toolbar/flags/featureFlagsLogic.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 frontend/src/toolbar/flags/featureFlagsLogic.test.ts diff --git a/frontend/src/toolbar/flags/featureFlagsLogic.test.ts b/frontend/src/toolbar/flags/featureFlagsLogic.test.ts new file mode 100644 index 0000000000000..4dae0657d1e29 --- /dev/null +++ b/frontend/src/toolbar/flags/featureFlagsLogic.test.ts @@ -0,0 +1,22 @@ +import { expectLogic } from 'kea-test-utils' +import { initKeaTestLogic } from '~/test/init' +import { featureFlagsLogic } from '~/toolbar/flags/featureFlagsLogic' +import { toolbarLogic } from '~/toolbar/toolbarLogic' + +describe('feature flags logic', () => { + let logic: ReturnType + + initKeaTestLogic() + + beforeEach(() => { + toolbarLogic({ apiURL: 'http://localhost' }).mount() + logic = featureFlagsLogic() + logic.mount() + }) + + it('has expected defaults', () => { + expectLogic(logic).toMatchValues({ + userFlags: [], + }) + }) +}) From 93759fc3e425d3bd14427a1a13b322638bd60575 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 20 Oct 2021 11:16:11 +0100 Subject: [PATCH 7/9] combine selectors --- .../src/toolbar/flags/featureFlagsLogic.ts | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/frontend/src/toolbar/flags/featureFlagsLogic.ts b/frontend/src/toolbar/flags/featureFlagsLogic.ts index 0c6efc68cd17a..46e963e3cb585 100644 --- a/frontend/src/toolbar/flags/featureFlagsLogic.ts +++ b/frontend/src/toolbar/flags/featureFlagsLogic.ts @@ -99,24 +99,21 @@ export const featureFlagsLogic = kea({ }) }, ], - flagNames: [(s) => [s.userFlags], (userFlags) => userFlags.map((uf) => uf.feature_flag.name)], - searchMatches: [ - (s) => [s.searchTerm, s.flagNames], - (searchTerm, flagNames) => { - const filteredNames = searchTerm - ? new Fuse(flagNames, { - threshold: 0.3, - }) - .search(searchTerm) - .map(({ item }) => item) - : flagNames - return new Set(filteredNames) - }, - ], filteredFlags: [ - (s) => [s.searchMatches, s.userFlagsWithCalculatedInfo], - (searchMatches, userFlagsWithCalculatedInfo) => - userFlagsWithCalculatedInfo.filter((uf) => searchMatches.has(uf.feature_flag.name)), + (s) => [s.searchTerm, s.userFlagsWithCalculatedInfo], + (searchTerm, userFlagsWithCalculatedInfo) => { + const flagNames = userFlagsWithCalculatedInfo.map((uf) => uf.feature_flag.name) + const filteredNames = new Set( + searchTerm + ? new Fuse(flagNames, { + threshold: 0.3, + }) + .search(searchTerm) + .map(({ item }) => item) + : flagNames + ) + return userFlagsWithCalculatedInfo.filter((uf) => filteredNames.has(uf.feature_flag.name)) + }, ], countFlagsOverridden: [(s) => [s.userFlags], (userFlags) => userFlags.filter((flag) => !!flag.override).length], }, From d3153e0c5031a3838b7264895c4cf33146e2f08f Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 20 Oct 2021 13:52:33 +0100 Subject: [PATCH 8/9] with more understanding of Fuse --- .../src/toolbar/flags/featureFlagsLogic.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/frontend/src/toolbar/flags/featureFlagsLogic.ts b/frontend/src/toolbar/flags/featureFlagsLogic.ts index 46e963e3cb585..d287803b25626 100644 --- a/frontend/src/toolbar/flags/featureFlagsLogic.ts +++ b/frontend/src/toolbar/flags/featureFlagsLogic.ts @@ -102,17 +102,14 @@ export const featureFlagsLogic = kea({ filteredFlags: [ (s) => [s.searchTerm, s.userFlagsWithCalculatedInfo], (searchTerm, userFlagsWithCalculatedInfo) => { - const flagNames = userFlagsWithCalculatedInfo.map((uf) => uf.feature_flag.name) - const filteredNames = new Set( - searchTerm - ? new Fuse(flagNames, { - threshold: 0.3, - }) - .search(searchTerm) - .map(({ item }) => item) - : flagNames - ) - return userFlagsWithCalculatedInfo.filter((uf) => filteredNames.has(uf.feature_flag.name)) + return searchTerm + ? new Fuse(userFlagsWithCalculatedInfo, { + threshold: 0.3, + keys: ['feature_flag.name'], + }) + .search(searchTerm) + .map(({ item }) => item) + : userFlagsWithCalculatedInfo }, ], countFlagsOverridden: [(s) => [s.userFlags], (userFlags) => userFlags.filter((flag) => !!flag.override).length], From 2b48d5208fcf6faa49e8e81d39cdb1cda7c52205 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 20 Oct 2021 22:49:39 +0100 Subject: [PATCH 9/9] add simple test for flag filtering --- .../toolbar/flags/featureFlagsLogic.test.ts | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/frontend/src/toolbar/flags/featureFlagsLogic.test.ts b/frontend/src/toolbar/flags/featureFlagsLogic.test.ts index 4dae0657d1e29..bcdc60b3294ba 100644 --- a/frontend/src/toolbar/flags/featureFlagsLogic.test.ts +++ b/frontend/src/toolbar/flags/featureFlagsLogic.test.ts @@ -2,6 +2,24 @@ import { expectLogic } from 'kea-test-utils' import { initKeaTestLogic } from '~/test/init' import { featureFlagsLogic } from '~/toolbar/flags/featureFlagsLogic' import { toolbarLogic } from '~/toolbar/toolbarLogic' +import { CombinedFeatureFlagAndOverrideType } from '~/types' + +const featureFlags = [ + { feature_flag: { name: 'flag 1' } }, + { feature_flag: { name: 'flag 2' } }, +] as CombinedFeatureFlagAndOverrideType[] + +const featureFlagsWithExtraInfo = [ + { currentValue: undefined, hasVariants: false, feature_flag: { name: 'flag 1' } }, + { currentValue: undefined, hasVariants: false, feature_flag: { name: 'flag 2' } }, +] + +global.fetch = jest.fn(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve(featureFlags), + } as any as Response) +) describe('feature flags logic', () => { let logic: ReturnType @@ -16,7 +34,17 @@ describe('feature flags logic', () => { it('has expected defaults', () => { expectLogic(logic).toMatchValues({ - userFlags: [], + userFlags: featureFlags, + searchTerm: '', + filteredFlags: featureFlagsWithExtraInfo, + }) + }) + + it('can filter the flags', async () => { + await expectLogic(logic, () => { + logic.actions.setSearchTerm('2') + }).toMatchValues({ + filteredFlags: [{ currentValue: undefined, hasVariants: false, feature_flag: { name: 'flag 2' } }], }) }) })