Skip to content

Commit 9410da4

Browse files
committed
fix(files): simplify recovery UI and restore canceled link carets
1 parent 166fe55 commit 9410da4

9 files changed

Lines changed: 169 additions & 202 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,7 @@ describe('FileDocProvider', () => {
786786
provider.destroy()
787787
})
788788

789-
it('does not recreate a discarded recovery record during page teardown or destroy', async () => {
789+
it('preserves pending recovery through page teardown and destroy', async () => {
790790
journalStorage.clear()
791791
const scope = { workspaceId: 'workspace-1', userId: 'user-1' }
792792
const { socket, fire } = createSocket(true)
@@ -801,15 +801,21 @@ describe('FileDocProvider', () => {
801801
)
802802
acceptJoin(fire, doc.clientID, 'doc-1')
803803
fire('disconnect')
804-
doc.getText('default').insert(0, 'discard me')
804+
doc.getText('default').insert(0, 'preserve me')
805805
const journal = new PendingFileDocUpdateJournal({ ...scope, fileId: 'file-1' })
806806
await vi.waitFor(async () => expect(await journal.load('doc-1')).not.toBeNull())
807807

808-
await provider.discardPendingChanges()
809808
;(provider as unknown as { handlePageHide: () => void }).handlePageHide()
810809
provider.destroy()
811810

812-
await expect(journal.load('doc-1')).resolves.toBeNull()
811+
const stored = await journal.load('doc-1')
812+
expect(stored).not.toBeNull()
813+
const recovered = new Y.Doc()
814+
Y.applyUpdate(recovered, stored!.recoverySnapshot!)
815+
Y.applyUpdate(recovered, stored!.pendingUpdate)
816+
expect(recovered.getText('default').toString()).toBe('preserve me')
817+
recovered.destroy()
818+
doc.destroy()
813819
})
814820

815821
it('hydrates the complete local draft before reporting a replaced document', async () => {
@@ -890,8 +896,7 @@ describe('FileDocProvider', () => {
890896
updatedAt: Date.now(),
891897
})
892898
const scope = { workspaceId: 'workspace-1', userId: 'user-1' }
893-
const discard = vi.spyOn(PendingFileDocUpdateJournal.prototype, 'discard').mockResolvedValue()
894-
const { socket, fire } = createSocket(true)
899+
const { socket, emit, fire } = createSocket(true)
895900
const doc = new Y.Doc()
896901
const provider = new FileDocProvider(
897902
socket,
@@ -904,12 +909,10 @@ describe('FileDocProvider', () => {
904909

905910
await vi.waitFor(() => expect(provider.joinError).toMatchObject({ code: 'INVALID_UPDATE' }))
906911
expect(doc.getText('default').toString()).toBe('')
907-
await provider.discardPendingChanges()
908-
expect(discard).toHaveBeenCalledWith('doc-1')
912+
expect(emit.mock.calls.some(([event]) => event === FILE_DOC_EVENTS.UPDATE)).toBe(false)
909913
provider.destroy()
910914
doc.destroy()
911915
load.mockRestore()
912-
discard.mockRestore()
913916
})
914917

915918
it('ignores an obsolete schema rejection when recovery finishes after reconnecting', async () => {

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,6 @@ export class FileDocProvider extends ObservableV2<FileDocProviderEvents> {
170170
private updateFlushInProgress = false
171171
private recoveryApplied = false
172172
private recoveryQueued = false
173-
private recoveryDocId: string | null = null
174-
private pendingChangesDiscarded = false
175173
private beforeUnloadProtected = false
176174
private readonly journal: PendingFileDocUpdateJournal | null
177175
private readonly journalLoad: ReturnType<PendingFileDocUpdateJournal['load']>
@@ -434,18 +432,14 @@ export class FileDocProvider extends ObservableV2<FileDocProviderEvents> {
434432
}
435433

436434
if (recovered !== null && !this.recoveryApplied) {
437-
this.recoveryDocId = recovered.docId
438435
const validationDoc = new Y.Doc()
439436
try {
440437
if (recovered.recoverySnapshot) {
441438
Y.applyUpdate(validationDoc, recovered.recoverySnapshot)
442439
}
443440
Y.applyUpdate(validationDoc, recovered.pendingUpdate)
444441
} catch {
445-
this.failFatally(
446-
'The local recovery copy is damaged. Download the current draft before discarding it.',
447-
'INVALID_UPDATE'
448-
)
442+
this.failFatally('The local recovery copy is damaged.', 'INVALID_UPDATE')
449443
return
450444
} finally {
451445
validationDoc.destroy()
@@ -456,10 +450,7 @@ export class FileDocProvider extends ObservableV2<FileDocProviderEvents> {
456450
}
457451
Y.applyUpdate(this.doc, recovered.pendingUpdate, RECOVERY_ORIGIN)
458452
} catch {
459-
this.failFatally(
460-
'The local recovery copy could not be restored. Download the current draft before discarding it.',
461-
'INVALID_UPDATE'
462-
)
453+
this.failFatally('The local recovery copy could not be restored.', 'INVALID_UPDATE')
463454
return
464455
}
465456
this.recoveryApplied = true
@@ -760,16 +751,13 @@ export class FileDocProvider extends ObservableV2<FileDocProviderEvents> {
760751
? Y.mergeUpdates([this.inFlightUpdate.update, update])
761752
: update
762753
const saved = await this.journal?.save(docId, journalUpdate, Y.encodeStateAsUpdate(this.doc))
763-
if (this.disposed || this.fatal || this.pendingChangesDiscarded) {
764-
if (!this.pendingChangesDiscarded) this.queuePendingUpdate(update)
754+
if (this.disposed || this.fatal) {
755+
this.queuePendingUpdate(update)
765756
return
766757
}
767758
if (saved?.status === 'limit-exceeded') {
768759
this.queuePendingUpdate(update)
769-
this.failFatally(
770-
'Local edits exceeded the safe recovery limit; download your draft before reloading',
771-
'PENDING_UPDATE_LIMIT'
772-
)
760+
this.failFatally('Local edits exceeded the safe recovery limit.', 'PENDING_UPDATE_LIMIT')
773761
return
774762
}
775763
const durableUpdate = saved?.pendingUpdate ?? update
@@ -881,7 +869,6 @@ export class FileDocProvider extends ObservableV2<FileDocProviderEvents> {
881869
}
882870

883871
private persistPendingSnapshot(): Promise<void> | undefined {
884-
if (this.pendingChangesDiscarded) return
885872
const update = this.pendingJournalUpdate()
886873
const docId = this.docId()
887874
if (!update || !docId || !this.journal) return
@@ -901,7 +888,6 @@ export class FileDocProvider extends ObservableV2<FileDocProviderEvents> {
901888
if (typeof window === 'undefined') return
902889
const shouldProtect =
903890
!this.disposed &&
904-
!this.pendingChangesDiscarded &&
905891
(this.pendingUpdateBatch.length > 0 ||
906892
this.inFlightUpdate !== null ||
907893
this.updateFlushInProgress)
@@ -911,17 +897,6 @@ export class FileDocProvider extends ObservableV2<FileDocProviderEvents> {
911897
else window.removeEventListener('beforeunload', this.handleBeforeUnload)
912898
}
913899

914-
/** Remove the stale recovery record before deliberately loading a replacement document. */
915-
discardPendingChanges(): Promise<void> {
916-
this.pendingChangesDiscarded = true
917-
this.clearUpdateTimers()
918-
this.pendingUpdateBatch = []
919-
this.inFlightUpdate = null
920-
this.updateBeforeUnloadProtection()
921-
const docId = this.recoveryDocId ?? this.docId()
922-
return docId ? (this.journal?.discard(docId) ?? Promise.resolve()) : Promise.resolve()
923-
}
924-
925900
private clearBufferedMessages(): void {
926901
this.bufferedMessages = []
927902
this.bufferedMessageBytes = 0

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/pending-update-journal.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,14 +176,14 @@ describe('PendingFileDocUpdateJournal', () => {
176176
await expect(subject.load()).resolves.toMatchObject({ docId: 'doc-4' })
177177
})
178178

179-
it('discards only the selected document identity', async () => {
179+
it('clears only the acknowledged document identity', async () => {
180180
const subject = journal()
181181
const oldUpdate = updateWith('old')
182182
const currentUpdate = updateWith('current')
183183
await subject.save('old-doc', oldUpdate, oldUpdate)
184184
await subject.save('current-doc', currentUpdate, currentUpdate)
185185

186-
await subject.discard('old-doc')
186+
await subject.clear('old-doc', oldUpdate)
187187

188188
await expect(subject.load('old-doc')).resolves.toBeNull()
189189
await expect(subject.load('current-doc')).resolves.toMatchObject({ docId: 'current-doc' })

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/pending-update-journal.ts

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -178,27 +178,14 @@ export class PendingFileDocUpdateJournal {
178178
)
179179
}
180180

181-
/** Deliberately abandon one recovery identity after the user has preserved its local draft. */
182-
discard(docId: string): Promise<void> {
183-
return this.enqueue(
184-
() =>
185-
updateValue<unknown>(this.key, (value) =>
186-
record(liveDocuments(value, Date.now()).filter((document) => document.docId !== docId))
187-
),
188-
undefined,
189-
true
190-
)
191-
}
192-
193-
private enqueue<T>(operation: () => Promise<T>, fallback: T, rethrow = false): Promise<T> {
181+
private enqueue<T>(operation: () => Promise<T>, fallback: T): Promise<T> {
194182
const result = this.mutationQueue.then(operation, operation)
195183
this.mutationQueue = result.then(
196184
() => undefined,
197185
() => undefined
198186
)
199187
return result.catch((error) => {
200188
logger.warn('Failed to persist pending file edits', { error })
201-
if (rethrow) throw error
202189
return fallback
203190
})
204191
}

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,9 @@ import {
1717
} from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/paste-admission'
1818
import { LoadedRichMarkdownEditor } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor'
1919

20-
const { collaborationRef, uploadFile, saveBlob } = vi.hoisted(() => ({
20+
const { collaborationRef, uploadFile } = vi.hoisted(() => ({
2121
collaborationRef: { current: null as unknown },
2222
uploadFile: vi.fn(),
23-
saveBlob: vi.fn(),
2423
}))
2524

2625
vi.mock('next/navigation', () => ({
@@ -32,7 +31,6 @@ vi.mock('@/hooks/queries/workspace-files', () => ({
3231
useUploadWorkspaceFile: () => ({ mutateAsync: uploadFile }),
3332
}))
3433
vi.mock('@/hooks/use-add-to-chat', () => ({ useAddToChat: () => vi.fn() }))
35-
vi.mock('@/lib/uploads/client/download', () => ({ saveBlob }))
3634
vi.mock('@/hooks/use-file-content-source', () => ({
3735
useFileContentSource: () => ({ resolveImageSrc: (src: string) => src }),
3836
}))
@@ -103,7 +101,6 @@ const onChange = vi.fn()
103101
const onEditSource = vi.fn()
104102
const onClientAutosaveChange = vi.fn()
105103
const onSaveShortcut = vi.fn()
106-
const onDownloadDraft = vi.fn()
107104
const onSuspendedRender = vi.fn()
108105
const pendingRender = new Promise<void>(() => {})
109106

@@ -122,7 +119,6 @@ function SuspendAfterEditor({ active }: SuspendAfterEditorProps) {
122119
class FakeFileDocProvider {
123120
synced = false
124121
joinError: JoinFileDocError | null = null
125-
discardPendingChanges = vi.fn(() => Promise.resolve())
126122
private readonly listeners = new Map<string, Set<(value: unknown) => void>>()
127123

128124
on(event: string, listener: (value: unknown) => void) {
@@ -182,7 +178,6 @@ async function render(
182178
onEditSource={onEditSource}
183179
onClientAutosaveChange={onClientAutosaveChange}
184180
onSaveShortcut={options.onSaveShortcut ?? onSaveShortcut}
185-
onDownloadDraft={onDownloadDraft}
186181
/>
187182
<SuspendAfterEditor active={options.suspend ?? false} />
188183
</Suspense>
@@ -261,9 +256,13 @@ describe('loaded rich editor lifecycle', () => {
261256
expect(editor.isEditable).toBe(true)
262257
expect(editor.view.dom.getAttribute('aria-readonly')).toBe('false')
263258
expect(container.textContent).not.toContain('Reconnecting…')
259+
expect(container.querySelector('[role="status"]')).toBeNull()
260+
expect(container.querySelector('[role="alert"]')).toBeNull()
261+
expect(toast.warning).not.toHaveBeenCalled()
262+
expect(toast.info).not.toHaveBeenCalled()
264263
})
265264

266-
it('keeps a revoked unacknowledged draft visible, read-only, and downloadable', async () => {
265+
it('keeps revoked pending edits visible and read-only without draft-management prompts', async () => {
267266
const provider = new FakeFileDocProvider()
268267
const doc = new Y.Doc()
269268
doc.getMap(FILE_DOC_SEED.configMap).set(FILE_DOC_SEED.flag, true)
@@ -294,7 +293,13 @@ describe('loaded rich editor lifecycle', () => {
294293
expect(editor.view.dom.closest('.hidden')).toBeNull()
295294
expect(container.textContent).not.toContain('stale opening snapshot')
296295
expect(container.textContent).not.toContain('Reconnecting…')
297-
expect(container.textContent).toContain('Download local draft')
296+
expect(container.querySelector('[role="status"]')?.textContent).toBe(
297+
'You no longer have edit access to this document.'
298+
)
299+
expect(container.querySelector('button')).toBeNull()
300+
expect(container.querySelector('[role="alert"], [role="dialog"]')).toBeNull()
301+
expect(toast.warning).not.toHaveBeenCalled()
302+
expect(toast.info).not.toHaveBeenCalled()
298303
})
299304

300305
it('shows stored content read-only when collaboration fails before the first sync', async () => {
@@ -326,7 +331,7 @@ describe('loaded rich editor lifecycle', () => {
326331
})
327332

328333
it.each(['DOCUMENT_REPLACED', 'PENDING_UPDATE_LIMIT', 'INVALID_UPDATE'])(
329-
'keeps a local draft downloadable before offering a destructive reload for %s',
334+
'preserves pending edits with only a passive status for %s',
330335
async (code) => {
331336
const provider = new FakeFileDocProvider()
332337
const doc = new Y.Doc()
@@ -350,21 +355,15 @@ describe('loaded rich editor lifecycle', () => {
350355
})
351356
)
352357

353-
const buttons = [...container.querySelectorAll('button')]
354-
const download = buttons.find((button) => button.textContent === 'Download local draft')
355-
expect(download).toBeDefined()
356-
expect(buttons.some((button) => button.textContent === 'Discard draft')).toBe(true)
357-
await act(async () => download?.click())
358-
expect(onDownloadDraft).not.toHaveBeenCalled()
359-
expect(saveBlob).toHaveBeenCalledOnce()
360-
const downloaded = saveBlob.mock.calls[0][0] as Blob
361-
const downloadedText = await new Promise<string>((resolve, reject) => {
362-
const reader = new FileReader()
363-
reader.onload = () => resolve(String(reader.result))
364-
reader.onerror = () => reject(reader.error)
365-
reader.readAsText(downloaded)
366-
})
367-
expect(downloadedText).toContain('preserved local change')
358+
expect(container.querySelector('[role="status"]')?.textContent).toBe(
359+
'Live editing is unavailable.'
360+
)
361+
expect(container.querySelector('button')).toBeNull()
362+
expect(container.querySelector('[role="alert"], [role="dialog"]')).toBeNull()
363+
expect(container.textContent).not.toContain('Reconnecting…')
364+
expect(toast.warning).not.toHaveBeenCalled()
365+
expect(toast.info).not.toHaveBeenCalled()
366+
expect(getEditor().isEditable).toBe(false)
368367
expect(getEditor().getText()).toContain('preserved local change')
369368
}
370369
)

0 commit comments

Comments
 (0)