Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Improve input constraints; general usability improvements #296

Merged
merged 1 commit into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,19 @@ export const FETCH_ALL_QUERY_BODY = {
size: 1000,
};
export const INDEX_NOT_FOUND_EXCEPTION = 'index_not_found_exception';
export const ERROR_GETTING_WORKFLOW_MSG = 'Failed to retrieve template';
export const NO_MODIFICATIONS_FOUND_TEXT =
'Template does not contain any modifications';
export const JSONPATH_ROOT_SELECTOR = '$.';
export enum SORT_ORDER {
ASC = 'asc',
DESC = 'desc',
}
export const MAX_DOCS = 1000;
export const MAX_STRING_LENGTH = 100;
export const MAX_JSON_STRING_LENGTH = 10000;
export const MAX_WORKFLOW_NAME_TO_DISPLAY = 40;
export const WORKFLOW_NAME_REGEXP = RegExp('^[a-zA-Z0-9_-]*$');

export enum PROCESSOR_CONTEXT {
INGEST = 'ingest',
Expand Down
11 changes: 11 additions & 0 deletions common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,14 @@ export const prettifyErrorMessage = (rawErrorMessage: string) => {
return `User ${match[2]} has no permissions to [${match[1]}].`;
}
};

export function getCharacterLimitedString(
input: string | undefined,
limit: number
): string {
return input !== undefined
? input.length > limit
? input.substring(0, limit - 3) + '...'
: input
: '';
}
50 changes: 0 additions & 50 deletions public/general_components/delete_workflow_modal.tsx

This file was deleted.

1 change: 0 additions & 1 deletion public/general_components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,5 @@
*/

export { MultiSelectFilter } from './multi_select_filter';
export { DeleteWorkflowModal } from './delete_workflow_modal';
export { ProcessorsTitle } from './processors_title';
export { ResourceList } from './resource_list';
11 changes: 9 additions & 2 deletions public/pages/workflow_detail/components/export_modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ import {
EuiModalFooter,
EuiSmallButtonEmpty,
} from '@elastic/eui';
import { CREATE_WORKFLOW_LINK, Workflow } from '../../../../common';
import {
CREATE_WORKFLOW_LINK,
Workflow,
getCharacterLimitedString,
} from '../../../../common';
import { reduceToTemplate } from '../../../utils';

