-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
ref(autofix): Move actions to document and remove message box (#83252)
(Note for issues team if you open this PR: this PR only contains changes inside the Sentry AI section of the drawer) I know this PR has a big change count, but it's really just one change: removing the message box. The user's actions are now on the root cause and fix cards, and logs and user input are in the output stream. It's a lot of moving existing code and tests, and deleting dead code. I also had to include some styling changes for this to all look coherent and readable, plus tacking on some other styling changes for other existing readability issues. Nothing has changed on functionality. (The insight card justifications will look better once the backend change goes in) <img width="694" alt="Screenshot 2025-01-10 at 8 18 52 AM" src="https://github.com/user-attachments/assets/70e01bc4-f508-45e1-880e-7f94a7bde67f" /> <img width="723" alt="Screenshot 2025-01-10 at 8 19 00 AM" src="https://github.com/user-attachments/assets/a96d362d-5ac9-4e77-b60f-65c30cbd2c91" /> <img width="675" alt="Screenshot 2025-01-10 at 8 49 31 AM" src="https://github.com/user-attachments/assets/423aec23-c0e5-44fd-9396-18b97d306074" /> <img width="662" alt="Screenshot 2025-01-10 at 11 42 49 AM" src="https://github.com/user-attachments/assets/ac230aae-cc94-4146-aa15-ee79ce8605ce" /> <img width="653" alt="Screenshot 2025-01-14 at 6 50 00 AM" src="https://github.com/user-attachments/assets/4cb3449c-6656-4663-9501-9212376af753" /> <img width="634" alt="Screenshot 2025-01-14 at 6 49 49 AM" src="https://github.com/user-attachments/assets/597c94e3-4833-45ad-8b3d-559d12029161" /> --------- Co-authored-by: Jenn Mueng <[email protected]>
- Loading branch information
Showing
22 changed files
with
2,960 additions
and
2,853 deletions.
There are no files selected for viewing
183 changes: 183 additions & 0 deletions
183
static/app/components/events/autofix/autofixChanges.analytics.spec.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,183 @@ | ||
import {AutofixCodebaseChangeData} from 'sentry-fixture/autofixCodebaseChangeData'; | ||
import {AutofixStepFixture} from 'sentry-fixture/autofixStep'; | ||
|
||
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; | ||
|
||
import {Button} from 'sentry/components/button'; | ||
import {AutofixChanges} from 'sentry/components/events/autofix/autofixChanges'; | ||
import { | ||
type AutofixChangesStep, | ||
AutofixStepType, | ||
} from 'sentry/components/events/autofix/types'; | ||
|
||
jest.mock('sentry/components/button', () => ({ | ||
Button: jest.fn(props => { | ||
// Forward the click handler while allowing us to inspect props | ||
return <button onClick={props.onClick}>{props.children}</button>; | ||
}), | ||
LinkButton: jest.fn(props => { | ||
return <a href={props.href}>{props.children}</a>; | ||
}), | ||
})); | ||
|
||
const mockButton = Button as jest.MockedFunction<typeof Button>; | ||
|
||
describe('AutofixChanges', () => { | ||
const defaultProps = { | ||
groupId: '123', | ||
runId: '456', | ||
step: AutofixStepFixture({ | ||
type: AutofixStepType.CHANGES, | ||
changes: [AutofixCodebaseChangeData()], | ||
}) as AutofixChangesStep, | ||
}; | ||
|
||
beforeEach(() => { | ||
MockApiClient.clearMockResponses(); | ||
mockButton.mockClear(); | ||
}); | ||
|
||
it('passes correct analytics props for Create PR button when write access is enabled', async () => { | ||
MockApiClient.addMockResponse({ | ||
url: '/issues/123/autofix/setup/?check_write_access=true', | ||
method: 'GET', | ||
body: { | ||
genAIConsent: {ok: true}, | ||
integration: {ok: true}, | ||
githubWriteIntegration: { | ||
repos: [{ok: true, owner: 'owner', name: 'hello-world', id: 100}], | ||
}, | ||
}, | ||
}); | ||
|
||
render(<AutofixChanges {...defaultProps} />); | ||
await userEvent.click(screen.getByRole('button', {name: 'Draft PR'})); | ||
|
||
const createPRButtonCall = mockButton.mock.calls.find( | ||
call => call[0]?.analyticsEventKey === 'autofix.create_pr_clicked' | ||
); | ||
expect(createPRButtonCall?.[0]).toEqual( | ||
expect.objectContaining({ | ||
analyticsEventKey: 'autofix.create_pr_clicked', | ||
analyticsEventName: 'Autofix: Create PR Clicked', | ||
analyticsParams: {group_id: '123'}, | ||
}) | ||
); | ||
}); | ||
|
||
it('passes correct analytics props for Create PR Setup button when write access is not enabled', () => { | ||
MockApiClient.addMockResponse({ | ||
url: '/issues/123/autofix/setup/?check_write_access=true', | ||
method: 'GET', | ||
body: { | ||
genAIConsent: {ok: true}, | ||
integration: {ok: true}, | ||
githubWriteIntegration: { | ||
repos: [{ok: false, owner: 'owner', name: 'hello-world', id: 100}], | ||
}, | ||
}, | ||
}); | ||
|
||
render(<AutofixChanges {...defaultProps} />); | ||
|
||
// Find the last call to Button that matches our Setup button | ||
const setupButtonCall = mockButton.mock.calls.find( | ||
call => call[0].children === 'Draft PR' | ||
); | ||
expect(setupButtonCall?.[0]).toEqual( | ||
expect.objectContaining({ | ||
analyticsEventKey: 'autofix.create_pr_setup_clicked', | ||
analyticsEventName: 'Autofix: Create PR Setup Clicked', | ||
analyticsParams: { | ||
group_id: '123', | ||
}, | ||
}) | ||
); | ||
}); | ||
|
||
it('passes correct analytics props for Create Branch button when write access is enabled', async () => { | ||
MockApiClient.addMockResponse({ | ||
url: '/issues/123/autofix/setup/?check_write_access=true', | ||
method: 'GET', | ||
body: { | ||
genAIConsent: {ok: true}, | ||
integration: {ok: true}, | ||
githubWriteIntegration: { | ||
repos: [{ok: true, owner: 'owner', name: 'hello-world', id: 100}], | ||
}, | ||
}, | ||
}); | ||
|
||
render(<AutofixChanges {...defaultProps} />); | ||
|
||
await userEvent.click(screen.getByRole('button', {name: 'Check Out Locally'})); | ||
|
||
const createBranchButtonCall = mockButton.mock.calls.find( | ||
call => call[0]?.analyticsEventKey === 'autofix.push_to_branch_clicked' | ||
); | ||
expect(createBranchButtonCall?.[0]).toEqual( | ||
expect.objectContaining({ | ||
analyticsEventKey: 'autofix.push_to_branch_clicked', | ||
analyticsEventName: 'Autofix: Push to Branch Clicked', | ||
analyticsParams: {group_id: '123'}, | ||
}) | ||
); | ||
}); | ||
|
||
it('passes correct analytics props for Create Branch Setup button when write access is not enabled', () => { | ||
MockApiClient.addMockResponse({ | ||
url: '/issues/123/autofix/setup/?check_write_access=true', | ||
method: 'GET', | ||
body: { | ||
genAIConsent: {ok: true}, | ||
integration: {ok: true}, | ||
githubWriteIntegration: { | ||
repos: [{ok: false, owner: 'owner', name: 'hello-world', id: 100}], | ||
}, | ||
}, | ||
}); | ||
|
||
render(<AutofixChanges {...defaultProps} />); | ||
|
||
const setupButtonCall = mockButton.mock.calls.find( | ||
call => call[0].children === 'Check Out Locally' | ||
); | ||
expect(setupButtonCall?.[0]).toEqual( | ||
expect.objectContaining({ | ||
analyticsEventKey: 'autofix.create_branch_setup_clicked', | ||
analyticsEventName: 'Autofix: Create Branch Setup Clicked', | ||
analyticsParams: { | ||
group_id: '123', | ||
}, | ||
}) | ||
); | ||
}); | ||
|
||
it('passes correct analytics props for Add Tests button', () => { | ||
MockApiClient.addMockResponse({ | ||
url: '/issues/123/autofix/setup/?check_write_access=true', | ||
method: 'GET', | ||
body: { | ||
genAIConsent: {ok: true}, | ||
integration: {ok: true}, | ||
githubWriteIntegration: { | ||
repos: [{ok: true, owner: 'owner', name: 'hello-world', id: 100}], | ||
}, | ||
}, | ||
}); | ||
|
||
render(<AutofixChanges {...defaultProps} />); | ||
screen.getByText('Add Tests').click(); | ||
|
||
const addTestsButtonCall = mockButton.mock.calls.find( | ||
call => call[0]?.analyticsEventKey === 'autofix.add_tests_clicked' | ||
); | ||
expect(addTestsButtonCall?.[0]).toEqual( | ||
expect.objectContaining({ | ||
analyticsEventKey: 'autofix.add_tests_clicked', | ||
analyticsEventName: 'Autofix: Add Tests Clicked', | ||
analyticsParams: {group_id: '123'}, | ||
}) | ||
); | ||
}); | ||
}); |
151 changes: 151 additions & 0 deletions
151
static/app/components/events/autofix/autofixChanges.spec.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,151 @@ | ||
import {AutofixCodebaseChangeData} from 'sentry-fixture/autofixCodebaseChangeData'; | ||
|
||
import {render, screen} from 'sentry-test/reactTestingLibrary'; | ||
|
||
import {AutofixChanges} from './autofixChanges'; | ||
import {AutofixStatus, AutofixStepType} from './types'; | ||
|
||
const mockUseAutofix = jest.fn(); | ||
jest.mock('sentry/components/events/autofix/useAutofix', () => ({ | ||
...jest.requireActual('sentry/components/events/autofix/useAutofix'), | ||
useAutofixData: () => mockUseAutofix(), | ||
})); | ||
|
||
const mockUseAutofixSetup = jest.fn(); | ||
jest.mock('sentry/components/events/autofix/useAutofixSetup', () => ({ | ||
useAutofixSetup: () => mockUseAutofixSetup(), | ||
})); | ||
|
||
const mockUpdateInsightCard = jest.fn(); | ||
jest.mock('sentry/components/events/autofix/autofixInsightCards', () => ({ | ||
useUpdateInsightCard: () => ({ | ||
mutate: mockUpdateInsightCard, | ||
}), | ||
})); | ||
|
||
describe('AutofixChanges', () => { | ||
const defaultProps = { | ||
groupId: '123', | ||
runId: 'run-123', | ||
step: { | ||
id: 'step-123', | ||
progress: [], | ||
title: 'Changes', | ||
type: AutofixStepType.CHANGES as const, | ||
index: 0, | ||
status: AutofixStatus.COMPLETED, | ||
changes: [AutofixCodebaseChangeData({pull_request: undefined})], | ||
}, | ||
}; | ||
|
||
beforeEach(() => { | ||
mockUseAutofix.mockReturnValue({ | ||
status: 'COMPLETED', | ||
steps: [ | ||
{ | ||
type: AutofixStepType.DEFAULT, | ||
index: 0, | ||
insights: [], | ||
}, | ||
], | ||
}); | ||
|
||
mockUseAutofixSetup.mockReturnValue({ | ||
data: { | ||
githubWriteIntegration: { | ||
repos: [ | ||
{ | ||
owner: 'getsentry', | ||
name: 'sentry', | ||
ok: true, | ||
}, | ||
], | ||
}, | ||
}, | ||
}); | ||
}); | ||
|
||
it('renders error state when step has error', () => { | ||
render( | ||
<AutofixChanges | ||
{...defaultProps} | ||
step={{ | ||
...defaultProps.step, | ||
status: AutofixStatus.ERROR, | ||
}} | ||
/> | ||
); | ||
|
||
expect(screen.getByText('Something went wrong.')).toBeInTheDocument(); | ||
}); | ||
|
||
it('renders empty state when no changes', () => { | ||
render( | ||
<AutofixChanges | ||
{...defaultProps} | ||
step={{ | ||
...defaultProps.step, | ||
changes: [], | ||
}} | ||
/> | ||
); | ||
|
||
expect(screen.getByText('Could not find a fix.')).toBeInTheDocument(); | ||
}); | ||
|
||
it('renders changes with action buttons', () => { | ||
render(<AutofixChanges {...defaultProps} />); | ||
|
||
expect(screen.getByText('Fixes')).toBeInTheDocument(); | ||
expect(screen.getByText('Add error handling')).toBeInTheDocument(); | ||
expect(screen.getByText('owner/hello-world')).toBeInTheDocument(); | ||
|
||
expect(screen.getByRole('button', {name: 'Add Tests'})).toBeInTheDocument(); | ||
expect(screen.getByRole('button', {name: 'Check Out Locally'})).toBeInTheDocument(); | ||
expect(screen.getByRole('button', {name: 'Draft PR'})).toBeInTheDocument(); | ||
}); | ||
|
||
it('shows PR links when PRs are created', () => { | ||
const changeWithPR = AutofixCodebaseChangeData({ | ||
pull_request: { | ||
pr_number: 123, | ||
pr_url: 'https://github.com/owner/hello-world/pull/123', | ||
}, | ||
}); | ||
|
||
render( | ||
<AutofixChanges | ||
{...defaultProps} | ||
step={{ | ||
...defaultProps.step, | ||
changes: [changeWithPR], | ||
}} | ||
/> | ||
); | ||
|
||
expect( | ||
screen.getByRole('button', {name: 'View PR in owner/hello-world'}) | ||
).toBeInTheDocument(); | ||
}); | ||
|
||
it('shows branch checkout buttons when branches are created', () => { | ||
const changeWithBranch = AutofixCodebaseChangeData({ | ||
branch_name: 'fix/issue-123', | ||
pull_request: undefined, | ||
}); | ||
|
||
render( | ||
<AutofixChanges | ||
{...defaultProps} | ||
step={{ | ||
...defaultProps.step, | ||
changes: [changeWithBranch], | ||
}} | ||
/> | ||
); | ||
|
||
expect( | ||
screen.getByRole('button', {name: 'Check out in owner/hello-world'}) | ||
).toBeInTheDocument(); | ||
}); | ||
}); |
Oops, something went wrong.