fix: graph response to collection hierarchy change

kswenson commented 2 months ago

[PT-188117637] Undo of graph response to moving attribute from child to parent collection not working

A couple of issues:

collection parent/child links weren't being cleared in some cases when hierarchy changes occurred
collection cleared its case id map cache unnecessarily in some cases
graph was responding too early to hierarchy change, before DataSet had completed its response

Here we fix the collection issues and add a mechanism for clients to listen for DataSet response to hierarchy changes.

cypress[bot] commented 2 months ago

Test summary

204 • 0 • 29 • 0 • 0

Run details


Project	codap-v3
Status	Passed
Commit	c7a6a6f6bc
Started	Aug 21, 2024 12:04 AM
Ended	Aug 21, 2024 12:13 AM
Duration	08:33 💡
OS	Linux Ubuntu -
Browser	Multiple

View run in Cypress Cloud ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.87%. Comparing base (b6f041c) to head (a868019). Report is 32 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1418 +/- ## ========================================== - Coverage 85.91% 84.87% -1.04% ========================================== Files 533 540 +7 Lines 26079 26755 +676 Branches 7115 7300 +185 ========================================== + Hits 22405 22708 +303 - Misses 3519 3740 +221 - Partials 155 307 +152 ``` | [Flag](https://app.codecov.io/gh/concord-consortium/codap/pull/1418/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | Coverage Δ | | |---|---|---| | [cypress](https://app.codecov.io/gh/concord-consortium/codap/pull/1418/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `71.23% <100.00%> (-1.31%)` | :arrow_down: | | [jest](https://app.codecov.io/gh/concord-consortium/codap/pull/1418/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `53.43% <59.09%> (-0.40%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kswenson commented 2 months ago

👍🏻LGTM Strikes me that this was a subtle bug. I still don't understand what caused the case ids to change.

The case ids are maintained in the groupKeyCaseIds cache. This was being cleared incorrectly for the child-most collection, which resulted in generation of new ids for the cases of the child-most collection.

concord-consortium / codap