Better scripted field editor (#36064) (#36340)

* fix: 🐛 make script input field full width * test: 💍 update Jest snapshots * feat: 🎸 add Groovy syntax highlighting * test: 💍 update test snapshot * test: 💍 fix scripted field form functional test * test: 💍 fix more functional tests for scripted field input * refactor: 💡 use import instead of require()
2019-05-09 13:12:22 +02:00 · 2019-05-09 13:12:22 +02:00 · 8aa6fb52f8
parent e0cc92f624
commit 8aa6fb52f8
7 changed files with 53 additions and 29 deletions
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@ -332,7 +332,7 @@ Test runner arguments:
    `node scripts/jest -t 'stops both admin and data clients' src/core/server/elasticsearch/elasticsearch_service.test.ts`
  - Run the api integration test case whose description matches the given string:
    `node scripts/functional_tests_server --config test/api_integration/config.js`
-    `node scripts/functional_tests_runner --config test/api_integration/config.js --grep='should return 404 if id does not match any sample data sets'`
+    `node scripts/functional_test_runner --config test/api_integration/config.js --grep='should return 404 if id does not match any sample data sets'`

 ### Debugging Unit Tests

--- a/src/legacy/ui/public/field_editor/snapshots/field_editor.test.js.snap
+++ b/src/legacy/ui/public/field_editor/snapshots/field_editor.test.js.snap
@ -151,13 +151,16 @@ exports[`FieldEditor should render create new scripted field correctly 1`] = `
    </eui-form-row>
    <eui-form-row
      error="Script is required"
+      fullWidth={true}
      isInvalid={true}
      label="Script"
    >
-      <eui-textArea
+      <Component
        data-test-subj="editorFieldScript"
-        isInvalid={true}
+        height="300px"
+        mode="groovy"
        onChange={[Function]}
+        width="100%"
      />
    </eui-form-row>
    <eui-form-row>
@ -379,14 +382,17 @@ exports[`FieldEditor should render edit scripted field correctly 1`] = `
    </eui-form-row>
    <eui-form-row
      error={null}
+      fullWidth={true}
      isInvalid={false}
      label="Script"
    >
-      <eui-textArea
+      <Component
        data-test-subj="editorFieldScript"
-        isInvalid={false}
+        height="300px"
+        mode="groovy"
        onChange={[Function]}
        value="doc.test.value"
+        width="100%"
      />
    </eui-form-row>
    <eui-form-row>
@ -672,13 +678,16 @@ exports[`FieldEditor should show conflict field warning 1`] = `
    </eui-form-row>
    <eui-form-row
      error="Script is required"
+      fullWidth={true}
      isInvalid={true}
      label="Script"
    >
-      <eui-textArea
+      <Component
        data-test-subj="editorFieldScript"
-        isInvalid={true}
+        height="300px"
+        mode="groovy"
        onChange={[Function]}
+        width="100%"
      />
    </eui-form-row>
    <eui-form-row>
@ -986,14 +995,17 @@ exports[`FieldEditor should show deprecated lang warning 1`] = `
    </eui-form-row>
    <eui-form-row
      error={null}
+      fullWidth={true}
      isInvalid={false}
      label="Script"
    >
-      <eui-textArea
+      <Component
        data-test-subj="editorFieldScript"
-        isInvalid={false}
+        height="300px"
+        mode="groovy"
        onChange={[Function]}
        value="doc.test.value"
+        width="100%"
      />
    </eui-form-row>
    <eui-form-row>
@ -1332,13 +1344,16 @@ exports[`FieldEditor should show multiple type field warning with a table contai
    </eui-form-row>
    <eui-form-row
      error="Script is required"
+      fullWidth={true}
      isInvalid={true}
      label="Script"
    >
-      <eui-textArea
+      <Component
        data-test-subj="editorFieldScript"
-        isInvalid={true}
+        height="300px"
+        mode="groovy"
        onChange={[Function]}
+        width="100%"
      />
    </eui-form-row>
    <eui-form-row>
--- a/src/legacy/ui/public/field_editor/field_editor.js
+++ b/src/legacy/ui/public/field_editor/field_editor.js
@ -45,6 +45,7 @@ import {
  EuiButtonEmpty,
  EuiCallOut,
  EuiCode,
+  EuiCodeEditor,
  EuiConfirmModal,
  EuiFieldNumber,
  EuiFieldText,
@ -58,7 +59,6 @@ import {
  EuiSelect,
  EuiSpacer,
  EuiText,
-  EuiTextArea,
  EUI_MODAL_CONFIRM_BUTTON,
 } from '@elastic/eui';

@ -80,6 +80,9 @@ import { copyField, getDefaultFormat, executeScript, isScriptValid } from './lib

 import { injectI18n, FormattedMessage } from '@kbn/i18n/react';

+// This loads Ace editor's "groovy" mode, used below to highlight the script.
+import 'brace/mode/groovy';
+
 export class FieldEditorComponent extends PureComponent {
  static propTypes = {
    indexPattern: PropTypes.object.isRequired,
@ -459,11 +462,11 @@ export class FieldEditorComponent extends PureComponent {
    );
  }

-  onScriptChange = (e) => {
+  onScriptChange = (value) => {
    this.setState({
      hasScriptError: false
    });
-    this.onFieldChange('script', e.target.value);
+    this.onFieldChange('script', value);
  }

  renderScript() {
@ -481,15 +484,18 @@ export class FieldEditorComponent extends PureComponent {
    return field.scripted ? (
      <Fragment>
        <EuiFormRow
+          fullWidth
          label={intl.formatMessage({ id: 'common.ui.fieldEditor.scriptLabel', defaultMessage: 'Script' })}
          isInvalid={isInvalid}
          error={isInvalid ? errorMsg : null}
        >
-          <EuiTextArea
+          <EuiCodeEditor
            value={field.script}
            data-test-subj="editorFieldScript"
            onChange={this.onScriptChange}
-            isInvalid={isInvalid}
+            mode="groovy"
+            width="100%"
+            height="300px"
          />
        </EuiFormRow>

--- a/src/legacy/ui/public/field_editor/field_editor.test.js
+++ b/src/legacy/ui/public/field_editor/field_editor.test.js
@ -22,6 +22,8 @@ jest.mock('ui/kfetch', () => ({}));
 import React from 'react';
 import { shallowWithIntl } from 'test_utils/enzyme_helpers';

+jest.mock('brace/mode/groovy', () => ({}));
+
 import { FieldEditorComponent } from './field_editor';

 jest.mock('@elastic/eui', () => ({
--- a/test/functional/apps/management/_scripted_fields.js
+++ b/test/functional/apps/management/_scripted_fields.js
@ -29,6 +29,10 @@
 // 3. Filter in Discover by the scripted field
 // 4. Visualize with aggregation on the scripted field by clicking discover.clickFieldListItemVisualize

+// NOTE: Scripted field input is managed by Ace editor, which automatically
+//   appends closing braces, for exmaple, if you type opening square brace [
+//   it will automatically insert a a closing square brace ], etc.
+
 import expect from '@kbn/expect';

 export default function ({ getService, getPageObjects }) {
@ -65,7 +69,7 @@ export default function ({ getService, getPageObjects }) {
      await PageObjects.settings.clickScriptedFieldsTab();
      await PageObjects.settings.clickAddScriptedField();
      await PageObjects.settings.setScriptedFieldName('doomedScriptedField');
-      await PageObjects.settings.setScriptedFieldScript(`doc['iHaveNoClosingTick].value`);
+      await PageObjects.settings.setScriptedFieldScript(`i n v a l i d  s c r i p t`);
      await PageObjects.settings.clickSaveScriptedField();
      await retry.try(async () => {
        const invalidScriptErrorExists = await testSubjects.exists('invalidScriptError');
@ -110,11 +114,9 @@ export default function ({ getService, getPageObjects }) {
        const startingCount = parseInt(await PageObjects.settings.getScriptedFieldsTabCount());
        await PageObjects.settings.clickScriptedFieldsTab();
        await log.debug('add scripted field');
-        const script = `if (doc['machine.ram'].size() == 0) {
-          return -1;
-        } else {
-          return doc['machine.ram'].value / (1024 * 1024 * 1024);
-        }`;
+        const script = `if (doc['machine.ram'].size() == 0) return -1;
+          else return doc['machine.ram'].value / (1024 * 1024 * 1024);
+        `;
        await PageObjects.settings.addScriptedField(scriptedPainlessFieldName, 'painless', 'number', null, '1', script);
        await retry.try(async function () {
          expect(parseInt(await PageObjects.settings.getScriptedFieldsTabCount())).to.be(startingCount + 1);
--- a/test/functional/apps/management/_scripted_fields_preview.js
+++ b/test/functional/apps/management/_scripted_fields_preview.js
@ -45,7 +45,7 @@ export default function ({ getService, getPageObjects }) {
    });

    it('should display script error when script is invalid', async function () {
-      const scriptResults = await PageObjects.settings.executeScriptedField(`doc['iHaveNoClosingTick].value`);
+      const scriptResults = await PageObjects.settings.executeScriptedField(`i n v a l i d  s c r i p t`);
      expect(scriptResults).to.contain('search_phase_execution_exception');
    });

--- a/test/functional/page_objects/settings_page.js
+++ b/test/functional/page_objects/settings_page.js
@ -510,13 +510,12 @@ export function SettingsPageProvider({ getService, getPageObjects }) {

    async setScriptedFieldScript(script) {
      log.debug('set scripted field script = ' + script);
-      const field = await testSubjects.find('editorFieldScript');
-      const currentValue = await field.getAttribute('value');
-      if (script === currentValue) {
-        return;
+      const aceEditorCssSelector = '[data-test-subj="codeEditorContainer"] .ace_editor';
+      await find.clickByCssSelector(aceEditorCssSelector);
+      for (let i = 0; i < 1000; i++) {
+        await browser.pressKeys(browser.keys.BACK_SPACE);
      }
-      await field.clearValueWithKeyboard({ charByChar: true });
-      await field.type(script);
+      await browser.pressKeys(...script.split(''));
    }

    async openScriptedFieldHelp(activeTab) {