From 4e232e9e303d8f327a3fdb1c6e93a9857137a349 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 31 Jul 2026 19:53:30 -0700 Subject: [PATCH 1/8] Adopt basecamp-sdk v0.12.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eleven Everything() aggregate methods gained a trailing *EverythingTaskFilters parameter: the nine paginated todo and card selectors, plus the two unpaginated overdue endpoints. Pass nil at every call site, so this bump is behavior-preserving — no test changes, and the account-wide listings issue exactly the requests they did before. The filters themselves are the point of the signature change, but threading real values through belongs with the flags that produce them. This commit is the mechanical half. SDK v0.12.0 is 7e2925d25078; the API provenance moves to bc3 d0edc1283b23. --- go.mod | 2 +- go.sum | 4 ++-- internal/commands/cards.go | 12 ++++++------ internal/commands/todos.go | 10 +++++----- internal/version/sdk-provenance.json | 10 +++++----- nix/package.nix | 2 +- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/go.mod b/go.mod index 68f5656b..8441769a 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.1.1 charm.land/bubbletea/v2 v2.0.8 charm.land/lipgloss/v2 v2.0.5 - github.com/basecamp/basecamp-sdk/go v0.11.0 + github.com/basecamp/basecamp-sdk/go v0.12.0 github.com/basecamp/cli v0.2.2-0.20260728023309-04e401b12c6c github.com/charmbracelet/bubbles v1.0.0 github.com/charmbracelet/glamour v1.0.0 diff --git a/go.sum b/go.sum index f0469f4e..b89a7152 100644 --- a/go.sum +++ b/go.sum @@ -23,8 +23,8 @@ github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= -github.com/basecamp/basecamp-sdk/go v0.11.0 h1:bjbwcjEZIAUh93PF/Mr/+KSnQStmhL5yB903OBTVBoM= -github.com/basecamp/basecamp-sdk/go v0.11.0/go.mod h1:r83ralDQ0q9vbAby5qQ5x9hgCgUdJLDLHYpiU6jaFjE= +github.com/basecamp/basecamp-sdk/go v0.12.0 h1:N+sdsz109J5PUu8AZM5UT7vS00Z76s+zG4ItFDeKXkM= +github.com/basecamp/basecamp-sdk/go v0.12.0/go.mod h1:r83ralDQ0q9vbAby5qQ5x9hgCgUdJLDLHYpiU6jaFjE= github.com/basecamp/cli v0.2.2-0.20260728023309-04e401b12c6c h1:+5sQBl8sqYoD1Qhwsibn8sBCKWPyZ9NDez6mnuo9Afo= github.com/basecamp/cli v0.2.2-0.20260728023309-04e401b12c6c/go.mod h1:EK1Dba6DEw8ZAilVBpf/jri3ONDV7LQkLACSDe73f/c= github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= diff --git a/internal/commands/cards.go b/internal/commands/cards.go index 95facaef..57de3c13 100644 --- a/internal/commands/cards.go +++ b/internal/commands/cards.go @@ -313,15 +313,15 @@ func runCardsListAccountWide(cmd *cobra.Command, app *appctx.App, opts cardsList ) switch selector { case cardsSelectorCompleted: - groupsPage, err = everything.CompletedCards(cmd.Context(), page) + groupsPage, err = everything.CompletedCards(cmd.Context(), page, nil) case cardsSelectorUnassigned: - groupsPage, err = everything.UnassignedCards(cmd.Context(), page) + groupsPage, err = everything.UnassignedCards(cmd.Context(), page, nil) case cardsSelectorNoDueDate: - groupsPage, err = everything.NoDueDateCards(cmd.Context(), page) + groupsPage, err = everything.NoDueDateCards(cmd.Context(), page, nil) case cardsSelectorNotNow: - groupsPage, err = everything.NotNowCards(cmd.Context(), page) + groupsPage, err = everything.NotNowCards(cmd.Context(), page, nil) default: - groupsPage, err = everything.OpenCards(cmd.Context(), page) + groupsPage, err = everything.OpenCards(cmd.Context(), page, nil) } if err != nil { return nil, basecamp.ListMeta{}, convertSDKError(err) @@ -388,7 +388,7 @@ func runCardsListOverdue(cmd *cobra.Command, app *appctx.App, opts cardsListOpti return output.ErrUsage("--sort position requires --column (position is per-column)") } - cards, err := app.Account().Everything().OverdueCards(cmd.Context()) + cards, err := app.Account().Everything().OverdueCards(cmd.Context(), nil) if err != nil { return convertSDKError(err) } diff --git a/internal/commands/todos.go b/internal/commands/todos.go index b0003186..e87ca193 100644 --- a/internal/commands/todos.go +++ b/internal/commands/todos.go @@ -427,7 +427,7 @@ func listOverdueTodosAcrossProjects(cmd *cobra.Command, app *appctx.App, flags t return output.ErrUsage("--sort position requires --list (position is per-todolist)") } - todos, err := app.Account().Everything().OverdueTodos(cmd.Context()) + todos, err := app.Account().Everything().OverdueTodos(cmd.Context(), nil) if err != nil { return convertSDKError(err) } @@ -502,13 +502,13 @@ func fetchAccountWideTodoGroups(ctx context.Context, app *appctx.App, filter tod everything := app.Account().Everything() switch filter { case todosFilterCompleted: - return everything.CompletedTodos(ctx, page) + return everything.CompletedTodos(ctx, page, nil) case todosFilterUnassigned: - return everything.UnassignedTodos(ctx, page) + return everything.UnassignedTodos(ctx, page, nil) case todosFilterNoDueDate: - return everything.NoDueDateTodos(ctx, page) + return everything.NoDueDateTodos(ctx, page, nil) default: - return everything.OpenTodos(ctx, page) + return everything.OpenTodos(ctx, page, nil) } } diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 1a889392..9fa578c1 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,13 +1,13 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.11.0", - "revision": "b7124ca6a62a", - "updated_at": "2026-07-31T09:00:51Z" + "version": "v0.12.0", + "revision": "7e2925d25078", + "updated_at": "2026-08-01T00:57:46Z" }, "api": { "repo": "basecamp/bc3", - "revision": "e83b273363891ff060f0c818f542916c681f1bfd", - "synced_at": "2026-07-30" + "revision": "d0edc1283b231c58b7c88b014df5f8d231b1f7c8", + "synced_at": "2026-07-31" } } diff --git a/nix/package.nix b/nix/package.nix index c2bcaefa..7d77c397 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -8,7 +8,7 @@ buildGoModule.override { go = go_1_26; } (finalAttrs: { src = lib.cleanSource ./..; # To update: set to lib.fakeHash, run `nix build`, use the hash from the error. - vendorHash = "sha256-DmCW1Ms9TmcvEzvNIcqutf1bxtOpN+MjpPO0XP1VaZQ="; + vendorHash = "sha256-+j9bY0gSONYBIYZG9uaKgyISp6QUqNl9n8WRZWIPUKI="; subPackages = [ "cmd/basecamp" ]; From 746201f2c5e5ecb582e346784d0c0d59a1f9c6b2 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 31 Jul 2026 20:01:16 -0700 Subject: [PATCH 2/8] Add bookmarks and drafts, the two personal feeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both are /my/ listings: private to the authenticated user, spanning every project, with no project to scope them to and so no --project flag. They are bounded like the account-wide listings, and for the same reason. Their List methods take a page where 0 means "follow the Link header across every page", so a default of "fetch page 0, then trim to --limit" would re-import the fetch-everything-then-truncate defect PR #590 spent a release removing. Instead both reuse accountWideCollect: the default walks positive pages to a cap of 100, --limit walks to N, --page N is exactly one request, and --all is the only path that reaches page 0. Both listings flatten their own display rows. The generic renderer skips nested objects during column detection, so a generic render of a Bookmark shows an id and a timestamp while dropping the bookmarked recording entirely, and a Draft loses the project it belongs to. A draft's nil parent and nil scheduled time are display states rather than gaps, and render as "project root" and "not scheduled" instead of blanks. bookmarks check reports its answer rather than signalling it through the exit code: both true and false exit 0. Exit codes here classify failures, so spending a nonzero code on "not bookmarked" would make a successful answer indistinguishable from a real error. add and remove are both idempotent server-side, which the help text says rather than making callers check first. The pagination combination rules move into a shared validator. The eight existing listings keep their inline copies — rewriting working call sites to prove a helper is a bigger diff than the helper earns. --- .surface | 170 ++++++++++++++ e2e/smoke/smoke_bookmarks.bats | 51 +++++ e2e/smoke/smoke_drafts.bats | 32 +++ internal/cli/root.go | 2 + internal/commands/accountwide.go | 28 +++ internal/commands/bookmarks.go | 343 ++++++++++++++++++++++++++++ internal/commands/bookmarks_test.go | 289 +++++++++++++++++++++++ internal/commands/commands.go | 7 + internal/commands/commands_test.go | 2 + internal/commands/drafts.go | 178 +++++++++++++++ internal/commands/drafts_test.go | 147 ++++++++++++ 11 files changed, 1249 insertions(+) create mode 100644 e2e/smoke/smoke_bookmarks.bats create mode 100644 e2e/smoke/smoke_drafts.bats create mode 100644 internal/commands/bookmarks.go create mode 100644 internal/commands/bookmarks_test.go create mode 100644 internal/commands/drafts.go create mode 100644 internal/commands/drafts_test.go diff --git a/.surface b/.surface index c0273e93..60bb7027 100644 --- a/.surface +++ b/.surface @@ -16,6 +16,9 @@ ARG basecamp bonfire layout load 00 ARG basecamp bonfire layout save 00 ARG basecamp bonfire layout save 01 ... ARG basecamp bonfire split 00 +ARG basecamp bookmarks add 00 +ARG basecamp bookmarks check 00 +ARG basecamp bookmarks remove 00 ARG basecamp boost create 00 ARG basecamp boost create 01 ARG basecamp boost delete 00 @@ -464,6 +467,11 @@ CMD basecamp bonfire layout list CMD basecamp bonfire layout load CMD basecamp bonfire layout save CMD basecamp bonfire split +CMD basecamp bookmarks +CMD basecamp bookmarks add +CMD basecamp bookmarks check +CMD basecamp bookmarks list +CMD basecamp bookmarks remove CMD basecamp boost CMD basecamp boost create CMD basecamp boost delete @@ -648,6 +656,8 @@ CMD basecamp documents vault list CMD basecamp documents vaults CMD basecamp documents vaults create CMD basecamp documents vaults list +CMD basecamp drafts +CMD basecamp drafts list CMD basecamp events CMD basecamp file CMD basecamp file archive @@ -1995,6 +2005,114 @@ FLAG basecamp bonfire split --stats type=bool FLAG basecamp bonfire split --styled type=bool FLAG basecamp bonfire split --todolist type=string FLAG basecamp bonfire split --verbose type=count +FLAG basecamp bookmarks --account type=string +FLAG basecamp bookmarks --agent type=bool +FLAG basecamp bookmarks --cache-dir type=string +FLAG basecamp bookmarks --count type=bool +FLAG basecamp bookmarks --help type=bool +FLAG basecamp bookmarks --hints type=bool +FLAG basecamp bookmarks --ids-only type=bool +FLAG basecamp bookmarks --in type=string +FLAG basecamp bookmarks --jq type=string +FLAG basecamp bookmarks --json type=bool +FLAG basecamp bookmarks --markdown type=bool +FLAG basecamp bookmarks --md type=bool +FLAG basecamp bookmarks --no-hints type=bool +FLAG basecamp bookmarks --no-stats type=bool +FLAG basecamp bookmarks --profile type=string +FLAG basecamp bookmarks --project type=string +FLAG basecamp bookmarks --quiet type=bool +FLAG basecamp bookmarks --stats type=bool +FLAG basecamp bookmarks --styled type=bool +FLAG basecamp bookmarks --todolist type=string +FLAG basecamp bookmarks --verbose type=count +FLAG basecamp bookmarks add --account type=string +FLAG basecamp bookmarks add --agent type=bool +FLAG basecamp bookmarks add --cache-dir type=string +FLAG basecamp bookmarks add --count type=bool +FLAG basecamp bookmarks add --help type=bool +FLAG basecamp bookmarks add --hints type=bool +FLAG basecamp bookmarks add --ids-only type=bool +FLAG basecamp bookmarks add --in type=string +FLAG basecamp bookmarks add --jq type=string +FLAG basecamp bookmarks add --json type=bool +FLAG basecamp bookmarks add --markdown type=bool +FLAG basecamp bookmarks add --md type=bool +FLAG basecamp bookmarks add --no-hints type=bool +FLAG basecamp bookmarks add --no-stats type=bool +FLAG basecamp bookmarks add --profile type=string +FLAG basecamp bookmarks add --project type=string +FLAG basecamp bookmarks add --quiet type=bool +FLAG basecamp bookmarks add --stats type=bool +FLAG basecamp bookmarks add --styled type=bool +FLAG basecamp bookmarks add --todolist type=string +FLAG basecamp bookmarks add --verbose type=count +FLAG basecamp bookmarks check --account type=string +FLAG basecamp bookmarks check --agent type=bool +FLAG basecamp bookmarks check --cache-dir type=string +FLAG basecamp bookmarks check --count type=bool +FLAG basecamp bookmarks check --help type=bool +FLAG basecamp bookmarks check --hints type=bool +FLAG basecamp bookmarks check --ids-only type=bool +FLAG basecamp bookmarks check --in type=string +FLAG basecamp bookmarks check --jq type=string +FLAG basecamp bookmarks check --json type=bool +FLAG basecamp bookmarks check --markdown type=bool +FLAG basecamp bookmarks check --md type=bool +FLAG basecamp bookmarks check --no-hints type=bool +FLAG basecamp bookmarks check --no-stats type=bool +FLAG basecamp bookmarks check --profile type=string +FLAG basecamp bookmarks check --project type=string +FLAG basecamp bookmarks check --quiet type=bool +FLAG basecamp bookmarks check --stats type=bool +FLAG basecamp bookmarks check --styled type=bool +FLAG basecamp bookmarks check --todolist type=string +FLAG basecamp bookmarks check --verbose type=count +FLAG basecamp bookmarks list --account type=string +FLAG basecamp bookmarks list --agent type=bool +FLAG basecamp bookmarks list --all type=bool +FLAG basecamp bookmarks list --cache-dir type=string +FLAG basecamp bookmarks list --count type=bool +FLAG basecamp bookmarks list --help type=bool +FLAG basecamp bookmarks list --hints type=bool +FLAG basecamp bookmarks list --ids-only type=bool +FLAG basecamp bookmarks list --in type=string +FLAG basecamp bookmarks list --jq type=string +FLAG basecamp bookmarks list --json type=bool +FLAG basecamp bookmarks list --limit type=int +FLAG basecamp bookmarks list --markdown type=bool +FLAG basecamp bookmarks list --md type=bool +FLAG basecamp bookmarks list --no-hints type=bool +FLAG basecamp bookmarks list --no-stats type=bool +FLAG basecamp bookmarks list --page type=int +FLAG basecamp bookmarks list --profile type=string +FLAG basecamp bookmarks list --project type=string +FLAG basecamp bookmarks list --quiet type=bool +FLAG basecamp bookmarks list --stats type=bool +FLAG basecamp bookmarks list --styled type=bool +FLAG basecamp bookmarks list --todolist type=string +FLAG basecamp bookmarks list --verbose type=count +FLAG basecamp bookmarks remove --account type=string +FLAG basecamp bookmarks remove --agent type=bool +FLAG basecamp bookmarks remove --cache-dir type=string +FLAG basecamp bookmarks remove --count type=bool +FLAG basecamp bookmarks remove --help type=bool +FLAG basecamp bookmarks remove --hints type=bool +FLAG basecamp bookmarks remove --ids-only type=bool +FLAG basecamp bookmarks remove --in type=string +FLAG basecamp bookmarks remove --jq type=string +FLAG basecamp bookmarks remove --json type=bool +FLAG basecamp bookmarks remove --markdown type=bool +FLAG basecamp bookmarks remove --md type=bool +FLAG basecamp bookmarks remove --no-hints type=bool +FLAG basecamp bookmarks remove --no-stats type=bool +FLAG basecamp bookmarks remove --profile type=string +FLAG basecamp bookmarks remove --project type=string +FLAG basecamp bookmarks remove --quiet type=bool +FLAG basecamp bookmarks remove --stats type=bool +FLAG basecamp bookmarks remove --styled type=bool +FLAG basecamp bookmarks remove --todolist type=string +FLAG basecamp bookmarks remove --verbose type=count FLAG basecamp boost --account type=string FLAG basecamp boost --agent type=bool FLAG basecamp boost --cache-dir type=string @@ -6405,6 +6523,51 @@ FLAG basecamp documents vaults list --styled type=bool FLAG basecamp documents vaults list --todolist type=string FLAG basecamp documents vaults list --vault type=string FLAG basecamp documents vaults list --verbose type=count +FLAG basecamp drafts --account type=string +FLAG basecamp drafts --agent type=bool +FLAG basecamp drafts --cache-dir type=string +FLAG basecamp drafts --count type=bool +FLAG basecamp drafts --help type=bool +FLAG basecamp drafts --hints type=bool +FLAG basecamp drafts --ids-only type=bool +FLAG basecamp drafts --in type=string +FLAG basecamp drafts --jq type=string +FLAG basecamp drafts --json type=bool +FLAG basecamp drafts --markdown type=bool +FLAG basecamp drafts --md type=bool +FLAG basecamp drafts --no-hints type=bool +FLAG basecamp drafts --no-stats type=bool +FLAG basecamp drafts --profile type=string +FLAG basecamp drafts --project type=string +FLAG basecamp drafts --quiet type=bool +FLAG basecamp drafts --stats type=bool +FLAG basecamp drafts --styled type=bool +FLAG basecamp drafts --todolist type=string +FLAG basecamp drafts --verbose type=count +FLAG basecamp drafts list --account type=string +FLAG basecamp drafts list --agent type=bool +FLAG basecamp drafts list --all type=bool +FLAG basecamp drafts list --cache-dir type=string +FLAG basecamp drafts list --count type=bool +FLAG basecamp drafts list --help type=bool +FLAG basecamp drafts list --hints type=bool +FLAG basecamp drafts list --ids-only type=bool +FLAG basecamp drafts list --in type=string +FLAG basecamp drafts list --jq type=string +FLAG basecamp drafts list --json type=bool +FLAG basecamp drafts list --limit type=int +FLAG basecamp drafts list --markdown type=bool +FLAG basecamp drafts list --md type=bool +FLAG basecamp drafts list --no-hints type=bool +FLAG basecamp drafts list --no-stats type=bool +FLAG basecamp drafts list --page type=int +FLAG basecamp drafts list --profile type=string +FLAG basecamp drafts list --project type=string +FLAG basecamp drafts list --quiet type=bool +FLAG basecamp drafts list --stats type=bool +FLAG basecamp drafts list --styled type=bool +FLAG basecamp drafts list --todolist type=string +FLAG basecamp drafts list --verbose type=count FLAG basecamp events --account type=string FLAG basecamp events --agent type=bool FLAG basecamp events --all type=bool @@ -16570,6 +16733,11 @@ SUB basecamp bonfire layout list SUB basecamp bonfire layout load SUB basecamp bonfire layout save SUB basecamp bonfire split +SUB basecamp bookmarks +SUB basecamp bookmarks add +SUB basecamp bookmarks check +SUB basecamp bookmarks list +SUB basecamp bookmarks remove SUB basecamp boost SUB basecamp boost create SUB basecamp boost delete @@ -16754,6 +16922,8 @@ SUB basecamp documents vault list SUB basecamp documents vaults SUB basecamp documents vaults create SUB basecamp documents vaults list +SUB basecamp drafts +SUB basecamp drafts list SUB basecamp events SUB basecamp file SUB basecamp file archive diff --git a/e2e/smoke/smoke_bookmarks.bats b/e2e/smoke/smoke_bookmarks.bats new file mode 100644 index 00000000..54b164ad --- /dev/null +++ b/e2e/smoke/smoke_bookmarks.bats @@ -0,0 +1,51 @@ +#!/usr/bin/env bats +# smoke_bookmarks.bats - Level 0: Personal bookmark operations + +load smoke_helper + +setup_file() { + ensure_token || return 1 +} + +@test "bookmarks list returns bookmarks" { + run_smoke basecamp bookmarks list --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "bookmarks list honors --limit" { + run_smoke basecamp bookmarks list --limit 1 --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "bookmarks list rejects --all with --limit" { + run_smoke basecamp bookmarks list --all --limit 1 --json + assert_failure +} + +@test "bookmarks check reports a boolean and exits 0" { + # check answers a question rather than signalling through the exit code, so + # a bookmarked recording must come back true *and* exit 0. + run_smoke basecamp bookmarks list --limit 1 --json + assert_success + local id + id=$(printf '%s' "$output" | jq -r '.data[0].recording.id // empty') + [[ -z "$id" ]] && mark_unverifiable "No bookmark exists to check against" + run_smoke basecamp bookmarks check "$id" --json + assert_success + assert_json_value '.data.bookmarked' 'true' +} + +@test "bookmarks check rejects a non-id argument" { + run_smoke basecamp bookmarks check not-an-id --json + assert_failure +} + +@test "bookmarks add is out of scope" { + mark_out_of_scope "Mutating - exercised by the live add/check/remove round-trip" +} + +@test "bookmarks remove is out of scope" { + mark_out_of_scope "Mutating - exercised by the live add/check/remove round-trip" +} diff --git a/e2e/smoke/smoke_drafts.bats b/e2e/smoke/smoke_drafts.bats new file mode 100644 index 00000000..c74b0fbe --- /dev/null +++ b/e2e/smoke/smoke_drafts.bats @@ -0,0 +1,32 @@ +#!/usr/bin/env bats +# smoke_drafts.bats - Level 0: Personal draft listing + +load smoke_helper + +setup_file() { + ensure_token || return 1 +} + +@test "drafts list returns drafts" { + run_smoke basecamp drafts list --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "drafts list honors --limit" { + run_smoke basecamp drafts list --limit 1 --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "drafts list rejects --page 0" { + # Page 0 is the SDK's "fetch every page" spelling. Only --all may reach it, + # so asking for it by number is a usage error rather than a full crawl. + run_smoke basecamp drafts list --page 0 --json + assert_failure +} + +@test "drafts list rejects --page with --all" { + run_smoke basecamp drafts list --page 1 --all --json + assert_failure +} diff --git a/internal/cli/root.go b/internal/cli/root.go index b81aefe0..d54937c5 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -339,6 +339,8 @@ func Execute() { cmd.AddCommand(commands.NewUploadCmd()) cmd.AddCommand(commands.NewGaugesCmd()) cmd.AddCommand(commands.NewAssignmentsCmd()) + cmd.AddCommand(commands.NewBookmarksCmd()) + cmd.AddCommand(commands.NewDraftsCmd()) cmd.AddCommand(commands.NewNotificationsCmd()) cmd.AddCommand(commands.NewTUICmd()) cmd.AddCommand(commands.NewBonfireCmd()) diff --git a/internal/commands/accountwide.go b/internal/commands/accountwide.go index 3647b3a4..6aabcda1 100644 --- a/internal/commands/accountwide.go +++ b/internal/commands/accountwide.go @@ -175,6 +175,34 @@ func accountWideCapNotice(capped bool, meta basecamp.ListMeta, count int, plural count, plural) } +// validateAccountWidePaginationFlags enforces the combination rules every +// bounded account-wide listing shares: --all and --limit both answer "how much", +// --page answers "which one", and mixing them asks for two different things at +// once. +// +// The older listings spell these rules out inline, one copy each. New listings +// call this instead; the existing copies are left alone rather than swept into +// this change, since rewriting eight working call sites to prove a helper is a +// bigger diff than the helper earns here. +func validateAccountWidePaginationFlags(cmd *cobra.Command, limit, page int, all bool) error { + if all && limit > 0 { + return output.ErrUsage("--all and --limit are mutually exclusive") + } + if page > 0 && (all || limit > 0) { + return output.ErrUsage("--page cannot be combined with --all or --limit") + } + if cmd.Flags().Changed("page") && page < 1 { + return output.ErrUsageHint( + "--page must be a positive page number", + "Omit --page, or pass --all, to follow every page", + ) + } + if limit < 0 { + return output.ErrUsage("--limit must be zero or positive") + } + return nil +} + // rejectScopedPaginationFlags refuses --limit/--page/--all on a path that has // no pagination to thread them onto. // diff --git a/internal/commands/bookmarks.go b/internal/commands/bookmarks.go new file mode 100644 index 00000000..e30db1ea --- /dev/null +++ b/internal/commands/bookmarks.go @@ -0,0 +1,343 @@ +package commands + +import ( + "fmt" + "strconv" + + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// NewBookmarksCmd creates the bookmarks command for the current user's personal +// bookmarks. +// +// Bookmarks are per-person: they are visible only to whoever created them, so +// there is no project to scope them to and no --project flag. Every leaf here +// addresses a recording by id or URL, since a bookmark is a link between the +// current user and one recording rather than a resource with an id of its own +// that anyone would paste. +func NewBookmarksCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "bookmarks", + Short: "Manage your personal bookmarks", + Long: `Manage your personal bookmarks. + +Bookmarks are private to you — nobody else can see what you have bookmarked. +Each one points at a single recording (a to-do, message, document, card, and +so on), addressed by its id or by pasting its Basecamp URL. + + basecamp bookmarks list + basecamp bookmarks add https://3.basecamp.com/1234567/buckets/89/todos/42 + basecamp bookmarks check 42 + basecamp bookmarks remove 42`, + Annotations: map[string]string{ + "agent_notes": "Account-wide and personal — no --in needed.\n" + + "add/remove are idempotent; check reports true/false and always exits 0.", + }, + } + + cmd.AddCommand( + newBookmarksListCmd(), + newBookmarksAddCmd(), + newBookmarksRemoveCmd(), + newBookmarksCheckCmd(), + ) + + return cmd +} + +func newBookmarksListCmd() *cobra.Command { + var ( + limit int + page int + all bool + ) + + cmd := &cobra.Command{ + Use: "list", + Short: "List your bookmarks", + Long: `List your bookmarks, most recently bookmarked first. + +This is a personal feed spanning every project, so the default is bounded: +it walks pages until it has ` + strconv.Itoa(accountWideDefaultLimit) + ` bookmarks rather than fetching the whole +listing and discarding most of it. --all is how you ask for every page.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + return runBookmarksList(cmd, limit, page, all) + }, + } + + cmd.Flags().IntVarP(&limit, "limit", "n", 0, "Maximum bookmarks to return") + cmd.Flags().IntVar(&page, "page", 0, "Return only this page") + cmd.Flags().BoolVar(&all, "all", false, "Fetch every page") + + return cmd +} + +func runBookmarksList(cmd *cobra.Command, limit, page int, all bool) error { + app := appctx.FromContext(cmd.Context()) + + if err := validateAccountWidePaginationFlags(cmd, limit, page, all); err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + fetch := func(p int32) ([]basecamp.Bookmark, basecamp.ListMeta, error) { + result, err := app.Account().Bookmarks().List(cmd.Context(), p) + if err != nil { + return nil, basecamp.ListMeta{}, convertSDKError(err) + } + return result.Bookmarks, result.Meta, nil + } + + // Page 0 is the SDK's "follow the Link header across every page", which is + // what --all asks for. No other path may reach it: a bounded default that + // fetched everything and then trimmed would be the same defect the + // account-wide listings were rewritten to remove. + var ( + bookmarks []basecamp.Bookmark + meta basecamp.ListMeta + capped bool + ) + if all || page > 0 { + sdkPage, err := accountWidePage(page, all) + if err != nil { + return err + } + if bookmarks, meta, err = fetch(sdkPage); err != nil { + return err + } + } else { + effectiveLimit := limit + if effectiveLimit == 0 { + effectiveLimit = accountWideDefaultLimit + } + var err error + if bookmarks, capped, meta, err = accountWideCollect(fetch, accountWideFlatCount[basecamp.Bookmark], effectiveLimit); err != nil { + return err + } + // The walk stops at a page boundary, so trim to the exact cap. + if len(bookmarks) > effectiveLimit { + bookmarks = bookmarks[:effectiveLimit] + } + } + + respOpts := accountWideRespOpts(len(bookmarks), "bookmark", "bookmarks", meta, limit > 0) + if notice := accountWideCapNotice(capped, meta, len(bookmarks), "bookmarks"); notice != "" { + respOpts = append(respOpts, output.WithNotice(notice)) + } + respOpts = append(respOpts, output.WithDisplayData(flattenBookmarks(bookmarks))) + respOpts = append(respOpts, + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "remove", + Cmd: "basecamp bookmarks remove ", + Description: "Remove a bookmark", + }, + output.Breadcrumb{ + Action: "show", + Cmd: "basecamp recordings show ", + Description: "View a bookmarked recording", + }, + ), + ) + + return app.OK(bookmarks, respOpts...) +} + +// flattenBookmarks builds the display rows for the bookmark listing. +// +// A Bookmark's own id and timestamps say nothing about what was bookmarked — +// the recording is nested, and the renderer's generic column detection skips +// nested objects. Rendering one generically therefore produces a table of ids +// and dates with the actual subject missing, so the rows are built by hand: +// the recording's id is what every other command takes as an argument, and the +// project is what makes a cross-project personal feed attributable. +func flattenBookmarks(bookmarks []basecamp.Bookmark) []map[string]any { + rows := make([]map[string]any, 0, len(bookmarks)) + for _, b := range bookmarks { + row := map[string]any{ + "id": b.Recording.ID, + "title": b.Recording.Title, + "type": b.Recording.Type, + "bookmarked_at": b.CreatedAt, + } + if b.Recording.Bucket != nil { + row["project"] = b.Recording.Bucket.Name + } + rows = append(rows, row) + } + return rows +} + +func newBookmarksAddCmd() *cobra.Command { + return &cobra.Command{ + Use: "add ", + Short: "Bookmark a recording", + Long: `Bookmark a recording so it shows up in your personal bookmarks. + +Idempotent: bookmarking something you have already bookmarked returns the +existing bookmark rather than failing or creating a duplicate. + + basecamp bookmarks add 42 + basecamp bookmarks add https://3.basecamp.com/1234567/buckets/89/todos/42`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + recordingID, err := bookmarkRecordingID(args[0]) + if err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + bookmark, err := app.Account().Bookmarks().Create(cmd.Context(), recordingID) + if err != nil { + return convertSDKError(err) + } + + return app.OK(bookmark, + output.WithSummary(fmt.Sprintf("Bookmarked %s", bookmarkLabel(bookmark.Recording))), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "list", + Cmd: "basecamp bookmarks list", + Description: "List your bookmarks", + }, + output.Breadcrumb{ + Action: "remove", + Cmd: fmt.Sprintf("basecamp bookmarks remove %d", recordingID), + Description: "Remove this bookmark", + }, + ), + ) + }, + } +} + +func newBookmarksRemoveCmd() *cobra.Command { + return &cobra.Command{ + Use: "remove ", + Short: "Remove a bookmark", + Long: `Remove a recording from your personal bookmarks. + +Idempotent: removing something you have not bookmarked also succeeds, so this +is safe to run without checking first. + + basecamp bookmarks remove 42`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + recordingID, err := bookmarkRecordingID(args[0]) + if err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + if err := app.Account().Bookmarks().Delete(cmd.Context(), recordingID); err != nil { + return convertSDKError(err) + } + + return app.OK(map[string]any{"id": recordingID, "bookmarked": false}, + output.WithSummary(fmt.Sprintf("Removed bookmark on recording %d", recordingID)), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "list", + Cmd: "basecamp bookmarks list", + Description: "List your bookmarks", + }, + ), + ) + }, + } +} + +func newBookmarksCheckCmd() *cobra.Command { + return &cobra.Command{ + Use: "check ", + Short: "Report whether you have bookmarked a recording", + Long: `Report whether you have bookmarked a recording. + +Reports the answer rather than signalling it through the exit code: both +outcomes exit 0, and "not bookmarked" is a successful answer. Exit codes here +mean a request failed, so reserving a nonzero code for "false" would be +indistinguishable from a real error. + + basecamp bookmarks check 42 + basecamp bookmarks check 42 --json # {"bookmarked": false}`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + recordingID, err := bookmarkRecordingID(args[0]) + if err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + bookmarked, err := app.Account().Bookmarks().Get(cmd.Context(), recordingID) + if err != nil { + return convertSDKError(err) + } + + summary := fmt.Sprintf("Recording %d is not bookmarked", recordingID) + breadcrumb := output.Breadcrumb{ + Action: "add", + Cmd: fmt.Sprintf("basecamp bookmarks add %d", recordingID), + Description: "Bookmark it", + } + if bookmarked { + summary = fmt.Sprintf("Recording %d is bookmarked", recordingID) + breadcrumb = output.Breadcrumb{ + Action: "remove", + Cmd: fmt.Sprintf("basecamp bookmarks remove %d", recordingID), + Description: "Remove this bookmark", + } + } + + return app.OK(map[string]any{"id": recordingID, "bookmarked": bookmarked}, + output.WithSummary(summary), + output.WithBreadcrumbs(breadcrumb), + ) + }, + } +} + +// bookmarkRecordingID resolves the positional every bookmark verb +// takes. With no way to browse bookmarkable recordings from this group, a +// pasted URL is the natural way to name one. +func bookmarkRecordingID(arg string) (int64, error) { + id, err := strconv.ParseInt(extractID(arg), 10, 64) + if err != nil { + return 0, output.ErrUsageHint( + fmt.Sprintf("%q is not a recording id or Basecamp URL", arg), + "Pass a numeric recording id, or paste the recording's Basecamp URL", + ) + } + return id, nil +} + +// bookmarkLabel names a recording for a summary line, falling back to the type +// and id when the projection carries no title. +func bookmarkLabel(r basecamp.Recording) string { + if r.Title != "" { + return fmt.Sprintf("%q", r.Title) + } + if r.Type != "" { + return fmt.Sprintf("%s %d", r.Type, r.ID) + } + return fmt.Sprintf("recording %d", r.ID) +} diff --git a/internal/commands/bookmarks_test.go b/internal/commands/bookmarks_test.go new file mode 100644 index 00000000..3fd6dca1 --- /dev/null +++ b/internal/commands/bookmarks_test.go @@ -0,0 +1,289 @@ +package commands + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "net/http" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" +) + +const bookmarksListPath = "/99999/my/bookmarks.json" + +func bookmarkRecordingPath(id int64) string { + return fmt.Sprintf("/99999/recordings/%d/bookmark.json", id) +} + +// bookmarksFeedBody builds n bookmarks, each wrapping a recording that carries +// a bucket — the nested field the display rows exist to surface. +func bookmarksFeedBody(n int) string { + items := make([]string, 0, n) + for i := 1; i <= n; i++ { + items = append(items, fmt.Sprintf(`{ + "id": %d, + "created_at": "2026-07-01T10:00:00.000Z", + "updated_at": "2026-07-01T10:00:00.000Z", + "recording": { + "id": %d, + "title": "Bookmarked item %d", + "type": "Todo", + "status": "active", + "created_at": "2026-06-01T10:00:00.000Z", + "updated_at": "2026-06-01T10:00:00.000Z", + "bucket": {"id": 977190, "name": "JD test proj", "type": "Project"} + } + }`, i, 1000+i, i)) + } + return "[" + strings.Join(items, ",") + "]" +} + +func bookmarksListRoute(n int) stubRoute { + return stubRoute{ + method: http.MethodGet, + path: bookmarksListPath, + status: http.StatusOK, + body: bookmarksFeedBody(n), + // The bounded walk stops on the first empty page, so a page-aware route + // is what distinguishes it from a full-account crawl. + pages: []string{bookmarksFeedBody(n)}, + } +} + +// setupPersonalFeedApp is the recording test app with its output swapped for a +// JSON envelope writer, so a test can assert on the transport and the rendered +// payload at once. +func setupPersonalFeedApp(t *testing.T, routes ...stubRoute) (*appctx.App, *recordingTransport, *bytes.Buffer) { + t.Helper() + app, transport := setupRecordingTestApp(t, routes...) + out := &bytes.Buffer{} + app.Output = output.New(output.Options{Format: output.FormatJSON, Writer: out}) + return app, transport, out +} + +// personalFeedEnvelope is the slice of the JSON success envelope these tests read. +type personalFeedEnvelope struct { + Data []json.RawMessage `json:"data"` + Summary string `json:"summary"` + Notice string `json:"notice"` +} + +func decodePersonalFeedEnvelope(t *testing.T, out *bytes.Buffer) personalFeedEnvelope { + t.Helper() + var envelope personalFeedEnvelope + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + return envelope +} + +func requireBookmarksUsageError(t *testing.T, err error) *output.Error { + t.Helper() + require.Error(t, err) + var outErr *output.Error + require.True(t, errors.As(err, &outErr)) + assert.Equal(t, output.CodeUsage, outErr.Code) + return outErr +} + +// The default must walk positive pages. Page 0 is the SDK's "fetch every page" +// spelling, and reaching it by default is the fetch-everything-then-truncate +// defect the account-wide listings were rewritten to remove. +func TestBookmarksListDefaultWalksPositivePages(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, bookmarksListRoute(2)) + + require.NoError(t, executeRecordingCommand(NewBookmarksCmd(), app, "list")) + + queries := transport.queriesFor(bookmarksListPath) + require.NotEmpty(t, queries) + for _, q := range queries { + assert.Contains(t, q, "page=", "no default request may omit page=") + assert.NotContains(t, q, "page=0") + } + assert.Equal(t, "page=1", queries[0]) +} + +func TestBookmarksListAllFetchesEveryPage(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, bookmarksListRoute(2)) + + require.NoError(t, executeRecordingCommand(NewBookmarksCmd(), app, "list", "--all")) + + queries := transport.queriesFor(bookmarksListPath) + require.Len(t, queries, 1, "--all is a single call into the SDK's own traversal") + assert.Empty(t, queries[0], "--all omits page= entirely") +} + +func TestBookmarksListPageIsExactlyOneRequest(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, bookmarksListRoute(2)) + + require.NoError(t, executeRecordingCommand(NewBookmarksCmd(), app, "list", "--page", "3")) + + assert.Equal(t, []string{"page=3"}, transport.queriesFor(bookmarksListPath)) +} + +func TestBookmarksListLimitTrimsExactly(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, bookmarksListRoute(5)) + + require.NoError(t, executeRecordingCommand(NewBookmarksCmd(), app, "list", "--limit", "2")) + + envelope := decodePersonalFeedEnvelope(t, out) + assert.Len(t, envelope.Data, 2) + assert.Equal(t, "2 bookmarks across all projects", envelope.Summary) +} + +func TestBookmarksListRejectsUnusablePaging(t *testing.T) { + assertRejected := func(t *testing.T, wantFragment string, args ...string) { + t.Helper() + app, transport, _ := setupPersonalFeedApp(t, bookmarksListRoute(2)) + + err := executeRecordingCommand(NewBookmarksCmd(), app, args...) + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Message, wantFragment) + assert.Empty(t, transport.recorded(), "a rejected listing must not reach the server") + } + + t.Run("explicit --page 0", func(t *testing.T) { + assertRejected(t, "--page", "list", "--page", "0") + }) + t.Run("negative --limit", func(t *testing.T) { + assertRejected(t, "--limit", "list", "--limit=-1") + }) + t.Run("--all with --limit", func(t *testing.T) { + assertRejected(t, "--limit", "list", "--all", "--limit", "5") + }) + t.Run("--page with --all", func(t *testing.T) { + assertRejected(t, "--page", "list", "--page", "2", "--all") + }) +} + +// The generic renderer skips nested objects, so a generic render of a Bookmark +// shows its own id and timestamp and drops the recording entirely — the one +// thing the row is about. These rows are built by hand for exactly that reason. +func TestFlattenBookmarksCarriesTheRecording(t *testing.T) { + rows := flattenBookmarks([]basecamp.Bookmark{{ + ID: 7, + CreatedAt: time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC), + Recording: basecamp.Recording{ + ID: 1001, + Title: "Bookmarked item 1", + Type: "Todo", + Bucket: &basecamp.Bucket{ID: 977190, Name: "JD test proj"}, + }, + }}) + + require.Len(t, rows, 1) + assert.Equal(t, int64(1001), rows[0]["id"], "the row's id is the recording's, not the bookmark's") + assert.Equal(t, "Bookmarked item 1", rows[0]["title"]) + assert.Equal(t, "Todo", rows[0]["type"]) + assert.Equal(t, "JD test proj", rows[0]["project"]) + assert.Contains(t, rows[0], "bookmarked_at") +} + +// check answers a question. Both answers are successes, so neither may be +// signalled through the exit code — that space belongs to real failures. +func TestBookmarksCheckReportsBothAnswersAsSuccess(t *testing.T) { + for _, tc := range []struct { + name string + body string + want bool + }{ + {"bookmarked", `{"bookmarked": true}`, true}, + {"not bookmarked", `{"bookmarked": false}`, false}, + } { + t.Run(tc.name, func(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, stubRoute{ + method: http.MethodGet, + path: bookmarkRecordingPath(42), + status: http.StatusOK, + body: tc.body, + }) + + err := executeRecordingCommand(NewBookmarksCmd(), app, "check", "42") + + require.NoError(t, err, "a false answer is still a successful call") + var envelope struct { + Data struct { + Bookmarked bool `json:"bookmarked"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + assert.Equal(t, tc.want, envelope.Data.Bookmarked) + }) + } +} + +func TestBookmarksCheckAcceptsAURL(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, stubRoute{ + method: http.MethodGet, + path: bookmarkRecordingPath(42), + status: http.StatusOK, + body: `{"bookmarked": true}`, + }) + + require.NoError(t, executeRecordingCommand(NewBookmarksCmd(), app, + "check", "https://3.basecamp.com/1234567/buckets/89/todos/42")) + + assert.Equal(t, bookmarkRecordingPath(42), transport.last(t).Path) +} + +func TestBookmarksVerbsRejectANonID(t *testing.T) { + for _, verb := range []string{"add", "remove", "check"} { + t.Run(verb, func(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t) + + err := executeRecordingCommand(NewBookmarksCmd(), app, verb, "not-an-id") + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Hint, "recording id") + assert.Empty(t, transport.recorded()) + }) + } +} + +func TestBookmarksAddPostsToTheRecording(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, stubRoute{ + method: http.MethodPost, + path: bookmarkRecordingPath(42), + status: http.StatusCreated, + body: `{ + "id": 7, + "created_at": "2026-07-01T10:00:00.000Z", + "updated_at": "2026-07-01T10:00:00.000Z", + "recording": { + "id": 42, "title": "Ship it", "type": "Todo", "status": "active", + "created_at": "2026-06-01T10:00:00.000Z", + "updated_at": "2026-06-01T10:00:00.000Z" + } + }`, + }) + + require.NoError(t, executeRecordingCommand(NewBookmarksCmd(), app, "add", "42")) + + call := transport.last(t) + assert.Equal(t, http.MethodPost, call.Method) + assert.Equal(t, bookmarkRecordingPath(42), call.Path) +} + +func TestBookmarksRemoveDeletesTheBookmark(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, stubRoute{ + method: http.MethodDelete, + path: bookmarkRecordingPath(42), + status: http.StatusNoContent, + body: "", + }) + + require.NoError(t, executeRecordingCommand(NewBookmarksCmd(), app, "remove", "42")) + + call := transport.last(t) + assert.Equal(t, http.MethodDelete, call.Method) + assert.Equal(t, bookmarkRecordingPath(42), call.Path) +} diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 67fcaa26..bada86bc 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -75,6 +75,13 @@ func CommandCategories() []CommandCategory { {Name: "assignments", Category: "scheduling", Description: "View my assignments", Actions: []string{"list", "completed", "due"}}, }, }, + { + Name: "Personal", + Commands: []CommandInfo{ + {Name: "bookmarks", Category: "personal", Description: "Manage your personal bookmarks", Actions: []string{"list", "add", "remove", "check"}}, + {Name: "drafts", Category: "personal", Description: "List your unpublished drafts", Actions: []string{"list"}}, + }, + }, { Name: "Organization", Commands: []CommandInfo{ diff --git a/internal/commands/commands_test.go b/internal/commands/commands_test.go index 14cfc07b..b433514e 100644 --- a/internal/commands/commands_test.go +++ b/internal/commands/commands_test.go @@ -111,6 +111,8 @@ func buildRootWithAllCommands() *cobra.Command { root.AddCommand(commands.NewSkillCmd()) root.AddCommand(commands.NewGaugesCmd()) root.AddCommand(commands.NewAssignmentsCmd()) + root.AddCommand(commands.NewBookmarksCmd()) + root.AddCommand(commands.NewDraftsCmd()) root.AddCommand(commands.NewNotificationsCmd()) root.AddCommand(commands.NewTUICmd()) root.AddCommand(commands.NewProfileCmd()) diff --git a/internal/commands/drafts.go b/internal/commands/drafts.go new file mode 100644 index 00000000..438d7c6a --- /dev/null +++ b/internal/commands/drafts.go @@ -0,0 +1,178 @@ +package commands + +import ( + "strconv" + + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// NewDraftsCmd creates the drafts command for the current user's unpublished +// drafts. +// +// Drafts are personal and cross-project, like bookmarks: only their author can +// see them, so there is no project to scope the listing to. The group has one +// leaf because the API has one endpoint — publishing a draft happens through +// the command for whatever the draft is (messages, docs, uploads). +func NewDraftsCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "drafts", + Short: "List your unpublished drafts", + Long: `List your unpublished drafts across every project. + +Drafts are private to you until published. A draft may be a message, document, +upload, client approval, or client correspondence. + + basecamp drafts list`, + Annotations: map[string]string{ + "agent_notes": "Account-wide and personal — no --in needed.\n" + + "Read-only: publish a draft with the command for its type.", + }, + } + + cmd.AddCommand(newDraftsListCmd()) + + return cmd +} + +func newDraftsListCmd() *cobra.Command { + var ( + limit int + page int + all bool + ) + + cmd := &cobra.Command{ + Use: "list", + Short: "List your unpublished drafts", + Long: `List your unpublished drafts, most recently updated first. + +This is a personal feed spanning every project, so the default is bounded: it +walks pages until it has ` + strconv.Itoa(accountWideDefaultLimit) + ` drafts rather than fetching the whole listing +and discarding most of it. --all is how you ask for every page. The server +caps the full listing at 250.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + return runDraftsList(cmd, limit, page, all) + }, + } + + cmd.Flags().IntVarP(&limit, "limit", "n", 0, "Maximum drafts to return") + cmd.Flags().IntVar(&page, "page", 0, "Return only this page") + cmd.Flags().BoolVar(&all, "all", false, "Fetch every page") + + return cmd +} + +func runDraftsList(cmd *cobra.Command, limit, page int, all bool) error { + app := appctx.FromContext(cmd.Context()) + + if err := validateAccountWidePaginationFlags(cmd, limit, page, all); err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + fetch := func(p int32) ([]basecamp.Draft, basecamp.ListMeta, error) { + result, err := app.Account().Drafts().List(cmd.Context(), p) + if err != nil { + return nil, basecamp.ListMeta{}, convertSDKError(err) + } + return result.Drafts, result.Meta, nil + } + + // Page 0 means "follow the Link header across every page", which is what + // --all asks for and no other path may reach. + var ( + drafts []basecamp.Draft + meta basecamp.ListMeta + capped bool + ) + if all || page > 0 { + sdkPage, err := accountWidePage(page, all) + if err != nil { + return err + } + if drafts, meta, err = fetch(sdkPage); err != nil { + return err + } + } else { + effectiveLimit := limit + if effectiveLimit == 0 { + effectiveLimit = accountWideDefaultLimit + } + var err error + if drafts, capped, meta, err = accountWideCollect(fetch, accountWideFlatCount[basecamp.Draft], effectiveLimit); err != nil { + return err + } + // The walk stops at a page boundary, so trim to the exact cap. + if len(drafts) > effectiveLimit { + drafts = drafts[:effectiveLimit] + } + } + + respOpts := accountWideRespOpts(len(drafts), "draft", "drafts", meta, limit > 0) + if notice := accountWideCapNotice(capped, meta, len(drafts), "drafts"); notice != "" { + respOpts = append(respOpts, output.WithNotice(notice)) + } + respOpts = append(respOpts, output.WithDisplayData(flattenDrafts(drafts))) + respOpts = append(respOpts, + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "messages", + Cmd: "basecamp messages list --in ", + Description: "List a project's messages", + }, + output.Breadcrumb{ + Action: "files", + Cmd: "basecamp files list --in ", + Description: "List a project's docs and files", + }, + ), + ) + + return app.OK(drafts, respOpts...) +} + +// flattenDrafts builds the display rows for the draft listing. +// +// A Draft nests its project and its parent, and the renderer's generic column +// detection skips nested objects — so a generic render drops the project a +// draft belongs to, which on a cross-project personal feed is the column that +// makes a row actionable. +// +// Parent and scheduled_posting_at are nil-able, and both nil states are +// meaningful rather than missing: a draft with no parent is filed directly +// under its project, and one with no scheduled time simply is not scheduled. +// Rendering them as blanks would read as absent data, so both states are +// spelled out. +func flattenDrafts(drafts []basecamp.Draft) []map[string]any { + rows := make([]map[string]any, 0, len(drafts)) + for _, d := range drafts { + row := map[string]any{ + "id": d.ID, + "title": d.Title, + "type": d.Type, + "project": d.Bucket.Name, + "updated": d.UpdatedAt, + } + + row["filed_under"] = "project root" + if d.Parent != nil { + row["filed_under"] = d.Parent.Title + } + + row["scheduled"] = "not scheduled" + if d.ScheduledPostingAt != nil { + row["scheduled"] = *d.ScheduledPostingAt + } + + rows = append(rows, row) + } + return rows +} diff --git a/internal/commands/drafts_test.go b/internal/commands/drafts_test.go new file mode 100644 index 00000000..3d463d36 --- /dev/null +++ b/internal/commands/drafts_test.go @@ -0,0 +1,147 @@ +package commands + +import ( + "fmt" + "net/http" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" +) + +const draftsListPath = "/99999/my/drafts.json" + +// draftsFeedBody builds n drafts. The first is bucket-rooted and unscheduled +// (both nil-able fields absent), the rest carry a parent and a scheduled time, +// so one fixture exercises both display states. +func draftsFeedBody(n int) string { + items := make([]string, 0, n) + for i := 1; i <= n; i++ { + parent := "null" + scheduled := "null" + if i > 1 { + parent = `{"id": 555, "title": "Kickoff", "app_url": "https://3.basecamp.com/x"}` + scheduled = `"2026-08-15T09:00:00.000Z"` + } + items = append(items, fmt.Sprintf(`{ + "id": %d, + "app_url": "https://3.basecamp.com/draft/%d", + "title": "Draft %d", + "type": "message", + "bucket": {"id": 977190, "name": "JD test proj", "app_url": "https://3.basecamp.com/p"}, + "parent": %s, + "excerpt": "", + "created_at": "2026-07-01T10:00:00.000Z", + "updated_at": "2026-07-02T10:00:00.000Z", + "scheduled_posting_at": %s + }`, i, i, i, parent, scheduled)) + } + return "[" + strings.Join(items, ",") + "]" +} + +func draftsListRoute(n int) stubRoute { + return stubRoute{ + method: http.MethodGet, + path: draftsListPath, + status: http.StatusOK, + body: draftsFeedBody(n), + pages: []string{draftsFeedBody(n)}, + } +} + +// The default must walk positive pages rather than reaching page 0, which is +// the SDK's "fetch every page" spelling. +func TestDraftsListDefaultWalksPositivePages(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, draftsListRoute(2)) + + require.NoError(t, executeRecordingCommand(NewDraftsCmd(), app, "list")) + + queries := transport.queriesFor(draftsListPath) + require.NotEmpty(t, queries) + for _, q := range queries { + assert.Contains(t, q, "page=", "no default request may omit page=") + assert.NotContains(t, q, "page=0") + } + assert.Equal(t, "page=1", queries[0]) +} + +func TestDraftsListAllFetchesEveryPage(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, draftsListRoute(2)) + + require.NoError(t, executeRecordingCommand(NewDraftsCmd(), app, "list", "--all")) + + queries := transport.queriesFor(draftsListPath) + require.Len(t, queries, 1) + assert.Empty(t, queries[0], "--all omits page= entirely") +} + +func TestDraftsListPageIsExactlyOneRequest(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, draftsListRoute(2)) + + require.NoError(t, executeRecordingCommand(NewDraftsCmd(), app, "list", "--page", "2")) + + assert.Equal(t, []string{"page=2"}, transport.queriesFor(draftsListPath)) +} + +func TestDraftsListLimitTrimsExactly(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, draftsListRoute(5)) + + require.NoError(t, executeRecordingCommand(NewDraftsCmd(), app, "list", "--limit", "2")) + + envelope := decodePersonalFeedEnvelope(t, out) + assert.Len(t, envelope.Data, 2) + assert.Equal(t, "2 drafts across all projects", envelope.Summary) +} + +func TestDraftsListRejectsUnusablePaging(t *testing.T) { + assertRejected := func(t *testing.T, args ...string) { + t.Helper() + app, transport, _ := setupPersonalFeedApp(t, draftsListRoute(2)) + + err := executeRecordingCommand(NewDraftsCmd(), app, args...) + + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded(), "a rejected listing must not reach the server") + } + + t.Run("explicit --page 0", func(t *testing.T) { assertRejected(t, "list", "--page", "0") }) + t.Run("negative --limit", func(t *testing.T) { assertRejected(t, "list", "--limit=-1") }) + t.Run("--all with --limit", func(t *testing.T) { assertRejected(t, "list", "--all", "--limit", "5") }) + t.Run("--page with --all", func(t *testing.T) { assertRejected(t, "list", "--page", "2", "--all") }) +} + +// A Draft nests its project, and the generic renderer skips nested objects — so +// a generic render drops the one column that makes a cross-project row +// actionable. Both nil-able fields are display states rather than gaps. +func TestFlattenDraftsRendersBothNilStates(t *testing.T) { + scheduled := time.Date(2026, 8, 15, 9, 0, 0, 0, time.UTC) + rows := flattenDrafts([]basecamp.Draft{ + { + ID: 1, + Title: "Bucket-rooted draft", + Type: "message", + Bucket: basecamp.DraftBucket{ID: 977190, Name: "JD test proj"}, + }, + { + ID: 2, + Title: "Filed and scheduled", + Type: "document", + Bucket: basecamp.DraftBucket{ID: 977190, Name: "JD test proj"}, + Parent: &basecamp.DraftParent{ID: 555, Title: "Kickoff"}, + ScheduledPostingAt: &scheduled, + }, + }) + + require.Len(t, rows, 2) + + assert.Equal(t, "JD test proj", rows[0]["project"], "the project is what makes the row attributable") + assert.Equal(t, "project root", rows[0]["filed_under"], "no parent is a state, not a blank") + assert.Equal(t, "not scheduled", rows[0]["scheduled"], "unscheduled is a state, not a blank") + + assert.Equal(t, "Kickoff", rows[1]["filed_under"]) + assert.Equal(t, scheduled, rows[1]["scheduled"]) +} From 1d549e5e8302bf00fd50a22dd7e636965c05e92f Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 31 Jul 2026 20:04:37 -0700 Subject: [PATCH 3/8] Add notes and calendars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both are singletons in their own way, and neither fits the list/show/create shape the other groups use. notes is one private scratchpad per person, addressed by no id at all — hence show/set rather than list/create/update. The note record does not exist until the first write, so a fresh account gets a nil id and empty content back. That is an empty note, not a missing one, and it renders as empty rather than failing: every account starts there. set writes through richtext.MarkdownToHTML like every other content-writing command, because the field is rich text (my_notes.md: content | HTML). Passing the raw string would store escaped markup. Content comes from an argument, --file, or piped stdin; naming two sources is a usage error rather than a silent precedence rule, and empty content is refused outright, since set replaces the whole note and a mistyped path would otherwise erase it. Attachments are out of scope — this writes the body only. calendars has no index endpoint, so there is no 'calendars list' to add and the discovery path is a pasted URL, which extractID already handles. Its color is validated client-side, which is a requirement rather than a courtesy. At v0.12.0 the SDK's parseErrorBody reads only error/error_description, so a 422 carrying {"errors":{"color":[...]}} arrives as a bare "validation error" naming neither the field, the value, nor the alternatives. Checking the eleven colors before the request is what makes the failure actionable, and the tests assert no request is issued. --- .surface | 144 +++++++++++++++++++ e2e/smoke/smoke_calendars.bats | 25 ++++ e2e/smoke/smoke_notes.bats | 20 +++ internal/cli/root.go | 2 + internal/commands/calendars.go | 188 +++++++++++++++++++++++++ internal/commands/calendars_test.go | 119 ++++++++++++++++ internal/commands/commands.go | 2 + internal/commands/commands_test.go | 2 + internal/commands/notes.go | 211 ++++++++++++++++++++++++++++ internal/commands/notes_test.go | 170 ++++++++++++++++++++++ 10 files changed, 883 insertions(+) create mode 100644 e2e/smoke/smoke_calendars.bats create mode 100644 e2e/smoke/smoke_notes.bats create mode 100644 internal/commands/calendars.go create mode 100644 internal/commands/calendars_test.go create mode 100644 internal/commands/notes.go create mode 100644 internal/commands/notes_test.go diff --git a/.surface b/.surface index 60bb7027..87f3f505 100644 --- a/.surface +++ b/.surface @@ -29,6 +29,9 @@ ARG basecamp boosts create 01 ARG basecamp boosts delete 00 ARG basecamp boosts list 00 ARG basecamp boosts show 00 +ARG basecamp calendars show 00 +ARG basecamp calendars update 00 +ARG basecamp calendars update 01 ARG basecamp campfire delete 00 ARG basecamp campfire line 00 ARG basecamp campfire post 00 @@ -245,6 +248,7 @@ ARG basecamp msgs show 00 ARG basecamp msgs trash 00 ARG basecamp msgs unpin 00 ARG basecamp msgs update 00 +ARG basecamp notes set 00 [content] ARG basecamp notifications read 00 ... ARG basecamp people add 00 ... ARG basecamp people remove 00 ... @@ -482,6 +486,9 @@ CMD basecamp boosts create CMD basecamp boosts delete CMD basecamp boosts list CMD basecamp boosts show +CMD basecamp calendars +CMD basecamp calendars show +CMD basecamp calendars update CMD basecamp campfire CMD basecamp campfire delete CMD basecamp campfire line @@ -823,6 +830,9 @@ CMD basecamp msgs show CMD basecamp msgs trash CMD basecamp msgs unpin CMD basecamp msgs update +CMD basecamp notes +CMD basecamp notes set +CMD basecamp notes show CMD basecamp notifications CMD basecamp notifications bubbleups CMD basecamp notifications list @@ -2327,6 +2337,70 @@ FLAG basecamp boosts show --stats type=bool FLAG basecamp boosts show --styled type=bool FLAG basecamp boosts show --todolist type=string FLAG basecamp boosts show --verbose type=count +FLAG basecamp calendars --account type=string +FLAG basecamp calendars --agent type=bool +FLAG basecamp calendars --cache-dir type=string +FLAG basecamp calendars --count type=bool +FLAG basecamp calendars --help type=bool +FLAG basecamp calendars --hints type=bool +FLAG basecamp calendars --ids-only type=bool +FLAG basecamp calendars --in type=string +FLAG basecamp calendars --jq type=string +FLAG basecamp calendars --json type=bool +FLAG basecamp calendars --markdown type=bool +FLAG basecamp calendars --md type=bool +FLAG basecamp calendars --no-hints type=bool +FLAG basecamp calendars --no-stats type=bool +FLAG basecamp calendars --profile type=string +FLAG basecamp calendars --project type=string +FLAG basecamp calendars --quiet type=bool +FLAG basecamp calendars --stats type=bool +FLAG basecamp calendars --styled type=bool +FLAG basecamp calendars --todolist type=string +FLAG basecamp calendars --verbose type=count +FLAG basecamp calendars show --account type=string +FLAG basecamp calendars show --agent type=bool +FLAG basecamp calendars show --cache-dir type=string +FLAG basecamp calendars show --count type=bool +FLAG basecamp calendars show --help type=bool +FLAG basecamp calendars show --hints type=bool +FLAG basecamp calendars show --ids-only type=bool +FLAG basecamp calendars show --in type=string +FLAG basecamp calendars show --jq type=string +FLAG basecamp calendars show --json type=bool +FLAG basecamp calendars show --markdown type=bool +FLAG basecamp calendars show --md type=bool +FLAG basecamp calendars show --no-hints type=bool +FLAG basecamp calendars show --no-stats type=bool +FLAG basecamp calendars show --profile type=string +FLAG basecamp calendars show --project type=string +FLAG basecamp calendars show --quiet type=bool +FLAG basecamp calendars show --stats type=bool +FLAG basecamp calendars show --styled type=bool +FLAG basecamp calendars show --todolist type=string +FLAG basecamp calendars show --verbose type=count +FLAG basecamp calendars update --account type=string +FLAG basecamp calendars update --agent type=bool +FLAG basecamp calendars update --cache-dir type=string +FLAG basecamp calendars update --color type=string +FLAG basecamp calendars update --count type=bool +FLAG basecamp calendars update --help type=bool +FLAG basecamp calendars update --hints type=bool +FLAG basecamp calendars update --ids-only type=bool +FLAG basecamp calendars update --in type=string +FLAG basecamp calendars update --jq type=string +FLAG basecamp calendars update --json type=bool +FLAG basecamp calendars update --markdown type=bool +FLAG basecamp calendars update --md type=bool +FLAG basecamp calendars update --no-hints type=bool +FLAG basecamp calendars update --no-stats type=bool +FLAG basecamp calendars update --profile type=string +FLAG basecamp calendars update --project type=string +FLAG basecamp calendars update --quiet type=bool +FLAG basecamp calendars update --stats type=bool +FLAG basecamp calendars update --styled type=bool +FLAG basecamp calendars update --todolist type=string +FLAG basecamp calendars update --verbose type=count FLAG basecamp campfire --account type=string FLAG basecamp campfire --agent type=bool FLAG basecamp campfire --cache-dir type=string @@ -10605,6 +10679,70 @@ FLAG basecamp msgs update --styled type=bool FLAG basecamp msgs update --title type=string FLAG basecamp msgs update --todolist type=string FLAG basecamp msgs update --verbose type=count +FLAG basecamp notes --account type=string +FLAG basecamp notes --agent type=bool +FLAG basecamp notes --cache-dir type=string +FLAG basecamp notes --count type=bool +FLAG basecamp notes --help type=bool +FLAG basecamp notes --hints type=bool +FLAG basecamp notes --ids-only type=bool +FLAG basecamp notes --in type=string +FLAG basecamp notes --jq type=string +FLAG basecamp notes --json type=bool +FLAG basecamp notes --markdown type=bool +FLAG basecamp notes --md type=bool +FLAG basecamp notes --no-hints type=bool +FLAG basecamp notes --no-stats type=bool +FLAG basecamp notes --profile type=string +FLAG basecamp notes --project type=string +FLAG basecamp notes --quiet type=bool +FLAG basecamp notes --stats type=bool +FLAG basecamp notes --styled type=bool +FLAG basecamp notes --todolist type=string +FLAG basecamp notes --verbose type=count +FLAG basecamp notes set --account type=string +FLAG basecamp notes set --agent type=bool +FLAG basecamp notes set --cache-dir type=string +FLAG basecamp notes set --count type=bool +FLAG basecamp notes set --file type=string +FLAG basecamp notes set --help type=bool +FLAG basecamp notes set --hints type=bool +FLAG basecamp notes set --ids-only type=bool +FLAG basecamp notes set --in type=string +FLAG basecamp notes set --jq type=string +FLAG basecamp notes set --json type=bool +FLAG basecamp notes set --markdown type=bool +FLAG basecamp notes set --md type=bool +FLAG basecamp notes set --no-hints type=bool +FLAG basecamp notes set --no-stats type=bool +FLAG basecamp notes set --profile type=string +FLAG basecamp notes set --project type=string +FLAG basecamp notes set --quiet type=bool +FLAG basecamp notes set --stats type=bool +FLAG basecamp notes set --styled type=bool +FLAG basecamp notes set --todolist type=string +FLAG basecamp notes set --verbose type=count +FLAG basecamp notes show --account type=string +FLAG basecamp notes show --agent type=bool +FLAG basecamp notes show --cache-dir type=string +FLAG basecamp notes show --count type=bool +FLAG basecamp notes show --help type=bool +FLAG basecamp notes show --hints type=bool +FLAG basecamp notes show --ids-only type=bool +FLAG basecamp notes show --in type=string +FLAG basecamp notes show --jq type=string +FLAG basecamp notes show --json type=bool +FLAG basecamp notes show --markdown type=bool +FLAG basecamp notes show --md type=bool +FLAG basecamp notes show --no-hints type=bool +FLAG basecamp notes show --no-stats type=bool +FLAG basecamp notes show --profile type=string +FLAG basecamp notes show --project type=string +FLAG basecamp notes show --quiet type=bool +FLAG basecamp notes show --stats type=bool +FLAG basecamp notes show --styled type=bool +FLAG basecamp notes show --todolist type=string +FLAG basecamp notes show --verbose type=count FLAG basecamp notifications --account type=string FLAG basecamp notifications --agent type=bool FLAG basecamp notifications --cache-dir type=string @@ -16748,6 +16886,9 @@ SUB basecamp boosts create SUB basecamp boosts delete SUB basecamp boosts list SUB basecamp boosts show +SUB basecamp calendars +SUB basecamp calendars show +SUB basecamp calendars update SUB basecamp campfire SUB basecamp campfire delete SUB basecamp campfire line @@ -17089,6 +17230,9 @@ SUB basecamp msgs show SUB basecamp msgs trash SUB basecamp msgs unpin SUB basecamp msgs update +SUB basecamp notes +SUB basecamp notes set +SUB basecamp notes show SUB basecamp notifications SUB basecamp notifications bubbleups SUB basecamp notifications list diff --git a/e2e/smoke/smoke_calendars.bats b/e2e/smoke/smoke_calendars.bats new file mode 100644 index 00000000..273f9334 --- /dev/null +++ b/e2e/smoke/smoke_calendars.bats @@ -0,0 +1,25 @@ +#!/usr/bin/env bats +# smoke_calendars.bats - Level 0: Calendar read and recolor + +load smoke_helper + +setup_file() { + ensure_token || return 1 +} + +@test "calendars show rejects a non-id argument" { + run_smoke basecamp calendars show not-an-id --json + assert_failure +} + +@test "calendars show requires a discoverable calendar" { + # There is no index endpoint, so nothing here can discover a calendar id. + mark_unverifiable "No calendars index endpoint to discover an id from" +} + +@test "calendars update rejects an unknown color" { + # Client-side validation: this must fail without issuing a request, since + # the SDK cannot surface the server's own field message. + run_smoke basecamp calendars update 999999 --color chartreuse --json + assert_failure +} diff --git a/e2e/smoke/smoke_notes.bats b/e2e/smoke/smoke_notes.bats new file mode 100644 index 00000000..f3a9a750 --- /dev/null +++ b/e2e/smoke/smoke_notes.bats @@ -0,0 +1,20 @@ +#!/usr/bin/env bats +# smoke_notes.bats - Level 0: Personal note + +load smoke_helper + +setup_file() { + ensure_token || return 1 +} + +@test "notes show returns the note" { + # An account that has never written a note returns an empty one rather than + # a 404, so this must succeed either way. + run_smoke basecamp notes show --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "notes set is out of scope" { + mark_out_of_scope "Mutating - set replaces the whole note; covered by the live round-trip" +} diff --git a/internal/cli/root.go b/internal/cli/root.go index d54937c5..6f974f02 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -341,6 +341,8 @@ func Execute() { cmd.AddCommand(commands.NewAssignmentsCmd()) cmd.AddCommand(commands.NewBookmarksCmd()) cmd.AddCommand(commands.NewDraftsCmd()) + cmd.AddCommand(commands.NewNotesCmd()) + cmd.AddCommand(commands.NewCalendarsCmd()) cmd.AddCommand(commands.NewNotificationsCmd()) cmd.AddCommand(commands.NewTUICmd()) cmd.AddCommand(commands.NewBonfireCmd()) diff --git a/internal/commands/calendars.go b/internal/commands/calendars.go new file mode 100644 index 00000000..5c9f99d2 --- /dev/null +++ b/internal/commands/calendars.go @@ -0,0 +1,188 @@ +package commands + +import ( + "fmt" + "strconv" + "strings" + + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// calendarColors are the colors a calendar accepts. +// +// Validated client-side because the SDK cannot report what the server says: at +// v0.12.0 its error parser reads only error/error_description, so a 422 whose +// body is {"errors":{"color":[...]}} degrades to a bare "validation error" with +// no mention of the field, the value, or the alternatives. Rejecting here turns +// that into an answer the caller can act on. +var calendarColors = []string{ + "white", "red", "orange", "yellow", "green", "blue", + "aqua", "purple", "gray", "pink", "brown", +} + +// NewCalendarsCmd creates the calendars command. +// +// There is no index endpoint, so this group cannot list calendars — the way a +// caller names one is by pasting its URL, which extractID turns into the id. +func NewCalendarsCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "calendars", + Short: "View and recolor calendars", + Long: `View and recolor calendars. + +Calendars have no listing endpoint, so name one by id or by pasting its +Basecamp URL. + + basecamp calendars show 12345 + basecamp calendars show https://3.basecamp.com/1234567/calendars/12345 + basecamp calendars update 12345 --color blue`, + Annotations: map[string]string{ + "agent_notes": "No index endpoint — there is no 'calendars list'.\n" + + "Address a calendar by id or by pasting its URL.", + }, + } + + cmd.AddCommand( + newCalendarsShowCmd(), + newCalendarsUpdateCmd(), + ) + + return cmd +} + +func newCalendarsShowCmd() *cobra.Command { + return &cobra.Command{ + Use: "show ", + Short: "Show a calendar", + Long: `Show a calendar's name, color, and linked schedule. + + basecamp calendars show 12345 + basecamp calendars show https://3.basecamp.com/1234567/calendars/12345`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + calendarID, err := calendarIDFromArg(args[0]) + if err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + calendar, err := app.Account().Calendars().Get(cmd.Context(), calendarID) + if err != nil { + return convertSDKError(err) + } + + return app.OK(calendar, + output.WithSummary(calendarSummary(calendar)), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "update", + Cmd: fmt.Sprintf("basecamp calendars update %d --color ", calendarID), + Description: "Change the calendar color", + }, + ), + ) + }, + } +} + +func newCalendarsUpdateCmd() *cobra.Command { + var color string + + cmd := &cobra.Command{ + Use: "update --color ", + Short: "Change a calendar's color", + Long: `Change a calendar's display color. + +Colors: ` + strings.Join(calendarColors, ", ") + ` + + basecamp calendars update 12345 --color blue`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + calendarID, err := calendarIDFromArg(args[0]) + if err != nil { + return err + } + if err := validateCalendarColor(color); err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + calendar, err := app.Account().Calendars().Update(cmd.Context(), calendarID, color) + if err != nil { + return convertSDKError(err) + } + + return app.OK(calendar, + output.WithSummary(fmt.Sprintf("Calendar color set to %s", color)), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "show", + Cmd: fmt.Sprintf("basecamp calendars show %d", calendarID), + Description: "View the calendar", + }, + ), + ) + }, + } + + cmd.Flags().StringVar(&color, "color", "", "Calendar color ("+strings.Join(calendarColors, ", ")+")") + + return cmd +} + +// validateCalendarColor rejects an unknown color before the request, naming the +// alternatives. The server would reject it too, but the SDK cannot carry its +// message back, so an unchecked value fails as a bare "validation error". +func validateCalendarColor(color string) error { + if color == "" { + return output.ErrUsageHint( + "--color is required", + "Pick one of: "+strings.Join(calendarColors, ", "), + ) + } + for _, valid := range calendarColors { + if color == valid { + return nil + } + } + return output.ErrUsageHint( + fmt.Sprintf("%q is not a valid calendar color", color), + "Pick one of: "+strings.Join(calendarColors, ", "), + ) +} + +// calendarIDFromArg resolves the positional. With no index endpoint, a +// pasted URL is the realistic way to name a calendar. +func calendarIDFromArg(arg string) (int64, error) { + id, err := strconv.ParseInt(extractID(arg), 10, 64) + if err != nil { + return 0, output.ErrUsageHint( + fmt.Sprintf("%q is not a calendar id or Basecamp URL", arg), + "Pass a numeric calendar id, or paste the calendar's Basecamp URL", + ) + } + return id, nil +} + +func calendarSummary(calendar *basecamp.Calendar) string { + if calendar == nil { + return "Calendar" + } + if calendar.Name == "" { + return fmt.Sprintf("Calendar %d (%s)", calendar.ID, calendar.Color) + } + return fmt.Sprintf("%s (%s)", calendar.Name, calendar.Color) +} diff --git a/internal/commands/calendars_test.go b/internal/commands/calendars_test.go new file mode 100644 index 00000000..9f32a91b --- /dev/null +++ b/internal/commands/calendars_test.go @@ -0,0 +1,119 @@ +package commands + +import ( + "encoding/json" + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func calendarPath(id int64) string { + return fmt.Sprintf("/99999/calendars/%d", id) +} + +func calendarRoute(method string, id int64, color string) stubRoute { + return stubRoute{ + method: method, + path: calendarPath(id), + status: http.StatusOK, + body: fmt.Sprintf(`{ + "id": %d, "type": "Calendar", "name": "Team calendar", "color": %q, + "created_at": "2026-07-01T10:00:00.000Z", + "updated_at": "2026-07-01T10:00:00.000Z", + "url": "", "app_url": "", "schedule_url": "" + }`, id, color), + } +} + +func TestCalendarsShowFetchesTheCalendar(t *testing.T) { + app, transport, out := setupPersonalFeedApp(t, calendarRoute(http.MethodGet, 12345, "blue")) + + require.NoError(t, executeRecordingCommand(NewCalendarsCmd(), app, "show", "12345")) + + assert.Equal(t, calendarPath(12345), transport.last(t).Path) + + var envelope struct { + Summary string `json:"summary"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + assert.Equal(t, "Team calendar (blue)", envelope.Summary) +} + +// There is no index endpoint, so pasting a URL is the realistic way to name a +// calendar — the discovery path the group depends on. +func TestCalendarsShowAcceptsAURL(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, calendarRoute(http.MethodGet, 12345, "blue")) + + require.NoError(t, executeRecordingCommand(NewCalendarsCmd(), app, + "show", "https://3.basecamp.com/1234567/calendars/12345")) + + assert.Equal(t, calendarPath(12345), transport.last(t).Path) +} + +func TestCalendarsUpdateSendsTheColor(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, calendarRoute(http.MethodPut, 12345, "aqua")) + + require.NoError(t, executeRecordingCommand(NewCalendarsCmd(), app, "update", "12345", "--color", "aqua")) + + call := transport.last(t) + assert.Equal(t, http.MethodPut, call.Method) + assert.Equal(t, calendarPath(12345), call.Path) + assert.Contains(t, call.Body, `"color":"aqua"`) +} + +// The SDK at v0.12.0 cannot carry the server's field message back: its error +// parser reads only error/error_description, so a 422 whose body names the +// color degrades to a bare "validation error". Validating here is what makes +// the failure actionable, so it must happen before any request is issued. +func TestCalendarsUpdateRejectsAnUnknownColorWithoutARequest(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, calendarRoute(http.MethodPut, 12345, "blue")) + + err := executeRecordingCommand(NewCalendarsCmd(), app, "update", "12345", "--color", "chartreuse") + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Message, "chartreuse") + assert.Contains(t, outErr.Hint, "blue", "the hint must name the alternatives") + assert.Empty(t, transport.recorded(), "an invalid color must not reach the server") +} + +func TestCalendarsUpdateRequiresAColor(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, calendarRoute(http.MethodPut, 12345, "blue")) + + err := executeRecordingCommand(NewCalendarsCmd(), app, "update", "12345") + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Message, "--color") + assert.Empty(t, transport.recorded()) +} + +func TestCalendarsAcceptsEveryDocumentedColor(t *testing.T) { + for _, color := range calendarColors { + t.Run(color, func(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, calendarRoute(http.MethodPut, 12345, color)) + + require.NoError(t, executeRecordingCommand(NewCalendarsCmd(), app, "update", "12345", "--color", color)) + + assert.NotEmpty(t, transport.recorded()) + }) + } +} + +func TestCalendarsRejectANonID(t *testing.T) { + for _, args := range [][]string{ + {"show", "not-an-id"}, + {"update", "not-an-id", "--color", "blue"}, + } { + t.Run(args[0], func(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t) + + err := executeRecordingCommand(NewCalendarsCmd(), app, args...) + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Hint, "calendar id") + assert.Empty(t, transport.recorded()) + }) + } +} diff --git a/internal/commands/commands.go b/internal/commands/commands.go index bada86bc..3197eef8 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -73,6 +73,7 @@ func CommandCategories() []CommandCategory { {Name: "timeline", Category: "scheduling", Description: "View activity timelines", Actions: []string{}}, {Name: "reports", Category: "scheduling", Description: "View reports", Actions: []string{"assignable", "assigned", "overdue", "schedule"}}, {Name: "assignments", Category: "scheduling", Description: "View my assignments", Actions: []string{"list", "completed", "due"}}, + {Name: "calendars", Category: "scheduling", Description: "View and recolor calendars", Actions: []string{"show", "update"}}, }, }, { @@ -80,6 +81,7 @@ func CommandCategories() []CommandCategory { Commands: []CommandInfo{ {Name: "bookmarks", Category: "personal", Description: "Manage your personal bookmarks", Actions: []string{"list", "add", "remove", "check"}}, {Name: "drafts", Category: "personal", Description: "List your unpublished drafts", Actions: []string{"list"}}, + {Name: "notes", Category: "personal", Description: "Read and write your personal note", Actions: []string{"show", "set"}}, }, }, { diff --git a/internal/commands/commands_test.go b/internal/commands/commands_test.go index b433514e..f9a8dc97 100644 --- a/internal/commands/commands_test.go +++ b/internal/commands/commands_test.go @@ -113,6 +113,8 @@ func buildRootWithAllCommands() *cobra.Command { root.AddCommand(commands.NewAssignmentsCmd()) root.AddCommand(commands.NewBookmarksCmd()) root.AddCommand(commands.NewDraftsCmd()) + root.AddCommand(commands.NewNotesCmd()) + root.AddCommand(commands.NewCalendarsCmd()) root.AddCommand(commands.NewNotificationsCmd()) root.AddCommand(commands.NewTUICmd()) root.AddCommand(commands.NewProfileCmd()) diff --git a/internal/commands/notes.go b/internal/commands/notes.go new file mode 100644 index 00000000..99e9704d --- /dev/null +++ b/internal/commands/notes.go @@ -0,0 +1,211 @@ +package commands + +import ( + "fmt" + "os" + "strings" + + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/richtext" +) + +// NewNotesCmd creates the notes command for the current user's personal note. +// +// This is a singleton, not a collection: one note per person, addressed by no +// id at all. Hence show/set rather than list/create/update — there is nothing +// to enumerate and nothing to choose between. +func NewNotesCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "notes", + Short: "Read and write your personal note", + Long: `Read and write your personal note — a single private scratchpad. + +The note is yours alone and lives outside any project. There is one per +person, so there is nothing to list and no id to pass. + + basecamp notes show + basecamp notes set "Remember to follow up on the Q3 rollout" + basecamp notes set --file notes.md + cat notes.md | basecamp notes set`, + Annotations: map[string]string{ + "agent_notes": "Account-wide and personal — no --in needed.\n" + + "Singleton: no id. 'set' replaces the whole note; it does not append.", + }, + } + + cmd.AddCommand( + newNotesShowCmd(), + newNotesSetCmd(), + ) + + return cmd +} + +func newNotesShowCmd() *cobra.Command { + return &cobra.Command{ + Use: "show", + Short: "Show your personal note", + Long: `Show your personal note. + +Before you have ever written to it the note does not exist server-side yet. +That is an empty note, not a missing one, so it renders as empty rather than +failing.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + note, err := app.Account().MyNotes().Get(cmd.Context()) + if err != nil { + return convertSDKError(err) + } + + return app.OK(note, + output.WithSummary(notesSummary(note)), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "set", + Cmd: `basecamp notes set ""`, + Description: "Replace your note", + }, + ), + ) + }, + } +} + +// notesSummary describes a note, including the pre-first-write state. +// +// A note that has never been written has a nil id and empty content. That is a +// real, reachable state rather than an error: every account starts there, so it +// is reported plainly instead of being treated as a missing record. +func notesSummary(note *basecamp.MyNote) string { + if note == nil || note.ID == nil { + return "Your note is empty (nothing written yet)" + } + if strings.TrimSpace(note.Content) == "" { + return "Your note is empty" + } + if note.UpdatedAt != nil { + return fmt.Sprintf("Your note, last updated %s", note.UpdatedAt.Format("2006-01-02 15:04")) + } + return "Your note" +} + +func newNotesSetCmd() *cobra.Command { + var file string + + cmd := &cobra.Command{ + Use: "set [content]", + Short: "Replace your personal note", + Long: `Replace your personal note with new content. + +Content comes from a positional argument, --file, or piped stdin. Markdown is +converted to HTML, since the note is a rich text field — passing raw text +through would store escaped markup rather than formatting. + +This replaces the whole note; it does not append. The first write creates the +note, so there is no separate "create" step. + + basecamp notes set "Follow up with Ann on the rollout" + basecamp notes set --file notes.md + cat notes.md | basecamp notes set + +Attachments are out of scope: this writes the note body only.`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + content, err := notesContent(cmd, args, file) + if err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + // The note is rich text (my_notes.md: content | HTML), so Markdown + // is converted like every other content-writing command. Sending the + // raw string would store escaped or malformed markup. + note, err := app.Account().MyNotes().Update(cmd.Context(), richtext.MarkdownToHTML(content)) + if err != nil { + return convertSDKError(err) + } + + return app.OK(note, + output.WithSummary("Note updated"), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "show", + Cmd: "basecamp notes show", + Description: "Read your note back", + }, + ), + ) + }, + } + + cmd.Flags().StringVarP(&file, "file", "f", "", "Read note content from a file") + + return cmd +} + +// notesContent resolves the note body from exactly one of the three inputs. +// +// Naming two sources is a usage error rather than a silent precedence rule: a +// caller who passes both an argument and --file has a wrong expectation about +// which one wins, and this command overwrites the whole note. +func notesContent(cmd *cobra.Command, args []string, file string) (string, error) { + positional := strings.Join(args, " ") + + if file != "" && positional != "" { + return "", output.ErrUsage("pass note content as an argument or --file, not both") + } + + if file != "" { + data, err := os.ReadFile(file) + if err != nil { + return "", output.ErrUsage(fmt.Sprintf("failed to read %s: %v", file, err)) + } + return notesRequireContent(string(data)) + } + + if positional != "" { + return notesRequireContent(positional) + } + + piped, ok, err := readPipedStdin(cmd) + if err != nil { + return "", err + } + if !ok { + return "", output.ErrUsageHint( + "note content is required", + `Pass it as an argument, with --file, or on stdin: basecamp notes set "..."`, + ) + } + return notesRequireContent(piped) +} + +// notesRequireContent refuses to blank the note by accident. +// +// set replaces everything, so an empty file or an empty pipe would silently +// erase the note. Clearing it is a reasonable thing to want, but it should be +// asked for on purpose rather than arrived at by a mistyped path. +func notesRequireContent(content string) (string, error) { + if strings.TrimSpace(content) == "" { + return "", output.ErrUsageHint( + "note content is empty", + "set replaces the whole note; pass content, or use --file with a non-empty file", + ) + } + return content, nil +} diff --git a/internal/commands/notes_test.go b/internal/commands/notes_test.go new file mode 100644 index 00000000..e4afb711 --- /dev/null +++ b/internal/commands/notes_test.go @@ -0,0 +1,170 @@ +package commands + +import ( + "encoding/json" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/richtext" +) + +const notesPath = "/99999/my/notes.json" + +func notesGetRoute(body string) stubRoute { + return stubRoute{ + method: http.MethodGet, + path: notesPath, + status: http.StatusOK, + body: body, + } +} + +func notesUpdateRoute() stubRoute { + return stubRoute{ + method: http.MethodPut, + path: notesPath, + status: http.StatusOK, + body: `{ + "id": 42, "type": "Notebook::Note", + "created_at": "2026-07-01T10:00:00.000Z", + "updated_at": "2026-07-31T10:00:00.000Z", + "content": "
hello
", + "content_attachments": [], + "url": "https://3.basecampapi.com/99999/my/notes.json", + "app_url": "https://3.basecamp.com/99999/my/notes" + }`, + } +} + +// Every account starts here: the note record does not exist until the first +// write, so id and the timestamps are null and content is "". That is an empty +// note, not a missing one, and must render rather than fail. +func TestNotesShowRendersThePreFirstWriteState(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, notesGetRoute(`{ + "id": null, "type": "Notebook::Note", + "created_at": null, "updated_at": null, + "content": "", "content_attachments": [], + "url": "", "app_url": "" + }`)) + + err := executeRecordingCommand(NewNotesCmd(), app, "show") + + require.NoError(t, err, "an unwritten note is an empty note, not an error") + var envelope struct { + OK bool `json:"ok"` + Summary string `json:"summary"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + assert.True(t, envelope.OK) + assert.Contains(t, envelope.Summary, "empty") + assert.Contains(t, envelope.Summary, "nothing written yet") +} + +func TestNotesShowRendersAWrittenNote(t *testing.T) { + app, _, out := setupPersonalFeedApp(t, notesGetRoute(`{ + "id": 42, "type": "Notebook::Note", + "created_at": "2026-07-01T10:00:00.000Z", + "updated_at": "2026-07-31T14:30:00.000Z", + "content": "
hello
", "content_attachments": [], + "url": "", "app_url": "" + }`)) + + require.NoError(t, executeRecordingCommand(NewNotesCmd(), app, "show")) + + var envelope struct { + Summary string `json:"summary"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + assert.Contains(t, envelope.Summary, "last updated 2026-07-31") +} + +// The note is a rich text field, so the body must go over the wire as HTML. +// Sending the raw Markdown would store escaped or malformed markup. +func TestNotesSetSendsMarkdownAsHTML(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, notesUpdateRoute()) + + require.NoError(t, executeRecordingCommand(NewNotesCmd(), app, "set", "**bold** and a [link](https://example.com)")) + + call := transport.last(t) + assert.Equal(t, http.MethodPut, call.Method) + assert.Equal(t, notesPath, call.Path) + + var body struct { + Note struct { + Content string `json:"content"` + } `json:"note"` + } + require.NoError(t, json.Unmarshal([]byte(call.Body), &body)) + assert.Equal(t, richtext.MarkdownToHTML("**bold** and a [link](https://example.com)"), body.Note.Content) + assert.Contains(t, body.Note.Content, "bold") + assert.NotContains(t, body.Note.Content, "**bold**", "raw Markdown must not reach the wire") +} + +func TestNotesSetReadsFromAFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "note.md") + require.NoError(t, os.WriteFile(path, []byte("# Heading\n\nBody text."), 0o600)) + + app, transport, _ := setupPersonalFeedApp(t, notesUpdateRoute()) + + require.NoError(t, executeRecordingCommand(NewNotesCmd(), app, "set", "--file", path)) + + var body struct { + Note struct { + Content string `json:"content"` + } `json:"note"` + } + require.NoError(t, json.Unmarshal([]byte(transport.last(t).Body), &body)) + assert.Contains(t, body.Note.Content, "Heading") + assert.NotContains(t, body.Note.Content, "# Heading", "raw Markdown must not reach the wire") +} + +func TestNotesSetReadsPipedStdin(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, notesUpdateRoute()) + + cmd := NewNotesCmd() + cmd.SetIn(strings.NewReader("piped note body")) + + require.NoError(t, executeRecordingCommand(cmd, app, "set")) + + var body struct { + Note struct { + Content string `json:"content"` + } `json:"note"` + } + require.NoError(t, json.Unmarshal([]byte(transport.last(t).Body), &body)) + assert.Contains(t, body.Note.Content, "piped note body") +} + +// set replaces the whole note, so the failure modes that would silently erase +// it are rejected before the request rather than written through. +func TestNotesSetRejectsAmbiguousOrEmptyInput(t *testing.T) { + emptyFile := filepath.Join(t.TempDir(), "empty.md") + require.NoError(t, os.WriteFile(emptyFile, []byte(" \n"), 0o600)) + + populated := filepath.Join(t.TempDir(), "note.md") + require.NoError(t, os.WriteFile(populated, []byte("content"), 0o600)) + + for _, tc := range []struct { + name string + args []string + }{ + {"argument and --file together", []string{"set", "inline", "--file", populated}}, + {"an empty file", []string{"set", "--file", emptyFile}}, + {"a missing file", []string{"set", "--file", filepath.Join(t.TempDir(), "nope.md")}}, + } { + t.Run(tc.name, func(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, notesUpdateRoute()) + + err := executeRecordingCommand(NewNotesCmd(), app, tc.args...) + + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded(), "a rejected write must not reach the server") + }) + } +} From c2de7f67a49da40dbc911673344a546d15c5fac3 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 31 Jul 2026 20:06:59 -0700 Subject: [PATCH 4/8] Add the Up Next verbs, and surface the id they need MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit prioritize, deprioritize and reorder manage the Up Next list. The listing gains priority_recording_id in its display rows, and that is not a separate nicety — it is what makes two of the three verbs usable. Which id to send depends on what is being addressed, and there are three cases rather than two: a to-do, or a card itself the entry's own id a step not yet prioritized the step's id, from the card's children a step already prioritized the entry's priority_recording_id The third is the one that bites. Once a step is prioritized the listing normalizes it under its parent card, so the entry's top-level id is the card's and only priority_recording_id names the step. That value appears in no URL and in no other command's output, so without it deprioritize and reorder have no way to name a card-step target — and the failure is silent, since the server answers 204 whether or not anything matched. All three verbs carry the rule in their help text, including the sharp edge that two prioritized steps on one card collapse to a single addressable entry. reorder is 1-based and refuses a position it cannot honor rather than clamping: serving a different slot than the one asked for would move the item somewhere the caller did not choose. The SDK deliberately never retries it, so a transient failure surfaces as a plain error rather than being replayed into a different position. --- .surface | 74 +++++++ e2e/smoke/smoke_assignments.bats | 23 ++ internal/commands/assignments.go | 206 ++++++++++++++++++ .../commands/assignments_priority_test.go | 134 ++++++++++++ internal/commands/commands.go | 2 +- 5 files changed, 438 insertions(+), 1 deletion(-) create mode 100644 internal/commands/assignments_priority_test.go diff --git a/.surface b/.surface index 87f3f505..2d2fb7fb 100644 --- a/.surface +++ b/.surface @@ -7,7 +7,11 @@ ARG basecamp api get 00 ARG basecamp api post 00 ARG basecamp api put 00 ARG basecamp assign 00 ... +ARG basecamp assignments deprioritize 00 ARG basecamp assignments due 00 [scope] +ARG basecamp assignments prioritize 00 +ARG basecamp assignments reorder 00 +ARG basecamp assignments reorder 01 ARG basecamp attach 00 ARG basecamp attach 01 [ @@ -453,8 +457,11 @@ CMD basecamp api put CMD basecamp assign CMD basecamp assignments CMD basecamp assignments completed +CMD basecamp assignments deprioritize CMD basecamp assignments due CMD basecamp assignments list +CMD basecamp assignments prioritize +CMD basecamp assignments reorder CMD basecamp attach CMD basecamp attachments CMD basecamp attachments download @@ -1626,6 +1633,27 @@ FLAG basecamp assignments completed --stats type=bool FLAG basecamp assignments completed --styled type=bool FLAG basecamp assignments completed --todolist type=string FLAG basecamp assignments completed --verbose type=count +FLAG basecamp assignments deprioritize --account type=string +FLAG basecamp assignments deprioritize --agent type=bool +FLAG basecamp assignments deprioritize --cache-dir type=string +FLAG basecamp assignments deprioritize --count type=bool +FLAG basecamp assignments deprioritize --help type=bool +FLAG basecamp assignments deprioritize --hints type=bool +FLAG basecamp assignments deprioritize --ids-only type=bool +FLAG basecamp assignments deprioritize --in type=string +FLAG basecamp assignments deprioritize --jq type=string +FLAG basecamp assignments deprioritize --json type=bool +FLAG basecamp assignments deprioritize --markdown type=bool +FLAG basecamp assignments deprioritize --md type=bool +FLAG basecamp assignments deprioritize --no-hints type=bool +FLAG basecamp assignments deprioritize --no-stats type=bool +FLAG basecamp assignments deprioritize --profile type=string +FLAG basecamp assignments deprioritize --project type=string +FLAG basecamp assignments deprioritize --quiet type=bool +FLAG basecamp assignments deprioritize --stats type=bool +FLAG basecamp assignments deprioritize --styled type=bool +FLAG basecamp assignments deprioritize --todolist type=string +FLAG basecamp assignments deprioritize --verbose type=count FLAG basecamp assignments due --account type=string FLAG basecamp assignments due --agent type=bool FLAG basecamp assignments due --cache-dir type=string @@ -1668,6 +1696,49 @@ FLAG basecamp assignments list --stats type=bool FLAG basecamp assignments list --styled type=bool FLAG basecamp assignments list --todolist type=string FLAG basecamp assignments list --verbose type=count +FLAG basecamp assignments prioritize --account type=string +FLAG basecamp assignments prioritize --agent type=bool +FLAG basecamp assignments prioritize --cache-dir type=string +FLAG basecamp assignments prioritize --count type=bool +FLAG basecamp assignments prioritize --help type=bool +FLAG basecamp assignments prioritize --hints type=bool +FLAG basecamp assignments prioritize --ids-only type=bool +FLAG basecamp assignments prioritize --in type=string +FLAG basecamp assignments prioritize --jq type=string +FLAG basecamp assignments prioritize --json type=bool +FLAG basecamp assignments prioritize --markdown type=bool +FLAG basecamp assignments prioritize --md type=bool +FLAG basecamp assignments prioritize --no-hints type=bool +FLAG basecamp assignments prioritize --no-stats type=bool +FLAG basecamp assignments prioritize --profile type=string +FLAG basecamp assignments prioritize --project type=string +FLAG basecamp assignments prioritize --quiet type=bool +FLAG basecamp assignments prioritize --stats type=bool +FLAG basecamp assignments prioritize --styled type=bool +FLAG basecamp assignments prioritize --todolist type=string +FLAG basecamp assignments prioritize --verbose type=count +FLAG basecamp assignments reorder --account type=string +FLAG basecamp assignments reorder --agent type=bool +FLAG basecamp assignments reorder --cache-dir type=string +FLAG basecamp assignments reorder --count type=bool +FLAG basecamp assignments reorder --help type=bool +FLAG basecamp assignments reorder --hints type=bool +FLAG basecamp assignments reorder --ids-only type=bool +FLAG basecamp assignments reorder --in type=string +FLAG basecamp assignments reorder --jq type=string +FLAG basecamp assignments reorder --json type=bool +FLAG basecamp assignments reorder --markdown type=bool +FLAG basecamp assignments reorder --md type=bool +FLAG basecamp assignments reorder --no-hints type=bool +FLAG basecamp assignments reorder --no-stats type=bool +FLAG basecamp assignments reorder --position type=int +FLAG basecamp assignments reorder --profile type=string +FLAG basecamp assignments reorder --project type=string +FLAG basecamp assignments reorder --quiet type=bool +FLAG basecamp assignments reorder --stats type=bool +FLAG basecamp assignments reorder --styled type=bool +FLAG basecamp assignments reorder --todolist type=string +FLAG basecamp assignments reorder --verbose type=count FLAG basecamp attach --account type=string FLAG basecamp attach --agent type=bool FLAG basecamp attach --cache-dir type=string @@ -16853,8 +16924,11 @@ SUB basecamp api put SUB basecamp assign SUB basecamp assignments SUB basecamp assignments completed +SUB basecamp assignments deprioritize SUB basecamp assignments due SUB basecamp assignments list +SUB basecamp assignments prioritize +SUB basecamp assignments reorder SUB basecamp attach SUB basecamp attachments SUB basecamp attachments download diff --git a/e2e/smoke/smoke_assignments.bats b/e2e/smoke/smoke_assignments.bats index 96c5c460..e7a391df 100644 --- a/e2e/smoke/smoke_assignments.bats +++ b/e2e/smoke/smoke_assignments.bats @@ -24,3 +24,26 @@ setup_file() { assert_success assert_json_value '.ok' 'true' } + +@test "assignments list surfaces priority_recording_id when present" { + # This listing is the only place priority_recording_id appears — it is in no + # URL and no other command's output — so reorder and deprioritize depend on + # it to address a prioritized card-table step. + run_smoke basecamp assignments list --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "assignments reorder rejects a zero position" { + # Positions are 1-based and refused rather than clamped. + run_smoke basecamp assignments reorder 999999 --position 0 --json + assert_failure +} + +@test "assignments prioritize is out of scope" { + mark_out_of_scope "Mutating - covered by the live card-step priority sequence" +} + +@test "assignments deprioritize is out of scope" { + mark_out_of_scope "Mutating - covered by the live card-step priority sequence" +} diff --git a/internal/commands/assignments.go b/internal/commands/assignments.go index c4ecb31b..80135846 100644 --- a/internal/commands/assignments.go +++ b/internal/commands/assignments.go @@ -2,9 +2,13 @@ package commands import ( "fmt" + "math" + "strconv" "github.com/spf13/cobra" + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + "github.com/basecamp/basecamp-cli/internal/appctx" "github.com/basecamp/basecamp-cli/internal/output" ) @@ -33,11 +37,172 @@ by completion status or due date.`, newAssignmentsListCmd(), newAssignmentsCompletedCmd(), newAssignmentsDueCmd(), + newAssignmentsPrioritizeCmd(), + newAssignmentsDeprioritizeCmd(), + newAssignmentsReorderCmd(), ) return cmd } +// upNextIDGuidance explains which id the Up Next verbs take. There are three +// cases, not two, and the difference is invisible in the payload unless you +// know to look: the assignments listing normalizes a prioritized card-table +// step under its parent card, so the entry's top-level id is the *card's*. +const upNextIDGuidance = `Which id to pass: + + A to-do, or a card itself the entry's own id + A step not yet prioritized the step's id, from the parent card's children + A step already prioritized the entry's priority_recording_id + +That last case is the one that bites: once a step is prioritized the listing +shows it under its parent card, so the entry's id belongs to the card and only +priority_recording_id addresses the step. 'basecamp assignments list' is the +only place that value appears — it is in no URL you can paste. + +If two steps on one card are prioritized, the listing shows the card once with +a single priority_recording_id, and the siblings are not separately +addressable.` + +func newAssignmentsPrioritizeCmd() *cobra.Command { + return &cobra.Command{ + Use: "prioritize ", + Short: "Add an assignment to Up Next", + Long: `Add an assignment to your Up Next list. + +Idempotent: prioritizing something already in Up Next succeeds and changes +nothing. + +` + upNextIDGuidance, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runAssignmentPriorityVerb(cmd, args[0], "prioritize") + }, + } +} + +func newAssignmentsDeprioritizeCmd() *cobra.Command { + return &cobra.Command{ + Use: "deprioritize ", + Short: "Remove an assignment from Up Next", + Long: `Remove an assignment from your Up Next list. + +This targets one exact recording, and the server answers 204 whether or not +anything matched — so an id that is not in Up Next reports success while +changing nothing. Read the id off 'basecamp assignments list' rather than +guessing it. + +` + upNextIDGuidance, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runAssignmentPriorityVerb(cmd, args[0], "deprioritize") + }, + } +} + +func newAssignmentsReorderCmd() *cobra.Command { + var position int + + cmd := &cobra.Command{ + Use: "reorder --position ", + Short: "Move an assignment within Up Next", + Long: `Move an assignment to a new position in your Up Next list. + +Positions are 1-based. This is never retried on a transient failure: replaying +a positional move could land the item somewhere else, so a failure here is a +real failure and safe to retry by hand. + +` + upNextIDGuidance, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + if !cmd.Flags().Changed("position") { + return output.ErrUsageHint( + "--position is required", + "Pass the 1-based slot to move it to: basecamp assignments reorder --position 1", + ) + } + // Bounded rather than clamped: serving a different position than the + // one asked for would silently move the item somewhere else. + if position < 1 || position > math.MaxInt32 { + return output.ErrUsage("--position must be 1 or greater (positions are 1-based)") + } + + recordingID, err := assignmentRecordingID(args[0]) + if err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + if err := app.Account().MyAssignments().Reorder(cmd.Context(), recordingID, int32(position)); err != nil { + return convertSDKError(err) + } + + return app.OK(map[string]any{"id": recordingID, "position": position}, + output.WithSummary(fmt.Sprintf("Moved %d to position %d in Up Next", recordingID, position)), + output.WithBreadcrumbs(assignmentsListBreadcrumb()), + ) + }, + } + + cmd.Flags().IntVar(&position, "position", 0, "1-based position in Up Next") + + return cmd +} + +// runAssignmentPriorityVerb performs prioritize/deprioritize, which differ only +// in the call and the wording. +func runAssignmentPriorityVerb(cmd *cobra.Command, arg, verb string) error { + app := appctx.FromContext(cmd.Context()) + + recordingID, err := assignmentRecordingID(arg) + if err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + summary := fmt.Sprintf("Added %d to Up Next", recordingID) + if verb == "deprioritize" { + err = app.Account().MyAssignments().Deprioritize(cmd.Context(), recordingID) + summary = fmt.Sprintf("Removed %d from Up Next", recordingID) + } else { + err = app.Account().MyAssignments().Prioritize(cmd.Context(), recordingID) + } + if err != nil { + return convertSDKError(err) + } + + return app.OK(map[string]any{"id": recordingID}, + output.WithSummary(summary), + output.WithBreadcrumbs(assignmentsListBreadcrumb()), + ) +} + +func assignmentsListBreadcrumb() output.Breadcrumb { + return output.Breadcrumb{ + Action: "list", + Cmd: "basecamp assignments list", + Description: "See Up Next and its priority_recording_id values", + } +} + +// assignmentRecordingID resolves the positional the Up Next verbs take. +func assignmentRecordingID(arg string) (int64, error) { + id, err := strconv.ParseInt(extractID(arg), 10, 64) + if err != nil { + return 0, output.ErrUsageHint( + fmt.Sprintf("%q is not a recording id or Basecamp URL", arg), + "Pass a numeric recording id, or paste the recording's Basecamp URL", + ) + } + return id, nil +} + func newAssignmentsListCmd() *cobra.Command { return &cobra.Command{ Use: "list", @@ -68,6 +233,7 @@ func runAssignmentsList(cmd *cobra.Command) error { } return app.OK(result, + output.WithDisplayData(flattenAssignments(result)), output.WithSummary(summary), output.WithBreadcrumbs( output.Breadcrumb{ @@ -84,6 +250,46 @@ func runAssignmentsList(cmd *cobra.Command) error { ) } +// flattenAssignments builds the display rows for the assignments listing. +// +// The rows exist mainly to carry priority_recording_id, which is what +// 'assignments reorder' and 'assignments deprioritize' need to address a +// prioritized card-table step. That value appears in no URL and in no other +// command's output, so a listing that omits it leaves those two verbs with no +// way to name their target — they would report a successful 204 while changing +// nothing. The project comes along for the same reason it does on every other +// account-wide listing: without it a cross-project row is unattributable. +func flattenAssignments(result *basecamp.MyAssignmentsResult) []map[string]any { + if result == nil { + return nil + } + + rows := make([]map[string]any, 0, len(result.Priorities)+len(result.NonPriorities)) + rows = appendAssignmentRows(rows, result.Priorities, true) + rows = appendAssignmentRows(rows, result.NonPriorities, false) + return rows +} + +func appendAssignmentRows(rows []map[string]any, items []basecamp.MyAssignment, priority bool) []map[string]any { + for _, item := range items { + row := map[string]any{ + "id": item.ID, + "content": item.Content, + "type": item.Type, + "project": item.Bucket.Name, + "due_on": item.DueOn, + "up_next": priority, + } + // Present only once the step or card has been prioritized, and the one + // id that addresses it thereafter. + if item.PriorityRecordingID != nil { + row["priority_recording_id"] = *item.PriorityRecordingID + } + rows = append(rows, row) + } + return rows +} + func newAssignmentsCompletedCmd() *cobra.Command { return &cobra.Command{ Use: "completed", diff --git a/internal/commands/assignments_priority_test.go b/internal/commands/assignments_priority_test.go new file mode 100644 index 00000000..18acbd49 --- /dev/null +++ b/internal/commands/assignments_priority_test.go @@ -0,0 +1,134 @@ +package commands + +import ( + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" +) + +const ( + assignmentsPrioritiesPath = "/99999/my/priorities.json" + assignmentsPriorityMovePath = "/99999/my/priority_moves.json" +) + +func assignmentsPriorityPath(id int64) string { + return fmt.Sprintf("/99999/my/priorities/%d", id) +} + +func noContentRoute(method, path string) stubRoute { + return stubRoute{method: method, path: path, status: http.StatusNoContent, body: ""} +} + +func TestAssignmentsPrioritizePosts(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, noContentRoute(http.MethodPost, assignmentsPrioritiesPath)) + + require.NoError(t, executeRecordingCommand(NewAssignmentsCmd(), app, "prioritize", "42")) + + call := transport.last(t) + assert.Equal(t, http.MethodPost, call.Method) + assert.Equal(t, assignmentsPrioritiesPath, call.Path) + assert.Contains(t, call.Body, "42") +} + +func TestAssignmentsDeprioritizeDeletes(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, noContentRoute(http.MethodDelete, assignmentsPriorityPath(42))) + + require.NoError(t, executeRecordingCommand(NewAssignmentsCmd(), app, "deprioritize", "42")) + + call := transport.last(t) + assert.Equal(t, http.MethodDelete, call.Method) + assert.Equal(t, assignmentsPriorityPath(42), call.Path) +} + +func TestAssignmentsReorderSendsThePosition(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, noContentRoute(http.MethodPost, assignmentsPriorityMovePath)) + + require.NoError(t, executeRecordingCommand(NewAssignmentsCmd(), app, "reorder", "42", "--position", "3")) + + call := transport.last(t) + assert.Equal(t, http.MethodPost, call.Method) + assert.Equal(t, assignmentsPriorityMovePath, call.Path) + assert.Contains(t, call.Body, "3") +} + +// Positions are 1-based, and a bad one is refused rather than clamped: serving +// a different position than the one asked for would move the item somewhere the +// caller did not choose. +func TestAssignmentsReorderRejectsBadPositions(t *testing.T) { + for _, tc := range []struct { + name string + args []string + }{ + {"missing --position", []string{"reorder", "42"}}, + {"zero", []string{"reorder", "42", "--position", "0"}}, + {"negative", []string{"reorder", "42", "--position=-1"}}, + } { + t.Run(tc.name, func(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, noContentRoute(http.MethodPost, assignmentsPriorityMovePath)) + + err := executeRecordingCommand(NewAssignmentsCmd(), app, tc.args...) + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Message, "--position") + assert.Empty(t, transport.recorded(), "a rejected move must not reach the server") + }) + } +} + +func TestAssignmentsPriorityVerbsRejectANonID(t *testing.T) { + for _, args := range [][]string{ + {"prioritize", "not-an-id"}, + {"deprioritize", "not-an-id"}, + {"reorder", "not-an-id", "--position", "1"}, + } { + t.Run(args[0], func(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t) + + err := executeRecordingCommand(NewAssignmentsCmd(), app, args...) + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Hint, "recording id") + assert.Empty(t, transport.recorded()) + }) + } +} + +// priority_recording_id is what reorder and deprioritize need to address a +// prioritized card-table step, and this listing is the only place it exists — +// it appears in no URL and no other command's output. A row that drops it +// leaves those verbs with no way to name their target, and the failure is +// silent: the server answers 204 either way. +func TestFlattenAssignmentsSurfacesPriorityRecordingID(t *testing.T) { + priorityID := int64(9001) + rows := flattenAssignments(&basecamp.MyAssignmentsResult{ + Priorities: []basecamp.MyAssignment{{ + ID: 777, + Content: "Card with a prioritized step", + Type: "Kanban::Card", + Bucket: basecamp.MyAssignmentBucket{ID: 977190, Name: "JD test proj"}, + PriorityRecordingID: &priorityID, + }}, + NonPriorities: []basecamp.MyAssignment{{ + ID: 888, + Content: "Not in Up Next", + Type: "Todo", + Bucket: basecamp.MyAssignmentBucket{ID: 977190, Name: "JD test proj"}, + }}, + }) + + require.Len(t, rows, 2) + + assert.Equal(t, int64(777), rows[0]["id"], "the entry id is the card's") + assert.Equal(t, priorityID, rows[0]["priority_recording_id"], "the step is addressed by this instead") + assert.Equal(t, true, rows[0]["up_next"]) + assert.Equal(t, "JD test proj", rows[0]["project"]) + + assert.NotContains(t, rows[1], "priority_recording_id", + "an unprioritized entry has no priority_recording_id yet") + assert.Equal(t, false, rows[1]["up_next"]) +} diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 3197eef8..7b8063c3 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -72,7 +72,7 @@ func CommandCategories() []CommandCategory { {Name: "timesheet", Category: "scheduling", Description: "Manage time tracking", Actions: []string{"report", "project", "item"}}, {Name: "timeline", Category: "scheduling", Description: "View activity timelines", Actions: []string{}}, {Name: "reports", Category: "scheduling", Description: "View reports", Actions: []string{"assignable", "assigned", "overdue", "schedule"}}, - {Name: "assignments", Category: "scheduling", Description: "View my assignments", Actions: []string{"list", "completed", "due"}}, + {Name: "assignments", Category: "scheduling", Description: "View my assignments", Actions: []string{"list", "completed", "due", "prioritize", "deprioritize", "reorder"}}, {Name: "calendars", Category: "scheduling", Description: "View and recolor calendars", Actions: []string{"show", "update"}}, }, }, From bb89073a0c1308527b843b568eaf476206e21ba1 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 31 Jul 2026 20:12:05 -0700 Subject: [PATCH 5/8] Extend todos and checkins with the remaining v0.12.0 operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit todos create --loose creates directly on the project's to-do set, outside any list. The flag is not spelled --todoset because that name is taken and means something else: which to-do set, versus no list at all. It resolves the set with ensureTodoset and skips todolist resolution entirely — no prompt, no config fallback, no name lookup — and refuses --list, since the two flags ask for opposite things. Creates are not idempotent and the SDK does not retry them, so a transient failure stays a failure rather than risking a duplicate. checkins gains pause/resume, notify, answerers, and an account-wide reminders feed. notify's two settings are tri-state, and the flags are in pairs for that reason: an unpassed flag stays out of the request so the server leaves that setting alone, while an explicit --no-... sends false. A single bool cannot distinguish "off" from "not mentioned" and would silently overwrite a setting nobody named. Naming no setting at all is refused rather than sent as an empty update. Neither answerers nor reminders registers --page. Both option structs document that the page number is not honored, so the flag would accept a value it could not act on — the defect the pagination contract exists to prevent. --limit is a real SDK-side bound and stays. A test asserts the absence, so a later "consistency" pass cannot add it back by reflex. The reminder feed flattens its rows: it nests the question, and the generic renderer skips nested objects, so a generic render would say when something is due without saying what or where. --- .surface | 261 ++++++++++++++++ e2e/smoke/smoke_checkins.bats | 32 ++ e2e/smoke/smoke_core.bats | 6 + internal/commands/checkins.go | 293 ++++++++++++++++++ .../commands/checkins_question_admin_test.go | 190 ++++++++++++ internal/commands/commands.go | 2 +- internal/commands/todos.go | 95 ++++-- internal/commands/todos_loose_test.go | 105 +++++++ 8 files changed, 958 insertions(+), 26 deletions(-) create mode 100644 internal/commands/checkins_question_admin_test.go create mode 100644 internal/commands/todos_loose_test.go diff --git a/.surface b/.surface index 2d2fb7fb..1e8e2333 100644 --- a/.surface +++ b/.surface @@ -88,7 +88,11 @@ ARG basecamp checkin answer update 00 ARG basecamp checkin answer update 01 ARG basecamp checkin answers 00 [question_id|url] ARG basecamp checkin question 00 +ARG basecamp checkin question answerers 00 ARG basecamp checkin question create 00 +ARG basecamp checkin question notify 00 <id|url> +ARG basecamp checkin question pause 00 <id|url> +ARG basecamp checkin question resume 00 <id|url> ARG basecamp checkin question show 00 <id|url> ARG basecamp checkin question update 00 <id|url> ARG basecamp checkin question update 01 [title] @@ -100,7 +104,11 @@ ARG basecamp checkins answer update 00 <id|url> ARG basecamp checkins answer update 01 <content> ARG basecamp checkins answers 00 [question_id|url] ARG basecamp checkins question 00 <id|url> +ARG basecamp checkins question answerers 00 <id|url> ARG basecamp checkins question create 00 <title> +ARG basecamp checkins question notify 00 <id|url> +ARG basecamp checkins question pause 00 <id|url> +ARG basecamp checkins question resume 00 <id|url> ARG basecamp checkins question show 00 <id|url> ARG basecamp checkins question update 00 <id|url> ARG basecamp checkins question update 01 [title] @@ -558,10 +566,15 @@ CMD basecamp checkin answer show CMD basecamp checkin answer update CMD basecamp checkin answers CMD basecamp checkin question +CMD basecamp checkin question answerers CMD basecamp checkin question create +CMD basecamp checkin question notify +CMD basecamp checkin question pause +CMD basecamp checkin question resume CMD basecamp checkin question show CMD basecamp checkin question update CMD basecamp checkin questions +CMD basecamp checkin reminders CMD basecamp checkins CMD basecamp checkins answer CMD basecamp checkins answer create @@ -569,10 +582,15 @@ CMD basecamp checkins answer show CMD basecamp checkins answer update CMD basecamp checkins answers CMD basecamp checkins question +CMD basecamp checkins question answerers CMD basecamp checkins question create +CMD basecamp checkins question notify +CMD basecamp checkins question pause +CMD basecamp checkins question resume CMD basecamp checkins question show CMD basecamp checkins question update CMD basecamp checkins questions +CMD basecamp checkins reminders CMD basecamp cmds CMD basecamp commands CMD basecamp comments @@ -3932,6 +3950,29 @@ FLAG basecamp checkin question --stats type=bool FLAG basecamp checkin question --styled type=bool FLAG basecamp checkin question --todolist type=string FLAG basecamp checkin question --verbose type=count +FLAG basecamp checkin question answerers --account type=string +FLAG basecamp checkin question answerers --agent type=bool +FLAG basecamp checkin question answerers --cache-dir type=string +FLAG basecamp checkin question answerers --count type=bool +FLAG basecamp checkin question answerers --help type=bool +FLAG basecamp checkin question answerers --hints type=bool +FLAG basecamp checkin question answerers --ids-only type=bool +FLAG basecamp checkin question answerers --in type=string +FLAG basecamp checkin question answerers --jq type=string +FLAG basecamp checkin question answerers --json type=bool +FLAG basecamp checkin question answerers --limit type=int +FLAG basecamp checkin question answerers --markdown type=bool +FLAG basecamp checkin question answerers --md type=bool +FLAG basecamp checkin question answerers --no-hints type=bool +FLAG basecamp checkin question answerers --no-stats type=bool +FLAG basecamp checkin question answerers --profile type=string +FLAG basecamp checkin question answerers --project type=string +FLAG basecamp checkin question answerers --questionnaire type=string +FLAG basecamp checkin question answerers --quiet type=bool +FLAG basecamp checkin question answerers --stats type=bool +FLAG basecamp checkin question answerers --styled type=bool +FLAG basecamp checkin question answerers --todolist type=string +FLAG basecamp checkin question answerers --verbose type=count FLAG basecamp checkin question create --account type=string FLAG basecamp checkin question create --agent type=bool FLAG basecamp checkin question create --cache-dir type=string @@ -3958,6 +3999,76 @@ FLAG basecamp checkin question create --time type=string FLAG basecamp checkin question create --todolist type=string FLAG basecamp checkin question create --verbose type=count FLAG basecamp checkin question create --visible-to-clients type=bool +FLAG basecamp checkin question notify --account type=string +FLAG basecamp checkin question notify --agent type=bool +FLAG basecamp checkin question notify --cache-dir type=string +FLAG basecamp checkin question notify --count type=bool +FLAG basecamp checkin question notify --digest-include-unanswered type=bool +FLAG basecamp checkin question notify --help type=bool +FLAG basecamp checkin question notify --hints type=bool +FLAG basecamp checkin question notify --ids-only type=bool +FLAG basecamp checkin question notify --in type=string +FLAG basecamp checkin question notify --jq type=string +FLAG basecamp checkin question notify --json type=bool +FLAG basecamp checkin question notify --markdown type=bool +FLAG basecamp checkin question notify --md type=bool +FLAG basecamp checkin question notify --no-digest-include-unanswered type=bool +FLAG basecamp checkin question notify --no-hints type=bool +FLAG basecamp checkin question notify --no-on-answer type=bool +FLAG basecamp checkin question notify --no-stats type=bool +FLAG basecamp checkin question notify --on-answer type=bool +FLAG basecamp checkin question notify --profile type=string +FLAG basecamp checkin question notify --project type=string +FLAG basecamp checkin question notify --questionnaire type=string +FLAG basecamp checkin question notify --quiet type=bool +FLAG basecamp checkin question notify --stats type=bool +FLAG basecamp checkin question notify --styled type=bool +FLAG basecamp checkin question notify --todolist type=string +FLAG basecamp checkin question notify --verbose type=count +FLAG basecamp checkin question pause --account type=string +FLAG basecamp checkin question pause --agent type=bool +FLAG basecamp checkin question pause --cache-dir type=string +FLAG basecamp checkin question pause --count type=bool +FLAG basecamp checkin question pause --help type=bool +FLAG basecamp checkin question pause --hints type=bool +FLAG basecamp checkin question pause --ids-only type=bool +FLAG basecamp checkin question pause --in type=string +FLAG basecamp checkin question pause --jq type=string +FLAG basecamp checkin question pause --json type=bool +FLAG basecamp checkin question pause --markdown type=bool +FLAG basecamp checkin question pause --md type=bool +FLAG basecamp checkin question pause --no-hints type=bool +FLAG basecamp checkin question pause --no-stats type=bool +FLAG basecamp checkin question pause --profile type=string +FLAG basecamp checkin question pause --project type=string +FLAG basecamp checkin question pause --questionnaire type=string +FLAG basecamp checkin question pause --quiet type=bool +FLAG basecamp checkin question pause --stats type=bool +FLAG basecamp checkin question pause --styled type=bool +FLAG basecamp checkin question pause --todolist type=string +FLAG basecamp checkin question pause --verbose type=count +FLAG basecamp checkin question resume --account type=string +FLAG basecamp checkin question resume --agent type=bool +FLAG basecamp checkin question resume --cache-dir type=string +FLAG basecamp checkin question resume --count type=bool +FLAG basecamp checkin question resume --help type=bool +FLAG basecamp checkin question resume --hints type=bool +FLAG basecamp checkin question resume --ids-only type=bool +FLAG basecamp checkin question resume --in type=string +FLAG basecamp checkin question resume --jq type=string +FLAG basecamp checkin question resume --json type=bool +FLAG basecamp checkin question resume --markdown type=bool +FLAG basecamp checkin question resume --md type=bool +FLAG basecamp checkin question resume --no-hints type=bool +FLAG basecamp checkin question resume --no-stats type=bool +FLAG basecamp checkin question resume --profile type=string +FLAG basecamp checkin question resume --project type=string +FLAG basecamp checkin question resume --questionnaire type=string +FLAG basecamp checkin question resume --quiet type=bool +FLAG basecamp checkin question resume --stats type=bool +FLAG basecamp checkin question resume --styled type=bool +FLAG basecamp checkin question resume --todolist type=string +FLAG basecamp checkin question resume --verbose type=count FLAG basecamp checkin question show --account type=string FLAG basecamp checkin question show --agent type=bool FLAG basecamp checkin question show --all-comments type=bool @@ -4033,6 +4144,29 @@ FLAG basecamp checkin questions --stats type=bool FLAG basecamp checkin questions --styled type=bool FLAG basecamp checkin questions --todolist type=string FLAG basecamp checkin questions --verbose type=count +FLAG basecamp checkin reminders --account type=string +FLAG basecamp checkin reminders --agent type=bool +FLAG basecamp checkin reminders --cache-dir type=string +FLAG basecamp checkin reminders --count type=bool +FLAG basecamp checkin reminders --help type=bool +FLAG basecamp checkin reminders --hints type=bool +FLAG basecamp checkin reminders --ids-only type=bool +FLAG basecamp checkin reminders --in type=string +FLAG basecamp checkin reminders --jq type=string +FLAG basecamp checkin reminders --json type=bool +FLAG basecamp checkin reminders --limit type=int +FLAG basecamp checkin reminders --markdown type=bool +FLAG basecamp checkin reminders --md type=bool +FLAG basecamp checkin reminders --no-hints type=bool +FLAG basecamp checkin reminders --no-stats type=bool +FLAG basecamp checkin reminders --profile type=string +FLAG basecamp checkin reminders --project type=string +FLAG basecamp checkin reminders --questionnaire type=string +FLAG basecamp checkin reminders --quiet type=bool +FLAG basecamp checkin reminders --stats type=bool +FLAG basecamp checkin reminders --styled type=bool +FLAG basecamp checkin reminders --todolist type=string +FLAG basecamp checkin reminders --verbose type=count FLAG basecamp checkins --account type=string FLAG basecamp checkins --agent type=bool FLAG basecamp checkins --cache-dir type=string @@ -4203,6 +4337,29 @@ FLAG basecamp checkins question --stats type=bool FLAG basecamp checkins question --styled type=bool FLAG basecamp checkins question --todolist type=string FLAG basecamp checkins question --verbose type=count +FLAG basecamp checkins question answerers --account type=string +FLAG basecamp checkins question answerers --agent type=bool +FLAG basecamp checkins question answerers --cache-dir type=string +FLAG basecamp checkins question answerers --count type=bool +FLAG basecamp checkins question answerers --help type=bool +FLAG basecamp checkins question answerers --hints type=bool +FLAG basecamp checkins question answerers --ids-only type=bool +FLAG basecamp checkins question answerers --in type=string +FLAG basecamp checkins question answerers --jq type=string +FLAG basecamp checkins question answerers --json type=bool +FLAG basecamp checkins question answerers --limit type=int +FLAG basecamp checkins question answerers --markdown type=bool +FLAG basecamp checkins question answerers --md type=bool +FLAG basecamp checkins question answerers --no-hints type=bool +FLAG basecamp checkins question answerers --no-stats type=bool +FLAG basecamp checkins question answerers --profile type=string +FLAG basecamp checkins question answerers --project type=string +FLAG basecamp checkins question answerers --questionnaire type=string +FLAG basecamp checkins question answerers --quiet type=bool +FLAG basecamp checkins question answerers --stats type=bool +FLAG basecamp checkins question answerers --styled type=bool +FLAG basecamp checkins question answerers --todolist type=string +FLAG basecamp checkins question answerers --verbose type=count FLAG basecamp checkins question create --account type=string FLAG basecamp checkins question create --agent type=bool FLAG basecamp checkins question create --cache-dir type=string @@ -4229,6 +4386,76 @@ FLAG basecamp checkins question create --time type=string FLAG basecamp checkins question create --todolist type=string FLAG basecamp checkins question create --verbose type=count FLAG basecamp checkins question create --visible-to-clients type=bool +FLAG basecamp checkins question notify --account type=string +FLAG basecamp checkins question notify --agent type=bool +FLAG basecamp checkins question notify --cache-dir type=string +FLAG basecamp checkins question notify --count type=bool +FLAG basecamp checkins question notify --digest-include-unanswered type=bool +FLAG basecamp checkins question notify --help type=bool +FLAG basecamp checkins question notify --hints type=bool +FLAG basecamp checkins question notify --ids-only type=bool +FLAG basecamp checkins question notify --in type=string +FLAG basecamp checkins question notify --jq type=string +FLAG basecamp checkins question notify --json type=bool +FLAG basecamp checkins question notify --markdown type=bool +FLAG basecamp checkins question notify --md type=bool +FLAG basecamp checkins question notify --no-digest-include-unanswered type=bool +FLAG basecamp checkins question notify --no-hints type=bool +FLAG basecamp checkins question notify --no-on-answer type=bool +FLAG basecamp checkins question notify --no-stats type=bool +FLAG basecamp checkins question notify --on-answer type=bool +FLAG basecamp checkins question notify --profile type=string +FLAG basecamp checkins question notify --project type=string +FLAG basecamp checkins question notify --questionnaire type=string +FLAG basecamp checkins question notify --quiet type=bool +FLAG basecamp checkins question notify --stats type=bool +FLAG basecamp checkins question notify --styled type=bool +FLAG basecamp checkins question notify --todolist type=string +FLAG basecamp checkins question notify --verbose type=count +FLAG basecamp checkins question pause --account type=string +FLAG basecamp checkins question pause --agent type=bool +FLAG basecamp checkins question pause --cache-dir type=string +FLAG basecamp checkins question pause --count type=bool +FLAG basecamp checkins question pause --help type=bool +FLAG basecamp checkins question pause --hints type=bool +FLAG basecamp checkins question pause --ids-only type=bool +FLAG basecamp checkins question pause --in type=string +FLAG basecamp checkins question pause --jq type=string +FLAG basecamp checkins question pause --json type=bool +FLAG basecamp checkins question pause --markdown type=bool +FLAG basecamp checkins question pause --md type=bool +FLAG basecamp checkins question pause --no-hints type=bool +FLAG basecamp checkins question pause --no-stats type=bool +FLAG basecamp checkins question pause --profile type=string +FLAG basecamp checkins question pause --project type=string +FLAG basecamp checkins question pause --questionnaire type=string +FLAG basecamp checkins question pause --quiet type=bool +FLAG basecamp checkins question pause --stats type=bool +FLAG basecamp checkins question pause --styled type=bool +FLAG basecamp checkins question pause --todolist type=string +FLAG basecamp checkins question pause --verbose type=count +FLAG basecamp checkins question resume --account type=string +FLAG basecamp checkins question resume --agent type=bool +FLAG basecamp checkins question resume --cache-dir type=string +FLAG basecamp checkins question resume --count type=bool +FLAG basecamp checkins question resume --help type=bool +FLAG basecamp checkins question resume --hints type=bool +FLAG basecamp checkins question resume --ids-only type=bool +FLAG basecamp checkins question resume --in type=string +FLAG basecamp checkins question resume --jq type=string +FLAG basecamp checkins question resume --json type=bool +FLAG basecamp checkins question resume --markdown type=bool +FLAG basecamp checkins question resume --md type=bool +FLAG basecamp checkins question resume --no-hints type=bool +FLAG basecamp checkins question resume --no-stats type=bool +FLAG basecamp checkins question resume --profile type=string +FLAG basecamp checkins question resume --project type=string +FLAG basecamp checkins question resume --questionnaire type=string +FLAG basecamp checkins question resume --quiet type=bool +FLAG basecamp checkins question resume --stats type=bool +FLAG basecamp checkins question resume --styled type=bool +FLAG basecamp checkins question resume --todolist type=string +FLAG basecamp checkins question resume --verbose type=count FLAG basecamp checkins question show --account type=string FLAG basecamp checkins question show --agent type=bool FLAG basecamp checkins question show --all-comments type=bool @@ -4304,6 +4531,29 @@ FLAG basecamp checkins questions --stats type=bool FLAG basecamp checkins questions --styled type=bool FLAG basecamp checkins questions --todolist type=string FLAG basecamp checkins questions --verbose type=count +FLAG basecamp checkins reminders --account type=string +FLAG basecamp checkins reminders --agent type=bool +FLAG basecamp checkins reminders --cache-dir type=string +FLAG basecamp checkins reminders --count type=bool +FLAG basecamp checkins reminders --help type=bool +FLAG basecamp checkins reminders --hints type=bool +FLAG basecamp checkins reminders --ids-only type=bool +FLAG basecamp checkins reminders --in type=string +FLAG basecamp checkins reminders --jq type=string +FLAG basecamp checkins reminders --json type=bool +FLAG basecamp checkins reminders --limit type=int +FLAG basecamp checkins reminders --markdown type=bool +FLAG basecamp checkins reminders --md type=bool +FLAG basecamp checkins reminders --no-hints type=bool +FLAG basecamp checkins reminders --no-stats type=bool +FLAG basecamp checkins reminders --profile type=string +FLAG basecamp checkins reminders --project type=string +FLAG basecamp checkins reminders --questionnaire type=string +FLAG basecamp checkins reminders --quiet type=bool +FLAG basecamp checkins reminders --stats type=bool +FLAG basecamp checkins reminders --styled type=bool +FLAG basecamp checkins reminders --todolist type=string +FLAG basecamp checkins reminders --verbose type=count FLAG basecamp cmds --account type=string FLAG basecamp cmds --agent type=bool FLAG basecamp cmds --cache-dir type=string @@ -14003,6 +14253,7 @@ FLAG basecamp todos create --in type=string FLAG basecamp todos create --jq type=string FLAG basecamp todos create --json type=bool FLAG basecamp todos create --list type=string +FLAG basecamp todos create --loose type=bool FLAG basecamp todos create --markdown type=bool FLAG basecamp todos create --md type=bool FLAG basecamp todos create --no-hints type=bool @@ -17025,10 +17276,15 @@ SUB basecamp checkin answer show SUB basecamp checkin answer update SUB basecamp checkin answers SUB basecamp checkin question +SUB basecamp checkin question answerers SUB basecamp checkin question create +SUB basecamp checkin question notify +SUB basecamp checkin question pause +SUB basecamp checkin question resume SUB basecamp checkin question show SUB basecamp checkin question update SUB basecamp checkin questions +SUB basecamp checkin reminders SUB basecamp checkins SUB basecamp checkins answer SUB basecamp checkins answer create @@ -17036,10 +17292,15 @@ SUB basecamp checkins answer show SUB basecamp checkins answer update SUB basecamp checkins answers SUB basecamp checkins question +SUB basecamp checkins question answerers SUB basecamp checkins question create +SUB basecamp checkins question notify +SUB basecamp checkins question pause +SUB basecamp checkins question resume SUB basecamp checkins question show SUB basecamp checkins question update SUB basecamp checkins questions +SUB basecamp checkins reminders SUB basecamp cmds SUB basecamp commands SUB basecamp comments diff --git a/e2e/smoke/smoke_checkins.bats b/e2e/smoke/smoke_checkins.bats index 3e09980a..b39b70b5 100644 --- a/e2e/smoke/smoke_checkins.bats +++ b/e2e/smoke/smoke_checkins.bats @@ -56,3 +56,35 @@ setup_file() { assert_json_value '.ok' 'true' assert_json_not_null '.data.id' } + +@test "checkins reminders lists pending reminders" { + run_smoke basecamp checkins reminders --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "checkins reminders honors --limit" { + run_smoke basecamp checkins reminders --limit 1 --json + assert_success + assert_json_value '.ok' 'true' +} + +@test "checkins question answerers rejects a non-id" { + run_smoke basecamp checkins question answerers not-an-id --json + assert_failure +} + +@test "checkins question notify requires a setting" { + # Naming no setting would be a no-op write, so it is refused before the + # request rather than sent as an empty update. + run_smoke basecamp checkins question notify 999999 --json + assert_failure +} + +@test "checkins question pause is out of scope" { + mark_out_of_scope "Mutating - pauses a live recurring question" +} + +@test "checkins question resume is out of scope" { + mark_out_of_scope "Mutating - resumes a live recurring question" +} diff --git a/e2e/smoke/smoke_core.bats b/e2e/smoke/smoke_core.bats index bfa14b80..307b84c1 100644 --- a/e2e/smoke/smoke_core.bats +++ b/e2e/smoke/smoke_core.bats @@ -78,3 +78,9 @@ setup_file() { assert_success assert_output_contains "USAGE" } + +@test "todos create --loose rejects --list" { + # --loose creates outside any list, so naming one contradicts it. + run_smoke basecamp todos create "smoke loose conflict" --loose --list 999999 --json + assert_failure +} diff --git a/internal/commands/checkins.go b/internal/commands/checkins.go index fc423fbc..885d2748 100644 --- a/internal/commands/checkins.go +++ b/internal/commands/checkins.go @@ -41,11 +41,82 @@ on a schedule (e.g., "What did you work on today?").`, newCheckinsQuestionCmd(&project), newCheckinsAnswersCmd(&project, &questionnaireID), newCheckinsAnswerCmd(&project), + newCheckinsRemindersCmd(), ) return cmd } +func newCheckinsRemindersCmd() *cobra.Command { + var limit int + + cmd := &cobra.Command{ + Use: "reminders", + Short: "List your pending check-in reminders", + Long: `List the check-in questions you are due to answer. + +This is your own reminder feed across every project, so it takes no +--project. + + basecamp checkins reminders + basecamp checkins reminders --limit 10`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + if limit < 0 { + return output.ErrUsage("--limit must be zero or positive") + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + // --limit is a real SDK-side bound rather than a local trim. There + // is deliberately no --page: QuestionReminderListOptions does not + // honor a page number, so the flag could not do what it says. + result, err := app.Account().Checkins().ListQuestionReminders(cmd.Context(), &basecamp.QuestionReminderListOptions{Limit: limit}) + if err != nil { + return convertSDKError(err) + } + + return app.OK(result.Reminders, + output.WithDisplayData(flattenQuestionReminders(result.Reminders)), + output.WithSummary(fmt.Sprintf("%d pending check-in reminders", len(result.Reminders))), + output.WithBreadcrumbs(output.Breadcrumb{ + Action: "answer", + Cmd: "basecamp checkins answer <question-id> \"<content>\"", + Description: "Answer a check-in question", + }), + ) + }, + } + + cmd.Flags().IntVarP(&limit, "limit", "n", 0, "Maximum reminders to return") + + return cmd +} + +// flattenQuestionReminders builds the display rows for the reminder feed. +// +// A QuestionReminder nests the question it is about, and the renderer skips +// nested objects — so a generic render would show a timestamp and nothing that +// says which question is due, or where. +func flattenQuestionReminders(reminders []basecamp.QuestionReminder) []map[string]any { + rows := make([]map[string]any, 0, len(reminders)) + for _, r := range reminders { + row := map[string]any{ + "question_id": r.Question.ID, + "question": r.Question.Title, + "remind_at": r.RemindAt, + } + if r.Question.Bucket != nil { + row["project"] = r.Question.Bucket.Name + } + rows = append(rows, row) + } + return rows +} + func newCheckinsQuestionsCmd(project, questionnaireID *string) *cobra.Command { var limit int var page int @@ -169,11 +240,233 @@ func newCheckinsQuestionCmd(project *string) *cobra.Command { newCheckinsQuestionShowCmd(project), newCheckinsQuestionCreateCmd(project), newCheckinsQuestionUpdateCmd(project), + newCheckinsQuestionPauseCmd(), + newCheckinsQuestionResumeCmd(), + newCheckinsQuestionNotifyCmd(), + newCheckinsQuestionAnswerersCmd(), + ) + + return cmd +} + +func newCheckinsQuestionPauseCmd() *cobra.Command { + return &cobra.Command{ + Use: "pause <id|url>", + Short: "Stop a question from being asked", + Long: `Stop a question from being asked on its schedule. + +The question and its existing answers stay put; only the recurring prompt +stops. Resume it with 'basecamp checkins question resume <id>'.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runCheckinsQuestionSchedule(cmd, args[0], true) + }, + } +} + +func newCheckinsQuestionResumeCmd() *cobra.Command { + return &cobra.Command{ + Use: "resume <id|url>", + Short: "Start asking a paused question again", + Long: "Start asking a paused question on its schedule again.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runCheckinsQuestionSchedule(cmd, args[0], false) + }, + } +} + +// runCheckinsQuestionSchedule performs pause/resume, which differ only in the +// call and the wording. +func runCheckinsQuestionSchedule(cmd *cobra.Command, arg string, pause bool) error { + app := appctx.FromContext(cmd.Context()) + + questionID, err := checkinsQuestionID(arg) + if err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + summary := fmt.Sprintf("Resumed question %d", questionID) + if pause { + err = app.Account().Checkins().PauseQuestion(cmd.Context(), questionID) + summary = fmt.Sprintf("Paused question %d", questionID) + } else { + err = app.Account().Checkins().ResumeQuestion(cmd.Context(), questionID) + } + if err != nil { + return convertSDKError(err) + } + + return app.OK(map[string]any{"id": questionID, "paused": pause}, + output.WithSummary(summary), + output.WithBreadcrumbs(output.Breadcrumb{ + Action: "show", + Cmd: fmt.Sprintf("basecamp checkins question show %d", questionID), + Description: "View the question", + }), + ) +} + +func newCheckinsQuestionNotifyCmd() *cobra.Command { + var ( + onAnswer bool + noOnAnswer bool + includeUnanswered bool + noIncludeUnanswer bool ) + cmd := &cobra.Command{ + Use: "notify <id|url>", + Short: "Change your notification settings for a question", + Long: `Change your own notification settings for a check-in question. + +Each setting is left alone unless you name it, so you can change one without +restating the other: + + basecamp checkins question notify 789 --on-answer + basecamp checkins question notify 789 --no-on-answer + basecamp checkins question notify 789 --digest-include-unanswered`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + questionID, err := checkinsQuestionID(args[0]) + if err != nil { + return err + } + + // Tri-state: an unpassed flag stays nil so the server leaves that + // setting alone, and an explicit --no-... sends false rather than + // being indistinguishable from "not mentioned". + req := &basecamp.UpdateQuestionNotificationSettingsRequest{} + if req.NotifyOnAnswer, err = checkinsTriState(cmd, "on-answer", "no-on-answer", onAnswer, noOnAnswer); err != nil { + return err + } + if req.DigestIncludeUnanswered, err = checkinsTriState(cmd, + "digest-include-unanswered", "no-digest-include-unanswered", + includeUnanswered, noIncludeUnanswer); err != nil { + return err + } + if req.NotifyOnAnswer == nil && req.DigestIncludeUnanswered == nil { + return output.ErrUsageHint( + "no notification setting was named", + "Pass --on-answer/--no-on-answer or --digest-include-unanswered/--no-digest-include-unanswered") + } + + if err := ensureAccount(cmd, app); err != nil { + return err + } + + settings, err := app.Account().Checkins().UpdateQuestionNotificationSettings(cmd.Context(), questionID, req) + if err != nil { + return convertSDKError(err) + } + + return app.OK(settings, + output.WithSummary(fmt.Sprintf("Updated your notification settings for question %d", questionID)), + output.WithBreadcrumbs(output.Breadcrumb{ + Action: "show", + Cmd: fmt.Sprintf("basecamp checkins question show %d", questionID), + Description: "View the question", + }), + ) + }, + } + + cmd.Flags().BoolVar(&onAnswer, "on-answer", false, "Notify you when someone answers") + cmd.Flags().BoolVar(&noOnAnswer, "no-on-answer", false, "Stop notifying you when someone answers") + cmd.Flags().BoolVar(&includeUnanswered, "digest-include-unanswered", false, "Include unanswered questions in your digest") + cmd.Flags().BoolVar(&noIncludeUnanswer, "no-digest-include-unanswered", false, "Exclude unanswered questions from your digest") + + return cmd +} + +// checkinsTriState resolves an on/off flag pair into the SDK's *bool. +// +// nil means "not mentioned, leave it alone", which is why these are two flags +// rather than one bool: a single --on-answer=false would be indistinguishable +// from omitting it, and would silently overwrite a setting the caller never +// asked about. +func checkinsTriState(cmd *cobra.Command, onName, offName string, on, off bool) (*bool, error) { + setOn := cmd.Flags().Changed(onName) && on + setOff := cmd.Flags().Changed(offName) && off + + switch { + case setOn && setOff: + return nil, output.ErrUsage(fmt.Sprintf("--%s and --%s are mutually exclusive", onName, offName)) + case setOn: + value := true + return &value, nil + case setOff: + value := false + return &value, nil + default: + return nil, nil + } +} + +func newCheckinsQuestionAnswerersCmd() *cobra.Command { + var limit int + + cmd := &cobra.Command{ + Use: "answerers <id|url>", + Short: "List the people who answer a question", + Long: `List the people who answer a check-in question. + + basecamp checkins question answerers 789`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + questionID, err := checkinsQuestionID(args[0]) + if err != nil { + return err + } + if limit < 0 { + return output.ErrUsage("--limit must be zero or positive") + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + + // No --page here: PeopleListOptions.Page does not honor the page + // number, so a flag for it could not do what it says. + result, err := app.Account().Checkins().ListAnswerers(cmd.Context(), questionID, &basecamp.PeopleListOptions{Limit: limit}) + if err != nil { + return convertSDKError(err) + } + + return app.OK(result.People, + output.WithSummary(fmt.Sprintf("%d people answer question %d", len(result.People), questionID)), + output.WithBreadcrumbs(output.Breadcrumb{ + Action: "answers", + Cmd: fmt.Sprintf("basecamp checkins answers %d", questionID), + Description: "Read the answers", + }), + ) + }, + } + + cmd.Flags().IntVarP(&limit, "limit", "n", 0, "Maximum people to return") + return cmd } +// checkinsQuestionID resolves the <id|url> positional the question verbs take. +func checkinsQuestionID(arg string) (int64, error) { + id, err := strconv.ParseInt(extractID(arg), 10, 64) + if err != nil { + return 0, output.ErrUsageHint( + fmt.Sprintf("%q is not a question id or Basecamp URL", arg), + "Pass a numeric question id, or paste the question's Basecamp URL", + ) + } + return id, nil +} + func newCheckinsQuestionShowCmd(project *string) *cobra.Command { cmd := &cobra.Command{ Use: "show <id|url>", diff --git a/internal/commands/checkins_question_admin_test.go b/internal/commands/checkins_question_admin_test.go new file mode 100644 index 00000000..46c034cc --- /dev/null +++ b/internal/commands/checkins_question_admin_test.go @@ -0,0 +1,190 @@ +package commands + +import ( + "encoding/json" + "fmt" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" +) + +const checkinsRemindersPath = "/99999/my/question_reminders.json" + +func checkinsQuestionSubPath(id int64, suffix string) string { + return fmt.Sprintf("/99999/questions/%d/%s", id, suffix) +} + +func TestCheckinsQuestionPauseAndResume(t *testing.T) { + for _, tc := range []struct { + verb string + suffix string + method string + }{ + // resume is a DELETE against the same pause resource, not its own path. + {"pause", "pause.json", http.MethodPost}, + {"resume", "pause.json", http.MethodDelete}, + } { + t.Run(tc.verb, func(t *testing.T) { + path := checkinsQuestionSubPath(789, tc.suffix) + app, transport, _ := setupPersonalFeedApp(t, noContentRoute(tc.method, path)) + + require.NoError(t, executeRecordingCommand(NewCheckinsCmd(), app, "question", tc.verb, "789")) + + call := transport.last(t) + assert.Equal(t, tc.method, call.Method) + assert.Equal(t, path, call.Path) + }) + } +} + +// The notification settings are tri-state: an unpassed flag must stay out of +// the request entirely so the server leaves that setting alone, while an +// explicit --no-... must send false. A single bool could not tell those apart +// and would silently overwrite a setting nobody asked about. +func TestCheckinsQuestionNotifyIsTriState(t *testing.T) { + settingsRoute := stubRoute{ + method: http.MethodPut, + path: checkinsQuestionSubPath(789, "notification_settings.json"), + status: http.StatusOK, + body: `{"responding": true, "subscribed": true}`, + } + + for _, tc := range []struct { + name string + args []string + want map[string]any + }{ + { + name: "only --on-answer is sent", + args: []string{"question", "notify", "789", "--on-answer"}, + want: map[string]any{"notify_on_answer": true}, + }, + { + name: "--no-on-answer sends an explicit false", + args: []string{"question", "notify", "789", "--no-on-answer"}, + want: map[string]any{"notify_on_answer": false}, + }, + { + name: "the untouched setting is omitted", + args: []string{"question", "notify", "789", "--digest-include-unanswered"}, + want: map[string]any{"digest_include_unanswered": true}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, settingsRoute) + + require.NoError(t, executeRecordingCommand(NewCheckinsCmd(), app, tc.args...)) + + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(transport.last(t).Body), &body)) + assert.Equal(t, tc.want, body, "only the named settings may reach the wire") + }) + } +} + +func TestCheckinsQuestionNotifyRejectsContradictoryFlags(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t) + + err := executeRecordingCommand(NewCheckinsCmd(), app, + "question", "notify", "789", "--on-answer", "--no-on-answer") + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Message, "mutually exclusive") + assert.Empty(t, transport.recorded()) +} + +func TestCheckinsQuestionNotifyRequiresASetting(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t) + + err := executeRecordingCommand(NewCheckinsCmd(), app, "question", "notify", "789") + + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded(), "a no-op update must not reach the server") +} + +func TestCheckinsQuestionAnswerersLists(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, stubRoute{ + method: http.MethodGet, + path: checkinsQuestionSubPath(789, "answers/by.json"), + status: http.StatusOK, + body: `[{"id": 1, "name": "Ann"}, {"id": 2, "name": "Bob"}]`, + }) + + require.NoError(t, executeRecordingCommand(NewCheckinsCmd(), app, "question", "answerers", "789")) + + assert.Equal(t, checkinsQuestionSubPath(789, "answers/by.json"), transport.last(t).Path) +} + +// PeopleListOptions and QuestionReminderListOptions both document that the page +// number is not honored, so neither command registers --page. A flag that +// cannot do what it says is the defect the pagination contract exists to stop. +func TestCheckinsPageIsNotRegisteredWhereItCannotWork(t *testing.T) { + root := NewCheckinsCmd() + + for _, path := range [][]string{ + {"question", "answerers"}, + {"reminders"}, + } { + t.Run(fmt.Sprint(path), func(t *testing.T) { + cmd, _, err := root.Find(path) + require.NoError(t, err) + assert.Nil(t, cmd.Flags().Lookup("page"), + "the SDK does not honor a page number here, so no --page may be offered") + assert.NotNil(t, cmd.Flags().Lookup("limit"), "--limit is a real bound and stays") + }) + } +} + +func TestCheckinsRemindersLists(t *testing.T) { + app, transport, out := setupPersonalFeedApp(t, stubRoute{ + method: http.MethodGet, + path: checkinsRemindersPath, + status: http.StatusOK, + body: `[{ + "group_on": "2026-08-01", + "remind_at": "2026-08-01T09:00:00.000Z", + "reminder_id": 5, + "question": { + "id": 789, "title": "What did you work on?", "status": "active", + "created_at": "2026-06-01T10:00:00.000Z", + "updated_at": "2026-06-01T10:00:00.000Z", + "bucket": {"id": 977190, "name": "JD test proj", "type": "Project"} + } + }]`, + }) + + require.NoError(t, executeRecordingCommand(NewCheckinsCmd(), app, "reminders")) + + assert.Equal(t, checkinsRemindersPath, transport.last(t).Path) + + var envelope struct { + Summary string `json:"summary"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &envelope)) + assert.Equal(t, "1 pending check-in reminders", envelope.Summary) +} + +// The reminder feed nests the question it is about, and the renderer skips +// nested objects — so a generic render would say when something is due without +// saying what, or where. +func TestFlattenQuestionRemindersCarriesTheQuestion(t *testing.T) { + rows := flattenQuestionReminders([]basecamp.QuestionReminder{{ + RemindAt: time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC), + Question: basecamp.Question{ + ID: 789, + Title: "What did you work on?", + Bucket: &basecamp.Bucket{ID: 977190, Name: "JD test proj"}, + }, + }}) + + require.Len(t, rows, 1) + assert.Equal(t, int64(789), rows[0]["question_id"]) + assert.Equal(t, "What did you work on?", rows[0]["question"]) + assert.Equal(t, "JD test proj", rows[0]["project"]) + assert.Contains(t, rows[0], "remind_at") +} diff --git a/internal/commands/commands.go b/internal/commands/commands.go index 7b8063c3..0d5c4e4a 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -45,7 +45,7 @@ func CommandCategories() []CommandCategory { {Name: "chat", Category: "core", Description: "Chat in real-time", Actions: []string{"list", "messages", "post", "upload", "line", "update", "delete"}}, {Name: "cards", Category: "core", Description: "Manage Kanban cards", Actions: []string{"list", "show", "create", "update", "move", "done", "columns", "wormholes", "steps", "trash", "archive", "restore"}}, {Name: "files", Category: "core", Description: "Manage files, documents, and folders", Actions: []string{"list", "show", "download", "update", "trash", "archive", "restore"}}, - {Name: "checkins", Category: "core", Description: "View automatic check-ins", Actions: []string{"questions", "question", "answers", "answer"}}, + {Name: "checkins", Category: "core", Description: "View automatic check-ins", Actions: []string{"questions", "question", "answers", "answer", "reminders"}}, {Name: "schedule", Category: "core", Description: "Manage schedule entries", Actions: []string{"show", "entries", "create", "update"}}, }, }, diff --git a/internal/commands/todos.go b/internal/commands/todos.go index e87ca193..f91a2f16 100644 --- a/internal/commands/todos.go +++ b/internal/commands/todos.go @@ -1164,11 +1164,19 @@ func newTodosCreateCmd() *cobra.Command { var description string var attachFiles []string var notifyOnCompletion string + var loose bool cmd := &cobra.Command{ Use: "create <content>", Short: "Create a new todo", - Long: "Create a new todo in a project.", + Long: `Create a new todo in a project. + +By default a todo goes into a to-do list. --loose creates it directly on the +project's to-do set instead, outside any list: + + basecamp todos create "Call the vendor back" --loose --in <project> + +--loose needs no list, so it neither prompts for one nor accepts --list.`, RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) if app == nil { @@ -1209,30 +1217,47 @@ func newTodosCreateCmd() *cobra.Command { } project = resolvedProject - // Use todolist from flag, config, or interactive prompt - if todolist == "" { - todolist = app.Flags.Todolist - } - if todolist == "" { - todolist = app.Config.TodolistID - } - // If still no todolist, try interactive selection (todoset-scoped) - if todolist == "" { - selectedTodolist, err := ensureTodolist(cmd, app, project, todoset) + // --loose creates directly on the to-do set, outside any list, so it + // resolves a todoset and skips todolist resolution entirely — there + // is no list to name, prompt for, or fall back to. + var resolvedTodolist, resolvedTodoset string + if loose { + if cmd.Flags().Changed("list") || app.Flags.Todolist != "" { + return output.ErrUsageHint( + "--loose creates a todo outside any list, so it cannot be combined with --list", + "Drop --list to create on the to-do set, or drop --loose to create in that list") + } + + resolvedTodoset, err = ensureTodoset(cmd, app, project, todoset) if err != nil { return err } - todolist = selectedTodolist - } + } else { + // Use todolist from flag, config, or interactive prompt + if todolist == "" { + todolist = app.Flags.Todolist + } + if todolist == "" { + todolist = app.Config.TodolistID + } + // If still no todolist, try interactive selection (todoset-scoped) + if todolist == "" { + selectedTodolist, err := ensureTodolist(cmd, app, project, todoset) + if err != nil { + return err + } + todolist = selectedTodolist + } - if todolist == "" { - return output.ErrUsage("--list is required (no default todolist found)") - } + if todolist == "" { + return output.ErrUsage("--list is required (no default todolist found)") + } - // Resolve todolist name to ID, scoped to --todoset when provided - resolvedTodolist, err := resolveTodolistInTodoset(cmd, app, todolist, project, todoset) - if err != nil { - return err + // Resolve todolist name to ID, scoped to --todoset when provided + resolvedTodolist, err = resolveTodolistInTodoset(cmd, app, todolist, project, todoset) + if err != nil { + return err + } } // Build SDK request @@ -1287,12 +1312,29 @@ func newTodosCreateCmd() *cobra.Command { req.CompletionSubscriberIDs = subscriberIDs } - todolistID, err := strconv.ParseInt(resolvedTodolist, 10, 64) - if err != nil { - return output.ErrUsage("Invalid todolist ID") - } + var todo *basecamp.Todo + if loose { + projectID, parseErr := strconv.ParseInt(project, 10, 64) + if parseErr != nil { + return output.ErrUsage("Invalid project ID") + } + todosetID, parseErr := strconv.ParseInt(resolvedTodoset, 10, 64) + if parseErr != nil { + return output.ErrUsage("Invalid todoset ID") + } + + // Creates are not idempotent and the SDK does not retry them, so + // a transient failure here surfaces as a plain error rather than + // risking a duplicate todo. + todo, err = app.Account().Todos().CreateInTodoset(cmd.Context(), projectID, todosetID, req) + } else { + todolistID, parseErr := strconv.ParseInt(resolvedTodolist, 10, 64) + if parseErr != nil { + return output.ErrUsage("Invalid todolist ID") + } - todo, err := app.Account().Todos().Create(cmd.Context(), todolistID, req) + todo, err = app.Account().Todos().Create(cmd.Context(), todolistID, req) + } if err != nil { return convertSDKError(err) } @@ -1331,6 +1373,9 @@ func newTodosCreateCmd() *cobra.Command { cmd.Flags().StringVar(&description, "description", "", "Extended description (Markdown)") cmd.Flags().StringArrayVar(&attachFiles, "attach", nil, "Attach file (repeatable)") cmd.Flags().StringVar(¬ifyOnCompletion, "notify-on-completion", "", "People to notify when done (names or IDs, comma-separated)") + // Not --todoset: that flag already means "which to-do set", and this one + // means "no list at all". + cmd.Flags().BoolVar(&loose, "loose", false, "Create on the to-do set, outside any list") // Register tab completion for flags completer := completion.NewCompleter(nil) diff --git a/internal/commands/todos_loose_test.go b/internal/commands/todos_loose_test.go new file mode 100644 index 00000000..eb4303a4 --- /dev/null +++ b/internal/commands/todos_loose_test.go @@ -0,0 +1,105 @@ +package commands + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + looseProjectPath = "/99999/projects/123.json" + looseTodosetPath = "/99999/buckets/123/todosets/300/todos.json" + looseTodolistPath = "/99999/todolists/30/todos.json" +) + +// looseDockRoute serves the project dock ensureTodoset reads to find the +// to-do set. --loose needs no todolist, so this is the only lookup it makes. +func looseDockRoute() stubRoute { + return stubRoute{ + method: http.MethodGet, + path: looseProjectPath, + status: http.StatusOK, + body: `{"id": 123, "name": "Test Project", "dock": [ + {"name": "todoset", "id": 300, "title": "To-dos", "enabled": true} + ]}`, + } +} + +func looseCreateRoute(path string) stubRoute { + return stubRoute{ + method: http.MethodPost, + path: path, + status: http.StatusCreated, + body: `{"id": 999, "content": "Call the vendor back", "status": "active", "completed": false}`, + } +} + +// --loose creates directly on the to-do set. It resolves a todoset and never +// touches todolist resolution, so no todolist request may appear. +func TestTodosCreateLooseCreatesOnTheTodoset(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, + projectsRoute(), + looseDockRoute(), + looseCreateRoute(looseTodosetPath), + ) + + require.NoError(t, executeRecordingCommand(NewTodosCmd(), app, + "create", "Call the vendor back", "--in", "123", "--loose")) + + call := transport.last(t) + assert.Equal(t, http.MethodPost, call.Method) + assert.Equal(t, looseTodosetPath, call.Path) + assert.Contains(t, call.Body, "Call the vendor back") + + for _, recorded := range transport.recorded() { + assert.NotContains(t, recorded.Path, "/todolists/", + "--loose must not resolve or create through a todolist") + } +} + +// --list and --loose ask for opposite things: one names a list, the other says +// there is none. Rejecting beats silently honoring whichever is checked first. +func TestTodosCreateLooseRejectsList(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, + projectsRoute(), + looseDockRoute(), + looseCreateRoute(looseTodosetPath), + ) + + err := executeRecordingCommand(NewTodosCmd(), app, + "create", "Call the vendor back", "--in", "123", "--loose", "--list", "30") + + outErr := requireBookmarksUsageError(t, err) + assert.Contains(t, outErr.Message, "--loose") + assert.Contains(t, outErr.Message, "--list") + + for _, recorded := range transport.recorded() { + assert.NotEqual(t, http.MethodPost, recorded.Method, + "a rejected create must not reach the server") + } +} + +// Without --loose the create path is unchanged. +func TestTodosCreateWithoutLooseStillUsesTheTodolist(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, + projectsRoute(), + looseDockRoute(), + // Only the non-loose path needs this: resolving --list walks the + // todoset's lists, which is exactly the work --loose skips. + stubRoute{ + method: http.MethodGet, + path: "/99999/todosets/300/todolists.json", + status: http.StatusOK, + body: `[{"id": 30, "name": "Sprint 1"}]`, + }, + looseCreateRoute(looseTodolistPath), + ) + + require.NoError(t, executeRecordingCommand(NewTodosCmd(), app, + "create", "Call the vendor back", "--in", "123", "--list", "30")) + + call := transport.last(t) + assert.Equal(t, looseTodolistPath, call.Path) +} From 05ae244291e90b4dc0cd85d883589834ea7df09c Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Fri, 31 Jul 2026 20:18:34 -0700 Subject: [PATCH 6/8] Track the five BC5 sections, and correct two counts while there MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit my_bookmarks, drafts, my_notes, calendars and question_reminders were the five BC5 sections bc-api#410 introduced and this matrix never tracked. They are now tracked and implemented, so the sentence saying they remain outside the matrix is gone rather than softened. Two counts were wrong in a way worth naming. The questions row claimed 5 endpoints while listing four actions, and the section also carries pause, resume, notification settings and answerers — so the 100%-of-tracked claim was resting on an undercount of the very section it counted. It now reads 8. And my_assignments was 3, missing the Up Next verbs. card_table_columns gained Subscribe/Unsubscribe in the SDK, which the CLI deliberately does not ship. 'cards column watch|unwatch' already performs the same action through the generic recording-subscription endpoint and returns the subscription details the specific endpoint does not, so the row records the alternate transport instead of the CLI growing a second spelling for one action. ACCOUNT-WIDE-LISTINGS.md picks up bookmarks and drafts as personal-feed rows. They are not EverythingService methods and have no --all-projects, but their List methods take the same page-0-means-every-page parameter, so they are exposed to the same trap and follow the same contract. Recording them is what gives the next /my/ feed a precedent to copy. checkins reminders is noted as sitting outside the table: its options struct honors no page number, so it takes --limit and no --page. SKILL.md gains the new groups in its frontmatter, triggers, Quick Reference and prose — including the Up Next id rule, which is the thing an agent is most likely to get wrong, since a wrong id there returns a successful 204 and changes nothing. --- ACCOUNT-WIDE-LISTINGS.md | 24 +++++++ API-COVERAGE.md | 63 +++++++++++++---- internal/commands/bookmarks.go | 2 +- internal/commands/bookmarks_test.go | 2 +- skills/basecamp/SKILL.md | 105 +++++++++++++++++++++++++++- 5 files changed, 179 insertions(+), 17 deletions(-) diff --git a/ACCOUNT-WIDE-LISTINGS.md b/ACCOUNT-WIDE-LISTINGS.md index de2bc250..2e0702dd 100644 --- a/ACCOUNT-WIDE-LISTINGS.md +++ b/ACCOUNT-WIDE-LISTINGS.md @@ -224,9 +224,33 @@ rows that differ for stated reasons. `--all` is how you ask for the account. | `files list` | cap 100 | **changed** from "all pages" | | `todos list --overdue` | cap 100 | unpaginated endpoint; accepts `--all`, rejects `--page` | | `cards list --overdue` | cap 100 | **changed** from uncapped; same rules as above | +| `bookmarks list` | cap 100 | personal feed — see below | +| `drafts list` | cap 100 | personal feed; server caps the full listing at 250 | Project-scoped defaults are untouched throughout. +**The two personal feeds.** `bookmarks list` and `drafts list` are not +`EverythingService` methods, and they belong to no project-scoped group — they +are `/my/` listings, private to the authenticated user, and there is no +`--all-projects` to pass because there is no project scope to leave. + +They are recorded here anyway, because the invariants are this document's. Both +`Bookmarks().List` and `Drafts().List` take a `page int32` where **0 means the +SDK follows the Link header across every page** — the same spelling, and so the +same trap. A default of "fetch page 0, then trim to `--limit`" would reintroduce +fetch-everything-then-truncate through a door the aggregates no longer have. + +So both reuse `accountWideCollect` unchanged, and follow the I5 flag table +exactly: bounded walk to 100 by default, `--limit N` walks to N, `--page N` is +exactly one request, and `--all` is the only path that reaches page 0. Leaving +them undocumented is how the contract erodes — the next `/my/` feed would have +no precedent to copy. + +`checkins reminders` is a third `/my/` feed but sits outside this table: its +options struct does not honor a page number at all, so it takes `--limit` (a +real SDK-side bound) and deliberately registers no `--page`. Under I3 a flag +that cannot act on its value is worse than an absent one. + **The two overdue rows.** Both endpoints are unpaginated, so `--page` has nothing to address and stays an error. `--all` is a different question: it means "skip the cap", and since the complete array is already in hand it costs no diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 3a235345..d4c1c105 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -6,24 +6,53 @@ Coverage of Basecamp 3 API endpoints. Source: [bc3-api/sections](https://github. | Status | Sections | Endpoints | |--------|----------|-----------| -| ✅ Implemented | 45 | 167 | +| ✅ Implemented | 50 | 184 | | ⏭️ Out of scope | 4 | 12 | -| **Total tracked** | **49** | **179** | +| **Total tracked** | **54** | **196** | -**100% coverage of tracked in-scope API** (167/167 endpoints). This is not a -complete bc-api parity figure. The other five BC5 sections introduced by -bc-api#410 remain untracked and outside this coverage matrix. The pinned SDK's -`EverythingService` is now fully reached — see [Account-wide +**100% coverage of tracked in-scope API** (184/184 endpoints). This is not a +complete bc-api parity figure. The five BC5 sections introduced by bc-api#410 +that were previously untracked — `my_bookmarks`, `drafts`, `my_notes`, +`calendars`, and `question_reminders` — are now tracked and implemented. The +pinned SDK's `EverythingService` is fully reached — see [Account-wide aggregates](#account-wide-aggregates). +Two corrections rode along with that count. The `questions` row claimed 5 +endpoints while listing four actions, and the section carries pause, resume, +notification settings, and answerers besides — so the row was undercounting the +very section the 100%-of-tracked claim rests on. It now reads 8. And +`card_table_columns` gained `subscribe`/`unsubscribe` operations in the SDK that +the CLI deliberately does not spell twice; see that row. + Out-of-scope sections are excluded from parity totals and scripts: chatbots (different auth), legacy Clientside (deprecated) > Note: the per-row `Endpoints` column in the Coverage by Section table sums higher than the Summary totals above. The discrepancy predates the BC5 baseline; the row count (48 sections) is authoritative for the `Since` column. Reconciling endpoint counts is pre-existing maintenance, tracked separately. -**SDK version:** v0.11.0 — carries `EverythingService` -(`AccountClient.Everything()`, basecamp/basecamp-sdk#435 and #438), a -16-method account-wide aggregate family covering cross-project messages, -comments, checkins, forwards, files, and the +**SDK version:** v0.12.0 — adds 20 exported Go methods over 13 new backend +operations. The extra seven wrap endpoints that already existed but were +reachable only through the raw generated client, which the andon-cord rule +forbids the CLI from calling. + +Those methods land as four new command groups (`bookmarks`, `drafts`, `notes`, +`calendars`) and three extensions (`assignments` gains the Up Next verbs, +`todos create` gains `--loose`, `checkins` gains question pause/resume/notify/ +answerers plus an account-wide `reminders` feed). + +v0.12.0 also gave 11 `EverythingService` methods a trailing +`*EverythingTaskFilters` parameter — the nine paginated todo and card selectors +plus the two unpaginated overdue endpoints. The family is 5 unchanged + 11 +changed = 16. + +One v0.12.0 defect shapes a command rather than just a call: `parseErrorBody` +reads only `error`/`error_description`, so a calendar 422 carrying +`{"errors":{"color":[…]}}` arrives as a bare `validation error` naming neither +field nor value. `calendars update` therefore validates its eleven colors +client-side. The SDK fixes this past this pin (#541 returns a `fieldErrors` +map), so a later bump could surface the server's own message. + +It carries `EverythingService` (`AccountClient.Everything()`, +basecamp/basecamp-sdk#435 and #438), a 16-method account-wide aggregate family +covering cross-project messages, comments, checkins, forwards, files, and the open/completed/unassigned/overdue/no-due-date todo and card rollups. **All 16 are reached from the CLI** — see [Account-wide aggregates](#account-wide-aggregates). @@ -124,7 +153,7 @@ The **Since** column tags each row with the Basecamp version that introduced its |---------|-----------|-------------|--------|-------|----------|-------| | **Core** | | projects | 9 | `projects` | ✅ | BC4 | - | list, show, create, update, delete | -| todos | 11 | `todos`, `todo`, `done`, `reopen` | ✅ | BC4 | - | list, show, create, update, complete, uncomplete, position (BC5: `steps` shown on `todos show`; edit via `cards step`) | +| todos | 12 | `todos`, `todo`, `done`, `reopen` | ✅ | BC4 | - | list, show, create, update, complete, uncomplete, position (BC5: `steps` shown on `todos show`; edit via `cards step`). `todos create --loose` creates on the to-do set, outside any list | | todolists | 9 | `todolists` | ✅ | BC4 | - | list, show, create, update, position | | todosets | 3 | `todosets` | ✅ | BC4 | - | Container for todolists, accessed via project dock (BC5: `todos_count`, `completed_loose_todos_count`, `todos_url`, `app_todos_url`) | | todolist_groups | 8 | `todolistgroups` | ✅ | BC4 | - | list, show, create, update, position | @@ -144,13 +173,17 @@ The **Since** column tags each row with the Basecamp version that introduced its | **Cards (Kanban)** | | card_tables | 3 | `cards` | ✅ | BC4 | - | Accessed via project dock | | card_table_cards | 9 | `cards` | ✅ | BC4 | - | list, show, create, update, move | -| card_table_columns | 11 | `cards columns` | ✅ | BC4 | - | list columns | +| card_table_columns | 11 | `cards columns` | ✅ | BC4 | - | list columns. SDK v0.12.0 added `Subscribe`/`Unsubscribe`; `cards column watch\|unwatch` already performs the same action through the generic recording-subscription endpoint and returns the resulting subscription details the specific endpoint does not, so the CLI keeps one spelling | | card_table_steps | 4 | `cards steps` | ✅ | BC4 | - | Workflow steps on cards | | card_table_wormholes | 3 | `cards wormholes` | ✅ | BC5 | - | list (via `wormholes[]` on card table), create, update, delete; `cards move --to-wormhole` teleports a card across projects (async, new id) | +| **Personal (My)** | +| my_bookmarks | 4 | `bookmarks` | ✅ | BC5 | - | list, check, add, remove. Private to the authenticated user; `add`/`remove` are idempotent, and `check` returns a bool reported in the payload rather than through the exit code. Bounded like the account-wide listings | +| drafts | 1 | `drafts` | ✅ | BC5 | - | list unpublished drafts across projects (server caps at 250). Bounded like the account-wide listings; publishing happens through the command for the draft's type | +| my_notes | 2 | `notes` | ✅ | BC5 | - | show, set. A singleton per person, so no id and no listing. Pre-first-write the record does not exist yet and renders as empty rather than 404. `set` writes Markdown as HTML; attachments are out of scope | | **People** | | people | 12 | `people`, `me` | ✅ | BC4 | - | list, show, pingable, add, remove (BC5: `tagline` alias of `bio` on person output) | | **Search & Recordings** | -| my_assignments | 3 | `assignments` | ✅ | BC4 | - | list (priorities/non-priorities), completed, due (with scope filter) | +| my_assignments | 6 | `assignments` | ✅ | BC4 | - | list (priorities/non-priorities), completed, due (with scope filter), prioritize, deprioritize, reorder. `list` surfaces `priority_recording_id`, which is the only way to address a prioritized card-table step — it appears in no URL | | search | 2 | `search` | ✅ | BC4 | - | Full-text search + metadata. Filters: `--project`/`--in`, `--type`, `--creator`, `--since` (BC5-only), `--file-type`, `--exclude-chat`. Metadata lists recording/file search types | | recordings | 4 | `recordings` | ✅ | BC4 | - | Browse by type/status, trash/archive/restore | | **Files & Documents** | @@ -159,6 +192,7 @@ The **Since** column tags each row with the Basecamp version that introduced its | documents | 8 | `files`, `docs` | ✅ | BC4 | - | list, show, create, update. Create supports `--subscribe`/`--no-subscribe`, `--visible-to-clients` (root vault only) | | attachments | 1 | `uploads`, `attachments` | ✅ | BC4 | - | Upload via `attach`; list embedded attachments via `attachments list` (parses `<bc-attachment>` from content) | | **Schedule** | +| calendars | 2 | `calendars` | ✅ | BC5 | - | show, update (color only). No index endpoint, so there is no `calendars list` — address one by id or pasted URL. The eleven colors are validated client-side, because the SDK at this pin cannot carry the server's 422 field message | | schedules | 2 | `schedule` | ✅ | BC4 | - | Schedule container + settings | | schedule_entries | 5 | `schedule` | ✅ | BC4 | - | list, show, create, update, occurrences. Create supports `--subscribe`/`--no-subscribe` | | events | 1 | `events` | ✅ | BC4 | - | Recording change audit trail | @@ -172,8 +206,9 @@ The **Since** column tags each row with the Basecamp version that introduced its | subscriptions | 4 | `subscriptions` | ✅ | BC4 | - | show, subscribe, unsubscribe, add/remove | | **Check-ins (Automatic)** | | questionnaires | 2 | `checkins` | ✅ | BC4 | - | Container for check-in questions | -| questions | 5 | `checkins` | ✅ | BC4 | - | list, show, create, update | +| questions | 8 | `checkins` | ✅ | BC4 | - | list, show, create, update, pause, resume, notification settings, answerers (`checkins question notify` is tri-state per setting; `answerers` takes no `--page`, since the SDK does not honor one) | | question_answers | 4 | `checkins` | ✅ | BC4 | - | list, show | +| question_reminders | 1 | `checkins reminders` | ✅ | BC5 | - | Account-wide pending-reminder feed (`GET /my/question_reminders.json`). `--limit` is a real SDK-side bound; no `--page`, since the options struct does not honor a page number | | **Inbox (Email Forwards)** | | inboxes | 1 | `forwards` | ✅ | BC4 | - | Inbox container | | forwards | 2 | `forwards` | ✅ | BC4 | - | list, show | diff --git a/internal/commands/bookmarks.go b/internal/commands/bookmarks.go index e30db1ea..c5f4e5e4 100644 --- a/internal/commands/bookmarks.go +++ b/internal/commands/bookmarks.go @@ -269,7 +269,7 @@ func newBookmarksCheckCmd() *cobra.Command { Short: "Report whether you have bookmarked a recording", Long: `Report whether you have bookmarked a recording. -Reports the answer rather than signalling it through the exit code: both +Reports the answer rather than signaling it through the exit code: both outcomes exit 0, and "not bookmarked" is a successful answer. Exit codes here mean a request failed, so reserving a nonzero code for "false" would be indistinguishable from a real error. diff --git a/internal/commands/bookmarks_test.go b/internal/commands/bookmarks_test.go index 3fd6dca1..4b968019 100644 --- a/internal/commands/bookmarks_test.go +++ b/internal/commands/bookmarks_test.go @@ -189,7 +189,7 @@ func TestFlattenBookmarksCarriesTheRecording(t *testing.T) { } // check answers a question. Both answers are successes, so neither may be -// signalled through the exit code — that space belongs to real failures. +// signaled through the exit code — that space belongs to real failures. func TestBookmarksCheckReportsBothAnswersAsSuccess(t *testing.T) { for _, tc := range []struct { name string diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 724df89b..66a8b391 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -3,7 +3,8 @@ name: basecamp description: | Interact with Basecamp via the Basecamp CLI. Full API coverage: projects, todos, cards, messages, files, schedule, check-ins, timeline, recordings, templates, webhooks, - subscriptions, lineup, chat, pings, gauges, assignments, notifications, and accounts. + subscriptions, lineup, chat, pings, gauges, assignments, notifications, bookmarks, + drafts, notes, calendars, and accounts. Use for ANY Basecamp question or action. triggers: # Direct invocations @@ -18,6 +19,10 @@ triggers: - basecamp messages - basecamp file - basecamp document + - basecamp bookmarks + - basecamp drafts + - basecamp notes + - basecamp calendars - basecamp schedule - basecamp checkin - basecamp check-in @@ -176,6 +181,16 @@ basecamp <cmd> --page 1 # First page only, no auto-pagination | Overdue todos (in project) | `basecamp todos list --overdue --in <project> --json` | | Overdue todos (cross-project) | `basecamp todos list --all-projects --overdue --json` (flat, oldest first) or `basecamp reports overdue --json` (bucketed by lateness) | | All cards (cross-project) | `basecamp cards list --all-projects --json` (grouped by project) | +| My bookmarks | `basecamp bookmarks list --json` | +| Bookmark something | `basecamp bookmarks add <id-or-url> --json` | +| Is it bookmarked? | `basecamp bookmarks check <id-or-url> --json` (always exits 0) | +| My unpublished drafts | `basecamp drafts list --json` | +| Read my personal note | `basecamp notes show --json` | +| Replace my personal note | `basecamp notes set "<content>" --json` | +| Check-ins I owe answers to | `basecamp checkins reminders --json` | +| Add to Up Next | `basecamp assignments prioritize <id> --json` | +| Recolor a calendar | `basecamp calendars update <id-or-url> --color blue --json` | +| Todo outside any list | `basecamp todos create "<content>" --loose --in <project> --json` | | Assign todo | `basecamp assign <id> [id...] --to <person> --in <project> --json` | | Assign card | `basecamp assign <id> [id...] --card --to <person> --in <project> --json` | | Assign card step | `basecamp assign <id> [id...] --step --to <person> --in <project> --json` | @@ -797,6 +812,33 @@ basecamp checkins answer update <id> "Updated" --in <project> **Client visibility:** `checkins question create` accepts `--visible-to-clients` to make the question visible to clients (omit for the server default; see the note under Messages for the context-dependent rule). +**Managing a question:** + +```bash +basecamp checkins question pause <id> --json # Stop asking it +basecamp checkins question resume <id> --json # Start asking it again +basecamp checkins question answerers <id> --json # Who answers it +basecamp checkins question notify <id> --on-answer --json +basecamp checkins question notify <id> --no-on-answer --json +basecamp checkins question notify <id> --digest-include-unanswered --json +``` + +`notify` changes **your own** settings, and each one is left alone unless you +name it — so `--on-answer` does not silently reset the digest setting. The +`--no-...` spellings send an explicit false; passing neither setting is refused +rather than sent as an empty update. + +**Your pending reminders** (account-wide, no `--in`): + +```bash +basecamp checkins reminders --json +basecamp checkins reminders --limit 10 --json +``` + +`reminders` and `answerers` take `--limit` but deliberately **no `--page`**: the +API does not honor a page number on these, so the flag would accept a value it +could not act on. + ### Timeline ```bash @@ -921,6 +963,67 @@ basecamp assignments due due_later_this_week --json # Due later this week **Scopes:** overdue, due_today, due_tomorrow, due_later_this_week, due_next_week, due_later. +**Up Next** — reorder the priority list: + +```bash +basecamp assignments prioritize <id> --json # Add to Up Next +basecamp assignments deprioritize <id> --json # Remove from Up Next +basecamp assignments reorder <id> --position 1 --json +``` + +**Which id to pass — three cases, not two.** A to-do or a card is addressed by +the entry's own `id`. A step that is *not yet* prioritized is addressed by the +step's own `id`, found in the parent card's `children`. But once a step *is* +prioritized, the listing shows it under its parent card, so the entry's top-level +`id` belongs to the **card**, and only `priority_recording_id` addresses the +step. + +`basecamp assignments list` is the only place `priority_recording_id` appears — +it is in no URL. Read it from there rather than guessing: `deprioritize` targets +one exact recording and the server answers 204 either way, so a wrong id reports +success while changing nothing. If two steps on one card are prioritized, the +listing shows the card once with a single `priority_recording_id` and the +siblings are not separately addressable. + +### Personal (bookmarks, drafts, notes) + +Private to you, spanning every project — no `--in <project>`. + +```bash +basecamp bookmarks list --json +basecamp bookmarks add <id-or-url> --json +basecamp bookmarks remove <id-or-url> --json +basecamp bookmarks check <id-or-url> --json +basecamp drafts list --json +basecamp notes show --json +basecamp notes set "<content>" --json +``` + +`bookmarks add` and `remove` are idempotent — re-adding returns the existing +bookmark, removing an absent one still succeeds. `check` reports +`{"bookmarked": true|false}` and **always exits 0**: both answers are successes, +so a nonzero exit here means the request failed, not that the answer was false. + +`bookmarks list` and `drafts list` are bounded like the account-wide listings: +default 100, `--limit N`, `--page N`, `--all` for every page. Drafts are capped +at 250 server-side. + +`notes` is a single private scratchpad — one per person, no id, nothing to list. +Before your first write it renders empty rather than 404ing. `set` **replaces** +the whole note (it does not append) and takes content from an argument, +`--file`, or piped stdin; Markdown is converted to HTML. + +### Calendars + +```bash +basecamp calendars show <id-or-url> --json +basecamp calendars update <id-or-url> --color blue --json +``` + +**There is no `calendars list`** — the API has no index endpoint, so address a +calendar by id or by pasting its URL. Colors: white, red, orange, yellow, green, +blue, aqua, purple, gray, pink, brown. + ### Notifications ```bash From 4577db37287891cdd89040b508519490b7e408eb Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Mon, 3 Aug 2026 17:59:58 -0700 Subject: [PATCH 7/8] Close three gaps review found in the v0.12.0 surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `notes set` could silently discard piped content. notesContent checked stdin only after an argument and --file had been ruled out, so `generate | basecamp notes set --file fallback.md` overwrote the note from the file and threw the generated body away. In the one command that replaces everything, that is the exact failure the function's own doc comment claims it prevents. All three sources are now detected before any is chosen; naming two is a usage error. An empty pipe stays a non-source, so a redirected-but-empty stdin does not break a valid --file call. `checkins reminders` silently ignored explicit scope. --project, --in and --questionnaire are persistent flags on the checkins parent, so cobra accepts them on a subcommand whose handler always calls the account-wide endpoint. The caller believed the request was scoped and got every project's reminders back with nothing saying otherwise. All three are now rejected before the request. Correct the 400 exit-code claim in API-COVERAGE.md. It said a 400's exit code moves from 7 to 9. It does not. `internal/output` defines no `validation` mapping and clioutput defaults an unrecognised code to ExitAPI, so a 400 still exits 7 — only its JSON `code` changed `api_error` to `validation`. Exit 9 is not reachable from the CLI. --- API-COVERAGE.md | 7 ++- internal/commands/checkins.go | 14 ++++++ .../commands/checkins_question_admin_test.go | 29 +++++++++++ internal/commands/notes.go | 48 +++++++++++------- internal/commands/notes_test.go | 50 +++++++++++++++++++ 5 files changed, 129 insertions(+), 19 deletions(-) diff --git a/API-COVERAGE.md b/API-COVERAGE.md index d4c1c105..20307708 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -86,8 +86,11 @@ Model and transport changes riding along: and `TodolistGroup` the tag carries no `omitempty`, so the key is always present in machine output. - HTTP 400 now maps to the `validation` error code rather than `api_error` (#482). - Since `convertSDKError` passes the SDK code straight through, **a 400's exit - code moves from 7 to 9**; 422 was already validation. + `convertSDKError` passes the SDK code straight through, so a 400's JSON `code` + changes `api_error` → `validation`. **Its exit code does not move: a 400 still + exits 7.** `internal/output` defines no `validation` mapping, and `clioutput` + defaults an unrecognised code to `ExitAPI` — so the new code lands on the same + exit status the old one did. Exit 9 is not reachable from the CLI at all. - Retry behavior: per-operation `retry.max` is honored as a ceiling (#483), `*WithBody` request bodies replay across retries (#481), and the declared `retry_on` status set is honored (#486). diff --git a/internal/commands/checkins.go b/internal/commands/checkins.go index 885d2748..142ce6c0 100644 --- a/internal/commands/checkins.go +++ b/internal/commands/checkins.go @@ -64,6 +64,20 @@ This is your own reminder feed across every project, so it takes no RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) + // --project/--in/--questionnaire are persistent flags on the parent, + // so cobra accepts them here even though this feed is account-wide. + // Accepting and ignoring them is worse than rejecting them: the + // caller believes they scoped the request and gets every project's + // reminders back, with nothing in the output saying otherwise. + for _, scoped := range []string{"project", "in", "questionnaire"} { + if cmd.Flags().Changed(scoped) { + return output.ErrUsageHint( + fmt.Sprintf("checkins reminders does not take --%s", scoped), + "This is your own reminder feed across every project. Drop the flag.", + ) + } + } + if limit < 0 { return output.ErrUsage("--limit must be zero or positive") } diff --git a/internal/commands/checkins_question_admin_test.go b/internal/commands/checkins_question_admin_test.go index 46c034cc..4755715a 100644 --- a/internal/commands/checkins_question_admin_test.go +++ b/internal/commands/checkins_question_admin_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/http" + "strings" "testing" "time" @@ -169,6 +170,34 @@ func TestCheckinsRemindersLists(t *testing.T) { assert.Equal(t, "1 pending check-in reminders", envelope.Summary) } +// --project, --in and --questionnaire are persistent flags on the checkins +// parent, so cobra accepts them on `reminders` even though the feed is +// account-wide. Silently ignoring them is the worst outcome: the caller +// believes the request was scoped and gets every project's reminders back, with +// nothing in the output to say otherwise. +func TestCheckinsRemindersRejectsScopeFlags(t *testing.T) { + for _, args := range [][]string{ + {"reminders", "--project", "977190"}, + {"reminders", "--in", "977190"}, + {"reminders", "--questionnaire", "123"}, + } { + t.Run(strings.Join(args, " "), func(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, stubRoute{ + method: http.MethodGet, + path: checkinsRemindersPath, + status: http.StatusOK, + body: `[]`, + }) + + err := executeRecordingCommand(NewCheckinsCmd(), app, args...) + + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded(), + "an ignored scope flag must not reach the server as an account-wide request") + }) + } +} + // The reminder feed nests the question it is about, and the renderer skips // nested objects — so a generic render would say when something is due without // saying what, or where. diff --git a/internal/commands/notes.go b/internal/commands/notes.go index 99e9704d..0813c0aa 100644 --- a/internal/commands/notes.go +++ b/internal/commands/notes.go @@ -163,36 +163,50 @@ Attachments are out of scope: this writes the note body only.`, // Naming two sources is a usage error rather than a silent precedence rule: a // caller who passes both an argument and --file has a wrong expectation about // which one wins, and this command overwrites the whole note. +// +// All three sources are detected before any of them is chosen. Checking stdin +// only after an argument and --file had been ruled out made +// `generate | basecamp notes set --file fallback.md` overwrite the note from +// the file and discard the generated body without a word — the precise failure +// this function exists to prevent, in the one command that replaces everything. func notesContent(cmd *cobra.Command, args []string, file string) (string, error) { positional := strings.Join(args, " ") - if file != "" && positional != "" { - return "", output.ErrUsage("pass note content as an argument or --file, not both") + piped, ok, err := readPipedStdin(cmd) + if err != nil { + return "", err + } + // An empty pipe is not a source. A redirected-but-empty stdin carries no + // body to lose, so it must not turn a valid `--file` call into an error. + hasPipe := ok && strings.TrimSpace(piped) != "" + + named := 0 + for _, present := range []bool{file != "", positional != "", hasPipe} { + if present { + named++ + } + } + if named > 1 { + return "", output.ErrUsage("pass note content as an argument, with --file, or on stdin — not more than one") } - if file != "" { + switch { + case file != "": data, err := os.ReadFile(file) if err != nil { return "", output.ErrUsage(fmt.Sprintf("failed to read %s: %v", file, err)) } return notesRequireContent(string(data)) - } - - if positional != "" { + case positional != "": return notesRequireContent(positional) + case hasPipe: + return notesRequireContent(piped) } - piped, ok, err := readPipedStdin(cmd) - if err != nil { - return "", err - } - if !ok { - return "", output.ErrUsageHint( - "note content is required", - `Pass it as an argument, with --file, or on stdin: basecamp notes set "..."`, - ) - } - return notesRequireContent(piped) + return "", output.ErrUsageHint( + "note content is required", + `Pass it as an argument, with --file, or on stdin: basecamp notes set "..."`, + ) } // notesRequireContent refuses to blank the note by accident. diff --git a/internal/commands/notes_test.go b/internal/commands/notes_test.go index e4afb711..e89b1293 100644 --- a/internal/commands/notes_test.go +++ b/internal/commands/notes_test.go @@ -168,3 +168,53 @@ func TestNotesSetRejectsAmbiguousOrEmptyInput(t *testing.T) { }) } } + +// A piped body must never lose to a flag. `generate | basecamp notes set --file +// fallback.md` used to overwrite the note from the file and throw the generated +// body away, because stdin was only consulted after --file had been ruled out. +func TestNotesSetRejectsPipedContentAlongsideAnotherSource(t *testing.T) { + populated := filepath.Join(t.TempDir(), "note.md") + require.NoError(t, os.WriteFile(populated, []byte("from the file"), 0o600)) + + for _, tc := range []struct { + name string + args []string + }{ + {"pipe and --file together", []string{"set", "--file", populated}}, + {"pipe and an argument together", []string{"set", "inline"}}, + } { + t.Run(tc.name, func(t *testing.T) { + app, transport, _ := setupPersonalFeedApp(t, notesUpdateRoute()) + + cmd := NewNotesCmd() + cmd.SetIn(strings.NewReader("piped note body")) + + err := executeRecordingCommand(cmd, app, tc.args...) + + requireBookmarksUsageError(t, err) + assert.Empty(t, transport.recorded(), "a rejected write must not reach the server") + }) + } +} + +// An empty pipe carries no body to lose, so it must not turn an otherwise valid +// --file call into an ambiguity error. +func TestNotesSetIgnoresAnEmptyPipeAlongsideAFile(t *testing.T) { + populated := filepath.Join(t.TempDir(), "note.md") + require.NoError(t, os.WriteFile(populated, []byte("from the file"), 0o600)) + + app, transport, _ := setupPersonalFeedApp(t, notesUpdateRoute()) + + cmd := NewNotesCmd() + cmd.SetIn(strings.NewReader(" \n")) + + require.NoError(t, executeRecordingCommand(cmd, app, "set", "--file", populated)) + + var body struct { + Note struct { + Content string `json:"content"` + } `json:"note"` + } + require.NoError(t, json.Unmarshal([]byte(transport.last(t).Body), &body)) + assert.Contains(t, body.Note.Content, "from the file") +} From ad39cd8c7a4475b81a1aead8b2841d9b7fb0172a Mon Sep 17 00:00:00 2001 From: Jeremy Daer <jeremy@37signals.com> Date: Mon, 3 Aug 2026 19:18:33 -0700 Subject: [PATCH 8/8] Make the enumerable id the one the verb takes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --ids-only prints a row's `id` field and nothing else, so `id` decides what a piped pipeline actually addresses. Two of the new listings got it wrong in opposite directions. assignments list exposed the parent card as `id` while the prioritized step lived only in priority_recording_id. So `assignments list --ids-only | xargs -n1 basecamp assignments deprioritize` addressed the card, and deprioritize answers 204 whether or not anything matched — the pipeline reported success and changed nothing. The prioritized entry now enumerates as the value the Up Next verbs take, with the parent card kept as recording_id. checkins reminders had the inverse problem: it exposed question_id but no `id` at all, so `checkins reminders --ids-only` printed nothing. The question id is the actionable one — answering takes `checkins answer <question-id>` — so it becomes `id`, and the reminder's own id is kept as reminder_id. Both are covered by tests that render through the ids-only writer rather than only asserting on the row map, since the row map was never the thing that was broken. Also document the note-clearing gap rather than leaving it implicit: `notes set` refuses empty content in every form, which means the CLI cannot clear the note at all. An explicit --clear would close that without weakening the guard, and is deliberately deferred to its own change — a destructive verb should not ride along on a bump. --- internal/commands/assignments.go | 33 +++++++++----- .../commands/assignments_priority_test.go | 43 ++++++++++++++++++- internal/commands/checkins.go | 10 +++++ .../commands/checkins_question_admin_test.go | 31 +++++++++++++ internal/commands/notes.go | 12 +++++- 5 files changed, 116 insertions(+), 13 deletions(-) diff --git a/internal/commands/assignments.go b/internal/commands/assignments.go index 80135846..22567219 100644 --- a/internal/commands/assignments.go +++ b/internal/commands/assignments.go @@ -53,12 +53,14 @@ const upNextIDGuidance = `Which id to pass: A to-do, or a card itself the entry's own id A step not yet prioritized the step's id, from the parent card's children - A step already prioritized the entry's priority_recording_id + A step already prioritized the entry's id, which is its priority_recording_id -That last case is the one that bites: once a step is prioritized the listing -shows it under its parent card, so the entry's id belongs to the card and only -priority_recording_id addresses the step. 'basecamp assignments list' is the -only place that value appears — it is in no URL you can paste. +Once a step is prioritized the listing shows it under its parent card. The +entry's 'id' is the value that addresses the step — the same one reported as +'priority_recording_id' — while the parent card stays available as +'recording_id'. That makes 'assignments list --ids-only' safe to pipe into these +verbs. 'basecamp assignments list' is the only place the value appears; it is in +no URL you can paste. If two steps on one card are prioritized, the listing shows the card once with a single priority_recording_id, and the siblings are not separately @@ -273,17 +275,26 @@ func flattenAssignments(result *basecamp.MyAssignmentsResult) []map[string]any { func appendAssignmentRows(rows []map[string]any, items []basecamp.MyAssignment, priority bool) []map[string]any { for _, item := range items { row := map[string]any{ - "id": item.ID, - "content": item.Content, - "type": item.Type, - "project": item.Bucket.Name, - "due_on": item.DueOn, - "up_next": priority, + "id": item.ID, + "recording_id": item.ID, + "content": item.Content, + "type": item.Type, + "project": item.Bucket.Name, + "due_on": item.DueOn, + "up_next": priority, } // Present only once the step or card has been prioritized, and the one // id that addresses it thereafter. + // + // It also becomes the row's `id`, because `id` is the enumerable one: + // --ids-only prints that field and nothing else, so leaving the parent + // card there made `assignments list --ids-only | xargs ... deprioritize` + // address the card instead of the prioritized step. Deprioritize answers + // 204 whether or not anything matched, so that pipeline reported success + // and changed nothing. The parent stays reachable as recording_id. if item.PriorityRecordingID != nil { row["priority_recording_id"] = *item.PriorityRecordingID + row["id"] = *item.PriorityRecordingID } rows = append(rows, row) } diff --git a/internal/commands/assignments_priority_test.go b/internal/commands/assignments_priority_test.go index 18acbd49..5bd393df 100644 --- a/internal/commands/assignments_priority_test.go +++ b/internal/commands/assignments_priority_test.go @@ -1,14 +1,18 @@ package commands import ( + "bytes" "fmt" "net/http" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/output" ) const ( @@ -123,12 +127,49 @@ func TestFlattenAssignmentsSurfacesPriorityRecordingID(t *testing.T) { require.Len(t, rows, 2) - assert.Equal(t, int64(777), rows[0]["id"], "the entry id is the card's") + assert.Equal(t, priorityID, rows[0]["id"], + "id must be the value the Up Next verbs take, not the parent card's") + assert.Equal(t, int64(777), rows[0]["recording_id"], "the parent card stays reachable") assert.Equal(t, priorityID, rows[0]["priority_recording_id"], "the step is addressed by this instead") assert.Equal(t, true, rows[0]["up_next"]) assert.Equal(t, "JD test proj", rows[0]["project"]) assert.NotContains(t, rows[1], "priority_recording_id", "an unprioritized entry has no priority_recording_id yet") + assert.Equal(t, int64(888), rows[1]["id"], "without one, id is the entry's own") + assert.Equal(t, int64(888), rows[1]["recording_id"]) assert.Equal(t, false, rows[1]["up_next"]) } + +// --ids-only prints the `id` field and nothing else, so it is the field that +// decides what `assignments list --ids-only | xargs -n1 basecamp assignments +// deprioritize` actually targets. With the parent card there, that pipeline +// addressed the card rather than the prioritized step — and deprioritize answers +// 204 whether or not anything matched, so it reported success and changed +// nothing. Assert on the rendered output, not just the row map. +func TestAssignmentsListIDsOnlyEmitsTheActionableID(t *testing.T) { + priorityID := int64(9001) + rows := flattenAssignments(&basecamp.MyAssignmentsResult{ + Priorities: []basecamp.MyAssignment{{ + ID: 777, + Content: "Card with a prioritized step", + Type: "Kanban::Card", + Bucket: basecamp.MyAssignmentBucket{ID: 977190, Name: "JD test proj"}, + PriorityRecordingID: &priorityID, + }}, + NonPriorities: []basecamp.MyAssignment{{ + ID: 888, + Content: "Not in Up Next", + Type: "Todo", + Bucket: basecamp.MyAssignmentBucket{ID: 977190, Name: "JD test proj"}, + }}, + }) + + var buf bytes.Buffer + writer := output.New(output.Options{Writer: &buf, Format: output.FormatIDs}) + require.NoError(t, writer.OK(rows)) + + ids := strings.Fields(buf.String()) + assert.Equal(t, []string{"9001", "888"}, ids, + "the prioritized row must enumerate as the step, not the parent card 777") +} diff --git a/internal/commands/checkins.go b/internal/commands/checkins.go index 142ce6c0..ee4a327f 100644 --- a/internal/commands/checkins.go +++ b/internal/commands/checkins.go @@ -115,14 +115,24 @@ This is your own reminder feed across every project, so it takes no // A QuestionReminder nests the question it is about, and the renderer skips // nested objects — so a generic render would show a timestamp and nothing that // says which question is due, or where. +// +// The question's id is the row's `id`, not just `question_id`: `id` is the +// enumerable field, so a row without one makes `checkins reminders --ids-only` +// print nothing at all. Answering a reminder takes the question id +// (`basecamp checkins answer <question-id>`), so that is the actionable value. +// The reminder's own id is kept as `reminder_id` for anything that needs it. func flattenQuestionReminders(reminders []basecamp.QuestionReminder) []map[string]any { rows := make([]map[string]any, 0, len(reminders)) for _, r := range reminders { row := map[string]any{ + "id": r.Question.ID, "question_id": r.Question.ID, "question": r.Question.Title, "remind_at": r.RemindAt, } + if r.ReminderID != nil { + row["reminder_id"] = *r.ReminderID + } if r.Question.Bucket != nil { row["project"] = r.Question.Bucket.Name } diff --git a/internal/commands/checkins_question_admin_test.go b/internal/commands/checkins_question_admin_test.go index 4755715a..d46215fc 100644 --- a/internal/commands/checkins_question_admin_test.go +++ b/internal/commands/checkins_question_admin_test.go @@ -1,6 +1,7 @@ package commands import ( + "bytes" "encoding/json" "fmt" "net/http" @@ -12,6 +13,8 @@ import ( "github.com/stretchr/testify/require" "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/output" ) const checkinsRemindersPath = "/99999/my/question_reminders.json" @@ -198,6 +201,34 @@ func TestCheckinsRemindersRejectsScopeFlags(t *testing.T) { } } +// --ids-only prints the `id` field and nothing else, so a row that carries only +// question_id enumerates as nothing at all — the command silently produces empty +// output. The question id is the actionable one, since answering takes +// `basecamp checkins answer <question-id>`. +func TestCheckinsRemindersIDsOnlyEmitsTheQuestionID(t *testing.T) { + reminderID := int64(5) + rows := flattenQuestionReminders([]basecamp.QuestionReminder{{ + RemindAt: time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC), + ReminderID: &reminderID, + Question: basecamp.Question{ + ID: 789, + Title: "What did you work on?", + }, + }}) + + require.Len(t, rows, 1) + assert.Equal(t, int64(789), rows[0]["id"], "the question id is the actionable one") + assert.Equal(t, int64(789), rows[0]["question_id"]) + assert.Equal(t, reminderID, rows[0]["reminder_id"], "the reminder's own id stays available") + + var buf bytes.Buffer + writer := output.New(output.Options{Writer: &buf, Format: output.FormatIDs}) + require.NoError(t, writer.OK(rows)) + + assert.Equal(t, []string{"789"}, strings.Fields(buf.String()), + "--ids-only must enumerate the reminder feed, not print nothing") +} + // The reminder feed nests the question it is about, and the renderer skips // nested objects — so a generic render would say when something is due without // saying what, or where. diff --git a/internal/commands/notes.go b/internal/commands/notes.go index 0813c0aa..f43d41f3 100644 --- a/internal/commands/notes.go +++ b/internal/commands/notes.go @@ -119,7 +119,17 @@ note, so there is no separate "create" step. basecamp notes set --file notes.md cat notes.md | basecamp notes set -Attachments are out of scope: this writes the note body only.`, +Attachments are out of scope: this writes the note body only. + +Empty content is refused, in every form — an empty argument, an empty file, an +empty pipe. 'set' replaces everything, so an empty write is indistinguishable +from a script whose input silently produced nothing, and the note it would erase +is not recoverable from here. + +The cost is that there is no way to clear the note from the CLI: do that on +Basecamp web. An explicit --clear would close that gap without weakening the +guard, and is deliberately left for its own change rather than folded in here — +a destructive verb deserves its own review, not a rider on a bump.`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context())