[ES|QL] new pattern for SORT autocomplete (#193595)

## Summary Part of #189662. This PR - updates the autocomplete behavior for `SORT` to be in line with other field-list-based experiences like `KEEP` - introduces a shared function, `handleFragment`, which is used to abstract some of the logic required to support this behavior - bulks up the `SORT` tests - restores the function suggestions which I noticed got lost in #189959 **Before** https://github.com/user-attachments/assets/cad1d073-c010-426f-9628-c0fc6b65eb3c **After** https://github.com/user-attachments/assets/e148ae58-4430-482c-9f8e-c55779c4d822 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Stratoula Kalafateli <[email protected]>
elastic · Sep 23, 2024 · f450e22 · f450e22
1 parent 6a0fa96
commit f450e22
Show file tree

Hide file tree

Showing 6 changed files with 480 additions and 207 deletions.
diff --git a/...esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts b/...esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts
@@ -7,21 +7,101 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
-import { setup, getFieldNamesByType } from './helpers';
+import {
+  setup,
+  getFieldNamesByType,
+  attachTriggerCommand,
+  getFunctionSignaturesByReturnType,
+} from './helpers';
 
 describe('autocomplete.suggest', () => {
   describe('SORT ( <column> [ ASC / DESC ] [ NULLS FIST / NULLS LAST ] )+', () => {
     describe('SORT <column> ...', () => {
-      test('suggests command on first character', async () => {
+      const expectedFieldSuggestions = getFieldNamesByType('any').map(attachTriggerCommand);
+      const expectedFunctionSuggestions = getFunctionSignaturesByReturnType('sort', 'any', {
+        scalar: true,
+      }).map(attachTriggerCommand);
+
+      test('suggests column', async () => {
         const { assertSuggestions } = await setup();
 
+        await assertSuggestions('from a | sort /', [
+          ...expectedFieldSuggestions,
+          ...expectedFunctionSuggestions,
+        ]);
+        await assertSuggestions('from a | sort keyw/', [
+          ...expectedFieldSuggestions,
+          ...expectedFunctionSuggestions,
+        ]);
         await assertSuggestions(
-          'from a | sort /',
-          [...getFieldNamesByType('any')].map((field) => `${field} `)
+          'from a | sort keywordField/',
+          [
+            {
+              filterText: 'keywordField',
+              text: 'keywordField, ',
+            },
+            {
+              filterText: 'keywordField',
+              text: 'keywordField | ',
+            },
+            {
+              filterText: 'keywordField',
+              text: 'keywordField ASC',
+            },
+            {
+              filterText: 'keywordField',
+              text: 'keywordField DESC',
+            },
+            {
+              filterText: 'keywordField',
+              text: 'keywordField NULLS FIRST',
+            },
+            {
+              filterText: 'keywordField',
+              text: 'keywordField NULLS LAST',
+            },
+          ].map(attachTriggerCommand)
         );
+      });
+      it('suggests subsequent column after comma', async () => {
+        const { assertSuggestions } = await setup();
+
+        await assertSuggestions('from a | sort keywordField, /', [
+          ...expectedFieldSuggestions,
+          ...expectedFunctionSuggestions,
+        ]);
+        await assertSuggestions('from a | sort keywordField, doubl/', [
+          ...expectedFieldSuggestions,
+          ...expectedFunctionSuggestions,
+        ]);
         await assertSuggestions(
-          'from a | sort column, /',
-          [...getFieldNamesByType('any')].map((field) => `${field} `)
+          'from a | sort keywordField, doubleField/',
+          [
+            {
+              filterText: 'doubleField',
+              text: 'doubleField, ',
+            },
+            {
+              filterText: 'doubleField',
+              text: 'doubleField | ',
+            },
+            {
+              filterText: 'doubleField',
+              text: 'doubleField ASC',
+            },
+            {
+              filterText: 'doubleField',
+              text: 'doubleField DESC',
+            },
+            {
+              filterText: 'doubleField',
+              text: 'doubleField NULLS FIRST',
+            },
+            {
+              filterText: 'doubleField',
+              text: 'doubleField NULLS LAST',
+            },
+          ].map(attachTriggerCommand)
         );
       });
     });
@@ -30,76 +110,134 @@ describe('autocomplete.suggest', () => {
       test('suggests all modifiers on first space', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | sort stringField /', [
-          'ASC ',
-          'DESC ',
-          'NULLS FIRST ',
-          'NULLS LAST ',
-          ',',
-          '| ',
-        ]);
+        await assertSuggestions(
+          'from a | sort stringField /',
+          ['ASC', 'DESC', 'NULLS FIRST', 'NULLS LAST', ', ', '| '].map(attachTriggerCommand)
+        );
       });
 
       test('when user starts to type ASC modifier', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | sort stringField A/', ['ASC ']);
+        await assertSuggestions(
+          'from a | sort stringField A/',
+          ['ASC', 'DESC', 'NULLS FIRST', 'NULLS LAST'].map(attachTriggerCommand)
+        );
+        await assertSuggestions(
+          'from a | sort stringField ASC/',
+          ['ASC NULLS FIRST', 'ASC NULLS LAST', 'ASC, ', 'ASC | '].map(attachTriggerCommand)
+        );
+        await assertSuggestions(
+          'from a | sort stringField asc/',
+          ['asc NULLS FIRST', 'asc NULLS LAST', 'asc, ', 'asc | '].map(attachTriggerCommand)
+        );
       });
 
       test('when user starts to type DESC modifier', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | sort stringField d/', ['DESC ']);
-        await assertSuggestions('from a | sort stringField des/', ['DESC ']);
-        await assertSuggestions('from a | sort stringField DES/', ['DESC ']);
+        await assertSuggestions(
+          'from a | sort stringField D/',
+          ['ASC', 'DESC', 'NULLS FIRST', 'NULLS LAST'].map(attachTriggerCommand)
+        );
+        await assertSuggestions(
+          'from a | sort stringField DESC/',
+          ['DESC NULLS FIRST', 'DESC NULLS LAST', 'DESC, ', 'DESC | '].map(attachTriggerCommand)
+        );
+        await assertSuggestions('from a | sort stringField desc/', [
+          'desc NULLS FIRST',
+          'desc NULLS LAST',
+          'desc, ',
+          'desc | ',
+        ]);
       });
     });
 
-    describe('... [ NULLS FIST / NULLS LAST ]', () => {
-      test('suggests command on first character', async () => {
+    describe('... [ NULLS FIRST / NULLS LAST ]', () => {
+      test('suggests nulls modifier after order modifier + space', async () => {
         const { assertSuggestions } = await setup();
 
         await assertSuggestions('from a | sort stringField ASC /', [
-          'NULLS FIRST ',
-          'NULLS LAST ',
-          ',',
+          'NULLS FIRST',
+          'NULLS LAST',
+          ', ',
           '| ',
         ]);
       });
 
       test('when user starts to type NULLS modifiers', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | sort stringField N/', ['NULLS FIRST ', 'NULLS LAST ']);
-        await assertSuggestions('from a | sort stringField null/', ['NULLS FIRST ', 'NULLS LAST ']);
+        // @TODO check for replacement range
+        await assertSuggestions('from a | sort stringField N/', [
+          'ASC',
+          'DESC',
+          'NULLS FIRST',
+          'NULLS LAST',
+        ]);
+        await assertSuggestions('from a | sort stringField null/', [
+          'ASC',
+          'DESC',
+          'NULLS FIRST',
+          'NULLS LAST',
+        ]);
         await assertSuggestions('from a | sort stringField nulls/', [
-          'NULLS FIRST ',
-          'NULLS LAST ',
+          'ASC',
+          'DESC',
+          'NULLS FIRST',
+          'NULLS LAST',
         ]);
         await assertSuggestions('from a | sort stringField nulls /', [
-          'NULLS FIRST ',
-          'NULLS LAST ',
+          'ASC',
+          'DESC',
+          'NULLS FIRST',
+          'NULLS LAST',
         ]);
       });
 
       test('when user types NULLS FIRST', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | sort stringField NULLS F/', ['NULLS FIRST ']);
-        await assertSuggestions('from a | sort stringField NULLS FI/', ['NULLS FIRST ']);
+        await assertSuggestions(
+          'from a | sort stringField NULLS F/',
+          [
+            'ASC',
+            'DESC',
+            { text: 'NULLS LAST', rangeToReplace: { start: 27, end: 34 } },
+            { text: 'NULLS FIRST', rangeToReplace: { start: 27, end: 34 } },
+          ].map(attachTriggerCommand)
+        );
+        await assertSuggestions(
+          'from a | sort stringField NULLS FI/',
+          [
+            'ASC',
+            'DESC',
+            { text: 'NULLS LAST', rangeToReplace: { start: 27, end: 35 } },
+            { text: 'NULLS FIRST', rangeToReplace: { start: 27, end: 35 } },
+          ].map(attachTriggerCommand)
+        );
       });
 
       test('when user types NULLS LAST', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | sort stringField NULLS L/', ['NULLS LAST ']);
-        await assertSuggestions('from a | sort stringField NULLS LAS/', ['NULLS LAST ']);
+        await assertSuggestions(
+          'from a | sort stringField NULLS L/',
+          ['ASC', 'DESC', 'NULLS LAST', 'NULLS FIRST'].map(attachTriggerCommand)
+        );
+        await assertSuggestions(
+          'from a | sort stringField NULLS LAS/',
+          ['ASC', 'DESC', 'NULLS LAST', 'NULLS FIRST'].map(attachTriggerCommand)
+        );
       });
 
       test('after nulls are entered, suggests comma or pipe', async () => {
         const { assertSuggestions } = await setup();
 
-        await assertSuggestions('from a | sort stringField NULLS LAST /', [',', '| ']);
+        await assertSuggestions(
+          'from a | sort stringField NULLS LAST /',
+          [', ', '| '].map(attachTriggerCommand)
+        );
       });
     });
   });

diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts
@@ -17,7 +17,7 @@ import { groupingFunctionDefinitions } from '../../definitions/grouping';
 import * as autocomplete from '../autocomplete';
 import type { ESQLCallbacks } from '../../shared/types';
 import type { EditorContext, SuggestionRawDefinition } from '../types';
-import { TIME_SYSTEM_PARAMS, getSafeInsertText } from '../factories';
+import { TIME_SYSTEM_PARAMS, TRIGGER_SUGGESTION_COMMAND, getSafeInsertText } from '../factories';
 import { getFunctionSignatures } from '../../definitions/helpers';
 import { ESQLRealField } from '../../validation/types';
 import {
@@ -348,3 +348,17 @@ export const setup = async (caret = '/') => {
     assertSuggestions,
   };
 };
+
+/**
+ * Attaches the trigger command to an expected suggestion to make
+ * sure the suggestions menu will be opened when the suggestion is accepted.
+ */
+export const attachTriggerCommand = (
+  s: string | PartialSuggestionWithText
+): PartialSuggestionWithText =>
+  typeof s === 'string'
+    ? {
+        text: s,
+        command: TRIGGER_SUGGESTION_COMMAND,
+      }
+    : { ...s, command: TRIGGER_SUGGESTION_COMMAND };
diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts
@@ -30,6 +30,7 @@ import {
   PartialSuggestionWithText,
   TIME_PICKER_SUGGESTION,
   setup,
+  attachTriggerCommand,
 } from './__tests__/helpers';
 import { METADATA_FIELDS } from '../shared/constants';
 import { ESQL_COMMON_NUMERIC_TYPES, ESQL_STRING_TYPES } from '../shared/esql_types';
@@ -287,7 +288,10 @@ describe('autocomplete', () => {
       'from a | grok key/',
       getFieldNamesByType(ESQL_STRING_TYPES).map((name) => `${name} `)
     );
-    testSuggestions('from a | grok keywordField/', []);
+    testSuggestions(
+      'from a | grok keywordField/',
+      ['keywordField ', 'textField '].map(attachTriggerCommand)
+    );
   });
 
   describe('dissect', () => {
@@ -327,7 +331,10 @@ describe('autocomplete', () => {
       'from a | dissect key/',
       getFieldNamesByType(ESQL_STRING_TYPES).map((name) => `${name} `)
     );
-    testSuggestions('from a | dissect keywordField/', []);
+    testSuggestions(
+      'from a | dissect keywordField/',
+      ['keywordField ', 'textField '].map(attachTriggerCommand)
+    );
   });
 
   describe('limit', () => {
@@ -699,16 +706,6 @@ describe('autocomplete', () => {
      * NOTE: Monaco uses an Invoke trigger kind when the show suggestions action is triggered (e.g. accepting the "FROM" suggestion)
      */
 
-    const attachTriggerCommand = (
-      s: string | PartialSuggestionWithText
-    ): PartialSuggestionWithText =>
-      typeof s === 'string'
-        ? {
-            text: s,
-            command: TRIGGER_SUGGESTION_COMMAND,
-          }
-        : { ...s, command: TRIGGER_SUGGESTION_COMMAND };
-
     const attachAsSnippet = (s: PartialSuggestionWithText): PartialSuggestionWithText => ({
       ...s,
       asSnippet: true,