From 5417662f5a121f9522be1d6eedbb6eb800b50d32 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Wed, 31 Jan 2024 18:56:56 +0900 Subject: [PATCH] chore: add metrics for conflict creation detection (#6022) This PR adds a 'change-request-conflict-created' event whenever someone save a strategy update for a strategy that's used in either pending or scheduled change requests. Data for pending change requests will only be sent if change requests are enabled. Data for scheduled change requests will be sent regardless. Getting this data is somewhat involved, so I've extracted as much of the logic into a separate file as possible. The event re-uses the existing `change_request` metric and sends the following data for each change request that we discover conflicts on: ```ts { state: ChangeRequestState, changeRequest: string, // # action: 'edit-strategy', eventType: 'conflict-created' } ``` There's only one action for this for now, but we could expand this event to things such as strategy deletion, feature archival, in the future. That said, I'd be happy to take it out. ## Discussion points ### Has the strategy actually been updated? This does not check whether a strategy has actually changed before emitting the event, only that you save your strategy changes. This assumes that most people will simply close the modal by clicking/tapping outside it or using the escape key instead of pressing save. However, it will likely lead to some false positives. If we think that is an issue, I would suggest adding a check that something in the strategy has actually changed in a follow-up PR. --- .../FeatureStrategyEdit.tsx | 39 +++++- .../change-request-conflict-data.test.ts | 113 ++++++++++++++++++ .../change-request-conflict-data.ts | 55 +++++++++ .../src/utils/unique-change-request-id.ts | 7 +- 4 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/change-request-conflict-data.test.ts create mode 100644 frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/change-request-conflict-data.ts diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.tsx index 4c19a9d7499..5a617ab7db6 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/FeatureStrategyEdit.tsx @@ -28,6 +28,11 @@ import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled'; import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi'; import { usePendingChangeRequests } from 'hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests'; import { usePlausibleTracker } from 'hooks/usePlausibleTracker'; +import { useScheduledChangeRequestsWithStrategy } from 'hooks/api/getters/useScheduledChangeRequestsWithStrategy/useScheduledChangeRequestsWithStrategy'; +import { + getChangeRequestConflictCreatedData, + getChangeRequestConflictCreatedDataFromScheduleData, +} from './change-request-conflict-data'; const useTitleTracking = () => { const [previousTitle, setPreviousTitle] = useState(''); @@ -92,7 +97,7 @@ export const FeatureStrategyEdit = () => { const navigate = useNavigate(); const { addChange } = useChangeRequestApi(); const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId); - const { refetch: refetchChangeRequests } = + const { refetch: refetchChangeRequests, data: pendingChangeRequests } = usePendingChangeRequests(projectId); const { setPreviousTitle } = useTitleTracking(); @@ -123,6 +128,37 @@ export const FeatureStrategyEdit = () => { } }, [feature]); + const { trackEvent } = usePlausibleTracker(); + const { changeRequests: scheduledChangeRequestThatUseStrategy } = + useScheduledChangeRequestsWithStrategy(projectId, strategyId); + + const pendingCrsUsingThisStrategy = getChangeRequestConflictCreatedData( + pendingChangeRequests, + featureId, + strategyId, + uiConfig, + ); + + const scheduledCrsUsingThisStrategy = + getChangeRequestConflictCreatedDataFromScheduleData( + scheduledChangeRequestThatUseStrategy, + uiConfig, + ); + + const emitConflictsCreatedEvents = (): void => + [ + ...pendingCrsUsingThisStrategy, + ...scheduledCrsUsingThisStrategy, + ].forEach((data) => + trackEvent('change_request', { + props: { + ...data, + action: 'edit-strategy', + eventType: 'conflict-created', + }, + }), + ); + const { segments: savedStrategySegments, refetchSegments: refetchSavedStrategySegments, @@ -182,6 +218,7 @@ export const FeatureStrategyEdit = () => { } else { await onStrategyEdit(payload); } + emitConflictsCreatedEvents(); refetchFeature(); navigate(formatFeaturePath(projectId, featureId)); } catch (error: unknown) { diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/change-request-conflict-data.test.ts b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/change-request-conflict-data.test.ts new file mode 100644 index 00000000000..262b20bf4d5 --- /dev/null +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/change-request-conflict-data.test.ts @@ -0,0 +1,113 @@ +import { IUiConfig } from 'interfaces/uiConfig'; +import { + getChangeRequestConflictCreatedData, + getChangeRequestConflictCreatedDataFromScheduleData, +} from './change-request-conflict-data'; + +const uiConfig: Pick = { + baseUriPath: '/some-base-uri', +}; +const unleashIdentifier = uiConfig.baseUriPath; +const featureId = 'flag-with-deleted-scheduler'; +const strategyId = 'ed2ffa14-004c-4ed1-931b-78761681c54a'; + +const changeRequestWithStrategy = { + id: 105, + features: [ + { + name: featureId, + changes: [ + { + action: 'updateStrategy' as const, + payload: { + id: strategyId, + }, + }, + ], + }, + ], + state: 'In review' as const, +}; + +const changeRequestWithoutStrategy = { + id: 106, + features: [ + { + name: featureId, + changes: [ + { + action: 'deleteStrategy' as const, + payload: { + id: strategyId, + }, + }, + ], + }, + { + name: featureId, + changes: [ + { + action: 'addStrategy' as const, + payload: {}, + }, + ], + }, + ], + state: 'In review' as const, +}; + +test('it finds crs that update a strategy', () => { + const results = getChangeRequestConflictCreatedData( + [changeRequestWithStrategy], + featureId, + strategyId, + uiConfig, + ); + + expect(results).toStrictEqual([ + { + state: changeRequestWithStrategy.state, + changeRequest: `${unleashIdentifier}#${changeRequestWithStrategy.id}`, + }, + ]); +}); + +test('it does not return crs that do not update a strategy', () => { + const results = getChangeRequestConflictCreatedData( + [changeRequestWithoutStrategy], + featureId, + strategyId, + uiConfig, + ); + + expect(results).toStrictEqual([]); +}); + +test('it maps scheduled change request data', () => { + const scheduledChanges = [ + { + id: 103, + environment: 'development', + }, + { + id: 104, + environment: 'development', + }, + ]; + + const results = getChangeRequestConflictCreatedDataFromScheduleData( + scheduledChanges, + uiConfig, + ); + + expect(results).toStrictEqual([ + { + state: 'Scheduled', + changeRequest: `${unleashIdentifier}#103`, + }, + { + state: 'Scheduled', + changeRequest: `${unleashIdentifier}#104`, + }, + ]); +}); diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/change-request-conflict-data.ts b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/change-request-conflict-data.ts new file mode 100644 index 00000000000..8df88432e70 --- /dev/null +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyEdit/change-request-conflict-data.ts @@ -0,0 +1,55 @@ +import { + ChangeRequestState, + ChangeRequestType, + IChangeRequestFeature, + IFeatureChange, +} from 'component/changeRequest/changeRequest.types'; +import { ScheduledChangeRequestViewModel } from 'hooks/api/getters/useScheduledChangeRequestsWithStrategy/useScheduledChangeRequestsWithStrategy'; +import { IUiConfig } from 'interfaces/uiConfig'; +import { getUniqueChangeRequestId } from 'utils/unique-change-request-id'; + +type ChangeRequestConflictCreatedData = { + changeRequest: string; + state: ChangeRequestState; +}; + +export const getChangeRequestConflictCreatedData = ( + changeRequests: + | { + state: ChangeRequestType['state']; + id: ChangeRequestType['id']; + features: { + name: IChangeRequestFeature['name']; + changes: (Pick & { + payload: { id?: number | string }; + })[]; + }[]; + }[] + | undefined, + featureId: string, + strategyId: string, + uiConfig: Pick, +): ChangeRequestConflictCreatedData[] => + changeRequests + ?.filter((cr) => + cr.features + .find((feature) => feature.name === featureId) + ?.changes.some( + (change) => + change.action === 'updateStrategy' && + change.payload.id === strategyId, + ), + ) + .map((cr) => ({ + changeRequest: getUniqueChangeRequestId(uiConfig, cr.id), + state: cr.state, + })) ?? []; + +export const getChangeRequestConflictCreatedDataFromScheduleData = ( + changeRequests: Pick[] | undefined, + uiConfig: Pick, +): ChangeRequestConflictCreatedData[] => + changeRequests?.map((cr) => ({ + changeRequest: getUniqueChangeRequestId(uiConfig, cr.id), + state: 'Scheduled' as const, + })) ?? []; diff --git a/frontend/src/utils/unique-change-request-id.ts b/frontend/src/utils/unique-change-request-id.ts index 15f16fbfe0c..7573551df78 100644 --- a/frontend/src/utils/unique-change-request-id.ts +++ b/frontend/src/utils/unique-change-request-id.ts @@ -3,8 +3,7 @@ import { IUiConfig } from 'interfaces/uiConfig'; export const getUniqueChangeRequestId = ( uiConfig: Pick, changeRequestId: number, -) => { - return `${ +) => + `${ uiConfig.baseUriPath || uiConfig.versionInfo?.instanceId - }/change-requests/${changeRequestId}?version=${uiConfig.versionInfo}`; -}; + }#${changeRequestId}`;