fix: specs swaps correctly reflected in state

[markov00]

Summary

This PR fixes the wrong state caused by swapping specs props between existing components.

The root cause is the specComponentFactory.
The issue appears in the following situation:

• we have two components with respective ids A and B
• this got parsed and stored in our state
• the consumer switch the ids of the two components: the first component (previously with id A) becomes the one with id B and the second component (previously with id B) becomes A.

Because we are processing these specs sequentially following the React life cycle, the first component calls the following part of the specComponentFactory

if (prevId && prevId !== props.id) {
    removeSpec(prevId);
}
upsertSpec(SpecInstance);

this removes the prevId spec (the A for the first component) and then add the spec B to the store (1).
Then it's time for the second component. The id of the component has changed so it follows the same path as the previous one: removes the spec B (its saved previous id) and then upsert the spec with id A, but the removed spec B is not the old one, but a new one added in the step (1), the new one provided by the first component.

The first time we send anupsertSpec action to the state, the reducer cleans up the specs array. Because of this, it is not needed to call first the removeSpec with the prevId, if that is different from the current one. We still clean and re-add all the specs on upserting.

fix #868

Checklist

• Unit tests were updated or added to match the most common scenarios

[codecov-io]

Codecov Report

Merging #901 (936b5dd) into master (ab1af38) will increase coverage by 0.55%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #901      +/-   ##
==========================================
+ Coverage   69.61%   70.17%   +0.55%     
==========================================
  Files         340      356      +16     
  Lines       10983    10612     -371     
  Branches     2286     2150     -136     
==========================================
- Hits         7646     7447     -199     
+ Misses       3323     3082     -241     
- Partials       14       83      +69     

Flag	Coverage Δ
unittests	?

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

Impacted Files	Coverage Δ
src/state/chart_state.ts	85.50% <ø> (-2.00%)	⬇️
src/state/spec_factory.ts	94.73% <100.00%> (-1.70%)	⬇️
src/utils/dimensions.ts	71.42% <0.00%> (-28.58%)	⬇️
src/chart_types/xy_chart/utils/panel_utils.ts	72.72% <0.00%> (-19.59%)	⬇️
src/utils/data_generators/data_generator.ts	45.45% <0.00%> (-9.10%)	⬇️
...t/state/selectors/compute_small_multiple_scales.ts	86.95% <0.00%> (-8.70%)	⬇️
src/components/portal/utils.ts	21.42% <0.00%> (-7.15%)	⬇️
.../partition_chart/state/selectors/compute_legend.ts	79.06% <0.00%> (-5.72%)	⬇️
...state/selectors/get_internal_is_tooltip_visible.ts	75.00% <0.00%> (-5.00%)	⬇️
...hart/state/selectors/get_elements_at_cursor_pos.ts	90.47% <0.00%> (-4.77%)	⬇️
... and 194 more

---

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 ab1af38...936b5dd. Read the comment docs.

[nickofthyme]

Changes LGTM, verified the issue is resolved locally.

[rshen91]

Looks great! I really like the additional tests - they make it really clear

[markov00]

🎉 This PR is included in version 24.1.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