[Discover 2.0] Fixed recent query and Data Selector styles #7918

ashwin-pc · 2024-08-29T12:54:35Z

Description

This PR contains the following changes:

Fixes recent query to not show duplicates
Fixes recent query styles
Fixed Data selector height issue

Issues Resolved

Screenshot

Screen.Recording.2024-08-29.at.5.55.03.AM.mov

Testing the changes

Changelog

skip

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

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

codecov · 2024-08-29T13:09:52Z

Codecov Report

Attention: Patch coverage is 20.00000% with 20 lines in your changes missing coverage. Please review.

Project coverage is 64.29%. Comparing base (07504f6) to head (2bde337).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...ns/data/public/query/query_string/query_history.ts	20.00%	20 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7918      +/-   ##
==========================================
- Coverage   64.30%   64.29%   -0.01%     
==========================================
  Files        3674     3674              
  Lines       81138    81150      +12     
  Branches    12927    12928       +1     
==========================================
+ Hits        52174    52176       +2     
- Misses      25752    25764      +12     
+ Partials     3212     3210       -2

Flag	Coverage Δ
Linux_1	`30.21% <8.00%> (-0.01%)`	⬇️
Linux_2	`56.19% <ø> (ø)`
Linux_3	`40.61% <20.00%> (+<0.01%)`	⬆️
Linux_4	`31.40% <8.00%> (-0.01%)`	⬇️
Windows_1	`30.22% <8.00%> (-0.01%)`	⬇️
Windows_2	`56.14% <ø> (ø)`
Windows_3	`40.61% <20.00%> (-0.01%)`	⬇️
Windows_4	`31.40% <8.00%> (-0.01%)`	⬇️

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.

sejli

LGTM

sejli · 2024-08-29T15:02:48Z

src/plugins/data/public/query/query_string/query_history.ts

 import { Query, TimeRange } from '../..';

-// Todo: Implement a more advanced QueryHistory class when needed for recent query history
+const MAX_HISTORY_SIZE = 500;


Should we make this a UI_SETTING?

Good call. Fast follow maybe? This was what Abby introduced in her PR but we don't today show any more than 10 to the user anyways, so having this value be user configurable isn't very useful yet. Once we implement more features for the table, then it makes sense adding it as an advanced setting

joshuali925

nothing blocking

joshuali925 · 2024-08-29T17:55:56Z

src/plugins/data/public/query/query_string/language_service/recent_query.tsx

+      id: idx,
+      query: query.query.query,
+      timeRange: query.timeRange,
+      time: moment(query.time).format('MMM D, YYYY HH:mm:ss'),


maybe later, i remember user can config the format when date time is displayed, should this be read from user config?

yeah from ui settings

joshuali925 · 2024-08-29T18:04:20Z

src/plugins/data/public/query/query_string/query_history.ts

+  public getHistory(): any[] {
+    return this.getHistoryKeys()
+      .map((key) => this.storage.get(key))
+      .sort((a, b) => b.time - a.time);


getHistoryKeys already sorts by time, is this sort needed?

Good catch. I added that sort afterwards. This is no longer needed

joshuali925 · 2024-08-29T18:06:19Z

src/plugins/data/public/query/query_string/query_history.ts

 export class QueryHistory {
-  constructor(private readonly storage: DataStorage) {}
+  private changeEmitter: BehaviorSubject<any[]>;


i think the type is just

interface QueryHistoryItem { time: number; query: Query; dateRange?: TimeRange; };

imo i think the type should extend savedquery

kavilla · 2024-08-29T22:27:49Z

src/plugins/data/public/query/query_string/query_history.ts

-    keys.splice(0, 500); // only maintain most recent X;
-    keys.forEach((key) => {
-      this.storage.remove(key);
+  public addQueryToHistory(query: Query, dateRange?: TimeRange): void {


should we just use the LRU library we have?

kavilla · 2024-08-29T22:29:17Z

src/plugins/data/public/ui/query_editor/query_editor.tsx

+            <RecentQueriesTable
+              isVisible={this.state.isRecentQueryVisible && useQueryEditor}
+              queryString={this.queryString}
+              onClickRecentQuery={this.onClickRecentQuery}


how come not just onClick?

opensearch-trigger-bot · 2024-08-29T22:29:22Z

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-7918-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 7f9aabbb02a7fe96f86b007663fe2774ceb6ad98
# Push it to GitHub
git push --set-upstream origin backport/backport-7918-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-7918-to-2.x.

kavilla · 2024-08-29T22:29:45Z

src/plugins/data/public/ui/query_editor/query_editor.tsx

+            <div className="osdQueryEditor__body">{languageEditor.Body()}</div>
+            <RecentQueriesTable
+              isVisible={this.state.isRecentQueryVisible && useQueryEditor}
+              queryString={this.queryString}


i think it can access the service with the gettersetters

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> (cherry picked from commit 7f9aabb) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…7927) (cherry picked from commit 7f9aabb) Signed-off-by: Ashwin P Chandran <ashwinpc@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>

[Discover 2.0] Fixed recent query and Data Selector styles

2bde337

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

github-actions bot added distinguished-contributor Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Aug 29, 2024

ashwin-pc added backport 2.x discover for discover reinvent v2.17.0 labels Aug 29, 2024

sejli approved these changes Aug 29, 2024

View reviewed changes

joshuali925 approved these changes Aug 29, 2024

View reviewed changes

sejli merged commit 7f9aabb into opensearch-project:main Aug 29, 2024
75 of 76 checks passed

kavilla reviewed Aug 29, 2024

View reviewed changes

opensearch-trigger-bot bot added the failed backport label Aug 29, 2024

kavilla reviewed Aug 29, 2024

View reviewed changes

kavilla added backport 2.x and removed backport 2.x labels Aug 29, 2024

opensearch-trigger-bot bot mentioned this pull request Aug 29, 2024

[Backport 2.x] [Discover 2.0] Fixed recent query and Data Selector styles #7927

Merged

ashwin-pc removed the failed backport label Sep 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover 2.0] Fixed recent query and Data Selector styles #7918

[Discover 2.0] Fixed recent query and Data Selector styles #7918

ashwin-pc commented Aug 29, 2024 •

edited

Loading

codecov bot commented Aug 29, 2024 •

edited

Loading

sejli left a comment

sejli Aug 29, 2024

ashwin-pc Aug 29, 2024

joshuali925 left a comment

joshuali925 Aug 29, 2024

kavilla Aug 29, 2024

joshuali925 Aug 29, 2024

ashwin-pc Aug 29, 2024

joshuali925 Aug 29, 2024

kavilla Aug 29, 2024

kavilla Aug 29, 2024

kavilla Aug 29, 2024

opensearch-trigger-bot bot commented Aug 29, 2024

kavilla Aug 29, 2024

[Discover 2.0] Fixed recent query and Data Selector styles #7918

[Discover 2.0] Fixed recent query and Data Selector styles #7918

Conversation

ashwin-pc commented Aug 29, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

codecov bot commented Aug 29, 2024 • edited Loading

Codecov Report

sejli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuali925 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 29, 2024

Choose a reason for hiding this comment

ashwin-pc commented Aug 29, 2024 •

edited

Loading

codecov bot commented Aug 29, 2024 •

edited

Loading