interface ExportModalProps {
Expand Down Expand Up @@ -69,7 +73,10 @@ export function ExportModal(props: ExportModalProps) {
<EuiModal onClose={() => props.setIsExportModalOpen(false)}>
<EuiModalHeader>
<EuiModalHeaderTitle>
<p>{`Export ${props.workflow?.name}`}</p>
<p>{`Export ${getCharacterLimitedString(
props.workflow?.name || '',
25
)}`}</p>
</EuiModalHeaderTitle>
</EuiModalHeader>
<EuiModalBody>
Expand Down
9 changes: 8 additions & 1 deletion public/pages/workflow_detail/components/header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ import {
} from '@elastic/eui';
import {
DEFAULT_NEW_WORKFLOW_STATE,
MAX_WORKFLOW_NAME_TO_DISPLAY,
WORKFLOW_STATE,
Workflow,
getCharacterLimitedString,
toFormattedDate,
} from '../../../../common';
import { APP_PATH } from '../../../utils';
Expand All @@ -38,7 +40,12 @@ export function WorkflowDetailHeader(props: WorkflowDetailHeaderProps) {

useEffect(() => {
if (props.workflow) {
setWorkflowName(props.workflow.name);
setWorkflowName(
getCharacterLimitedString(
props.workflow.name,
MAX_WORKFLOW_NAME_TO_DISPLAY
)
);
setWorkflowState(props.workflow.state || DEFAULT_NEW_WORKFLOW_STATE);
try {
const formattedDate = toFormattedDate(
Expand Down
55 changes: 46 additions & 9 deletions public/pages/workflow_detail/workflow_detail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,16 @@ import React, { useEffect, ReactElement } from 'react';
import { RouteComponentProps } from 'react-router-dom';
import { useSelector } from 'react-redux';
import { ReactFlowProvider } from 'reactflow';
import { EuiPage, EuiPageBody } from '@elastic/eui';
import { BREADCRUMBS } from '../../utils';
import { escape } from 'lodash';
import {
EuiButton,
EuiEmptyPrompt,
EuiFlexGroup,
EuiFlexItem,
EuiPage,
EuiPageBody,
} from '@elastic/eui';
import { APP_PATH, BREADCRUMBS } from '../../utils';
import { getCore } from '../../services';
import { WorkflowDetailHeader } from './components';
import {
Expand All @@ -19,16 +27,21 @@ import {
} from '../../store';
import { ResizableWorkspace } from './resizable_workspace';
import {
DEFAULT_NEW_WORKFLOW_NAME,
ERROR_GETTING_WORKFLOW_MSG,
FETCH_ALL_QUERY_BODY,
MAX_WORKFLOW_NAME_TO_DISPLAY,
getCharacterLimitedString,
} from '../../../common';
import { MountPoint } from '../../../../../src/core/public';

// styling
import './workflow-detail-styles.scss';
import '../../global-styles.scss';

import { getDataSourceId } from '../../utils/utils';
import {
constructHrefWithDataSourceId,
getDataSourceId,
} from '../../utils/utils';

import {
getDataSourceManagementPlugin,
Expand Down Expand Up @@ -57,12 +70,17 @@ export function WorkflowDetail(props: WorkflowDetailProps) {
const dispatch = useAppDispatch();
const dataSourceEnabled = getDataSourceEnabled().enabled;
const dataSourceId = getDataSourceId();
const { workflows } = useSelector((state: AppState) => state.workflows);
const { workflows, errorMessage } = useSelector(
(state: AppState) => state.workflows
);

// selected workflow state
const workflowId = props.match?.params?.workflowId;
const workflowId = escape(props.match?.params?.workflowId);
const workflow = workflows[workflowId];
const workflowName = workflow ? workflow.name : DEFAULT_NEW_WORKFLOW_NAME;
const workflowName = getCharacterLimitedString(
workflow?.name || '',
MAX_WORKFLOW_NAME_TO_DISPLAY
);

useEffect(() => {
if (dataSourceEnabled) {
Expand All @@ -78,7 +96,7 @@ export function WorkflowDetail(props: WorkflowDetailProps) {
{ text: workflowName },
]);
}
}, []);
}, [workflowName]);

// On initial load:
// - fetch workflow
Expand Down Expand Up @@ -107,7 +125,26 @@ export function WorkflowDetail(props: WorkflowDetailProps) {
);
}

return (
return errorMessage.includes(ERROR_GETTING_WORKFLOW_MSG) ? (
<EuiFlexGroup direction="column" alignItems="center">
<EuiFlexItem grow={3}>
<EuiEmptyPrompt
iconType={'cross'}
title={<h2>Oops! We couldn't find that workflow</h2>}
titleSize="s"
/>
</EuiFlexItem>
<EuiFlexItem grow={7}>
<EuiButton
style={{ width: '200px' }}
fill={false}
href={constructHrefWithDataSourceId(APP_PATH.WORKFLOWS, dataSourceId)}
>
Return to home
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
) : (
<ReactFlowProvider>
{dataSourceEnabled && renderDataSourceComponent}
<EuiPage>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export function SourceData(props: SourceDataProps) {
/>
<EuiSpacer size="s" />
<JsonField
label="Documents"
fieldPath={'ingest.docs'}
helpText="Documents should be formatted as a valid JSON array."
// when ingest doc values change, don't update the form
Expand Down Expand Up @@ -123,6 +124,7 @@ export function SourceData(props: SourceDataProps) {
</EuiFlexItem>
<EuiFlexItem grow={false}>
<JsonField
label="Documents"
fieldPath={'ingest.docs'}
helpText="Documents should be formatted as a valid JSON array."
// when ingest doc values change, don't update the form
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export function JsonField(props: JsonFieldProps) {
{({ field, form }: FieldProps) => {
return (
<EuiCompressedFormRow
fullWidth={true}
key={props.fieldPath}
label={props.label || camelCaseToTitleString(field.name)}
labelAppend={
Expand Down Expand Up @@ -97,6 +98,9 @@ export function JsonField(props: JsonFieldProps) {
setOptions={{
fontSize: '14px',
useWorker: validate,
highlightActiveLine: !props.readOnly,
highlightSelectedWord: !props.readOnly,
highlightGutterLine: !props.readOnly,
}}
aria-label="Code Editor"
tabSize={2}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ import {
EuiTitle,
} from '@elastic/eui';
import {
MAX_WORKFLOW_NAME_TO_DISPLAY,
SearchHit,
TemplateNode,
WORKFLOW_STEP_TYPE,
Workflow,
WorkflowConfig,
WorkflowFormValues,
WorkflowTemplate,
getCharacterLimitedString,
} from '../../../../common';
import { IngestInputs } from './ingest_inputs';
import { SearchInputs } from './search_inputs';
Expand Down Expand Up @@ -596,7 +598,10 @@ export function WorkflowInputs(props: WorkflowInputsProps) {
<EuiModal onClose={() => setIsModalOpen(false)}>
<EuiModalHeader>
<EuiModalHeaderTitle>
<p>{`Delete resources for workflow ${props.workflow?.name}?`}</p>
<p>{`Delete resources for workflow ${getCharacterLimitedString(
props.workflow?.name || '',
MAX_WORKFLOW_NAME_TO_DISPLAY
)}?`}</p>
</EuiModalHeaderTitle>
</EuiModalHeader>
<EuiModalBody>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
} from '../../../store';
import { FETCH_ALL_QUERY_BODY, Workflow } from '../../../../common';
import { WORKFLOWS_TAB } from '../workflows';
import { getDataSourceId } from '../../../utils/utils';
import { getDataSourceId } from '../../../utils/utils';

interface ImportWorkflowModalProps {
isImportModalOpen: boolean;
Expand All @@ -51,6 +51,9 @@ export function ImportWorkflowModal(props: ImportWorkflowModalProps) {
const dispatch = useAppDispatch();
const dataSourceId = getDataSourceId();

// transient importing state for button state
const [isImporting, setIsImporting] = useState<boolean>(false);

// file contents & file obj state
const [fileContents, setFileContents] = useState<string | undefined>(
undefined
Expand Down Expand Up @@ -133,8 +136,10 @@ export function ImportWorkflowModal(props: ImportWorkflowModalProps) {
<EuiModalFooter>
<EuiButtonEmpty onClick={() => onModalClose()}>Cancel</EuiButtonEmpty>
<EuiButton
disabled={!isValidWorkflow(fileObj)}
disabled={!isValidWorkflow(fileObj) || isImporting}
isLoading={isImporting}
onClick={() => {
setIsImporting(true);
dispatch(
createWorkflow({
apiBody: fileObj as Workflow,
Expand All @@ -159,6 +164,7 @@ export function ImportWorkflowModal(props: ImportWorkflowModalProps) {
getCore().notifications.toasts.addDanger(error);
})
.finally(() => {
setIsImporting(false);
onModalClose();
});
}}
Expand Down
17 changes: 13 additions & 4 deletions public/pages/workflows/new_workflow/use_case.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
EuiCompressedFieldText,
EuiCompressedFormRow,
} from '@elastic/eui';
import { Workflow } from '../../../../common';
import { WORKFLOW_NAME_REGEXP, Workflow } from '../../../../common';
import { APP_PATH } from '../../../utils';
import { processWorkflowName } from './utils';
import { createWorkflow, useAppDispatch } from '../../../store';
Expand All @@ -45,6 +45,15 @@ export function UseCase(props: UseCaseProps) {
processWorkflowName(props.workflow.name)
);

// custom sanitization on workflow name
function isInvalid(name: string): boolean {
return (
name === '' ||
name.length > 100 ||
WORKFLOW_NAME_REGEXP.test(name) === false
);
}

return (
<>
{isNameModalOpen && (
Expand All @@ -57,8 +66,8 @@ export function UseCase(props: UseCaseProps) {
<EuiModalBody>
<EuiCompressedFormRow
label={'Name'}
error={'Name cannot be empty'}
isInvalid={workflowName === ''}
error={'Invalid name'}
isInvalid={isInvalid(workflowName)}
>
<EuiCompressedFieldText
placeholder={processWorkflowName(props.workflow.name)}
Expand All @@ -74,7 +83,7 @@ export function UseCase(props: UseCaseProps) {
Cancel
</EuiSmallButtonEmpty>
<EuiSmallButton
disabled={workflowName === ''}
disabled={isInvalid(workflowName)}
onClick={() => {
const workflowToCreate = {
...props.workflow,
Expand Down
Loading
Loading