[Discover][Lens] Removes the dataview dependency from the text based mode #158531

kertal · 2023-05-26T06:39:46Z

Summary

This PR:

make the text based languages to work with adhoc dataviews and not permanent dataviews. The text based languages will not be related with dataviews
enables the timepicker always for text based languages
the timepicker is disabled if the index pattern doesn't have an @timestamp field
the timepicker is enabled if the index pattern has an @timestamp field
the timepicker is enabled if the index pattern doesn't have an @timestamp field but there is a dirty state (user is writing the query and hasn't hit the update button. We do that to give the user the ability to change the timepicker with the query
An info text has been added to the editor footer to inform the users about the @timestamp existence

The timepicker in the disabled state needs to have a disabled status text (All time) but this is not possible atm. I have created an issue to eui elastic/eui#6814 to add this property. This is going to be tackled before the 8.9 FF but we don't want to block this PR

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

…-fix'

…ddon

stratoula · 2023-05-29T07:12:43Z

src/plugins/unified_histogram/public/chart/chart.tsx

@@ -136,7 +136,7 @@ export function Chart({
    !chart.hidden &&
    dataView.id &&
    dataView.type !== DataViewType.ROLLUP &&
-    dataView.isTimeBased()
+    (isPlainRecord || (!isPlainRecord && dataView.isTimeBased()))


ℹ️ This was a bug, the chart in text based languages should render regardless if the dataview is timeBased or not as we don't show the histogram but the Lens suggestions

stratoula · 2023-05-29T07:31:20Z

test/functional/apps/discover/group3/_sidebar.ts

@@ -404,7 +404,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
        await PageObjects.discover.waitUntilSidebarHasLoaded();

        expect(await PageObjects.discover.getSidebarAriaDescription()).to.be(
-          '1 popular field. 53 available fields. 0 empty fields. 3 meta fields.'


ℹ️ this needs to change as adhoc dataviews do not store information for the popular fields

elasticmachine · 2023-05-29T08:46:35Z

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

davismcphee

Pulled and tested locally, and it's working well overall! I left a few comments and questions on the code, but mainly I encountered two things while testing:

After reloading the page, the back button causes a warning toast about the data view ID when going back to a query that generates a new ad hoc data view. This is probably just the standard behaviour when an ad hoc data view isn't in the history state, but I wonder if we should do anything about it here:

I suspect this one might already exist in main, but I haven't checked yet. It seems that clicking into the text-based editor while the page is still loading will cause the text cursor to get messed up:

src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx

src/plugins/discover/public/application/main/utils/get_data_view_by_text_based_query_lang.ts

x-pack/plugins/lens/public/datasources/text_based/utils.ts

packages/kbn-text-based-editor/src/editor_footer.tsx

stratoula · 2023-05-30T06:02:27Z

I suspect this one might already exist in main, but I haven't checked yet. It seems that clicking into the text-based editor while the page is still loading will cause the text cursor to get messed up:

Yes this happens in main too, I have it on my list to see what is going wrong here

stratoula · 2023-05-30T06:44:44Z

After reloading the page, the back button causes a warning toast about the data view ID when going back to a query that generates a new ad hoc data view. This is probably just the standard behaviour when an ad hoc data view isn't in the history state, but I wonder if we should do anything about it here:

Good catch! I fixed it, we don't need these warnings for the text based mode. We had already silenced one in the past

davismcphee

Thanks for the changes, looks like the warning toast is fixed! LGTM 👍

kibana-ci · 2023-05-31T07:38:53Z

💚 Build Succeeded

Buildkite Build
Commit: f0a3559

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	459	460	+1

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
`@kbn/text-based-editor`	13	14	+1
`textBasedLanguages`	16	17	+1
total			+2

Async chunks

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

id	before	after	diff
`discover`	421.2KB	421.6KB	+434.0B
`lens`	1.2MB	1.2MB	-131.0B
`textBasedLanguages`	67.9KB	68.6KB	+759.0B
`unifiedHistogram`	43.4KB	43.4KB	+58.0B
`unifiedSearch`	211.2KB	212.0KB	+755.0B
total			+1.8KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/text-based-editor`	14	15	+1
`textBasedLanguages`	16	17	+1
total			+2

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	416	420	+4
total			+6

References to deprecated APIs

id	before	after	diff
`discover`	29	24	-5

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	500	504	+4
total			+6

History

💚 Build #130892 succeeded 36d6828
💚 Build #130743 succeeded fd4759b
💔 Build #130453 failed eb32f8f
💔 Build #130406 failed 845bc8c
💔 Build #130390 failed ec57a50
💔 Build #130382 failed ee4a3c7

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @kertal @stratoula

stratoula

As we are going to remove the ESQL entrypoint from Lens and the unified search changes are minimal I am approving on behalf of the visualizations team

stratoula and others added 15 commits May 24, 2023 13:52

Always create adhoc dataviews, add @timestamp info

cc9a00d

Fix FT, no popular fields

0375016

Enable the datepicker for text based

42e380f

Some fixes

0d94682

Fixes

5fdfcd1

Merge branch 'main' into text-based-no-dataviews

3f7d9d8

Initial changes

ee4a3c7

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

ec57a50

…-fix'

Fix tests

5484753

merge

845bc8c

Merge branch 'text-based-no-dataviews' into text-based-no-dataviews-a…

2804c43

…ddon

Fix tests

eb32f8f

Merge branch 'main' into text-based-no-dataviews-addon

fdac14d

Fix FT test

0d2a9bd

Fix transition

c384ecc

stratoula reviewed May 29, 2023

View reviewed changes

Add some unit tests

fd4759b

stratoula reviewed May 29, 2023

View reviewed changes

stratoula assigned kertal and stratoula May 29, 2023

stratoula changed the title ~~[Discover] Text based no dataviews addon~~ [Discover][Lens] Removes the dataview dependency from the text based mode May 29, 2023

stratoula added v8.9.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Feature:Discover Discover Application Feature:Unified search Unified search related tasks labels May 29, 2023

stratoula marked this pull request as ready for review May 29, 2023 08:46

stratoula requested review from a team as code owners May 29, 2023 08:46

stratoula added the Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. label May 29, 2023

davismcphee reviewed May 30, 2023

View reviewed changes

stratoula added 2 commits May 30, 2023 09:26

Address PR comments

79c3fd1

hide the dataview warnings for the text based mode

d9dcaeb

Merge branch 'main' into text-based-no-dataviews-addon

36d6828

davismcphee approved these changes May 31, 2023

View reviewed changes

Merge branch 'main' into text-based-no-dataviews-addon

f0a3559

stratoula approved these changes May 31, 2023

View reviewed changes

stratoula merged commit 3ef8b46 into elastic:main May 31, 2023

kertal mentioned this pull request Jun 2, 2023

[Discover] Opening a text-based saved search causes infinite loading #158893

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover][Lens] Removes the dataview dependency from the text based mode #158531

[Discover][Lens] Removes the dataview dependency from the text based mode #158531

kertal commented May 26, 2023 •

edited by stratoula

Loading

stratoula May 29, 2023 •

edited

Loading

stratoula May 29, 2023

elasticmachine commented May 29, 2023

davismcphee left a comment

stratoula commented May 30, 2023

stratoula commented May 30, 2023

davismcphee left a comment

kibana-ci commented May 31, 2023

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

stratoula left a comment

[Discover][Lens] Removes the dataview dependency from the text based mode #158531

[Discover][Lens] Removes the dataview dependency from the text based mode #158531

Conversation

kertal commented May 26, 2023 • edited by stratoula Loading

Summary

Checklist

stratoula May 29, 2023 • edited Loading

Choose a reason for hiding this comment

stratoula May 29, 2023

Choose a reason for hiding this comment

elasticmachine commented May 29, 2023

davismcphee left a comment

Choose a reason for hiding this comment

stratoula commented May 30, 2023

stratoula commented May 30, 2023

davismcphee left a comment

Choose a reason for hiding this comment

kibana-ci commented May 31, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

stratoula left a comment

Choose a reason for hiding this comment

kertal commented May 26, 2023 •

edited by stratoula

Loading

stratoula May 29, 2023 •

edited

Loading