Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds a search box to the toolbar featureflag list #6527

Merged
merged 9 commits into from
Oct 20, 2021
Merged

adds a search box to the toolbar featureflag list #6527

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
876959e
adds a search box to the toolbar featureflag list
pauldambra Oct 18, 2021
7203229
test flakes because react is re-rendering between the get and the cli…
pauldambra Oct 19, 2021
ed57718
move filtering toolbar feature flags to a Set and out of CSS
pauldambra Oct 19, 2021
634ded7
support initKeaTestLogic() with no args
mariusandra Oct 20, 2021
992a4aa
fix query-selector-all-deep jest bug
mariusandra Oct 20, 2021
7e93b41
add simple test case for feature flags logic
mariusandra Oct 20, 2021
93759fc
combine selectors
pauldambra Oct 20, 2021
d3153e0
with more understanding of Fuse
pauldambra Oct 20, 2021
2b48d52
add simple test for flag filtering
pauldambra Oct 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cypress/integration/invitesMembers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]')
Expand Down
154 changes: 88 additions & 66 deletions frontend/src/toolbar/flags/FeatureFlags.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -40,79 +42,99 @@ export function FeatureFlags(): JSX.Element {
</div>
</div>
) : (
<List
dataSource={userFlagsWithCalculatedInfo}
renderItem={({
feature_flag,
value_for_user_without_override,
override,
hasVariants,
currentValue,
}) => {
return (
<div className="feature-flag-row">
<Row
className={
override ? 'feature-flag-row-header overridden' : 'feature-flag-row-header'
}
<>
<Input.Search
allowClear
autoFocus
placeholder="Search"
value={searchTerm}
className={'feature-flag-row'}
onChange={(e) => setSearchTerm(e.target.value)}
/>
<List
dataSource={userFlagsWithCalculatedInfo}
renderItem={({
feature_flag,
value_for_user_without_override,
override,
hasVariants,
currentValue,
}) => {
return (
<div
className={clsx([
'feature-flag-row',
{ hidden: isHiddenBySearch(feature_flag.name) },
])}
>
<Typography.Text ellipsis className="feature-flag-title">
{feature_flag.key}
</Typography.Text>
<a
className="feature-flag-external-link"
href={`${apiURL}${
feature_flag.id ? urls.featureFlag(feature_flag.id) : urls.featureFlags()
}`}
target="_blank"
rel="noopener noreferrer"
>
<IconExternalLinkBold />
</a>
<Switch
checked={!!currentValue}
onChange={(checked) => {
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)
}
}}
/>
</Row>

<AnimatedCollapsible collapsed={!hasVariants || !currentValue}>
<Row
className={override ? 'variant-radio-group overridden' : 'variant-radio-group'}
className={
override ? 'feature-flag-row-header overridden' : 'feature-flag-row-header'
}
>
<Radio.Group
disabled={!currentValue}
value={currentValue}
onChange={(event) => {
const newValue = event.target.value
<Typography.Text ellipsis className="feature-flag-title">
{feature_flag.key}
</Typography.Text>
<a
className="feature-flag-external-link"
href={`${apiURL}${
feature_flag.id
? urls.featureFlag(feature_flag.id)
: urls.featureFlags()
}`}
target="_blank"
rel="noopener noreferrer"
>
<IconExternalLinkBold />
</a>
<Switch
checked={!!currentValue}
onChange={(checked) => {
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) => (
<Radio key={variant.key} value={variant.key}>
{`${variant.key} - ${variant.name} (${variant.rollout_percentage}%)`}
</Radio>
))}
</Radio.Group>
/>
</Row>
</AnimatedCollapsible>
</div>
)
}}
/>

<AnimatedCollapsible collapsed={!hasVariants || !currentValue}>
<Row
className={
override ? 'variant-radio-group overridden' : 'variant-radio-group'
}
>
<Radio.Group
disabled={!currentValue}
value={currentValue}
onChange={(event) => {
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) => (
<Radio key={variant.key} value={variant.key}>
{`${variant.key} - ${variant.name} (${variant.rollout_percentage}%)`}
</Radio>
))}
</Radio.Group>
</Row>
</AnimatedCollapsible>
</div>
)
}}
/>
</>
)}
</div>
)
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/toolbar/flags/featureFlags.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
32 changes: 25 additions & 7 deletions frontend/src/toolbar/flags/featureFlagsLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ 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<featureFlagsLogicType>({
actions: {
getUserFlags: true,
setOverriddenUserFlag: (flagId: number, overrideValue: string | boolean) => ({ flagId, overrideValue }),
deleteOverriddenUserFlag: (overrideId: number) => ({ overrideId }),
setShowLocalFeatureFlagWarning: (showWarning: boolean) => ({ showWarning }),
setSearchTerm: (searchTerm: string) => ({ searchTerm }),
},

loaders: ({ values }) => ({
Expand All @@ -23,8 +25,7 @@ export const featureFlagsLogic = kea<featureFlagsLogicType>({
if (!response.ok) {
return []
}
const results = await response.json()
return results
return await response.json()
},
setOverriddenUserFlag: async ({ flagId, overrideValue }, breakpoint) => {
const response = await toolbarFetch(
Expand Down Expand Up @@ -67,6 +68,12 @@ export const featureFlagsLogic = kea<featureFlagsLogicType>({
],
}),
reducers: {
searchTerm: [
'',
{
setSearchTerm: (_, { searchTerm }) => searchTerm,
},
],
showLocalFeatureFlagWarning: [
false,
{
Expand All @@ -92,12 +99,23 @@ export const featureFlagsLogic = kea<featureFlagsLogicType>({
})
},
],
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),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This isHiddenBySearch has a o(n^2) issue that might get annoying if you have a thousand flags on a slow system. If you want to improve this, make searchMatches return a Set instead.

  • However I'd use a different pattern altogether. Make a selector filteredFlags, and filter the userFlags through it. You'll end up with a smaller array that you'll loop over in the component instead. There is some value in not removing the element from the DOM and just hiding it with CSS, but I'm not sure it outweighs the added complexity here.

],
countFlagsOverridden: [(s) => [s.userFlags], (userFlags) => userFlags.filter((flag) => !!flag.override).length],
},
events: ({ actions }) => ({
afterMount: () => {
Expand Down