[BUG][Discover] Check if the timestamp is already included to remove duplicate col #6983

ananzh · 2024-06-10T17:06:36Z

Issues Resolved

#6982

Screenshot

Changelog

fix: [Discover] Check if the timestamp is already included to remove duplicate col

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

codecov · 2024-06-10T17:21:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.46%. Comparing base (2c58c99) to head (39fd156).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6983      +/-   ##
==========================================
+ Coverage   67.45%   67.46%   +0.01%     
==========================================
  Files        3442     3443       +1     
  Lines       67816    67829      +13     
  Branches    11027    11035       +8     
==========================================
+ Hits        45742    45761      +19     
+ Misses      19408    19398      -10     
- Partials     2666     2670       +4

Flag	Coverage Δ
Linux_1	`33.08% <ø> (ø)`
Linux_2	`55.11% <ø> (ø)`
Linux_3	`45.20% <100.00%> (+<0.01%)`	⬆️
Linux_4	`34.86% <ø> (ø)`
Windows_1	`33.13% <ø> (+0.02%)`	⬆️
Windows_2	`55.06% <ø> (ø)`
Windows_3	`45.22% <100.00%> (+<0.01%)`	⬆️
Windows_4	`34.86% <ø> (ø)`

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

virajsanghvi · 2024-06-10T17:51:55Z

src/plugins/discover/public/application/components/doc_viewer/doc_viewer_render_tab.tsx

@@ -29,7 +29,7 @@
 */

 import React, { useRef, useEffect } from 'react';
-import { DocViewRenderFn, DocViewRenderProps } from '../../../doc_views/doc_views_types';
+import { DocViewRenderFn, DocViewRenderProps } from '../../doc_views/doc_views_types';


How did this build if this wasn't imported correctly?

I do see a type error when I debug the issue

Cannot find module '../../../doc_views/doc_views_types' or its corresponding type declarations.ts(2307)

I believe this is related to an old discussion #1660, which highlights that the standard build scripts in the project do not fail on or show TypeScript errors. This has led to a situation where TypeScript compilation errors (like the one I encountered) do not cause the build to fail, allowing many such errors to creep into the project unnoticed.

I believe we don't have the bandwidth to address this issue right now, but we should consider implementing a way to prevent new type errors from being added. This will help ensure the problem doesn't grow larger.

CC: @AMoo-Miki @ashwin-pc

Yeah typescript error right now rely on the IDE or us manually running tsc and validating the diff. I have a work in progress PR to help catch this in the CI but its not complete yet. #6586

virajsanghvi · 2024-06-10T17:53:55Z

src/plugins/discover/public/application/components/default_discover_table/helper.tsx

+  return !hideTimeField &&
+    indexPattern.timeFieldName &&
+    !columns.includes(indexPattern.timeFieldName)


nit: consider assigning this to a meaningfully named variable so the reader doesn't have to parse out what this is trying to do

Will think a good name and update

Issue Resolve opensearch-project#6982 Signed-off-by: Anan Zhuang <ananzh@amazon.com>

Signed-off-by: Anan Zhuang <ananzh@amazon.com>

…duplicate col (#6983) * check if the timestamp is already included to remove duplicate col --------- Signed-off-by: Anan Zhuang <ananzh@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 0db5848) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…duplicate col (opensearch-project#6983) * check if the timestamp is already included to remove duplicate col --------- Signed-off-by: Anan Zhuang <ananzh@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

…duplicate col (#6983) * check if the timestamp is already included to remove duplicate col --------- Signed-off-by: Anan Zhuang <ananzh@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 0db5848) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…duplicate col (#6983) (#7399) * check if the timestamp is already included to remove duplicate col --------- (cherry picked from commit 0db5848) Signed-off-by: Anan Zhuang <ananzh@amazon.com> 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> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

…duplicate col (#6983) (#7047) * check if the timestamp is already included to remove duplicate col --------- (cherry picked from commit 0db5848) Signed-off-by: Anan Zhuang <ananzh@amazon.com> 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> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

ananzh requested review from kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, curq, bandinib-amzn, SuZhou-Joe, ruanyl, BionIT, xinruiba, zhyuanqi and mengweieric as code owners June 10, 2024 17:06

github-actions bot added the distinguished-contributor label Jun 10, 2024

opensearch-changeset-bot bot added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Jun 10, 2024

Changeset file for PR opensearch-project#6983 created/updated

0b8d23f

virajsanghvi reviewed Jun 10, 2024

View reviewed changes

bandinib-amzn assigned bandinib-amzn and abbyhu2000 Jun 11, 2024

ananzh and others added 3 commits June 11, 2024 21:11

check if the timestamp is already included to remove duplicate col

6741f09

Issue Resolve opensearch-project#6982 Signed-off-by: Anan Zhuang <ananzh@amazon.com>

Changeset file for PR opensearch-project#6983 created/updated

4100214

fix comment

39fd156

Signed-off-by: Anan Zhuang <ananzh@amazon.com>

ananzh force-pushed the fix-6982 branch from 0b8d23f to 39fd156 Compare June 11, 2024 21:12

ananzh added bug Something isn't working discover for discover reinvent labels Jun 11, 2024

ananzh added backport 2.x v2.16.0 labels Jun 11, 2024

virajsanghvi approved these changes Jun 17, 2024

View reviewed changes

abbyhu2000 approved these changes Jun 17, 2024

View reviewed changes

zhyuanqi approved these changes Jun 17, 2024

View reviewed changes

ananzh merged commit 0db5848 into opensearch-project:main Jun 17, 2024
75 checks passed

opensearch-trigger-bot bot mentioned this pull request Jun 17, 2024

[Backport 2.x] [BUG][Discover] Check if the timestamp is already included to remove duplicate col #7047

Merged

sejli added the backport 2.16 label Jul 23, 2024

opensearch-trigger-bot bot mentioned this pull request Jul 23, 2024

[Backport 2.16] [BUG][Discover] Check if the timestamp is already included to remove duplicate col #7399

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG][Discover] Check if the timestamp is already included to remove duplicate col #6983

[BUG][Discover] Check if the timestamp is already included to remove duplicate col #6983

ananzh commented Jun 10, 2024

codecov bot commented Jun 10, 2024 •

edited

Loading

virajsanghvi Jun 10, 2024

ananzh Jun 11, 2024

ananzh Jun 11, 2024

ashwin-pc Jun 12, 2024

virajsanghvi Jun 10, 2024

ananzh Jun 11, 2024

[BUG][Discover] Check if the timestamp is already included to remove duplicate col #6983

[BUG][Discover] Check if the timestamp is already included to remove duplicate col #6983

Conversation

ananzh commented Jun 10, 2024

Issues Resolved

Screenshot

Changelog

Check List

codecov bot commented Jun 10, 2024 • edited Loading

Codecov Report

virajsanghvi Jun 10, 2024

Choose a reason for hiding this comment

ananzh Jun 11, 2024

Choose a reason for hiding this comment

ananzh Jun 11, 2024

Choose a reason for hiding this comment

ashwin-pc Jun 12, 2024

Choose a reason for hiding this comment

virajsanghvi Jun 10, 2024

Choose a reason for hiding this comment

ananzh Jun 11, 2024

Choose a reason for hiding this comment

codecov bot commented Jun 10, 2024 •

edited

Loading