Skip to content

[Unified search] Display the solutions recommended queries in the help menu#223362

Merged
stratoula merged 16 commits intoelastic:mainfrom
stratoula:help-menu-recommended-queries
Jun 12, 2025
Merged

[Unified search] Display the solutions recommended queries in the help menu#223362
stratoula merged 16 commits intoelastic:mainfrom
stratoula:help-menu-recommended-queries

Conversation

@stratoula
Copy link
Contributor

@stratoula stratoula commented Jun 11, 2025

Summary

In #221474 we introduced the mechanism to register queries per solution at the editor.

This PR displays these queries in the unified search menu too. In the following screenshot the 2 first queries are the oblt solution registered queries. I also have a max-height just to be sure that this list doesnt get too long now that the solutions can register queries.

image

Note

This change won't display anything in the popover as no solution has registered any queries yet. This PR sets the mechanism though on when they will do so.

How to test

If you want to test it the fastest way is to go to the esql plugin (server side) and add (change the examples as you wish):

this.extensionsRegistry.setRecommendedQueries(
      [
        {
          name: 'Logs count by log level',
          query: 'from logs* | STATS count(*) by log_level',
        },
        {
          name: 'Apache logs counts',
          query: 'from logs-apache_error | STATS count(*)',
        },
        {
          name: 'Another index, not logs',
          query: 'from movies | STATS count(*)',
        },
      ],
      'oblt'
    );

Then go to Discover (solutions mode) and type from logs* and hit the query. Open the popover and check the 2 first recommendations are been suggested.

Checklist

@stratoula stratoula added v9.1.0 v8.19.0 Feature:ES|QL ES|QL related features in Kibana Feature:Unified search Unified search related tasks release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels labels Jun 11, 2025
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
unifiedSearch 113 114 +1

Async chunks

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

id before after diff
unifiedSearch 360.9KB 361.7KB +808.0B
Unknown metric groups

API count

id before after diff
@kbn/esql-validation-autocomplete 189 191 +2
unifiedSearch 151 152 +1
total +3

History

@stratoula stratoula marked this pull request as ready for review June 12, 2025 05:30
@stratoula stratoula requested review from a team as code owners June 12, 2025 05:30
Copy link
Contributor

@sddonne sddonne left a comment

Choose a reason for hiding this comment

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

Looks nice! left two questions :)

