Skip to content

Commit

Permalink
address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
alisonelizabeth committed Sep 23, 2020
1 parent 2752f77 commit 4b4c81b
Show file tree
Hide file tree
Showing 9 changed files with 198 additions and 127 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,14 @@ const createActions = (testBed: TestBed<TestSubject>) => {

async toggleDocumentsAccordion() {
await act(async () => {
find('importDocumentsAccordion').simulate('click');
find('addDocumentsAccordion').simulate('click');
});
component.update();
},

async clickImportButton() {
async clickAddDocumentsButton() {
await act(async () => {
find('importDocumentButton').simulate('click');
find('addDocumentButton').simulate('click');
});
component.update();
},
Expand Down Expand Up @@ -248,7 +248,7 @@ type TestSubject =
| 'configurationTab'
| 'outputTab'
| 'processorOutputTabContent'
| 'importDocumentsAccordion'
| 'importDocumentButton'
| 'importDocumentError'
| 'addDocumentsAccordion'
| 'addDocumentButton'
| 'addDocumentError'
| string;
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ describe('Test pipeline', () => {
expect(find('pipelineExecutionError').text()).toContain(error.message);
});

describe('Import indexed documents', () => {
test('should successfully import an index document', async () => {
describe('Add indexed documents', () => {
test('should successfully add an indexed document', async () => {
const { actions, form } = testBed;

const { _index: index, _id: documentId } = DOCUMENTS[0];
Expand All @@ -186,7 +186,7 @@ describe('Test pipeline', () => {

// Open documents accordion, click run without required fields, and verify error messages
await actions.toggleDocumentsAccordion();
await actions.clickImportButton();
await actions.clickAddDocumentsButton();
expect(form.getErrorsMessages()).toEqual([
'An index name is required.',
'A document ID is required.',
Expand All @@ -195,7 +195,7 @@ describe('Test pipeline', () => {
// Add required fields, and click run
form.setInputValue('indexField.input', index);
form.setInputValue('idField.input', documentId);
await actions.clickImportButton();
await actions.clickAddDocumentsButton();

// Verify request
const latestRequest = server.requests[server.requests.length - 1];
Expand Down Expand Up @@ -226,11 +226,11 @@ describe('Test pipeline', () => {
await actions.toggleDocumentsAccordion();
form.setInputValue('indexField.input', nonExistentDoc.index);
form.setInputValue('idField.input', nonExistentDoc.id);
await actions.clickImportButton();
await actions.clickAddDocumentsButton();

// Verify error rendered
expect(exists('importDocumentError')).toBe(true);
expect(find('importDocumentError').text()).toContain(error.message);
expect(exists('addDocumentError')).toBe(true);
expect(find('addDocumentError').text()).toContain(error.message);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { useTestPipelineContext } from '../../context';
import { serialize } from '../../serialize';
import { DeserializeResult } from '../../deserialize';
import { Document } from '../../types';
import { useIsMounted } from '../../use_is_mounted';
import { TestPipelineFlyout as ViewComponent } from './test_pipeline_flyout';

import { TestPipelineFlyoutTab } from './test_pipeline_flyout_tabs';
Expand All @@ -34,6 +35,7 @@ export const TestPipelineFlyout: React.FunctionComponent<Props> = ({
processors,
}) => {
const { services } = useKibana();
const isMounted = useIsMounted();

const {
testPipelineData,
Expand Down Expand Up @@ -74,6 +76,10 @@ export const TestPipelineFlyout: React.FunctionComponent<Props> = ({
pipeline: { ...serializedProcessors },
});

if (!isMounted.current) {
return { isSuccessful: false };
}

setIsRunningTest(false);

if (error) {
Expand Down Expand Up @@ -123,6 +129,7 @@ export const TestPipelineFlyout: React.FunctionComponent<Props> = ({
return { isSuccessful: true };
},
[
isMounted,
processors,
services.api,
services.notifications.toasts,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,47 +26,51 @@ import {
fieldValidators,
FieldConfig,
} from '../../../../../../shared_imports';
import { useIsMounted } from '../../../use_is_mounted';

const UseField = getUseField({ component: Field });

const { emptyField } = fieldValidators;

const i18nTexts = {
importDocumentButton: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.loadDocuments.importDocButtonLabel',
addDocumentButton: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.addDocuments.addDocumentButtonLabel',
{
defaultMessage: 'Import',
defaultMessage: 'Add',
}
),
importDocumentErrorMessage: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.loadDocuments.importDocErrorMessage',
addDocumentErrorMessage: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.addDocuments.addDocumentErrorMessage',
{
defaultMessage: 'Error importing document',
defaultMessage: 'Error adding document',
}
),
addDocumentSuccessMessage: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.addDocuments.addDocumentSuccessMessage',
{
defaultMessage: 'Document added',
}
),
indexField: {
fieldLabel: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.loadDocuments.table.indexColumnLabel',
'xpack.ingestPipelines.pipelineEditor.addDocuments.indexFieldLabel',
{
defaultMessage: 'Index',
}
),
validationMessage: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.loadDocuments.indexRequiredError',
'xpack.ingestPipelines.pipelineEditor.addDocuments.indexRequiredErrorMessage',
{
defaultMessage: 'An index name is required.',
}
),
},
idField: {
fieldLabel: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.loadDocuments.table.documentIdColumnLabel',
{
defaultMessage: 'Document ID',
}
),
fieldLabel: i18n.translate('xpack.ingestPipelines.pipelineEditor.addDocuments.idFieldLabel', {
defaultMessage: 'Document ID',
}),
validationMessage: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.loadDocuments.idRequiredError',
'xpack.ingestPipelines.pipelineEditor.loadDocuments.idRequiredErrorMessage',
{
defaultMessage: 'A document ID is required.',
}
Expand Down Expand Up @@ -97,11 +101,13 @@ interface Props {
onAddDocuments: (document: any) => void;
}

export const ImportDocumentForm: FunctionComponent<Props> = ({ onAddDocuments }) => {
export const AddDocumentForm: FunctionComponent<Props> = ({ onAddDocuments }) => {
const { services } = useKibana();
const isMounted = useIsMounted();

const [isLoadingDocument, setIsLoadingDocument] = useState<boolean>(false);
const [loadingDocumentError, setLoadingDocumentError] = useState<Error | undefined>(undefined);
const [documentError, setDocumentError] = useState<Error | undefined>(undefined);
const [isNewDocumentAdded, setIsNewDocumentAdded] = useState<boolean>(false);

const { form } = useForm({ defaultValue: { index: '', id: '' } });

Expand All @@ -112,39 +118,59 @@ export const ImportDocumentForm: FunctionComponent<Props> = ({ onAddDocuments })

if (isValid) {
setIsLoadingDocument(true);
setLoadingDocumentError(undefined);
setDocumentError(undefined);
setIsNewDocumentAdded(false);

const { error, data: document } = await services.api.loadDocument(index, id);

if (!isMounted.current) {
return;
}

setIsLoadingDocument(false);

if (error) {
setLoadingDocumentError(error);
setDocumentError(error);
return;
}

setIsNewDocumentAdded(true);
onAddDocuments(document);
form.reset();
}
};

return (
<Form form={form} onSubmit={submitForm}>
{loadingDocumentError && (
{documentError && (
<>
<EuiCallOut
title={i18nTexts.importDocumentErrorMessage}
title={i18nTexts.addDocumentErrorMessage}
color="danger"
iconType="alert"
data-test-subj="importDocumentError"
data-test-subj="addDocumentError"
size="s"
>
<p>{loadingDocumentError.message}</p>
<p>{documentError.message}</p>
</EuiCallOut>

<EuiSpacer size="m" />
</>
)}

{isNewDocumentAdded && (
<>
<EuiCallOut
title={i18nTexts.addDocumentSuccessMessage}
color="success"
iconType="check"
data-test-subj="addDocumentSuccess"
size="s"
/>

<EuiSpacer size="m" />
</>
)}

<EuiPanel paddingSize="m">
<EuiFlexGroup>
<EuiFlexItem>
Expand Down Expand Up @@ -173,10 +199,10 @@ export const ImportDocumentForm: FunctionComponent<Props> = ({ onAddDocuments })
<EuiFormRow hasEmptyLabelSpace>
<EuiButton
onClick={submitForm}
data-test-subj="importDocumentButton"
data-test-subj="addDocumentButton"
isLoading={isLoadingDocument}
>
{i18nTexts.importDocumentButton}
{i18nTexts.addDocumentButton}
</EuiButton>
</EuiFormRow>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FunctionComponent, useState, useEffect } from 'react';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';

import { EuiAccordion, EuiText, EuiSpacer, EuiLink } from '@elastic/eui';

import { useKibana } from '../../../../../../shared_imports';
import { useIsMounted } from '../../../use_is_mounted';
import { AddDocumentForm } from './add_document_form';

const i18nTexts = {
addDocumentsButton: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.addDocumentsAccordion.addDocumentsButtonLabel',
{
defaultMessage: 'Add documents from index',
}
),
};

interface Props {
onAddDocuments: (document: any) => void;
}

export const AddDocumentsAccordion: FunctionComponent<Props> = ({ onAddDocuments }) => {
const { services } = useKibana();
const isMounted = useIsMounted();
const [discoverLink, setDiscoverLink] = useState<string | undefined>(undefined);

useEffect(() => {
const getDiscoverUrl = async (): Promise<void> => {
const { isDeprecated, createUrl } = services.urlGenerators.getUrlGenerator(
'DISCOVER_APP_URL_GENERATOR'
);

if (isDeprecated) {
setDiscoverLink(undefined);
return;
}

const discoverUrl = await createUrl({ indexPatternId: undefined });

if (isMounted.current) {
setDiscoverLink(discoverUrl);
}
};

getDiscoverUrl();
}, [isMounted, services.urlGenerators]);

return (
<EuiAccordion
id="addDocumentsAccordion"
buttonContent={i18nTexts.addDocumentsButton}
paddingSize="s"
data-test-subj="addDocumentsAccordion"
>
<>
<EuiText size="s" color="subdued">
<p>
<FormattedMessage
id="xpack.ingestPipelines.pipelineEditor.addDocumentsAccordion.contentDescriptionText"
defaultMessage="Provide the index name and document ID of the indexed document to test."
/>
{discoverLink && (
<>
{' '}
<FormattedMessage
id="xpack.ingestPipelines.pipelineEditor.addDocumentsAccordion.discoverLinkDescriptionText"
defaultMessage="To explore your existing data, use {discoverLink}."
values={{
discoverLink: (
<EuiLink href={discoverLink} target="_blank" external>
Discover
</EuiLink>
),
}}
/>
</>
)}
</p>
</EuiText>

<EuiSpacer size="m" />

<AddDocumentForm onAddDocuments={onAddDocuments} />
</>
</EuiAccordion>
);
};
Loading

0 comments on commit 4b4c81b

Please sign in to comment.