Skip to content

Commit 923d715

Browse files
authored
chore(fe): Extract many functions out of AbstractExternalIssueForm (#87021)
This is actually way more complicated than the other one because of how many methods were on the abstract class. This PR changes nothing functionally about the class, but pulls out many of the methods into pure functions so that we can convert the child classes without referencing the base class. There is only one shared react element, so I moved that to its own file. I also moved the children classes into this directory since they were not grouped but shared almost all of their code.
1 parent d901a5f commit 923d715

10 files changed

+419
-203
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
1-
import {Fragment} from 'react';
2-
import debounce from 'lodash/debounce';
3-
import * as qs from 'query-string';
4-
51
import type {ModalRenderProps} from 'sentry/actionCreators/modal';
6-
import {Client} from 'sentry/api';
72
import DeprecatedAsyncComponent from 'sentry/components/deprecatedAsyncComponent';
8-
import FieldFromConfig from 'sentry/components/forms/fieldFromConfig';
3+
import {ExternalForm} from 'sentry/components/externalIssues/externalForm';
4+
import {
5+
debouncedOptionLoad,
6+
ensureCurrentOption,
7+
getConfigName,
8+
getDefaultOptions,
9+
getDynamicFields,
10+
getFieldProps,
11+
getOptions,
12+
hasErrorInFields,
13+
loadAsyncThenFetchAllFields,
14+
} from 'sentry/components/externalIssues/utils';
915
import type {FormProps} from 'sentry/components/forms/form';
10-
import Form from 'sentry/components/forms/form';
1116
import type {FieldValue} from 'sentry/components/forms/model';
1217
import FormModel from 'sentry/components/forms/model';
13-
import QuestionTooltip from 'sentry/components/questionTooltip';
1418
import {tct} from 'sentry/locale';
1519
import type {Choices, SelectValue} from 'sentry/types/core';
1620
import type {IntegrationIssueConfig, IssueConfigField} from 'sentry/types/integrations';
@@ -39,11 +43,6 @@ type State = {
3943
integrationDetails: IntegrationIssueConfig | null;
4044
} & DeprecatedAsyncComponent['state'];
4145

42-
// This exists because /extensions/type/search API is not prefixed with
43-
// /api/0/, but the default API client on the abstract issue form is...
44-
const API_CLIENT = new Client({baseUrl: '', headers: {}});
45-
46-
const DEBOUNCE_MS = 200;
4746
/**
4847
* @abstract
4948
*/
@@ -82,36 +81,17 @@ export default class AbstractExternalIssueForm<
8281
};
8382

8483
getConfigName = (): 'createIssueConfig' | 'linkIssueConfig' => {
85-
// Explicitly returning a non-interpolated string for clarity.
86-
const {action} = this.state;
87-
switch (action) {
88-
case 'create':
89-
return 'createIssueConfig';
90-
case 'link':
91-
return 'linkIssueConfig';
92-
default:
93-
throw new Error('illegal action');
94-
}
84+
return getConfigName(this.state.action);
9585
};
9686

97-
/**
98-
* Convert IntegrationIssueConfig to an object that maps field names to the
99-
* values of fields where `updatesFrom` is true. This function prefers to read
100-
* configs from its parameters and otherwise falls back to reading from state.
101-
* @param integrationDetailsParam
102-
* @returns Object of field names to values.
103-
*/
10487
getDynamicFields = (
10588
integrationDetailsParam?: IntegrationIssueConfig
10689
): {[key: string]: FieldValue | null} => {
107-
const {integrationDetails: integrationDetailsFromState} = this.state;
108-
const integrationDetails = integrationDetailsParam || integrationDetailsFromState;
109-
const config = integrationDetails?.[this.getConfigName()];
110-
return Object.fromEntries(
111-
(config || [])
112-
.filter((field: IssueConfigField) => field.updatesForm)
113-
.map((field: IssueConfigField) => [field.name, field.default || null])
114-
);
90+
return getDynamicFields({
91+
action: this.state.action,
92+
paramConfig: integrationDetailsParam,
93+
stateConfig: this.state.integrationDetails,
94+
});
11595
};
11696

11797
onRequestSuccess = ({stateKey, data}: any) => {
@@ -159,125 +139,48 @@ export default class AbstractExternalIssueForm<
159139
});
160140
};
161141

