Skip to content

fix: remove old specs with changed ids#167

Merged
emmacunningham merged 2 commits intoelastic:masterfrom
emmacunningham:fix/series-spec-ids
Apr 16, 2019
Merged

fix: remove old specs with changed ids#167
emmacunningham merged 2 commits intoelastic:masterfrom
emmacunningham:fix/series-spec-ids

Conversation

@emmacunningham
Copy link
Copy Markdown
Contributor

@emmacunningham emmacunningham commented Apr 11, 2019

Summary

fix #158

This PR implements the fix for the issue of SpecComponent updates not clearing the previous spec version when the update involves a specId change. Now, we check if the specId has changed on update, and if so, remove the spec by the previous specId from the chartStore.

Before:

broken_bar_spec

broken_line_spec

broken_area_spec

After:

fixed_bar_spec

fixed_line_spec

fixed_area_spec

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Unit tests were updated or added to match the most common scenarios

  • Proper documentation or storybook story was added for features that require explanation or tutorials
  • Each commit follows the convention

@emmacunningham emmacunningham added the wip work in progress label Apr 11, 2019
@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 11, 2019

Codecov Report

Merging #167 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #167   +/-   ##
=======================================
  Coverage   96.26%   96.26%           
=======================================
  Files          36       36           
  Lines        1849     1849           
  Branches      239      239           
=======================================
  Hits         1780     1780           
  Misses         60       60           
  Partials        9        9

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d0a897...c8c2f35. Read the comment docs.

@emmacunningham emmacunningham added :chart Chart element related issue bug Something isn't working and removed wip work in progress labels Apr 15, 2019
@emmacunningham emmacunningham requested a review from markov00 April 15, 2019 20:33
Copy link
Copy Markdown
Collaborator

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Code LGTM.

@emmacunningham emmacunningham merged commit 6c4f705 into elastic:master Apr 16, 2019
markov00 pushed a commit that referenced this pull request Apr 16, 2019
# [3.11.0](v3.10.2...v3.11.0) (2019-04-16)

### Bug Fixes

* remove old specs with changed ids ([#167](#167)) ([6c4f705](6c4f705))

### Features

* allow individual series styling ([#170](#170)) ([c780d98](c780d98))
@markov00
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 3.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Apr 16, 2019
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working :chart Chart element related issue released Issue released publicly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Old series spec configuration remains when spec id is changed

3 participants