From b847ff5533ab24fd3cb5fd3577fbd7f08a1fa214 Mon Sep 17 00:00:00 2001 From: MOONSTAR Date: Mon, 6 Jul 2026 11:32:56 -0500 Subject: [PATCH 1/2] Fix uncategorize undo save error --- src/actions/__tests__/uncategorize.ts | 25 +++++++ src/actions/uncategorize.ts | 12 +-- src/commands/__tests__/uncategorize.ts | 97 +++++++++++++++++++++++++ src/redux-enhancers/undoRedoEnhancer.ts | 7 +- 4 files changed, 128 insertions(+), 13 deletions(-) diff --git a/src/actions/__tests__/uncategorize.ts b/src/actions/__tests__/uncategorize.ts index b6b5ef6eb37..83b90478f77 100644 --- a/src/actions/__tests__/uncategorize.ts +++ b/src/actions/__tests__/uncategorize.ts @@ -109,6 +109,31 @@ describe('normal view', () => { expectPathToEqual(stateNew, stateNew.cursor, ['a', 'c']) }) + it('does not merge a child into the category being uncategorized', () => { + const steps = [ + importText({ + text: ` + - a + - b + - b + - c + `, + }), + setCursor(['a', 'b']), + uncategorize({}), + ] + + const stateNew = reducerFlow(steps)(initialState()) + const exported = exportContext(stateNew, [HOME_TOKEN], 'text/plain') + + expect(exported).toBe(`- ${HOME_TOKEN} + - a + - b + - c`) + + expectPathToEqual(stateNew, stateNew.cursor, ['a', 'b']) + }) + it('after uncategorize context set cursor to the first visible children.', () => { const steps = [ newThought('a'), diff --git a/src/actions/uncategorize.ts b/src/actions/uncategorize.ts index a9cdbb0fccf..07d9c408cad 100644 --- a/src/actions/uncategorize.ts +++ b/src/actions/uncategorize.ts @@ -16,13 +16,11 @@ import rootedParentOf from '../selectors/rootedParentOf' import simplifyPath from '../selectors/simplifyPath' import { registerActionMetadata } from '../util/actionMetadata.registry' import appendToPath from '../util/appendToPath' -import createId from '../util/createId' import head from '../util/head' import isAttribute from '../util/isAttribute' import parentOf from '../util/parentOf' import reducerFlow from '../util/reducerFlow' import deleteThought from './deleteThought' -import editThought from './editThought' import sort from './sort' interface Options { @@ -105,14 +103,6 @@ const uncategorize = (state: State, { at }: Options): State => { } return reducerFlow([ - // first edit the unacategorized thought to a unique value - // otherwise, it could get merged when children are outdented in the next step - editThought({ - oldValue: thought.value, - newValue: createId(), // unique value - path: simplePath, - }), - // Sort parent context if sort preference exists and parent does not have a sort preference // Sort preference must be moved up before sort to prevent conversion to manual sort. contextHasSortPreference && !parentHasSortPreference @@ -135,6 +125,8 @@ const uncategorize = (state: State, { at }: Options): State => { oldPath: appendToPath(simplePath, child.id), newPath: appendToPath(parentOf(simplePath), child.id), newRank: getNewRank(state, child), + // If a child has the same value as the category being deleted, do not merge it into the category. + skipMerge: child.value === thought.value, }) }), diff --git a/src/commands/__tests__/uncategorize.ts b/src/commands/__tests__/uncategorize.ts index 9c86a1c7562..d8e19c02555 100644 --- a/src/commands/__tests__/uncategorize.ts +++ b/src/commands/__tests__/uncategorize.ts @@ -1,9 +1,13 @@ import { importTextActionCreator as importText } from '../../actions/importText' +import { undoActionCreator as undo } from '../../actions/undo' import { executeCommandWithMulticursor } from '../../commands' import { HOME_TOKEN } from '../../constants' +import db from '../../data-providers/yjs/thoughtspace' +import { initialize } from '../../initialize' import exportContext from '../../selectors/exportContext' import store from '../../stores/app' import { addMulticursorAtFirstMatchActionCreator as addMulticursor } from '../../test-helpers/addMulticursorAtFirstMatch' +import expectPathToEqual from '../../test-helpers/expectPathToEqual' import initStore from '../../test-helpers/initStore' import { setCursorFirstMatchActionCreator as setCursor } from '../../test-helpers/setCursorFirstMatch' import uncategorizeCommand from '../uncategorize' @@ -11,6 +15,99 @@ import uncategorizeCommand from '../uncategorize' beforeEach(initStore) describe('uncategorize', () => { + it('undoes uncategorize of a duplicate uncle', () => { + store.dispatch([ + importText({ + text: ` + - a + - b + - a + - c + `, + }), + setCursor(['b', 'a']), + ]) + + executeCommandWithMulticursor(uncategorizeCommand, { store }) + + let state = store.getState() + expect(exportContext(state, [HOME_TOKEN], 'text/plain')).toEqual(`- ${HOME_TOKEN} + - a + - b + - c`) + expectPathToEqual(state, state.cursor, ['b', 'c']) + + expect(() => store.dispatch(undo())).not.toThrow() + + state = store.getState() + expect(exportContext(state, [HOME_TOKEN], 'text/plain')).toEqual(`- ${HOME_TOKEN} + - a + - b + - a + - c`) + expectPathToEqual(state, state.cursor, ['b', 'a']) + }) + + it('pushes complete lexeme updates when undoing uncategorize of a duplicate uncle', async () => { + const updateThoughtsSpy = vi.spyOn(db, 'updateThoughts').mockResolvedValue(undefined) + + try { + store.dispatch([ + importText({ + text: ` + - a + - b + - a + - c + `, + }), + setCursor(['b', 'a']), + ]) + + executeCommandWithMulticursor(uncategorizeCommand, { store }) + store.dispatch(undo()) + + await vi.runOnlyPendingTimersAsync() + await Promise.resolve() + + const malformedLexemeUpdates = updateThoughtsSpy.mock.calls + .flatMap(([payload]) => Object.values(payload.lexemeIndexUpdates)) + .filter(lexeme => lexeme && !Array.isArray(lexeme.contexts)) + + expect(malformedLexemeUpdates).toEqual([]) + } finally { + updateThoughtsSpy.mockRestore() + } + }) + + it('persists undoing uncategorize of a duplicate uncle without a save error', async () => { + const { cleanup } = await initialize() + + try { + store.dispatch([ + importText({ + text: ` + - a + - b + - a + - c + `, + }), + setCursor(['b', 'a']), + ]) + + executeCommandWithMulticursor(uncategorizeCommand, { store }) + store.dispatch(undo()) + + await vi.runOnlyPendingTimersAsync() + await Promise.resolve() + + expect(store.getState().alert?.value).not.toContain('not able to save the last change') + } finally { + cleanup() + } + }, 10000) + describe('multicursor', () => { it('collapses multiple thoughts', async () => { store.dispatch([ diff --git a/src/redux-enhancers/undoRedoEnhancer.ts b/src/redux-enhancers/undoRedoEnhancer.ts index 8438353fa6c..025d2c0051b 100644 --- a/src/redux-enhancers/undoRedoEnhancer.ts +++ b/src/redux-enhancers/undoRedoEnhancer.ts @@ -68,11 +68,12 @@ const restorePushQueueFromPatches = (state: State, oldState: State, patch: Patch const lexemeIndexChanges = patch.filter(p => p?.path.startsWith('/thoughts/lexemeIndex/')) const thoughtIndexChanges = patch.filter(p => p?.path.startsWith('/thoughts/thoughtIndex/')) - const lexemeIndexUpdates = lexemeIndexChanges.reduce>((acc, op) => { - const lexemeKey = op.path.slice('/thoughts/lexemeIndex/'.length).split('/')[0] + const lexemeIndexUpdates = lexemeIndexChanges.reduce>((acc, { path }) => { + const lexemeKey = path.slice('/thoughts/lexemeIndex/'.length).split('/')[0] return { ...acc, - [lexemeKey]: op.value || null, + // Patch paths may target nested lexeme properties such as contexts. Persist the full lexeme. + [lexemeKey]: state.thoughts.lexemeIndex[lexemeKey] || null, } }, {}) const thoughtIndexUpdates = thoughtIndexChanges.reduce((acc, { path }) => { From 056cc2ea485ca98efdfefc7075be6d49b8de4297 Mon Sep 17 00:00:00 2001 From: MOONSTAR Date: Mon, 6 Jul 2026 22:59:38 -0500 Subject: [PATCH 2/2] Address uncategorize undo review feedback --- src/commands/__tests__/uncategorize.ts | 35 +------------------------- 1 file changed, 1 insertion(+), 34 deletions(-) diff --git a/src/commands/__tests__/uncategorize.ts b/src/commands/__tests__/uncategorize.ts index d8e19c02555..17fb6c7ffab 100644 --- a/src/commands/__tests__/uncategorize.ts +++ b/src/commands/__tests__/uncategorize.ts @@ -2,7 +2,6 @@ import { importTextActionCreator as importText } from '../../actions/importText' import { undoActionCreator as undo } from '../../actions/undo' import { executeCommandWithMulticursor } from '../../commands' import { HOME_TOKEN } from '../../constants' -import db from '../../data-providers/yjs/thoughtspace' import { initialize } from '../../initialize' import exportContext from '../../selectors/exportContext' import store from '../../stores/app' @@ -48,38 +47,6 @@ describe('uncategorize', () => { expectPathToEqual(state, state.cursor, ['b', 'a']) }) - it('pushes complete lexeme updates when undoing uncategorize of a duplicate uncle', async () => { - const updateThoughtsSpy = vi.spyOn(db, 'updateThoughts').mockResolvedValue(undefined) - - try { - store.dispatch([ - importText({ - text: ` - - a - - b - - a - - c - `, - }), - setCursor(['b', 'a']), - ]) - - executeCommandWithMulticursor(uncategorizeCommand, { store }) - store.dispatch(undo()) - - await vi.runOnlyPendingTimersAsync() - await Promise.resolve() - - const malformedLexemeUpdates = updateThoughtsSpy.mock.calls - .flatMap(([payload]) => Object.values(payload.lexemeIndexUpdates)) - .filter(lexeme => lexeme && !Array.isArray(lexeme.contexts)) - - expect(malformedLexemeUpdates).toEqual([]) - } finally { - updateThoughtsSpy.mockRestore() - } - }) - it('persists undoing uncategorize of a duplicate uncle without a save error', async () => { const { cleanup } = await initialize() @@ -102,7 +69,7 @@ describe('uncategorize', () => { await vi.runOnlyPendingTimersAsync() await Promise.resolve() - expect(store.getState().alert?.value).not.toContain('not able to save the last change') + expect(store.getState().error).toBeNull() } finally { cleanup() }