try {
// Fetch ESQL solutions recommended queries from the registry
const queriesFromRegistry: RecommendedQuery[] = await http.get(
`/internal/esql_registry/extensions/${activeSolutionId}/${queryForRecommendedQueries}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this route be a constant imported from @kbn/esql-types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure y not 552bf1d

solutionsRecommendedQueries,
]);

const toggleLanguageComponent = useCallback(async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is toggleLanguageComponent callback async?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't and I fixed it here 552bf1d but is irrelevant with my changes

panelProps={{
['data-test-subj']: 'esql-menu-popover',
css: { width: 240 },
css: css`
Copy link
Contributor

Choose a reason for hiding this comment

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

@mbondyra Does this need to be memoized?

Copy link
Contributor

@mbondyra mbondyra Jun 12, 2025

Choose a reason for hiding this comment

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

The rule of thumb would be that if the style has less than 5 lines (and it's the only css tag in the component), it doesn't have to. However, since it doesn't have to be memoized but can be just defined outside, I'd move it outside of component so it gets serialized only once.

const esqlMenuPopoverStyles = css`
  width: 240px;
  max-height: 350px;
  overflow-y: auto;
  ${useEuiScrollBar()};
`

and then here:

Suggested change
css: css`
css: esqlMenuPopoverStyles

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renderESQLPopover();

// Assert that http.get was NOT called
await waitFor(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This assertion has a race condition. httpModule.http.get is called inside a useEffect, meaning that its called after the component has rendered. expect(httpModule.http.get).not.toHaveBeenCalled() is true before useEffect is executed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the implementation 8500e31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed them after discussing async with Nate 375ae6c

renderESQLPopover(stubIndexPattern);

// Assert that http.get was NOT called
await waitFor(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This assertion has a race condition. httpModule.http.get is called inside a useEffect, meaning that its called after the component has rendered. expect(httpModule.http.get).not.toHaveBeenCalled() is true before useEffect is executed.

Copy link
Contributor Author

@stratoula stratoula Jun 12, 2025

Choose a reason for hiding this comment

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

Changed the implementation 8500e31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed them after discussing async with Nate 375ae6c

@stratoula stratoula merged commit 25fe1fa into elastic:main Jun 12, 2025
8 of 10 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.19

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

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.19 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 223362

Questions ?

Please refer to the Backport tool documentation

@stratoula stratoula removed backport:version Backport to applied version labels v8.19.0 labels Jun 12, 2025
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Jun 12, 2025
stratoula added a commit that referenced this pull request Jun 12, 2025
… the help menu" (#223598)

Reverts #223362

I accidentally merged the previous PR, I am so sorry. I am reverting and
open it again to get the presentation team approval
stratoula added a commit that referenced this pull request Jun 13, 2025
…p menu (#223602)

## Summary

Reopening of #223362

In #221474 we introduced the
mechanism to register queries per solution at the editor.

This PR displays these queries in the unified search menu too. In the
following screenshot the 2 first queries are the oblt solution
registered queries. I also have a max-height just to be sure that this
list doesnt get too long now that the solutions can register queries.

<img width="439" alt="image"
src="https://github.com/user-attachments/assets/8ca48b8a-1880-42f3-944f-dd1fb012661f"
/>

### Note

This change won't display anything in the popover as no solution has
registered any queries yet. This PR sets the mechanism though on when
they will do so.

### How to test
If you want to test it the fastest way is to go to the esql plugin
(server side) and add (change the examples as you wish):

```
this.extensionsRegistry.setRecommendedQueries(
      [
        {
          name: 'Logs count by log level',
          query: 'from logs* | STATS count(*) by log_level',
        },
        {
          name: 'Apache logs counts',
          query: 'from logs-apache_error | STATS count(*)',
        },
        {
          name: 'Another index, not logs',
          query: 'from movies | STATS count(*)',
        },
      ],
      'oblt'
    );

```
Then go to Discover (solutions mode) and type `from logs*` and hit the
query. Open the popover and check the 2 first recommendations are been
suggested.

### Checklist

- [ ] [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





<!--ONMERGE {"backportTargets":["8.19"]} ONMERGE-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
iblancof pushed a commit to iblancof/kibana that referenced this pull request Jun 16, 2025
…p menu (elastic#223362)

## Summary

In elastic#221474 we introduced the
mechanism to register queries per solution at the editor.

This PR displays these queries in the unified search menu too. In the
following screenshot the 2 first queries are the oblt solution
registered queries. I also have a max-height just to be sure that this
list doesnt get too long now that the solutions can register queries.

<img width="439" alt="image"
src="https://github.com/user-attachments/assets/8ca48b8a-1880-42f3-944f-dd1fb012661f"
/>

### Note

This change won't display anything in the popover as no solution has
registered any queries yet. This PR sets the mechanism though on when
they will do so.

### How to test
If you want to test it the fastest way is to go to the esql plugin
(server side) and add (change the examples as you wish):

```
this.extensionsRegistry.setRecommendedQueries(
      [
        {
          name: 'Logs count by log level',
          query: 'from logs* | STATS count(*) by log_level',
        },
        {
          name: 'Apache logs counts',
          query: 'from logs-apache_error | STATS count(*)',
        },
        {
          name: 'Another index, not logs',
          query: 'from movies | STATS count(*)',
        },
      ],
      'oblt'
    );

```
Then go to Discover (solutions mode) and type `from logs*` and hit the
query. Open the popover and check the 2 first recommendations are been
suggested.

### Checklist

- [ ] [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: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
iblancof pushed a commit to iblancof/kibana that referenced this pull request Jun 16, 2025
… the help menu" (elastic#223598)

Reverts elastic#223362

I accidentally merged the previous PR, I am so sorry. I am reverting and
open it again to get the presentation team approval
iblancof pushed a commit to iblancof/kibana that referenced this pull request Jun 16, 2025
…p menu (elastic#223602)

## Summary

Reopening of elastic#223362

In elastic#221474 we introduced the
mechanism to register queries per solution at the editor.

This PR displays these queries in the unified search menu too. In the
following screenshot the 2 first queries are the oblt solution
registered queries. I also have a max-height just to be sure that this
list doesnt get too long now that the solutions can register queries.

<img width="439" alt="image"
src="https://github.com/user-attachments/assets/8ca48b8a-1880-42f3-944f-dd1fb012661f"
/>

### Note

This change won't display anything in the popover as no solution has
registered any queries yet. This PR sets the mechanism though on when
they will do so.

### How to test
If you want to test it the fastest way is to go to the esql plugin
(server side) and add (change the examples as you wish):

```
this.extensionsRegistry.setRecommendedQueries(
      [
        {
          name: 'Logs count by log level',
          query: 'from logs* | STATS count(*) by log_level',
        },
        {
          name: 'Apache logs counts',
          query: 'from logs-apache_error | STATS count(*)',
        },
        {
          name: 'Another index, not logs',
          query: 'from movies | STATS count(*)',
        },
      ],
      'oblt'
    );

```
Then go to Discover (solutions mode) and type `from logs*` and hit the
query. Open the popover and check the 2 first recommendations are been
suggested.

### Checklist

- [ ] [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





<!--ONMERGE {"backportTargets":["8.19"]} ONMERGE-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
stratoula added a commit to stratoula/kibana that referenced this pull request Jun 17, 2025
…p menu (elastic#223602)

## Summary

Reopening of elastic#223362

In elastic#221474 we introduced the
mechanism to register queries per solution at the editor.

This PR displays these queries in the unified search menu too. In the
following screenshot the 2 first queries are the oblt solution
registered queries. I also have a max-height just to be sure that this
list doesnt get too long now that the solutions can register queries.

<img width="439" alt="image"
src="https://github.com/user-attachments/assets/8ca48b8a-1880-42f3-944f-dd1fb012661f"
/>

### Note

This change won't display anything in the popover as no solution has
registered any queries yet. This PR sets the mechanism though on when
they will do so.

### How to test
If you want to test it the fastest way is to go to the esql plugin
(server side) and add (change the examples as you wish):

```
this.extensionsRegistry.setRecommendedQueries(
      [
        {
          name: 'Logs count by log level',
          query: 'from logs* | STATS count(*) by log_level',
        },
        {
          name: 'Apache logs counts',
          query: 'from logs-apache_error | STATS count(*)',
        },
        {
          name: 'Another index, not logs',
          query: 'from movies | STATS count(*)',
        },
      ],
      'oblt'
    );

```
Then go to Discover (solutions mode) and type `from logs*` and hit the
query. Open the popover and check the 2 first recommendations are been
suggested.

### Checklist

- [ ] [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

<!--ONMERGE {"backportTargets":["8.19"]} ONMERGE-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 563abe2)

# Conflicts:
#	src/platform/plugins/shared/unified_search/public/search_bar/search_bar.test.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:ES|QL ES|QL related features in Kibana Feature:Unified search Unified search related tasks release_note:skip Skip the PR/issue when compiling release notes v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants