Skip to content

fix(cards): carry over step title when updating assignees or due date#496

Open
nnemirovsky wants to merge 5 commits into
basecamp:mainfrom
nnemirovsky:fix/step-update-title
Open

fix(cards): carry over step title when updating assignees or due date#496
nnemirovsky wants to merge 5 commits into
basecamp:mainfrom
nnemirovsky:fix/step-update-title

Conversation

@nnemirovsky

@nnemirovsky nnemirovsky commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Problem

basecamp cards step update <id> --assignees <person> always fails with 400 Bad Request. Same for basecamp assign <id> --step and basecamp unassign <id> --step.

The server rejects a step update whose body has no title key, even though the API docs list every parameter as optional. All three code paths sent {"assignee_ids": [...]} with no title, so every assignee-only or due-date-only update failed. The same request with the current title included succeeds.

Fix

Carry over the current title when the caller does not provide a new one:

  • cards step update: when invoked without a new title, fetch the step first and reuse its title.
  • assign --step / unassign --step: pass the title of the step object these paths already fetch. No extra request.

Tests

Added two unit tests with a capturing mock transport: assignee-only updates fetch the step once and send its current title, and explicit-title updates skip the fetch and send the new title as-is. bin/ci is green.


Summary by cubic

Carry over the current step title when updating only assignees or the due date to prevent 400 Bad Request errors. This fixes basecamp cards step update, basecamp assign --step, and basecamp unassign --step when no new title is given.

  • Bug Fixes
    • cards step update: if no title is provided, fetch the step and include its current title in the update.
    • assign --step / unassign --step: include step.Title in the update without extra requests; send assignee_ids: [] explicitly on unassign.
    • Tests: added coverage for assign/unassign title carry-over; explicit-title updates skip fetching; assert single-step endpoint (suffix match); fail on unexpected routes; echo title in mock responses; surface mock IO errors.

Written for commit 84c2c5f. Summary will update on new commits.

Review in cubic

The step update endpoint rejects requests without a title even though
the API docs mark it optional. Updating only assignees or the due date
sent a body with no title and the server returned 400 Bad Request.

Carry over the current title instead. The step update command fetches
the step first when no new title is given. Assign and unassign reuse
the step they already fetched.
Copilot AI review requested due to automatic review settings June 10, 2026 06:56
@github-actions github-actions Bot added commands CLI command implementations tests Tests (unit and e2e) bug Something isn't working labels Jun 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR ensures step updates always include a title to satisfy the Basecamp API requirement, even when the user only changes other fields (e.g., assignees).

Changes:

  • Carry over the current step title during cards step update when no explicit title is provided.
  • Include the existing step title when assigning/unassigning step assignees.
  • Add tests to verify title carry-over behavior and that unnecessary fetches are skipped when a title is provided.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
internal/commands/cards.go Fetches current step to populate Title when updating other fields without an explicit title.
internal/commands/assign.go Ensures step assignee updates include Title in the update request.
internal/commands/cards_test.go Adds tests + a mock transport to assert fetch behavior and request payload content.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/commands/cards_test.go Outdated
Comment thread internal/commands/cards_test.go Outdated
Comment thread internal/commands/cards.go

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread internal/commands/cards_test.go
Validate that step updates hit the single-step endpoint so a stray call
to the wrong path fails the test instead of passing on stale data. Echo
the title back in the PUT response and surface IO errors from the mock
transport.
@nnemirovsky nnemirovsky requested a review from Copilot June 10, 2026 09:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread internal/commands/cards.go
Comment thread internal/commands/cards.go
Comment thread internal/commands/cards_test.go Outdated
Suffix matching keeps the routing assertion while tolerating account ID
and prefix changes in the SDK path shape.
@nnemirovsky nnemirovsky requested a review from Copilot June 10, 2026 09:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread internal/commands/cards_test.go
Comment thread internal/commands/assign.go
Comment thread internal/commands/assign.go
The assign and unassign step paths must send the current title or the
API rejects the update with 400. Add tests asserting the captured PUT
body includes the carried-over title for both paths.
@nnemirovsky nnemirovsky requested a review from Copilot June 10, 2026 09:43

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread internal/commands/assign_test.go
Comment thread internal/commands/assign_test.go Outdated
Comment thread internal/commands/assign_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread internal/commands/cards_test.go
Comment thread internal/commands/assign_test.go
Fail on unexpected GET routes, assert the PUT targets the step endpoint,
and check assignee_ids is sent as an explicit empty array on unassign.
@nnemirovsky nnemirovsky requested a review from Copilot June 10, 2026 09:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread internal/commands/cards_test.go
Comment thread internal/commands/cards.go
@nnemirovsky nnemirovsky requested a review from Copilot June 10, 2026 10:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comment thread internal/commands/cards_test.go
Comment thread internal/commands/cards_test.go
Comment thread internal/commands/assign_test.go
Comment thread internal/commands/cards.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working commands CLI command implementations tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants