Skip to content

Commit

Permalink
[8.x] [ES|QL] new pattern for SORT autocomplete (#193595) (#193801)
Browse files Browse the repository at this point in the history 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] new pattern for `SORT` autocomplete
(#193595)](#193595)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Drew
Tate","email":"drew.tate@elastic.co"},"sourceCommit":{"committedDate":"2024-09-23T14:23:23Z","message":"[ES|QL]
new pattern for `SORT` autocomplete (#193595)\n\n## Summary\r\n\r\nPart
of #189662. This PR\r\n- updates
the autocomplete behavior for `SORT` to be in line with
other\r\nfield-list-based experiences like `KEEP`\r\n- introduces a
shared function, `handleFragment`, which is used to\r\nabstract some of
the logic required to support this behavior\r\n- bulks up the `SORT`
tests\r\n- restores the function suggestions which I noticed got lost
in\r\nhttps://github.com//pull/189959\r\n\r\n**Before**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/cad1d073-c010-426f-9628-c0fc6b65eb3c\r\n\r\n**After**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/e148ae58-4430-482c-9f8e-c55779c4d822\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli
<efstratia.kalafateli@elastic.co>","sha":"f450e228b38d317a57d906f6c59f6e69d1dd458d","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","backport:prev-minor","Feature:ES|QL","Team:ESQL"],"number":193595,"url":"https://github.com/elastic/kibana/pull/193595","mergeCommit":{"message":"[ES|QL]
new pattern for `SORT` autocomplete (#193595)\n\n## Summary\r\n\r\nPart
of #189662. This PR\r\n- updates
the autocomplete behavior for `SORT` to be in line with
other\r\nfield-list-based experiences like `KEEP`\r\n- introduces a
shared function, `handleFragment`, which is used to\r\nabstract some of
the logic required to support this behavior\r\n- bulks up the `SORT`
tests\r\n- restores the function suggestions which I noticed got lost
in\r\nhttps://github.com//pull/189959\r\n\r\n**Before**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/cad1d073-c010-426f-9628-c0fc6b65eb3c\r\n\r\n**After**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/e148ae58-4430-482c-9f8e-c55779c4d822\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli
<efstratia.kalafateli@elastic.co>","sha":"f450e228b38d317a57d906f6c59f6e69d1dd458d"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193595","number":193595,"mergeCommit":{"message":"[ES|QL]
new pattern for `SORT` autocomplete (#193595)\n\n## Summary\r\n\r\nPart
of #189662. This PR\r\n- updates
the autocomplete behavior for `SORT` to be in line with
other\r\nfield-list-based experiences like `KEEP`\r\n- introduces a
shared function, `handleFragment`, which is used to\r\nabstract some of
the logic required to support this behavior\r\n- bulks up the `SORT`
tests\r\n- restores the function suggestions which I noticed got lost
in\r\nhttps://github.com//pull/189959\r\n\r\n**Before**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/cad1d073-c010-426f-9628-c0fc6b65eb3c\r\n\r\n**After**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/e148ae58-4430-482c-9f8e-c55779c4d822\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Stratoula Kalafateli
<efstratia.kalafateli@elastic.co>","sha":"f450e228b38d317a57d906f6c59f6e69d1dd458d"}}]}]
BACKPORT-->
  • Loading branch information
@drewdaemon
drewdaemon authored Sep 24, 2024
1 parent 87ac87a commit 64e4646
Show file tree
Hide file tree
Showing 6 changed files with 480 additions and 207 deletions.
206 changes: 172 additions & 34 deletions ...esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
});
});
Expand All @@ -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)
);
});
});
});
Expand Down
16 changes: 15 additions & 1 deletion packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 };
21 changes: 9 additions & 12 deletions packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 64e4646

Please sign in to comment.