From 4b79d3183780616824ac7cc0ca61c3f4e7345452 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 9 Oct 2026 16:52:37 -0300 Subject: [PATCH 01/11] feat(dependencies): add the API layer for staging dependencies in a change request Real endpoints to create and delete a change request, and an optional changeRequestId on adding and removing a dependency. Staging is faked until #8449: the API ignores ?change_request today and would publish the change live, so those calls go to fakeChangeRequestDependencies.ts. Refs #8450 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../fakeChangeRequestDependencies.test.ts | 86 +++++++++++++ .../services/fakeChangeRequestDependencies.ts | 114 ++++++++++++++++++ frontend/common/services/useChangeRequest.ts | 27 +++++ .../common/services/useFeatureDependency.ts | 83 +++++++++++-- frontend/common/services/useFeatureState.ts | 1 + frontend/common/types/requests.ts | 28 +++++ frontend/common/types/responses.ts | 1 + frontend/common/utils/stagedDependencyEdge.ts | 18 +++ 8 files changed, 348 insertions(+), 10 deletions(-) create mode 100644 frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts create mode 100644 frontend/common/services/fakeChangeRequestDependencies.ts create mode 100644 frontend/common/utils/stagedDependencyEdge.ts diff --git a/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts b/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts new file mode 100644 index 000000000000..388ba64583cd --- /dev/null +++ b/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts @@ -0,0 +1,86 @@ +import { DependencyEdge } from 'common/types/responses' +import { + getStagedDependencyChanges, + stageAdd, + stageRemove, +} from 'common/services/fakeChangeRequestDependencies' + +const edge = (featureId: number, prerequisiteId: number) => + ({ + feature: { id: featureId, name: `feature_${featureId}` }, + prerequisite: { id: prerequisiteId, name: `feature_${prerequisiteId}` }, + } as unknown as DependencyEdge) + +const prerequisite = { id: 3, name: 'feature_3' } +const args = { + changeRequestId: 12, + dependentEdges: [], + featureId: 1, + liveEdges: [], + prerequisite, + prerequisiteEdges: [], +} + +beforeEach(() => { + const store: Record = {} + globalThis.localStorage = { + getItem: (key: string) => store[key] ?? null, + setItem: (key: string, value: string) => { + store[key] = value + }, + } as Storage +}) + +describe('stageAdd', () => { + it('holds the add against the change request and feature', () => { + expect(stageAdd(args)).toBeNull() + expect(getStagedDependencyChanges(12, 1)).toEqual([ + { action: 'add', prerequisite }, + ]) + expect(getStagedDependencyChanges(12, 2)).toEqual([]) + }) + + it('refuses a prerequisite that is already live', () => { + expect(stageAdd({ ...args, liveEdges: [edge(1, 3)] })?.code).toBe( + 'dependency_exists', + ) + }) + + it('refuses the same prerequisite staged twice', () => { + stageAdd(args) + expect(stageAdd(args)?.code).toBe('dependency_exists') + }) + + it('refuses a prerequisite that has prerequisites of its own', () => { + expect( + stageAdd({ ...args, prerequisiteEdges: [edge(3, 9)] })?.message, + ).toBe( + 'The prerequisite "feature_3" already has a prerequisite "feature_9".', + ) + }) +}) + +describe('stageAdd, a feature that is a prerequisite', () => { + it('refuses, naming the feature that depends on it', () => { + expect(stageAdd({ ...args, dependentEdges: [edge(5, 1)] })).toEqual({ + code: 'feature_is_prerequisite', + message: + 'The feature "feature_1" is already a prerequisite for the feature "feature_5".', + }) + }) +}) + +describe('stageRemove', () => { + it('drops a staged add rather than staging both', () => { + stageAdd(args) + stageRemove(12, 1, prerequisite) + expect(getStagedDependencyChanges(12, 1)).toEqual([]) + }) + + it('stages the removal of a live prerequisite', () => { + stageRemove(12, 1, prerequisite) + expect(getStagedDependencyChanges(12, 1)).toEqual([ + { action: 'remove', prerequisite }, + ]) + }) +}) diff --git a/frontend/common/services/fakeChangeRequestDependencies.ts b/frontend/common/services/fakeChangeRequestDependencies.ts new file mode 100644 index 000000000000..4ee3c16ecb48 --- /dev/null +++ b/frontend/common/services/fakeChangeRequestDependencies.ts @@ -0,0 +1,114 @@ +// Stands in for staging a dependency in a change request (#8449) until the API +// supports it. The API ignores ?change_request today and would publish the +// change live, so a staged change never reaches it. Delete this file, and the +// branches in useFeatureDependency that call it, once #8449 ships. +import { + DependencyConflictCode, + DependencyEdge, + DependencyFeature, +} from 'common/types/responses' + +export type StagedDependencyChange = { + action: 'add' | 'remove' + prerequisite: DependencyFeature +} + +type StagedChanges = Record + +// In local storage so the change request page can read what the feature modal +// staged, across navigations and reloads. +const STORAGE_KEY = 'fakeChangeRequestDependencies' + +const read = (): StagedChanges => { + try { + return JSON.parse(localStorage.getItem(STORAGE_KEY) || '{}') + } catch { + return {} + } +} + +const write = (changes: StagedChanges) => { + try { + localStorage.setItem(STORAGE_KEY, JSON.stringify(changes)) + } catch { + // Private mode or blocked storage: the change is lost on reload. + } +} + +const keyOf = (changeRequestId: number, featureId: number) => + `${changeRequestId}-${featureId}` + +export const getStagedDependencyChanges = ( + changeRequestId: number, + featureId: number, +): StagedDependencyChange[] => read()[keyOf(changeRequestId, featureId)] ?? [] + +export type FakeConflict = { code: DependencyConflictCode; message: string } + +type StageArgs = { + changeRequestId: number + featureId: number + prerequisite: DependencyFeature + // What is live today, read from the real API by the caller. + liveEdges: DependencyEdge[] + prerequisiteEdges: DependencyEdge[] + dependentEdges: DependencyEdge[] +} + +// The same refusals the API gives a direct add, checked against live data. +export const stageAdd = ({ + changeRequestId, + dependentEdges, + featureId, + liveEdges, + prerequisite, + prerequisiteEdges, +}: StageArgs): FakeConflict | null => { + const changes = read() + const key = keyOf(changeRequestId, featureId) + const staged = changes[key] ?? [] + const isLive = liveEdges.some((e) => e.prerequisite.id === prerequisite.id) + if (isLive || staged.some((c) => c.prerequisite.id === prerequisite.id)) { + return { + code: 'dependency_exists', + message: `"${prerequisite.name}" is already a prerequisite.`, + } + } + if (dependentEdges.length) { + const [edge] = dependentEdges + return { + code: 'feature_is_prerequisite', + message: `The feature "${edge.prerequisite.name}" is already a prerequisite for the feature "${edge.feature.name}".`, + } + } + const ownEdge = prerequisiteEdges.find( + (e) => e.feature.id === prerequisite.id, + ) + if (ownEdge) { + return { + code: 'prerequisite_has_prerequisite', + message: `The prerequisite "${prerequisite.name}" already has a prerequisite "${ownEdge.prerequisite.name}".`, + } + } + changes[key] = [...staged, { action: 'add', prerequisite }] + write(changes) + return null +} + +// Removing a staged add drops it rather than staging both, as #8449 specifies. +export const stageRemove = ( + changeRequestId: number, + featureId: number, + prerequisite: DependencyFeature, +) => { + const changes = read() + const key = keyOf(changeRequestId, featureId) + const staged = changes[key] ?? [] + const stagedAdd = staged.some( + (c) => c.action === 'add' && c.prerequisite.id === prerequisite.id, + ) + changes[key] = stagedAdd + ? staged.filter((c) => c.prerequisite.id !== prerequisite.id) + : [...staged, { action: 'remove', prerequisite }] + write(changes) +} diff --git a/frontend/common/services/useChangeRequest.ts b/frontend/common/services/useChangeRequest.ts index 3a055a66dc05..a25928aab089 100644 --- a/frontend/common/services/useChangeRequest.ts +++ b/frontend/common/services/useChangeRequest.ts @@ -15,6 +15,31 @@ export const changeRequestService = service .enhanceEndpoints({ addTagTypes: ['ChangeRequest'] }) .injectEndpoints({ endpoints: (builder) => ({ + createEnvironmentChangeRequest: builder.mutation< + Res['createEnvironmentChangeRequest'], + Req['createEnvironmentChangeRequest'] + >({ + invalidatesTags: [{ id: 'LIST', type: 'ChangeRequest' }], + query: ({ approvals, environmentId, ...rest }) => ({ + body: { + ...rest, + // The modal hands back users and groups in one list. + approvals: approvals.filter((approval) => !!approval.user), + // Required even when the changes are in change sets. + feature_states: [], + group_assignments: approvals.filter((approval) => !!approval.group), + }, + method: 'POST', + url: `environments/${environmentId}/create-change-request/`, + }), + }), + deleteChangeRequest: builder.mutation({ + invalidatesTags: [{ id: 'LIST', type: 'ChangeRequest' }], + query: ({ id }) => ({ + method: 'DELETE', + url: `features/workflows/change-requests/${id}/`, + }), + }), getChangeRequests: builder.query< Res['changeRequests'], Req['getChangeRequests'] @@ -67,6 +92,8 @@ export async function updateChangeRequest( // END OF FUNCTION_EXPORTS export const { + useCreateEnvironmentChangeRequestMutation, + useDeleteChangeRequestMutation, useGetChangeRequestsQuery, useUpdateChangeRequestMutation, // END OF EXPORTS diff --git a/frontend/common/services/useFeatureDependency.ts b/frontend/common/services/useFeatureDependency.ts index d534865befef..7e4c595ac05b 100644 --- a/frontend/common/services/useFeatureDependency.ts +++ b/frontend/common/services/useFeatureDependency.ts @@ -1,5 +1,7 @@ import { Req } from 'common/types/requests' import { Res } from 'common/types/responses' +import { stagedDependencyEdge } from 'common/utils/stagedDependencyEdge' +import { stageAdd, stageRemove } from './fakeChangeRequestDependencies' import { service } from 'common/service' type DependencyTag = { id: string; type: 'FeatureDependency' } @@ -25,6 +27,16 @@ const invalidateEnvironment = (query: { { id: `LIST-${query.environmentId}`, type: 'FeatureDependency' }, ] +const dependencyUrl = (query: { + environmentId: string + featureId: number + prerequisiteFeatureId: number +}) => + `environments/${query.environmentId}/features/${query.featureId}/dependencies/${query.prerequisiteFeatureId}/` + +const dependenciesUrl = (query: { environmentId: string }, featureId: number) => + `environments/${query.environmentId}/features/${featureId}/dependencies/` + export const featureDependencyService = service .enhanceEndpoints({ addTagTypes: ['FeatureDependency'] }) .injectEndpoints({ @@ -33,10 +45,13 @@ export const featureDependencyService = service Res['featureDependency'], Req['createFeatureDependency'] >({ - invalidatesTags: (res, err, query) => invalidateEnvironment(query), + // A staged change is not live, so no list changes. + invalidatesTags: (res, err, query) => + query.changeRequestId ? [] : invalidateEnvironment(query), // The POST answers with the edge, so the row need not wait on the // refetch the invalidation triggers. onQueryStarted: async (query, { dispatch, queryFulfilled }) => { + if (query.changeRequestId) return let edge: Res['featureDependency'] try { edge = (await queryFulfilled).data @@ -60,20 +75,68 @@ export const featureDependencyService = service ), ) }, - query: (query: Req['createFeatureDependency']) => ({ - method: 'POST', - url: `environments/${query.environmentId}/features/${query.featureId}/dependencies/${query.prerequisiteFeatureId}/`, - }), + queryFn: async (query, _, _2, baseQuery) => { + if (!query.changeRequestId) { + const res = await baseQuery({ + method: 'POST', + url: dependencyUrl(query), + }) + return res.error + ? { error: res.error } + : { data: res.data as Res['featureDependency'] } + } + // Staged: checked against live data, then held by the fake. + const [liveRes, prerequisiteRes, dependentsRes] = await Promise.all([ + baseQuery({ url: dependenciesUrl(query, query.featureId) }), + baseQuery({ + url: dependenciesUrl(query, query.prerequisiteFeatureId), + }), + baseQuery({ + url: `environments/${query.environmentId}/features/${query.featureId}/dependents/`, + }), + ]) + const failed = + liveRes.error || prerequisiteRes.error || dependentsRes.error + if (failed) return { error: failed } + const prerequisite = { + id: query.prerequisiteFeatureId, + name: query.prerequisiteName ?? '', + } + const conflict = stageAdd({ + changeRequestId: query.changeRequestId, + dependentEdges: (dependentsRes.data as Res['featureDependents']) + .results, + featureId: query.featureId, + liveEdges: (liveRes.data as Res['featureDependencies']).results, + prerequisite, + prerequisiteEdges: ( + prerequisiteRes.data as Res['featureDependencies'] + ).results, + }) + if (conflict) return { error: { data: conflict, status: 400 } } + return { data: stagedDependencyEdge(query.featureId, prerequisite) } + }, }), deleteFeatureDependency: builder.mutation< void, Req['deleteFeatureDependency'] >({ - invalidatesTags: (res, err, query) => invalidateEnvironment(query), - query: (query: Req['deleteFeatureDependency']) => ({ - method: 'DELETE', - url: `environments/${query.environmentId}/features/${query.featureId}/dependencies/${query.prerequisiteFeatureId}/`, - }), + invalidatesTags: (res, err, query) => + query.changeRequestId ? [] : invalidateEnvironment(query), + queryFn: async (query, _, _2, baseQuery) => { + if (!query.changeRequestId) { + const res = await baseQuery({ + method: 'DELETE', + url: dependencyUrl(query), + }) + return res.error ? { error: res.error } : { data: undefined } + } + stageRemove(query.changeRequestId, query.featureId, { + id: query.prerequisiteFeatureId, + name: query.prerequisiteName ?? '', + }) + return { data: undefined } + }, }), getFeatureDependencies: builder.query< Res['featureDependencies'], diff --git a/frontend/common/services/useFeatureState.ts b/frontend/common/services/useFeatureState.ts index bc2ffcd37211..3bd4f516bc96 100644 --- a/frontend/common/services/useFeatureState.ts +++ b/frontend/common/services/useFeatureState.ts @@ -109,5 +109,6 @@ export async function updateFeatureState( export const { useGetAllEnvironmentFeatureStatesQuery, useGetFeatureStatesQuery, + useLazyGetFeatureStatesQuery, useUpdateFeatureStateMutation, } = featureStateService diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index 8887023bd172..f312e8a55805 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -1,4 +1,6 @@ import { + ChangeSet, + MultivariateFeatureStateValue, Account, ExternalResource, FeatureState, @@ -875,12 +877,38 @@ export type Req = { environmentId: string featureId: number prerequisiteFeatureId: number + // Stages the change in this change request instead of publishing it. + changeRequestId?: number + // Read only by the fake that stands in for #8449. + prerequisiteName?: string } deleteFeatureDependency: { environmentId: string featureId: number prerequisiteFeatureId: number + changeRequestId?: number + prerequisiteName?: string } + createEnvironmentChangeRequest: { + environmentId: string + title: string + description: string + live_from?: string + approvals: Approval[] + change_sets?: (Omit & { + // New states, so no ids, theirs or their variations'. + feature_states_to_update: (Omit< + FeatureState, + 'id' | 'multivariate_feature_state_values' + > & { + multivariate_feature_state_values?: Omit< + MultivariateFeatureStateValue, + 'id' + >[] + })[] + })[] + } + deleteChangeRequest: { id: number } getSamlConfiguration: { name: string } getSamlConfigurations: { organisation_id: number } getSamlConfigurationMetadata: { name: string } diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index f4512f1b29cb..4cd75465d7b5 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -1590,6 +1590,7 @@ export type Res = { groupWithRole: PagedResponse changeRequests: PagedResponse updateChangeRequest: ChangeRequest + createEnvironmentChangeRequest: ChangeRequest groupSummaries: UserGroupSummary[] supportedContentType: ContentType[] externalResource: PagedResponse diff --git a/frontend/common/utils/stagedDependencyEdge.ts b/frontend/common/utils/stagedDependencyEdge.ts new file mode 100644 index 000000000000..fc4de2a33cce --- /dev/null +++ b/frontend/common/utils/stagedDependencyEdge.ts @@ -0,0 +1,18 @@ +import { DependencyEdge, DependencyFeature } from 'common/types/responses' + +// A prerequisite held for a change request has no segment until the change +// request is published, so it gets a stand-in. +export const stagedDependencyEdge = ( + featureId: number, + prerequisite: DependencyFeature, +): DependencyEdge => ({ + feature: { id: featureId, name: '' }, + prerequisite, + segment: { + condition_json_path: '', + id: -prerequisite.id, + is_system: true, + name: '', + rules: [], + }, +}) From 33685bc49eb5473c45d402c00dd5e8e119620b8a Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 9 Oct 2026 16:52:41 -0300 Subject: [PATCH 02/11] feat(dependencies): stage dependency changes in a change request Where an environment requires change requests, the Dependencies tab holds adds and removes until a change request is created, then stages each one in it. If any is refused, the change request is deleted and the refused row is marked. Environments on v1 feature versioning get a read-only tab. As the API refuses an empty change request, it carries the feature's current value unchanged. Gated behind flag_dependencies_change_requests. Refs #8450 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../modals/create-feature/index.tsx | 9 +- .../ChangeRequestFooter.tsx | 30 +++ .../ChangeRequestNotice.tsx | 61 ++++++ .../FeatureDependenciesTab.tsx | 80 ++++++-- .../PrerequisitesTable.scss | 24 +++ .../PrerequisitesTable/PrerequisitesTable.tsx | 75 ++++++-- .../__tests__/dependenciesMode.test.ts | 17 ++ .../__tests__/prerequisiteState.test.ts | 39 ++++ .../__tests__/stagingError.test.ts | 38 ++++ .../dependenciesMode.ts | 12 ++ .../hooks/useDependencies.ts | 3 + .../hooks/usePrerequisites.tsx | 13 +- .../hooks/useStagedPrerequisites.tsx | 180 ++++++++++++++++++ .../prerequisiteState.ts | 37 +++- .../FeatureDependenciesTab/stagingError.ts | 21 ++ .../unchangedChangeSet.ts | 37 ++++ 16 files changed, 641 insertions(+), 35 deletions(-) create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter.tsx create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestNotice.tsx create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/dependenciesMode.test.ts create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/stagingError.test.ts create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/dependenciesMode.ts create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/stagingError.ts create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/unchangedChangeSet.ts diff --git a/frontend/web/components/modals/create-feature/index.tsx b/frontend/web/components/modals/create-feature/index.tsx index 6ce0a4165751..53d77414e0c3 100644 --- a/frontend/web/components/modals/create-feature/index.tsx +++ b/frontend/web/components/modals/create-feature/index.tsx @@ -460,12 +460,13 @@ const CreateFeatureModal: FC = (props) => { controlValue < 0 const isVersionedChangeRequest = existingChangeRequest && isVersioned const hideIdentityOverridesTab = Utils.getShouldHideIdentityOverridesTab() - // Change requests are out of scope for the first version, so the tab stays - // hidden where they are enforced. See #8428. + // Where change requests are enforced, the tab waits on its own flag while the + // API for staging dependencies (#8449) is faked. const isDependenciesTabEnabled = Utils.getFlagsmithHasFeature('flag_dependencies') && !existingChangeRequest && - !is4Eyes + (!is4Eyes || + Utils.getFlagsmithHasFeature('flag_dependencies_change_requests')) let regexValid = true try { @@ -835,6 +836,8 @@ const CreateFeatureModal: FC = (props) => { environmentName={environmentName} projectId={projectId} projectFlag={projectFlag} + requiresChangeRequests={is4Eyes} + isVersioned={isVersioned} onSelectFeature={(featureId) => { // The features list, never the current page: it // owns the deep link and hydrates an off-page diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter.tsx new file mode 100644 index 000000000000..c0b2192718a8 --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter.tsx @@ -0,0 +1,30 @@ +import { FC } from 'react' +import Button from 'components/base/forms/Button' + +type ChangeRequestFooterProps = { + count: number + isSubmitting: boolean + onCreate: () => void + onDiscard: () => void +} + +const ChangeRequestFooter: FC = ({ + count, + isSubmitting, + onCreate, + onDiscard, +}) => ( +
+ + {count} {count === 1 ? 'change' : 'changes'} staged + + + +
+) + +export default ChangeRequestFooter diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestNotice.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestNotice.tsx new file mode 100644 index 000000000000..cba94fe8e7d7 --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestNotice.tsx @@ -0,0 +1,61 @@ +import { FC } from 'react' +import Banner from 'components/base/Banner' +import Button from 'components/base/forms/Button' +import { DependenciesMode } from './dependenciesMode' + +type ChangeRequestNoticeProps = { + mode: DependenciesMode + canCreate: boolean + environmentId: string + environmentName: string + projectId: number +} + +const ChangeRequestNotice: FC = ({ + canCreate, + environmentId, + environmentName, + mode, + projectId, +}) => { + if (mode === 'changeRequest' && !canCreate) { + return ( + + {environmentName} requires change requests, and you don't have + permission to create them. + + ) + } + if (mode === 'changeRequest') { + return ( + + {environmentName} requires change requests. Dependency changes are + staged here and go live when the change request is published. + + ) + } + if (mode === 'readOnly') { + return ( + + Enable feature versioning + + } + > + Dependency changes need feature versioning to go through a change + request. + + ) + } + return null +} + +export default ChangeRequestNotice diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx index e2526150d0df..09cfb74b4dbe 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx @@ -17,7 +17,12 @@ import DependentFeatures from './DependentFeatures' import FeatureDependenciesSkeleton from './FeatureDependenciesSkeleton' import PrerequisitesTable from './PrerequisitesTable' import { PrerequisitesEmptyState } from './DependenciesEmptyStates' +import ChangeRequestFooter from './ChangeRequestFooter' +import ChangeRequestNotice from './ChangeRequestNotice' +import { getDependenciesMode } from './dependenciesMode' +import { withStagedChanges } from './prerequisiteState' import { usePrerequisites } from './hooks/usePrerequisites' +import { useStagedPrerequisites } from './hooks/useStagedPrerequisites' // Flag dependencies have no docs page of their own yet, so this points at the // nearest one that exists. @@ -29,6 +34,8 @@ type FeatureDependenciesTabProps = { environmentName: string projectId: number projectFlag: ProjectFlag + requiresChangeRequests: boolean + isVersioned: boolean // Opens another feature's modal on this tab. onSelectFeature: (featureId: number) => void } @@ -36,16 +43,27 @@ type FeatureDependenciesTabProps = { const FeatureDependenciesTab: FC = ({ environmentId, environmentName, + isVersioned, onSelectFeature, projectFlag, projectId, + requiresChangeRequests, }) => { - const { isLoading: isLoadingPermission, permission: canManage } = - useHasPermission({ - id: environmentId, - level: 'environment', - permission: EnvironmentPermission.MANAGE_SEGMENT_OVERRIDES, - }) + const mode = getDependenciesMode(requiresChangeRequests, isVersioned) + const isChangeRequest = mode === 'changeRequest' + const { isLoading: isLoadingPermission, permission } = useHasPermission({ + id: environmentId, + level: 'environment', + permission: isChangeRequest + ? EnvironmentPermission.CREATE_CHANGE_REQUEST + : EnvironmentPermission.MANAGE_SEGMENT_OVERRIDES, + }) + const canManage = permission && mode !== 'readOnly' + const staging = useStagedPrerequisites({ + environmentId, + projectFlag, + projectId, + }) const { addingName, @@ -53,6 +71,7 @@ const FeatureDependenciesTab: FC = ({ dependentEdges, flashedId, isCreating, + isEnabled, isError, isLoading, isOpenForAdding, @@ -80,6 +99,16 @@ const FeatureDependenciesTab: FC = ({ return } + const shownRows = isChangeRequest + ? withStagedChanges(rows, staging.staged, projectFlag.id, isEnabled) + : rows + const handleAdd = isChangeRequest + ? (feature: ProjectFlag) => { + staging.add(feature) + onAddingChange(false) + } + : onAdd + return (
@@ -107,19 +136,29 @@ const FeatureDependenciesTab: FC = ({
)} + + {/* addingName too: on a first add the picker has closed and no row has arrived yet, and the empty state would take the pending row's place. */} - {rows.length || isOpenForAdding || addingName ? ( + {shownRows.length || isOpenForAdding || addingName ? ( <> staging.undo(edge.prerequisite.id)} onSelect={(edge) => onSelectFeature(edge.prerequisite.id)} addControl={ isOpenForAdding && ( @@ -132,9 +171,9 @@ const FeatureDependenciesTab: FC = ({ // list. ignore={[ projectFlag.id, - ...rows.map((row) => row.edge.prerequisite.id), + ...shownRows.map((row) => row.edge.prerequisite.id), ]} - onChange={onAdd} + onChange={handleAdd} /> ) } @@ -159,6 +198,16 @@ const FeatureDependenciesTab: FC = ({ )} + {!!staging.error && ( + + {staging.error.message} + + )} + {!isPrerequisite && canManage && !isOpenForAdding && ( )} + {isChangeRequest && !!staging.staged.length && ( + + )} + .btn { + min-width: 0; +} + +.prerequisite-name__text { + min-width: 0; +} + +// Held for a change request: still listed, but on its way out. On the inner +// span, as .btn-link clears text-decoration with !important. +.prerequisite-row--staged-remove .prerequisite-name .btn-link { + color: var(--color-text-secondary); +} + +.prerequisite-row--staged-remove .prerequisite-name__text { + text-decoration: line-through; +} + +// On the cell, as with the input row, so hover does not replace it. +.prerequisite-row--refused td { + background: var(--color-surface-danger); +} + // The select needs permission to shrink or it pushes the row past the panel. .prerequisite-add-control > :first-child { flex: 1 1 auto; diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PrerequisitesTable/PrerequisitesTable.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PrerequisitesTable/PrerequisitesTable.tsx index e846cef51f41..7a061c894b4e 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PrerequisitesTable/PrerequisitesTable.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PrerequisitesTable/PrerequisitesTable.tsx @@ -3,6 +3,7 @@ import cn from 'classnames' import { DependencyEdge } from 'common/types/responses' import { colorSurfaceAction, colorSurfaceEmphasis } from 'common/theme/tokens' import Button from 'components/base/forms/Button' +import Chip from 'components/base/Chip' import Table, { TableBody, TableCell, @@ -26,6 +27,10 @@ type PrerequisitesTableProps = { flashedId?: number isRemoving?: number onRemove: (edge: DependencyEdge) => void + // Takes back a held removal. + onUndo?: (edge: DependencyEdge) => void + // The row a change request refused, marked until the user acts. + refusedId?: number onSelect: (edge: DependencyEdge) => void // The prerequisite being added, shown as a row of its own until the POST // resolves. Otherwise nothing moves between picking one and it appearing. @@ -86,6 +91,8 @@ const PrerequisitesTable: FC = ({ onCancelAdd, onRemove, onSelect, + onUndo, + refusedId, rows, }) => ( @@ -103,25 +110,43 @@ const PrerequisitesTable: FC = ({ - {rows.map(({ edge, isEnabled, isMet }) => ( + {rows.map(({ edge, isEnabled, isMet, staged }) => ( onSelect(edge)} pending={isRemoving === edge.prerequisite.id} > - - + + {/* The name gives way to the chip, so a long name cannot hide + what is about to happen to it. */} +
+ + {!!staged && ( + + {staged === 'add' ? 'Staged: add' : 'Staged: remove'} + + )} +
@@ -131,21 +156,35 @@ const PrerequisitesTable: FC = ({ {canManage && ( - {/* An edge from a hand-written segment condition cannot be removed - through the dependencies API, so it gets no control. */} - {edge.segment.is_system && ( + {staged === 'remove' && onUndo ? ( { e.stopPropagation() - onRemove(edge) + onUndo(edge) }} - aria-label={`Remove ${edge.prerequisite.name} as a prerequisite`} + aria-label={`Keep ${edge.prerequisite.name} as a prerequisite`} > - + + ) : ( + // An edge from a hand-written segment condition cannot be + // removed through the dependencies API, so it gets no control. + edge.segment.is_system && ( + { + e.stopPropagation() + onRemove(edge) + }} + aria-label={`Remove ${edge.prerequisite.name} as a prerequisite`} + > + + + ) )} )} diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/dependenciesMode.test.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/dependenciesMode.test.ts new file mode 100644 index 000000000000..a3cceb07fcee --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/dependenciesMode.test.ts @@ -0,0 +1,17 @@ +import { getDependenciesMode } from 'components/modals/create-feature/tabs/FeatureDependenciesTab/dependenciesMode' + +describe('getDependenciesMode', () => { + it.each([ + [false, false, 'direct'], + [false, true, 'direct'], + [true, true, 'changeRequest'], + [true, false, 'readOnly'], + ] as const)( + 'change requests %s, versioned %s: %s', + (requiresChangeRequests, isVersioned, expected) => { + expect(getDependenciesMode(requiresChangeRequests, isVersioned)).toBe( + expected, + ) + }, + ) +}) diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/prerequisiteState.test.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/prerequisiteState.test.ts index 8b3d39b20a1c..3cf2f26a09ce 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/prerequisiteState.test.ts +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/prerequisiteState.test.ts @@ -4,6 +4,7 @@ import { isBlocked, toPrerequisiteRow, toPrerequisiteRows, + withStagedChanges, } from 'components/modals/create-feature/tabs/FeatureDependenciesTab/prerequisiteState' const edge = ( @@ -141,3 +142,41 @@ describe('describeOffCount', () => { ) }) }) + +describe('withStagedChanges', () => { + const live = [ + toPrerequisiteRow(edge(1), true), + toPrerequisiteRow(edge(2), false), + ] + const isEnabled = (id: number) => id === 3 + + it('marks a held removal on its live row', () => { + const rows = withStagedChanges( + live, + [{ action: 'remove', prerequisite: { id: 2, name: 'prereq_2' } }], + 1, + isEnabled, + ) + expect(rows.map((r) => r.staged)).toEqual([undefined, 'remove']) + }) + + it('appends a held add after the live rows, with its current state', () => { + const rows = withStagedChanges( + live, + [{ action: 'add', prerequisite: { id: 3, name: 'prereq_3' } }], + 1, + isEnabled, + ) + expect(rows).toHaveLength(3) + expect(rows[2]).toMatchObject({ + isEnabled: true, + isMet: true, + staged: 'add', + }) + expect(rows[2].edge.prerequisite).toEqual({ id: 3, name: 'prereq_3' }) + }) + + it('leaves the rows alone with nothing held', () => { + expect(withStagedChanges(live, [], 1, isEnabled)).toEqual(live) + }) +}) diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/stagingError.test.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/stagingError.test.ts new file mode 100644 index 000000000000..e4f2306a28c1 --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/__tests__/stagingError.test.ts @@ -0,0 +1,38 @@ +import { + describeApiError, + toStagingError, +} from 'components/modals/create-feature/tabs/FeatureDependenciesTab/stagingError' + +describe('describeApiError', () => { + it.each([ + [{ data: { code: 'x', message: 'Refused.' } }, 'Refused.'], + [{ data: { detail: 'Not found.' } }, 'Not found.'], + [ + { data: { feature_states: ['This field is required.'] } }, + 'feature_states: This field is required.', + ], + [{ status: 500 }, undefined], + [undefined, undefined], + ])('%j reads as %j', (error, expected) => { + expect(describeApiError(error)).toBe(expected) + }) +}) + +describe('toStagingError', () => { + it('keeps a staging error, with the row it refers to', () => { + const error = { message: 'Refused.', prerequisiteId: 3 } + expect(toStagingError(error, 'Fallback')).toBe(error) + }) + + it('reads an API error', () => { + expect(toStagingError({ data: { detail: 'Nope.' } }, 'Fallback')).toEqual({ + message: 'Nope.', + }) + }) + + it('falls back when there is nothing to read', () => { + expect(toStagingError({ status: 500 }, 'Fallback')).toEqual({ + message: 'Fallback', + }) + }) +}) diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/dependenciesMode.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/dependenciesMode.ts new file mode 100644 index 000000000000..822e49634d5a --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/dependenciesMode.ts @@ -0,0 +1,12 @@ +// How dependency changes reach an environment: written directly, held for a +// change request, or not at all where change requests run on v1 versioning, +// which #8449 does not support. +export type DependenciesMode = 'direct' | 'changeRequest' | 'readOnly' + +export const getDependenciesMode = ( + requiresChangeRequests: boolean, + isVersioned: boolean, +): DependenciesMode => { + if (!requiresChangeRequests) return 'direct' + return isVersioned ? 'changeRequest' : 'readOnly' +} 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 378a09488ad1..e03e04a69b61 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 @@ -80,6 +80,9 @@ export const useDependencies = ({ }).unwrap(), dependentEdges: dependents?.results ?? [], isCreating, + isEnabled: (featureId: number) => + !!featureList?.results.find((feature) => feature.id === featureId) + ?.environment_feature_state?.enabled, // 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. diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/usePrerequisites.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/usePrerequisites.tsx index 6901bd19bd98..081b129eb0a2 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/usePrerequisites.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/usePrerequisites.tsx @@ -28,8 +28,16 @@ export const usePrerequisites = ({ const [addingName, setAddingName] = useState() const { flash, flashedId } = useRowFlash() - const { add, dependentEdges, isCreating, isError, isLoading, remove, rows } = - useDependencies({ environmentId, featureId: projectFlag.id, projectId }) + const { + add, + dependentEdges, + isCreating, + isEnabled, + isError, + isLoading, + remove, + rows, + } = useDependencies({ environmentId, featureId: projectFlag.id, projectId }) // A refusal belongs to the add it came from, so it ends with it. const onAddingChange = useCallback((adding: boolean) => { @@ -84,6 +92,7 @@ export const usePrerequisites = ({ dependentEdges, flashedId, isCreating, + isEnabled, isError, isLoading, // A flag that gains a dependent cannot take prerequisites, so the picker diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx new file mode 100644 index 000000000000..cb324c3d95a0 --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx @@ -0,0 +1,180 @@ +import { useState } from 'react' +import { + DependencyEdge, + DependencyFeature, + ProjectFlag, +} from 'common/types/responses' +import { Req } from 'common/types/requests' +import { + useCreateEnvironmentChangeRequestMutation, + useDeleteChangeRequestMutation, +} from 'common/services/useChangeRequest' +import { + useCreateFeatureDependencyMutation, + useDeleteFeatureDependencyMutation, +} from 'common/services/useFeatureDependency' +import { useLazyGetFeatureStatesQuery } from 'common/services/useFeatureState' +import { useProjectEnvironments } from 'common/hooks/useProjectEnvironments' +import ChangeRequestModal from 'components/modals/ChangeRequestModal' +import { + StagingError, + describeApiError, + toStagingError, +} from 'components/modals/create-feature/tabs/FeatureDependenciesTab/stagingError' +import { unchangedChangeSet } from 'components/modals/create-feature/tabs/FeatureDependenciesTab/unchangedChangeSet' + +export type StagedChange = { + action: 'add' | 'remove' + prerequisite: DependencyFeature +} + +type UseStagedPrerequisitesArgs = { + environmentId: string + projectId: number + projectFlag: ProjectFlag +} + +type ChangeRequestFields = Omit< + Req['createEnvironmentChangeRequest'], + 'environmentId' +> + +/** + * Where change requests are required, nothing is written until the change + * request exists. Changes are held here, then staged in it one by one. + */ +export const useStagedPrerequisites = ({ + environmentId, + projectFlag, + projectId, +}: UseStagedPrerequisitesArgs) => { + const [staged, setStaged] = useState([]) + const [error, setError] = useState(null) + const [isSubmitting, setIsSubmitting] = useState(false) + + const [createChangeRequest] = useCreateEnvironmentChangeRequestMutation() + const [deleteChangeRequest] = useDeleteChangeRequestMutation() + const [createDependency] = useCreateFeatureDependencyMutation() + const [deleteDependency] = useDeleteFeatureDependencyMutation() + const [getFeatureStates] = useLazyGetFeatureStatesQuery() + const { getEnvironmentIdFromKey } = useProjectEnvironments(projectId) + + const drop = (prerequisiteId: number) => + setStaged((prev) => + prev.filter((change) => change.prerequisite.id !== prerequisiteId), + ) + + const add = (feature: ProjectFlag) => { + setError(null) + setStaged((prev) => [ + ...prev, + { action: 'add', prerequisite: { id: feature.id, name: feature.name } }, + ]) + } + + // A held add is dropped rather than held as both. + const remove = (edge: DependencyEdge) => { + setError(null) + if ( + staged.some((change) => change.prerequisite.id === edge.prerequisite.id) + ) + return drop(edge.prerequisite.id) + setStaged((prev) => [ + ...prev, + { action: 'remove', prerequisite: edge.prerequisite }, + ]) + } + + const stage = async (changeRequestId: number) => { + // In order, so the first refusal names the change that caused it. + for (const { action, prerequisite } of staged) { + const query = { + changeRequestId, + environmentId, + featureId: projectFlag.id, + prerequisiteFeatureId: prerequisite.id, + prerequisiteName: prerequisite.name, + } + try { + await (action === 'add' + ? createDependency(query).unwrap() + : deleteDependency(query).unwrap()) + } catch (e) { + const error: StagingError = { + message: + describeApiError(e) ?? `Could not stage ${prerequisite.name}.`, + prerequisiteId: prerequisite.id, + } + throw error + } + } + } + + const submitChangeRequest = async (fields: ChangeRequestFields) => { + setIsSubmitting(true) + setError(null) + let changeRequestId: number | undefined + try { + const featureStates = await getFeatureStates({ + environment: getEnvironmentIdFromKey(environmentId), + feature: projectFlag.id, + }).unwrap() + const changeSet = unchangedChangeSet( + projectFlag.id, + featureStates.results, + fields.live_from, + ) + changeRequestId = ( + await createChangeRequest({ + environmentId, + ...fields, + change_sets: changeSet ? [changeSet] : undefined, + }).unwrap() + ).id + await stage(changeRequestId) + setStaged([]) + toast( + <> + Change request created.{' '} + + View it + + , + ) + } catch (e) { + // A change request missing some of the changes is worse than none. + if (changeRequestId) await deleteChangeRequest({ id: changeRequestId }) + setError(toStagingError(e, 'Could not create the change request.')) + } finally { + setIsSubmitting(false) + } + } + + const openChangeRequest = () => + openModal2( + 'New Change Request', + { + closeModal2() + submitChangeRequest({ approvals, description, live_from, title }) + }} + />, + ) + + return { + add, + discard: () => { + setError(null) + setStaged([]) + }, + error, + isSubmitting, + openChangeRequest, + remove, + staged, + undo: drop, + } +} 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 4fe5774d6fcc..cbd6d16dd91e 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/prerequisiteState.ts +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/prerequisiteState.ts @@ -1,12 +1,21 @@ -import { DependencyEdge, ProjectFlag } from 'common/types/responses' +import { + DependencyEdge, + DependencyFeature, + ProjectFlag, +} from 'common/types/responses' +import { stagedDependencyEdge } from 'common/utils/stagedDependencyEdge' export type PrerequisiteRow = { edge: DependencyEdge isEnabled: boolean // Undefined where the rule cannot be read. See toPrerequisiteRow. isMet?: boolean + // Held for a change request, not live yet. + staged?: StagedAction } +export type StagedAction = 'add' | 'remove' + // A system edge always carries the hardcoded `enabled != true` condition from // _get_or_create_dependency_segment in api/features/dependencies/services.py, // so on means met. A hand-written segment referencing a flag is listed here @@ -35,6 +44,32 @@ export const toPrerequisiteRows = ( ), ) +// Live rows with their held removals marked, then the held adds. +export const withStagedChanges = ( + rows: PrerequisiteRow[], + staged: { action: StagedAction; prerequisite: DependencyFeature }[], + featureId: number, + isEnabled: (prerequisiteId: number) => boolean, +): PrerequisiteRow[] => [ + ...rows.map((row) => + staged.some( + (c) => + c.action === 'remove' && c.prerequisite.id === row.edge.prerequisite.id, + ) + ? { ...row, staged: 'remove' as const } + : row, + ), + ...staged + .filter((c) => c.action === 'add') + .map(({ prerequisite }) => ({ + ...toPrerequisiteRow( + stagedDependencyEdge(featureId, prerequisite), + isEnabled(prerequisite.id), + ), + staged: 'add' as const, + })), +] + export const isBlocked = (rows: PrerequisiteRow[]): boolean => rows.some((row) => row.isMet === false) diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/stagingError.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/stagingError.ts new file mode 100644 index 000000000000..4e3d3fce57b3 --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/stagingError.ts @@ -0,0 +1,21 @@ +export type StagingError = { message: string; prerequisiteId?: number } + +const isStagingError = (e: unknown): e is StagingError => + typeof e === 'object' && + e !== null && + typeof (e as StagingError).message === 'string' + +// The API answers with { message }, { detail } or { field: [messages] }. +export const describeApiError = (e: unknown): string | undefined => { + const data = typeof e === 'object' && e !== null && 'data' in e && e.data + if (!data || typeof data !== 'object') return + if ('message' in data && typeof data.message === 'string') return data.message + if ('detail' in data && typeof data.detail === 'string') return data.detail + const [field, messages] = Object.entries(data)[0] ?? [] + return field + ? `${field}: ${([] as unknown[]).concat(messages).join(' ')}` + : undefined +} + +export const toStagingError = (e: unknown, fallback: string): StagingError => + isStagingError(e) ? e : { message: describeApiError(e) ?? fallback } diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/unchangedChangeSet.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/unchangedChangeSet.ts new file mode 100644 index 000000000000..51b1ff6b4079 --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/unchangedChangeSet.ts @@ -0,0 +1,37 @@ +import { FeatureState } from 'common/types/responses' +import { Req } from 'common/types/requests' + +type ChangeSetBody = NonNullable< + Req['createEnvironmentChangeRequest']['change_sets'] +>[number] + +// Until #8449, the API refuses a change request with no changes, so it +// carries the feature's current value unchanged. If the value changes before +// this is published, publishing puts the old one back; the page flags it as a +// conflict. +export const unchangedChangeSet = ( + featureId: number, + featureStates: FeatureState[], + liveFrom: string | undefined, +): ChangeSetBody | undefined => { + const current = featureStates.find( + (state) => !state.feature_segment && !state.identity, + ) + if (!current) return + const { id: _id, multivariate_feature_state_values, ...state } = current + return { + feature: featureId, + feature_states_to_create: [], + feature_states_to_update: [ + { + ...state, + multivariate_feature_state_values: + multivariate_feature_state_values?.map( + ({ id: _valueId, ...value }) => value, + ), + }, + ], + live_from: liveFrom ?? new Date().toISOString(), + segment_ids_to_delete_overrides: [], + } +} From f0b6c7180acbf3453b12db2264370f61b2136921 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 9 Oct 2026 10:23:51 -0300 Subject: [PATCH 03/11] feat(dependencies): show dependency changes waiting in change requests The Dependencies tab lists the prerequisite changes held in each open or scheduled change request for the feature, linking to it. The change request page shows the prerequisite changes it would publish. Both read from the fake that stands in for #8449, and a deleted change request takes its staged changes with it. Gated behind flag_dependencies_change_requests. Refs #8450 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../fakeChangeRequestDependencies.test.ts | 32 +++++++ .../services/fakeChangeRequestDependencies.ts | 30 +++++-- frontend/common/services/useChangeRequest.ts | 10 +++ .../common/services/useFeatureDependency.ts | 42 ++++++++- frontend/common/types/requests.ts | 2 + frontend/common/types/responses.ts | 13 +++ .../DependencyChangesTable.scss | 4 + .../DependencyChangesTable.tsx | 47 ++++++++++ .../DependencyChangesTable/index.ts | 1 + .../web/components/diff/DiffDependencies.tsx | 55 ++++++++++++ .../FeatureDependenciesTab.tsx | 9 ++ .../PendingChangeRequests.tsx | 89 +++++++++++++++++++ .../hooks/useStagedPrerequisites.tsx | 9 +- .../pages/ChangeRequestDetailPage.tsx | 9 ++ 14 files changed, 336 insertions(+), 16 deletions(-) create mode 100644 frontend/web/components/DependencyChangesTable/DependencyChangesTable.scss create mode 100644 frontend/web/components/DependencyChangesTable/DependencyChangesTable.tsx create mode 100644 frontend/web/components/DependencyChangesTable/index.ts create mode 100644 frontend/web/components/diff/DiffDependencies.tsx create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests.tsx diff --git a/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts b/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts index 388ba64583cd..07a9355a1a31 100644 --- a/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts +++ b/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts @@ -1,5 +1,8 @@ import { DependencyEdge } from 'common/types/responses' import { + discardChangeRequest, + getPendingByChangeRequest, + getPendingByFeature, getStagedDependencyChanges, stageAdd, stageRemove, @@ -84,3 +87,32 @@ describe('stageRemove', () => { ]) }) }) + +describe('pending changes', () => { + it('lists staged changes by feature and by change request', () => { + stageAdd(args) + stageRemove(14, 1, { id: 4, name: 'feature_4' }) + stageAdd({ ...args, featureId: 2 }) + expect(getPendingByFeature(1).map((e) => e.changeRequestId)).toEqual([ + 12, 14, + ]) + expect(getPendingByChangeRequest(12).map((e) => e.featureId)).toEqual([ + 1, 2, + ]) + }) + + it('drops a change request emptied by its own removals', () => { + stageAdd(args) + stageRemove(12, 1, prerequisite) + expect(getPendingByFeature(1)).toEqual([]) + }) + + it('forgets every feature of a discarded change request', () => { + stageAdd(args) + stageAdd({ ...args, featureId: 2 }) + stageAdd({ ...args, changeRequestId: 120 }) + discardChangeRequest(12) + expect(getPendingByChangeRequest(12)).toEqual([]) + expect(getPendingByChangeRequest(120)).toHaveLength(1) + }) +}) diff --git a/frontend/common/services/fakeChangeRequestDependencies.ts b/frontend/common/services/fakeChangeRequestDependencies.ts index 4ee3c16ecb48..c7a9bcc74784 100644 --- a/frontend/common/services/fakeChangeRequestDependencies.ts +++ b/frontend/common/services/fakeChangeRequestDependencies.ts @@ -6,13 +6,10 @@ import { DependencyConflictCode, DependencyEdge, DependencyFeature, + PendingDependencyChanges, + StagedDependencyChange, } from 'common/types/responses' -export type StagedDependencyChange = { - action: 'add' | 'remove' - prerequisite: DependencyFeature -} - type StagedChanges = Record // In local storage so the change request page can read what the feature modal @@ -112,3 +109,26 @@ export const stageRemove = ( : [...staged, { action: 'remove', prerequisite }] write(changes) } + +const entries = (): PendingDependencyChanges[] => + Object.entries(read()) + .filter(([, changes]) => changes.length) + .map(([key, changes]) => { + const [changeRequestId, featureId] = key.split('-').map(Number) + return { changeRequestId, changes, featureId } + }) + +export const getPendingByFeature = (featureId: number) => + entries().filter((entry) => entry.featureId === featureId) + +export const getPendingByChangeRequest = (changeRequestId: number) => + entries().filter((entry) => entry.changeRequestId === changeRequestId) + +// A deleted change request takes its staged changes with it. +export const discardChangeRequest = (changeRequestId: number) => { + const changes = read() + Object.keys(changes) + .filter((key) => key.startsWith(`${changeRequestId}-`)) + .forEach((key) => delete changes[key]) + write(changes) +} diff --git a/frontend/common/services/useChangeRequest.ts b/frontend/common/services/useChangeRequest.ts index a25928aab089..5accf6ce9b33 100644 --- a/frontend/common/services/useChangeRequest.ts +++ b/frontend/common/services/useChangeRequest.ts @@ -10,6 +10,7 @@ import Utils from 'common/utils/utils' import sortBy from 'lodash/sortBy' import moment from 'moment' import transformCorePaging from 'common/transformCorePaging' +import { discardChangeRequest } from './fakeChangeRequestDependencies' export const changeRequestService = service .enhanceEndpoints({ addTagTypes: ['ChangeRequest'] }) @@ -35,6 +36,15 @@ export const changeRequestService = service }), deleteChangeRequest: builder.mutation({ invalidatesTags: [{ id: 'LIST', type: 'ChangeRequest' }], + // Until #8449, staged dependencies live outside the API. + onQueryStarted: async ({ id }, { queryFulfilled }) => { + try { + await queryFulfilled + discardChangeRequest(id) + } catch { + // The caller reports it. + } + }, query: ({ id }) => ({ method: 'DELETE', url: `features/workflows/change-requests/${id}/`, diff --git a/frontend/common/services/useFeatureDependency.ts b/frontend/common/services/useFeatureDependency.ts index 7e4c595ac05b..d4b5b32522ad 100644 --- a/frontend/common/services/useFeatureDependency.ts +++ b/frontend/common/services/useFeatureDependency.ts @@ -1,7 +1,12 @@ import { Req } from 'common/types/requests' import { Res } from 'common/types/responses' import { stagedDependencyEdge } from 'common/utils/stagedDependencyEdge' -import { stageAdd, stageRemove } from './fakeChangeRequestDependencies' +import { + getPendingByChangeRequest, + getPendingByFeature, + stageAdd, + stageRemove, +} from './fakeChangeRequestDependencies' import { service } from 'common/service' type DependencyTag = { id: string; type: 'FeatureDependency' } @@ -37,6 +42,11 @@ const dependencyUrl = (query: { const dependenciesUrl = (query: { environmentId: string }, featureId: number) => `environments/${query.environmentId}/features/${featureId}/dependencies/` +const pendingTags = (featureId: number): DependencyTag[] => [ + { id: `PENDING-${featureId}`, type: 'FeatureDependency' }, + { id: 'PENDING', type: 'FeatureDependency' }, +] + export const featureDependencyService = service .enhanceEndpoints({ addTagTypes: ['FeatureDependency'] }) .injectEndpoints({ @@ -45,9 +55,11 @@ export const featureDependencyService = service Res['featureDependency'], Req['createFeatureDependency'] >({ - // A staged change is not live, so no list changes. + // A staged change is not live, so only the pending lists change. invalidatesTags: (res, err, query) => - query.changeRequestId ? [] : invalidateEnvironment(query), + query.changeRequestId + ? pendingTags(query.featureId) + : invalidateEnvironment(query), // The POST answers with the edge, so the row need not wait on the // refetch the invalidation triggers. onQueryStarted: async (query, { dispatch, queryFulfilled }) => { @@ -122,7 +134,9 @@ export const featureDependencyService = service Req['deleteFeatureDependency'] >({ invalidatesTags: (res, err, query) => - query.changeRequestId ? [] : invalidateEnvironment(query), + query.changeRequestId + ? pendingTags(query.featureId) + : invalidateEnvironment(query), queryFn: async (query, _, _2, baseQuery) => { if (!query.changeRequestId) { const res = await baseQuery({ @@ -138,6 +152,17 @@ export const featureDependencyService = service return { data: undefined } }, }), + // Faked until #8449: the API will list a feature's change requests and + // read each one's staged dependencies with GET ?change_request. + getChangeRequestDependencyChanges: builder.query< + Res['pendingDependencyChanges'], + Req['getChangeRequestDependencyChanges'] + >({ + providesTags: [{ id: 'PENDING', type: 'FeatureDependency' }], + queryFn: (query) => ({ + data: getPendingByChangeRequest(query.changeRequestId), + }), + }), getFeatureDependencies: builder.query< Res['featureDependencies'], Req['getFeatureDependencies'] @@ -156,6 +181,13 @@ export const featureDependencyService = service url: `environments/${query.environmentId}/features/${query.featureId}/dependents/`, }), }), + getPendingDependencyChanges: builder.query< + Res['pendingDependencyChanges'], + Req['getPendingDependencyChanges'] + >({ + providesTags: (res, err, query) => pendingTags(query.featureId), + queryFn: (query) => ({ data: getPendingByFeature(query.featureId) }), + }), // END OF ENDPOINTS }), }) @@ -221,8 +253,10 @@ export async function getFeatureDependents( export const { useCreateFeatureDependencyMutation, useDeleteFeatureDependencyMutation, + useGetChangeRequestDependencyChangesQuery, useGetFeatureDependenciesQuery, useGetFeatureDependentsQuery, + useGetPendingDependencyChangesQuery, // END OF EXPORTS } = featureDependencyService diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index f312e8a55805..cfb3b6be47d3 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -909,6 +909,8 @@ export type Req = { })[] } deleteChangeRequest: { id: number } + getPendingDependencyChanges: { featureId: number } + getChangeRequestDependencyChanges: { changeRequestId: number } getSamlConfiguration: { name: string } getSamlConfigurations: { organisation_id: number } getSamlConfigurationMetadata: { name: string } diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 4cd75465d7b5..41baacbfe89b 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -185,6 +185,18 @@ export type DependencyEdge = { segment: DependencyEdgeSegment } +// A dependency held in a change request, not live until it is published. +export type StagedDependencyChange = { + action: 'add' | 'remove' + prerequisite: DependencyFeature +} + +export type PendingDependencyChanges = { + changeRequestId: number + featureId: number + changes: StagedDependencyChange[] +} + export type DependencyConflictCode = | 'circular_dependency' | 'dependency_exists' @@ -1603,6 +1615,7 @@ export type Res = { featureDependencies: { results: DependencyEdge[] } featureDependents: { results: DependencyEdge[] } featureDependency: DependencyEdge + pendingDependencyChanges: PendingDependencyChanges[] featureVersions: PagedResponse users: User[] enableFeatureVersioning: { id: string } diff --git a/frontend/web/components/DependencyChangesTable/DependencyChangesTable.scss b/frontend/web/components/DependencyChangesTable/DependencyChangesTable.scss new file mode 100644 index 000000000000..9ff6ee91e18a --- /dev/null +++ b/frontend/web/components/DependencyChangesTable/DependencyChangesTable.scss @@ -0,0 +1,4 @@ +// Wide enough for the longer of the two chips, so the names line up. +.dependency-changes__change { + width: 104px; +} diff --git a/frontend/web/components/DependencyChangesTable/DependencyChangesTable.tsx b/frontend/web/components/DependencyChangesTable/DependencyChangesTable.tsx new file mode 100644 index 000000000000..d3b922e8e90d --- /dev/null +++ b/frontend/web/components/DependencyChangesTable/DependencyChangesTable.tsx @@ -0,0 +1,47 @@ +import { FC } from 'react' +import { StagedDependencyChange } from 'common/types/responses' +import Chip from 'components/base/Chip' +import './DependencyChangesTable.scss' +import Table, { + TableBody, + TableCell, + TableColumnHeader, + TableHeader, + TableRow, +} from 'components/base/Table' + +type DependencyChangesTableProps = { + changes: StagedDependencyChange[] +} + +// What a change request does to a feature's prerequisites once published. +const DependencyChangesTable: FC = ({ + changes, +}) => ( +
+ + + + Change + + Prerequisite + + + + {changes.map(({ action, prerequisite }) => ( + + + + {action === 'add' ? 'Add' : 'Remove'} + + + + {prerequisite.name} + + + ))} + +
+) + +export default DependencyChangesTable diff --git a/frontend/web/components/DependencyChangesTable/index.ts b/frontend/web/components/DependencyChangesTable/index.ts new file mode 100644 index 000000000000..4564a17ff442 --- /dev/null +++ b/frontend/web/components/DependencyChangesTable/index.ts @@ -0,0 +1 @@ +export { default } from './DependencyChangesTable' diff --git a/frontend/web/components/diff/DiffDependencies.tsx b/frontend/web/components/diff/DiffDependencies.tsx new file mode 100644 index 000000000000..a8cc056b3fce --- /dev/null +++ b/frontend/web/components/diff/DiffDependencies.tsx @@ -0,0 +1,55 @@ +import { FC } from 'react' +import { PendingDependencyChanges } from 'common/types/responses' +import { useGetChangeRequestDependencyChangesQuery } from 'common/services/useFeatureDependency' +import { useGetProjectFlagQuery } from 'common/services/useProjectFlag' +import DependencyChangesTable from 'components/DependencyChangesTable' +import Panel from 'components/base/grid/Panel' + +type DiffDependenciesProps = { + changeRequestId: number + projectId: string +} + +const FeatureDependencyChanges: FC< + PendingDependencyChanges & { projectId: string } +> = ({ changes, featureId, projectId }) => { + const { data: feature } = useGetProjectFlagQuery({ + id: featureId, + project: Number(projectId), + }) + return ( + +

+ Prerequisite changes in this request. Nothing changes until it is + published. +

+ +
+ ) +} + +// The prerequisite changes staged in a change request, per feature. +const DiffDependencies: FC = ({ + changeRequestId, + projectId, +}) => { + const { data } = useGetChangeRequestDependencyChangesQuery({ + changeRequestId, + }) + return ( + <> + {(data ?? []).map((entry) => ( + + ))} + + ) +} + +export default DiffDependencies diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx index 09cfb74b4dbe..5c102d1946cc 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx @@ -19,6 +19,7 @@ import PrerequisitesTable from './PrerequisitesTable' import { PrerequisitesEmptyState } from './DependenciesEmptyStates' import ChangeRequestFooter from './ChangeRequestFooter' import ChangeRequestNotice from './ChangeRequestNotice' +import PendingChangeRequests from './PendingChangeRequests' import { getDependenciesMode } from './dependenciesMode' import { withStagedChanges } from './prerequisiteState' import { usePrerequisites } from './hooks/usePrerequisites' @@ -228,6 +229,14 @@ const FeatureDependenciesTab: FC = ({ /> )} + {isChangeRequest && ( + + )} + = ({ + environmentId, + featureId, + projectId, +}) => { + const now = useRef(new Date().toISOString()) + const { data: pending } = useGetPendingDependencyChangesQuery({ featureId }) + const { data: open, isError: isOpenError } = useGetChangeRequestsQuery({ + committed: false, + environmentId, + page_size: 100, + }) + const { data: scheduled, isError: isScheduledError } = + useGetChangeRequestsQuery({ + committed: true, + environmentId, + live_from_after: now.current, + page_size: 100, + }) + + // A change request that was published or deleted has nothing pending. + const groups = useMemo(() => { + const live = [...(open?.results ?? []), ...(scheduled?.results ?? [])] + return (pending ?? []).flatMap(({ changeRequestId, changes }) => { + const changeRequest = live.find((cr) => cr.id === changeRequestId) + return changeRequest ? [{ changeRequest, changes }] : [] + }) + }, [pending, open, scheduled]) + + // Without the change request lists, staged changes cannot be matched to + // one, and hiding them would read as nothing pending. + if (pending?.length && (isOpenError || isScheduledError)) { + return ( + + ) + } + + if (!groups.length) return null + + return ( +
+
Pending in change requests
+

+ Not live yet. These go live when their change request is published. +

+
+ {groups.map(({ changeRequest, changes }) => ( + +
+ + #{changeRequest.id} {changeRequest.title} + + {describeStatus(changeRequest)} +
+ +
+ ))} +
+
+ ) +} + +const describeStatus = (changeRequest: ChangeRequestSummary) => + changeRequest.committed_at && changeRequest.live_from + ? `Scheduled for ${moment(changeRequest.live_from).format('D MMM, HH:mm')}` + : 'Awaiting approval' + +export default PendingChangeRequests diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx index cb324c3d95a0..ca63167abdb8 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx @@ -1,8 +1,8 @@ import { useState } from 'react' import { DependencyEdge, - DependencyFeature, ProjectFlag, + StagedDependencyChange, } from 'common/types/responses' import { Req } from 'common/types/requests' import { @@ -23,11 +23,6 @@ import { } from 'components/modals/create-feature/tabs/FeatureDependenciesTab/stagingError' import { unchangedChangeSet } from 'components/modals/create-feature/tabs/FeatureDependenciesTab/unchangedChangeSet' -export type StagedChange = { - action: 'add' | 'remove' - prerequisite: DependencyFeature -} - type UseStagedPrerequisitesArgs = { environmentId: string projectId: number @@ -48,7 +43,7 @@ export const useStagedPrerequisites = ({ projectFlag, projectId, }: UseStagedPrerequisitesArgs) => { - const [staged, setStaged] = useState([]) + const [staged, setStaged] = useState([]) const [error, setError] = useState(null) const [isSubmitting, setIsSubmitting] = useState(false) diff --git a/frontend/web/components/pages/ChangeRequestDetailPage.tsx b/frontend/web/components/pages/ChangeRequestDetailPage.tsx index 781e2f59f40d..231b3b2155cb 100644 --- a/frontend/web/components/pages/ChangeRequestDetailPage.tsx +++ b/frontend/web/components/pages/ChangeRequestDetailPage.tsx @@ -37,6 +37,7 @@ import UserSelect from 'components/UserSelect' import MyGroupsSelect from 'components/MyGroupsSelect' import Panel from 'components/base/grid/Panel' import DiffChangeRequest from 'components/diff/DiffChangeRequest' +import DiffDependencies from 'components/diff/DiffDependencies' import JSONReference from 'components/JSONReference' import ErrorMessage from 'components/ErrorMessage' import ConfigProvider from 'common/providers/ConfigProvider' @@ -407,6 +408,14 @@ const ChangeRequestDetailPage: FC = ({ match }) => { projectId={projectId} /> ) : null} + {Utils.getFlagsmithHasFeature( + 'flag_dependencies_change_requests', + ) && ( + + )}
} /> From 64d6d574134d788f19a1c5633de702042b1d3fb7 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 9 Oct 2026 10:39:08 -0300 Subject: [PATCH 04/11] feat(dependencies): make staging in a change request easier to follow - The staged changes footer sticks to the bottom of the tab, slides in with the first change and pulses its count on each one. - Create Change Request says what is in flight: creating, then staging N of M. - The new change request's pending group flashes and scrolls into view, so the staged rows can be followed to where they went. - Held changes mark the tab as unsaved, and closing the modal asks first. Refs #8450 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../modals/create-feature/index.tsx | 18 +++++- .../ChangeRequestFooter.tsx | 30 ---------- .../ChangeRequestFooter.scss | 46 +++++++++++++++ .../ChangeRequestFooter.tsx | 42 ++++++++++++++ .../ChangeRequestFooter/index.ts | 1 + .../FeatureDependenciesTab.tsx | 12 +++- .../PendingChangeRequests.scss | 23 ++++++++ .../PendingChangeRequests.tsx | 56 ++++++++++++++----- .../PendingChangeRequests/index.ts | 1 + .../hooks/useStagedPrerequisites.tsx | 19 +++++-- 10 files changed, 195 insertions(+), 53 deletions(-) delete mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter.tsx create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.tsx create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/index.ts create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.scss rename frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/{ => PendingChangeRequests}/PendingChangeRequests.tsx (61%) create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/index.ts diff --git a/frontend/web/components/modals/create-feature/index.tsx b/frontend/web/components/modals/create-feature/index.tsx index 53d77414e0c3..5d6418af1fdf 100644 --- a/frontend/web/components/modals/create-feature/index.tsx +++ b/frontend/web/components/modals/create-feature/index.tsx @@ -145,6 +145,7 @@ const CreateFeatureModal: FC = (props) => { const [valueChanged, setValueChanged] = useState(false) const [settingsChanged, setSettingsChanged] = useState(false) const [segmentsChanged, setSegmentsChanged] = useState(false) + const [dependenciesChanged, setDependenciesChanged] = useState(false) const [hasMetadataRequired, setHasMetadataRequired] = useState(false) const [featureLimitAlert, setFeatureLimitAlert] = useState({ percentage: 0, @@ -164,7 +165,12 @@ const CreateFeatureModal: FC = (props) => { const onClosing = useCallback(() => { if (isEdit) { return new Promise((resolve) => { - if (settingsChanged || valueChanged || segmentsChanged) { + if ( + settingsChanged || + valueChanged || + segmentsChanged || + dependenciesChanged + ) { openConfirm({ body: 'Closing this will discard your unsaved changes.', noText: 'Cancel', @@ -179,7 +185,13 @@ const CreateFeatureModal: FC = (props) => { }) } return Promise.resolve(true) - }, [isEdit, settingsChanged, valueChanged, segmentsChanged]) + }, [ + isEdit, + settingsChanged, + valueChanged, + segmentsChanged, + dependenciesChanged, + ]) const fetchChangeRequests = useCallback( (forceRefetch?: boolean) => { @@ -830,6 +842,7 @@ const CreateFeatureModal: FC = (props) => { = (props) => { projectFlag={projectFlag} requiresChangeRequests={is4Eyes} isVersioned={isVersioned} + onDirtyChange={setDependenciesChanged} onSelectFeature={(featureId) => { // The features list, never the current page: it // owns the deep link and hydrates an off-page diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter.tsx deleted file mode 100644 index c0b2192718a8..000000000000 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter.tsx +++ /dev/null @@ -1,30 +0,0 @@ -import { FC } from 'react' -import Button from 'components/base/forms/Button' - -type ChangeRequestFooterProps = { - count: number - isSubmitting: boolean - onCreate: () => void - onDiscard: () => void -} - -const ChangeRequestFooter: FC = ({ - count, - isSubmitting, - onCreate, - onDiscard, -}) => ( -
- - {count} {count === 1 ? 'change' : 'changes'} staged - - - -
-) - -export default ChangeRequestFooter diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss new file mode 100644 index 000000000000..a883903a1327 --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss @@ -0,0 +1,46 @@ +// Pinned to the bottom of the tab, so the way out stays in view however long +// the table gets. +.change-request-footer { + position: sticky; + bottom: 0; + z-index: 1; + margin-top: 16px; + padding: 12px 0; + border-top: 1px solid var(--color-border-default); + background: var(--color-surface-default); + animation: change-request-footer-enter var(--duration-normal) + var(--easing-entrance) both; +} + +.change-request-footer__count { + display: inline-block; + font-weight: var(--font-weight-semibold); + color: var(--color-text-default); + animation: change-request-footer-pulse var(--duration-slow) + var(--easing-standard); +} + +@keyframes change-request-footer-enter { + from { + opacity: 0; + transform: translateY(8px); + } + + to { + opacity: 1; + transform: none; + } +} + +@keyframes change-request-footer-pulse { + 50% { + transform: scale(1.25); + } +} + +@media (prefers-reduced-motion: reduce) { + .change-request-footer, + .change-request-footer__count { + animation: none; + } +} diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.tsx new file mode 100644 index 000000000000..dc886570e5bb --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.tsx @@ -0,0 +1,42 @@ +import { FC } from 'react' +import Button from 'components/base/forms/Button' +import { SubmitProgress } from 'components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites' +import './ChangeRequestFooter.scss' + +type ChangeRequestFooterProps = { + count: number + progress: SubmitProgress | null + onCreate: () => void + onDiscard: () => void +} + +// The calls run one after another, so say which one is in flight. +const describeProgress = (progress: SubmitProgress) => + progress.done === null + ? 'Creating change request…' + : `Staging ${progress.done + 1} of ${progress.total}…` + +const ChangeRequestFooter: FC = ({ + count, + onCreate, + onDiscard, + progress, +}) => ( +
+ + {/* Keyed on the count, so each change replays the pulse. */} + + {count} + {' '} + {count === 1 ? 'change' : 'changes'} staged + + + +
+) + +export default ChangeRequestFooter diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/index.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/index.ts new file mode 100644 index 000000000000..677a2e32b9f3 --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/index.ts @@ -0,0 +1 @@ +export { default } from './ChangeRequestFooter' diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx index 5c102d1946cc..c3b5872d2360 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/FeatureDependenciesTab.tsx @@ -1,4 +1,4 @@ -import { FC } from 'react' +import { FC, useEffect } from 'react' import { ProjectFlag } from 'common/types/responses' import { EnvironmentPermission } from 'common/types/permissions.types' import { useHasPermission } from 'common/providers/Permission' @@ -37,6 +37,8 @@ type FeatureDependenciesTabProps = { projectFlag: ProjectFlag requiresChangeRequests: boolean isVersioned: boolean + // Held changes are lost when the modal closes, so the modal needs to know. + onDirtyChange?: (isDirty: boolean) => void // Opens another feature's modal on this tab. onSelectFeature: (featureId: number) => void } @@ -45,6 +47,7 @@ const FeatureDependenciesTab: FC = ({ environmentId, environmentName, isVersioned, + onDirtyChange, onSelectFeature, projectFlag, projectId, @@ -65,6 +68,10 @@ const FeatureDependenciesTab: FC = ({ projectFlag, projectId, }) + const isDirty = !!staging.staged.length + useEffect(() => { + onDirtyChange?.(isDirty) + }, [isDirty, onDirtyChange]) const { addingName, @@ -223,7 +230,7 @@ const FeatureDependenciesTab: FC = ({ {isChangeRequest && !!staging.staged.length && ( @@ -231,6 +238,7 @@ const FeatureDependenciesTab: FC = ({ {isChangeRequest && ( = ({ environmentId, featureId, + highlightId, projectId, }) => { const now = useRef(new Date().toISOString()) + const highlightRef = useRef(null) const { data: pending } = useGetPendingDependencyChangesQuery({ featureId }) const { data: open, isError: isOpenError } = useGetChangeRequestsQuery({ committed: false, @@ -45,6 +51,20 @@ const PendingChangeRequests: FC = ({ }) }, [pending, open, scheduled]) + const hasHighlight = groups.some( + ({ changeRequest }) => changeRequest.id === highlightId, + ) + useEffect(() => { + if (!hasHighlight) return + const reduceMotion = window.matchMedia( + '(prefers-reduced-motion: reduce)', + ).matches + highlightRef.current?.scrollIntoView({ + behavior: reduceMotion ? 'auto' : 'smooth', + block: 'nearest', + }) + }, [hasHighlight]) + // Without the change request lists, staged changes cannot be matched to // one, and hiding them would read as nothing pending. if (pending?.length && (isOpenError || isScheduledError)) { @@ -63,18 +83,26 @@ const PendingChangeRequests: FC = ({

{groups.map(({ changeRequest, changes }) => ( - -
- - #{changeRequest.id} {changeRequest.title} - - {describeStatus(changeRequest)} -
- -
+
+ +
+ + #{changeRequest.id} {changeRequest.title} + + {describeStatus(changeRequest)} +
+ +
+
))}
diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/index.ts b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/index.ts new file mode 100644 index 000000000000..bb887b1bad62 --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/index.ts @@ -0,0 +1 @@ +export { default } from './PendingChangeRequests' diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx index ca63167abdb8..16999d0a10bb 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx @@ -23,6 +23,9 @@ import { } from 'components/modals/create-feature/tabs/FeatureDependenciesTab/stagingError' import { unchangedChangeSet } from 'components/modals/create-feature/tabs/FeatureDependenciesTab/unchangedChangeSet' +// `done` is null while the change request itself is being created. +export type SubmitProgress = { done: number | null; total: number } + type UseStagedPrerequisitesArgs = { environmentId: string projectId: number @@ -45,7 +48,9 @@ export const useStagedPrerequisites = ({ }: UseStagedPrerequisitesArgs) => { const [staged, setStaged] = useState([]) const [error, setError] = useState(null) - const [isSubmitting, setIsSubmitting] = useState(false) + const [progress, setProgress] = useState(null) + // The change request the last submit created, so the tab can point at it. + const [createdId, setCreatedId] = useState() const [createChangeRequest] = useCreateEnvironmentChangeRequestMutation() const [deleteChangeRequest] = useDeleteChangeRequestMutation() @@ -82,7 +87,8 @@ export const useStagedPrerequisites = ({ const stage = async (changeRequestId: number) => { // In order, so the first refusal names the change that caused it. - for (const { action, prerequisite } of staged) { + for (const [index, { action, prerequisite }] of staged.entries()) { + setProgress({ done: index, total: staged.length }) const query = { changeRequestId, environmentId, @@ -106,8 +112,9 @@ export const useStagedPrerequisites = ({ } const submitChangeRequest = async (fields: ChangeRequestFields) => { - setIsSubmitting(true) + setProgress({ done: null, total: staged.length }) setError(null) + setCreatedId(undefined) let changeRequestId: number | undefined try { const featureStates = await getFeatureStates({ @@ -128,6 +135,7 @@ export const useStagedPrerequisites = ({ ).id await stage(changeRequestId) setStaged([]) + setCreatedId(changeRequestId) toast( <> Change request created.{' '} @@ -143,7 +151,7 @@ export const useStagedPrerequisites = ({ if (changeRequestId) await deleteChangeRequest({ id: changeRequestId }) setError(toStagingError(e, 'Could not create the change request.')) } finally { - setIsSubmitting(false) + setProgress(null) } } @@ -161,13 +169,14 @@ export const useStagedPrerequisites = ({ return { add, + createdId, discard: () => { setError(null) setStaged([]) }, error, - isSubmitting, openChangeRequest, + progress, remove, staged, undo: drop, From 6e71c6d03af2cd9c9cbbac964b3dc1fa67b95071 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 9 Oct 2026 12:42:43 -0300 Subject: [PATCH 05/11] fix(dependencies): pulse the staged count in colour so it is not clipped Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ChangeRequestFooter/ChangeRequestFooter.scss | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss index a883903a1327..928083b453d5 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss @@ -13,7 +13,6 @@ } .change-request-footer__count { - display: inline-block; font-weight: var(--font-weight-semibold); color: var(--color-text-default); animation: change-request-footer-pulse var(--duration-slow) @@ -32,9 +31,11 @@ } } +// Colour, not scale: the count sits on the tab's clipping edge, so growing +// it cut it off. @keyframes change-request-footer-pulse { - 50% { - transform: scale(1.25); + from { + color: var(--color-text-action); } } From 8967c7caa13402b209ef251fad3b1b89f6186cc3 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 9 Oct 2026 12:54:08 -0300 Subject: [PATCH 06/11] feat(dependencies): show each pending change request as an accordion Co-Authored-By: Claude Opus 5.5 (1M context) --- .../PendingChangeRequests.scss | 2 +- .../PendingChangeRequests.tsx | 21 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.scss b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.scss index 94e02e779eb1..5c4da37e1035 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.scss +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.scss @@ -1,7 +1,7 @@ // The change request that was just created: the staged rows went into it, so // the eye is led from where they were to where they are now. .pending-change-request--flash { - border-radius: var(--radius-md); + border-radius: var(--radius-lg); animation: pending-change-request-flash 2s var(--easing-standard); } diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx index a0aebc64f248..ce4405f21668 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx @@ -4,10 +4,10 @@ import moment from 'moment' import { ChangeRequestSummary } from 'common/types/responses' import { useGetChangeRequestsQuery } from 'common/services/useChangeRequest' import { useGetPendingDependencyChangesQuery } from 'common/services/useFeatureDependency' +import Accordion from 'components/base/Accordion' import Chip from 'components/base/Chip' import ErrorMessage from 'components/ErrorMessage' import DependencyChangesTable from 'components/DependencyChangesTable' -import DependenciesPanel from 'components/modals/create-feature/tabs/FeatureDependenciesTab/DependenciesPanel' import './PendingChangeRequests.scss' type PendingChangeRequestsProps = { @@ -90,18 +90,23 @@ const PendingChangeRequests: FC = ({ 'pending-change-request--flash': changeRequest.id === highlightId, })} > - -
+ {describeStatus(changeRequest)}} + > + + {/* In the body, as the header is the toggle and a link cannot + sit inside a button. */} + - - +
))} From 1a8af45d48a15f6ea5cd15b5a37e7af74c46b697 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 9 Oct 2026 13:02:46 -0300 Subject: [PATCH 07/11] fix(dependencies): open change request links in a new tab Leaving the page dropped the modal and anything staged in it, without the close confirmation. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ChangeRequestLink.tsx | 31 +++++++++++++++++++ .../PendingChangeRequests.tsx | 9 ++++-- .../hooks/useStagedPrerequisites.tsx | 9 ++++-- 3 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink.tsx diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink.tsx new file mode 100644 index 000000000000..cde04135ac2f --- /dev/null +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink.tsx @@ -0,0 +1,31 @@ +import { FC, ReactNode } from 'react' +import Icon from 'components/icons/Icon' + +type ChangeRequestLinkProps = { + projectId: number + environmentId: string + changeRequestId: number + children: ReactNode +} + +// A new tab, as leaving the page would drop the modal and anything staged in +// it without the close confirmation. +const ChangeRequestLink: FC = ({ + changeRequestId, + children, + environmentId, + projectId, +}) => ( + + {children} + + (opens in a new tab) + +) + +export default ChangeRequestLink diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx index ce4405f21668..57c5a7b0c7fa 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx @@ -5,6 +5,7 @@ import { ChangeRequestSummary } from 'common/types/responses' import { useGetChangeRequestsQuery } from 'common/services/useChangeRequest' import { useGetPendingDependencyChangesQuery } from 'common/services/useFeatureDependency' import Accordion from 'components/base/Accordion' +import ChangeRequestLink from 'components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink' import Chip from 'components/base/Chip' import ErrorMessage from 'components/ErrorMessage' import DependencyChangesTable from 'components/DependencyChangesTable' @@ -100,11 +101,13 @@ const PendingChangeRequests: FC = ({ {/* In the body, as the header is the toggle and a link cannot sit inside a button. */} diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx index 16999d0a10bb..f96ea6a191d4 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/hooks/useStagedPrerequisites.tsx @@ -16,6 +16,7 @@ import { import { useLazyGetFeatureStatesQuery } from 'common/services/useFeatureState' import { useProjectEnvironments } from 'common/hooks/useProjectEnvironments' import ChangeRequestModal from 'components/modals/ChangeRequestModal' +import ChangeRequestLink from 'components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink' import { StagingError, describeApiError, @@ -139,11 +140,13 @@ export const useStagedPrerequisites = ({ toast( <> Change request created.{' '} - View it - + , ) } catch (e) { From dce5c4e1031d17bef95647d03b112bfd992f56d3 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 9 Oct 2026 13:33:17 -0300 Subject: [PATCH 08/11] fix(dependencies): label the new-tab icon instead of a visible note Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tabs/FeatureDependenciesTab/ChangeRequestLink.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink.tsx index cde04135ac2f..f4998ad7c5d2 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink.tsx @@ -23,8 +23,12 @@ const ChangeRequestLink: FC = ({ className='d-inline-flex align-items-center gap-1' > {children} - - (opens in a new tab) + ) From 50e269b01fa0974c082116b6b17938c9064e3217 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 9 Oct 2026 17:04:03 -0300 Subject: [PATCH 09/11] refactor(dependencies): use utilities for the footer's spacing and colours Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ChangeRequestFooter/ChangeRequestFooter.scss | 5 ----- .../ChangeRequestFooter/ChangeRequestFooter.tsx | 7 +++++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss index 928083b453d5..00a7883f887b 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.scss @@ -4,17 +4,12 @@ position: sticky; bottom: 0; z-index: 1; - margin-top: 16px; - padding: 12px 0; border-top: 1px solid var(--color-border-default); - background: var(--color-surface-default); animation: change-request-footer-enter var(--duration-normal) var(--easing-entrance) both; } .change-request-footer__count { - font-weight: var(--font-weight-semibold); - color: var(--color-text-default); animation: change-request-footer-pulse var(--duration-slow) var(--easing-standard); } diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.tsx index dc886570e5bb..320296025bfe 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestFooter/ChangeRequestFooter.tsx @@ -22,10 +22,13 @@ const ChangeRequestFooter: FC = ({ onDiscard, progress, }) => ( -
+
{/* Keyed on the count, so each change replays the pulse. */} - + {count} {' '} {count === 1 ? 'change' : 'changes'} staged From 6e9e8af5a3627e277468a56bc6301fa12a3e80b7 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Mon, 12 Oct 2026 10:25:04 -0300 Subject: [PATCH 10/11] feat(dependencies): adopt the change icons, status colours and state column From the v2 design: - A plus or minus icon before each change, and a red Remove chip. - A change request icon before each pending group's title. - Awaiting approval reads as a warning; scheduled stays neutral. - Pending changes show each prerequisite's current state. Adds warning and danger Chip variants and a minus icon drawn from plus's bar. Refs #8450 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../documentation/components/Chip.stories.tsx | 8 +++ .../components/Icons.stories.tsx | 1 + .../DependencyChangesTable.scss | 8 ++- .../DependencyChangesTable.tsx | 52 ++++++++++++++----- frontend/web/components/base/Chip/Chip.scss | 8 +++ frontend/web/components/base/Chip/Chip.tsx | 6 ++- frontend/web/components/icons/Icon.tsx | 21 ++++++++ .../FeatureDependenciesTab.tsx | 1 + .../PendingChangeRequests.tsx | 37 ++++++++++--- 9 files changed, 119 insertions(+), 23 deletions(-) diff --git a/frontend/documentation/components/Chip.stories.tsx b/frontend/documentation/components/Chip.stories.tsx index c88fe9860935..24e38a8c4c46 100644 --- a/frontend/documentation/components/Chip.stories.tsx +++ b/frontend/documentation/components/Chip.stories.tsx @@ -28,6 +28,14 @@ export const Accent: Story = { args: { children: '"hello"', variant: 'accent' }, } +export const Warning: Story = { + args: { children: 'Awaiting approval', variant: 'warning' }, +} + +export const Danger: Story = { + args: { children: 'Remove', variant: 'danger' }, +} + export const Sizes: Story = { render: () => (
diff --git a/frontend/documentation/components/Icons.stories.tsx b/frontend/documentation/components/Icons.stories.tsx index dff1c2ba7125..4554f8ed8228 100644 --- a/frontend/documentation/components/Icons.stories.tsx +++ b/frontend/documentation/components/Icons.stories.tsx @@ -43,6 +43,7 @@ const CATEGORIES: IconCategory[] = [ 'more-vertical', 'options-2', 'plus', + 'minus', 'refresh', 'search', 'trash-2', diff --git a/frontend/web/components/DependencyChangesTable/DependencyChangesTable.scss b/frontend/web/components/DependencyChangesTable/DependencyChangesTable.scss index 9ff6ee91e18a..50df6092d6ba 100644 --- a/frontend/web/components/DependencyChangesTable/DependencyChangesTable.scss +++ b/frontend/web/components/DependencyChangesTable/DependencyChangesTable.scss @@ -1,4 +1,8 @@ -// Wide enough for the longer of the two chips, so the names line up. +// Fixed widths so the names line up across tables. .dependency-changes__change { - width: 104px; + width: 128px; +} + +.dependency-changes__state { + width: 120px; } diff --git a/frontend/web/components/DependencyChangesTable/DependencyChangesTable.tsx b/frontend/web/components/DependencyChangesTable/DependencyChangesTable.tsx index d3b922e8e90d..9b6969fc29de 100644 --- a/frontend/web/components/DependencyChangesTable/DependencyChangesTable.tsx +++ b/frontend/web/components/DependencyChangesTable/DependencyChangesTable.tsx @@ -1,6 +1,8 @@ import { FC } from 'react' import { StagedDependencyChange } from 'common/types/responses' +import { colorTextAction, colorTextDanger } from 'common/theme/tokens' import Chip from 'components/base/Chip' +import Icon from 'components/icons/Icon' import './DependencyChangesTable.scss' import Table, { TableBody, @@ -12,11 +14,14 @@ import Table, { type DependencyChangesTableProps = { changes: StagedDependencyChange[] + // Adds a column with each prerequisite's state in this environment. + isEnabled?: (prerequisiteId: number) => boolean } // What a change request does to a feature's prerequisites once published. const DependencyChangesTable: FC = ({ changes, + isEnabled, }) => ( @@ -25,21 +30,44 @@ const DependencyChangesTable: FC = ({ Change Prerequisite + {!!isEnabled && ( + + Current state + + )} - {changes.map(({ action, prerequisite }) => ( - - - - {action === 'add' ? 'Add' : 'Remove'} - - - - {prerequisite.name} - - - ))} + {changes.map(({ action, prerequisite }) => { + const isAdd = action === 'add' + return ( + + + + + + {isAdd ? 'Add' : 'Remove'} + + + + + {prerequisite.name} + + {!!isEnabled && ( + + + {isEnabled(prerequisite.id) ? 'Enabled' : 'Disabled'} + + + )} + + ) + })}
) diff --git a/frontend/web/components/base/Chip/Chip.scss b/frontend/web/components/base/Chip/Chip.scss index 854427ce3f98..ddd54d4462f7 100644 --- a/frontend/web/components/base/Chip/Chip.scss +++ b/frontend/web/components/base/Chip/Chip.scss @@ -13,6 +13,14 @@ border-color: var(--color-border-action); } + &--warning { + border-color: var(--color-border-warning); + } + + &--danger { + border-color: var(--color-border-danger); + } + // Sizes (default is the base above). &--sm { padding: 3px 8px; diff --git a/frontend/web/components/base/Chip/Chip.tsx b/frontend/web/components/base/Chip/Chip.tsx index 296219372dda..90a0d0f19b0a 100644 --- a/frontend/web/components/base/Chip/Chip.tsx +++ b/frontend/web/components/base/Chip/Chip.tsx @@ -5,7 +5,7 @@ import { colorIconSecondary } from 'common/theme/tokens' import './Chip.scss' export type ChipSize = 'default' | 'sm' | 'xs' -export type ChipVariant = 'neutral' | 'accent' +export type ChipVariant = 'neutral' | 'accent' | 'warning' | 'danger' export type ChipProps = { children: ReactNode @@ -30,7 +30,9 @@ export type ChipProps = { // bg + text come from token utilities; the variant border lives in Chip.scss. const VARIANT_UTILITIES: Record = { accent: 'bg-surface-action-subtle text-action', + danger: 'bg-surface-danger text-danger', neutral: 'bg-surface-subtle text-default', + warning: 'bg-surface-warning text-warning', } // Token-based chip primitive. Uses `ds-chip` rather than the legacy `.chip` @@ -61,7 +63,7 @@ const Chip = ({ 'ds-chip d-inline-flex align-items-center align-middle gap-1 rounded-sm', VARIANT_UTILITIES[variant], { - 'ds-chip--accent': variant === 'accent', + [`ds-chip--${variant}`]: variant !== 'neutral', 'ds-chip--clickable': interactive, [`ds-chip--${size}`]: size !== 'default', 'ds-chip--truncate': truncate, diff --git a/frontend/web/components/icons/Icon.tsx b/frontend/web/components/icons/Icon.tsx index 07f4e53a8e42..f9718782b8df 100644 --- a/frontend/web/components/icons/Icon.tsx +++ b/frontend/web/components/icons/Icon.tsx @@ -47,6 +47,7 @@ export type IconName = | 'person' | 'pie-chart' | 'plus' + | 'minus' | 'pr-closed' | 'pr-draft' | 'pr-linked' @@ -216,6 +217,26 @@ const Icon: FC = ({ ) } + // The plus icon's horizontal bar, so the two read as a pair. + case 'minus': { + return ( + + + + ) + } case 'eye': { return ( = ({ {isChangeRequest && ( boolean // The change request just created, flashed and scrolled to on arrival. highlightId?: number } @@ -25,6 +28,7 @@ const PendingChangeRequests: FC = ({ environmentId, featureId, highlightId, + isEnabled, projectId, }) => { const now = useRef(new Date().toISOString()) @@ -94,10 +98,20 @@ const PendingChangeRequests: FC = ({ {describeStatus(changeRequest)}} + title={ + + + #{changeRequest.id} {changeRequest.title} + + } + meta={} > - + {/* In the body, as the header is the toggle and a link cannot sit inside a button. */}
@@ -117,9 +131,18 @@ const PendingChangeRequests: FC = ({ ) } -const describeStatus = (changeRequest: ChangeRequestSummary) => - changeRequest.committed_at && changeRequest.live_from - ? `Scheduled for ${moment(changeRequest.live_from).format('D MMM, HH:mm')}` - : 'Awaiting approval' +// Waiting on people reads as a warning; a scheduled change only needs time. +const StatusChip: FC<{ changeRequest: ChangeRequestSummary }> = ({ + changeRequest, +}) => + changeRequest.committed_at && changeRequest.live_from ? ( + + Scheduled for {moment(changeRequest.live_from).format('D MMM, HH:mm')} + + ) : ( + + Awaiting approval + + ) export default PendingChangeRequests From be39eaac829eb4dad771b433e764e94f59276e3f Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Mon, 12 Oct 2026 11:12:58 -0300 Subject: [PATCH 11/11] refactor(dependencies): read pending changes the way #8720 serves them #8720 lists a change request's dependencies as if it were live, rather than as a diff. The fake now answers GET ?change_request the same way, and the pending section and change request page work out what changed against the live list with diffDependencies. Swapping to the real API then means deleting the fake rather than reworking the UI. Refs #8450 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../fakeChangeRequestDependencies.test.ts | 40 +++++++---- .../services/fakeChangeRequestDependencies.ts | 41 +++++++++--- .../common/services/useFeatureDependency.ts | 67 +++++++++++++------ frontend/common/types/requests.ts | 11 ++- frontend/common/types/responses.ts | 9 +-- .../utils/__tests__/diffDependencies.test.ts | 34 ++++++++++ frontend/common/utils/diffDependencies.ts | 19 ++++++ .../ChangeRequestDependencyChanges.tsx | 46 +++++++++++++ .../web/components/diff/DiffDependencies.tsx | 33 ++++----- .../PendingChangeRequests.tsx | 28 ++++---- .../pages/ChangeRequestDetailPage.tsx | 1 + 11 files changed, 248 insertions(+), 81 deletions(-) create mode 100644 frontend/common/utils/__tests__/diffDependencies.test.ts create mode 100644 frontend/common/utils/diffDependencies.ts create mode 100644 frontend/web/components/ChangeRequestDependencyChanges.tsx diff --git a/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts b/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts index 07a9355a1a31..5f3f3756a0a1 100644 --- a/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts +++ b/frontend/common/services/__tests__/fakeChangeRequestDependencies.test.ts @@ -1,8 +1,9 @@ import { DependencyEdge } from 'common/types/responses' import { discardChangeRequest, - getPendingByChangeRequest, - getPendingByFeature, + applyStagedChanges, + getChangeRequestIdsStaging, + getFeatureIdsStaged, getStagedDependencyChanges, stageAdd, stageRemove, @@ -88,23 +89,19 @@ describe('stageRemove', () => { }) }) -describe('pending changes', () => { - it('lists staged changes by feature and by change request', () => { +describe('staged ids', () => { + it('lists change requests by feature, and features by change request', () => { stageAdd(args) stageRemove(14, 1, { id: 4, name: 'feature_4' }) stageAdd({ ...args, featureId: 2 }) - expect(getPendingByFeature(1).map((e) => e.changeRequestId)).toEqual([ - 12, 14, - ]) - expect(getPendingByChangeRequest(12).map((e) => e.featureId)).toEqual([ - 1, 2, - ]) + expect(getChangeRequestIdsStaging(1)).toEqual([12, 14]) + expect(getFeatureIdsStaged(12)).toEqual([1, 2]) }) it('drops a change request emptied by its own removals', () => { stageAdd(args) stageRemove(12, 1, prerequisite) - expect(getPendingByFeature(1)).toEqual([]) + expect(getChangeRequestIdsStaging(1)).toEqual([]) }) it('forgets every feature of a discarded change request', () => { @@ -112,7 +109,24 @@ describe('pending changes', () => { stageAdd({ ...args, featureId: 2 }) stageAdd({ ...args, changeRequestId: 120 }) discardChangeRequest(12) - expect(getPendingByChangeRequest(12)).toEqual([]) - expect(getPendingByChangeRequest(120)).toHaveLength(1) + expect(getFeatureIdsStaged(12)).toEqual([]) + expect(getFeatureIdsStaged(120)).toHaveLength(1) + }) +}) + +describe('applyStagedChanges', () => { + it('lists the dependencies as if the change request were live', () => { + stageAdd(args) + stageRemove(12, 1, { id: 4, name: 'feature_4' }) + expect( + applyStagedChanges(12, 1, [edge(1, 4), edge(1, 5)]).map( + (e) => e.prerequisite.id, + ), + ).toEqual([5, 3]) + }) + + it('leaves the live list alone for another change request', () => { + stageAdd(args) + expect(applyStagedChanges(99, 1, [edge(1, 5)])).toEqual([edge(1, 5)]) }) }) diff --git a/frontend/common/services/fakeChangeRequestDependencies.ts b/frontend/common/services/fakeChangeRequestDependencies.ts index c7a9bcc74784..552a0ab1e5a6 100644 --- a/frontend/common/services/fakeChangeRequestDependencies.ts +++ b/frontend/common/services/fakeChangeRequestDependencies.ts @@ -6,9 +6,9 @@ import { DependencyConflictCode, DependencyEdge, DependencyFeature, - PendingDependencyChanges, StagedDependencyChange, } from 'common/types/responses' +import { stagedDependencyEdge } from 'common/utils/stagedDependencyEdge' type StagedChanges = Record @@ -110,19 +110,40 @@ export const stageRemove = ( write(changes) } -const entries = (): PendingDependencyChanges[] => +// As #8720 answers GET ?change_request: the list as if the change request +// were live. +export const applyStagedChanges = ( + changeRequestId: number, + featureId: number, + liveEdges: DependencyEdge[], +): DependencyEdge[] => { + const staged = getStagedDependencyChanges(changeRequestId, featureId) + const removed = (id: number) => + staged.some((c) => c.action === 'remove' && c.prerequisite.id === id) + return [ + ...liveEdges.filter((edge) => !removed(edge.prerequisite.id)), + ...staged + .filter((c) => c.action === 'add') + .map((c) => stagedDependencyEdge(featureId, c.prerequisite)), + ] +} + +const stagedKeys = () => Object.entries(read()) .filter(([, changes]) => changes.length) - .map(([key, changes]) => { - const [changeRequestId, featureId] = key.split('-').map(Number) - return { changeRequestId, changes, featureId } - }) + .map(([key]) => key.split('-').map(Number)) -export const getPendingByFeature = (featureId: number) => - entries().filter((entry) => entry.featureId === featureId) +// Stand in for listing change requests by feature, and a change request's +// change sets, until real change sets carry the staged dependencies. +export const getChangeRequestIdsStaging = (featureId: number) => + stagedKeys() + .filter(([, id]) => id === featureId) + .map(([changeRequestId]) => changeRequestId) -export const getPendingByChangeRequest = (changeRequestId: number) => - entries().filter((entry) => entry.changeRequestId === changeRequestId) +export const getFeatureIdsStaged = (changeRequestId: number) => + stagedKeys() + .filter(([id]) => id === changeRequestId) + .map(([, featureId]) => featureId) // A deleted change request takes its staged changes with it. export const discardChangeRequest = (changeRequestId: number) => { diff --git a/frontend/common/services/useFeatureDependency.ts b/frontend/common/services/useFeatureDependency.ts index d4b5b32522ad..07418e463dda 100644 --- a/frontend/common/services/useFeatureDependency.ts +++ b/frontend/common/services/useFeatureDependency.ts @@ -2,8 +2,9 @@ import { Req } from 'common/types/requests' import { Res } from 'common/types/responses' import { stagedDependencyEdge } from 'common/utils/stagedDependencyEdge' import { - getPendingByChangeRequest, - getPendingByFeature, + applyStagedChanges, + getChangeRequestIdsStaging, + getFeatureIdsStaged, stageAdd, stageRemove, } from './fakeChangeRequestDependencies' @@ -152,26 +153,47 @@ export const featureDependencyService = service return { data: undefined } }, }), - // Faked until #8449: the API will list a feature's change requests and - // read each one's staged dependencies with GET ?change_request. - getChangeRequestDependencyChanges: builder.query< - Res['pendingDependencyChanges'], - Req['getChangeRequestDependencyChanges'] + + // Faked until #8449: will be the change requests listed by feature. + getChangeRequestIdsStaging: builder.query< + Res['changeRequestIdsStaging'], + Req['getChangeRequestIdsStaging'] >({ - providesTags: [{ id: 'PENDING', type: 'FeatureDependency' }], + providesTags: (res, err, query) => pendingTags(query.featureId), queryFn: (query) => ({ - data: getPendingByChangeRequest(query.changeRequestId), + data: getChangeRequestIdsStaging(query.featureId), }), }), + getFeatureDependencies: builder.query< Res['featureDependencies'], Req['getFeatureDependencies'] >({ - providesTags: (res, err, query) => listTags(query), - query: (query: Req['getFeatureDependencies']) => ({ - url: `environments/${query.environmentId}/features/${query.featureId}/dependencies/`, - }), + providesTags: (res, err, query) => [ + ...listTags(query), + ...(query.changeRequestId ? pendingTags(query.featureId) : []), + ], + queryFn: async (query, _, _2, baseQuery) => { + const res = await baseQuery({ + url: dependenciesUrl(query, query.featureId), + }) + if (res.error) return { error: res.error } + const live = res.data as Res['featureDependencies'] + // Faked until #8449: the API will apply ?change_request itself. + return { + data: query.changeRequestId + ? { + results: applyStagedChanges( + query.changeRequestId, + query.featureId, + live.results, + ), + } + : live, + } + }, }), + getFeatureDependents: builder.query< Res['featureDependents'], Req['getFeatureDependents'] @@ -181,12 +203,17 @@ export const featureDependencyService = service url: `environments/${query.environmentId}/features/${query.featureId}/dependents/`, }), }), - getPendingDependencyChanges: builder.query< - Res['pendingDependencyChanges'], - Req['getPendingDependencyChanges'] + + // Faked until #8449: real change sets will say which features a change + // request stages dependencies for. + getFeatureIdsStaged: builder.query< + Res['featureIdsStaged'], + Req['getFeatureIdsStaged'] >({ - providesTags: (res, err, query) => pendingTags(query.featureId), - queryFn: (query) => ({ data: getPendingByFeature(query.featureId) }), + providesTags: [{ id: 'PENDING', type: 'FeatureDependency' }], + queryFn: (query) => ({ + data: getFeatureIdsStaged(query.changeRequestId), + }), }), // END OF ENDPOINTS }), @@ -253,10 +280,10 @@ export async function getFeatureDependents( export const { useCreateFeatureDependencyMutation, useDeleteFeatureDependencyMutation, - useGetChangeRequestDependencyChangesQuery, + useGetChangeRequestIdsStagingQuery, useGetFeatureDependenciesQuery, useGetFeatureDependentsQuery, - useGetPendingDependencyChangesQuery, + useGetFeatureIdsStagedQuery, // END OF EXPORTS } = featureDependencyService diff --git a/frontend/common/types/requests.ts b/frontend/common/types/requests.ts index cfb3b6be47d3..d8258b72df71 100644 --- a/frontend/common/types/requests.ts +++ b/frontend/common/types/requests.ts @@ -871,7 +871,12 @@ export type Req = { environmentKey: string } getFeatureSegment: { id: number } - getFeatureDependencies: { environmentId: string; featureId: number } + getFeatureDependencies: { + environmentId: string + featureId: number + // Lists them as if this change request were live. + changeRequestId?: number + } getFeatureDependents: { environmentId: string; featureId: number } createFeatureDependency: { environmentId: string @@ -909,8 +914,8 @@ export type Req = { })[] } deleteChangeRequest: { id: number } - getPendingDependencyChanges: { featureId: number } - getChangeRequestDependencyChanges: { changeRequestId: number } + getChangeRequestIdsStaging: { featureId: number } + getFeatureIdsStaged: { changeRequestId: number } getSamlConfiguration: { name: string } getSamlConfigurations: { organisation_id: number } getSamlConfigurationMetadata: { name: string } diff --git a/frontend/common/types/responses.ts b/frontend/common/types/responses.ts index 41baacbfe89b..ef88a34faae5 100644 --- a/frontend/common/types/responses.ts +++ b/frontend/common/types/responses.ts @@ -191,12 +191,6 @@ export type StagedDependencyChange = { prerequisite: DependencyFeature } -export type PendingDependencyChanges = { - changeRequestId: number - featureId: number - changes: StagedDependencyChange[] -} - export type DependencyConflictCode = | 'circular_dependency' | 'dependency_exists' @@ -1615,7 +1609,8 @@ export type Res = { featureDependencies: { results: DependencyEdge[] } featureDependents: { results: DependencyEdge[] } featureDependency: DependencyEdge - pendingDependencyChanges: PendingDependencyChanges[] + changeRequestIdsStaging: number[] + featureIdsStaged: number[] featureVersions: PagedResponse users: User[] enableFeatureVersioning: { id: string } diff --git a/frontend/common/utils/__tests__/diffDependencies.test.ts b/frontend/common/utils/__tests__/diffDependencies.test.ts new file mode 100644 index 000000000000..a5c97fb28582 --- /dev/null +++ b/frontend/common/utils/__tests__/diffDependencies.test.ts @@ -0,0 +1,34 @@ +import { DependencyEdge } from 'common/types/responses' +import { diffDependencies } from 'common/utils/diffDependencies' + +const edge = (prerequisiteId: number) => + ({ + feature: { id: 1, name: 'checkout' }, + prerequisite: { id: prerequisiteId, name: `prereq_${prerequisiteId}` }, + } as unknown as DependencyEdge) + +describe('diffDependencies', () => { + it('reads a prerequisite only in the change request as an add', () => { + expect(diffDependencies([edge(1)], [edge(1), edge(2)])).toEqual([ + { action: 'add', prerequisite: { id: 2, name: 'prereq_2' } }, + ]) + }) + + it('reads a live prerequisite missing from the change request as a removal', () => { + expect(diffDependencies([edge(1), edge(2)], [edge(1)])).toEqual([ + { action: 'remove', prerequisite: { id: 2, name: 'prereq_2' } }, + ]) + }) + + it('lists adds before removals', () => { + expect( + diffDependencies([edge(1), edge(2)], [edge(1), edge(3)]).map( + (change) => change.action, + ), + ).toEqual(['add', 'remove']) + }) + + it('finds no change when the lists match', () => { + expect(diffDependencies([edge(1)], [edge(1)])).toEqual([]) + }) +}) diff --git a/frontend/common/utils/diffDependencies.ts b/frontend/common/utils/diffDependencies.ts new file mode 100644 index 000000000000..ae1b4ee6aa74 --- /dev/null +++ b/frontend/common/utils/diffDependencies.ts @@ -0,0 +1,19 @@ +import { DependencyEdge, StagedDependencyChange } from 'common/types/responses' + +// The API lists a change request's dependencies as if it were live, so what +// it changes is that list against today's. +export const diffDependencies = ( + live: DependencyEdge[], + scoped: DependencyEdge[], +): StagedDependencyChange[] => { + const liveIds = new Set(live.map((edge) => edge.prerequisite.id)) + const scopedIds = new Set(scoped.map((edge) => edge.prerequisite.id)) + return [ + ...scoped + .filter((edge) => !liveIds.has(edge.prerequisite.id)) + .map(({ prerequisite }) => ({ action: 'add' as const, prerequisite })), + ...live + .filter((edge) => !scopedIds.has(edge.prerequisite.id)) + .map(({ prerequisite }) => ({ action: 'remove' as const, prerequisite })), + ] +} diff --git a/frontend/web/components/ChangeRequestDependencyChanges.tsx b/frontend/web/components/ChangeRequestDependencyChanges.tsx new file mode 100644 index 000000000000..8bf000fe2d8a --- /dev/null +++ b/frontend/web/components/ChangeRequestDependencyChanges.tsx @@ -0,0 +1,46 @@ +import { FC } from 'react' +import { useGetFeatureDependenciesQuery } from 'common/services/useFeatureDependency' +import { diffDependencies } from 'common/utils/diffDependencies' +import DependencyChangesTable from './DependencyChangesTable' +import ErrorMessage from './ErrorMessage' + +type ChangeRequestDependencyChangesProps = { + environmentId: string + featureId: number + changeRequestId: number + isEnabled?: (prerequisiteId: number) => boolean +} + +// A feature's prerequisite changes in one change request: its dependencies +// as if the change request were live, against today's. +const ChangeRequestDependencyChanges: FC< + ChangeRequestDependencyChangesProps +> = ({ changeRequestId, environmentId, featureId, isEnabled }) => { + const live = useGetFeatureDependenciesQuery({ environmentId, featureId }) + const scoped = useGetFeatureDependenciesQuery({ + changeRequestId, + environmentId, + featureId, + }) + + if (live.isLoading || scoped.isLoading) { + return ( +
+ +
+ ) + } + if (live.isError || scoped.isError || !live.data || !scoped.data) { + return + } + + const changes = diffDependencies(live.data.results, scoped.data.results) + if (!changes.length) { + return ( +

No prerequisite changes.

+ ) + } + return +} + +export default ChangeRequestDependencyChanges diff --git a/frontend/web/components/diff/DiffDependencies.tsx b/frontend/web/components/diff/DiffDependencies.tsx index a8cc056b3fce..f3032b43df9a 100644 --- a/frontend/web/components/diff/DiffDependencies.tsx +++ b/frontend/web/components/diff/DiffDependencies.tsx @@ -1,18 +1,18 @@ import { FC } from 'react' -import { PendingDependencyChanges } from 'common/types/responses' -import { useGetChangeRequestDependencyChangesQuery } from 'common/services/useFeatureDependency' +import { useGetFeatureIdsStagedQuery } from 'common/services/useFeatureDependency' import { useGetProjectFlagQuery } from 'common/services/useProjectFlag' -import DependencyChangesTable from 'components/DependencyChangesTable' +import ChangeRequestDependencyChanges from 'components/ChangeRequestDependencyChanges' import Panel from 'components/base/grid/Panel' type DiffDependenciesProps = { changeRequestId: number + environmentId: string projectId: string } const FeatureDependencyChanges: FC< - PendingDependencyChanges & { projectId: string } -> = ({ changes, featureId, projectId }) => { + DiffDependenciesProps & { featureId: number } +> = ({ changeRequestId, environmentId, featureId, projectId }) => { const { data: feature } = useGetProjectFlagQuery({ id: featureId, project: Number(projectId), @@ -26,26 +26,27 @@ const FeatureDependencyChanges: FC< Prerequisite changes in this request. Nothing changes until it is published.

- + ) } // The prerequisite changes staged in a change request, per feature. -const DiffDependencies: FC = ({ - changeRequestId, - projectId, -}) => { - const { data } = useGetChangeRequestDependencyChangesQuery({ - changeRequestId, +const DiffDependencies: FC = (props) => { + const { data: featureIds } = useGetFeatureIdsStagedQuery({ + changeRequestId: props.changeRequestId, }) return ( <> - {(data ?? []).map((entry) => ( + {(featureIds ?? []).map((featureId) => ( ))} diff --git a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx index 038c0ff31bbc..27ba56206e17 100644 --- a/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx +++ b/frontend/web/components/modals/create-feature/tabs/FeatureDependenciesTab/PendingChangeRequests/PendingChangeRequests.tsx @@ -3,14 +3,14 @@ import cn from 'classnames' import moment from 'moment' import { ChangeRequestSummary } from 'common/types/responses' import { useGetChangeRequestsQuery } from 'common/services/useChangeRequest' -import { useGetPendingDependencyChangesQuery } from 'common/services/useFeatureDependency' +import { useGetChangeRequestIdsStagingQuery } from 'common/services/useFeatureDependency' import Accordion from 'components/base/Accordion' import ChangeRequestLink from 'components/modals/create-feature/tabs/FeatureDependenciesTab/ChangeRequestLink' import Chip from 'components/base/Chip' import Icon from 'components/icons/Icon' import { colorIconSecondary } from 'common/theme/tokens' import ErrorMessage from 'components/ErrorMessage' -import DependencyChangesTable from 'components/DependencyChangesTable' +import ChangeRequestDependencyChanges from 'components/ChangeRequestDependencyChanges' import './PendingChangeRequests.scss' type PendingChangeRequestsProps = { @@ -33,7 +33,9 @@ const PendingChangeRequests: FC = ({ }) => { const now = useRef(new Date().toISOString()) const highlightRef = useRef(null) - const { data: pending } = useGetPendingDependencyChangesQuery({ featureId }) + const { data: changeRequestIds } = useGetChangeRequestIdsStagingQuery({ + featureId, + }) const { data: open, isError: isOpenError } = useGetChangeRequestsQuery({ committed: false, environmentId, @@ -50,14 +52,11 @@ const PendingChangeRequests: FC = ({ // A change request that was published or deleted has nothing pending. const groups = useMemo(() => { const live = [...(open?.results ?? []), ...(scheduled?.results ?? [])] - return (pending ?? []).flatMap(({ changeRequestId, changes }) => { - const changeRequest = live.find((cr) => cr.id === changeRequestId) - return changeRequest ? [{ changeRequest, changes }] : [] - }) - }, [pending, open, scheduled]) + return live.filter((cr) => changeRequestIds?.includes(cr.id)) + }, [changeRequestIds, open, scheduled]) const hasHighlight = groups.some( - ({ changeRequest }) => changeRequest.id === highlightId, + (changeRequest) => changeRequest.id === highlightId, ) useEffect(() => { if (!hasHighlight) return @@ -72,7 +71,7 @@ const PendingChangeRequests: FC = ({ // Without the change request lists, staged changes cannot be matched to // one, and hiding them would read as nothing pending. - if (pending?.length && (isOpenError || isScheduledError)) { + if (changeRequestIds?.length && (isOpenError || isScheduledError)) { return ( ) @@ -87,7 +86,7 @@ const PendingChangeRequests: FC = ({ Not live yet. These go live when their change request is published.

- {groups.map(({ changeRequest, changes }) => ( + {groups.map((changeRequest) => (
= ({ } meta={} > - + {/* In the body, as the header is the toggle and a link cannot sit inside a button. */}
diff --git a/frontend/web/components/pages/ChangeRequestDetailPage.tsx b/frontend/web/components/pages/ChangeRequestDetailPage.tsx index 231b3b2155cb..fe86c900f5b8 100644 --- a/frontend/web/components/pages/ChangeRequestDetailPage.tsx +++ b/frontend/web/components/pages/ChangeRequestDetailPage.tsx @@ -413,6 +413,7 @@ const ChangeRequestDetailPage: FC = ({ match }) => { ) && ( )}