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.

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

feat(flags): Add FeatureFlagStatusIndicator to detail view for flags #26412

Merged
merged 79 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
79 commits
Select commit Hold shift + click to select a range
c9c0e69
(WIP) feat(flags): add GET feature_flags/:id/status for getting stale…
havenbarnes Nov 15, 2024
8cabf20
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Nov 18, 2024
6393b43
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Nov 19, 2024
ebb3d79
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Nov 19, 2024
d5d7fe5
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Nov 19, 2024
54315d7
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Nov 20, 2024
e831e92
finished + tested
havenbarnes Nov 21, 2024
c9de2fd
cleanup
havenbarnes Nov 21, 2024
f7bacde
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Nov 21, 2024
865673c
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Nov 21, 2024
91cb491
Fix linter
havenbarnes Nov 21, 2024
6724a4e
cleanup
havenbarnes Nov 21, 2024
4969eb9
tweak
havenbarnes Nov 21, 2024
25e2abf
Merge branch 'master' into detect-stale-flags
havenbarnes Nov 21, 2024
5f73662
Merge branch 'master' into detect-stale-flags
havenbarnes Nov 25, 2024
5554015
Merge branch 'master' into detect-stale-flags
havenbarnes Nov 25, 2024
c9340ba
Merge branch 'master' into detect-stale-flags
havenbarnes Nov 25, 2024
20b15ad
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Nov 25, 2024
cfd96e8
adjust a little after chatting with Dylan
havenbarnes Nov 25, 2024
799cba3
Merge branch 'detect-stale-flags' of https://github.com/PostHog/posth…
havenbarnes Nov 25, 2024
548d1ce
feat(flags): Add FeatureFlagStatusBanner to detail view for flags
havenbarnes Nov 26, 2024
be3c96f
tweak
havenbarnes Nov 26, 2024
836185c
Merge branch 'master' into detect-stale-flags
havenbarnes Nov 26, 2024
6f5cfb3
Merge branch 'detect-stale-flags' into stale-flag-ui
havenbarnes Nov 26, 2024
acbe9a5
tweak
havenbarnes Nov 26, 2024
d32b0b0
Merge branch 'detect-stale-flags' of https://github.com/PostHog/posth…
havenbarnes Nov 26, 2024
4873a02
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Nov 26, 2024
5c618dc
Merge branch 'detect-stale-flags' into stale-flag-ui
havenbarnes Nov 26, 2024
c9418aa
Update posthog/models/feature_flag/flag_status.py
havenbarnes Nov 26, 2024
3dccf1d
Update posthog/models/feature_flag/flag_status.py
havenbarnes Nov 26, 2024
fda646e
Merge branch 'detect-stale-flags' into stale-flag-ui
havenbarnes Nov 26, 2024
9062d8c
Merge branch 'master' into detect-stale-flags
dmarticus Nov 26, 2024
51bc657
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Dec 2, 2024
76360f9
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Dec 2, 2024
f785cb7
tweak select clause
havenbarnes Dec 2, 2024
1c7f2bb
fix copy
havenbarnes Dec 2, 2024
864dd33
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Dec 2, 2024
2aa65be
update tests
havenbarnes Dec 2, 2024
23320f0
Update query snapshots
github-actions[bot] Dec 2, 2024
3b6a43a
Merge branch 'master' into detect-stale-flags
havenbarnes Dec 2, 2024
2e5f51d
Merge branch 'detect-stale-flags' of https://github.com/PostHog/posth…
havenbarnes Dec 2, 2024
e5a35bf
Merge branch 'stale-flag-ui' of https://github.com/PostHog/posthog in…
havenbarnes Dec 2, 2024
e6727a7
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Dec 2, 2024
d2d42b2
tweak
havenbarnes Dec 2, 2024
92150a6
Merge branch 'master' of https://github.com/PostHog/posthog into stal…
havenbarnes Dec 2, 2024
20a349e
Update query snapshots
github-actions[bot] Dec 2, 2024
a197ca5
Merge branch 'detect-stale-flags' into stale-flag-ui
havenbarnes Dec 2, 2024
af637e5
Merge branch 'master' into detect-stale-flags
havenbarnes Dec 2, 2024
5334c34
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Dec 3, 2024
550870c
Merge branch 'detect-stale-flags' of https://github.com/PostHog/posth…
havenbarnes Dec 3, 2024
10061d1
Merge branch 'detect-stale-flags' into stale-flag-ui
havenbarnes Dec 3, 2024
90c0f19
Update query snapshots
github-actions[bot] Dec 3, 2024
8ceafd9
Merge branch 'detect-stale-flags' into stale-flag-ui
havenbarnes Dec 3, 2024
c89d215
Merge branch 'master' into detect-stale-flags
havenbarnes Dec 3, 2024
da4fa13
Merge branch 'master' of https://github.com/PostHog/posthog into stal…
havenbarnes Dec 4, 2024
34a9001
Merge branch 'stale-flag-ui' of https://github.com/PostHog/posthog in…
havenbarnes Dec 4, 2024
d0fd266
fix storybook test
havenbarnes Dec 4, 2024
930a4e0
Merge branch 'detect-stale-flags' of https://github.com/PostHog/posth…
havenbarnes Dec 4, 2024
09b600f
Merge branch 'master' of https://github.com/PostHog/posthog into dete…
havenbarnes Dec 4, 2024
c1633fa
Merge branch 'detect-stale-flags' of https://github.com/PostHog/posth…
havenbarnes Dec 4, 2024
1e99459
Merge branch 'detect-stale-flags' of https://github.com/PostHog/posth…
havenbarnes Dec 4, 2024
b8ff72c
Merge branch 'master' into detect-stale-flags
havenbarnes Dec 5, 2024
c93f1a3
Use proper term "rolled out" instead of "enabled"
havenbarnes Dec 5, 2024
f4767ed
Merge branch 'detect-stale-flags' of https://github.com/PostHog/posth…
havenbarnes Dec 5, 2024
5308ac5
Merge branch 'master' of https://github.com/PostHog/posthog into stal…
havenbarnes Dec 5, 2024
f7010cc
Merge branch 'master' into detect-stale-flags
havenbarnes Dec 5, 2024
077a7a8
Merge branch 'master' of https://github.com/PostHog/posthog into stal…
havenbarnes Dec 5, 2024
f382fc2
re-work to use a Tag with tooltip
havenbarnes Dec 5, 2024
fe22911
Merge branch 'master' into detect-stale-flags
havenbarnes Dec 6, 2024
c754314
Merge branch 'detect-stale-flags' of https://github.com/PostHog/posth…
havenbarnes Dec 6, 2024
519e013
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 6, 2024
87cd011
Merge branch 'master' of https://github.com/PostHog/posthog into stal…
havenbarnes Dec 6, 2024
cf5db3d
Merge branch 'stale-flag-ui' of https://github.com/PostHog/posthog in…
havenbarnes Dec 6, 2024
5893965
tweak
havenbarnes Dec 6, 2024
96dae51
Merge branch 'master' into stale-flag-ui
havenbarnes Dec 9, 2024
bb7b012
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 9, 2024
15958bd
Update UI snapshots for `chromium` (2)
github-actions[bot] Dec 9, 2024
225270d
Merge branch 'master' into stale-flag-ui
havenbarnes Dec 9, 2024
efc864e
Merge branch 'master' into stale-flag-ui
havenbarnes Dec 9, 2024
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 14 additions & 0 deletions frontend/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
ExternalDataSourceSyncSchema,
ExternalDataSourceType,
FeatureFlagAssociatedRoleType,
FeatureFlagStatusResponse,
FeatureFlagType,
Group,
GroupListParams,
Expand Down Expand Up @@ -663,6 +664,13 @@ class ApiRequest {
)
}

public featureFlagStatus(teamId: TeamType['id'], featureFlagId: FeatureFlagType['id']): ApiRequest {
return this.projectsDetail(teamId)
.addPathComponent('feature_flags')
.addPathComponent(String(featureFlagId))
.addPathComponent('status')
}

public featureFlagCreateScheduledChange(teamId: TeamType['id']): ApiRequest {
return this.projectsDetail(teamId).addPathComponent('scheduled_changes')
}
Expand Down Expand Up @@ -1042,6 +1050,12 @@ const api = {
): Promise<{ scheduled_change: ScheduledChangeType }> {
return await new ApiRequest().featureFlagDeleteScheduledChange(teamId, scheduledChangeId).delete()
},
async getStatus(
teamId: TeamType['id'],
featureFlagId: FeatureFlagType['id']
): Promise<FeatureFlagStatusResponse> {
return await new ApiRequest().featureFlagStatus(teamId, featureFlagId).get()
},
},

organizationFeatureFlags: {
Expand Down
67 changes: 36 additions & 31 deletions frontend/src/scenes/feature-flags/FeatureFlag.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ import FeatureFlagProjects from './FeatureFlagProjects'
import { FeatureFlagReleaseConditions } from './FeatureFlagReleaseConditions'
import FeatureFlagSchedule from './FeatureFlagSchedule'
import { featureFlagsLogic, FeatureFlagsTab } from './featureFlagsLogic'
import { FeatureFlagStatusIndicator } from './FeatureFlagStatusIndicator'
import { RecentFeatureFlagInsights } from './RecentFeatureFlagInsightsCard'

export const scene: SceneExport = {
Expand Down Expand Up @@ -734,6 +735,7 @@ function FeatureFlagRollout({ readOnly }: { readOnly?: boolean }): JSX.Element {
aggregationTargetName,
featureFlag,
recordingFilterForFlag,
flagStatus,
} = useValues(featureFlagLogic)
const {
distributeVariantsEqually,
Expand Down Expand Up @@ -788,38 +790,41 @@ function FeatureFlagRollout({ readOnly }: { readOnly?: boolean }): JSX.Element {
Deleted
</LemonTag>
) : (
<LemonSwitch
onChange={(newValue) => {
LemonDialog.open({
title: `${newValue === true ? 'Enable' : 'Disable'} this flag?`,
description: `This flag will be immediately ${
newValue === true ? 'rolled out to' : 'rolled back from'
} the users matching the release conditions.`,
primaryButton: {
children: 'Confirm',
type: 'primary',
onClick: () => {
const updatedFlag = { ...featureFlag, active: newValue }
setFeatureFlag(updatedFlag)
saveFeatureFlag(updatedFlag)
<div className="flex gap-2">
<LemonSwitch
onChange={(newValue) => {
LemonDialog.open({
title: `${newValue === true ? 'Enable' : 'Disable'} this flag?`,
description: `This flag will be immediately ${
newValue === true ? 'rolled out to' : 'rolled back from'
} the users matching the release conditions.`,
primaryButton: {
children: 'Confirm',
type: 'primary',
onClick: () => {
const updatedFlag = { ...featureFlag, active: newValue }
setFeatureFlag(updatedFlag)
saveFeatureFlag(updatedFlag)
},
size: 'small',
},
size: 'small',
},
secondaryButton: {
children: 'Cancel',
type: 'tertiary',
size: 'small',
},
})
}}
label="Enabled"
disabledReason={
!featureFlag.can_edit
? "You only have view access to this feature flag. To make changes, contact the flag's creator."
: null
}
checked={featureFlag.active}
/>
secondaryButton: {
children: 'Cancel',
type: 'tertiary',
size: 'small',
},
})
}}
label="Enabled"
disabledReason={
!featureFlag.can_edit
? "You only have view access to this feature flag. To make changes, contact the flag's creator."
: null
}
checked={featureFlag.active}
/>
<FeatureFlagStatusIndicator flagStatus={flagStatus} />
</div>
)}
</div>
<div className="col-span-6">
Expand Down
40 changes: 40 additions & 0 deletions frontend/src/scenes/feature-flags/FeatureFlagStatusIndicator.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { LemonTag } from 'lib/lemon-ui/LemonTag'
import { Tooltip } from 'lib/lemon-ui/Tooltip'

import { FeatureFlagStatus, FeatureFlagStatusResponse } from '~/types'

export function FeatureFlagStatusIndicator({
flagStatus,
}: {
flagStatus: FeatureFlagStatusResponse | null
}): JSX.Element | null {
if (
!flagStatus ||
[FeatureFlagStatus.ACTIVE, FeatureFlagStatus.DELETED, FeatureFlagStatus.UNKNOWN].includes(flagStatus.status)
) {
return null
}

return (
<Tooltip
title={
<>
<div className="text-sm">{flagStatus.reason}</div>
<div className="text-xs">
{flagStatus.status === FeatureFlagStatus.STALE &&
'Make sure to remove any references to this flag in your code before deleting it.'}
{flagStatus.status === FeatureFlagStatus.INACTIVE &&
'It is probably not being used in your code, but be sure to remove any references to this flag before deleting it.'}
</div>
</>
}
placement="right"
>
<span>
<LemonTag type="warning" className="uppercase cursor-default">
{flagStatus.status}
</LemonTag>
</span>
</Tooltip>
)
}
8 changes: 8 additions & 0 deletions frontend/src/scenes/feature-flags/FeatureFlags.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { mswDecorator } from '~/mocks/browser'
import featureFlags from './__mocks__/feature_flags.json'

const meta: Meta = {
tags: ['ff'],
title: 'Scenes-App/Feature Flags',
parameters: {
layout: 'fullscreen',
Expand All @@ -33,6 +34,13 @@ const meta: Meta = {
200,
featureFlags.results.find((r) => r.id === Number(req.params['flagId'])),
],
'/api/projects/:team_id/feature_flags/:flagId/status': () => [
200,
{
status: 'active',
reason: 'Feature flag is active',
},
],
},
post: {
'/api/environments/:team_id/query': {},
Expand Down
15 changes: 15 additions & 0 deletions frontend/src/scenes/feature-flags/featureFlagLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
EarlyAccessFeatureType,
FeatureFlagGroupType,
FeatureFlagRollbackConditions,
FeatureFlagStatusResponse,
FeatureFlagType,
FilterLogicalOperator,
FilterType,
Expand Down Expand Up @@ -755,6 +756,18 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
}
},
},
flagStatus: [
null as FeatureFlagStatusResponse | null,
{
loadFeatureFlagStatus: () => {
const { currentTeamId } = values
if (currentTeamId && props.id && props.id !== 'new' && props.id !== 'link') {
return api.featureFlags.getStatus(currentTeamId, props.id)
}
return null
},
},
],
})),
listeners(({ actions, values, props }) => ({
submitNewDashboardSuccessWithResult: async ({ result }) => {
Expand Down Expand Up @@ -1040,8 +1053,10 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
actions.setFeatureFlag(formatPayloadsWithFlag)
actions.loadRelatedInsights()
actions.loadAllInsightsForFlag()
actions.loadFeatureFlagStatus()
} else if (props.id !== 'new') {
actions.loadFeatureFlag()
actions.loadFeatureFlagStatus()
}
}),
])
13 changes: 13 additions & 0 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2994,6 +2994,19 @@ export interface FeatureFlagRollbackConditions {
operator?: string
}

