Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Allow Save in Top Nav Menu to capture filter and query #6636

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented Apr 25, 2024

Description

Before this PR, can't save when filter or query change.
After this PR:

  1. if the canvas is empty, can not save filter and query updates. This is expected due to empty canvas error The canvas is empty. Add some aggregations before saving
  2. if the canvas is not empty, can save filter and query updates.
vb-save-top-nav.mov
  1. for embedded vis builder in dashboard, when edit vb, update filter and query can also enable save button
embeddable.mov

Issues Resolved

#5645

Testing the changes

We will need to add some cypress tests. Here is the meta issue to track #6635

Changelog

  • fix: [BUG] Allow Save in Top Nav Menu to capture filter and query

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

Copy link
Contributor

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "fix". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

Before this PR, can't save when filter or query change.
After this PR, if the canvas is not empty, can save filter and query updates.

Issue Resolve
opensearch-project#5645

Signed-off-by: Anan Zhuang <[email protected]>
Copy link

codecov bot commented Apr 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 67.73%. Comparing base (d2d410b) to head (e17d6af).
Report is 33 commits behind head on main.

Files Patch % Lines
..._builder/public/application/components/top_nav.tsx 0.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #6636       +/-   ##
===========================================
+ Coverage   32.93%   67.73%   +34.79%     
===========================================
  Files        2260     3403     +1143     
  Lines       45769    66613    +20844     
  Branches     7200    10828     +3628     
===========================================
+ Hits        15075    45119    +30044     
+ Misses      29984    18854    -11130     
- Partials      710     2640     +1930     
Flag Coverage Δ
Linux_1 33.05% <0.00%> (+0.11%) ⬆️
Linux_2 55.60% <ø> (?)
Linux_3 45.24% <ø> (?)
Linux_4 34.93% <ø> (?)
Windows_1 33.07% <0.00%> (?)
Windows_2 55.57% <ø> (?)
Windows_3 45.25% <ø> (?)
Windows_4 34.93% <ø> (?)

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.

Copy link
Contributor

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "fix". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

@@ -3,7 +3,8 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useEffect, useState } from 'react';
import React, { useEffect, useState, useRef } from 'react';
import { isEqual } from 'lodash';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guilty of doing this too. but i think we wanted to. avoid using lodash right?

Copy link
Contributor

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "fix". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

@ananzh ananzh merged commit 7aa7a74 into opensearch-project:main Apr 30, 2024
71 of 74 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 30, 2024
Before this PR, can't save when filter or query change.
After this PR, if the canvas is not empty, can save filter and query updates.

Issue Resolve
#5645

Signed-off-by: Anan Zhuang <[email protected]>
(cherry picked from commit 7aa7a74)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ananzh pushed a commit that referenced this pull request Apr 30, 2024
…6687)

Before this PR, can't save when filter or query change.
After this PR, if the canvas is not empty, can save filter and query updates.

Issue Resolve
#5645


(cherry picked from commit 7aa7a74)

Signed-off-by: Anan Zhuang <[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>
LDrago27 pushed a commit to LDrago27/OpenSearch-Dashboards that referenced this pull request Jun 3, 2024
…rch-project#6636)

Before this PR, can't save when filter or query change.
After this PR, if the canvas is not empty, can save filter and query updates.

Issue Resolve
opensearch-project#5645

Signed-off-by: Anan Zhuang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants