[Discover] Rename default column in the advanced settings #114100

majagrubic · 2021-10-06T13:08:50Z

Summary

This small fix renames the default column in "Advanced Settings" from _source to Document. Since Document doesn't really exist, we need to continue treating it as special case in the code.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

majagrubic · 2021-10-07T13:46:19Z

@elasticmachine merge upstream

majagrubic · 2021-10-07T14:36:13Z

@elsticmachine merge master

majagrubic · 2021-10-07T14:36:59Z

@elasticmachine merge upstream

majagrubic · 2021-10-11T07:19:38Z

@elasticmachine merge upstream

src/plugins/discover/server/ui_settings.ts

majagrubic · 2021-10-12T09:54:41Z

@elasticmachine merge upstream

kertal · 2021-10-13T07:48:17Z

@elasticmachine merge upstream

src/plugins/discover/server/ui_settings.ts

kertal · 2021-10-13T17:10:51Z

LGTM, will finish reviewing tomorrow

elasticmachine · 2021-10-14T07:38:22Z

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

kertal

LGTM! 👍 nice cleanup, just title and description need to be updated, since it's no longer an rename, more a cleanup 🧹

kertal · 2021-10-14T08:32:15Z

src/plugins/discover/public/application/apps/main/utils/get_state_defaults.ts

@@ -19,6 +23,9 @@ function getDefaultColumns(savedSearch: SavedSearch, config: IUiSettingsClient)
  if (savedSearch.columns && savedSearch.columns.length > 0) {
    return [...savedSearch.columns];
  }
+  if (config.get(SEARCH_FIELDS_FROM_SOURCE) && isEqual(config.get(DEFAULT_COLUMNS_SETTING), [])) {


this could be simplified by just checking for the length of the the array provided by DEFAULT_COLUMNS_SETTING to be 0 ... however not mandatory

timroes · 2021-10-14T09:27:52Z

src/plugins/discover/server/ui_settings.ts

    description: i18n.translate('discover.advancedSettings.defaultColumnsText', {
-      defaultMessage: 'Columns displayed by default in the Discovery tab',
+      defaultMessage:
+        'Columns displayed by default in the Discover app. If empty, `Document` column will be displayed.',


I feel naming it "Document column" might not be the best user-facing naming. What do you think about maybe rephrasing this to: "If empty, a summary of the document will be displayed."

kibanamachine · 2021-10-14T11:53:49Z

💚 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
`discover`	327.7KB	327.9KB	+156.0B

History

💚 Build #160429 succeeded 31a4861
💚 Build #160328 succeeded 82fe18b
💚 Build #159873 succeeded 9487a9f
💔 Build #159854 failed 372141e
💔 Build #159829 failed 8dead76

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

…4100) * [Discover] Rename default column in the advanced settings * Fix eslint * Rename default column to an empty string * Fix typo * Fix default column filtering * Update comment * Make an empty array a default columns * Improve functional test * Wording change Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…115006) * [Discover] Rename default column in the advanced settings * Fix eslint * Rename default column to an empty string * Fix typo * Fix default column filtering * Update comment * Make an empty array a default columns * Improve functional test * Wording change Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…4100) * [Discover] Rename default column in the advanced settings * Fix eslint * Rename default column to an empty string * Fix typo * Fix default column filtering * Update comment * Make an empty array a default columns * Improve functional test * Wording change Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

bhavyarm · 2021-11-01T13:00:01Z

I was confused about which change to look for while release testing. Adding a screenshot for future QA team :) Thanks!

Maja Grubic added 2 commits October 6, 2021 15:08

[Discover] Rename default column in the advanced settings

c77231d

Fix eslint

04e434d

majagrubic marked this pull request as ready for review October 7, 2021 12:31

majagrubic requested a review from kertal October 7, 2021 12:31

majagrubic added v8.0.0 backport-v7.16.0 release_note:enhancement and removed backport-v7.16.0 labels Oct 7, 2021

majagrubic requested a review from a team as a code owner October 7, 2021 12:32

majagrubic added v7.16.0 Feature:Discover Discover Application labels Oct 7, 2021

Merge branch 'master' into discover-default-column

475d9ec

Merge branch 'master' into discover-default-column

b60533c

Merge branch 'master' into discover-default-column

eacea46

kertal reviewed Oct 11, 2021

View reviewed changes

src/plugins/discover/server/ui_settings.ts Outdated Show resolved Hide resolved

Maja Grubic added 5 commits October 11, 2021 20:28

Merge branch 'master' into discover-default-column

80426da

Rename default column to an empty string

e5b146e

Fix typo

8dead76

Fix default column filtering

f9c8356

Update comment

372141e

Merge branch 'master' into discover-default-column

9487a9f

majagrubic requested a review from kertal October 12, 2021 14:05

Merge branch 'master' into discover-default-column

82fe18b

kertal reviewed Oct 13, 2021

View reviewed changes

src/plugins/discover/server/ui_settings.ts Outdated Show resolved Hide resolved

kertal reviewed Oct 13, 2021

View reviewed changes

src/plugins/discover/server/ui_settings.ts Outdated Show resolved Hide resolved

Maja Grubic added 2 commits October 13, 2021 14:47

Make an empty array a default columns

4f80cbe

Improve functional test

31a4861

kertal added the Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) label Oct 14, 2021

kertal self-requested a review October 14, 2021 08:44

kertal approved these changes Oct 14, 2021

View reviewed changes

timroes reviewed Oct 14, 2021

View reviewed changes

Wording change

d7f0d60

majagrubic merged commit 586682a into elastic:master Oct 14, 2021

majagrubic deleted the discover-default-column branch October 14, 2021 13:49

majagrubic mentioned this pull request Oct 14, 2021

[7.x] [Discover] Rename default column in the advanced settings (#114100) #115006

Merged

timroes added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels Oct 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Rename default column in the advanced settings #114100

[Discover] Rename default column in the advanced settings #114100

majagrubic commented Oct 6, 2021 •

edited

Loading

majagrubic commented Oct 7, 2021

majagrubic commented Oct 7, 2021

majagrubic commented Oct 7, 2021

majagrubic commented Oct 11, 2021

majagrubic commented Oct 12, 2021

kertal commented Oct 13, 2021

kertal commented Oct 13, 2021

elasticmachine commented Oct 14, 2021

kertal left a comment

kertal Oct 14, 2021

timroes Oct 14, 2021

kibanamachine commented Oct 14, 2021

bhavyarm commented Nov 1, 2021

[Discover] Rename default column in the advanced settings #114100

[Discover] Rename default column in the advanced settings #114100

Conversation

majagrubic commented Oct 6, 2021 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

majagrubic commented Oct 7, 2021

majagrubic commented Oct 7, 2021

majagrubic commented Oct 7, 2021

majagrubic commented Oct 11, 2021

majagrubic commented Oct 12, 2021

kertal commented Oct 13, 2021

kertal commented Oct 13, 2021

elasticmachine commented Oct 14, 2021

kertal left a comment

Choose a reason for hiding this comment

kertal Oct 14, 2021

Choose a reason for hiding this comment

timroes Oct 14, 2021

Choose a reason for hiding this comment

kibanamachine commented Oct 14, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

bhavyarm commented Nov 1, 2021

majagrubic commented Oct 6, 2021 •

edited

Loading