export enum FeatureFlagStatus {
ACTIVE = 'active',
INACTIVE = 'inactive',
STALE = 'stale',
DELETED = 'deleted',
UNKNOWN = 'unknown',
}

export interface FeatureFlagStatusResponse {
status: FeatureFlagStatus
reason: string
}
Comment on lines +2997 to +3008
Copy link
Contributor

Choose a reason for hiding this comment

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

how is inactive differentiated from disabled in this case? Are all disabled flags also inactive? If so, I feel like we shouldn't show an indicator if that's the case – I only care about a flag being inactive/stale if I have the flag enabled in my app.

Copy link
Contributor Author

@havenbarnes havenbarnes Dec 5, 2024

Choose a reason for hiding this comment

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

If a flag is disabled (to be precise, flag.active = false), then the endpoint currently would only determine it to be inactive if there have been no recent evaluations. active = false does not imply inactive or stale.

I did this intentionally; a good example of why this feels correct to me is if a flag exists in staging and production projects but is only enabled in staging, you don't want to make the flag show up as stale in the prod project, unless it isn't being evaluated at all which would indicate that you aren't actually referencing the flag in code at all then (in this case we would call it "inactive", because we don't actually know for sure it is "stale", we only know it is unused).

# - STALE: The feature flag has been fully rolled out to users. Its evaluations can not vary.
# - INACTIVE: The feature flag is not being actively evaluated. STALE takes precedence over INACTIVE.


