diff --git a/frontend/common/hooks/useFeatureStates.ts b/frontend/common/hooks/useFeatureStates.ts index 5d1a6a593775..ede2399cb4a4 100644 --- a/frontend/common/hooks/useFeatureStates.ts +++ b/frontend/common/hooks/useFeatureStates.ts @@ -1,10 +1,9 @@ -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 @@ -12,8 +11,8 @@ type UseFeatureStatesArgs = { 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, @@ -26,16 +25,13 @@ 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 }, ) @@ -43,6 +39,7 @@ export const useFeatureStates = ({ const features = data?.results ?? NO_FEATURES return { features, + hasLoadedFeatures: data !== undefined, isEnabled: (featureId: number) => !!features.find((feature) => feature.id === featureId) ?.environment_feature_state?.enabled, diff --git a/frontend/web/components/modals/create-feature/index.tsx b/frontend/web/components/modals/create-feature/index.tsx index 1d45759ce455..8d39cf4d5e93 100644 --- a/frontend/web/components/modals/create-feature/index.tsx +++ b/frontend/web/components/modals/create-feature/index.tsx @@ -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' @@ -162,6 +167,22 @@ const CreateFeatureModal: FC = (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) const focusTimeoutRef = useRef | null>(null) const close = useCallback(() => { @@ -828,6 +849,7 @@ const CreateFeatureModal: FC = (props) => { isVersioned={isVersioned} isSaving={isSaving} existingChangeRequest={!!existingChangeRequest} + isBlockedByDependencies={isBlockedByDependencies} hasVariationChanges={hasVariationChanges} onSaveVariationValues={onSaveVariationValues} originalMultivariateOptions={originalMultivariateOptions} @@ -881,6 +903,11 @@ const CreateFeatureModal: FC = (props) => { data-test='feature_dependencies' tabLabel='Dependencies' isDirty={dependenciesChanged} + tabBadge={ + prerequisitesMet + ? `${prerequisitesMet.met}/${prerequisitesMet.total}` + : undefined + } > { }) }) +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( diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useDependencies.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useDependencies.ts index 763603987930..ea0abe367ad3 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useDependencies.ts +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useDependencies.ts @@ -7,30 +7,31 @@ 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() @@ -38,14 +39,41 @@ export const useDependencies = ({ // 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({ @@ -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, diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/index.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/index.ts index f15752e6820f..5cbac68ef593 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/index.ts +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/index.ts @@ -8,5 +8,9 @@ export { DependentsEmptyState, PrerequisitesEmptyState, } from './DependenciesEmptyStates' -export { toPrerequisiteRow } from './prerequisiteState' +export { + countMetPrerequisites, + isBlocked, + toPrerequisiteRow, +} from './prerequisiteState' export type { PrerequisiteRow } from './prerequisiteState' diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/prerequisiteState.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/prerequisiteState.ts index 4545aaf77d67..f77148ab1e1c 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/prerequisiteState.ts +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/prerequisiteState.ts @@ -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[], @@ -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 diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx index 172cefada982..245e6cf18ff4 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureValueTab.tsx @@ -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' @@ -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 @@ -97,6 +101,7 @@ const FeatureValueTab: FC = ({ hasVariationChanges, identity, is4Eyes, + isBlockedByDependencies, isSaving, isVersioned, noPermissions, @@ -374,6 +379,26 @@ const FeatureValueTab: FC = ({ environmentId={environmentId} /> )} + {!!isBlockedByDependencies && ( + + This flag is currently forced off because one or more of its + prerequisites are not met.{' '} + {existingChangeRequest ? ( + <> + Open the feature's Dependencies tab outside this + change request for details. + + ) : ( + <> + See the Dependencies tab for details. + + )} + + )} ( )} + {child.props.tabBadge !== undefined && ( + {child.props.tabBadge} + )} ) diff --git a/frontend/web/components/navigation/TabMenu/TabItem.tsx b/frontend/web/components/navigation/TabMenu/TabItem.tsx index 622184d05abc..1eb8b6ae2b14 100644 --- a/frontend/web/components/navigation/TabMenu/TabItem.tsx +++ b/frontend/web/components/navigation/TabMenu/TabItem.tsx @@ -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 = ({ children }) => { diff --git a/frontend/web/styles/components/_tabs.scss b/frontend/web/styles/components/_tabs.scss index 9b73f1aed934..687dfa8b3ec4 100644 --- a/frontend/web/styles/components/_tabs.scss +++ b/frontend/web/styles/components/_tabs.scss @@ -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;