diff --git a/.surface b/.surface index d21d9d951..8092d5922 100644 --- a/.surface +++ b/.surface @@ -28,11 +28,15 @@ ARG basecamp boost create 01 ARG basecamp boost delete 00 ARG basecamp boost list 00 ARG basecamp boost show 00 +ARG basecamp boost update 00 +ARG basecamp boost update 01 ARG basecamp boosts create 00 ARG basecamp boosts create 01 ARG basecamp boosts delete 00 ARG basecamp boosts list 00 ARG basecamp boosts show 00 +ARG basecamp boosts update 00 +ARG basecamp boosts update 01 ARG basecamp bubble-up add 00 ARG basecamp bubble-up remove 00 ARG basecamp calendars show 00 @@ -577,11 +581,13 @@ CMD basecamp boost create CMD basecamp boost delete CMD basecamp boost list CMD basecamp boost show +CMD basecamp boost update CMD basecamp boosts CMD basecamp boosts create CMD basecamp boosts delete CMD basecamp boosts list CMD basecamp boosts show +CMD basecamp boosts update CMD basecamp bubble-up CMD basecamp bubble-up add CMD basecamp bubble-up remove @@ -2548,6 +2554,27 @@ FLAG basecamp boost show --stats type=bool FLAG basecamp boost show --styled type=bool FLAG basecamp boost show --todolist type=string FLAG basecamp boost show --verbose type=count +FLAG basecamp boost update --account type=string +FLAG basecamp boost update --agent type=bool +FLAG basecamp boost update --cache-dir type=string +FLAG basecamp boost update --count type=bool +FLAG basecamp boost update --help type=bool +FLAG basecamp boost update --hints type=bool +FLAG basecamp boost update --ids-only type=bool +FLAG basecamp boost update --in type=string +FLAG basecamp boost update --jq type=string +FLAG basecamp boost update --json type=bool +FLAG basecamp boost update --markdown type=bool +FLAG basecamp boost update --md type=bool +FLAG basecamp boost update --no-hints type=bool +FLAG basecamp boost update --no-stats type=bool +FLAG basecamp boost update --profile type=string +FLAG basecamp boost update --project type=string +FLAG basecamp boost update --quiet type=bool +FLAG basecamp boost update --stats type=bool +FLAG basecamp boost update --styled type=bool +FLAG basecamp boost update --todolist type=string +FLAG basecamp boost update --verbose type=count FLAG basecamp boosts --account type=string FLAG basecamp boosts --agent type=bool FLAG basecamp boosts --cache-dir type=string @@ -2655,6 +2682,27 @@ 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 boosts update --account type=string +FLAG basecamp boosts update --agent type=bool +FLAG basecamp boosts update --cache-dir type=string +FLAG basecamp boosts update --count type=bool +FLAG basecamp boosts update --help type=bool +FLAG basecamp boosts update --hints type=bool +FLAG basecamp boosts update --ids-only type=bool +FLAG basecamp boosts update --in type=string +FLAG basecamp boosts update --jq type=string +FLAG basecamp boosts update --json type=bool +FLAG basecamp boosts update --markdown type=bool +FLAG basecamp boosts update --md type=bool +FLAG basecamp boosts update --no-hints type=bool +FLAG basecamp boosts update --no-stats type=bool +FLAG basecamp boosts update --profile type=string +FLAG basecamp boosts update --project type=string +FLAG basecamp boosts update --quiet type=bool +FLAG basecamp boosts update --stats type=bool +FLAG basecamp boosts update --styled type=bool +FLAG basecamp boosts update --todolist type=string +FLAG basecamp boosts update --verbose type=count FLAG basecamp bubble-up --account type=string FLAG basecamp bubble-up --agent type=bool FLAG basecamp bubble-up --cache-dir type=string @@ -19265,11 +19313,13 @@ SUB basecamp boost create SUB basecamp boost delete SUB basecamp boost list SUB basecamp boost show +SUB basecamp boost update SUB basecamp boosts SUB basecamp boosts create SUB basecamp boosts delete SUB basecamp boosts list SUB basecamp boosts show +SUB basecamp boosts update SUB basecamp bubble-up SUB basecamp bubble-up add SUB basecamp bubble-up remove diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 77af1740e..4d50b0574 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -213,7 +213,7 @@ cannot faithfully cover at least one endpoint for a reason outside the CLI. A | campfires | 14 | `chat` | ✅ | BC4 | - | list, messages, post, line show/update/delete. @mentions in content | | pings | 3 | `pings` | ✅ | BC5 | - | list (`GET /circles.json`: active pings with at least one line, newest first, unpaginated), show (`GET /circles/:id.json`, archived included), start (`POST /circles.json`, which opens or reuses: an active direct ping is always reused, a group with the same people unless `--new-group`). A ping is a Circle bucket with one Campfire, so `messages`, `post` and `upload` run through the Campfire lines endpoints on the ping's `campfire_id`, with @mentions resolved among the ping's people. `` is an id, a ping URL, or a person (a direct ping: read from the list, or started when writing). `show` and `chat line` take ping URLs (`/circles/:id`, `/circles/:id@:line`). 403 `Pings have been disabled` and 422 (an agent starting a ping with nobody who can ping) surface as errors. Requires basecamp/bc3#13943 | | comments | 8 | `comment`, `comments` | ✅ | BC4 | - | list, show, thread, create, update. @mentions in content. `show` surfaces `reply_target` + paste-ready `mention` from its single Get (no new calls). `thread` composes Get + parent recording (via type endpoint) + List into a deterministic reply-ready context (no new endpoints) | -| boosts | 6 | `boost`, `react` | ✅ | BC4 | - | list (recording + event), show, create (recording + event), delete. No account-wide listing — BC5 withdrew `/boosts.json` (basecamp/bc3#12464); temporary, returns via basecamp/bc3#12463 | +| boosts | 7 | `boost`, `react` | ✅ | BC4 | - | list (recording + event), show, create (recording + event), update (`PUT /boosts/{id}`, the creator only; same 16-character cap as create), delete. No account-wide listing — BC5 withdrew `/boosts.json` (basecamp/bc3#12464); temporary, returns via basecamp/bc3#12463 | | notifications | 2 | `notifications` | ✅ | BC4 | - | list, mark as read (BC5: `bubble_ups`/`scheduled_bubble_ups` sections; `memories` is BC4-only) | | bubble_ups | 3 | `bubble-up`, `notifications bubbleups` | ✅ | BC5 | - | `bubble-up add`/`remove` create and delete a per-recording bubble-up (`POST`/`DELETE /recordings/{id}/bubble_up.json`); `add --at` schedules. Dedicated list is `notifications bubbleups` (`GET /my/readings/bubble_ups.json`, paginated) plus the `limit_bubble_ups` variant behind `notifications list --limit-bubble-ups`. Per-recording GET is an unrenderable API gap, so there is no `check`. | | **Cards (Kanban)** | diff --git a/e2e/smoke/smoke_communication_write.bats b/e2e/smoke/smoke_communication_write.bats index e01a63a38..e813102b0 100644 --- a/e2e/smoke/smoke_communication_write.bats +++ b/e2e/smoke/smoke_communication_write.bats @@ -46,6 +46,18 @@ setup_file() { assert_json_not_null '.data.id' } +@test "boost update changes a boost's content" { + local id_file="$BATS_FILE_TMPDIR/boost_id" + [[ -f "$id_file" ]] || mark_unverifiable "No boost created in prior test" + local boost_id + boost_id=$(<"$id_file") + + run_smoke basecamp boost update "$boost_id" "✅ done" --json + assert_success + assert_json_value '.ok' 'true' + assert_json_value '.data.content' '✅ done' +} + @test "boost delete removes a boost" { local id_file="$BATS_FILE_TMPDIR/boost_id" [[ -f "$id_file" ]] || mark_unverifiable "No boost created in prior test" diff --git a/internal/commands/boost.go b/internal/commands/boost.go index 410f8e83c..b2ff3733d 100644 --- a/internal/commands/boost.go +++ b/internal/commands/boost.go @@ -3,12 +3,15 @@ package commands import ( "fmt" "strconv" + "strings" "unicode/utf8" "github.com/spf13/cobra" "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/hostutil" "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/urlarg" ) // NewBoostsCmd creates the boost command for managing boosts. @@ -27,6 +30,7 @@ characters max) or emoji. Use 'basecamp boost list ' to see boosts on an item. Use 'basecamp boost show ' to view a specific boost. Use 'basecamp boost create "content"' to boost an item. +Use 'basecamp boost update "content"' to change a boost you gave. Use 'basecamp boost delete ' to remove a boost. Tip: In the TUI, press 'b' on any item to boost interactively.`, @@ -40,6 +44,7 @@ Tip: In the TUI, press 'b' on any item to boost interactively.`, newBoostListCmd(&project), newBoostShowCmd(&project), newBoostCreateCmd(&project), + newBoostUpdateCmd(&project), newBoostDeleteCmd(), ) @@ -307,9 +312,21 @@ Use --event to boost a specific event within the item.`, return cmd } +// boostContentMax is the longest note a boost holds, in characters. +const boostContentMax = 16 + +// checkBoostLength refuses content the server would not accept as a boost, +// before any request is made. +func checkBoostLength(content string) error { + if n := utf8.RuneCountInString(content); n > boostContentMax { + return output.ErrUsage(fmt.Sprintf("Boost content too long (%d characters, max %d)", n, boostContentMax)) + } + return nil +} + func runBoostCreate(cmd *cobra.Command, app *appctx.App, recording, project, content, eventID string) error { - if n := utf8.RuneCountInString(content); n > 16 { - return output.ErrUsage(fmt.Sprintf("Boost content too long (%d characters, max 16)", n)) + if err := checkBoostLength(content); err != nil { + return err } recordingID, urlProjectID := extractCommentWithProject(recording) @@ -390,6 +407,100 @@ func runBoostCreate(cmd *cobra.Command, app *appctx.App, recording, project, con ) } +func newBoostUpdateCmd(project *string) *cobra.Command { + cmd := &cobra.Command{ + Use: "update ", + Short: "Change the content of a boost", + Long: `Replace the content of a boost with a new short note or emoji. + +Only the boost's creator can update it; anyone else, admins included, is +refused. Content is a short note, 16 characters max, as with create. + +You can pass either a boost ID or a Basecamp URL: + basecamp boost update 789 "✅ done" + basecamp boost update https://3.basecamp.com/123/buckets/456/boosts/789 "👍" + +Use "-" as the content to read it from stdin. + +A boost is addressed by its own ID, so no project or item is needed.`, + Example: ` basecamp boost update 789 "✅ done"`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + app := appctx.FromContext(cmd.Context()) + + // A URL's host and shape are settled before content is read, so a + // bad reference doesn't cost the caller a drained pipe. + boostID, urlProjectID, urlAccountID := args[0], "", "" + if urlarg.IsURL(args[0]) { + if !hostutil.IsTrustedBasecampHost(args[0], app.Config.BaseURL) { + return output.ErrUsage("refusing untrusted host in URL — expected a Basecamp URL") + } + // A recording's boost list (/recordings/{r}/boosts) parses as + // boosts too, with the recording's ID: only one boost will do. + parsed := urlarg.Parse(args[0]) + if parsed == nil || parsed.Type != "boosts" || parsed.IsCollection || parsed.RecordingID == "" { + return output.ErrUsage("expected a boost ID or a boost URL of the form /buckets/{p}/boosts/{b}") + } + boostID, urlProjectID, urlAccountID = parsed.RecordingID, parsed.ProjectID, parsed.AccountID + } + boostIDInt, err := strconv.ParseInt(boostID, 10, 64) + if err != nil { + return output.ErrUsage("Invalid boost ID") + } + + content, err := resolveContentValue(cmd, args[1], 1, "") + if err != nil { + return err + } + if strings.TrimSpace(content) == "" { + return output.ErrUsage("Boost content required") + } + if err := checkBoostLength(content); err != nil { + return err + } + if err := ensureAccount(cmd, app); err != nil { + return err + } + if urlAccountID != "" && urlAccountID != app.Config.AccountID { + return output.ErrUsage(fmt.Sprintf("URL account %s does not match the configured account %s", urlAccountID, app.Config.AccountID)) + } + + boost, err := app.Account().Boosts().Update(cmd.Context(), boostIDInt, content) + if err != nil { + return convertSDKError(err) + } + + scope := " --project " + for _, candidate := range []string{*project, urlProjectID, app.Flags.Project, app.Config.ProjectID} { + if candidate != "" { + scope = breadcrumbScope(candidate) + break + } + } + + return app.OK(boost, + output.WithSummary(fmt.Sprintf("Updated boost #%d to %s", boost.ID, boost.Content)), + output.WithBreadcrumbs( + output.Breadcrumb{ + Action: "show", + Cmd: fmt.Sprintf("basecamp boost show %d%s", boost.ID, scope), + Description: "View boost", + }, + output.Breadcrumb{ + Action: "delete", + Cmd: fmt.Sprintf("basecamp boost delete %d", boost.ID), + Description: "Delete boost", + }, + ), + ) + }, + } + + allowDash(cmd, "arg:1") + + return cmd +} + func newBoostDeleteCmd() *cobra.Command { cmd := &cobra.Command{ Use: "delete ", diff --git a/internal/commands/boost_test.go b/internal/commands/boost_test.go index b41a3c794..7a360da53 100644 --- a/internal/commands/boost_test.go +++ b/internal/commands/boost_test.go @@ -433,3 +433,200 @@ func (t *mockBoostNilBoosterTransport) RoundTrip(req *http.Request) (*http.Respo Header: header, }, nil } + +// --- boost update --- + +const boostUpdatePath = "/99999/boosts/789" + +func boostUpdateRoute(status int, body string) stubRoute { + return stubRoute{method: http.MethodPut, path: boostUpdatePath, status: status, body: body} +} + +func requireBoostUpdateSent(t *testing.T, transport *recordingTransport, content string) { + t.Helper() + + call := transport.last(t) + assert.Equal(t, http.MethodPut, call.Method) + assert.Equal(t, boostUpdatePath, call.Path) + var sent map[string]any + require.NoError(t, json.Unmarshal([]byte(call.Body), &sent)) + assert.Equal(t, content, sent["content"]) +} + +func TestBoostUpdateSendsNewContent(t *testing.T) { + app, transport := setupRecordingTestApp(t, + boostUpdateRoute(http.StatusOK, `{"id":789,"content":"✅ done","created_at":"2024-01-01T00:00:00Z"}`)) + buf := &bytes.Buffer{} + app.Output = output.New(output.Options{Format: output.FormatJSON, Writer: buf}) + + require.NoError(t, executeBoostCommand(NewBoostsCmd(), app, "update", "789", "✅ done")) + + requireBoostUpdateSent(t, transport, "✅ done") + assert.Len(t, transport.recorded(), 1, "a boost is addressed by its own ID: no project or item lookup") + + var envelope struct { + Summary string `json:"summary"` + Data struct { + ID int `json:"id"` + Content string `json:"content"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) + assert.Equal(t, "Updated boost #789 to ✅ done", envelope.Summary) + assert.Equal(t, 789, envelope.Data.ID) + assert.Equal(t, "✅ done", envelope.Data.Content) +} + +func TestBoostsAliasUpdates(t *testing.T) { + app, transport := setupRecordingTestApp(t, + boostUpdateRoute(http.StatusOK, `{"id":789,"content":"👍","created_at":"2024-01-01T00:00:00Z"}`)) + + root := &cobra.Command{Use: "basecamp"} + root.AddCommand(NewBoostsCmd()) + require.NoError(t, executeBoostCommand(root, app, "boosts", "update", "789", "👍")) + + requireBoostUpdateSent(t, transport, "👍") +} + +func TestBoostUpdateAcceptsBoostURL(t *testing.T) { + app, transport := setupRecordingTestApp(t, + boostUpdateRoute(http.StatusOK, `{"id":789,"content":"👍","created_at":"2024-01-01T00:00:00Z"}`)) + + url := "https://3.basecamp.com/99999/buckets/123/boosts/789" + require.NoError(t, executeBoostCommand(NewBoostsCmd(), app, "update", url, "👍")) + + requireBoostUpdateSent(t, transport, "👍") +} + +func TestBoostUpdateReadsContentFromStdin(t *testing.T) { + app, transport := setupRecordingTestApp(t, + boostUpdateRoute(http.StatusOK, `{"id":789,"content":"🎉","created_at":"2024-01-01T00:00:00Z"}`)) + + cmd := NewBoostsCmd() + cmd.SetIn(strings.NewReader("🎉\n")) + require.NoError(t, executeBoostCommand(cmd, app, "update", "789", "-")) + + requireBoostUpdateSent(t, transport, "🎉") +} + +func TestBoostUpdateRefusesLongContentBeforeAnyRequest(t *testing.T) { + app, transport := setupRecordingTestApp(t, boostUpdateRoute(http.StatusOK, `{}`)) + + err := executeBoostCommand(NewBoostsCmd(), app, "update", "789", "this is way long!") + + requireBoostUsageError(t, err, "Boost content too long (17 characters, max 16)") + assert.Empty(t, transport.recorded(), "an oversize boost must not reach the API") +} + +func TestBoostUpdateAcceptsMaxContent(t *testing.T) { + app, transport := setupRecordingTestApp(t, + boostUpdateRoute(http.StatusOK, `{"id":789,"content":"exactly16chars!!","created_at":"2024-01-01T00:00:00Z"}`)) + + require.NoError(t, executeBoostCommand(NewBoostsCmd(), app, "update", "789", "exactly16chars!!")) + + requireBoostUpdateSent(t, transport, "exactly16chars!!") +} + +func TestBoostUpdateCountsCharactersNotBytes(t *testing.T) { + app, transport := setupRecordingTestApp(t, + boostUpdateRoute(http.StatusOK, `{"id":789,"content":"x","created_at":"2024-01-01T00:00:00Z"}`)) + + sixteenEmoji := strings.Repeat("🎉", 16) + require.NoError(t, executeBoostCommand(NewBoostsCmd(), app, "update", "789", sixteenEmoji)) + requireBoostUpdateSent(t, transport, sixteenEmoji) +} + +func TestBoostUpdateRefusesEmptyContentBeforeAnyRequest(t *testing.T) { + for _, content := range []string{"", " "} { + app, transport := setupRecordingTestApp(t, boostUpdateRoute(http.StatusOK, `{}`)) + + err := executeBoostCommand(NewBoostsCmd(), app, "update", "789", content) + + requireBoostUsageError(t, err, "Boost content required") + assert.Empty(t, transport.recorded(), "blank content %q must not reach the API", content) + } +} + +func TestBoostUpdateRefusesBadIDBeforeAnyRequest(t *testing.T) { + app, transport := setupRecordingTestApp(t, boostUpdateRoute(http.StatusOK, `{}`)) + + err := executeBoostCommand(NewBoostsCmd(), app, "update", "nope", "👍") + + requireBoostUsageError(t, err, "Invalid boost ID") + assert.Empty(t, transport.recorded()) +} + +func TestBoostUpdateMapsServerRefusals(t *testing.T) { + cases := []struct { + name string + status int + body string + code string + }{ + {"not the creator", http.StatusForbidden, ``, output.CodeForbidden}, + {"no such boost", http.StatusNotFound, ``, output.CodeNotFound}, + {"rejected content", http.StatusUnprocessableEntity, `{"error":"Content can't be blank"}`, output.CodeValidation}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + app, _ := setupRecordingTestApp(t, boostUpdateRoute(tc.status, tc.body)) + + err := executeBoostCommand(NewBoostsCmd(), app, "update", "789", "👍") + + require.Error(t, err) + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Equal(t, tc.status, e.HTTPStatus) + assert.Equal(t, tc.code, e.Code) + }) + } +} + +// A URL is held to one boost in the configured account, on a Basecamp host, +// before anything is sent: a URL naming another account, another kind of +// thing, or a list of boosts would otherwise update the wrong boost. +func TestBoostUpdateRefusesAURLThatIsNotOneBoostHere(t *testing.T) { + for _, tc := range []struct { + name, url, want string + }{ + {"another account", "https://3.basecamp.com/88888/buckets/123/boosts/789", "does not match the configured account"}, + {"an untrusted host", "https://evil.example.com/99999/buckets/123/boosts/789", "untrusted host"}, + {"a to-do", "https://3.basecamp.com/99999/buckets/123/todos/789", "expected a boost ID or a boost URL"}, + {"a recording's boosts", "https://3.basecamp.com/99999/buckets/123/recordings/789/boosts", "expected a boost ID or a boost URL"}, + {"a project's boosts", "https://3.basecamp.com/99999/buckets/123/boosts", "expected a boost ID or a boost URL"}, + } { + t.Run(tc.name, func(t *testing.T) { + app, transport := setupRecordingTestApp(t, boostUpdateRoute(http.StatusOK, `{"id":789,"content":"👍"}`)) + + err := executeBoostCommand(NewBoostsCmd(), app, "update", tc.url, "👍") + + require.Error(t, err) + var e *output.Error + require.True(t, errors.As(err, &e), "a usage error: %v", err) + assert.Equal(t, output.CodeUsage, e.Code) + assert.Contains(t, e.Message, tc.want) + assert.Empty(t, transport.recorded(), "nothing is sent") + }) + } +} + +// The show breadcrumb keeps a project name with a space as one argument. +func TestBoostUpdateQuotesTheProjectInItsBreadcrumb(t *testing.T) { + app, _ := setupRecordingTestApp(t, + boostUpdateRoute(http.StatusOK, `{"id":789,"content":"👍","created_at":"2024-01-01T00:00:00Z"}`)) + buf := &bytes.Buffer{} + app.Output = output.New(output.Options{Format: output.FormatJSON, Writer: buf}) + app.Flags.Hints = true + + require.NoError(t, executeBoostCommand(NewBoostsCmd(), app, "update", "789", "👍", "--project", "My Project")) + + var envelope struct { + Breadcrumbs []struct { + Cmd string `json:"cmd"` + } `json:"breadcrumbs"` + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) + require.NotEmpty(t, envelope.Breadcrumbs) + assert.Equal(t, "basecamp boost show 789 --project 'My Project'", envelope.Breadcrumbs[0].Cmd) +} diff --git a/internal/commands/commands.go b/internal/commands/commands.go index f2c5f2a4d..604595a68 100644 --- a/internal/commands/commands.go +++ b/internal/commands/commands.go @@ -105,7 +105,7 @@ func CommandCategories() []CommandCategory { {Name: "subscriptions", Category: "communication", Description: "Manage notification subscriptions", Actions: []string{"show", "subscribe", "unsubscribe", "add", "remove"}}, {Name: "attachments", Category: "communication", Description: "List and download attachments", Actions: []string{"list", "download"}}, {Name: "comments", Category: "communication", Description: "Manage comments", Actions: []string{"create", "list", "show", "thread", "update", "trash", "archive", "restore"}}, - {Name: "boost", Category: "communication", Description: "Manage boosts (reactions)", Actions: []string{"list", "show", "create", "delete"}}, + {Name: "boost", Category: "communication", Description: "Manage boosts (reactions)", Actions: []string{"list", "show", "create", "update", "delete"}}, {Name: "notifications", Category: "communication", Description: "View and manage notifications", Actions: []string{"list", "read", "bubbleups"}}, {Name: "inbox", Category: "communication", Description: "Poll addressed items from the account event feed (agents only)"}, },