162-
/**
163-
* Ensures current result from Async select fields is never discarded. Without this method,
164-
* searching in an async select field without selecting one of the returned choices will
165-
* result in a value saved to the form, and no associated label; appearing empty.
166-
* @param field The field being examined
167-
* @param result The result from its asynchronous query
168-
* @returns The result with a tooltip attached to the current option
169-
*/
170142
ensureCurrentOption = (
171143
field: IssueConfigField,
172144
result: Array<SelectValue<string | number>>
173145
): Array<SelectValue<string | number>> => {
174-
const currentOption = this.getDefaultOptions(field).find(
175-
option => option.value === this.model.getValue(field.name)
176-
);
177-
178-
if (!currentOption) {
179-
return result;
180-
}
181-
if (typeof currentOption.label === 'string') {
182-
currentOption.label = (
183-
<Fragment>
184-
<QuestionTooltip
185-
title={tct('This is your current [label].', {
186-
label: field.label as React.ReactNode,
187-
})}
188-
size="xs"
189-
/>{' '}
190-
{currentOption.label}
191-
</Fragment>
192-
);
193-
}
194-
const currentOptionResultIndex = result.findIndex(
195-
obj => obj.value === currentOption?.value
196-
);
197-
// Has a selected option, and it is in API results
198-
if (currentOptionResultIndex >= 0) {
199-
const newResult = result;
200-
newResult[currentOptionResultIndex] = currentOption;
201-
return newResult;
202-
}
203-
// Has a selected option, and it is not in API results
204-
205-
return [...result, currentOption];
146+
return ensureCurrentOption({field, result, model: this.model});
206147
};
207148

