From f9d65debe92bb8ea463dec18df7084883c43131e Mon Sep 17 00:00:00 2001 From: Rushi <84287593+Rishi943@users.noreply.github.com> Date: Sat, 11 Jul 2026 11:59:50 -0400 Subject: [PATCH] refactor(tasks): extract shared task ordering helpers (#8926) * refactor(tasks): extract shared task ordering helpers Extract getReorderedSubTaskIds (membership check + move shared by moveSubTaskUp/Down/ToTop/ToBottom) and moveValidIdsToFront (shared by removeTasksFromTodayTag and localRemoveOverdueFromToday) into pure, tested utils. No behavior change. Closes #7912 Co-Authored-By: Claude Fable 5 * refactor(tasks): drop getReorderedSubTaskIds, inline check in reorderSubTask Review feedback (#8926): the moveSubTask* actions already funnel through the single reorderSubTask helper, so the extraction added no dedup value. Keep moveValidIdsToFront, which removes real duplication. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- src/app/features/tasks/store/task.reducer.ts | 26 ++++++++----- .../util/move-valid-ids-to-front.spec.ts | 37 +++++++++++++++++++ .../tasks/util/move-valid-ids-to-front.ts | 23 ++++++++++++ 3 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 src/app/features/tasks/util/move-valid-ids-to-front.spec.ts create mode 100644 src/app/features/tasks/util/move-valid-ids-to-front.ts diff --git a/src/app/features/tasks/store/task.reducer.ts b/src/app/features/tasks/store/task.reducer.ts index dd06fe8a80..cede160553 100644 --- a/src/app/features/tasks/store/task.reducer.ts +++ b/src/app/features/tasks/store/task.reducer.ts @@ -52,6 +52,7 @@ import { } from '../../time-tracking/store/time-tracking.actions'; import { TaskLog } from '../../../core/log'; import { devError } from '../../../util/dev-error'; +import { moveValidIdsToFront } from '../util/move-valid-ids-to-front'; export { taskAdapter }; @@ -141,6 +142,7 @@ const reorderSubTask = ( TaskLog.err(`Parent task ${parentId} not found`); return state; } + const parentSubTaskIds = parentTask.subTaskIds; // Check if the subtask is actually in the parent's subtask list @@ -699,25 +701,31 @@ export const taskReducer = createReducer( }), on(TaskSharedActions.removeTasksFromTodayTag, (state, { taskIds }) => { - const validTaskIds = taskIds.filter((id) => !!state.entities[id]); - if (validTaskIds.length !== taskIds.length) { - devError( - `removeTasksFromTodayTag: ${taskIds.length - validTaskIds.length} orphan ID(s) filtered`, - ); + // we do this to maintain the order of tasks when they are moved to overdue + const { ids, invalidCount } = moveValidIdsToFront( + state.ids as string[], + taskIds, + (id) => !!state.entities[id], + ); + if (invalidCount > 0) { + devError(`removeTasksFromTodayTag: ${invalidCount} orphan ID(s) filtered`); } return { ...state, - // we do this to maintain the order of tasks when they are moved to overdue - ids: [...validTaskIds, ...state.ids.filter((id) => !taskIds.includes(id))], + ids, }; }), // Same reordering for the non-persistent variant (#6992) on(TaskSharedActions.localRemoveOverdueFromToday, (state, { taskIds }) => { - const validTaskIds = taskIds.filter((id) => !!state.entities[id]); + const { ids } = moveValidIdsToFront( + state.ids as string[], + taskIds, + (id) => !!state.entities[id], + ); return { ...state, - ids: [...validTaskIds, ...state.ids.filter((id) => !taskIds.includes(id))], + ids, }; }), ); diff --git a/src/app/features/tasks/util/move-valid-ids-to-front.spec.ts b/src/app/features/tasks/util/move-valid-ids-to-front.spec.ts new file mode 100644 index 0000000000..37df079d84 --- /dev/null +++ b/src/app/features/tasks/util/move-valid-ids-to-front.spec.ts @@ -0,0 +1,37 @@ +import { moveValidIdsToFront } from './move-valid-ids-to-front'; + +describe('moveValidIdsToFront', () => { + it('moves all ids to the front when all are valid, preserving relative order', () => { + const result = moveValidIdsToFront( + ['t1', 't2', 't3', 't4'], + ['t2', 't4'], + () => true, + ); + + expect(result.ids).toEqual(['t2', 't4', 't1', 't3']); + expect(result.invalidCount).toBe(0); + }); + + it('filters out invalid ids and only moves the valid ones', () => { + const isValid = (id: string): boolean => id !== 'nonexistent'; + const result = moveValidIdsToFront( + ['t1', 't2', 't3', 't4'], + ['t2', 'nonexistent', 't4'], + isValid, + ); + + expect(result.ids).toEqual(['t2', 't4', 't1', 't3']); + expect(result.invalidCount).toBe(1); + }); + + it('reports every id invalid and leaves order unchanged when none are valid', () => { + const result = moveValidIdsToFront( + ['t1', 't2'], + ['nonexistent1', 'nonexistent2'], + () => false, + ); + + expect(result.ids).toEqual(['t1', 't2']); + expect(result.invalidCount).toBe(2); + }); +}); diff --git a/src/app/features/tasks/util/move-valid-ids-to-front.ts b/src/app/features/tasks/util/move-valid-ids-to-front.ts new file mode 100644 index 0000000000..67b3f6975e --- /dev/null +++ b/src/app/features/tasks/util/move-valid-ids-to-front.ts @@ -0,0 +1,23 @@ +/** + * Moves the subset of `idsToMove` that pass `isValidId` to the front of + * `allIds`, preserving their relative order, while leaving the remaining ids + * in their original order. Shared by removeTasksFromTodayTag and + * localRemoveOverdueFromToday in task.reducer.ts so tasks removed from Today + * keep a stable position when they reappear (e.g. as overdue). See #6992. + */ +export interface MoveValidIdsToFrontResult { + ids: string[]; + invalidCount: number; +} + +export const moveValidIdsToFront = ( + allIds: string[], + idsToMove: string[], + isValidId: (id: string) => boolean, +): MoveValidIdsToFrontResult => { + const validIds = idsToMove.filter(isValidId); + return { + ids: [...validIds, ...allIds.filter((id) => !idsToMove.includes(id))], + invalidCount: idsToMove.length - validIds.length, + }; +};