fix(edit-content): refresh History, Comments and Reference Pages after save #36617 - #36901
fix(edit-content): refresh History, Comments and Reference Pages after save #36617#36901adrianjm-dotCMS wants to merge 4 commits into
Conversation
…r save #36617 The sidebar's History, Comments and Reference Pages stayed stale after a save/publish, requiring a manual page reload. Two root causes: - The refresh effects lived on DotEditContentSidebarComponent, which the layout's @if destroys and recreates, taking the effects with it. Moved them into store-level withHooks({onInit}) in withActivities and withInformation, matching the existing lock/workflow/history features. - withHistory only invalidated its memo on `${identifier}:${languageId}`, but a save mints a new inode under the SAME identifier and locale, so loadVersions never re-fired. Full-screen rendered an empty list (initializeExistingContent had already cleared it); the dialog host, which never navigates, rendered a stale one. Added two invalidation signals: a cleared list (status INIT, ignored mid-reload so it cannot fetch the identifier being left behind) and a live-inode baseline that only advances when not viewing a historical version, so browsing versions and returning from them does not refetch. Both effects now read the isSidebarOpen leaf instead of uiState(): every writer replaces that slice wholesale, so depending on the object refetched on unrelated UI changes, including the view flip loadVersions performs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @adrianjm-dotCMS's task in 6m 44s —— View job Code Review
I traced all three root causes against the store and reload flow. The reasoning holds up and the invalidation logic is carefully guarded. Findings below. New IssuesNone blocking. Observations (non-blocking)
Verified correct
Nice write-up in the PR body; the per-host symptom table made the two root causes easy to verify. |
…+ e2e #36617 The feature specs verify the store effects against a synthetic store with no component mounted, so they cannot catch the regression this fix addresses: the effects used to live on DotEditContentSidebarComponent, which the layout's @if destroys and recreates. Moving them back would leave every feature spec green. Two layers close that gap, both parameterized over the full-screen and dialog hosts since the bug had a different face in each: - Integration (dot-edit-content.layout.component.spec.ts): real store mounted in the real layout with the sidebar as a MockComponent. Asserts the fetches happen while the sidebar is not rendered at all, that they survive the sidebar being destroyed and recreated, and that a save minting a new inode refreshes without any re-initialization (the dialog path). Verified these fail when the store hooks are removed. - E2E (apps/dotcms-ui-e2e/.../sidebar/history-refresh.spec.ts): publishes and comments through the UI and asserts History/Comments update with no reload, in full-screen and in the dialog (reached via the relationship field's "New Content", which needs no page/template fixture). Verified against the pre-fix code: full-screen reproduced the empty list (expected 2, got 0) and the dialog reproduced the stale list (expected 2, got 1). Dialog comments are deliberately not covered — the comment form is hidden for content opened as 'new', which is the only mode that entry point offers. It needs the UVE pencil flow and a page fixture; documented in the spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…not-refreshing-when
`nx format:check` — the CI's format-test goal — flagged this file. The lint-staged hook ran `nx format:write` on it at commit time but left it unformatted, so the check only surfaced in the pipeline. Formatting only; no test logic changed. 2186 tests still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixes #36617
Proposed Changes
The sidebar's History, Comments and Reference Pages stayed stale after a save/publish and needed a manual page reload. There were two independent root causes.
The refresh effects lived on a component that gets destroyed. They sat on
DotEditContentSidebarComponent, which the layout's@ifdestroys and recreates — taking the effects with it. Moved into store-levelwithHooks({ onInit })inwithActivitiesandwithInformation, so they live as long as the store does. This matches the existinglock/workflow/historyfeatures.withHistorydid not recognise a newly minted version. Its memo only invalidated on${identifier}:${languageId}, but a save mints a new inode under the same identifier and locale, soloadVersionsnever re-fired. The symptom differed per host, which is why it looked like two separate bugs:Screen.Recording.2026-08-05.at.3.47.52.PM.mov
initializeExistingContentversions: [])Added two invalidation signals: a cleared list (
status === INIT, ignored mid-reload so it cannot fetch the identifier being left behind) and a live-inode baseline that only advances when not viewing a historical version, so browsing versions — and returning from them — does not refetch.store.uiState(), i.e. the whole slice. Every writer replaces that object wholesale, so the effects refetched on unrelated UI changes — including theviewfliploadVersionsperforms internally, costing 3–4 redundant requests per save. Now they read thestore.uiState.isSidebarOpen()leaf.Checklist
Additional Info
Tests: 2180 passing across 111 suites in
edit-content; lint and typecheck clean. New coverage includes publish-in-dialog, publish-while-comparing, the mid-reload case, historical/compare round trips, and a regression test for the signal-granularity fix (verified it fails against the previous code).One existing test changed expectation —
should not reload anything when only the version inode changesasserted that moving only the inode does not reload, which was literally the bug. Rewritten asshould reload versions but not push publish history when the live inode moves. This is safe because browsing versions always goes throughloadVersionContent, which setsisViewingHistoricalVersion; a bare inode move only happens on save/publish. The browsing cases are now covered by dedicated tests that set the realistic state.Known follow-ups, deliberately out of scope:
clearVersions()/resetVersions()/clearPushPublishHistory()/resetPushPublishHistory()inhistory.feature.tshave no production call sites — dead code worth removing separately.🤖 Generated with Claude Code