208-
/**
209-
* Get the list of options for a field via debounced API call. For example,
210-
* the list of users that match the input string. The Promise rejects if there
211-
* are any errors.
212-
*/
213-
getOptions = (field: IssueConfigField, input: string) =>
214-
new Promise((resolve, reject) => {
215-
if (!input) {
216-
return resolve(this.getDefaultOptions(field));
217-
}
218-
return this.debouncedOptionLoad(field, input, (err, result) => {
219-
if (err) {
220-
reject(err);
221-
} else {
222-
result = this.ensureCurrentOption(field, result);
223-
this.updateFetchedFieldOptionsCache(field, result);
224-
resolve(result);
225-
}
226-
});
149+
getOptions = (field: IssueConfigField, input: string) => {
150+
return getOptions({
151+
field,
152+
input,
153+
model: this.model,
154+
dynamicFieldValues: this.state.dynamicFieldValues || {},
155+
successCallback: params => {
156+
this.updateFetchedFieldOptionsCache(params.field, params.result);
157+
},
227158
});
159+
};
228160

229-
debouncedOptionLoad = debounce(
230-
async (
231-
field: IssueConfigField,
232-
input: string,
233-
cb: (err: Error | null, result?: any) => void
234-
) => {
235-
const {dynamicFieldValues} = this.state;
236-
const query = qs.stringify({
237-
...dynamicFieldValues,
238-
field: field.name,
239-
query: input,
240-
});
241-
242-
const url = field.url || '';
243-
const separator = url.includes('?') ? '&' : '?';
244-
// We can't use the API client here since the URL is not scoped under the
245-
// API endpoints (which the client prefixes)
246-
247-
try {
248-
const response = await API_CLIENT.requestPromise(url + separator + query);
249-
cb(null, response);
250-
} catch (err) {
251-
cb(err);
252-
}
253-
},
254-
DEBOUNCE_MS,
255-
{trailing: true}
256-
);
161+
debouncedOptionLoad = (
162+
field: IssueConfigField,
163+
input: string,
164+
callback: (err: Error | null, result?: any) => void
165+
) => {
166+
return debouncedOptionLoad({
167+
field,
168+
input,
169+
callback,
170+
dynamicFieldValues: this.state.dynamicFieldValues || {},
171+
});
172+
};
257173

258174
getDefaultOptions = (field: IssueConfigField) => {
259-
const choices =
260-
(field.choices as Array<[number | string, number | string | React.ReactElement]>) ||
261-
[];
262-
return choices.map(([value, label]) => ({value, label}));
175+
return getDefaultOptions({field});
263176
};
264177

265-
/**
266-
* If this field is an async select (field.url is not null), add async props.
267-
*/
268-
getFieldProps = (field: IssueConfigField) =>
269-
field.url
270-
? {
271-
async: true,
272-
autoload: true,
273-
cache: false,
274-
loadOptions: (input: string) => this.getOptions(field, input),
275-
defaultOptions: this.getDefaultOptions(field),
276-
onBlurResetsInput: false,
277-
onCloseResetsInput: false,
278-
onSelectResetsInput: false,
279-
}
280-
: {};
178+
getFieldProps = (field: IssueConfigField) => {
179+
return getFieldProps({
180+
field,
181+
loadOptions: (input: string) => this.getOptions(field, input),
182+
});
183+
};
281184

282185
// Abstract methods.
283186
handleReceiveIntegrationDetails = (_data: any) => {
@@ -294,9 +197,7 @@ export default class AbstractExternalIssueForm<
294197
};
295198

296199
hasErrorInFields = (): boolean => {
297-
// check if we have any form fields with name error and type blank
298-
const fields = this.loadAsyncThenFetchAllFields();
299-
return fields.some(field => field.name === 'error' && field.type === 'blank');
200+
return hasErrorInFields({fields: this.loadAsyncThenFetchAllFields()});
300201
};
301202

302203
getDefaultFormProps = (): FormProps => {
@@ -315,17 +216,10 @@ export default class AbstractExternalIssueForm<
315216
* for each async field.
316217
*/
317218
loadAsyncThenFetchAllFields = (): IssueConfigField[] => {
318-
const {fetchedFieldOptionsCache, integrationDetails} = this.state;
319-
320-
const configsFromAPI = integrationDetails?.[this.getConfigName()];
321-
return (configsFromAPI || []).map(field => {
322-
const fieldCopy = {...field};
323-
// Overwrite choices from cache.
324-
if (fetchedFieldOptionsCache?.hasOwnProperty(field.name)) {
325-
fieldCopy.choices = fetchedFieldOptionsCache[field.name];
326-
}
327-
328-
return fieldCopy;
219+
return loadAsyncThenFetchAllFields({
220+
configName: this.getConfigName(),
221+
integrationDetails: this.state.integrationDetails,
222+
fetchedFieldOptionsCache: this.state.fetchedFieldOptionsCache,
329223
});
330224
};
331225

@@ -337,6 +231,7 @@ export default class AbstractExternalIssueForm<
337231
formFields?: IssueConfigField[],
338232
errors: ExternalIssueFormErrors = {}
339233
) => {
234+
const {Header, Body} = this.props as ModalRenderProps;
340235
const initialData: {[key: string]: any} = (formFields || []).reduce(
341236
(accumulator, field: FormField) => {
342237
// @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
@@ -346,47 +241,22 @@ export default class AbstractExternalIssueForm<
346241
{}
347242
);
348243

349-
const {Header, Body} = this.props as ModalRenderProps;
350-
351244
return (
352-
<Fragment>
353-
<Header closeButton>
354-
<h4>{this.getTitle()}</h4>
355-
</Header>
356-
{this.renderNavTabs()}
357-
<Body>
358-
{this.shouldRenderLoading ? (
359-
this.renderLoading()
360-
) : (
361-
<Fragment>
362-
{this.renderBodyText()}
363-
<Form initialData={initialData} {...this.getFormProps()}>
364-
{(formFields || [])
365-
.filter((field: FormField) => field.hasOwnProperty('name'))
366-
.map(fields => ({
367-
...fields,
368-
noOptionsMessage: () => 'No options. Type to search.',
369-
}))
370-
.map((field, i) => {
371-
return (
372-
<Fragment key={`${field.name}-${i}`}>
373-
<FieldFromConfig
374-
disabled={this.state.reloading}
375-
field={field}
376-
flexibleControlStateSize
377-
inline={false}
378-
stacked
379-
{...this.getFieldProps(field)}
380-
/>
381-
{errors[field.name] && errors[field.name]}
382-
</Fragment>
383-
);
384-
})}
385-
</Form>
386-
</Fragment>
387-
)}
388-
</Body>
389-
</Fragment>
245+
<ExternalForm
246+
Header={Header}
247+
Body={Body}
248+
formFields={formFields}
249+
errors={errors}
250+
isLoading={this.shouldRenderLoading}
251+
formProps={{
252+
...this.getFormProps(),
253+
initialData,
254+
}}
255+
title={this.getTitle()}
256+
navTabs={this.renderNavTabs()}
257+
bodyText={this.renderBodyText()}
258+
getFieldProps={this.getFieldProps}
259+
/>
390260
);
391261
};
392262
}

0 commit comments

Comments
 (0)