[Data Explorer][Discover 2.0] Fix display for index pattern without a default time field #4821

ananzh · 2023-08-25T16:58:03Z

Description

This PR is blocked by #4816
Please see the 2nd commit. Need to rebase after #4816 is merged.

Before

After
no date picker in the top nav
only show hits if index pattern has no time field

Issues Resolved

#4820

Check List

codecov · 2023-08-25T17:17:03Z

Codecov Report

Merging #4821 (df20e7c) into main (72ced4d) will increase coverage by 0.03%.
Report is 1 commits behind head on main.
The diff coverage is n/a.

❗ Current head df20e7c differs from pull request most recent head c9073dd. Consider uploading reports for the commit c9073dd to get more accurate results

@@            Coverage Diff             @@
##             main    #4821      +/-   ##
==========================================
+ Coverage   66.39%   66.43%   +0.03%     
==========================================
  Files        3397     3398       +1     
  Lines       64805    64893      +88     
  Branches    10359    10359              
==========================================
+ Hits        43028    43110      +82     
- Misses      19218    19304      +86     
+ Partials     2559     2479      -80

Flag	Coverage Δ
Linux_1	`34.85% <ø> (ø)`
Linux_2	`55.15% <ø> (ø)`
Linux_3	`44.24% <ø> (ø)`
Linux_4	`35.06% <ø> (+0.14%)`	⬆️
Windows_1	`34.87% <ø> (ø)`
Windows_2	`55.12% <ø> (ø)`
Windows_3	`44.24% <ø> (+<0.01%)`	⬆️
Windows_4	`35.06% <ø> (+0.14%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 22 files with indirect coverage changes

ashwin-pc

You accidentally removed a flag that allows the search bar queries to be saved. the rest of the PR looks good :)

ashwin-pc · 2023-08-31T01:18:35Z

src/plugins/discover/public/application/view_components/canvas/top_nav.tsx

  return (
    <TopNavMenu
      appName={PLUGIN_ID}
      config={topNavLinks}
      showSearchBar
-      showSaveQuery
+      showDatePicker={showDatePicker}


Suggested change

showDatePicker={showDatePicker}

showDatePicker={showDatePicker}

showSaveQuery

AMoo-Miki · 2023-08-31T04:11:10Z

src/plugins/discover/public/application/components/doc_views/context_app.tsx

@@ -100,7 +105,7 @@ export function ContextApp({
          onSort={() => {}}
          sort={sort}
          rows={rows}
-          displayTimeColumn={true}
+          displayTimeColumn={displayTimeColumn || true}


Since displayTimeColumn is boolean too, this will be true always. I don't think that was your intention.

The displayTimeColumn constant could technically be undefined if indexPattern is undefined or if uiSettings.get(DOC_HIDE_TIME_COLUMN_SETTING, false) returns a falsy value and indexPattern?.isTimeBased() is undefined.

I have updated it to be

const displayTimeColumn = useMemo( () => !!(!uiSettings.get(DOC_HIDE_TIME_COLUMN_SETTING, false) && indexPattern?.isTimeBased()), [indexPattern, uiSettings] );

to convert the value of the expression to a boolean, turning undefined or any other falsy values to false. And I changed this line to displayTimeColumn={displayTimeColumn.

AMoo-Miki · 2023-08-31T04:12:57Z

src/plugins/discover/public/application/view_components/canvas/discover_chart_container.tsx

-  if (!hits || !bucketInterval || !chartData) {
-    // TODO: handle better
-    return null;
-  }


Just double checking. We are good with not having bucketInterval or chartData?

hits itself is enough. hits = 0 will always has no chartData or bucketInterval.

AMoo-Miki · 2023-08-31T04:13:27Z

src/plugins/discover/public/application/view_components/canvas/discover_table.tsx

@@ -93,7 +103,7 @@ export const DiscoverTable = ({ history }: Props) => {
      onSort={onSetSort}
      sort={sort}
      rows={rows}
-      displayTimeColumn={true}
+      displayTimeColumn={displayTimeColumn || true}


Doesn't seem right.

same as above.

…ield Issue Resolve opensearch-project#4820 Signed-off-by: ananzh <[email protected]>

opensearch-trigger-bot · 2023-08-31T23:58:02Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-4821-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 83cd7f629c3211a9d5d216b80cc5c3db40adefbc
# Push it to GitHub
git push --set-upstream origin backport/backport-4821-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-4821-to-2.x.

…ield (#4821) Issue Resolve #4820 Signed-off-by: ananzh <[email protected]> (cherry picked from commit 83cd7f6) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…ield (#4821) (#4901) Issue Resolve #4820 (cherry picked from commit 83cd7f6) Signed-off-by: ananzh <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

ananzh requested review from kavilla, seanneumann, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, kristenTian, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, BSFishy and curq as code owners August 25, 2023 16:58

github-actions bot added the distinguished-contributor label Aug 25, 2023

ananzh assigned ashwin-pc Aug 25, 2023

ananzh added data explorer Issues related to the Data Explorer project discover for discover reinvent v2.10.0 and removed distinguished-contributor labels Aug 25, 2023

ashwin-pc added the backport 2.x label Aug 30, 2023

ananzh force-pushed the feature/data-explorer-no-timefield branch from 92207a1 to b6c449f Compare August 30, 2023 21:30

ananzh changed the base branch from feature/data-explorer to main August 30, 2023 21:31

ananzh force-pushed the feature/data-explorer-no-timefield branch 4 times, most recently from cb95e58 to 99d7b03 Compare August 30, 2023 23:17

ashwin-pc added the Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry label Aug 30, 2023

ashwin-pc reviewed Aug 31, 2023

View reviewed changes

AMoo-Miki reviewed Aug 31, 2023

View reviewed changes

ananzh force-pushed the feature/data-explorer-no-timefield branch from d17d9bf to df20e7c Compare August 31, 2023 16:40

[Data Explorer]Fix display for index pattern without a default time f…

c9073dd

…ield Issue Resolve opensearch-project#4820 Signed-off-by: ananzh <[email protected]>

ananzh force-pushed the feature/data-explorer-no-timefield branch from df20e7c to c9073dd Compare August 31, 2023 18:25

ashwin-pc approved these changes Aug 31, 2023

View reviewed changes

abbyhu2000 approved these changes Aug 31, 2023

View reviewed changes

ashwin-pc merged commit 83cd7f6 into opensearch-project:main Aug 31, 2023

opensearch-trigger-bot bot added the failed backport label Aug 31, 2023

ashwin-pc added backport 2.x and removed backport 2.x labels Sep 1, 2023

opensearch-trigger-bot bot mentioned this pull request Sep 1, 2023

[Backport 2.x] [Data Explorer][Discover 2.0] Fix display for index pattern without a default time field #4901

Merged

ashwin-pc removed the failed backport label Sep 2, 2023

ananzh mentioned this pull request Sep 21, 2023

[BUG][Data Explorer] Large string error on data explorer #5068

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Data Explorer][Discover 2.0] Fix display for index pattern without a default time field #4821

[Data Explorer][Discover 2.0] Fix display for index pattern without a default time field #4821

ananzh commented Aug 25, 2023

codecov bot commented Aug 25, 2023 •

edited

Loading

ashwin-pc left a comment

ashwin-pc Aug 31, 2023

ananzh Aug 31, 2023

AMoo-Miki Aug 31, 2023

ananzh Aug 31, 2023

AMoo-Miki Aug 31, 2023

ananzh Aug 31, 2023

AMoo-Miki Aug 31, 2023

ananzh Aug 31, 2023

opensearch-trigger-bot bot commented Aug 31, 2023

	showDatePicker={showDatePicker}
	showDatePicker={showDatePicker}
	showSaveQuery

[Data Explorer][Discover 2.0] Fix display for index pattern without a default time field #4821

[Data Explorer][Discover 2.0] Fix display for index pattern without a default time field #4821

Conversation

ananzh commented Aug 25, 2023

Description

Issues Resolved

Check List

codecov bot commented Aug 25, 2023 • edited Loading

Codecov Report

ashwin-pc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Aug 31, 2023

codecov bot commented Aug 25, 2023 •

edited

Loading