diff --git a/go.mod b/go.mod index 8efce125..b67835ea 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.1.0 charm.land/bubbletea/v2 v2.0.7 charm.land/lipgloss/v2 v2.0.4 - github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260423230153-f54589f0924a + github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260629111348-cc8e9772e729 github.com/basecamp/cli v0.2.1 github.com/charmbracelet/bubbles v1.0.0 github.com/charmbracelet/glamour v1.0.0 @@ -66,7 +66,7 @@ require ( github.com/muesli/cancelreader v0.2.2 // indirect github.com/muesli/reflow v0.3.0 // indirect github.com/muesli/termenv v0.16.0 // indirect - github.com/oapi-codegen/runtime v1.4.0 // indirect + github.com/oapi-codegen/runtime v1.4.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/rogpeppe/go-internal v1.14.1 // indirect diff --git a/go.sum b/go.sum index d4baa9c8..2d86117a 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.7.4-0.20260423230153-f54589f0924a h1:TPVDkxRbdon4oxEYycnTV1Aslz2ZyMqgPiPUutNc+cg= -github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260423230153-f54589f0924a/go.mod h1:g53B/9z0VNYo217NrAf4zuEDc2yNolFBa09C3vSHbUI= +github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260629111348-cc8e9772e729 h1:bYGNjaxnlTTDUzn7SNgBGawMFkQ0UZB1Kfw3WJWi8eY= +github.com/basecamp/basecamp-sdk/go v0.7.4-0.20260629111348-cc8e9772e729/go.mod h1:39n0Qo1z9IWCnIIJGgTu19OukfnFbvJk27c2+4KzDMI= github.com/basecamp/cli v0.2.1 h1:8GyehPVtsTXla0oOPu4QgXRjwwzJ99prlByvyi+0HRQ= github.com/basecamp/cli v0.2.1/go.mod h1:p8tt/DatJ2LAzWO6N6tNfV8x3gF5T3IxDTo+U8FfWPo= github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= @@ -129,8 +129,10 @@ github.com/muesli/reflow v0.3.0 h1:IFsN6K9NfGtjeggFP+68I4chLZV2yIKsXJFNZ+eWh6s= github.com/muesli/reflow v0.3.0/go.mod h1:pbwTDkVPibjO2kyvBQRBxTWEEGDGq0FlB1BIKtnHY/8= github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc= github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= -github.com/oapi-codegen/runtime v1.4.0 h1:KLOSFOp7UzkbS7Cs1ms6NBEKYr0WmH2wZG0KKbd2er4= -github.com/oapi-codegen/runtime v1.4.0/go.mod h1:5sw5fxCDmnOzKNYmkVNF8d34kyUeejJEY8HNT2WaPec= +github.com/oapi-codegen/nullable v1.1.0 h1:eAh8JVc5430VtYVnq00Hrbpag9PFRGWLjxR1/3KntMs= +github.com/oapi-codegen/nullable v1.1.0/go.mod h1:KUZ3vUzkmEKY90ksAmit2+5juDIhIZhfDl+0PwOQlFY= +github.com/oapi-codegen/runtime v1.4.2 h1:GMxFVYLzoYLua+/KvzgSphkyK1lLTReQI9Vf4hvATKE= +github.com/oapi-codegen/runtime v1.4.2/go.mod h1:GwV7hC2hviaMzj+ITfHVRESK5J2W/GefVwIND/bMGvU= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rivo/uniseg v0.1.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= @@ -161,8 +163,8 @@ github.com/yuin/goldmark-emoji v1.0.6/go.mod h1:ukxJDKFpdFb5x0a5HqbdlcKtebh086iJ github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPccHs= github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= -golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= +golang.org/x/exp v0.0.0-20240404231335-c0f41cb1a7a0 h1:985EYyeCOxTpcgOTJpflJUwOeEz0CQOdPt73OzpE9F8= +golang.org/x/exp v0.0.0-20240404231335-c0f41cb1a7a0/go.mod h1:/lliqkxwWAhPjf5oSOIJup2XcqJaw8RGS6k3TGEc7GI= golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ= golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0= golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= diff --git a/internal/commands/cards.go b/internal/commands/cards.go index 68cc0c70..7d8c2e30 100644 --- a/internal/commands/cards.go +++ b/internal/commands/cards.go @@ -1177,9 +1177,9 @@ func newCardsColumnCmd(project, cardTable *string) *cobra.Command { newCardsColumnMoveCmd(project, cardTable), newCardsColumnWatchCmd(), newCardsColumnUnwatchCmd(), - newCardsColumnOnHoldCmd(), - newCardsColumnNoOnHoldCmd(), - newCardsColumnColorCmd(), + newCardsColumnOnHoldCmd(project), + newCardsColumnNoOnHoldCmd(project), + newCardsColumnColorCmd(project), ) return cmd @@ -1560,7 +1560,7 @@ You can pass either a column ID or a Basecamp URL: return cmd } -func newCardsColumnOnHoldCmd() *cobra.Command { +func newCardsColumnOnHoldCmd(project *string) *cobra.Command { cmd := &cobra.Command{ Use: "on-hold ", Short: "Enable on-hold section", @@ -1577,14 +1577,23 @@ You can pass either a column ID or a Basecamp URL: return err } - // Extract ID from URL if provided - columnIDStr := extractID(args[0]) + // Extract ID and project from URL if provided + columnIDStr, urlProjectID := extractWithProject(args[0]) columnID, err := strconv.ParseInt(columnIDStr, 10, 64) if err != nil { return output.ErrUsage("Invalid column ID") } - col, err := app.Account().CardColumns().EnableOnHold(cmd.Context(), columnID) + if err := requireStandardColumn(cmd, app, columnID, "Enabling on-hold"); err != nil { + return err + } + + bucketID, err := resolveColumnBucketID(cmd, app, *project, urlProjectID) + if err != nil { + return err + } + + col, err := app.Account().CardColumns().EnableOnHold(cmd.Context(), bucketID, columnID) if err != nil { return convertSDKError(err) } @@ -1597,7 +1606,7 @@ You can pass either a column ID or a Basecamp URL: return cmd } -func newCardsColumnNoOnHoldCmd() *cobra.Command { +func newCardsColumnNoOnHoldCmd(project *string) *cobra.Command { cmd := &cobra.Command{ Use: "no-on-hold ", Short: "Disable on-hold section", @@ -1614,14 +1623,23 @@ You can pass either a column ID or a Basecamp URL: return err } - // Extract ID from URL if provided - columnIDStr := extractID(args[0]) + // Extract ID and project from URL if provided + columnIDStr, urlProjectID := extractWithProject(args[0]) columnID, err := strconv.ParseInt(columnIDStr, 10, 64) if err != nil { return output.ErrUsage("Invalid column ID") } - col, err := app.Account().CardColumns().DisableOnHold(cmd.Context(), columnID) + if err := requireStandardColumn(cmd, app, columnID, "Disabling on-hold"); err != nil { + return err + } + + bucketID, err := resolveColumnBucketID(cmd, app, *project, urlProjectID) + if err != nil { + return err + } + + col, err := app.Account().CardColumns().DisableOnHold(cmd.Context(), bucketID, columnID) if err != nil { return convertSDKError(err) } @@ -1634,7 +1652,7 @@ You can pass either a column ID or a Basecamp URL: return cmd } -func newCardsColumnColorCmd() *cobra.Command { +func newCardsColumnColorCmd(project *string) *cobra.Command { var color string cmd := &cobra.Command{ @@ -1658,14 +1676,23 @@ You can pass either a column ID or a Basecamp URL: return err } - // Extract ID from URL if provided - columnIDStr := extractID(args[0]) + // Extract ID and project from URL if provided + columnIDStr, urlProjectID := extractWithProject(args[0]) columnID, err := strconv.ParseInt(columnIDStr, 10, 64) if err != nil { return output.ErrUsage("Invalid column ID") } - col, err := app.Account().CardColumns().SetColor(cmd.Context(), columnID, color) + if err := requireStandardColumn(cmd, app, columnID, "Setting a column color"); err != nil { + return err + } + + bucketID, err := resolveColumnBucketID(cmd, app, *project, urlProjectID) + if err != nil { + return err + } + + col, err := app.Account().CardColumns().SetColor(cmd.Context(), bucketID, columnID, color) if err != nil { return convertSDKError(err) } @@ -2165,6 +2192,67 @@ func getCardTableID(cmd *cobra.Command, app *appctx.App, projectID, explicitCard return "", ambiguousCardTablesError(cardTables) } +// resolveColumnBucketID resolves the numeric project (bucket) ID for a column +// command that takes a column ID or URL plus the --in/--project flag. +// +// Precedence: URL > flag > config > interactive. This intentionally differs from +// most card commands, which seed from the flag first: a column URL encodes the +// column's actual bucket, so honoring it over a mismatched --in avoids targeting +// the wrong bucket (a guaranteed 404). +func resolveColumnBucketID(cmd *cobra.Command, app *appctx.App, project, urlProjectID string) (int64, error) { + projectID := urlProjectID + if projectID == "" { + projectID = project + } + if projectID == "" { + projectID = app.Flags.Project + } + if projectID == "" { + projectID = app.Config.ProjectID + } + if projectID == "" { + if err := ensureProject(cmd, app); err != nil { + return 0, err + } + projectID = app.Config.ProjectID + } + + resolvedProjectID, _, err := app.Names.ResolveProject(cmd.Context(), projectID) + if err != nil { + return 0, err + } + + bucketID, err := strconv.ParseInt(resolvedProjectID, 10, 64) + if err != nil { + return 0, output.ErrUsage("Project ID must be numeric") + } + return bucketID, nil +} + +// standardColumnType is the only card column type that supports color and +// on-hold sections. The API rejects these actions on Triage, Not now, and Done +// columns with a bare 404, so we check the type up front and return a clearer +// error. +const standardColumnType = "Kanban::Column" + +// requireStandardColumn verifies the column supports color/on-hold actions. +// Triage, Not now, and Done columns don't, so it returns a friendly usage error +// instead of the API's bare 404. action names the attempted operation, e.g. +// "Enabling on-hold". +func requireStandardColumn(cmd *cobra.Command, app *appctx.App, columnID int64, action string) error { + col, err := app.Account().CardColumns().Get(cmd.Context(), columnID) + if err != nil { + return convertSDKError(err) + } + if col.Type != standardColumnType { + return output.ErrUsageHint( + fmt.Sprintf("%s is only available on standard columns", action), + fmt.Sprintf("%q is not a standard column — color and on-hold sections apply only to regular columns, not Triage, Not now, or Done", col.Title), + ) + } + return nil +} + func listProjectCardTables(cmd *cobra.Command, app *appctx.App, projectID string) ([]projectCardTable, error) { path := fmt.Sprintf("/projects/%s.json", projectID) resp, err := app.Account().Get(cmd.Context(), path) diff --git a/internal/commands/cards_test.go b/internal/commands/cards_test.go index 85fcce44..159b283f 100644 --- a/internal/commands/cards_test.go +++ b/internal/commands/cards_test.go @@ -127,7 +127,8 @@ func TestCardsColumnColorShowsHelp(t *testing.T) { // Configure app with project app.Config.ProjectID = "123" - cmd := newCardsColumnColorCmd() + project := "" + cmd := newCardsColumnColorCmd(&project) err := executeCommand(cmd, app, "456") // column ID but no --color assert.NoError(t, err, "expected help output, not error") @@ -1782,3 +1783,162 @@ func TestResolveAssigneeIDAcceptsPositive(t *testing.T) { require.NoError(t, err) assert.Equal(t, int64(42), id) } + +// mockCardColumnTransport serves the endpoints used by the column color/on-hold +// commands. columnType controls the type returned by the column GET (which the +// type guard inspects); getStatus lets a test simulate a missing column. It +// records the method and path of any color/on-hold mutation so tests can assert +// the correct bucket-scoped endpoint was called — or that no mutation happened. +type mockCardColumnTransport struct { + columnType string + getStatus int + + mutateMethod string + mutatePath string +} + +func (m *mockCardColumnTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + respond := func(status int, body string) (*http.Response, error) { + return &http.Response{ + StatusCode: status, + Body: io.NopCloser(strings.NewReader(body)), + Header: header, + }, nil + } + column := func(colType string) string { + return fmt.Sprintf(`{"id":789,"type":%q,"title":"In progress","status":"active",`+ + `"inherits_status":true,"visible_to_clients":false,`+ + `"created_at":"2026-07-03T00:00:00Z","updated_at":"2026-07-03T00:00:00Z",`+ + `"url":"https://example.test/columns/789","app_url":"https://example.test/columns/789",`+ + `"cards_count":0,"comment_count":0}`, colType) + } + + path := req.URL.Path + switch { + case req.Method == "GET" && strings.Contains(path, "/projects.json"): + return respond(200, `[{"id":123,"name":"Test Project"},{"id":999,"name":"Other Project"}]`) + case strings.Contains(path, "/on_hold.json"), strings.Contains(path, "/color.json"): + m.mutateMethod = req.Method + m.mutatePath = path + return respond(200, column(standardColumnType)) + case req.Method == "GET" && strings.Contains(path, "/card_tables/columns/"): + status := m.getStatus + if status == 0 { + status = 200 + } + if status != 200 { + return respond(status, `{"error":"Not Found"}`) + } + return respond(200, column(m.columnType)) + default: + return respond(404, `{"error":"Not Found"}`) + } +} + +func TestCardsColumnOnHoldStandardColumn(t *testing.T) { + tr := &mockCardColumnTransport{columnType: standardColumnType} + app, _ := newTestAppWithTransport(t, tr) + + project := "" + err := executeCommand(newCardsColumnOnHoldCmd(&project), app, "789") + require.NoError(t, err) + assert.Equal(t, "POST", tr.mutateMethod) + assert.Contains(t, tr.mutatePath, "/buckets/123/card_tables/columns/789/on_hold.json") +} + +func TestCardsColumnNoOnHoldStandardColumn(t *testing.T) { + tr := &mockCardColumnTransport{columnType: standardColumnType} + app, _ := newTestAppWithTransport(t, tr) + + project := "" + err := executeCommand(newCardsColumnNoOnHoldCmd(&project), app, "789") + require.NoError(t, err) + assert.Equal(t, "DELETE", tr.mutateMethod) + assert.Contains(t, tr.mutatePath, "/buckets/123/card_tables/columns/789/on_hold.json") +} + +func TestCardsColumnColorStandardColumn(t *testing.T) { + tr := &mockCardColumnTransport{columnType: standardColumnType} + app, _ := newTestAppWithTransport(t, tr) + + project := "" + err := executeCommand(newCardsColumnColorCmd(&project), app, "789", "--color", "blue") + require.NoError(t, err) + assert.Equal(t, "PUT", tr.mutateMethod) + assert.Contains(t, tr.mutatePath, "/buckets/123/card_tables/columns/789/color.json") +} + +// TestCardsColumnActionsRejectNonStandardColumns verifies the type guard: on +// Triage, Not now, and Done columns the on-hold and color commands return a +// friendly usage error and never call the mutating endpoint (which the API would +// answer with a bare 404). +func TestCardsColumnActionsRejectNonStandardColumns(t *testing.T) { + for _, colType := range []string{"Kanban::Triage", "Kanban::NotNowColumn", "Kanban::DoneColumn"} { + t.Run(colType, func(t *testing.T) { + cases := []struct { + name string + cmd func(*string) *cobra.Command + args []string + }{ + {"on-hold", newCardsColumnOnHoldCmd, []string{"789"}}, + {"no-on-hold", newCardsColumnNoOnHoldCmd, []string{"789"}}, + {"color", newCardsColumnColorCmd, []string{"789", "--color", "blue"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tr := &mockCardColumnTransport{columnType: colType} + app, _ := newTestAppWithTransport(t, tr) + + project := "" + err := executeCommand(tc.cmd(&project), app, tc.args...) + require.Error(t, err) + + var e *output.Error + require.True(t, errors.As(err, &e)) + assert.Contains(t, e.Message, "standard columns") + assert.Empty(t, tr.mutatePath, "mutation endpoint must not be called for a non-standard column") + }) + } + }) + } +} + +func TestCardsColumnOnHoldColumnNotFound(t *testing.T) { + tr := &mockCardColumnTransport{columnType: standardColumnType, getStatus: 404} + app, _ := newTestAppWithTransport(t, tr) + + project := "" + err := executeCommand(newCardsColumnOnHoldCmd(&project), app, "789") + require.Error(t, err) + assert.Empty(t, tr.mutatePath, "mutation must not be attempted when the column lookup fails") +} + +// TestCardsColumnColorResolvesProjectFromURL verifies the bucket ID is taken from +// the URL (123) over the configured project (999) and threaded into the endpoint. +func TestCardsColumnColorResolvesProjectFromURL(t *testing.T) { + tr := &mockCardColumnTransport{columnType: standardColumnType} + app, _ := newTestAppWithTransport(t, tr) + app.Config.ProjectID = "999" + + project := "" + url := "https://3.basecamp.com/99999/buckets/123/card_tables/columns/789" + err := executeCommand(newCardsColumnColorCmd(&project), app, url, "--color", "blue") + require.NoError(t, err) + assert.Contains(t, tr.mutatePath, "/buckets/123/card_tables/columns/789/color.json") +} + +// TestCardsColumnColorURLBucketBeatsFlag verifies the URL's bucket (123) wins over +// an explicit --in/--project flag pointing elsewhere (999): the URL identifies the +// column's real bucket, so targeting the flag project would 404. +func TestCardsColumnColorURLBucketBeatsFlag(t *testing.T) { + tr := &mockCardColumnTransport{columnType: standardColumnType} + app, _ := newTestAppWithTransport(t, tr) + + project := "999" // explicit --in/--project pointing at a different bucket + url := "https://3.basecamp.com/99999/buckets/123/card_tables/columns/789" + err := executeCommand(newCardsColumnColorCmd(&project), app, url, "--color", "blue") + require.NoError(t, err) + assert.Contains(t, tr.mutatePath, "/buckets/123/card_tables/columns/789/color.json") +} diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 736a603c..6b6ec2ad 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,9 +1,9 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.7.4-0.20260423230153-f54589f0924a", - "revision": "f54589f0924a", - "updated_at": "2026-04-23T23:01:53Z" + "version": "v0.7.4-0.20260629111348-cc8e9772e729", + "revision": "cc8e9772e729", + "updated_at": "2026-06-29T11:13:48Z" }, "api": { "repo": "basecamp/bc3",