Skip to content

CDP-6131: make triggerBroadcast data and recipients optional#220

Merged
thethp merged 3 commits into
mainfrom
cdp-6131
Jun 11, 2026
Merged

CDP-6131: make triggerBroadcast data and recipients optional#220
thethp merged 3 commits into
mainfrom
cdp-6131

Conversation

@thethp

@thethp thethp commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

The README documents both data and recipients as optional for api.triggerBroadcast(), but the signature declared them as required positional arguments. Two failure modes:

  1. api.triggerBroadcast(4)recipients is undefined, so recipients[field] throws a TypeError
  2. api.triggerBroadcast(4, {}, {}) — the empty recipients object is forwarded to the API, which rejects it with a 422 (empty recipients filter is not valid)

Both params are now optional, and empty/omitted data/recipients are left out of the payload, so triggering a broadcast with just its id sends to the broadcast's configured recipients as documented.

Test plan

  • New ava cases: broadcastId only, data without recipients, and empty {} objects for both
  • npm test — 253 tests pass, coverage stays at 100%
  • npm run lint and npm run build clean

Fixes CDP-6131

🤖 Generated with Claude Code


Note

Low Risk
Targeted SDK fix for broadcast triggers with new tests; behavior change only affects previously broken or invalid call patterns.

Overview
APIClient.triggerBroadcast now treats data and recipients as optional and only adds them to the POST body when they are present and non-empty. Calling with just a broadcast id sends {}, so the broadcast uses its configured recipients instead of throwing on missing recipients or getting a 422 from an empty recipients filter.

Docs and JSDoc explain that both arguments are optional and that recipients without data must use undefined for the second positional argument. Tests cover id-only, data-only, recipients-only, and {}/{} omission.

Reviewed by Cursor Bugbot for commit 6546597. Bugbot is set up for automated code reviews on this repo. Configure here.

The README documents both data and recipients as optional, but the
signature required them: calling with only a broadcastId crashed with a
TypeError on recipients[field], and passing {} for recipients sent an
empty recipients filter that the API rejects with a 422.

Empty or omitted data/recipients are now left out of the payload, so
triggering a broadcast with just its id sends to the broadcast's
configured recipients as documented.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
triggerBroadcast(1, { emails: [...] }) type-checks but sends the
selector as liquid data, triggering the broadcast's configured
recipients. Document the undefined placeholder pattern in the JSDoc and
README, and pin triggerBroadcast(1, undefined, recipients) with a test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@thethp

thethp commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the positional-ambiguity feedback in 17c6112: triggerBroadcast(1, { emails: [...] }) still type-checks but sends the selector as liquid data, triggering the broadcast's configured recipients. Since data can legitimately contain keys like emails, a runtime guard would reject valid calls — so this is addressed by documentation and a pinned test instead:

  • JSDoc and README now explicitly document the undefined placeholder pattern: triggerBroadcast(1, undefined, { emails: [...] })
  • New test asserts the recipients-without-data call sends the selector as a recipient filter (not as data)

254 tests passing, lint and build clean.

mike-engel
mike-engel previously approved these changes Jun 11, 2026
Comment thread lib/api.ts Outdated
triggerBroadcast(broadcastId: string | number, data?: RequestData, recipients?: Recipients) {
let payload: Record<string, unknown> = {};

if (data && Object.keys(data).length > 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (data && Object.keys(data).length > 0) {
if (data != null && Object.keys(data).length > 0) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied in 6546597, thanks!

Comment thread lib/api.ts Outdated
payload.data = data;
}

if (recipients && Object.keys(recipients).length > 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (recipients && Object.keys(recipients).length > 0) {
if (recipients != null && Object.keys(recipients).length > 0) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied in 6546597, thanks!

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@thethp thethp merged commit b90d19f into main Jun 11, 2026
11 checks passed
@thethp thethp deleted the cdp-6131 branch June 11, 2026 16:27
@thethp thethp mentioned this pull request Jun 11, 2026
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.

2 participants