diff --git a/static/app/components/events/autofix/autofixActionSelector.tsx b/static/app/components/events/autofix/autofixActionSelector.tsx index 6432bdc9a8627b..7342234f05305f 100644 --- a/static/app/components/events/autofix/autofixActionSelector.tsx +++ b/static/app/components/events/autofix/autofixActionSelector.tsx @@ -83,6 +83,7 @@ function AutofixActionSelector({ const Container = styled('div')` min-height: 40px; + padding: 0 ${space(1)}; `; const ContentWrapper = styled('div')` diff --git a/static/app/components/events/autofix/autofixChanges.tsx b/static/app/components/events/autofix/autofixChanges.tsx index 014e76a2a5c8cf..4050f5b8447e8e 100644 --- a/static/app/components/events/autofix/autofixChanges.tsx +++ b/static/app/components/events/autofix/autofixChanges.tsx @@ -1,21 +1,29 @@ -import {Fragment} from 'react'; +import React, {Fragment, useEffect, useState} from 'react'; import styled from '@emotion/styled'; import {AnimatePresence, type AnimationProps, motion} from 'framer-motion'; import ClippedBox from 'sentry/components/clippedBox'; import {AutofixDiff} from 'sentry/components/events/autofix/autofixDiff'; -import type { - AutofixChangesStep, - AutofixCodebaseChange, +import {SetupAndCreatePRsButton} from 'sentry/components/events/autofix/autofixPrButton'; +import {AutofixViewPrButton} from 'sentry/components/events/autofix/autofixViewPrButton'; +import { + type AutofixChangesStep, + type AutofixCodebaseChange, + AutofixStatus, } from 'sentry/components/events/autofix/types'; import {useAutofixData} from 'sentry/components/events/autofix/useAutofix'; -import {IconFix} from 'sentry/icons'; +import LoadingIndicator from 'sentry/components/loadingIndicator'; +import {IconChevron, IconFix} from 'sentry/icons'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import testableTransition from 'sentry/utils/testableTransition'; +import usePrevious from 'sentry/utils/usePrevious'; type AutofixChangesProps = { + changesVersionIndex: number; groupId: string; + hasMoreThanOneChangesStep: boolean; + hasStepBelow: boolean; runId: string; step: AutofixChangesStep; }; @@ -30,12 +38,19 @@ function AutofixRepoChange({ runId: string; }) { return ( - +
{change.title} {change.repo_name}
+ {change.pull_request && ( + + )} - + ); } @@ -69,12 +84,29 @@ const cardAnimationProps: AnimationProps = { }), }; -export function AutofixChanges({step, groupId, runId}: AutofixChangesProps) { +export function AutofixChanges({ + step, + groupId, + runId, + hasStepBelow, + changesVersionIndex, + hasMoreThanOneChangesStep, +}: AutofixChangesProps) { const data = useAutofixData({groupId}); + const [isExpanded, setIsExpanded] = useState(!hasStepBelow); + + const previousHasStepBelow = usePrevious(hasStepBelow); + + useEffect(() => { + // When a new step shows up below this one, we want to auto collapse it. + if (previousHasStepBelow && !hasStepBelow) { + setIsExpanded(false); + } + }, [previousHasStepBelow, hasStepBelow]); if (step.status === 'ERROR' || data?.status === 'ERROR') { return ( - + {data?.error_message ? ( @@ -85,38 +117,93 @@ export function AutofixChanges({step, groupId, runId}: AutofixChangesProps) { {t('Something went wrong.')} )} - + ); } - if (!step.changes.length) { + if ( + step.status === AutofixStatus.COMPLETED && + Object.keys(step.codebase_changes).length === 0 + ) { return ( - + {t('Could not find a fix.')} - + ); } - const allChangesHavePullRequests = step.changes.every(change => change.pull_request); + const allChangesHavePullRequests = Object.values(step.codebase_changes).every( + change => change.pull_request + ); + + const changesText = hasMoreThanOneChangesStep + ? hasStepBelow + ? t('Changes (version %s)', changesVersionIndex + 1) + : t('Latest Changes (version %s)', changesVersionIndex + 1) + : t('Changes'); + + const changesAreReady = Object.values(step.codebase_changes).every( + change => change.diff_str + ); return ( - - - - - {t('Fixes')} - - {step.changes.map((change, i) => ( - - {i > 0 && } - - - ))} - + + hasStepBelow && setIsExpanded(!isExpanded)} + role={hasStepBelow ? 'button' : undefined} + aria-expanded={hasStepBelow ? isExpanded : undefined} + expandable={hasStepBelow} + > + + + + {changesText} + + + + {changesAreReady ? ( + + {(!hasStepBelow || isExpanded) && !allChangesHavePullRequests && ( + + )} + {hasStepBelow && ( + + + + )} + + ) : ( + + )} + + + {(!hasStepBelow || isExpanded) && changesAreReady && ( + + {Object.values(step.codebase_changes).map((change, i) => ( + + {i > 0 && } + + + ))} + + )} @@ -136,21 +223,24 @@ const AnimationWrapper = styled(motion.div)` const PrefixText = styled('span')``; -const ChangesContainer = styled('div')<{allChangesHavePullRequests: boolean}>` - border: 2px solid +const ChangesContainer = styled('div')<{ + allChangesHavePullRequests: boolean; + hasStepBelow?: boolean; +}>` + border: ${p => (p.hasStepBelow ? 1 : 2)}px solid ${p => - p.allChangesHavePullRequests - ? p.theme.alert.success.border - : p.theme.alert.info.border}; + p.hasStepBelow + ? p.theme.innerBorder + : p.allChangesHavePullRequests + ? p.theme.alert.success.border + : p.theme.alert.info.border}; border-radius: ${p => p.theme.borderRadius}; box-shadow: ${p => p.theme.dropShadowMedium}; - padding-left: ${space(2)}; - padding-right: ${space(2)}; - padding-top: ${space(1)}; + overflow: hidden; `; -const Content = styled('div')` - padding: 0 ${space(1)} ${space(1)} ${space(1)}; +const RepoChangeContent = styled('div')` + padding: 0 ${space(2)} ${space(2)} ${space(2)}; `; const Title = styled('div')` @@ -164,9 +254,10 @@ const PullRequestTitle = styled('div')` const RepoChangesHeader = styled('div')` padding: ${space(2)} 0; - display: grid; - align-items: center; - grid-template-columns: 1fr auto; + display: flex; + align-items: flex-start; + justify-content: space-between; + gap: ${space(2)}; `; const Separator = styled('hr')` @@ -175,10 +266,61 @@ const Separator = styled('hr')` margin: ${space(2)} -${space(2)} 0 -${space(2)}; `; -const HeaderText = styled('div')` +const HeaderRow = styled('div')<{expandable?: boolean}>` + display: flex; + align-items: center; + width: 100%; + justify-content: space-between; + gap: ${space(1)}; + height: calc((2 * ${space(2)}) + ${p => p.theme.form.sm.height}px); + padding: ${space(2)}; + cursor: ${p => (p.expandable ? 'pointer' : 'default')}; + + &:hover { + background-color: ${p => + p.expandable ? p.theme.backgroundSecondary : 'transparent'}; + } +`; + +const HeaderButtonsWrapper = styled('div')` + display: flex; + align-items: center; + gap: ${space(2)}; +`; + +const StyledLoadingIndicator = styled(LoadingIndicator)` + && { + margin: 0; + height: 16px; + width: 16px; + } +`; + +const HeaderText = styled('div')<{isPreviousChanges?: boolean}>` font-weight: bold; - font-size: 1.2em; + font-size: ${p => (p.isPreviousChanges ? 1.1 : 1.2)}em; + color: ${p => (p.isPreviousChanges ? p.theme.subText : p.theme.textColor)}; display: flex; align-items: center; gap: ${space(1)}; `; + +const HeaderTextWrapper = styled('div')` + display: flex; + align-items: center; + gap: ${space(1)}; +`; + +const CollapseButton = styled('button')` + background: none; + border: none; + cursor: pointer; + padding: 0; + color: ${p => p.theme.subText}; + display: flex; + align-items: center; + + &:hover { + color: ${p => p.theme.textColor}; + } +`; diff --git a/static/app/components/events/autofix/autofixInsightCards.tsx b/static/app/components/events/autofix/autofixInsightCards.tsx index 1d185dd5d314ff..892bd3f0d09747 100644 --- a/static/app/components/events/autofix/autofixInsightCards.tsx +++ b/static/app/components/events/autofix/autofixInsightCards.tsx @@ -412,6 +412,39 @@ export function useUpdateInsightCard({groupId, runId}: {groupId: string; runId: }); } +export function useSendFeedbackOnChanges({ + groupId, + runId, +}: { + groupId: string; + runId: string; +}) { + const api = useApi({persistInFlight: true}); + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: (params: {message: string}) => { + return api.requestPromise(`/issues/${groupId}/autofix/update/`, { + method: 'POST', + data: { + run_id: runId, + payload: { + type: 'continue_with_feedback', + message: params.message, + }, + }, + }); + }, + onSuccess: _ => { + queryClient.invalidateQueries({queryKey: makeAutofixQueryKey(groupId)}); + addSuccessMessage(t('Thanks, rethinking this...')); + }, + onError: () => { + addErrorMessage(t('Something went wrong when sending Autofix your message.')); + }, + }); +} + function ChainLink({ groupId, runId, diff --git a/static/app/components/events/autofix/autofixMessageBox.tsx b/static/app/components/events/autofix/autofixMessageBox.tsx index fe8106687d5ba8..9dd7bf4a283b2e 100644 --- a/static/app/components/events/autofix/autofixMessageBox.tsx +++ b/static/app/components/events/autofix/autofixMessageBox.tsx @@ -3,22 +3,16 @@ import styled from '@emotion/styled'; import {AnimatePresence, type AnimationProps, motion} from 'framer-motion'; import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator'; -import {openModal} from 'sentry/actionCreators/modal'; import {Button, LinkButton} from 'sentry/components/button'; import AutofixActionSelector from 'sentry/components/events/autofix/autofixActionSelector'; import AutofixFeedback from 'sentry/components/events/autofix/autofixFeedback'; -import {AutofixSetupWriteAccessModal} from 'sentry/components/events/autofix/autofixSetupWriteAccessModal'; +import {SetupAndCreatePRsButton} from 'sentry/components/events/autofix/autofixPrButton'; import { - type AutofixCodebaseChange, AutofixStatus, type AutofixStep, AutofixStepType, } from 'sentry/components/events/autofix/types'; -import { - makeAutofixQueryKey, - useAutofixData, -} from 'sentry/components/events/autofix/useAutofix'; -import {useAutofixSetup} from 'sentry/components/events/autofix/useAutofixSetup'; +import {makeAutofixQueryKey} from 'sentry/components/events/autofix/useAutofix'; import Input from 'sentry/components/input'; import LoadingIndicator from 'sentry/components/loadingIndicator'; import {ScrollCarousel} from 'sentry/components/scrollCarousel'; @@ -81,102 +75,6 @@ interface AutofixMessageBoxProps { scrollText?: string; } -function CreatePRsButton({ - changes, - groupId, -}: { - changes: AutofixCodebaseChange[]; - groupId: string; -}) { - const autofixData = useAutofixData({groupId}); - const api = useApi(); - const queryClient = useQueryClient(); - const [hasClickedCreatePr, setHasClickedCreatePr] = useState(false); - - const createPRs = () => { - setHasClickedCreatePr(true); - for (const change of changes) { - createPr({change}); - } - }; - - const {mutate: createPr} = useMutation({ - mutationFn: ({change}: {change: AutofixCodebaseChange}) => { - return api.requestPromise(`/issues/${groupId}/autofix/update/`, { - method: 'POST', - data: { - run_id: autofixData?.run_id, - payload: { - type: 'create_pr', - repo_external_id: change.repo_external_id, - repo_id: change.repo_id, // The repo_id is only here for temporary backwards compatibility for LA customers, and we should remove it soon. - }, - }, - }); - }, - onSuccess: () => { - addSuccessMessage(t('Created pull requests.')); - queryClient.invalidateQueries({queryKey: makeAutofixQueryKey(groupId)}); - }, - onError: () => { - setHasClickedCreatePr(false); - addErrorMessage(t('Failed to create a pull request')); - }, - }); - - return ( - - ); -} - -function SetupAndCreatePRsButton({ - changes, - groupId, -}: { - changes: AutofixCodebaseChange[]; - groupId: string; -}) { - const {data: setupData} = useAutofixSetup({groupId, checkWriteAccess: true}); - - if ( - !changes.every( - change => - setupData?.githubWriteIntegration?.repos?.find( - repo => `${repo.owner}/${repo.name}` === change.repo_name - )?.ok - ) - ) { - return ( - - ); - } - - return ; -} - interface RootCauseAndFeedbackInputAreaProps { actionText: string; changesMode: 'give_feedback' | 'add_tests' | 'create_prs' | null; @@ -275,10 +173,10 @@ function RootCauseAndFeedbackInputArea({ function StepIcon({step}: {step: AutofixStep}) { if (step.type === AutofixStepType.CHANGES) { - if (step.changes?.length === 0) { + if (Object.keys(step.codebase_changes).length === 0) { return ; } - if (step.changes.every(change => change.pull_request)) { + if (Object.values(step.codebase_changes).every(change => change.pull_request)) { return ; } return null; @@ -343,7 +241,9 @@ function AutofixMessageBox({ >(null); const changes = - isChangesStep && step?.type === AutofixStepType.CHANGES ? step.changes : []; + isChangesStep && step?.type === AutofixStepType.CHANGES + ? Object.values(step.codebase_changes) + : []; const prsMade = step?.status === AutofixStatus.COMPLETED && changes.length >= 1 && @@ -509,9 +409,13 @@ function AutofixMessageBox({ Draft {changes.length} pull request - {changes.length > 1 ? 's' : ''} for the above changes? + {changes.length > 1 ? 's' : ''} for the latest changes? - + )} @@ -564,7 +468,7 @@ const Placeholder = styled('div')` const ViewPRButtons = styled(ScrollCarousel)` width: 100%; - padding: 0 ${space(1)}; + padding: 0 ${space(1)} ${space(2)} ${space(1)}; `; const ScrollIntoViewButtonWrapper = styled('div')` diff --git a/static/app/components/events/autofix/autofixPrButton.tsx b/static/app/components/events/autofix/autofixPrButton.tsx new file mode 100644 index 00000000000000..f3236b4144e977 --- /dev/null +++ b/static/app/components/events/autofix/autofixPrButton.tsx @@ -0,0 +1,140 @@ +import {useState} from 'react'; +import styled from '@emotion/styled'; + +import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator'; +import {openModal} from 'sentry/actionCreators/modal'; +import {Button} from 'sentry/components/button'; +import {AutofixSetupWriteAccessModal} from 'sentry/components/events/autofix/autofixSetupWriteAccessModal'; +import type {AutofixCodebaseChange} from 'sentry/components/events/autofix/types'; +import { + makeAutofixQueryKey, + useAutofixData, +} from 'sentry/components/events/autofix/useAutofix'; +import {useAutofixSetup} from 'sentry/components/events/autofix/useAutofixSetup'; +import LoadingIndicator from 'sentry/components/loadingIndicator'; +import {t} from 'sentry/locale'; +import {useMutation, useQueryClient} from 'sentry/utils/queryClient'; +import useApi from 'sentry/utils/useApi'; + +function CreatePRsButton({ + changes, + groupId, + changesStepId, + isPrimary = true, +}: { + changes: AutofixCodebaseChange[]; + changesStepId: string; + groupId: string; + isPrimary?: boolean; +}) { + const autofixData = useAutofixData({groupId}); + const api = useApi(); + const queryClient = useQueryClient(); + const [hasClickedCreatePr, setHasClickedCreatePr] = useState(false); + + const createPRs = () => { + setHasClickedCreatePr(true); + for (const change of changes) { + createPr({change}); + } + }; + + const {mutate: createPr} = useMutation({ + mutationFn: ({change}: {change: AutofixCodebaseChange}) => { + return api.requestPromise(`/issues/${groupId}/autofix/update/`, { + method: 'POST', + data: { + run_id: autofixData?.run_id, + payload: { + type: 'create_pr', + repo_external_id: change.repo_external_id, + changes_step_id: changesStepId, + }, + }, + }); + }, + onSuccess: () => { + addSuccessMessage(t('Created pull requests.')); + queryClient.invalidateQueries({queryKey: makeAutofixQueryKey(groupId)}); + }, + onError: () => { + setHasClickedCreatePr(false); + addErrorMessage(t('Failed to create a pull request')); + }, + }); + + return ( + + ); +} + +export function SetupAndCreatePRsButton({ + changes, + groupId, + changesStepId, + hasStepBelow, + isPrimary = true, +}: { + changes: AutofixCodebaseChange[]; + changesStepId: string; + groupId: string; + hasStepBelow?: boolean; + isPrimary?: boolean; +}) { + const {data: setupData} = useAutofixSetup({groupId, checkWriteAccess: true}); + + const areAllReposWriteable = changes.every( + change => + setupData?.githubWriteIntegration?.repos?.find( + repo => `${repo.owner}/${repo.name}` === change.repo_name + )?.ok + ); + + if (!areAllReposWriteable) { + return ( + + ); + } + + return ( + + ); +} + +const ProcessingStatusIndicator = styled(LoadingIndicator)` + && { + margin: 0; + height: 14px; + width: 14px; + } +`; diff --git a/static/app/components/events/autofix/autofixSteps.tsx b/static/app/components/events/autofix/autofixSteps.tsx index d6e7d10b3b389f..2dd2ddf947276e 100644 --- a/static/app/components/events/autofix/autofixSteps.tsx +++ b/static/app/components/events/autofix/autofixSteps.tsx @@ -4,7 +4,7 @@ import {AnimatePresence, type AnimationProps, motion} from 'framer-motion'; import {AutofixChanges} from 'sentry/components/events/autofix/autofixChanges'; import AutofixInsightCards, { - useUpdateInsightCard, + useSendFeedbackOnChanges, } from 'sentry/components/events/autofix/autofixInsightCards'; import AutofixMessageBox from 'sentry/components/events/autofix/autofixMessageBox'; import {AutofixOutputStream} from 'sentry/components/events/autofix/autofixOutputStream'; @@ -16,6 +16,7 @@ import { type AutofixData, type AutofixProgressItem, type AutofixRepository, + AutofixStatus, type AutofixStep, AutofixStepType, } from 'sentry/components/events/autofix/types'; @@ -29,8 +30,10 @@ const animationProps: AnimationProps = { transition: testableTransition({duration: 0.3}), }; interface StepProps { + changesVersionIndex: number; groupId: string; hasErroredStepBefore: boolean; + hasMoreThanOneChangesStep: boolean; hasStepAbove: boolean; hasStepBelow: boolean; repos: AutofixRepository[]; @@ -70,6 +73,8 @@ export function Step({ hasStepAbove, hasErroredStepBefore, shouldHighlightRethink, + changesVersionIndex, + hasMoreThanOneChangesStep, }: StepProps) { return ( @@ -107,7 +112,14 @@ export function Step({ /> )} {step.type === AutofixStepType.CHANGES && ( - + )} @@ -146,15 +158,9 @@ export function AutofixSteps({data, groupId, runId}: AutofixStepsProps) { } }; - const {mutate: sendFeedbackOnChanges} = useUpdateInsightCard({groupId, runId}); + const {mutate: sendFeedbackOnChanges} = useSendFeedbackOnChanges({groupId, runId}); const iterateOnChangesStep = (text: string) => { - const planStep = steps?.[steps.length - 2]; - if (!planStep || planStep.type !== AutofixStepType.DEFAULT) { - return; - } sendFeedbackOnChanges({ - step_index: planStep.index, - retain_insight_card_index: planStep.insights.length - 1, message: text, }); }; @@ -223,15 +229,21 @@ export function AutofixSteps({data, groupId, runId}: AutofixStepsProps) { const isRootCauseSelectionStep = lastStep.type === AutofixStepType.ROOT_CAUSE_ANALYSIS && - lastStep.status === 'COMPLETED'; + lastStep.status === AutofixStatus.COMPLETED; const isChangesStep = - lastStep.type === AutofixStepType.CHANGES && lastStep.status === 'COMPLETED'; + lastStep.type === AutofixStepType.CHANGES && + lastStep.status === AutofixStatus.COMPLETED; + + const changesStepIds = steps + .filter(step => step.type === AutofixStepType.CHANGES) + .map(step => step.id); return (
{steps.map((step, index) => { + // TODO: Refactor and clean this up, very hard to follow const previousStep = index > 0 ? steps[index - 1] : null; const previousStepErrored = previousStep !== null && @@ -254,12 +266,19 @@ export function AutofixSteps({data, groupId, runId}: AutofixStepsProps) { nextStep?.insights?.length === 0; const isNextStepLastStep = index === steps.length - 2; + + // The current index of the changes step within all changes steps, or -1 if not a changes step + const changesVersionIndex = changesStepIds.indexOf(step.id); + const shouldHighlightRethink = (nextStep?.type === AutofixStepType.ROOT_CAUSE_ANALYSIS && isNextStepLastStep) || (nextStep?.type === AutofixStepType.CHANGES && - nextStep.changes.length > 0 && - !nextStep.changes.every(change => change.pull_request)); + Object.keys(nextStep.codebase_changes).length > 0 && + !Object.values(nextStep.codebase_changes).every( + change => change.pull_request + ) && + isNextStepLastStep); return (
(stepsRef.current[index] = el)} key={step.id}> @@ -277,6 +296,8 @@ export function AutofixSteps({data, groupId, runId}: AutofixStepsProps) { repos={repos} hasErroredStepBefore={previousStepErrored} shouldHighlightRethink={shouldHighlightRethink} + changesVersionIndex={changesVersionIndex} + hasMoreThanOneChangesStep={changesStepIds.length > 1} />
); @@ -285,11 +306,10 @@ export function AutofixSteps({data, groupId, runId}: AutofixStepsProps) { )} - } + href={prUrl} + external + > + View PR in {repoName} + + ); +} diff --git a/static/app/components/events/autofix/types.ts b/static/app/components/events/autofix/types.ts index 3426b284e04dec..dd48bc429cab5e 100644 --- a/static/app/components/events/autofix/types.ts +++ b/static/app/components/events/autofix/types.ts @@ -154,8 +154,8 @@ export type AutofixCodebaseChange = { repo_id?: number; // The repo_id is only here for temporary backwards compatibility for LA customers, and we should remove it soon. Use repo_external_id instead. }; -export interface AutofixChangesStep extends BaseStep { - changes: AutofixCodebaseChange[]; +export interface AutofixChangesStep extends Omit { + codebase_changes: Record; type: AutofixStepType.CHANGES; }