Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 11 additions & 14 deletions frontend/common/hooks/useFeatureStates.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import { useGetFeatureListQuery } from 'common/services/useProjectFlag'
import { useProjectEnvironments } from './useProjectEnvironments'
import { useGetProjectFlagsQuery } from 'common/services/useProjectFlag'
import { ProjectFlag } from 'common/types/responses'
import { useProjectEnvironments } from './useProjectEnvironments'

// Stable, so a memo over the list doesn't rerun while it loads.
// Stable, so consumers do not see a new empty list while loading.
const NO_FEATURES: ProjectFlag[] = []

type UseFeatureStatesArgs = {
// The api key, as everywhere else in the UI.
environmentId: string
projectId: number
skip?: boolean
}

// Whether each of a project's features is on in one environment, from the
// feature list. getFeatureList takes the numeric environment id, not the key.
// Whether each feature is on in one environment, from all project flags.
// The recursive project-flags query follows every page.
export const useFeatureStates = ({
environmentId,
projectId,
Expand All @@ -26,23 +25,21 @@ export const useFeatureStates = ({
} = useProjectEnvironments(projectId)
const numericEnvId = getEnvironmentIdFromKey(environmentId)
const {
data,
currentData: data,
isError: isFeatureListError,
isLoading: isLoadingFeatureList,
} = useGetFeatureListQuery(
isFetching: isLoadingFeatureList,
} = useGetProjectFlagsQuery(
{
environmentId: String(numericEnvId ?? ''),
page: 1,
// One page has to cover every feature, or one outside it reads as off.
page_size: 999,
projectId,
environment: numericEnvId,
project: String(projectId),
},
{ skip: skip || !numericEnvId },
)

const features = data?.results ?? NO_FEATURES
return {
features,
hasLoadedFeatures: data !== undefined,
isEnabled: (featureId: number) =>
!!features.find((feature) => feature.id === featureId)
?.environment_feature_state?.enabled,
Expand Down
27 changes: 27 additions & 0 deletions frontend/web/components/modals/create-feature/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ import SegmentOverridesTab, {
import UsageTab from './tabs/UsageTab'
import FeatureDependenciesTab from './tabs/FeatureDependenciesTab'
import { useStagedPrerequisites } from './tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites'
import { useDependencies } from './tabs/FeatureDependenciesTab/hooks/useDependencies'
import {
countMetPrerequisites,
isBlocked,
} from './tabs/FeatureDependenciesTab/prerequisiteState'
import FeatureLimitAlert from './components/FeatureLimitAlert'
import FeatureUpdateSummary from './components/FeatureUpdateSummary'
import FeatureNameInput from './components/FeatureNameInput'
Expand Down Expand Up @@ -162,6 +167,22 @@ const CreateFeatureModal: FC<CreateFeatureModalProps> = (props) => {
projectFlag,
})
const dependenciesChanged = !!dependencyChanges.staged.length

// Share the same RTK Query cache as FeatureDependenciesTab (no extra fetch).
// Used to show the blocked-by-dependencies notice in the Value tab and the
// met/total badge on the Dependencies tab.
const { isReady: areDependenciesReady, rows: prerequisiteRows } =
useDependencies({
environmentId,
featureId: projectFlag.id,
projectId,
skip: !projectFlag.id || Boolean(identity),
})
const prerequisitesMet = areDependenciesReady
? countMetPrerequisites(prerequisiteRows)
: undefined
const isBlockedByDependencies =
areDependenciesReady && isBlocked(prerequisiteRows)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
const focusTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null)

const close = useCallback(() => {
Expand Down Expand Up @@ -828,6 +849,7 @@ const CreateFeatureModal: FC<CreateFeatureModalProps> = (props) => {
isVersioned={isVersioned}
isSaving={isSaving}
existingChangeRequest={!!existingChangeRequest}
isBlockedByDependencies={isBlockedByDependencies}
hasVariationChanges={hasVariationChanges}
onSaveVariationValues={onSaveVariationValues}
originalMultivariateOptions={originalMultivariateOptions}
Expand Down Expand Up @@ -881,6 +903,11 @@ const CreateFeatureModal: FC<CreateFeatureModalProps> = (props) => {
data-test='feature_dependencies'
tabLabel='Dependencies'
isDirty={dependenciesChanged}
tabBadge={
prerequisitesMet
? `${prerequisitesMet.met}/${prerequisitesMet.total}`
: undefined
}
>
<FeatureDependenciesTab
environmentId={environmentId}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { DependencyEdge, ProjectFlag } from 'common/types/responses'
import {
describeOffCount,
hasPrerequisiteStates,
isBlocked,
toPrerequisiteRow,
toPrerequisiteRows,
Expand Down Expand Up @@ -59,6 +60,20 @@ describe('toPrerequisiteRow', () => {
})
})

describe('hasPrerequisiteStates', () => {
it('waits until each prerequisite is present in the feature list', () => {
expect(
hasPrerequisiteStates([edge(11), edge(12)], [feature(11, true)]),
).toBe(false)
expect(
hasPrerequisiteStates(
[edge(11), edge(12)],
[feature(11, true), feature(12, false)],
),
).toBe(true)
})
})

describe('toPrerequisiteRows', () => {
it('orders by segment id, so a new prerequisite lands at the bottom', () => {
const rows = toPrerequisiteRows(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,45 +7,73 @@ import {
useGetFeatureDependentsQuery,
} from 'common/services/useFeatureDependency'
import { useFeatureStates } from 'common/hooks/useFeatureStates'
import { toPrerequisiteRows } from 'components/modals/create-feature/tabs/FeatureDependenciesTab/prerequisiteState'
import {
hasPrerequisiteStates,
toPrerequisiteRows,
} from 'components/modals/create-feature/tabs/FeatureDependenciesTab/prerequisiteState'

type UseDependenciesArgs = {
environmentId: string
featureId: number
projectId: number
skip?: boolean
}

export const useDependencies = ({
environmentId,
featureId,
projectId,
skip = false,
}: UseDependenciesArgs) => {
const query = { environmentId, featureId }
const {
data: dependencies,
isError: isDependenciesError,
isLoading: isLoadingDependencies,
} = useGetFeatureDependenciesQuery(query)
const {
data: dependents,
isError: isDependentsError,
isLoading: isLoadingDependents,
} = useGetFeatureDependentsQuery(query)
} = useGetFeatureDependenciesQuery(query, { skip })
const { data: dependents } = useGetFeatureDependentsQuery(query, { skip })

const [createDependency, { isLoading: isCreating }] =
useCreateFeatureDependencyMutation()
const [deleteDependency] = useDeleteFeatureDependencyMutation()

// The edges carry only the prerequisite's name and id, so its current state
// in this environment comes from the feature list.
const featureStates = useFeatureStates({ environmentId, projectId })
const featureStates = useFeatureStates({ environmentId, projectId, skip })
const prerequisites = dependencies?.results ?? []
const hasAllPrerequisiteStates = hasPrerequisiteStates(
prerequisites,
featureStates.features,
)
const isReady =
!skip &&
!isDependenciesError &&
!isLoadingDependencies &&
!featureStates.isLoading &&
!featureStates.isError &&
featureStates.hasLoadedFeatures &&
hasAllPrerequisiteStates

const rows = useMemo(
() =>
toPrerequisiteRows(dependencies?.results ?? [], featureStates.features),
[dependencies, featureStates.features],
isReady
? toPrerequisiteRows(
dependencies?.results ?? [],
featureStates.features,
)
: [],
[dependencies, featureStates.features, isReady],
)

const isLoading =
!skip &&
(isLoadingDependencies ||
featureStates.isLoading ||
(!featureStates.isError && !featureStates.hasLoadedFeatures) ||
(!isDependenciesError &&
!isLoadingDependencies &&
!hasAllPrerequisiteStates))

return {
add: (prerequisite: ProjectFlag) =>
createDependency({
Expand All @@ -56,12 +84,10 @@ export const useDependencies = ({
dependentEdges: dependents?.results ?? [],
isCreating,
isEnabled: featureStates.isEnabled,
// The feature list counts too. Without it every prerequisite reads as off,
// so the tab would claim the flag is serving off while the list is still
// in flight, or for good if the request fails.
isError: isDependenciesError || isDependentsError || featureStates.isError,
isLoading:
isLoadingDependencies || isLoadingDependents || featureStates.isLoading,
// Prerequisite status does not rely on dependents loading successfully.
isError: isDependenciesError || featureStates.isError,
isLoading,
isReady,
remove: (prerequisiteFeatureId: number) =>
deleteDependency({
environmentId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,9 @@ export {
DependentsEmptyState,
PrerequisitesEmptyState,
} from './DependenciesEmptyStates'
export { toPrerequisiteRow } from './prerequisiteState'
export {
countMetPrerequisites,
isBlocked,
toPrerequisiteRow,
} from './prerequisiteState'
export type { PrerequisiteRow } from './prerequisiteState'
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ export const toPrerequisiteRow = (
})

// Each prerequisite gets its own segment, so segment id is the order added.
export const hasPrerequisiteStates = (
edges: DependencyEdge[],
features: ProjectFlag[],
): boolean => {
const featureIds = new Set(features.map((feature) => feature.id))
return edges.every((edge) => featureIds.has(edge.prerequisite.id))
}

export const toPrerequisiteRows = (
edges: DependencyEdge[],
features: ProjectFlag[],
Expand Down Expand Up @@ -75,6 +83,16 @@ export const withStagedChanges = (
export const isBlocked = (rows: PrerequisiteRow[]): boolean =>
rows.some((row) => row.isMet === false)

// Returns the number of judged prerequisites that are met, out of the total
// number of judged prerequisites. Returns null when there are no judged rows.
export const countMetPrerequisites = (
rows: PrerequisiteRow[],
): { met: number; total: number } | null => {
const judged = rows.filter((row) => row.isMet !== undefined)
if (!judged.length) return null
return { met: judged.filter((row) => row.isMet).length, total: judged.length }
}

export const describeOffCount = (rows: PrerequisiteRow[]): string => {
const total = rows.length
const offCount = rows.filter((row) => !row.isEnabled).length
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { AddVariationButton } from 'components/mv/AddVariationButton'
import ErrorMessage from 'components/ErrorMessage'
import InfoMessage from 'components/InfoMessage'
import WarningMessage from 'components/WarningMessage'
import Banner from 'components/base/Banner'
import Tooltip from 'components/Tooltip'
import Icon from 'components/icons/Icon'
import Switch from 'components/Switch'
Expand Down Expand Up @@ -65,6 +66,9 @@ type FeatureValueTabProps = {
isVersioned?: boolean
isSaving?: boolean
existingChangeRequest?: boolean
// When true, a warning banner is shown explaining the flag is served off due
// to unmet prerequisites.
isBlockedByDependencies?: boolean
onSaveFeatureValue?: (schedule?: boolean) => void
hasVariationChanges?: boolean
onSaveVariationValues?: () => void
Expand Down Expand Up @@ -97,6 +101,7 @@ const FeatureValueTab: FC<FeatureValueTabProps> = ({
hasVariationChanges,
identity,
is4Eyes,
isBlockedByDependencies,
isSaving,
isVersioned,
noPermissions,
Expand Down Expand Up @@ -374,6 +379,26 @@ const FeatureValueTab: FC<FeatureValueTabProps> = ({
environmentId={environmentId}
/>
)}
{!!isBlockedByDependencies && (
<Banner
type='warning'
className='mb-4'
title='Served off by dependencies'
>
This flag is currently forced off because one or more of its
prerequisites are not met.{' '}
{existingChangeRequest ? (
<>
Open the feature's <strong>Dependencies</strong> tab outside this
change request for details.
</>
) : (
<>
See the <strong>Dependencies</strong> tab for details.
</>
)}
</Banner>
)}
<FormGroup className='mb-4'>
<Tooltip
title={
Expand Down
3 changes: 3 additions & 0 deletions frontend/web/components/navigation/TabMenu/TabButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ const TabButton = React.forwardRef<HTMLButtonElement | null, TabButtonProps>(
<ColorSwatch color={colorIconWarning} shape='circle' size='sm' />
</span>
)}
{child.props.tabBadge !== undefined && (
<span className='btn-tab__badge'>{child.props.tabBadge}</span>
)}
</span>
</Button>
)
Expand Down
2 changes: 2 additions & 0 deletions frontend/web/components/navigation/TabMenu/TabItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ type TabItemType = {
// so positioning lives in one place and TabButton can wrap label + badge in
// a flex container that prevents the badge from breaking onto a new line.
isDirty?: boolean
// Renders an informational badge (e.g. "1/3") next to the label.
tabBadge?: ReactNode
}

const TabItem: FC<TabItemType> = ({ children }) => {
Expand Down
15 changes: 15 additions & 0 deletions frontend/web/styles/components/_tabs.scss
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,21 @@
gap: 4px;
}

.btn-tab__badge {
display: inline-flex;
align-items: center;
justify-content: center;
font-size: $tab-btn-counter-value-font-size;
background-color: $tab-btn-counter-value-bg;
line-height: $line-height-xxsm;
border-radius: 10px;
padding: 0 5px;
min-width: $tab-btn-counter-value-width;
height: $tab-btn-counter-value-width;
color: $text-icon-light;
font-weight: $tab-btn-font-weight;
}

&.btn-no-focus {
&:focus {
box-shadow: none !important;
Expand Down