Skip to content

Commit 28bd01a

Browse files
malwilleymalwilley
authored
fix(aci): Selected project state should react to form changes (#112484)
This primarily fixes an issue where the selected project in other parts of the form were not updating correctly when the project was changed. The full project object was previously stored in a context at the top of the detector form, so other components could use things like the project icon, slug, etc. This PR simplifies things a bit by taking it out of the context and creating a new hook `useDetectorFormProject()` which takes the project ID from the form state and looks up the project in our cache.
1 parent b25a43a commit 28bd01a

File tree

16 files changed

+102
-92
lines changed

16 files changed

+102
-92
lines changed

‎static/app/components/workflowEngine/ui/formSection.tsx‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import {Disclosure} from '@sentry/scraps/disclosure';
44
import {Stack} from '@sentry/scraps/layout';
55
import {Heading, Text} from '@sentry/scraps/text';
66

7+
import {ErrorBoundary} from 'sentry/components/errorBoundary';
8+
79
type FormSectionProps = {
810
title: React.ReactNode;
911
children?: React.ReactNode;
@@ -24,30 +26,32 @@ export function FormSection({
2426
defaultExpanded = true,
2527
}: FormSectionProps) {
2628
return (
27-
<Disclosure
28-
as="section"
29-
size="md"
30-
role="region"
31-
defaultExpanded={defaultExpanded}
32-
className={className}
33-
>
34-
<Disclosure.Title trailingItems={trailingItems}>
35-
<Heading as="h3">
36-
{step ? `${step}. ` : ''}
37-
{title}
38-
</Heading>
39-
</Disclosure.Title>
40-
<Disclosure.Content>
41-
<Stack gap="lg">
42-
{description && (
43-
<FormSectionDescription as="p" variant="secondary">
44-
{description}
45-
</FormSectionDescription>
46-
)}
47-
<Stack gap="md">{children}</Stack>
48-
</Stack>
49-
</Disclosure.Content>
50-
</Disclosure>
29+
<ErrorBoundary mini>
30+
<Disclosure
31+
as="section"
32+
size="md"
33+
role="region"
34+
defaultExpanded={defaultExpanded}
35+
className={className}
36+
>
37+
<Disclosure.Title trailingItems={trailingItems}>
38+
<Heading as="h3">
39+
{step ? `${step}. ` : ''}
40+
{title}
41+
</Heading>
42+
</Disclosure.Title>
43+
<Disclosure.Content>
44+
<Stack gap="lg">
45+
{description && (
46+
<FormSectionDescription as="p" variant="secondary">
47+
{description}
48+
</FormSectionDescription>
49+
)}
50+
<Stack gap="md">{children}</Stack>
51+
</Stack>
52+
</Disclosure.Content>
53+
</Disclosure>
54+
</ErrorBoundary>
5155
);
5256
}
5357

‎static/app/views/detectors/components/forms/automateSection.spec.tsx‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
import {selectEvent} from 'sentry-test/selectEvent';
1818

1919
import {Form} from 'sentry/components/forms/form';
20+
import {ProjectsStore} from 'sentry/stores/projectsStore';
2021
import {ActionGroup, ActionType} from 'sentry/types/workflowEngine/actions';
2122
import {
2223
DataConditionHandlerGroupType,
@@ -37,6 +38,7 @@ describe('AutomateSection', () => {
3738
beforeEach(() => {
3839
jest.resetAllMocks();
3940
MockApiClient.clearMockResponses();
41+
ProjectsStore.loadInitialData([project]);
4042

4143
MockApiClient.addMockResponse({
4244
url: '/organizations/org-slug/workflows/',
@@ -130,8 +132,8 @@ describe('AutomateSection', () => {
130132

131133
it('can connect an existing automation', async () => {
132134
render(
133-
<DetectorFormProvider detectorType="metric_issue" project={project}>
134-
<Form>
135+
<DetectorFormProvider detectorType="metric_issue">
136+
<Form initialData={{projectId: project.id}}>
135137
<AutomateSection />
136138
</Form>
137139
</DetectorFormProvider>
@@ -165,8 +167,8 @@ describe('AutomateSection', () => {
165167

166168
it('can disconnect an existing automation', async () => {
167169
render(
168-
<DetectorFormProvider detectorType="metric_issue" project={project}>
169-
<Form initialData={{workflowIds: [automation1.id]}}>
170+
<DetectorFormProvider detectorType="metric_issue">
171+
<Form initialData={{projectId: project.id, workflowIds: [automation1.id]}}>
170172
<AutomateSection />
171173
</Form>
172174
</DetectorFormProvider>
@@ -217,8 +219,8 @@ describe('AutomateSection', () => {
217219
});
218220

219221
render(
220-
<DetectorFormProvider detectorType="metric_issue" project={project}>
221-
<Form>
222+
<DetectorFormProvider detectorType="metric_issue">
223+
<Form initialData={{projectId: project.id}}>
222224
<AutomateSection />
223225
</Form>
224226
</DetectorFormProvider>

‎static/app/views/detectors/components/forms/automateSection.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ import {AutomationBuilderDrawerForm} from 'sentry/views/automations/components/a
1515
import {ConnectAutomationsDrawer} from 'sentry/views/detectors/components/connectAutomationsDrawer';
1616
import {ConnectedAutomationsList} from 'sentry/views/detectors/components/connectedAutomationList';
1717
import {ConnectedAlertsEmptyState} from 'sentry/views/detectors/components/connectedAutomationsEmptyState';
18-
import {useDetectorFormContext} from 'sentry/views/detectors/components/forms/context';
18+
import {useDetectorFormProject} from 'sentry/views/detectors/components/forms/common/useDetectorFormProject';
1919

2020
export function AutomateSection({step}: {step?: number}) {
2121
const ref = useRef<HTMLButtonElement>(null);
2222
const formContext = useContext(FormContext);
2323
const {openDrawer, closeDrawer, isDrawerOpen} = useDrawer();
2424

25-
const {project} = useDetectorFormContext();
25+
const project = useDetectorFormProject();
2626
const workflowIds = useFormField('workflowIds') as string[];
2727
const setWorkflowIds = useCallback(
2828
(newWorkflowIds: string[]) =>

‎static/app/views/detectors/components/forms/common/projectField.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ import styled from '@emotion/styled';
33
import {SentryProjectSelectorField} from 'sentry/components/forms/fields/sentryProjectSelectorField';
44
import {t} from 'sentry/locale';
55
import {useProjects} from 'sentry/utils/useProjects';
6+
import {useDetectorFormProject} from 'sentry/views/detectors/components/forms/common/useDetectorFormProject';
67
import {useDetectorFormContext} from 'sentry/views/detectors/components/forms/context';
78
import {useCanEditDetector} from 'sentry/views/detectors/utils/useCanEditDetector';
89

910
export function ProjectField() {
1011
const {projects, fetching} = useProjects();
11-
const {project, detectorType} = useDetectorFormContext();
12+
const {detectorType} = useDetectorFormContext();
13+
const project = useDetectorFormProject();
1214
const canEditDetector = useCanEditDetector({projectId: project.id, detectorType});
1315

1416
return (

‎static/app/views/detectors/components/forms/common/useDetectorFormProject.tsx‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import {useMemo} from 'react';
2+
3+
import {useFormField} from 'sentry/components/workflowEngine/form/useFormField';
4+
import type {Project} from 'sentry/types/project';
5+
import {useProjects} from 'sentry/utils/useProjects';
6+
7+
export function useDetectorFormProject(): Project {
8+
const projectId = useFormField<string>('projectId');
9+
const {projects} = useProjects();
10+
const project = useMemo(
11+
() => projects.find(p => p.id === projectId),
12+
[projects, projectId]
13+
);
14+
// There's a top-level spinner when projects are loading, and you can't select a
15+
// project that's not in the ProjectStore, so this should never happen.
16+
if (!project) {
17+
throw new Error(
18+
`useDetectorFormProject: no project found for projectId "${projectId}"`
19+
);
20+
}
21+
return project;
22+
}

‎static/app/views/detectors/components/forms/common/useSetAutomaticName.spec.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ describe('useSetAutomaticName', () => {
3434

3535
const renderDetectorForm = (detector?: Detector, initialFormData = {}) => {
3636
return render(
37-
<DetectorFormProvider detectorType="error" project={project} detector={detector}>
37+
<DetectorFormProvider detectorType="error" detector={detector}>
3838
<NewDetectorLayout
3939
detectorType="error"
4040
formDataToEndpointPayload={data => data as any}

‎static/app/views/detectors/components/forms/context.tsx‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import {createContext, useContext, useState} from 'react';
22

3-
import type {Project} from 'sentry/types/project';
43
import type {Detector, DetectorType} from 'sentry/types/workflowEngine/detectors';
54

65
type DetectorFormContextType = {
@@ -10,7 +9,6 @@ type DetectorFormContextType = {
109
* Used by useSetAutomaticName to disable automatic name generation.
1110
*/
1211
hasSetDetectorName: boolean;
13-
project: Project;
1412
setHasSetDetectorName: (value: boolean) => void;
1513
detector?: Detector;
1614
};
@@ -19,20 +17,18 @@ const DetectorFormContext = createContext<DetectorFormContextType | null>(null);
1917

2018
export function DetectorFormProvider({
2119
detectorType,
22-
project,
2320
detector,
2421
children,
2522
}: {
2623
children: React.ReactNode;
2724
detectorType: DetectorType;
28-
project: Project;
2925
detector?: Detector;
3026
}) {
3127
const [hasSetDetectorName, setHasSetDetectorName] = useState(false);
3228

3329
return (
3430
<DetectorFormContext.Provider
35-
value={{detectorType, project, hasSetDetectorName, setHasSetDetectorName, detector}}
31+
value={{detectorType, hasSetDetectorName, setHasSetDetectorName, detector}}
3632
>
3733
{children}
3834
</DetectorFormContext.Provider>

‎static/app/views/detectors/components/forms/cron/cronIssuePreview.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {t} from 'sentry/locale';
22
import {DetectorIssuePreview} from 'sentry/views/detectors/components/forms/common/detectorIssuePreview';
33
import {IssuePreviewSection} from 'sentry/views/detectors/components/forms/common/issuePreviewSection';
44
import {ownerToActor} from 'sentry/views/detectors/components/forms/common/ownerToActor';
5-
import {useDetectorFormContext} from 'sentry/views/detectors/components/forms/context';
5+
import {useDetectorFormProject} from 'sentry/views/detectors/components/forms/common/useDetectorFormProject';
66
import {useCronDetectorFormField} from 'sentry/views/detectors/components/forms/cron/fields';
77

88
const FALLBACK_ISSUE_TITLE = t('Cron failure: …');
@@ -22,7 +22,7 @@ export function CronIssuePreview({step}: {step?: number}) {
2222
const owner = useCronDetectorFormField('owner');
2323
const issueTitle = useCronIssueTitle();
2424
const assignee = ownerToActor(owner);
25-
const {project} = useDetectorFormContext();
25+
const project = useDetectorFormProject();
2626

2727
return (
2828
<IssuePreviewSection step={step}>

‎static/app/views/detectors/components/forms/cron/index.spec.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('NewCronDetectorForm', () => {
4343

4444
const renderForm = (routerConfig?: any) => {
4545
return render(
46-
<DetectorFormProvider detectorType="monitor_check_in_failure" project={project}>
46+
<DetectorFormProvider detectorType="monitor_check_in_failure">
4747
<NewCronDetectorForm />
4848
</DetectorFormProvider>,
4949
{

‎static/app/views/detectors/components/forms/error/index.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ export function EditExistingErrorDetectorForm({detector}: {detector: ErrorDetect
141141
<EditLayout
142142
formProps={{
143143
initialData: {
144+
projectId: detector.projectId,
144145
workflowIds: detector.workflowIds,
145146
},
146147
onSubmit: handleFormSubmit,

0 commit comments

Comments
 (0)