Skip to content

fix: send customData with multi-recipient SMS sends - #16

Open
eastagiletracker wants to merge 1 commit into
CloudContactAI:mainfrom
eastagiletracker:agile-board/sms-campaign-custom-data
Open

fix: send customData with multi-recipient SMS sends#16
eastagiletracker wants to merge 1 commit into
CloudContactAI:mainfrom
eastagiletracker:agile-board/sms-campaign-custom-data

Conversation

@eastagiletracker

Copy link
Copy Markdown

This PR proposes fixing the customData argument on the multi-recipient SMS.SendAsync overload, which is documented as sent with the message but never reaches the API. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/504. You can sign in with your GitHub ID to claim ownership of the project.

What is wrong

ISMSService.SendAsync(accounts, message, title, customData: ...) documents customData as "Custom data to be included with the message", but the value is discarded before the request is built. SMSService.SendAsync(SMSRequest) constructs the outgoing SMSCampaign from Accounts, Message, Title and SenderPhone only, so SMSRequest.CustomData has nowhere to go — the API carries custom data per recipient, as Account.CustomData serialized to messageData. SendSingleAsync is unaffected because SMSRequest.CreateSingle copies customData onto the single Account it builds; only the multi-recipient path loses it, silently and with no error.

Reproducing it on main (c8c7bc3)

A ~30-line console program that references src/CCAI.NET and swaps in an HttpMessageHandler that captures the outgoing body instead of calling the API:

await ccai.SMS.SendAsync(
    accounts: new[]
    {
        new Account { FirstName = "John", LastName = "Doe", Phone = "+15551234567" },
        new Account { FirstName = "Jane", LastName = "Doe", Phone = "+15557654321" }
    },
    message: "Hello ${FirstName}!",
    title: "Order confirmations",
    customData: "{\"orderId\":\"ORD-123\"}");

The captured request body on main today — ORD-123 is nowhere in it:

{
  "accounts": [
    { "firstName": "John", "lastName": "Doe", "phone": "+15551234567" },
    { "firstName": "Jane", "lastName": "Doe", "phone": "+15557654321" }
  ],
  "message": "Hello ${FirstName}!",
  "title": "Order confirmations"
}

With this change, the same program sends:

{
  "accounts": [
    { "firstName": "John", "lastName": "Doe", "phone": "+15551234567", "messageData": "{\"orderId\":\"ORD-123\"}" },
    { "firstName": "Jane", "lastName": "Doe", "phone": "+15557654321", "messageData": "{\"orderId\":\"ORD-123\"}" }
  ],
  "message": "Hello ${FirstName}!",
  "title": "Order confirmations"
}

The change

Eleven lines in SMSService.SendAsync: when request.CustomData is set, it is applied to every account that does not already define its own, immediately before the SMSCampaign is built. This matches how ccai-node maps customData onto messageData for each recipient in its own send().

It is deliberately conservative. A per-account CustomData always wins, so a caller who already sets it per recipient sees no change. Account is a record and the accounts are projected with with, so the caller's own instances are never mutated. No public signature changes, and since the value was previously dropped outright, no existing caller can be depending on the old behaviour.

How it was verified

Four tests added to tests/CCAI.NET.Tests/SMS/SMSCustomDataWebhookTests.cs: two recipients both receiving a request-level customData; a per-account value taking precedence over it; the null case, where nothing is added; and a check that the caller's Account instances come back untouched. The first two fail on the unmodified tree (Expected: "OrderBatch-42" / Actual: null) and pass with the fix.

dotnet build CCAI.NET.sln and dotnet test CCAI.NET.sln were run on the clean tree first as a baseline and again afterwards. Baseline: build succeeded, 121 warnings (all pre-existing CS1591/CS1573 doc warnings), 75 tests passed, none failed. After: build succeeded, the same 121 warnings, 79 tests passed, none failed. No new failures and no new warnings.

One thing worth flagging: your open #15 edits the argument validation a few lines above this hunk on the test branch. The two changes sit in the same method but not on the same lines, so I would expect them to apply cleanly in either order.

How this was managed

This work was tracked as a single story, SMS request-level customData is dropped for multi-recipient sends, on a board at https://eastagiletracker.com/projects/504 that was imported from this repository's own issues and pull requests — 15 stories, mirroring your merged and open work into iterations.

board

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.


Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

SMSService.SendAsync built the outgoing SMSCampaign from Accounts, Message,
Title and SenderPhone only, so SMSRequest.CustomData never reached the wire.
Callers of the multi-recipient overload lost the value silently; only
SendSingleAsync worked, because CreateSingle copies customData onto the single
Account it builds, where it serializes as messageData.

Apply a campaign-level CustomData to every account that does not define its
own. Per-account values keep precedence and the caller's Account instances are
left untouched.
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.

1 participant