Skip to content

[Console] Avoid duplicate suggestions in autocomplete#201766

Merged
sabarasaba merged 4 commits intoelastic:mainfrom
sabarasaba:console-avoid_duplicate_suggestions_autocomplete
Nov 29, 2024
Merged

[Console] Avoid duplicate suggestions in autocomplete#201766
sabarasaba merged 4 commits intoelastic:mainfrom
sabarasaba:console-avoid_duplicate_suggestions_autocomplete

Conversation

@sabarasaba
Copy link
Copy Markdown
Member

@sabarasaba sabarasaba commented Nov 26, 2024 •
edited by kibanamachine
Loading

Fixes: #201670

Summary

The duplicate options are the ones being suggested by both the "script processor" and the "script" global defined within console. Previously in the ace editor these were being internally deduped by brace but this isnt a thing with monaco.

This PR fixes the issue by deduping the entries before returning the autocomplete suggestions.

How to test

Using the following command, trigger the autocomplete within the script body:

POST _ingest/pipeline/_simulate
{
  "pipeline": {
    "processors": [
      {
        "script": {
          >>>> here
        }
      }
    ]
  }
}

@sabarasaba sabarasaba added Feature:Console Dev Tools Console Feature Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor labels Nov 26, 2024
@sabarasaba sabarasaba self-assigned this Nov 26, 2024
@sabarasaba sabarasaba marked this pull request as ready for review November 26, 2024 13:26
@sabarasaba sabarasaba requested a review from a team as a code owner November 26, 2024 13:26
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 26, 2024

Pinging @elastic/kibana-management (Team:Kibana Management)

mattkime
mattkime reviewed Nov 27, 2024
View reviewed changes
src/plugins/console/public/lib/autocomplete/engine.js Outdated
}
autoCompleteSet.push(term);
// Make sure we don't have duplicates
if (!_.find(autoCompleteSet, { name: term.name })) {
Copy link
Copy Markdown
Contributor

@mattkime mattkime Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would make more sense to use a Set here so that items can be added without needing to loop over the array. Does the array get big? Do we have any performance concerns with this code?

Copy link
Copy Markdown
Member Author

@sabarasaba sabarasaba Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we can reduce complexity if we avoid the find, I ended up going for a map rather than a set to avoid having to serialize the term with JSON.stringify. It doesnt get big at all afaict, mostly autocomplete terms are in the order of <=20 or so, so should be pretty safe performance wise

@sabarasaba
Copy link
Copy Markdown
Member Author

sabarasaba commented Nov 28, 2024

@elasticmachine merge upstream

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 28, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
console 206.0KB 206.1KB +37.0B

History

cc @sabarasaba

SoniaSanzV
SoniaSanzV reviewed Nov 28, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@SoniaSanzV SoniaSanzV left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, changes LGTM. Just a small comment

src/plugins/console/public/lib/autocomplete/engine.js
autoCompleteSet.push(term);

// Add the term to the autoCompleteSet if it doesn't already exist
if (!autoCompleteSet.has(term.name)) {
Copy link
Copy Markdown
Contributor

@SoniaSanzV SoniaSanzV Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to avoid using a Set instead of a Map? This comparation won't be needed

Copy link
Copy Markdown
Contributor

@SoniaSanzV SoniaSanzV Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I just read [this] explanation that you give to Matt in this same PR. It makes sense. Approving.

@SoniaSanzV SoniaSanzV self-requested a review November 28, 2024 13:34
@sabarasaba sabarasaba merged commit 82de734 into elastic:main Nov 29, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Nov 29, 2024

Starting backport for target branches: 8.16, 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12084340732

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Nov 29, 2024

💔 All backports failed

Status Branch Result
❌ 8.16 Backport failed because of merge conflicts
❌ 8.17 Backport failed because of merge conflicts
❌ 8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 201766

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine mentioned this pull request Dec 2, 2024
Merged
1 task
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 2, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Dec 2, 2024

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 201766 locally

@sabarasaba sabarasaba mentioned this pull request Dec 2, 2024
Merged
sabarasaba added a commit to sabarasaba/kibana that referenced this pull request Dec 2, 2024
@sabarasaba
[Console] Avoid duplicate suggestions in autocomplete (elastic#201766)
d1b668a 
(cherry picked from commit 82de734)

# Conflicts:
#	test/functional/apps/console/_autocomplete.ts
@sabarasaba sabarasaba mentioned this pull request Dec 2, 2024
Merged
sabarasaba added a commit to sabarasaba/kibana that referenced this pull request Dec 2, 2024
@sabarasaba
[Console] Avoid duplicate suggestions in autocomplete (elastic#201766)
7110c5b 
(cherry picked from commit 82de734)

# Conflicts:
#	test/functional/apps/console/_autocomplete.ts
@sabarasaba
Copy link
Copy Markdown
Member Author

sabarasaba commented Dec 2, 2024

💚 All backports created successfully

Status Branch Result
✅ 8.17
✅ 8.16

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@sabarasaba sabarasaba mentioned this pull request Dec 2, 2024
Merged
@sabarasaba
Copy link
Copy Markdown
Member Author

sabarasaba commented Dec 2, 2024

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

sabarasaba added a commit to sabarasaba/kibana that referenced this pull request Dec 2, 2024
@sabarasaba
[Console] Avoid duplicate suggestions in autocomplete (elastic#201766)
52023f7 
(cherry picked from commit 82de734)

# Conflicts:
#	test/functional/apps/console/_autocomplete.ts
sabarasaba added a commit that referenced this pull request Dec 3, 2024
@sabarasaba
[8.17] [Console] Avoid duplicate suggestions in autocomplete (#201766) (
66f5708 
#202481)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Console] Avoid duplicate suggestions in autocomplete
(#201766)](#201766)

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

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

<!--BACKPORT [{"author":{"name":"Ignacio
Rivas","email":"rivasign@gmail.com"},"sourceCommit":{"committedDate":"2024-11-29T11:58:39Z","message":"[Console]
Avoid duplicate suggestions in autocomplete
(#201766)","sha":"82de734e6c7b67d90213b7058b1109f9f375710e","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Console","Team:Kibana
Management","release_note:skip","backport
missing","v9.0.0","backport:prev-minor","v8.16.0","v8.17.0"],"number":201766,"url":"https://github.com/elastic/kibana/pull/201766","mergeCommit":{"message":"[Console]
Avoid duplicate suggestions in autocomplete
(#201766)","sha":"82de734e6c7b67d90213b7058b1109f9f375710e"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201766","number":201766,"mergeCommit":{"message":"[Console]
Avoid duplicate suggestions in autocomplete
(#201766)","sha":"82de734e6c7b67d90213b7058b1109f9f375710e"}},{"branch":"8.16","label":"v8.16.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
sabarasaba added a commit that referenced this pull request Dec 3, 2024
@sabarasaba
[8.16] [Console] Avoid duplicate suggestions in autocomplete (#201766) (
2670a3f 
#202482)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Console] Avoid duplicate suggestions in autocomplete
(#201766)](#201766)

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

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

<!--BACKPORT [{"author":{"name":"Ignacio
Rivas","email":"rivasign@gmail.com"},"sourceCommit":{"committedDate":"2024-11-29T11:58:39Z","message":"[Console]
Avoid duplicate suggestions in autocomplete
(#201766)","sha":"82de734e6c7b67d90213b7058b1109f9f375710e","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Console","Team:Kibana
Management","release_note:skip","backport
missing","v9.0.0","backport:prev-minor","v8.16.0","v8.17.0"],"number":201766,"url":"https://github.com/elastic/kibana/pull/201766","mergeCommit":{"message":"[Console]
Avoid duplicate suggestions in autocomplete
(#201766)","sha":"82de734e6c7b67d90213b7058b1109f9f375710e"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201766","number":201766,"mergeCommit":{"message":"[Console]
Avoid duplicate suggestions in autocomplete
(#201766)","sha":"82de734e6c7b67d90213b7058b1109f9f375710e"}},{"branch":"8.16","label":"v8.16.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
sabarasaba added a commit that referenced this pull request Dec 3, 2024
@sabarasaba
[8.x] [Console] Avoid duplicate suggestions in autocomplete (#201766) (…
5716092 
…#202484)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Console] Avoid duplicate suggestions in autocomplete
(#201766)](#201766)

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

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

<!--BACKPORT [{"author":{"name":"Ignacio
Rivas","email":"rivasign@gmail.com"},"sourceCommit":{"committedDate":"2024-11-29T11:58:39Z","message":"[Console]
Avoid duplicate suggestions in autocomplete
(#201766)","sha":"82de734e6c7b67d90213b7058b1109f9f375710e","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Console","Team:Kibana
Management","release_note:skip","backport
missing","v9.0.0","backport:prev-minor","v8.16.0","v8.17.0"],"number":201766,"url":"https://github.com/elastic/kibana/pull/201766","mergeCommit":{"message":"[Console]
Avoid duplicate suggestions in autocomplete
(#201766)","sha":"82de734e6c7b67d90213b7058b1109f9f375710e"}},"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/201766","number":201766,"mergeCommit":{"message":"[Console]
Avoid duplicate suggestions in autocomplete
(#201766)","sha":"82de734e6c7b67d90213b7058b1109f9f375710e"}},{"branch":"8.16","label":"v8.16.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/202482","number":202482,"state":"OPEN"},{"branch":"8.17","label":"v8.17.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/202481","number":202481,"state":"OPEN"}]}]
BACKPORT-->
@kibanamachine kibanamachine added v8.18.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Dec 3, 2024
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
@sabarasaba @CAWilson94
[Console] Avoid duplicate suggestions in autocomplete (elastic#201766)
f64dec5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattkime mattkime mattkime left review comments

@SoniaSanzV SoniaSanzV SoniaSanzV approved these changes

Assignees

@sabarasaba sabarasaba

Labels

Feature:Console Dev Tools Console Feature release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v8.16.0 v8.16.2 v8.17.0 v8.18.0 v9.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Console] Autocomplete sometimes shows duplicate suggestions

5 participants

@sabarasaba @elasticmachine @kibanamachine @mattkime @SoniaSanzV