diff --git a/src/sentry/investigations/endpoints/organization_investigation_action_request_apply.py b/src/sentry/investigations/endpoints/organization_investigation_action_request_apply.py index effe8723deac..bd0bcaa9292e 100644 --- a/src/sentry/investigations/endpoints/organization_investigation_action_request_apply.py +++ b/src/sentry/investigations/endpoints/organization_investigation_action_request_apply.py @@ -90,6 +90,7 @@ def post( request=request, actor_id=actor_id, dry_run=validator.validated_data["dry_run"], + expected_action=validator.validated_data.get("action"), ) except ApplyFailed as failed: return Response(failed.error, status=_STATUS.get(failed.error["code"], 409)) diff --git a/src/sentry/investigations/endpoints/validators/action_request.py b/src/sentry/investigations/endpoints/validators/action_request.py index 138c3f481689..36424b478403 100644 --- a/src/sentry/investigations/endpoints/validators/action_request.py +++ b/src/sentry/investigations/endpoints/validators/action_request.py @@ -10,3 +10,8 @@ class ActionRequestApplyValidator(StrictCamelSnakeValidator): default=False, help_text="Run every check without applying, to see whether the apply would work.", ) + action = serializers.JSONField( + required=False, + help_text="The action the user saw. If the request's action is different, the apply fails " + "with `action_changed`.", + ) diff --git a/src/sentry/investigations/services/action_kinds/base.py b/src/sentry/investigations/services/action_kinds/base.py index 2f2539a45a44..e7b9199b221f 100644 --- a/src/sentry/investigations/services/action_kinds/base.py +++ b/src/sentry/investigations/services/action_kinds/base.py @@ -44,6 +44,7 @@ class ApplyErrorCode(StrEnum): WITHDRAWN = "withdrawn" STALE = "stale" VALIDATION_FAILED = "validation_failed" + ACTION_CHANGED = "action_changed" class ApplyError(TypedDict): diff --git a/src/sentry/investigations/services/action_requests.py b/src/sentry/investigations/services/action_requests.py index 2c15eefcb20d..836a156cade0 100644 --- a/src/sentry/investigations/services/action_requests.py +++ b/src/sentry/investigations/services/action_requests.py @@ -1,7 +1,8 @@ from __future__ import annotations import logging -from collections.abc import Sequence +from collections.abc import Mapping, Sequence +from typing import Any from django.db import router, transaction from django.db.models import Q, QuerySet @@ -287,6 +288,7 @@ def _apply( request: Request, actor_id: int, dry_run: bool, + expected_action: Mapping[str, Any] | None, ) -> InvestigationActionRequest: investigations = Investigation.objects.select_related("organization") runs = InvestigationOrchestrationRun.objects.all() @@ -300,6 +302,14 @@ def _apply( run = runs.get(investigation=locked) action_request = action_requests.get(id=action_request.id) kind = _check_can_apply(locked, action_request, request) + # Seer can rewrite a suggestion under the same id, so only apply the one the user saw. + if expected_action is not None and expected_action != action_request.action_payload: + raise ApplyFailed( + { + "detail": "Seer changed this suggestion. Check the new one.", + "code": ApplyErrorCode.ACTION_CHANGED, + } + ) context = ApplyContext( investigation=locked, @@ -354,6 +364,7 @@ def apply_action_request( request: Request, actor_id: int, dry_run: bool = False, + expected_action: Mapping[str, Any] | None = None, ) -> InvestigationActionRequest: """Apply a suggested action as the requesting user, or raise ApplyFailed. A dry run only says whether it would work, and doesn't take locks or change anything. @@ -375,6 +386,7 @@ def apply_action_request( request=request, actor_id=actor_id, dry_run=dry_run, + expected_action=expected_action, ) except ApplyFailed as failed: error = failed diff --git a/static/app/views/investigations/__stories__/investigationFixtureApi.tsx b/static/app/views/investigations/__stories__/investigationFixtureApi.tsx index e2d7d82ea1e1..7306f4088a01 100644 --- a/static/app/views/investigations/__stories__/investigationFixtureApi.tsx +++ b/static/app/views/investigations/__stories__/investigationFixtureApi.tsx @@ -14,6 +14,7 @@ import {RequestError} from 'sentry/utils/requestError/requestError'; import {useOrganization} from 'sentry/utils/useOrganization'; import {useUser} from 'sentry/utils/useUser'; import {commentStoryAuthors} from 'sentry/views/investigations/__stories__/commentStoryFixtures'; +import type {ActionRequestError} from 'sentry/views/investigations/api'; import { InvestigationBlockExecutionFixture, InvestigationCommentFixture, @@ -23,7 +24,9 @@ import { InvestigationQueryOutputFixture, InvestigationTranscriptBlockFixture, } from 'sentry/views/investigations/fixtures'; +import {getSourceMonitor} from 'sentry/views/investigations/source'; import type { + InvestigationActionRequest, InvestigationComment, InvestigationDetail, InvestigationExecutionDetail, @@ -44,6 +47,8 @@ type InvestigationFixtureApiProps = { /** Page-level and block comments, keyed by investigation id. */ comments?: Record; details?: InvestigationDetail[]; + /** Dry-run error responses, keyed by action request id. */ + dryRunErrors?: Record; executions?: Record; featureEnabled?: boolean; list?: InvestigationListItem[]; @@ -65,6 +70,11 @@ type FixtureResponse = { headers?: Record; }; +type FixtureErrorResponse = { + body: ActionRequestError & {detail: string}; + status: number; +}; + type FixtureHandlerResult = Promise | typeof NO_MATCH; type FixtureHandler = ( path: string, @@ -87,6 +97,7 @@ export function InvestigationFixtureApi({ commentPageLinks = {}, comments = {}, details = [], + dryRunErrors = {}, executions = {}, featureEnabled = true, list = [], @@ -117,6 +128,7 @@ export function InvestigationFixtureApi({ commentPageLinks, comments, details, + dryRunErrors, executions, featureEnabled, list, @@ -239,16 +251,11 @@ function createFixtureHandler(config: FixtureApiConfig): FixtureHandler { return new Promise(() => {}); } if (config.mode === 'error') { - const method = normalizeMethod(options.method); - const response: ResponseMeta = { - status: 500, - statusText: 'Internal Server Error', - responseJSON: {detail: 'Fixture API error'}, - responseText: 'Fixture API error', - getResponseHeader: () => null, - }; return Promise.reject( - new RequestError(method, path, new Error('Fixture API error'), response) + fixtureRequestError(normalizeMethod(options.method), path, { + status: 500, + body: {detail: 'Fixture API error'}, + }) ); } @@ -262,6 +269,7 @@ type FixtureState = { commentPageLinks: Record; comments: Map; details: Map; + dryRunErrors: Record; executions: Map; list: InvestigationListItem[]; orchestration: Map; @@ -300,6 +308,7 @@ function createFixtureState(config: FixtureApiConfig): FixtureState { ]) ), details, + dryRunErrors: {...config.dryRunErrors}, list, orchestration: new Map( Object.entries(config.orchestration ?? {}).map(([key, run]) => [ @@ -455,6 +464,54 @@ function handleFixtureRequest( }; } + if ( + parts[1] === 'action-requests' && + parts[3] === 'apply' && + parts.length === 4 && + method === 'POST' + ) { + const detail = getFixtureDetail(state, investigationId); + const actionRequest = detail.actionRequests?.find( + candidate => candidate.id === parts[2] + ); + if (!actionRequest?.action) { + throw new Error(`Unknown fixture action request: ${parts[2]}`); + } + if (data.dryRun === true) { + const dryRunError = state.dryRunErrors[actionRequest.id]; + if (dryRunError) { + throw fixtureRequestError(method, path, dryRunError); + } + return {body: {actionRequest: cloneFixture(actionRequest)}}; + } + const {action} = actionRequest; + const applied: InvestigationActionRequest = { + ...actionRequest, + state: 'applied', + appliedBy: state.viewer, + appliedDetails: + action.kind === 'update_detector' + ? { + detectorId: action.detectorId, + detectorName: + getSourceMonitor(detail.source, action.detectorId)?.name ?? + action.detectorId, + fields: Object.keys(action.changes), + } + : null, + canApply: false, + cannotApplyReason: 'not_suggested', + dateApplied: new Date().toISOString(), + }; + setFixtureDetail(state, { + ...detail, + actionRequests: detail.actionRequests?.map(candidate => + candidate.id === applied.id ? applied : candidate + ), + }); + return {body: {actionRequest: cloneFixture(applied)}}; + } + if (parts[1] === 'title-generation' && method === 'GET') { const detail = getFixtureDetail(state, investigationId); return { @@ -802,6 +859,21 @@ function setFixtureDetail(state: FixtureState, detail: InvestigationDetail) { state.list[listIndex] = {...state.list[listIndex], ...detail}; } +function fixtureRequestError( + method: ReturnType, + path: string, + {body, status}: {body: {detail: string}; status: number} +) { + const response: ResponseMeta = { + status, + statusText: 'Fixture API error', + responseJSON: body, + responseText: JSON.stringify(body), + getResponseHeader: () => null, + }; + return new RequestError(method, path, new Error('Fixture API error'), response); +} + function normalizeMethod(method: RequestOptions['method']) { return method ?? 'GET'; } diff --git a/static/app/views/investigations/actionRequests/actionKinds.tsx b/static/app/views/investigations/actionRequests/actionKinds.tsx new file mode 100644 index 000000000000..92b68bf107fd --- /dev/null +++ b/static/app/views/investigations/actionRequests/actionKinds.tsx @@ -0,0 +1,52 @@ +import type {ComponentType, ReactNode} from 'react'; + +import type {ApplyCheck} from 'sentry/views/investigations/actionRequests/applyButton'; +import {updateDetectorUI} from 'sentry/views/investigations/actionRequests/updateDetector'; +import type {ActionRequestError} from 'sentry/views/investigations/api'; +import type { + InvestigationAction, + InvestigationActionRequest, + InvestigationDetail, +} from 'sentry/views/investigations/types'; + +type ActionByKind = {[A in InvestigationAction as A['kind']]: A}; +type ActionKind = keyof ActionByKind; + +export type ApplyBlockReason = + | InvestigationActionRequest['cannotApplyReason'] + | ActionRequestError['code']; + +type KindProps = {action: A; actionRequest: InvestigationActionRequest}; + +export type ActionKindApplyProps = KindProps & { + applying: boolean; + check: ApplyCheck; + message: string | null; + onApply: () => void; +}; + +export type ActionKindDetailsProps = KindProps & { + source: InvestigationDetail['source'] | undefined; +}; + +export type ActionKindUI = { + Apply: ComponentType>; + checksBeforeApply: boolean; + getAppliedSummary: ( + props: KindProps & {source: InvestigationDetail['source'] | undefined} + ) => ReactNode; + getCannotApplyMessage: (reason: ApplyBlockReason) => string | null; + Details?: ComponentType>; + getSuccessMessage?: () => string; +}; + +const ACTION_KIND_UI: {[K in ActionKind]?: ActionKindUI} = { + update_detector: updateDetectorUI, +}; + +export function getActionKind( + action: ActionByKind[K] | null +): {action: ActionByKind[K]; ui: ActionKindUI} | null { + const ui = action ? ACTION_KIND_UI[action.kind as K] : undefined; + return action && ui ? {action, ui} : null; +} diff --git a/static/app/views/investigations/actionRequests/actionRequestCard.tsx b/static/app/views/investigations/actionRequests/actionRequestCard.tsx index 2b0bdbad23bf..ca040b6c6a96 100644 --- a/static/app/views/investigations/actionRequests/actionRequestCard.tsx +++ b/static/app/views/investigations/actionRequests/actionRequestCard.tsx @@ -1,18 +1,42 @@ import {useId} from 'react'; +import {Alert} from '@sentry/scraps/alert'; import {Stack} from '@sentry/scraps/layout'; import {Heading, Text} from '@sentry/scraps/text'; +import {toast} from '@sentry/scraps/toast'; -import {tct} from 'sentry/locale'; -import {ActionRequestDetails} from 'sentry/views/investigations/actionRequests/actionRequestDetails'; +import {TimeSince} from 'sentry/components/timeSince'; +import {t, tct} from 'sentry/locale'; +import {useOrganization} from 'sentry/utils/useOrganization'; +import {useUser} from 'sentry/utils/useUser'; +import { + ActionRequestApply, + ActionRequestDetails, + getAppliedSummary, + getApplyErrorMessage, + getApplySuccessMessage, + getCannotApplyMessage, + useApplyCheck, +} from 'sentry/views/investigations/actionRequests/actionRequestDetails'; +import { + parseApplyError, + useApplyInvestigationActionRequestMutation, +} from 'sentry/views/investigations/api'; import {ensureSentenceEnd} from 'sentry/views/investigations/sentence'; import type { InvestigationActionRequest, InvestigationDetail, } from 'sentry/views/investigations/types'; +const SHOWN_STATES = new Set([ + 'suggested', + 'applied', + 'withdrawn', +]); + type ActionRequestCardProps = { actionRequest: InvestigationActionRequest; + investigationId: string; heading?: string; source?: InvestigationDetail['source']; }; @@ -20,11 +44,37 @@ type ActionRequestCardProps = { export function ActionRequestCard({ actionRequest, heading, + investigationId, source, }: ActionRequestCardProps) { const headingId = useId(); + const organization = useOrganization(); + const check = useApplyCheck(investigationId, actionRequest); + const applyMutation = useApplyInvestigationActionRequestMutation( + organization.slug, + investigationId, + { + onSuccess: (_response, applied) => { + const message = getApplySuccessMessage(applied.action); + if (message) { + toast.success(message); + } + }, + onError: (error, applied) => { + const applyError = parseApplyError(error); + if (applyError?.code === 'validation_failed') { + return; + } + toast.error( + applyError + ? getCannotApplyMessage(applied.action, applyError.code) + : t('Unable to apply this change.') + ); + }, + } + ); - if (actionRequest.state !== 'suggested' && actionRequest.state !== 'withdrawn') { + if (!isShownActionRequest(actionRequest)) { return null; } @@ -53,16 +103,72 @@ export function ActionRequestCard({ ); } + const applying = + applyMutation.isPending && applyMutation.variables?.id === actionRequest.id; + const applyError = + applyMutation.variables?.id === actionRequest.id + ? parseApplyError(applyMutation.error) + : null; + const appliedSummary = + actionRequest.state === 'applied' ? getAppliedSummary(actionRequest, source) : null; + return ( {headingElement} - - {tct('[title] [rationale]', { - title: {ensureSentenceEnd(actionRequest.title)}, - rationale: actionRequest.rationale, - })} - + + + {appliedSummary ?? + tct('[title] [rationale]', { + title: {ensureSentenceEnd(actionRequest.title)}, + rationale: actionRequest.rationale, + })} + + {actionRequest.state === 'applied' ? ( + + ) : null} + + {applyError?.code === 'validation_failed' ? ( + + + {t( + 'This change could not be applied. %s', + getApplyErrorMessage(actionRequest.action, applyError) + )} + + + ) : null} + applyMutation.mutate(actionRequest)} + /> ); } + +function AppliedBy({actionRequest}: {actionRequest: InvestigationActionRequest}) { + const user = useUser(); + const {appliedBy, dateApplied} = actionRequest; + const time = dateApplied ? : null; + if (!appliedBy) { + return ( + + {time ? tct('Applied [time]', {time}) : t('Applied')} + + ); + } + const name = appliedBy.id === user.id ? t('you') : appliedBy.name || appliedBy.email; + return ( + + {time + ? tct('Applied by [name] • [time]', {name, time}) + : tct('Applied by [name]', {name})} + + ); +} + +function isShownActionRequest(actionRequest: InvestigationActionRequest) { + return SHOWN_STATES.has(actionRequest.state); +} diff --git a/static/app/views/investigations/actionRequests/actionRequestDetails.tsx b/static/app/views/investigations/actionRequests/actionRequestDetails.tsx index f71b64c0a9ca..1a0c56971e02 100644 --- a/static/app/views/investigations/actionRequests/actionRequestDetails.tsx +++ b/static/app/views/investigations/actionRequests/actionRequestDetails.tsx @@ -1,8 +1,22 @@ +import type {ReactNode} from 'react'; +import {useQuery} from '@tanstack/react-query'; + import {Text} from '@sentry/scraps/text'; -import {MonitorChanges} from 'sentry/views/investigations/actionRequests/monitorChanges'; -import {getSourceMonitor} from 'sentry/views/investigations/source'; +import {t} from 'sentry/locale'; +import {useOrganization} from 'sentry/utils/useOrganization'; +import { + getActionKind, + type ApplyBlockReason, +} from 'sentry/views/investigations/actionRequests/actionKinds'; +import type {ApplyCheck} from 'sentry/views/investigations/actionRequests/applyButton'; +import { + actionRequestDryRunQueryOptions, + type ActionRequestError, +} from 'sentry/views/investigations/api'; +import {ensureSentenceEnd} from 'sentry/views/investigations/sentence'; import type { + InvestigationAction, InvestigationActionRequest, InvestigationDetail, } from 'sentry/views/investigations/types'; @@ -13,22 +27,143 @@ type ActionRequestDetailsProps = { }; export function ActionRequestDetails({actionRequest, source}: ActionRequestDetailsProps) { - const {action, impact} = actionRequest; - - switch (action?.kind) { - case 'update_detector': - return ( - - ); + const kind = getActionKind(actionRequest.action); + if (kind?.ui.Details) { + return ( + + ); + } + return actionRequest.impact ? ( + + {actionRequest.impact} + + ) : null; +} + +type ActionRequestApplyProps = { + actionRequest: InvestigationActionRequest; + applying: boolean; + check: ApplyCheck; + onApply: () => void; +}; + +export function ActionRequestApply({ + actionRequest, + applying, + check, + onApply, +}: ActionRequestApplyProps) { + const kind = getActionKind(actionRequest.action); + if (!kind) { + return null; + } + return ( + + ); +} + +export function useApplyCheck( + investigationId: string, + actionRequest: InvestigationActionRequest +): ApplyCheck { + const organization = useOrganization(); + const checksBeforeApply = + getActionKind(actionRequest.action)?.ui.checksBeforeApply ?? false; + const enabled = actionRequest.canApply && checksBeforeApply; + const {data, isError, isPending} = useQuery({ + ...actionRequestDryRunQueryOptions(organization.slug, investigationId, actionRequest), + enabled, + }); + if (!enabled) { + return {status: 'passed'}; + } + if (isError) { + return {status: 'unavailable'}; + } + if (isPending) { + return {status: 'pending'}; + } + return data ? {status: 'blocked', error: data} : {status: 'passed'}; +} + +function getApplyMessage(actionRequest: InvestigationActionRequest, check: ApplyCheck) { + if (!actionRequest.canApply) { + return getCannotApplyMessage(actionRequest.action, actionRequest.cannotApplyReason); + } + switch (check.status) { + case 'blocked': + return getApplyErrorMessage(actionRequest.action, check.error); + case 'unavailable': + return t("Couldn't check this change in advance. Applying it checks it again."); + default: + return null; + } +} + +export function getAppliedSummary( + actionRequest: InvestigationActionRequest, + source: InvestigationDetail['source'] | undefined +): ReactNode { + const kind = getActionKind(actionRequest.action); + return kind + ? kind.ui.getAppliedSummary({action: kind.action, actionRequest, source}) + : null; +} + +export function getApplySuccessMessage(action: InvestigationAction | null) { + return getActionKind(action)?.ui.getSuccessMessage?.() ?? null; +} + +export function getApplyErrorMessage( + action: InvestigationAction | null, + error: ActionRequestError +) { + return error.code === 'validation_failed' + ? error.messages.map(ensureSentenceEnd).join(' ') + : getCannotApplyMessage(action, error.code); +} + +export function getCannotApplyMessage( + action: InvestigationAction | null, + reason: ApplyBlockReason +) { + return ( + getActionKind(action)?.ui.getCannotApplyMessage(reason) ?? + getSharedCannotApplyMessage(reason) ?? + t("This change can't be applied.") + ); +} + +function getSharedCannotApplyMessage(reason: ApplyBlockReason) { + switch (reason) { + case 'investigation_archived': + return t('Archived investigations are read-only.'); + case 'not_suggested': + return t('This can no longer be applied.'); + case 'unsupported_action': + return t("Seer can't apply this kind of change yet."); + case 'no_access': + return t("You don't have access to apply this."); + case 'already_applied': + return t('This was already applied.'); + case 'withdrawn': + return t('Seer no longer suggests this.'); + case 'action_changed': + return t('Seer changed this suggestion. Check the new one.'); + case 'validation_failed': + return t("This change wasn't valid."); default: - return impact ? ( - - {impact} - - ) : null; + return null; } } diff --git a/static/app/views/investigations/actionRequests/applyButton.tsx b/static/app/views/investigations/actionRequests/applyButton.tsx new file mode 100644 index 000000000000..90f95a9aecec --- /dev/null +++ b/static/app/views/investigations/actionRequests/applyButton.tsx @@ -0,0 +1,55 @@ +import {useId} from 'react'; + +import {Button} from '@sentry/scraps/button'; +import {Flex} from '@sentry/scraps/layout'; +import {Text} from '@sentry/scraps/text'; + +import type {ActionRequestError} from 'sentry/views/investigations/api'; + +export type ApplyCheck = + | {status: 'pending'} + | {status: 'passed'} + | {error: ActionRequestError; status: 'blocked'} + | {status: 'unavailable'}; + +type ApplyButtonProps = { + applying: boolean; + busyLabel: string; + canApply: boolean; + check: ApplyCheck; + icon: React.ReactNode; + label: string; + message: string | null; + onApply: () => void; +}; + +export function ApplyButton({ + applying, + busyLabel, + canApply, + check, + icon, + label, + message, + onApply, +}: ApplyButtonProps) { + const messageId = useId(); + const checking = canApply && check.status === 'pending'; + return ( + + + + {message} + + + ); +} diff --git a/static/app/views/investigations/actionRequests/monitorChanges.tsx b/static/app/views/investigations/actionRequests/monitorChanges.tsx index e36eef8729de..0166a89ff22e 100644 --- a/static/app/views/investigations/actionRequests/monitorChanges.tsx +++ b/static/app/views/investigations/actionRequests/monitorChanges.tsx @@ -1,7 +1,9 @@ import styled from '@emotion/styled'; import {VisuallyHidden} from '@react-aria/visually-hidden'; +import {IconCheckmark} from '@sentry/icons/checkmark'; import {IconSiren} from '@sentry/icons/siren'; +import {Tag} from '@sentry/scraps/badge'; import {Container, Flex, Grid, Stack} from '@sentry/scraps/layout'; import {Text} from '@sentry/scraps/text'; @@ -42,11 +44,19 @@ const PRIORITY_CHANGE_KEYS: Partial action.changes[key] !== undefined); const subtitle = [ monitor?.projectSlug, @@ -66,21 +76,28 @@ export function MonitorChanges({action, impact, monitor}: MonitorChangesProps) { return ( - - - - - {monitor?.name ?? t('Monitor')} - - - {subtitle.length > 0 ? ( - - - {subtitle.join(' • ')} + + + + + + {name ?? t('Monitor')} - + + {subtitle.length > 0 ? ( + + + {subtitle.join(' • ')} + + + ) : null} + + {applied ? ( + }> + {t('Applied')} + ) : null} - + {measures.length > 0 ? ( @@ -90,30 +107,43 @@ export function MonitorChanges({action, impact, monitor}: MonitorChangesProps) { ) : null} {changedKeys.map(key => { const {getLabel, monospace} = CHANGE_FIELDS[key]; + const before = formatChangeValue( + key, + action.before[key], + monitor?.detectionType, + currentAggregate + ); + const after = formatChangeValue( + key, + action.changes[key], + monitor?.detectionType, + suggestedAggregate + ); return ( {getLabel()} - - {t('Current value:')} - - {formatChangeValue( - key, - action.before[key], - monitor?.detectionType, - currentAggregate - )} + {applied ? ( + + + {after} + {' '} + + {t('was %s', before)} + - → - , {t('suggested value:')} - - {formatChangeValue( - key, - action.changes[key], - monitor?.detectionType, - suggestedAggregate - )} + ) : ( + + {t('Current value:')} + + {before} + + → + , {t('suggested value:')} + + {after} + - + )} ); })} diff --git a/static/app/views/investigations/actionRequests/nextStepCard.spec.tsx b/static/app/views/investigations/actionRequests/nextStepCard.spec.tsx index 878c8cb78f56..4209fc659076 100644 --- a/static/app/views/investigations/actionRequests/nextStepCard.spec.tsx +++ b/static/app/views/investigations/actionRequests/nextStepCard.spec.tsx @@ -1,8 +1,23 @@ -import {render, screen, within} from 'sentry-test/reactTestingLibrary'; +import {QueryClientProvider} from '@tanstack/react-query'; -import {NextStepCard} from 'sentry/views/investigations/actionRequests/nextStepCard'; +import {makeTestQueryClient} from 'sentry-test/queryClient'; +import { + render, + screen, + userEvent, + waitFor, + within, +} from 'sentry-test/reactTestingLibrary'; + +import {ConfigStore} from 'sentry/stores/configStore'; +import { + getPrimaryActionRequest, + NextStepCard, +} from 'sentry/views/investigations/actionRequests/nextStepCard'; +import {getInvestigationDetailQueryOptions} from 'sentry/views/investigations/api'; import { InvestigationActionRequestFixture, + InvestigationDetailFixture, InvestigationOrchestrationFixture, } from 'sentry/views/investigations/fixtures'; import type { @@ -58,14 +73,49 @@ function runWith( }); } +const applyUrl = + '/organizations/org-slug/investigations/investigation-1/action-requests/312/apply/'; + +type MockResponse = Parameters[0]; + +function mockApply(response: MockResponse) { + return MockApiClient.addMockResponse({ + url: applyUrl, + method: 'POST', + match: [(_url, options) => !options.data?.dryRun], + ...response, + }); +} + +function mockDryRun(response: MockResponse = {}) { + return MockApiClient.addMockResponse({ + url: applyUrl, + method: 'POST', + match: [MockApiClient.matchData({dryRun: true})], + body: {actionRequest: InvestigationActionRequestFixture()}, + ...response, + }); +} + function getCard() { return screen.getByRole('region', {name: 'Recommended action'}); } +async function findApplyButton() { + const button = screen.getByRole('button', {name: 'Update monitor'}); + await waitFor(() => expect(button).toBeEnabled()); + return button; +} + describe('NextStepCard', () => { + beforeEach(() => { + mockDryRun(); + }); + it('renders the primary request', () => { render( { it('describes cleared filters, all environments and thresholds', () => { render( { it('renders advice without an action as text only', () => { render( { it('does not add a period after closing punctuation', () => { render( { it('renders nothing without a primary request', () => { render( @@ -171,6 +225,7 @@ describe('NextStepCard', () => { it('only shows a primary from the newest report', () => { render( { it('renders nothing for a state it does not know', () => { render( @@ -210,6 +266,7 @@ describe('NextStepCard', () => { it('renders text from Seer as plain text', () => { render( { it('collapses a request Seer withdrew to its reason', () => { render( { it('collapses a withdrawn request without a reason to its title', () => { render( @@ -268,6 +327,7 @@ describe('NextStepCard', () => { it('holds space while the next step generates', () => { render( { it('holds space before the next step starts', () => { render( { it('holds space for a completed next step until its request arrives', () => { render( { it('renders nothing for a completed next step that suggested nothing', () => { render( @@ -321,6 +384,7 @@ describe('NextStepCard', () => { it('renders nothing while the run is loading', () => { render( { it('shows the newest primary over one Seer withdrew', () => { render( { expect(getCard()).not.toHaveTextContent('no longer recommends'); }); + it('still counts an applied primary as the next step', () => { + const applied = InvestigationActionRequestFixture({id: '313', state: 'applied'}); + + expect( + getPrimaryActionRequest([InvestigationActionRequestFixture(), applied], undefined) + ).toBe(applied); + }); + + it('shows the live primary over a newer one Seer replaced', () => { + render( + + ); + + expect(getCard()).toHaveTextContent('Widen the interval.'); + }); + it('holds space for a new next step over a withdrawn one', () => { render( { it('explains a next step that failed', () => { render( @@ -390,6 +485,7 @@ describe('NextStepCard', () => { it('hides an old primary while a new report is written', () => { render( { it('names the monitor and what it measures', () => { render( { it('formats static thresholds with the aggregate unit', () => { render( { it('formats each threshold with its own aggregate unit', () => { render( { it('does not describe the opening condition when the aggregate changes', () => { render( { it('formats percent thresholds like the monitor page', () => { render( { it('describes the condition that opens an issue for a percent monitor', () => { render( { it('describes the opening condition from a snapshot without a detection type', () => { render( { it('reads a percent monitor from a snapshot without a detection type', () => { render( { it('names the monitor without an opening condition for a dynamic monitor', () => { render( { it('skips a condition type it does not know', () => { render( { it('leaves out monitor details when the snapshot does not parse', () => { render( { it('leaves out a different monitor than the one the suggestion changes', () => { render( { it('shows query and environment changes in place of what they replace', () => { render( { expect(card).toHaveTextContent('Measuresp95(span.duration)'); expect(card).not.toHaveTextContent('p95(span.duration) • transaction:/api/cart'); }); + + it('applies the change and stores the applied request', async () => { + const actionRequest = InvestigationActionRequestFixture(); + const applied = InvestigationActionRequestFixture({ + state: 'applied', + canApply: false, + cannotApplyReason: 'not_suggested', + }); + const apply = mockApply({body: {actionRequest: applied}}); + const queryClient = makeTestQueryClient(); + const detailKey = getInvestigationDetailQueryOptions( + 'org-slug', + 'investigation-1' + ).queryKey; + queryClient.setQueryData(detailKey, { + headers: {}, + json: InvestigationDetailFixture({ + id: 'investigation-1', + version: 4, + actionRequests: [actionRequest], + }), + }); + render( + , + { + additionalWrapper: ({children}) => ( + {children} + ), + } + ); + + await userEvent.click(await findApplyButton()); + + expect(apply).toHaveBeenCalledWith( + applyUrl, + expect.objectContaining({method: 'POST', data: {action: actionRequest.action}}) + ); + expect(await screen.findByText('Monitor updated.')).toBeInTheDocument(); + const cached = queryClient.getQueryData(detailKey)?.json; + expect(cached?.actionRequests).toEqual([applied]); + expect(cached?.version).toBe(4); + }); + + it('shows the updating state while the change is applied', async () => { + mockApply({ + body: {actionRequest: InvestigationActionRequestFixture({state: 'applied'})}, + asyncDelay: 1_000_000, + }); + render( + + ); + + await userEvent.click(await findApplyButton()); + + expect(await screen.findByRole('button', {name: 'Updating monitor…'})).toBeDisabled(); + }); + + it.each([ + { + status: 500, + body: {detail: 'Internal Error', errorId: 'abc123'}, + message: 'Unable to apply this change.', + }, + { + status: 403, + body: {detail: "This action can't be applied.", code: 'permission_denied'}, + message: "You don't have permission to edit this monitor.", + }, + { + status: 409, + body: { + detail: 'This changed since Seer suggested it.', + code: 'stale', + current: {timeWindowSeconds: 600}, + }, + message: 'The monitor changed since Seer suggested this.', + }, + ])('says when an apply fails with $status', async ({status, body, message}) => { + mockApply({statusCode: status, body}); + render( + + ); + + await userEvent.click(await findApplyButton()); + + expect(await screen.findByText(message)).toBeInTheDocument(); + }); + + it.each([ + {status: 500, body: {detail: 'Internal Error'}}, + {status: 409, body: {detail: "This action can't be applied.", code: 'not_suggested'}}, + ])('refreshes the investigation when an apply fails with $status', async response => { + mockApply({statusCode: response.status, body: response.body}); + const queryClient = makeTestQueryClient(); + const detailKey = getInvestigationDetailQueryOptions( + 'org-slug', + 'investigation-1' + ).queryKey; + queryClient.setQueryData(detailKey, { + headers: {}, + json: InvestigationDetailFixture({id: 'investigation-1'}), + }); + render( + , + { + additionalWrapper: ({children}) => ( + {children} + ), + } + ); + + await userEvent.click(await findApplyButton()); + + await waitFor(() => + expect(queryClient.getQueryState(detailKey)?.isInvalidated).toBe(true) + ); + }); + + it.each([ + ['permission_denied', "You don't have permission to edit this monitor."], + ['detector_not_found', 'This monitor is no longer available.'], + [ + 'not_target_detector', + 'This change targets a different monitor than this investigation.', + ], + ['dynamic_detector', "Seer can't change dynamic monitors yet."], + ['investigation_archived', 'Archived investigations are read-only.'], + ])('says why a %s change cannot apply', (cannotApplyReason, message) => { + render( + + ); + + const button = within(getCard()).getByRole('button', {name: 'Update monitor'}); + expect(button).toBeDisabled(); + expect(button).toHaveAccessibleDescription(message); + }); + + it('holds the button while the change is checked', () => { + mockDryRun({asyncDelay: 1_000_000}); + render( + + ); + + const button = screen.getByRole('button', {name: 'Update monitor'}); + expect(button).toBeDisabled(); + expect(button).toHaveAttribute('aria-busy', 'true'); + }); + + it('checks the change before it is applied', async () => { + const dryRun = mockDryRun(); + render( + + ); + + expect(await findApplyButton()).toBeEnabled(); + expect(dryRun).toHaveBeenCalledTimes(1); + }); + + it.each([ + { + status: 409, + body: { + detail: 'This changed since Seer suggested it.', + code: 'stale', + current: {timeWindowSeconds: 600}, + }, + message: 'The monitor changed since Seer suggested this.', + }, + { + status: 403, + body: {detail: "This action can't be applied.", code: 'permission_denied'}, + message: "You don't have permission to edit this monitor.", + }, + { + status: 400, + body: { + detail: "The change wasn't valid.", + code: 'validation_failed', + messages: ['Invalid Time Window: 900'], + }, + message: 'Invalid Time Window: 900.', + }, + ])( + 'holds the update when the check fails with $body.code', + async ({status, body, message}) => { + mockDryRun({statusCode: status, body}); + render( + + ); + + await waitFor(() => expect(screen.getByRole('status')).toHaveTextContent(message)); + const button = screen.getByRole('button', {name: 'Update monitor'}); + expect(button).toBeDisabled(); + expect(button).toHaveAccessibleDescription(message); + } + ); + + it.each([ + {status: 500, body: {detail: 'Internal Error', errorId: 'abc123'}}, + {status: 409, body: {detail: "This action can't be applied.", code: 'new_reason'}}, + ])('keeps the update available when the check returns $status', async response => { + mockDryRun({statusCode: response.status, body: response.body}); + render( + + ); + + await waitFor(() => + expect(screen.getByRole('status')).toHaveTextContent( + "Couldn't check this change in advance. Applying it checks it again." + ) + ); + expect(screen.getByRole('button', {name: 'Update monitor'})).toBeEnabled(); + }); + + it('checks the change again when Seer changes it', async () => { + const dryRun = mockDryRun(); + const actionRequest = InvestigationActionRequestFixture(); + const {rerender} = render( + + ); + await waitFor(() => expect(dryRun).toHaveBeenCalledTimes(1)); + + rerender( + + ); + + await waitFor(() => expect(dryRun).toHaveBeenCalledTimes(2)); + }); + + it('waits for the new check instead of trusting the old one', async () => { + mockDryRun(); + const actionRequest = InvestigationActionRequestFixture(); + const {rerender} = render( + + ); + await findApplyButton(); + + MockApiClient.clearMockResponses(); + mockDryRun({asyncDelay: 50}); + rerender( + + ); + + expect(screen.getByRole('button', {name: 'Update monitor'})).toBeDisabled(); + await findApplyButton(); + }); + + it('does not check a change the viewer cannot apply', () => { + const dryRun = mockDryRun(); + render( + + ); + + expect(dryRun).not.toHaveBeenCalled(); + }); + + it('has no apply button for advice', () => { + render( + + ); + + expect( + screen.queryByRole('button', {name: 'Update monitor'}) + ).not.toBeInTheDocument(); + expect(getCard()).not.toHaveTextContent("Seer can't apply this kind of change yet."); + }); + + it('says why the check would reject a change', async () => { + mockDryRun({ + statusCode: 400, + body: { + detail: "The change wasn't valid.", + code: 'validation_failed', + messages: ['Invalid Time Window: 900', 'A valid number is required.'], + }, + }); + render( + + ); + + await waitFor(() => + expect(screen.getByRole('status')).toHaveTextContent( + 'Invalid Time Window: 900. A valid number is required.' + ) + ); + expect(screen.getByRole('button', {name: 'Update monitor'})).toBeDisabled(); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + + it('shows why an apply was rejected and lets the user retry', async () => { + const apply = mockApply({ + statusCode: 400, + body: { + detail: "The change wasn't valid.", + code: 'validation_failed', + messages: ['Invalid Time Window: 900'], + }, + }); + render( + + ); + + await userEvent.click(await findApplyButton()); + + expect(await screen.findByRole('alert')).toHaveTextContent( + 'This change could not be applied. Invalid Time Window: 900.' + ); + expect(apply).toHaveBeenCalledTimes(1); + expect(screen.getByRole('button', {name: 'Update monitor'})).toBeEnabled(); + expect(screen.queryByText("This change wasn't valid.")).not.toBeInTheDocument(); + }); + + it('renders the applied change with who applied it', () => { + render( + + ); + + const card = getCard(); + expect(card).toHaveTextContent( + 'Monitor updated. Cart p95 latency was changed to the settings below.' + ); + expect(card).toHaveTextContent(/Applied by Grace Hopper •/); + expect(card).not.toHaveTextContent('Widen the interval.'); + expect(within(card).getByText('Applied')).toBeInTheDocument(); + expect(within(card).getByText('15 minutes')).toBeInTheDocument(); + expect(within(card).getByText('was 5 minutes')).toBeInTheDocument(); + expect(within(card).getByRole('button', {name: 'View monitor'})).toHaveAttribute( + 'href', + '/organizations/org-slug/monitors/789/' + ); + expect( + within(card).queryByRole('button', {name: 'Update monitor'}) + ).not.toBeInTheDocument(); + expect(card).not.toHaveTextContent('This can no longer be applied.'); + }); + + it('says when the viewer applied the change', () => { + render( + + ); + + expect(getCard()).toHaveTextContent('Applied by you'); + expect(getCard()).not.toHaveTextContent('Ada Lovelace'); + }); + + it('says a change was applied when nobody is named', () => { + render( + + ); + + const card = getCard(); + expect(card).toHaveTextContent('Applied 5 minutes ago'); + expect(card).not.toHaveTextContent('Applied by'); + }); + + it('keeps the title of an applied change of a kind it does not know', () => { + render( + + ); + + const card = getCard(); + expect(card).toHaveTextContent('Connect Datadog. Seer needs traces.'); + expect(within(card).getByText('Applied')).toBeInTheDocument(); + }); + + it('names the monitor as it was when the change was applied', () => { + render( + + ); + + const card = getCard(); + expect(card).toHaveTextContent( + 'Monitor updated. Cart latency (renamed) was changed to the settings below.' + ); + expect(within(card).queryByText('Cart p95 latency')).not.toBeInTheDocument(); + }); + + it('reports an already applied request', async () => { + mockApply({ + statusCode: 409, + body: {detail: 'This action was already applied.', code: 'already_applied'}, + }); + render( + + ); + + await userEvent.click(await findApplyButton()); + + expect(await screen.findByText('This was already applied.')).toBeInTheDocument(); + }); }); diff --git a/static/app/views/investigations/actionRequests/nextStepCard.stories.tsx b/static/app/views/investigations/actionRequests/nextStepCard.stories.tsx index f44e136966fd..b36c4495a05a 100644 --- a/static/app/views/investigations/actionRequests/nextStepCard.stories.tsx +++ b/static/app/views/investigations/actionRequests/nextStepCard.stories.tsx @@ -1,12 +1,18 @@ -import {Fragment} from 'react'; +import {Fragment, type ComponentProps} from 'react'; +import {useQuery} from '@tanstack/react-query'; import {Container} from '@sentry/scraps/layout'; import {Text} from '@sentry/scraps/text'; import * as Storybook from 'sentry/stories'; +import {useOrganization} from 'sentry/utils/useOrganization'; +import {commentStoryAuthors} from 'sentry/views/investigations/__stories__/commentStoryFixtures'; +import {InvestigationFixtureApi} from 'sentry/views/investigations/__stories__/investigationFixtureApi'; import {NextStepCard} from 'sentry/views/investigations/actionRequests/nextStepCard'; +import {getInvestigationDetailQueryOptions} from 'sentry/views/investigations/api'; import { InvestigationActionRequestFixture, + InvestigationDetailFixture, InvestigationOrchestrationFixture, } from 'sentry/views/investigations/fixtures'; import type { @@ -38,17 +44,51 @@ function runWith( }); } +function investigationWith(id: string, actionRequests: InvestigationActionRequest[]) { + return InvestigationDetailFixture({id, actionRequests}); +} + function CardStory({ actionRequests, + dryRunErrors, + id, orchestration = completedRun, }: { actionRequests: InvestigationActionRequest[]; + id: string; + dryRunErrors?: ComponentProps['dryRunErrors']; orchestration?: InvestigationOrchestration; }) { return ( - - - + + + + + + ); +} + +function FixtureCard({ + investigationId, + orchestration, +}: { + investigationId: string; + orchestration: InvestigationOrchestration; +}) { + const organization = useOrganization(); + const {data: investigation} = useQuery( + getInvestigationDetailQueryOptions(organization.slug, investigationId) + ); + return ( + ); } @@ -58,12 +98,38 @@ export default Storybook.story('Investigations — Recommended Action', story => Seer's primary request from the latest report sits under the summary. - + )); + story('Cannot apply', () => ( + + )); + + story('Check failed', () => ( + + )); + story('Threshold change', () => ( story('Query change', () => ( Advice Seer cannot carry out has no action, so the card shows the advice only. )); + story('Applied', () => ( + + )); + story('Withdrawn', () => ( story('Generating', () => ( )); story('Failed', () => ( - + )); }); diff --git a/static/app/views/investigations/actionRequests/nextStepCard.tsx b/static/app/views/investigations/actionRequests/nextStepCard.tsx index 5405b27e5b3b..c586f004ed0b 100644 --- a/static/app/views/investigations/actionRequests/nextStepCard.tsx +++ b/static/app/views/investigations/actionRequests/nextStepCard.tsx @@ -14,6 +14,7 @@ import type { type NextStepCardProps = { actionRequests: InvestigationActionRequest[] | undefined; + investigationId: string; orchestration: InvestigationOrchestration | undefined; /** The action requests predate the run's current next step. */ actionRequestsStale?: boolean; @@ -24,6 +25,7 @@ type NextStepCardProps = { export function NextStepCard({ actionRequests, actionRequestsStale = false, + investigationId, orchestration, orchestrationLoading = false, source, @@ -44,6 +46,7 @@ export function NextStepCard({ ); @@ -69,7 +72,10 @@ export function getPrimaryActionRequest( Math.max(...actionRequests.map(actionRequest => actionRequest.reportRevision)); return actionRequests .filter( - actionRequest => actionRequest.primary && actionRequest.reportRevision >= revision + actionRequest => + actionRequest.primary && + actionRequest.reportRevision >= revision && + actionRequest.state !== 'superseded' ) .toSorted( (a, b) => diff --git a/static/app/views/investigations/actionRequests/updateDetector.tsx b/static/app/views/investigations/actionRequests/updateDetector.tsx new file mode 100644 index 000000000000..21353bd06cd6 --- /dev/null +++ b/static/app/views/investigations/actionRequests/updateDetector.tsx @@ -0,0 +1,123 @@ +import {Fragment} from 'react'; +import {IconSiren} from '@sentry/icons/siren'; +import {z} from 'zod'; + +import {LinkButton} from '@sentry/scraps/button'; +import {Text} from '@sentry/scraps/text'; + +import {t} from 'sentry/locale'; +import {useOrganization} from 'sentry/utils/useOrganization'; +import {makeMonitorDetailsPathname} from 'sentry/views/detectors/pathnames'; +import type { + ActionKindApplyProps, + ActionKindDetailsProps, + ActionKindUI, +} from 'sentry/views/investigations/actionRequests/actionKinds'; +import {ApplyButton} from 'sentry/views/investigations/actionRequests/applyButton'; +import {MonitorChanges} from 'sentry/views/investigations/actionRequests/monitorChanges'; +import {getSourceMonitor, type SourceMonitor} from 'sentry/views/investigations/source'; +import type { + InvestigationActionRequest, + InvestigationUpdateDetectorAction, +} from 'sentry/views/investigations/types'; + +export const updateDetectorUI: ActionKindUI = { + Details: UpdateDetectorDetails, + Apply: UpdateDetectorApply, + checksBeforeApply: true, + getAppliedSummary: ({action, actionRequest, source}) => ( + + {t('Monitor updated.')}{' '} + {t( + '%s was changed to the settings below.', + getMonitorName(actionRequest, getSourceMonitor(source, action.detectorId)) ?? + t('The monitor') + )} + + ), + getCannotApplyMessage: reason => { + switch (reason) { + case 'permission_denied': + return t("You don't have permission to edit this monitor."); + case 'detector_not_found': + return t('This monitor is no longer available.'); + case 'not_target_detector': + return t('This change targets a different monitor than this investigation.'); + case 'dynamic_detector': + return t("Seer can't change dynamic monitors yet."); + case 'stale': + return t('The monitor changed since Seer suggested this.'); + default: + return null; + } + }, + getSuccessMessage: () => t('Monitor updated.'), +}; + +function UpdateDetectorDetails({ + action, + actionRequest, + source, +}: ActionKindDetailsProps) { + const monitor = getSourceMonitor(source, action.detectorId); + return ( + + ); +} + +function UpdateDetectorApply({ + action, + actionRequest, + applying, + check, + message, + onApply, +}: ActionKindApplyProps) { + const organization = useOrganization(); + switch (actionRequest.state) { + case 'suggested': + return ( + } + label={t('Update monitor')} + busyLabel={t('Updating monitor…')} + /> + ); + case 'applied': + return ( + } + to={makeMonitorDetailsPathname(organization.slug, action.detectorId)} + > + {t('View monitor')} + + ); + default: + return null; + } +} + +const appliedDetailsSchema = z.object({detectorName: z.string()}); + +function getMonitorName( + actionRequest: InvestigationActionRequest, + monitor: SourceMonitor | null +) { + return ( + appliedDetailsSchema.safeParse(actionRequest.appliedDetails).data?.detectorName ?? + monitor?.name ?? + null + ); +} diff --git a/static/app/views/investigations/api.ts b/static/app/views/investigations/api.ts index dcbee2f6cb1f..8380e8dc8968 100644 --- a/static/app/views/investigations/api.ts +++ b/static/app/views/investigations/api.ts @@ -1,14 +1,19 @@ import { + queryOptions, useMutation, useQueryClient, type UseMutationOptions, } from '@tanstack/react-query'; +import {z} from 'zod'; -import type {ApiResponse} from 'sentry/utils/api/apiFetch'; +import {apiFetch, type ApiResponse} from 'sentry/utils/api/apiFetch'; import {apiOptions} from 'sentry/utils/api/apiOptions'; import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {fetchMutation} from 'sentry/utils/queryClient'; +import {RequestError} from 'sentry/utils/requestError/requestError'; import type { + InvestigationActionRequest, + InvestigationActionRequestApplyResponse, InvestigationCandidate, InvestigationBlock, InvestigationBlockExecutionStart, @@ -669,6 +674,153 @@ export function useRunInvestigationBlockMutation( }); } +const cannotApplyReasonSchema = z.enum([ + 'investigation_archived', + 'not_suggested', + 'unsupported_action', + 'no_access', + 'permission_denied', + 'not_target_detector', + 'detector_not_found', + 'dynamic_detector', +]); + +export type CannotApplyReason = z.infer; + +const actionRequestErrorSchema = z.discriminatedUnion('code', [ + z.object({ + code: z.enum([ + ...cannotApplyReasonSchema.options, + 'already_applied', + 'withdrawn', + 'stale', + 'action_changed', + ]), + }), + z.object({code: z.literal('validation_failed'), messages: z.array(z.string())}), +]); + +export type ActionRequestError = z.infer; + +export function parseApplyError(error: unknown): ActionRequestError | null { + const parsed = actionRequestErrorSchema.safeParse( + error instanceof RequestError ? error.responseJSON : undefined + ); + return parsed.success ? parsed.data : null; +} + +function getActionRequestApplyUrl( + organizationSlug: string, + investigationId: string, + actionRequestId: string +) { + return getApiUrl( + '/organizations/$organizationIdOrSlug/investigations/$investigationId/action-requests/$actionRequestId/apply/', + { + path: { + organizationIdOrSlug: organizationSlug, + investigationId, + actionRequestId, + }, + } + ); +} + +export function actionRequestDryRunQueryOptions( + organizationSlug: string, + investigationId: string, + actionRequest: InvestigationActionRequest +) { + const url = getActionRequestApplyUrl( + organizationSlug, + investigationId, + actionRequest.id + ); + const endpoint = { + method: 'POST', + data: {dryRun: true, action: actionRequest.action}, + } as const; + return queryOptions({ + queryKey: [url, endpoint] as const, + queryFn: async (): Promise => { + try { + await apiFetch({ + queryKey: [url, endpoint], + }); + return null; + } catch (error) { + const applyError = parseApplyError(error); + if (applyError) { + return applyError; + } + throw error; + } + }, + staleTime: 0, + retry: false, + }); +} + +export function useApplyInvestigationActionRequestMutation( + organizationSlug: string, + investigationId: string, + options?: MutationOptions< + InvestigationActionRequestApplyResponse, + InvestigationActionRequest + > +) { + const queryClient = useQueryClient(); + const detailOptions = getInvestigationDetailQueryOptions( + organizationSlug, + investigationId + ); + const orchestrationOptions = investigationOrchestrationQueryOptions( + organizationSlug, + investigationId + ); + + return useMutation({ + ...options, + mutationFn: actionRequest => + fetchMutation({ + url: getActionRequestApplyUrl( + organizationSlug, + investigationId, + actionRequest.id + ), + method: 'POST', + data: {action: actionRequest.action}, + }), + onSuccess: async (response, variables, onMutateResult, context) => { + await queryClient.cancelQueries({queryKey: detailOptions.queryKey}); + queryClient.setQueryData(detailOptions.queryKey, current => + current + ? { + ...current, + json: { + ...current.json, + actionRequests: current.json.actionRequests?.map(actionRequest => + actionRequest.id === response.actionRequest.id + ? response.actionRequest + : actionRequest + ), + }, + } + : current + ); + void queryClient.invalidateQueries({queryKey: orchestrationOptions.queryKey}); + await options?.onSuccess?.(response, variables, onMutateResult, context); + }, + onError: async (error, variables, onMutateResult, context) => { + await Promise.all([ + queryClient.invalidateQueries({queryKey: detailOptions.queryKey}), + queryClient.invalidateQueries({queryKey: orchestrationOptions.queryKey}), + ]); + await options?.onError?.(error, variables, onMutateResult, context); + }, + }); +} + export function useUpdateInvestigationBlockPromptMutation( organizationSlug: string, investigationId: string diff --git a/static/app/views/investigations/detail/index.spec.tsx b/static/app/views/investigations/detail/index.spec.tsx index 67825cdc99ba..eb6c857af7e0 100644 --- a/static/app/views/investigations/detail/index.spec.tsx +++ b/static/app/views/investigations/detail/index.spec.tsx @@ -134,6 +134,13 @@ describe('Investigation detail', () => { body: {viewers: [], total: 0, heartbeatIntervalMs: 5000}, }); MockApiClient.addMockResponse({url: `${detailUrl}comments/`, body: []}); + for (const id of ['312', '320']) { + MockApiClient.addMockResponse({ + url: `${detailUrl}action-requests/${id}/apply/`, + method: 'POST', + body: {actionRequest: InvestigationActionRequestFixture({id})}, + }); + } }); it('loads and renders the complete investigation response', async () => { diff --git a/static/app/views/investigations/detail/index.tsx b/static/app/views/investigations/detail/index.tsx index 285edb8535e6..3fa29c9334ad 100644 --- a/static/app/views/investigations/detail/index.tsx +++ b/static/app/views/investigations/detail/index.tsx @@ -574,6 +574,7 @@ function InvestigationPageContent({investigation}: {investigation: Investigation | null; + canApply: boolean; + cannotApplyReason: InvestigationOrchestrationOpenString | null; dateApplied: string | null; dateCreated: string; - error: Record | null; + error: ActionRequestError | null; id: string; impact: string | null; key: string; @@ -310,6 +316,10 @@ export type InvestigationActionRequest = { title: string; }; +export type InvestigationActionRequestApplyResponse = { + actionRequest: InvestigationActionRequest; +}; + type InvestigationOrchestrationError = { code: string; message: string; diff --git a/tests/sentry/investigations/endpoints/test_organization_investigation_action_request_apply.py b/tests/sentry/investigations/endpoints/test_organization_investigation_action_request_apply.py index eda046591cef..52e0e84f926f 100644 --- a/tests/sentry/investigations/endpoints/test_organization_investigation_action_request_apply.py +++ b/tests/sentry/investigations/endpoints/test_organization_investigation_action_request_apply.py @@ -323,6 +323,32 @@ def test_a_dry_run_runs_the_same_checks(self) -> None: assert response.status_code == 409 assert response.data["code"] == "not_suggested" + def test_applies_the_action_the_user_saw(self) -> None: + action_request = self.suggest() + assert action_request.action_payload is not None + + response = self.apply(action_request, action=action_request.action_payload) + + assert response.status_code == 200, response.data + assert self.snuba_query().time_window == 900 + + def test_an_action_seer_changed_since_the_user_saw_it_conflicts(self) -> None: + action_request = self.suggest() + assert action_request.action_payload is not None + seen = {**action_request.action_payload, "changes": {"timeWindowSeconds": 600}} + + for data in ({"action": seen}, {"action": seen, "dryRun": True}): + response = self.apply(action_request, **data) + + assert response.status_code == 409 + assert response.data == { + "detail": "Seer changed this suggestion. Check the new one.", + "code": "action_changed", + } + action_request.refresh_from_db() + assert action_request.state == InvestigationActionRequestState.SUGGESTED + assert self.snuba_query().time_window == 300 + def test_rejects_an_unknown_field(self) -> None: response = self.apply(self.suggest(), dryRn=True)