export interface CombinedFeatureFlagAndValueType {
feature_flag: FeatureFlagType
value: boolean | string
Expand Down
24 changes: 24 additions & 0 deletions posthog/api/test/test_feature_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -6287,6 +6287,30 @@ def test_flag_status_reasons(self):
FeatureFlagStatus.ACTIVE,
)

# Request status for multivariate flag with a variant set to 100% but no release condition set to 100%
multivariate_flag_rolled_out_release_condition_half_variant = FeatureFlag.objects.create(
name="Multivariate flag with release condition set to 100%, but variants still 50%",
key="multivariate-rolled-out-release-half-variant-flag",
team=self.team,
active=True,
filters={
"multivariate": {
"variants": [
{"key": "var1key", "name": "test", "rollout_percentage": 50},
{"key": "var2key", "name": "control", "rollout_percentage": 50},
],
},
"groups": [
{"variant": None, "properties": [], "rollout_percentage": 100},
],
},
)
self.create_feature_flag_called_event(multivariate_flag_rolled_out_release_condition_half_variant.key)
self.assert_expected_response(
multivariate_flag_rolled_out_release_condition_half_variant.id,
FeatureFlagStatus.ACTIVE,
)

# Request status for multivariate flag with variants set to 100% and a filtered release condition
multivariate_flag_rolled_out_variant_rolled_out_filtered_release = FeatureFlag.objects.create(
name="Multivariate flag with variant and release condition set to 100%",
Expand Down
2 changes: 1 addition & 1 deletion posthog/models/feature_flag/flag_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def is_flag_fully_rolled_out(self, flag: FeatureFlag) -> tuple[bool, FeatureFlag
)
if multivariate and is_multivariate_flag_fully_rolled_out:
return True, f'This flag will always use the variant "{fully_rolled_out_variant_name}"'
elif self.is_boolean_flag_fully_rolled_out(flag):
elif not multivariate and self.is_boolean_flag_fully_rolled_out(flag):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Meant to push this to #26340, caught an edge case (the new test checks for this)

return True, 'This boolean flag will always evaluate to "true"'

return False, ""
Expand Down
Loading