Normalize shared universe objectview identities#27
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7bf082946
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return { | ||
| ...objectview, | ||
| id: nextId, | ||
| }; |
There was a problem hiding this comment.
Rewrite group references when objectview ids change
When a generated group objectview is normalized here, member objectviews keep their old group value because only relship endpoint refs are remapped. Group membership is resolved by comparing ov.group === group.id (for example in cxModelView.findObjectViewsByGroup), so loading a modelview where a group id equals its objectRef will rename the group but leave its children pointing at the stale id, detaching the group in the diagram. Please apply the same idMap rewrite to objectview fields that store objectview ids, at least group.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR introduces a normalization step for metis.models[].modelviews[].objectviews[].id so objectview IDs are unique (and no longer collide with objectRef) when loading/updating shared-universe state, and updates relationship-view endpoint references accordingly. This fits into the shared universe data pipeline by ensuring objectview identity collisions don’t break shared modelview collections and downstream GoJS behaviors.
Changes:
- Added
normalizeModelviewObjectviewIdentities(...)to generate stable, modelview-scoped objectview IDs and rewrite relshipview endpoint refs. - Applied normalization across shared-universe metis update/load paths (including legacy state hydration and
setUniverseState/setUniversePhData). - Added unit tests covering normalization behavior and reducer load actions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/sharedUniverse/universeSlice.ts |
Adds and wires in objectview ID normalization during metis ingestion/updates. |
src/sharedUniverse/universeSlice.test.mjs |
Adds unit tests for normalization behavior and load-path integration. |
src/reducers/reducer.js |
Applies normalization when legacy reducers load/merge metis into phData. |
| const relshipviews: any[] = Array.isArray(modelview?.relshipviews) ? modelview.relshipviews : []; | ||
| const normalizedRelshipviews = idMap.size | ||
| ? relshipviews.map((relshipview) => { | ||
| const fromobjviewRef = idMap.get(relshipview?.fromobjviewRef); | ||
| const toobjviewRef = idMap.get(relshipview?.toobjviewRef); | ||
| if (!fromobjviewRef && !toobjviewRef) return relshipview; | ||
| modelviewDidChange = true; | ||
| return { | ||
| ...relshipview, | ||
| ...(fromobjviewRef ? { fromobjviewRef } : {}), | ||
| ...(toobjviewRef ? { toobjviewRef } : {}), | ||
| }; | ||
| }) | ||
| : relshipviews; | ||
|
|
||
| if (!modelviewDidChange) return modelview; | ||
| modelDidChange = true; | ||
| didChange = true; | ||
| return { | ||
| ...modelview, | ||
| objectviews: normalizedObjectviews, | ||
| ...(Array.isArray(modelview?.relshipviews) ? { relshipviews: normalizedRelshipviews } : {}), | ||
| }; |
| { | ||
| id: 'view-2', | ||
| objectviews: [ | ||
| { id: 'object-1', objectRef: 'object-1', loc: '100 100' }, | ||
| { id: 'object-2', objectRef: 'object-2', loc: '200 200' }, | ||
| ], |
| assert.equal(view2.relshipviews[0].fromobjviewRef, view2.objectviews[0].id); | ||
| assert.equal(view2.relshipviews[0].toobjviewRef, view2.objectviews[1].id); |
Summary
Verification
npm testnpm run build