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.

Sign up for GitHub

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

Jump to bottom

[Viz Builder] State validation before dispatching and loading #2351

Merged
merged 3 commits into from
Sep 29, 2022
Merged

[Viz Builder] State validation before dispatching and loading #2351

merged 3 commits into from
Sep 29, 2022

Conversation

abbyhu2000
Copy link
Member

Description

Validate style and visualization state schema before dispatching state updates when loading wizard. Error messages are shown via toast.

Signed-off-by: abbyhu2000 abigailhu2000@gmail.com

Issues Resolved

resolves #1882

Check List

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

@abbyhu2000 abbyhu2000 self-assigned this Sep 14, 2022
@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2022 •
edited
Loading

Codecov Report

Merging #2351 (5668bad) into main (806e96c) will decrease coverage by 0.00%.
The diff coverage is 31.25%.

@@            Coverage Diff             @@
##             main    #2351      +/-   ##
==========================================
- Coverage   66.74%   66.73%   -0.01%     
==========================================
  Files        3194     3195       +1     
  Lines       60803    60818      +15     
  Branches     9238     9241       +3     
==========================================
+ Hits        40583    40588       +5     
- Misses      18011    18021      +10     
  Partials     2209     2209
Impacted Files Coverage Δ
...blic/application/utils/use/use_saved_wizard_vis.ts 2.50% <0.00%> (-0.84%) ⬇️
...ublic/application/utils/wizard_state_validation.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@abbyhu2000 abbyhu2000 marked this pull request as ready for review September 15, 2022 02:26
@abbyhu2000 abbyhu2000 requested a review from a team as a code owner September 15, 2022 02:26
@abbyhu2000 abbyhu2000 force-pushed the complete_unhappy_load_path branch 2 times, most recently from 5bd163c to f0163e3 Compare September 16, 2022 16:28
ashwin-pc
ashwin-pc previously approved these changes Sep 17, 2022
View reviewed changes
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

thanks for the changes, this looks good :)

ananzh
ananzh previously approved these changes Sep 26, 2022
View reviewed changes
Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

LGTM

ashwin-pc
ashwin-pc reviewed Sep 26, 2022
View reviewed changes
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

Sorry i missed a few points in the initial review.

src/plugins/wizard/public/application/utils/wizard_state_validation.test.ts Show resolved Hide resolved
src/plugins/wizard/public/application/utils/wizard_state_validation.ts Show resolved Hide resolved
src/plugins/wizard/public/application/utils/wizard_state_validation.ts Outdated Show resolved Hide resolved
kavilla
kavilla reviewed Sep 27, 2022
View reviewed changes
yarn.lock
version "1.0.5"
resolved "https://registry.yarnpkg.com/@amoo-miki/vega-forced-csp-compliant-interpreter/-/vega-forced-csp-compliant-interpreter-1.0.5.tgz#49970be9b00ca7e45ced0617fbf373c77a28aab4"
integrity sha512-lfeU77lVoUbSCC6N1ywdKg+I6K08xpkd82TLon+LebtKyC8aLCe7P5Dd/89zAPyFwRyobKftHu8z0xpV7R7a4Q==
"vega-interpreter@npm:@amoo-miki/vega-forced-csp-compliant-interpreter@1.0.6":
Copy link
Member

Choose a reason for hiding this comment

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

@AMoo-Miki is this ok?

kavilla
kavilla reviewed Sep 27, 2022
View reviewed changes
package.json Show resolved Hide resolved
@abbyhu2000
State validation before dispatching
a60c2da 
Validate style and visualization state schema before dispatching state updates
when loading wizard. Show error via toast.

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
@abbyhu2000
Fixes and add unit tests
2d99206 
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
@abbyhu2000 abbyhu2000 changed the title [D&D] State validation before dispatching and loading [Viz Builder] State validation before dispatching and loading Sep 28, 2022
@abbyhu2000
Add PR to changelog.md
5668bad 
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
ashwin-pc
ashwin-pc approved these changes Sep 28, 2022
View reviewed changes
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

Nice! Ship-it 🚢

@ananzh ananzh added the v2.4.0 'Issues and PRs related to version v2.4.0' label Sep 28, 2022
@abbyhu2000 abbyhu2000 merged commit 0279588 into opensearch-project:main Sep 29, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 29, 2022
@abbyhu2000 @github-actions
[Viz Builder] State validation before dispatching and loading (#2351)
c2e420d 
* State validation before dispatching

Validate style and visualization state schema before dispatching state updates
when loading wizard. Show error via toast.

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

* Fixes and add unit tests

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

* Add PR to changelog.md

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
(cherry picked from commit 0279588)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Sep 29, 2022
Merged
ashwin-pc pushed a commit that referenced this pull request Sep 29, 2022
@abbyhu2000 @ashwin-pc
[Viz Builder] State validation before dispatching and loading (#2351)
b031f81 
* State validation before dispatching

Validate style and visualization state schema before dispatching state updates
when loading wizard. Show error via toast.

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

* Fixes and add unit tests

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

* Add PR to changelog.md

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
(cherry picked from commit 0279588)
kavilla pushed a commit that referenced this pull request Sep 29, 2022
@opensearch-trigger-bot @abbyhu2000
[Viz Builder] State validation before dispatching and loading (#2351) (…
a68302b 
…#2449)

* State validation before dispatching
  * Validate style and visualization state schema before dispatching state updates
     when loading wizard. Show error via toast.
* Fixes and add unit tests
* Add PR to changelog.md

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
(cherry picked from commit 0279588)

Co-authored-by: Qingyang(Abby) Hu <abigailhu2000@gmail.com>
@AMoo-Miki AMoo-Miki added the enhancement New feature or request label Nov 5, 2022
@AMoo-Miki AMoo-Miki changed the title [Viz Builder] State validation before dispatching and loading [Viz Builder] Add state validation before dispatching and loading Nov 5, 2022
sipopo pushed a commit to sipopo/OpenSearch-Dashboards that referenced this pull request Dec 16, 2022
@abbyhu2000 @sipopo
[Viz Builder] State validation before dispatching and loading (opense…
fc62286 
…arch-project#2351)

* State validation before dispatching

Validate style and visualization state schema before dispatching state updates
when loading wizard. Show error via toast.

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

* Fixes and add unit tests

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

* Add PR to changelog.md

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
Signed-off-by: Sergey V. Osipov <sipopo@yandex.ru>
@ashwin-pc ashwin-pc changed the title [Viz Builder] Add state validation before dispatching and loading [Viz Builder] State validation before dispatching and loading Jan 10, 2023
@abbyhu2000 abbyhu2000 deleted the complete_unhappy_load_path branch June 30, 2023 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kavilla kavilla kavilla left review comments

@ananzh ananzh ananzh approved these changes

@ashwin-pc ashwin-pc ashwin-pc approved these changes

@joshuarrrr joshuarrrr Awaiting requested review from joshuarrrr joshuarrrr is a code owner

Assignees

@abbyhu2000 abbyhu2000

Labels
backport 2.x enhancement New feature or request v2.4.0 'Issues and PRs related to version v2.4.0'
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[D&D] Complete unhappy load paths (error handling flows)
7 participants
@abbyhu2000 @codecov-commenter @joshuarrrr @kavilla @ashwin-pc @ananzh @AMoo-Miki