Skip to content

Commit da54d0c

Browse files
authored
Add --list flag to todos position for cross-list moves (#372)
* Add --list flag to todos position for cross-list moves Support moving a todo to a different todolist within the same project via the reposition endpoint's parent_id field. Detects and rejects cross-project moves with guidance. Blocked on SDK exposing parentID on Reposition — will compile after rebasing onto main with the new SDK release. Closes #337 * Support todolist name resolution and improve cross-project validation - Resolve --list via resolveTodolistInTodoset so names like "Sprint 1" work, not just numeric IDs and URLs - Use project context from todo URL, --in flag, and config for both name resolution and cross-project validation - Use ErrUsageHint for the cross-project error to show actionable guidance - Update help text and SKILL.md placeholder to reflect name support * Resolve project name before cross-project comparison Project context from --in may be a name like "myproject" while the list URL yields a numeric project ID. Resolve the name to an ID before comparing, so --in myproject --list <same-project-url> doesn't falsely reject. Also consolidates project resolution: the resolved ID is reused for both cross-project validation and todolist name resolution. * Update surface snapshot with --list flag on todos position * Add tests for --list flag on todos position Cover the new cross-list move path: - Cross-project URL rejection - Same-project URL passes validation - List name without project context requires --in - Cross-project via config project ID * Narrow cross-project guard to URL-sourced projects, include todolist_id in response - Only fire the cross-project check when the todo's project comes from its URL, not from config/flags. Config project is a default context that may not match where a bare-ID todo actually lives. - Include todolist_id in JSON response when --list is used so automation can confirm the cross-list move. * Defer project resolution to when it's actually needed ResolveProject was called unconditionally when --list was provided and the project context was non-numeric, even when the list was already a numeric ID and no resolution was needed. This made otherwise valid moves fail if the configured project name was stale. Now only resolves when cross-project URL validation or todolist name resolution actually requires the numeric project ID. * Include --page in notifications list breadcrumb when not on first page The read breadcrumb from 'notifications list --page 2' said 'basecamp notifications read <id>' without --page, leading users into a "not found" error because read defaults to page 0. Now carries the page context forward. * Guard against empty list ID from malformed todolist URLs extractWithProject can return an empty recording ID for URLs that parse but have no recording segment (e.g. project URLs). Fail fast with a targeted error instead of falling through to a confusing "Invalid todolist ID". * Validate --list URL type to reject non-todolist URLs A todo URL or project URL passed as --list would silently extract the wrong ID and use it as a todolist ID. Now validates via urlarg.Parse that the URL is actually a todolists URL (correct type, not a collection, has a recording ID).
1 parent a142e7e commit da54d0c

5 files changed

Lines changed: 191 additions & 10 deletions

File tree

.surface

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13320,6 +13320,7 @@ FLAG basecamp todos move --ids-only type=bool
1332013320
FLAG basecamp todos move --in type=string
1332113321
FLAG basecamp todos move --jq type=string
1332213322
FLAG basecamp todos move --json type=bool
13323+
FLAG basecamp todos move --list type=string
1332313324
FLAG basecamp todos move --markdown type=bool
1332413325
FLAG basecamp todos move --md type=bool
1332513326
FLAG basecamp todos move --no-hints type=bool
@@ -13343,6 +13344,7 @@ FLAG basecamp todos position --ids-only type=bool
1334313344
FLAG basecamp todos position --in type=string
1334413345
FLAG basecamp todos position --jq type=string
1334513346
FLAG basecamp todos position --json type=bool
13347+
FLAG basecamp todos position --list type=string
1334613348
FLAG basecamp todos position --markdown type=bool
1334713349
FLAG basecamp todos position --md type=bool
1334813350
FLAG basecamp todos position --no-hints type=bool
@@ -13387,6 +13389,7 @@ FLAG basecamp todos reorder --ids-only type=bool
1338713389
FLAG basecamp todos reorder --in type=string
1338813390
FLAG basecamp todos reorder --jq type=string
1338913391
FLAG basecamp todos reorder --json type=bool
13392+
FLAG basecamp todos reorder --list type=string
1339013393
FLAG basecamp todos reorder --markdown type=bool
1339113394
FLAG basecamp todos reorder --md type=bool
1339213395
FLAG basecamp todos reorder --no-hints type=bool

internal/commands/notifications.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,14 @@ func runNotificationsList(cmd *cobra.Command, page int32) error {
7878
if page == 0 {
7979
nextPage = 2
8080
}
81+
readCmd := "basecamp notifications read <id>"
82+
if page > 0 {
83+
readCmd = fmt.Sprintf("basecamp notifications read <id> --page %d", page)
84+
}
8185
breadcrumbs := []output.Breadcrumb{
8286
{
8387
Action: "read",
84-
Cmd: "basecamp notifications read <id>",
88+
Cmd: readCmd,
8589
Description: "Mark as read",
8690
},
8791
{

internal/commands/todos.go

Lines changed: 96 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/basecamp/basecamp-cli/internal/dateparse"
1919
"github.com/basecamp/basecamp-cli/internal/output"
2020
"github.com/basecamp/basecamp-cli/internal/richtext"
21+
"github.com/basecamp/basecamp-cli/internal/urlarg"
2122
)
2223

2324
// todosListFlags holds the flags for the todos list command.
@@ -1788,17 +1789,26 @@ func reopenTodos(cmd *cobra.Command, todoIDs []string) error {
17881789
}
17891790

17901791
func newTodosPositionCmd() *cobra.Command {
1791-
var position int
1792+
var (
1793+
position int
1794+
list string
1795+
)
17921796

17931797
cmd := &cobra.Command{
17941798
Use: "position <id|url>",
17951799
Aliases: []string{"move", "reorder"},
1796-
Short: "Change todo position",
1797-
Long: `Reorder a todo within its todolist. Position is 1-based (1 = top).
1800+
Short: "Change todo position or move between lists",
1801+
Long: `Reorder a todo within its todolist, or move it to a different list in the
1802+
same project. Position is 1-based (1 = top).
17981803
17991804
You can pass either a todo ID or a Basecamp URL:
18001805
basecamp todos position 789 --to 1
1801-
basecamp todos position https://3.basecamp.com/123/buckets/456/todos/789 --to 1`,
1806+
basecamp todos position https://3.basecamp.com/123/buckets/456/todos/789 --to 1
1807+
1808+
Move to a different todolist in the same project:
1809+
basecamp todos position 789 --to 1 --list "Sprint 1" --in myproject
1810+
basecamp todos position 789 --to 1 --list 321
1811+
basecamp todos position <todo-url> --to 1 --list <todolist-url>`,
18021812
RunE: func(cmd *cobra.Command, args []string) error {
18031813
if len(args) == 0 {
18041814
return missingArg(cmd, "<id|url>")
@@ -1817,21 +1827,97 @@ You can pass either a todo ID or a Basecamp URL:
18171827
return output.ErrUsage("--to is required (1 = top)")
18181828
}
18191829

1820-
// Extract ID from URL if provided
1821-
todoIDStr := extractID(args[0])
1830+
// Extract todo ID and project from URL if provided
1831+
todoIDStr, todoProjectID := extractWithProject(args[0])
18221832

18231833
todoID, err := strconv.ParseInt(todoIDStr, 10, 64)
18241834
if err != nil {
18251835
return output.ErrUsage("Invalid todo ID")
18261836
}
18271837

1828-
err = app.Account().Todos().Reposition(cmd.Context(), todoID, position, nil)
1838+
// Resolve destination todolist when --list is provided
1839+
var parentID *int64
1840+
if list != "" {
1841+
listIDStr, listProjectID := extractWithProject(list)
1842+
1843+
// When --list is a URL, validate it's a todolist URL — not a
1844+
// todo, project, or collection URL that would silently extract
1845+
// the wrong ID.
1846+
if parsed := urlarg.Parse(list); parsed != nil {
1847+
if parsed.RecordingID == "" || parsed.Type != "todolists" || parsed.IsCollection {
1848+
return output.ErrUsage("Expected a todolist URL (.../todolists/<id>), " +
1849+
"or pass a todolist ID or name.")
1850+
}
1851+
}
1852+
1853+
// Build project context: todo URL > --in flag > config
1854+
project := todoProjectID
1855+
if project == "" {
1856+
project = app.Flags.Project
1857+
}
1858+
if project == "" {
1859+
project = app.Config.ProjectID
1860+
}
1861+
1862+
// Resolve project name to numeric ID only when needed:
1863+
// cross-project URL validation or todolist name resolution.
1864+
resolvedProject := project
1865+
needsResolve := (todoProjectID != "" && listProjectID != "") || !isNumeric(listIDStr)
1866+
if needsResolve && project != "" && !isNumeric(project) {
1867+
rp, _, resolveErr := app.Names.ResolveProject(cmd.Context(), project)
1868+
if resolveErr != nil {
1869+
return resolveErr
1870+
}
1871+
resolvedProject = rp
1872+
}
1873+
1874+
// Cross-project moves are not supported by the reposition endpoint.
1875+
// Only enforce when the todo's project comes from its URL (high
1876+
// confidence). Config/flag project is a default context — it may
1877+
// not match where a bare-ID todo actually lives.
1878+
if todoProjectID != "" && listProjectID != "" && resolvedProject != listProjectID {
1879+
return output.ErrUsageHint(
1880+
"Cannot move a todo to a list in a different project.",
1881+
"Pass a todolist from the same project; cross-project moves are not supported.",
1882+
)
1883+
}
1884+
1885+
// Resolve todolist name to ID when not already numeric
1886+
if !isNumeric(listIDStr) {
1887+
if resolvedProject == "" {
1888+
return output.ErrUsage("--in is required to resolve todolist names")
1889+
}
1890+
resolved, resolveErr := resolveTodolistInTodoset(cmd, app, listIDStr, resolvedProject, "")
1891+
if resolveErr != nil {
1892+
return resolveErr
1893+
}
1894+
listIDStr = resolved
1895+
}
1896+
1897+
listID, parseErr := strconv.ParseInt(listIDStr, 10, 64)
1898+
if parseErr != nil {
1899+
return output.ErrUsage("Invalid todolist ID")
1900+
}
1901+
parentID = &listID
1902+
}
1903+
1904+
err = app.Account().Todos().Reposition(cmd.Context(), todoID, position, parentID)
18291905
if err != nil {
18301906
return convertSDKError(err)
18311907
}
18321908

1833-
return app.OK(map[string]any{"repositioned": true, "position": position},
1834-
output.WithSummary(fmt.Sprintf("Moved todo #%d to position %d", todoID, position)),
1909+
summary := fmt.Sprintf("Moved todo #%d to position %d", todoID, position)
1910+
if parentID != nil {
1911+
summary = fmt.Sprintf("Moved todo #%d to list #%d at position %d", todoID, *parentID, position)
1912+
}
1913+
1914+
response := map[string]any{"repositioned": true, "position": position}
1915+
if parentID != nil {
1916+
response["todolist_id"] = *parentID
1917+
}
1918+
1919+
return app.OK(response,
1920+
output.WithSummary(summary),
18351921
output.WithBreadcrumbs(
18361922
output.Breadcrumb{
18371923
Action: "show",
@@ -1845,6 +1931,7 @@ You can pass either a todo ID or a Basecamp URL:
18451931

18461932
cmd.Flags().IntVar(&position, "to", 0, "Target position, 1-based (1 = top)")
18471933
cmd.Flags().IntVar(&position, "position", 0, "Target position (alias for --to)")
1934+
cmd.Flags().StringVarP(&list, "list", "l", "", "Destination todolist ID, name, or URL (move to a different list)")
18481935

18491936
return cmd
18501937
}

internal/commands/todos_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,92 @@ func TestTodosPositionRequiresPosition(t *testing.T) {
184184
assert.Equal(t, "--to is required (1 = top)", err.Error())
185185
}
186186

187+
// TestTodosPositionRejectsCrossProjectListURL tests that --list with a URL from a
188+
// different project than the todo URL is rejected with a clear error.
189+
func TestTodosPositionRejectsCrossProjectListURL(t *testing.T) {
190+
app, _ := setupTodosTestApp(t)
191+
192+
cmd := NewTodosCmd()
193+
194+
err := executeTodosCommand(cmd, app, "position",
195+
"https://3.basecamp.com/99999/buckets/100/todos/789",
196+
"--to", "1",
197+
"--list", "https://3.basecamp.com/99999/buckets/200/todolists/321",
198+
)
199+
require.Error(t, err)
200+
assert.Contains(t, err.Error(), "Cannot move a todo to a list in a different project")
201+
}
202+
203+
// TestTodosPositionAcceptsSameProjectListURL tests that --list with a URL from the
204+
// same project as the todo URL passes the cross-project check (fails at network,
205+
// not at validation).
206+
func TestTodosPositionAcceptsSameProjectListURL(t *testing.T) {
207+
app, _ := setupTodosTestApp(t)
208+
209+
cmd := NewTodosCmd()
210+
211+
err := executeTodosCommand(cmd, app, "position",
212+
"https://3.basecamp.com/99999/buckets/100/todos/789",
213+
"--to", "1",
214+
"--list", "https://3.basecamp.com/99999/buckets/100/todolists/321",
215+
)
216+
// Should pass validation and fail at the SDK call (network disabled),
217+
// not at the cross-project check.
218+
require.Error(t, err)
219+
assert.NotContains(t, err.Error(), "different project")
220+
}
221+
222+
// TestTodosPositionListNameRequiresProject tests that --list with a name (not numeric)
223+
// requires a project context via --in or config.
224+
func TestTodosPositionListNameRequiresProject(t *testing.T) {
225+
app, _ := setupTodosTestApp(t)
226+
227+
cmd := NewTodosCmd()
228+
229+
err := executeTodosCommand(cmd, app, "position", "789",
230+
"--to", "1",
231+
"--list", "Sprint 1",
232+
)
233+
require.Error(t, err)
234+
assert.Contains(t, err.Error(), "--in is required to resolve todolist names")
235+
}
236+
237+
// TestTodosPositionBareIDSkipsCrossProjectGuard tests that the cross-project
238+
// guard does not fire when the todo is a bare ID. Config project is a default
239+
// context that may not match where the todo actually lives.
240+
func TestTodosPositionBareIDSkipsCrossProjectGuard(t *testing.T) {
241+
app, _ := setupTodosTestApp(t)
242+
app.Config.ProjectID = "100"
243+
244+
cmd := NewTodosCmd()
245+
246+
// Todo is bare ID (config project = "100"), list URL has project 200.
247+
// Should NOT reject — bare ID means we don't know the todo's project.
248+
err := executeTodosCommand(cmd, app, "position", "789",
249+
"--to", "1",
250+
"--list", "https://3.basecamp.com/99999/buckets/200/todolists/321",
251+
)
252+
require.Error(t, err)
253+
assert.NotContains(t, err.Error(), "different project")
254+
}
255+
256+
// TestTodosPositionRejectsNonTodolistURL tests that --list rejects URLs that
257+
// aren't todolist URLs (e.g. todo URLs, project URLs).
258+
func TestTodosPositionRejectsNonTodolistURL(t *testing.T) {
259+
app, _ := setupTodosTestApp(t)
260+
261+
cmd := NewTodosCmd()
262+
263+
// A todo URL, not a todolist URL — should not silently use the todo ID
264+
err := executeTodosCommand(cmd, app, "position",
265+
"https://3.basecamp.com/99999/buckets/100/todos/789",
266+
"--to", "1",
267+
"--list", "https://3.basecamp.com/99999/buckets/100/todos/555",
268+
)
269+
require.Error(t, err)
270+
assert.Contains(t, err.Error(), "todolist URL")
271+
}
272+
187273
// TestTodoShortcutRequiresContent tests that todo shortcut requires content.
188274
func TestTodoShortcutShowsHelpWithoutContent(t *testing.T) {
189275
app, _ := setupTodosTestApp(t)

skills/basecamp/SKILL.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,7 @@ basecamp unassign <id> [id...] --card --from <person> --in <project> # Remove ca
424424
basecamp assign <id> [id...] --step --to <person> --in <project> # Assign card step
425425
basecamp unassign <id> [id...] --step --from <person> --in <project> # Remove step assignee
426426
basecamp todos position <id> --to 1 # Move to top
427+
basecamp todos position <id> --to 1 --list <id|name|url> # Move to different list
427428
basecamp todos sweep --overdue --complete --comment "Done" --in <project>
428429
```
429430

0 commit comments

Comments
 (0)