Skip to content

activities.update update fields need to be wrapped in a body object to be able to update the activity. - #207

Open
bigandy wants to merge 3 commits into
node-strava:mainfrom
bigandy:bug/activity-update-body
Open

activities.update update fields need to be wrapped in a body object to be able to update the activity. #207
bigandy wants to merge 3 commits into
node-strava:mainfrom
bigandy:bug/activity-update-body

Conversation

@bigandy

@bigandy bigandy commented Jul 24, 2026

Copy link
Copy Markdown

activities.update update fields need to be wrapped in a body object to be able to update the activity.

See https://developers.strava.com/docs/reference/#api-models-UpdatableActivity and https://developers.strava.com/docs/reference/#api-Activities-updateActivityById

Resolved #206

Summary by CodeRabbit

  • Bug Fixes

    • Updated activity edits to require an update body payload.
    • Added clearer validation error when the update payload is missing.
  • Type Updates

    • Refined activity update argument definitions to nest editable fields under body.
  • Behavior Changes

    • Allowed hide_from_home to be included in activity update requests.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Activity update fields are now nested under body, with runtime validation and an expanded update allowlist. Tests use the nested request shape, and ActivityType removes Elliptical.

Changes

Activity update contract

Layer / File(s) Summary
Update argument type contract
index.d.ts
ActivityUpdateArgs moves editable fields into body, while ActivityType removes Elliptical.
Runtime validation and update tests
lib/activities.js, test/activities.js
activities.update() requires args.body, includes hide_from_home in its allowlist, and tests pass name and sport type through the nested body.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: markstos

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Removing "Elliptical" from ActivityType is unrelated to fixing activities.update and appears outside the issue scope. Keep the update-body changes, but revert the ActivityType union change unless it is required by a linked issue.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The update API now requires body, the interface reflects it, and tests cover the working body signature for issue #206.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: activity update fields now belong under a required body object.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@index.d.ts`:
- Around line 681-690: Align the `body` fields in the relevant TypeScript
declaration with the runtime allowlist in `lib/activities.js`: remove
unsupported `hide_from_home`, add supported `private`, and ensure every
runtime-supported field is represented. Add coverage that verifies all supported
activity body fields remain synchronized between the declaration and runtime
behavior.

In `@lib/activities.js`:
- Around line 82-83: Update the activity update flow around _requireActivityId
and _requireActivityUpdateBody so serialization extracts allowed update
properties from args.body rather than the outer args object. Ensure
_requireActivityUpdateBody rejects null and non-object bodies, while preserving
validation of the required nested fields and correctly serializing values such
as { name }.

In `@test/activities.js`:
- Around line 118-120: Update the `ActivityUpdateArgs.body` payload in the
activity test to use the serialized `sport_type` key instead of `sportType`,
ensuring the field is preserved in the outgoing form and matches the runtime
allowlist.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5708922b-3bf7-4943-b40e-403365194426

📥 Commits

Reviewing files that changed from the base of the PR and between f62fba1 and 5ec7bfc.

📒 Files selected for processing (3)
  • index.d.ts
  • lib/activities.js
  • test/activities.js

Comment thread index.d.ts
Comment thread lib/activities.js
Comment thread test/activities.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Activity update does not work properly

1 participant