Skip to content

fix(store): preserve calendar-object identity on unchanged refetch#8378

Merged
SebastianKrupinski merged 1 commit into
nextcloud:mainfrom
ndo84bw:fix/8367-editor-stale-state
Jun 4, 2026
Merged

fix(store): preserve calendar-object identity on unchanged refetch#8378
SebastianKrupinski merged 1 commit into
nextcloud:mainfrom
ndo84bw:fix/8367-editor-stale-state

Conversation

@ndo84bw
Copy link
Copy Markdown
Contributor

@ndo84bw ndo84bw commented May 21, 2026

Summary

Why

The editor shows stale state after save+reopen because of an object-identity detachment:

  1. The editor opens an event; this.calendarObject points to the store instance calendarObjects[id].
  2. While the editor is open, a background refetch (getEventsFromCalendarInTimeRange) runs and appendOrUpdateCalendarObjectsMutation does calendarObjects[id] = newInstance - even when the refetched object is byte-identical (same ETag). The store entry is now a different instance; the editor still holds the old one.
  3. On save, updateCalendarObject mutates and PUTs the editor's (now detached) object. The server is updated correctly, but the store entry is untouched.
  4. On reopen, getEventByObjectId returns the store entry (the un-mutated instance) -> the editor shows the pre-save state.
  5. The next background sync re-parses the server and replaces the store entry, so it self-heals after ~60s (longer without notify_push). The intermittency depends on whether a refetch happens to land during the open editor session.

The fix skips the replacement when the ETag is unchanged (same ETag means byte-identical content, so nothing is lost), preserving instance identity. Genuinely changed objects (different ETag) are still replaced.

Alternatives considered

  • Update the existing instance in place (copy dav / calendarComponent / ... onto the stored object) instead of skipping. This also preserves identity, but it would overwrite the editor's in-progress, unsaved working copy when that copy shares the calendarComponent, and it needs field-by-field copying that is easy to get wrong. The ETag-skip avoids touching anything precisely when there is nothing new to apply.
  • Verify freshness in getEventByObjectId (its existing TODO: send a HEAD and compare ETags on editor open). That would address the reopen read but not the underlying instance churn, and it adds a request on every open.
  • Out of scope: a genuine multi-client conflict, i.e. a refetch with a different ETag landing while the editor is open. That legitimately replaces the instance and is a separate conflict-resolution concern, not this stale-state bug.

We chose the ETag-skip as the smallest change that fixes the observed cause without risking the editor's unsaved edits.

Tested

Root cause confirmed on the live instance (NC 33.0.3.2 / Calendar 6.4.1) via temporary instrumentation: logging every overwrite, save and reopen-read showed that stale reopens coincide exactly with the editor's calendarObject no longer being identical to the store entry, and that the detaching overwrite always carried the same ETag as the store entry.

Fix verified live on the same instance: changing the location 20 times in a row (edit -> save -> reopen each time) showed the just-saved value every time, with no stale reopen. Reproduced clean in both the full editor and the popup editor (both go through the same EditorMixin -> store path).

Unit tests (added in this PR, npm run test:unit):

  • An unchanged refetch (same ETag) keeps the existing instance (identity preserved).
  • A changed refetch (different ETag) replaces the instance.
  • A new object (not in the store) is appended.
  • Confirmed the unchanged-ETag test fails against the current code.

Copy link
Copy Markdown
Contributor

@SebastianKrupinski SebastianKrupinski left a comment

Choose a reason for hiding this comment

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

Hi @ndo84bw

I am going to block this change, I can not reproduce the issue that you stated in

#8367

Updating information on a event is properly displayed after re-opening the event

@ndo84bw
Copy link
Copy Markdown
Contributor Author

ndo84bw commented May 22, 2026

Hi @SebastianKrupinski

Since it is a timing issue and there were not any step-by-step instructions, it was hard for me to reproduce at first. I have added another video to issue #8367. The trick is to keep the editor open until the background synchronization is complete and only then save your changes.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Nico Donath <ndo84bw@gmx.de>
@SebastianKrupinski SebastianKrupinski force-pushed the fix/8367-editor-stale-state branch from 3105232 to 211044b Compare June 4, 2026 14:44
Copy link
Copy Markdown
Contributor

@SebastianKrupinski SebastianKrupinski left a comment

Choose a reason for hiding this comment

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

I was finally able to reproduce the issue.

Tested the fix, it work as expected.

@SebastianKrupinski
Copy link
Copy Markdown
Contributor

Rebase to head and trimmed the comment a bit.

@SebastianKrupinski
Copy link
Copy Markdown
Contributor

/backport to stable6.4

@backportbot backportbot Bot added the backport-request A backport was requested for this pull request label Jun 4, 2026
@SebastianKrupinski SebastianKrupinski merged commit bec267b into nextcloud:main Jun 4, 2026
41 checks passed
@backportbot backportbot Bot removed the backport-request A backport was requested for this pull request label Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editor shows stale state after save+reopen until background sync settles

2 participants