From 30f901dea7aca060ecfc96565001e9f7234ae7b0 Mon Sep 17 00:00:00 2001 From: Rui Martins Date: Mon, 18 May 2026 23:22:51 +0100 Subject: [PATCH 1/3] feat: add section reorder command --- skills/todoist-cli/SKILL.md | 4 + src/commands/section/index.ts | 36 +++ src/commands/section/reorder.ts | 172 +++++++++++++++ src/commands/section/section.test.ts | 317 ++++++++++++++++++++++++++- src/lib/api/sections-sync.ts | 11 + src/lib/skills/content.ts | 4 + src/test-support/fixtures.ts | 7 + 7 files changed, 550 insertions(+), 1 deletion(-) create mode 100644 src/commands/section/reorder.ts create mode 100644 src/lib/api/sections-sync.ts diff --git a/skills/todoist-cli/SKILL.md b/skills/todoist-cli/SKILL.md index 6abbd09b..a7ed5422 100644 --- a/skills/todoist-cli/SKILL.md +++ b/skills/todoist-cli/SKILL.md @@ -223,6 +223,10 @@ td section list --search "Planning" td section list --search "Planning" --project "Roadmap" td section create --project "Roadmap" --name "In Progress" td section update id:123 --name "Done" +td section reorder "Review" --project "Roadmap" --before "Done" +td section reorder "Review" --project "Roadmap" --after "In Progress" +td section reorder --section "Review" --project "Roadmap" --position 0 --dry-run +td section reorder "Review" --project "Roadmap" --position 2 --json td section archive id:123 td section unarchive id:123 td section delete id:123 --yes diff --git a/src/commands/section/index.ts b/src/commands/section/index.ts index 7b768ff8..e8b12d21 100644 --- a/src/commands/section/index.ts +++ b/src/commands/section/index.ts @@ -1,11 +1,13 @@ import { Command } from 'commander' import { CliError } from '../../lib/errors.js' import type { PaginatedViewOptions } from '../../lib/options.js' +import { parseOrderArg } from '../../lib/order.js' import { archiveSection } from './archive.js' import { browseSection } from './browse.js' import { createSection } from './create.js' import { deleteSection } from './delete.js' import { listSections } from './list.js' +import { reorderSection } from './reorder.js' import { unarchiveSection } from './unarchive.js' import { updateSection } from './update.js' @@ -85,6 +87,40 @@ export function registerSectionCommand(program: Command): void { return updateSection(id, options) }) + const reorderCmd = section + .command('reorder [ref]') + .description('Reorder a section within a project') + .option('--section ', 'Section name or id:xxx') + .option('--project ', 'Project name or id:xxx (required)') + .option('--before ', 'Place before this sibling section') + .option('--after ', 'Place after this sibling section') + .option('--position ', 'Move to a 0-indexed position within the project', parseOrderArg) + .option('--json', 'Output the new ordering as JSON') + .option('--dry-run', 'Preview what would happen without executing') + .addHelpText( + 'after', + ` +Examples: + td section reorder "Review" --project "Roadmap" --before "Done" + td section reorder "Review" --project "Roadmap" --after "In Progress" + td section reorder --section "Review" --project "Roadmap" --position 0 + td section reorder "Review" --project "Roadmap" --position 0`, + ) + .action((ref, options) => { + if (ref && options.section) { + throw new CliError( + 'CONFLICTING_OPTIONS', + 'Cannot specify section both as argument and --section flag', + ) + } + const sectionRef = ref || options.section + if (!sectionRef) { + reorderCmd.help() + return + } + return reorderSection(sectionRef, options) + }) + const archiveCmd = section .command('archive [id]') .description('Archive a section') diff --git a/src/commands/section/reorder.ts b/src/commands/section/reorder.ts new file mode 100644 index 00000000..43571040 --- /dev/null +++ b/src/commands/section/reorder.ts @@ -0,0 +1,172 @@ +import type { Section, TodoistApi } from '@doist/todoist-sdk' +import { getApi } from '../../lib/api/core.js' +import { reorderSections } from '../../lib/api/sections-sync.js' +import { CliError } from '../../lib/errors.js' +import { isQuiet } from '../../lib/global-args.js' +import { formatJson } from '../../lib/output.js' +import { paginate } from '../../lib/pagination.js' +import { extractId, isIdRef, looksLikeRawId, resolveProjectId } from '../../lib/refs.js' + +export type ReorderSectionOptions = { + section?: string + project?: string + before?: string + after?: string + position?: number + json?: boolean + dryRun?: boolean +} + +async function loadProjectSections(api: TodoistApi, projectId: string): Promise { + const { results } = await paginate( + (cursor, limit) => api.getSections({ projectId, cursor: cursor ?? undefined, limit }), + { limit: Number.MAX_SAFE_INTEGER }, + ) + return results +} + +function resolveSectionFromList(sections: Section[], ref: string): Section { + if (!ref.trim()) { + throw new CliError('INVALID_SECTION', 'section reference cannot be empty.') + } + + if (isIdRef(ref)) { + const id = extractId(ref) + const match = sections.find((section) => section.id === id) + if (!match) { + throw new CliError('SECTION_NOT_FOUND', `Section id:${id} not found in project.`) + } + return match + } + + const lower = ref.toLowerCase() + const exact = sections.filter((section) => section.name.toLowerCase() === lower) + if (exact.length === 1) return exact[0] + if (exact.length > 1) { + throw new CliError( + 'AMBIGUOUS_SECTION', + `Multiple sections match "${ref}" exactly in project:`, + exact.slice(0, 5).map((section) => `"${section.name}" (id:${section.id})`), + ) + } + + const partial = sections.filter((section) => section.name.toLowerCase().includes(lower)) + if (partial.length === 1) return partial[0] + if (partial.length > 1) { + throw new CliError( + 'AMBIGUOUS_SECTION', + `Multiple sections match "${ref}" in project:`, + partial.slice(0, 5).map((section) => `"${section.name}" (id:${section.id})`), + ) + } + + if (looksLikeRawId(ref)) { + const byId = sections.find((section) => section.id === ref) + if (byId) return byId + } + + throw new CliError('SECTION_NOT_FOUND', `Section "${ref}" not found in project.`) +} + +export async function reorderSection(ref: string, options: ReorderSectionOptions): Promise { + if (!options.project) { + throw new CliError('MISSING_PROJECT', 'Specify --project to reorder a section.', [ + 'Section names are scoped to a project.', + ]) + } + + const flagCount = [options.before, options.after, options.position].filter( + (value) => value !== undefined, + ).length + if (flagCount === 0) { + throw new CliError( + 'INVALID_OPTIONS', + 'Specify exactly one of --before , --after , or --position .', + ) + } + if (flagCount > 1) { + throw new CliError( + 'INVALID_OPTIONS', + '--before, --after, and --position are mutually exclusive.', + ) + } + + const api = await getApi() + const projectId = await resolveProjectId(api, options.project) + const sections = (await loadProjectSections(api, projectId)).sort( + (a, b) => a.sectionOrder - b.sectionOrder, + ) + + const target = resolveSectionFromList(sections, ref) + const oldIndex = sections.findIndex((section) => section.id === target.id) + if (oldIndex === -1) { + throw new CliError('SECTION_NOT_FOUND', 'Target section not found in project.') + } + + let newIndex: number + if (options.position !== undefined) { + newIndex = Math.min(options.position, sections.length - 1) + } else { + const siblingRef = (options.before ?? options.after) as string + const sibling = resolveSectionFromList(sections, siblingRef) + if (sibling.id === target.id) { + throw new CliError('INVALID_OPTIONS', 'Cannot reorder a section relative to itself.') + } + + const siblingIndex = sections.findIndex((section) => section.id === sibling.id) + const adjusted = siblingIndex > oldIndex ? siblingIndex - 1 : siblingIndex + newIndex = options.before !== undefined ? adjusted : adjusted + 1 + } + + if (newIndex === oldIndex) { + if (options.json) { + console.log( + formatJson(sections.map((section, index) => sectionPosition(section, index))), + ) + return + } + if (!isQuiet()) { + console.log(`No change: "${target.name}" already at position ${oldIndex}.`) + } + return + } + + const newOrder = [...sections] + const [removed] = newOrder.splice(oldIndex, 1) + newOrder.splice(newIndex, 0, removed) + + const items = newOrder.map((section, index) => ({ + id: section.id, + sectionOrder: index + 1, + })) + + if (options.dryRun) { + console.log(`Would reorder "${target.name}": position ${oldIndex} -> ${newIndex}`) + console.log('New section order:') + for (let index = 0; index < newOrder.length; index++) { + const marker = newOrder[index].id === target.id ? '>' : ' ' + console.log(` ${marker} ${index}: ${newOrder[index].name} (id:${newOrder[index].id})`) + } + return + } + + await reorderSections(items) + + if (options.json) { + console.log(formatJson(newOrder.map((section, index) => sectionPosition(section, index)))) + return + } + + if (!isQuiet()) { + console.log( + `Reordered "${target.name}" (id:${target.id}): position ${oldIndex} -> ${newIndex} of ${sections.length - 1}.`, + ) + } +} + +function sectionPosition( + section: Section, + position: number, +): { id: string; name: string; position: number } { + return { id: section.id, name: section.name, position } +} diff --git a/src/commands/section/section.test.ts b/src/commands/section/section.test.ts index 4f9fa6de..bf87b819 100644 --- a/src/commands/section/section.test.ts +++ b/src/commands/section/section.test.ts @@ -1,15 +1,22 @@ import { Command } from 'commander' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('../../lib/api/core.js', () => ({ getApi: vi.fn(), })) +vi.mock('../../lib/api/sections-sync.js', () => ({ + reorderSections: vi.fn().mockResolvedValue(undefined), +})) + import { getApi } from '../../lib/api/core.js' +import { reorderSections } from '../../lib/api/sections-sync.js' +import { fixtures } from '../../test-support/fixtures.js' import { createMockApi, type MockApi } from '../../test-support/mock-api.js' import { registerSectionCommand } from './index.js' const mockGetApi = vi.mocked(getApi) +const mockReorderSections = vi.mocked(reorderSections) function createProgram() { const program = new Command() @@ -264,6 +271,314 @@ describe('section update --json', () => { }) }) +describe('section reorder', () => { + let mockApi: MockApi + let consoleSpy: ReturnType + + const sections = [ + fixtures.sections.planning, + fixtures.sections.inProgress, + fixtures.sections.review, + fixtures.sections.done, + ] + + beforeEach(() => { + vi.clearAllMocks() + mockApi = createMockApi() + mockGetApi.mockResolvedValue(mockApi) + mockApi.getProject.mockResolvedValue(fixtures.projects.work) + mockApi.getSections.mockImplementation(async () => ({ + results: [...sections], + nextCursor: null, + })) + consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + }) + + afterEach(() => { + consoleSpy.mockRestore() + }) + + it('requires --project', async () => { + const program = createProgram() + + await expect( + program.parseAsync(['node', 'td', 'section', 'reorder', 'Review', '--position', '0']), + ).rejects.toHaveProperty('code', 'MISSING_PROJECT') + }) + + it('throws when no placement flag is provided', async () => { + const program = createProgram() + + await expect( + program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Review', + '--project', + 'id:proj-work', + ]), + ).rejects.toHaveProperty('code', 'INVALID_OPTIONS') + }) + + it('accepts --section instead of positional ref', async () => { + const program = createProgram() + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + '--section', + 'Review', + '--project', + 'id:proj-work', + '--position', + '0', + ]) + + expect(mockReorderSections).toHaveBeenCalledWith([ + { id: 'sec-3', sectionOrder: 1 }, + { id: 'sec-1', sectionOrder: 2 }, + { id: 'sec-2', sectionOrder: 3 }, + { id: 'sec-4', sectionOrder: 4 }, + ]) + }) + + it('errors when both positional and --section are provided', async () => { + const program = createProgram() + + await expect( + program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Review', + '--section', + 'Done', + '--project', + 'id:proj-work', + '--position', + '0', + ]), + ).rejects.toThrow('Cannot specify section both as argument and --section flag') + }) + + it('throws when more than one placement flag is provided', async () => { + const program = createProgram() + + await expect( + program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Review', + '--project', + 'id:proj-work', + '--before', + 'Done', + '--position', + '0', + ]), + ).rejects.toHaveProperty('code', 'INVALID_OPTIONS') + }) + + it('rejects non-integer --position values', async () => { + const program = createProgram() + + await expect( + program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Review', + '--project', + 'id:proj-work', + '--position', + '1.5', + ]), + ).rejects.toHaveProperty('code', 'INVALID_ORDER') + }) + + it('--position 0 moves target to first', async () => { + const program = createProgram() + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Review', + '--project', + 'id:proj-work', + '--position', + '0', + ]) + + expect(mockReorderSections).toHaveBeenCalledWith([ + { id: 'sec-3', sectionOrder: 1 }, + { id: 'sec-1', sectionOrder: 2 }, + { id: 'sec-2', sectionOrder: 3 }, + { id: 'sec-4', sectionOrder: 4 }, + ]) + }) + + it('--position clamps to last when out of range', async () => { + const program = createProgram() + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Planning', + '--project', + 'id:proj-work', + '--position', + '999', + ]) + + expect(mockReorderSections).toHaveBeenCalledWith([ + { id: 'sec-2', sectionOrder: 1 }, + { id: 'sec-3', sectionOrder: 2 }, + { id: 'sec-4', sectionOrder: 3 }, + { id: 'sec-1', sectionOrder: 4 }, + ]) + }) + + it('--before places target immediately before sibling', async () => { + const program = createProgram() + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Done', + '--project', + 'id:proj-work', + '--before', + 'In Progress', + ]) + + expect(mockReorderSections).toHaveBeenCalledWith([ + { id: 'sec-1', sectionOrder: 1 }, + { id: 'sec-4', sectionOrder: 2 }, + { id: 'sec-2', sectionOrder: 3 }, + { id: 'sec-3', sectionOrder: 4 }, + ]) + }) + + it('--after places target immediately after sibling', async () => { + const program = createProgram() + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Planning', + '--project', + 'id:proj-work', + '--after', + 'Review', + ]) + + expect(mockReorderSections).toHaveBeenCalledWith([ + { id: 'sec-2', sectionOrder: 1 }, + { id: 'sec-3', sectionOrder: 2 }, + { id: 'sec-1', sectionOrder: 3 }, + { id: 'sec-4', sectionOrder: 4 }, + ]) + }) + + it('rejects relative placement against itself', async () => { + const program = createProgram() + + await expect( + program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Review', + '--project', + 'id:proj-work', + '--before', + 'Review', + ]), + ).rejects.toHaveProperty('code', 'INVALID_OPTIONS') + }) + + it('--dry-run does not call reorderSections', async () => { + const program = createProgram() + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Planning', + '--project', + 'id:proj-work', + '--position', + '2', + '--dry-run', + ]) + + expect(mockReorderSections).not.toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Would reorder "Planning"')) + }) + + it('no-ops when target already has the requested position', async () => { + const program = createProgram() + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'In Progress', + '--project', + 'id:proj-work', + '--position', + '1', + ]) + + expect(mockReorderSections).not.toHaveBeenCalled() + }) + + it('--json outputs the new ordering', async () => { + const program = createProgram() + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Review', + '--project', + 'id:proj-work', + '--position', + '0', + '--json', + ]) + + expect(consoleSpy).toHaveBeenCalledTimes(1) + const parsed = JSON.parse(consoleSpy.mock.calls[0][0]) + expect(parsed).toEqual([ + { id: 'sec-3', name: 'Review', position: 0 }, + { id: 'sec-1', name: 'Planning', position: 1 }, + { id: 'sec-2', name: 'In Progress', position: 2 }, + { id: 'sec-4', name: 'Done', position: 3 }, + ]) + }) +}) + describe('section create', () => { let mockApi: MockApi diff --git a/src/lib/api/sections-sync.ts b/src/lib/api/sections-sync.ts new file mode 100644 index 00000000..ef04941e --- /dev/null +++ b/src/lib/api/sections-sync.ts @@ -0,0 +1,11 @@ +import { createCommand } from '@doist/todoist-sdk' +import { getApi } from './core.js' + +export async function reorderSections( + items: Array<{ id: string; sectionOrder: number }>, +): Promise { + const api = await getApi() + await api.sync({ + commands: [createCommand('section_reorder', { sections: items })], + }) +} diff --git a/src/lib/skills/content.ts b/src/lib/skills/content.ts index 1b94d16c..33885154 100644 --- a/src/lib/skills/content.ts +++ b/src/lib/skills/content.ts @@ -219,6 +219,10 @@ td section list --search "Planning" td section list --search "Planning" --project "Roadmap" td section create --project "Roadmap" --name "In Progress" td section update id:123 --name "Done" +td section reorder "Review" --project "Roadmap" --before "Done" +td section reorder "Review" --project "Roadmap" --after "In Progress" +td section reorder --section "Review" --project "Roadmap" --position 0 --dry-run +td section reorder "Review" --project "Roadmap" --position 2 --json td section archive id:123 td section unarchive id:123 td section delete id:123 --yes diff --git a/src/test-support/fixtures.ts b/src/test-support/fixtures.ts index c0ae0b18..9bd08702 100644 --- a/src/test-support/fixtures.ts +++ b/src/test-support/fixtures.ts @@ -165,6 +165,13 @@ export const fixtures = { sectionOrder: 3, url: 'https://todoist.com/app/section/sec-3', } as Section, + done: { + id: 'sec-4', + name: 'Done', + projectId: 'proj-work', + sectionOrder: 4, + url: 'https://todoist.com/app/section/sec-4', + } as Section, }, labels: { urgent: { From fd3d0451a5c64e1ce506e5e3f027ca39b31be626 Mon Sep 17 00:00:00 2001 From: Rui Martins Date: Mon, 18 May 2026 23:46:06 +0100 Subject: [PATCH 2/3] Address PR review feedback - Reuse shared ref resolution for section reorder - Extract shared reorder placement validation - Assert section reorder sync payloads in tests --- CODEBASE.md | 6 +- src/commands/project/reorder.ts | 17 +--- src/commands/section/index.ts | 4 +- src/commands/section/reorder.ts | 82 +++++-------------- src/commands/section/section.test.ts | 116 +++++++++++++++++++++++---- src/lib/refs.test.ts | 12 +++ src/lib/refs.ts | 18 +++-- src/lib/reorder.ts | 27 +++++++ src/test-support/mock-api.ts | 1 + 9 files changed, 183 insertions(+), 100 deletions(-) create mode 100644 src/lib/reorder.ts diff --git a/CODEBASE.md b/CODEBASE.md index 8ac72874..6531f223 100644 --- a/CODEBASE.md +++ b/CODEBASE.md @@ -150,8 +150,10 @@ New subcommand? Copy a sibling in the target group, wire it in that group's - **`refs.ts`** — `isIdRef`, `extractId`, `looksLikeRawId`, `lenientIdRef`, `resolveTaskRef`, `resolveProjectRef`, `resolveProjectId`, `resolveSectionId`, `resolveParentTaskId`, `resolveWorkspaceRef`, - `resolveFolderRef`, `resolveAppRef`, `resolveGoalRef`, `parseTodoistUrl`, - `classifyTodoistUrl` + `resolveFolderRef`, `resolveAppRef`, `resolveGoalRef`, `resolveFromList`, + `parseTodoistUrl`, `classifyTodoistUrl` +- **`reorder.ts`** — `validateReorderPlacement()` for shared + `--before` / `--after` / `--position` validation. - **`urls.ts`** — `taskUrl`, `projectUrl`, `labelUrl`, `sectionUrl`, `commentUrl`, `filterUrl` - **`task-list.ts`** — `fetchProjects`, `filterByWorkspaceOrPersonal`, diff --git a/src/commands/project/reorder.ts b/src/commands/project/reorder.ts index 32e08079..ed511376 100644 --- a/src/commands/project/reorder.ts +++ b/src/commands/project/reorder.ts @@ -5,6 +5,7 @@ import { CliError } from '../../lib/errors.js' import { isQuiet } from '../../lib/global-args.js' import { formatJson } from '../../lib/output.js' import { resolveProjectRef } from '../../lib/refs.js' +import { validateReorderPlacement } from '../../lib/reorder.js' import { loadPersonalProjects, resolvePersonalFromList } from './helpers.js' export type ReorderOptions = { @@ -16,21 +17,7 @@ export type ReorderOptions = { } export async function reorderProject(ref: string, options: ReorderOptions): Promise { - const flagCount = [options.before, options.after, options.position].filter( - (v) => v !== undefined, - ).length - if (flagCount === 0) { - throw new CliError( - 'INVALID_OPTIONS', - 'Specify exactly one of --before , --after , or --position .', - ) - } - if (flagCount > 1) { - throw new CliError( - 'INVALID_OPTIONS', - '--before, --after, and --position are mutually exclusive.', - ) - } + validateReorderPlacement(options) const api = await getApi() const target = await resolveProjectRef(api, ref) diff --git a/src/commands/section/index.ts b/src/commands/section/index.ts index e8b12d21..d22591ca 100644 --- a/src/commands/section/index.ts +++ b/src/commands/section/index.ts @@ -103,8 +103,8 @@ export function registerSectionCommand(program: Command): void { Examples: td section reorder "Review" --project "Roadmap" --before "Done" td section reorder "Review" --project "Roadmap" --after "In Progress" - td section reorder --section "Review" --project "Roadmap" --position 0 - td section reorder "Review" --project "Roadmap" --position 0`, + td section reorder --section "Review" --project "Roadmap" --position 0 --dry-run + td section reorder "Review" --project "Roadmap" --position 2 --json`, ) .action((ref, options) => { if (ref && options.section) { diff --git a/src/commands/section/reorder.ts b/src/commands/section/reorder.ts index 43571040..15949cc4 100644 --- a/src/commands/section/reorder.ts +++ b/src/commands/section/reorder.ts @@ -4,8 +4,8 @@ import { reorderSections } from '../../lib/api/sections-sync.js' import { CliError } from '../../lib/errors.js' import { isQuiet } from '../../lib/global-args.js' import { formatJson } from '../../lib/output.js' -import { paginate } from '../../lib/pagination.js' -import { extractId, isIdRef, looksLikeRawId, resolveProjectId } from '../../lib/refs.js' +import { resolveFromList, resolveProjectId } from '../../lib/refs.js' +import { validateReorderPlacement } from '../../lib/reorder.js' export type ReorderSectionOptions = { section?: string @@ -18,11 +18,20 @@ export type ReorderSectionOptions = { } async function loadProjectSections(api: TodoistApi, projectId: string): Promise { - const { results } = await paginate( - (cursor, limit) => api.getSections({ projectId, cursor: cursor ?? undefined, limit }), - { limit: Number.MAX_SAFE_INTEGER }, - ) - return results + const sections: Section[] = [] + let cursor: string | null = null + + do { + const { results, nextCursor } = await api.getSections({ + projectId, + cursor: cursor ?? undefined, + limit: 200, + }) + sections.push(...results) + cursor = nextCursor ?? null + } while (cursor) + + return sections } function resolveSectionFromList(sections: Section[], ref: string): Section { @@ -30,42 +39,7 @@ function resolveSectionFromList(sections: Section[], ref: string): Section { throw new CliError('INVALID_SECTION', 'section reference cannot be empty.') } - if (isIdRef(ref)) { - const id = extractId(ref) - const match = sections.find((section) => section.id === id) - if (!match) { - throw new CliError('SECTION_NOT_FOUND', `Section id:${id} not found in project.`) - } - return match - } - - const lower = ref.toLowerCase() - const exact = sections.filter((section) => section.name.toLowerCase() === lower) - if (exact.length === 1) return exact[0] - if (exact.length > 1) { - throw new CliError( - 'AMBIGUOUS_SECTION', - `Multiple sections match "${ref}" exactly in project:`, - exact.slice(0, 5).map((section) => `"${section.name}" (id:${section.id})`), - ) - } - - const partial = sections.filter((section) => section.name.toLowerCase().includes(lower)) - if (partial.length === 1) return partial[0] - if (partial.length > 1) { - throw new CliError( - 'AMBIGUOUS_SECTION', - `Multiple sections match "${ref}" in project:`, - partial.slice(0, 5).map((section) => `"${section.name}" (id:${section.id})`), - ) - } - - if (looksLikeRawId(ref)) { - const byId = sections.find((section) => section.id === ref) - if (byId) return byId - } - - throw new CliError('SECTION_NOT_FOUND', `Section "${ref}" not found in project.`) + return resolveFromList(ref, sections, (section) => section.name, 'section', 'in project') } export async function reorderSection(ref: string, options: ReorderSectionOptions): Promise { @@ -75,21 +49,7 @@ export async function reorderSection(ref: string, options: ReorderSectionOptions ]) } - const flagCount = [options.before, options.after, options.position].filter( - (value) => value !== undefined, - ).length - if (flagCount === 0) { - throw new CliError( - 'INVALID_OPTIONS', - 'Specify exactly one of --before , --after , or --position .', - ) - } - if (flagCount > 1) { - throw new CliError( - 'INVALID_OPTIONS', - '--before, --after, and --position are mutually exclusive.', - ) - } + validateReorderPlacement(options) const api = await getApi() const projectId = await resolveProjectId(api, options.project) @@ -141,10 +101,10 @@ export async function reorderSection(ref: string, options: ReorderSectionOptions })) if (options.dryRun) { - console.log(`Would reorder "${target.name}": position ${oldIndex} -> ${newIndex}`) + console.log(`Would reorder "${target.name}": position ${oldIndex} → ${newIndex}`) console.log('New section order:') for (let index = 0; index < newOrder.length; index++) { - const marker = newOrder[index].id === target.id ? '>' : ' ' + const marker = newOrder[index].id === target.id ? '→' : ' ' console.log(` ${marker} ${index}: ${newOrder[index].name} (id:${newOrder[index].id})`) } return @@ -159,7 +119,7 @@ export async function reorderSection(ref: string, options: ReorderSectionOptions if (!isQuiet()) { console.log( - `Reordered "${target.name}" (id:${target.id}): position ${oldIndex} -> ${newIndex} of ${sections.length - 1}.`, + `Reordered "${target.name}" (id:${target.id}): position ${oldIndex} → ${newIndex} of ${sections.length - 1}.`, ) } } diff --git a/src/commands/section/section.test.ts b/src/commands/section/section.test.ts index bf87b819..27931473 100644 --- a/src/commands/section/section.test.ts +++ b/src/commands/section/section.test.ts @@ -5,18 +5,12 @@ vi.mock('../../lib/api/core.js', () => ({ getApi: vi.fn(), })) -vi.mock('../../lib/api/sections-sync.js', () => ({ - reorderSections: vi.fn().mockResolvedValue(undefined), -})) - import { getApi } from '../../lib/api/core.js' -import { reorderSections } from '../../lib/api/sections-sync.js' import { fixtures } from '../../test-support/fixtures.js' import { createMockApi, type MockApi } from '../../test-support/mock-api.js' import { registerSectionCommand } from './index.js' const mockGetApi = vi.mocked(getApi) -const mockReorderSections = vi.mocked(reorderSections) function createProgram() { const program = new Command() @@ -25,6 +19,21 @@ function createProgram() { return program } +function expectSectionReorderCommand( + mockApi: MockApi, + sections: Array<{ id: string; sectionOrder: number }>, +) { + expect(mockApi.sync).toHaveBeenCalledTimes(1) + expect(mockApi.sync).toHaveBeenCalledWith({ + commands: [ + expect.objectContaining({ + type: 'section_reorder', + args: { sections }, + }), + ], + }) +} + describe('section list', () => { let mockApi: MockApi @@ -338,7 +347,7 @@ describe('section reorder', () => { '0', ]) - expect(mockReorderSections).toHaveBeenCalledWith([ + expectSectionReorderCommand(mockApi, [ { id: 'sec-3', sectionOrder: 1 }, { id: 'sec-1', sectionOrder: 2 }, { id: 'sec-2', sectionOrder: 3 }, @@ -419,7 +428,7 @@ describe('section reorder', () => { '0', ]) - expect(mockReorderSections).toHaveBeenCalledWith([ + expectSectionReorderCommand(mockApi, [ { id: 'sec-3', sectionOrder: 1 }, { id: 'sec-1', sectionOrder: 2 }, { id: 'sec-2', sectionOrder: 3 }, @@ -442,7 +451,7 @@ describe('section reorder', () => { '999', ]) - expect(mockReorderSections).toHaveBeenCalledWith([ + expectSectionReorderCommand(mockApi, [ { id: 'sec-2', sectionOrder: 1 }, { id: 'sec-3', sectionOrder: 2 }, { id: 'sec-4', sectionOrder: 3 }, @@ -450,6 +459,48 @@ describe('section reorder', () => { ]) }) + it('loads all section pages before reordering', async () => { + const program = createProgram() + mockApi.getSections + .mockResolvedValueOnce({ + results: [fixtures.sections.planning, fixtures.sections.inProgress], + nextCursor: 'next-page', + }) + .mockResolvedValueOnce({ + results: [fixtures.sections.review, fixtures.sections.done], + nextCursor: null, + }) + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'Review', + '--project', + 'id:proj-work', + '--position', + '0', + ]) + + expect(mockApi.getSections).toHaveBeenNthCalledWith(1, { + projectId: 'proj-work', + cursor: undefined, + limit: 200, + }) + expect(mockApi.getSections).toHaveBeenNthCalledWith(2, { + projectId: 'proj-work', + cursor: 'next-page', + limit: 200, + }) + expectSectionReorderCommand(mockApi, [ + { id: 'sec-3', sectionOrder: 1 }, + { id: 'sec-1', sectionOrder: 2 }, + { id: 'sec-2', sectionOrder: 3 }, + { id: 'sec-4', sectionOrder: 4 }, + ]) + }) + it('--before places target immediately before sibling', async () => { const program = createProgram() @@ -465,7 +516,7 @@ describe('section reorder', () => { 'In Progress', ]) - expect(mockReorderSections).toHaveBeenCalledWith([ + expectSectionReorderCommand(mockApi, [ { id: 'sec-1', sectionOrder: 1 }, { id: 'sec-4', sectionOrder: 2 }, { id: 'sec-2', sectionOrder: 3 }, @@ -488,7 +539,7 @@ describe('section reorder', () => { 'Review', ]) - expect(mockReorderSections).toHaveBeenCalledWith([ + expectSectionReorderCommand(mockApi, [ { id: 'sec-2', sectionOrder: 1 }, { id: 'sec-3', sectionOrder: 2 }, { id: 'sec-1', sectionOrder: 3 }, @@ -514,7 +565,7 @@ describe('section reorder', () => { ).rejects.toHaveProperty('code', 'INVALID_OPTIONS') }) - it('--dry-run does not call reorderSections', async () => { + it('--dry-run previews the new ordering without syncing', async () => { const program = createProgram() await program.parseAsync([ @@ -530,8 +581,16 @@ describe('section reorder', () => { '--dry-run', ]) - expect(mockReorderSections).not.toHaveBeenCalled() - expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Would reorder "Planning"')) + expect(mockApi.sync).not.toHaveBeenCalled() + const logLines = (consoleSpy.mock.calls as unknown[][]).map((call) => call[0]) + expect(logLines).toEqual([ + 'Would reorder "Planning": position 0 → 2', + 'New section order:', + ' 0: In Progress (id:sec-2)', + ' 1: Review (id:sec-3)', + ' → 2: Planning (id:sec-1)', + ' 3: Done (id:sec-4)', + ]) }) it('no-ops when target already has the requested position', async () => { @@ -549,7 +608,34 @@ describe('section reorder', () => { '1', ]) - expect(mockReorderSections).not.toHaveBeenCalled() + expect(mockApi.sync).not.toHaveBeenCalled() + }) + + it('--json on a no-op outputs the current ordering', async () => { + const program = createProgram() + + await program.parseAsync([ + 'node', + 'td', + 'section', + 'reorder', + 'In Progress', + '--project', + 'id:proj-work', + '--position', + '1', + '--json', + ]) + + expect(mockApi.sync).not.toHaveBeenCalled() + expect(consoleSpy).toHaveBeenCalledTimes(1) + const parsed = JSON.parse(consoleSpy.mock.calls[0][0]) + expect(parsed).toEqual([ + { id: 'sec-1', name: 'Planning', position: 0 }, + { id: 'sec-2', name: 'In Progress', position: 1 }, + { id: 'sec-3', name: 'Review', position: 2 }, + { id: 'sec-4', name: 'Done', position: 3 }, + ]) }) it('--json outputs the new ordering', async () => { diff --git a/src/lib/refs.test.ts b/src/lib/refs.test.ts index 67a6ec00..8379996e 100644 --- a/src/lib/refs.test.ts +++ b/src/lib/refs.test.ts @@ -542,6 +542,18 @@ describe('resolveSectionId', () => { expect(result).toBe('sec-1') }) + it('throws on ambiguous exact name match', async () => { + const api = createMockApi({ + getSections: vi.fn().mockResolvedValue({ + results: [...sections, { id: 'sec-5', name: 'Planning' }], + }), + }) + + await expect(resolveSectionId(api, 'planning', 'proj-1')).rejects.toThrow( + 'Multiple sections match "planning" exactly in project', + ) + }) + it('resolves partial name match when unique', async () => { const api = createMockApi({ getSections: vi.fn().mockResolvedValue({ results: sections }), diff --git a/src/lib/refs.ts b/src/lib/refs.ts index d1cc564c..90737652 100644 --- a/src/lib/refs.ts +++ b/src/lib/refs.ts @@ -125,13 +125,21 @@ function fuzzyMatchInList( context?: string, ): T | null { const lower = ref.toLowerCase() - const exact = items.find((item) => getName(item).toLowerCase() === lower) - if (exact) return exact + const suffix = context ? ` ${context}` : '' + + const exact = items.filter((item) => getName(item).toLowerCase() === lower) + if (exact.length === 1) return exact[0] + if (exact.length > 1) { + throw new CliError( + `AMBIGUOUS_${entityType.toUpperCase()}`, + `Multiple ${entityType}s match "${ref}" exactly${suffix}:`, + exact.slice(0, 5).map((item) => `"${getName(item)}" (id:${item.id})`), + ) + } const partial = items.filter((item) => getName(item).toLowerCase().includes(lower)) if (partial.length === 1) return partial[0] if (partial.length > 1) { - const suffix = context ? ` ${context}` : '' throw new CliError( `AMBIGUOUS_${entityType.toUpperCase()}`, `Multiple ${entityType}s match "${ref}"${suffix}:`, @@ -142,7 +150,7 @@ function fuzzyMatchInList( return null } -function resolveFromList( +export function resolveFromList( ref: string, items: T[], getName: (item: T) => string, @@ -162,7 +170,7 @@ function resolveFromList( ) } - const match = fuzzyMatchInList(ref, items, getName, entityType) + const match = fuzzyMatchInList(ref, items, getName, entityType, context) if (match) return match if (looksLikeRawId(ref)) { diff --git a/src/lib/reorder.ts b/src/lib/reorder.ts new file mode 100644 index 00000000..f79ba94a --- /dev/null +++ b/src/lib/reorder.ts @@ -0,0 +1,27 @@ +import { CliError } from './errors.js' + +export type ReorderPlacementOptions = { + before?: string + after?: string + position?: number +} + +export function validateReorderPlacement(options: ReorderPlacementOptions): void { + const flagCount = [options.before, options.after, options.position].filter( + (value) => value !== undefined, + ).length + + if (flagCount === 0) { + throw new CliError( + 'INVALID_OPTIONS', + 'Specify exactly one of --before , --after , or --position .', + ) + } + + if (flagCount > 1) { + throw new CliError( + 'INVALID_OPTIONS', + '--before, --after, and --position are mutually exclusive.', + ) + } +} diff --git a/src/test-support/mock-api.ts b/src/test-support/mock-api.ts index 84f0e46f..a140797d 100644 --- a/src/test-support/mock-api.ts +++ b/src/test-support/mock-api.ts @@ -11,6 +11,7 @@ export type MockApi = { export function createMockApi(overrides: Partial = {}): MockApi { return { + sync: vi.fn().mockResolvedValue({}), // Tasks getTasks: vi.fn().mockResolvedValue({ results: [], nextCursor: null }), getTask: vi.fn(), From c3d7afa36e619d355b8aa3ca3607058ce0f5d90e Mon Sep 17 00:00:00 2001 From: Rui Martins Date: Tue, 19 May 2026 11:34:32 +0100 Subject: [PATCH 3/3] Address section reorder review follow-up - Print section order after successful reorders - Share reorder command args in section tests --- src/commands/section/reorder.ts | 15 ++- src/commands/section/section.test.ts | 135 ++++++++------------------- 2 files changed, 51 insertions(+), 99 deletions(-) diff --git a/src/commands/section/reorder.ts b/src/commands/section/reorder.ts index 15949cc4..a1bf952c 100644 --- a/src/commands/section/reorder.ts +++ b/src/commands/section/reorder.ts @@ -102,11 +102,7 @@ export async function reorderSection(ref: string, options: ReorderSectionOptions if (options.dryRun) { console.log(`Would reorder "${target.name}": position ${oldIndex} → ${newIndex}`) - console.log('New section order:') - for (let index = 0; index < newOrder.length; index++) { - const marker = newOrder[index].id === target.id ? '→' : ' ' - console.log(` ${marker} ${index}: ${newOrder[index].name} (id:${newOrder[index].id})`) - } + printNewSectionOrder(newOrder, target.id) return } @@ -121,6 +117,15 @@ export async function reorderSection(ref: string, options: ReorderSectionOptions console.log( `Reordered "${target.name}" (id:${target.id}): position ${oldIndex} → ${newIndex} of ${sections.length - 1}.`, ) + printNewSectionOrder(newOrder, target.id) + } +} + +function printNewSectionOrder(newOrder: Section[], targetId: string): void { + console.log('New section order:') + for (let index = 0; index < newOrder.length; index++) { + const marker = newOrder[index].id === targetId ? '→' : ' ' + console.log(` ${marker} ${index}: ${newOrder[index].name} (id:${newOrder[index].id})`) } } diff --git a/src/commands/section/section.test.ts b/src/commands/section/section.test.ts index 27931473..12de7ce5 100644 --- a/src/commands/section/section.test.ts +++ b/src/commands/section/section.test.ts @@ -290,6 +290,8 @@ describe('section reorder', () => { fixtures.sections.review, fixtures.sections.done, ] + const reorderCommandArgs = ['node', 'td', 'section', 'reorder'] as const + const projectArgs = ['--project', 'id:proj-work'] as const beforeEach(() => { vi.clearAllMocks() @@ -311,7 +313,7 @@ describe('section reorder', () => { const program = createProgram() await expect( - program.parseAsync(['node', 'td', 'section', 'reorder', 'Review', '--position', '0']), + program.parseAsync([...reorderCommandArgs, 'Review', '--position', '0']), ).rejects.toHaveProperty('code', 'MISSING_PROJECT') }) @@ -319,15 +321,7 @@ describe('section reorder', () => { const program = createProgram() await expect( - program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', - 'Review', - '--project', - 'id:proj-work', - ]), + program.parseAsync([...reorderCommandArgs, 'Review', ...projectArgs]), ).rejects.toHaveProperty('code', 'INVALID_OPTIONS') }) @@ -335,14 +329,10 @@ describe('section reorder', () => { const program = createProgram() await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, '--section', 'Review', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '0', ]) @@ -360,15 +350,11 @@ describe('section reorder', () => { await expect( program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Review', '--section', 'Done', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '0', ]), @@ -380,13 +366,9 @@ describe('section reorder', () => { await expect( program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Review', - '--project', - 'id:proj-work', + ...projectArgs, '--before', 'Done', '--position', @@ -400,13 +382,9 @@ describe('section reorder', () => { await expect( program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Review', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '1.5', ]), @@ -417,13 +395,9 @@ describe('section reorder', () => { const program = createProgram() await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Review', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '0', ]) @@ -434,19 +408,24 @@ describe('section reorder', () => { { id: 'sec-2', sectionOrder: 3 }, { id: 'sec-4', sectionOrder: 4 }, ]) + const logLines = (consoleSpy.mock.calls as unknown[][]).map((call) => call[0]) + expect(logLines).toEqual([ + 'Reordered "Review" (id:sec-3): position 2 → 0 of 3.', + 'New section order:', + ' → 0: Review (id:sec-3)', + ' 1: Planning (id:sec-1)', + ' 2: In Progress (id:sec-2)', + ' 3: Done (id:sec-4)', + ]) }) it('--position clamps to last when out of range', async () => { const program = createProgram() await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Planning', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '999', ]) @@ -472,13 +451,9 @@ describe('section reorder', () => { }) await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Review', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '0', ]) @@ -505,13 +480,9 @@ describe('section reorder', () => { const program = createProgram() await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Done', - '--project', - 'id:proj-work', + ...projectArgs, '--before', 'In Progress', ]) @@ -528,13 +499,9 @@ describe('section reorder', () => { const program = createProgram() await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Planning', - '--project', - 'id:proj-work', + ...projectArgs, '--after', 'Review', ]) @@ -552,13 +519,9 @@ describe('section reorder', () => { await expect( program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Review', - '--project', - 'id:proj-work', + ...projectArgs, '--before', 'Review', ]), @@ -569,13 +532,9 @@ describe('section reorder', () => { const program = createProgram() await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Planning', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '2', '--dry-run', @@ -597,13 +556,9 @@ describe('section reorder', () => { const program = createProgram() await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'In Progress', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '1', ]) @@ -615,13 +570,9 @@ describe('section reorder', () => { const program = createProgram() await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'In Progress', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '1', '--json', @@ -642,13 +593,9 @@ describe('section reorder', () => { const program = createProgram() await program.parseAsync([ - 'node', - 'td', - 'section', - 'reorder', + ...reorderCommandArgs, 'Review', - '--project', - 'id:proj-work', + ...projectArgs, '--position', '0', '--json',