Skip to content

fix(users): send the name as full_name in users.update - #140

Merged
lmjabreu merged 3 commits into
mainfrom
lmjabreu/update-user-full-name
Nov 12, 2026
Merged

lmjabreu merged 3 commits into
mainfrom
lmjabreu/update-user-full-name

Conversation

@lmjabreu

@lmjabreu lmjabreu commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Context

  • users.update({ name }) sent name, but comms-backend's UserUpdateBody reads full_name and ignores fields it does not know. The call returned 200 and changed nothing, so a rename from comms-mcp's update-user-info looked like it worked.

What was changed

  • update() sends name as full_name. The public name argument is unchanged, so callers need no change.
  • New test in users-client.test.ts fails without the fix (name sent where full_name is expected) and passes with it. npm run verify passes.
  • Live against Comms with the same name, so nothing changed: {"name": ...} returns 200 and writes nothing; {"full_name": ...} now reaches Todoist.
  • Not fixed here: with an OAuth token, that Todoist hop returns 401 "Invalid token" for name, timezone and language alike, because comms-backend forwards the caller's token to Todoist's /user/update (Doist/comms-backend#518). That needs a backend change.

Refs

🤖 Generated with Claude Code

Comms reads the name as full_name and ignores fields it does not know,
so users.update({ name }) returned 200 without changing anything.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lmjabreu lmjabreu added the 👀 Show PR requires passing checks before merge and independent human review before or after merge. label Nov 11, 2026
@lmjabreu
lmjabreu marked this pull request as ready for review November 11, 2026 00:49
@lmjabreu
lmjabreu requested review from a team and engfragui and removed request for a team November 11, 2026 00:50

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR fixes users.update to send the rename as full_name, the field comms-backend actually reads, while keeping the public name argument unchanged — so a previously silent no-op now reaches Todoist, and new MSW tests pin the exact wire format including the no-name case. No inline issues were flagged; the mapping through the existing snakeCaseKeys serialization and the test coverage match the repo's established patterns.

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (2)
  • P3 src/clients/users-client.test.ts:246: These new tests instantiate UsersClient directly, but the repository's write-tests convention and AGENTS.md's definition of done exercise clients through the public interface: new CommsApi(TEST_API_TOKEN) then api.users.update(...). Routing through CommsApi keeps the tests on the public API and matches most other client test files.
  • P3 src/clients/users-client.test.ts:267: This test would still pass with the change undone (update({ timezone }) already sent exactly { timezone }), so it adds no proof for this fix. It also cannot guard the name === undefined branch it seems to target: even { ...rest, fullName: name } with name undefined serializes to {"timezone":"Europe/London"} because JSON.stringify(snakeCaseKeys(...)) drops full_name: undefined before MSW parses the body. Drop it, or fold the timezone passthrough check into the first test.

Share Feedback • Review Logs

…test

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lmjabreu

Copy link
Copy Markdown
Contributor Author

These new tests instantiate UsersClient directly, but the repository's write-tests convention and AGENTS.md's definition of done exercise clients through the public interface

Done in 83093c0. The test now calls api.users.update(...) on a new CommsApi(TEST_API_TOKEN).

This test would still pass with the change undone (update({ timezone }) already sent exactly { timezone }), so it adds no proof for this fix.

Done in 83093c0. Dropped it, since the first test already checks timezone and lang pass through next to full_name.

Comment thread src/clients/users-client.ts Resolved
Comment thread src/clients/users-client.ts Resolved
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lmjabreu
lmjabreu merged commit 24a7472 into main Nov 12, 2026
5 checks passed
@lmjabreu
lmjabreu deleted the lmjabreu/update-user-full-name branch November 12, 2026 16:08
doist-release-bot Bot added a commit that referenced this pull request Nov 12, 2026
## [5.0.2](v5.0.1...v5.0.2) (2026-11-12)

### Bug Fixes

* **users:** send the name as full_name in users.update ([#140](#140)) ([24a7472](24a7472))
@lmjabreu

Copy link
Copy Markdown
Contributor Author

@engfragui the release after this merged got stuck halfway: GitHub returned a 500 while semantic-release was pushing its notes, so the v5.0.2 tag and release commit are on main but nothing reached npm (still 5.0.1). Re-running the job doesn't help, it skips because main has moved past that commit (run).

Would you be able to publish 5.0.2, or know who can? No rush if another release is due soon, #147 would carry it too.

doist-release-bot Bot added a commit that referenced this pull request Nov 12, 2026
## [5.0.2](v5.0.1...v5.0.2) (2026-11-12)

### Bug Fixes

* **users:** send the name as full_name in users.update ([#140](#140)) ([24a7472](24a7472))
@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 5.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@scottlovegrove

Copy link
Copy Markdown
Collaborator

@lmjabreu All done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released 👀 Show PR requires passing checks before merge and independent human review before or after merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants