Broadcast shows successfullyDelivered#48
Conversation
WalkthroughReplaced PastBroadcast.totalReplies with successfullyDelivered in the API model, updated UI to display “Delivered successfully” using the new field, and adjusted related tests and mock data accordingly. No endpoint or function signatures changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @kyphan1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request refactors the broadcast statistics by replacing the "totalReplies" metric with a more precise "successfullyDelivered" metric. This change ensures that the displayed broadcast data accurately reflects the number of messages that were successfully delivered to recipients, rather than just those that elicited a response.
Highlights
- API Interface Update: The PastBroadcast interface in src/apis/broadcasts.ts has been updated to include successfullyDelivered and remove totalReplies.
- UI Metric Renaming: The user interface in src/pages/broadcast/LastBatchSection.tsx now displays "Delivered successfully" instead of "Responded" and uses the new successfullyDelivered data.
- Test Data Alignment: All relevant test files (BatchItem.test.tsx, LastBatchSection.test.tsx, PastBroadcastsSection.test.tsx) have been updated to reflect the new successfullyDelivered field in their mock data and assertions.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request correctly replaces the totalReplies field with successfullyDelivered in the PastBroadcast data model and updates all related UI components and tests accordingly. The changes are consistent and align with the description. I have one minor suggestion to improve code clarity in LastBatchSection.tsx by removing a redundant optional chaining operator.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 3643677 in 1 minute and 11 seconds. Click for details.
- Reviewed
108lines of code in5files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/apis/broadcasts.ts:22
- Draft comment:
Replaced 'totalReplies' with 'successfullyDelivered'. Ensure backend payload matches this update. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. src/pages/broadcast/BatchItem.test.tsx:15
- Draft comment:
Updated mock: 'totalReplies' replaced by 'successfullyDelivered'. Consider verifying UI output if needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =40%<= threshold50%The comment suggests verifying the UI output, which is similar to asking the author to double-check something. This violates the rule against asking the author to ensure behavior is intended or tested. However, it does point out a specific change in the code, which could be useful.
3. src/pages/broadcast/LastBatchSection.test.tsx:79
- Draft comment:
Updated expected label and number to 'Delivered successfully' with 'successfullyDelivered' prop. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment seems purely informative, as it is just stating what was changed without providing any suggestions or asking for clarification. It doesn't ask for any specific action or confirmation from the PR author.
4. src/pages/broadcast/LastBatchSection.tsx:69
- Draft comment:
UI now displays 'Delivered successfully' using the 'successfullyDelivered' field; verify consistency with design. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify consistency with the design, which falls under asking the author to confirm or double-check something. This violates the rules.
5. src/pages/broadcast/PastBroadcastsSection.test.tsx:43
- Draft comment:
Mock data updated: replaced 'totalReplies' with 'successfullyDelivered'. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only states what was changed without providing any suggestion, question, or request for confirmation. It doesn't align with the rules for useful comments.
Workflow ID: wflow_XoSIgZqNeztGlyuB
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/pages/broadcast/BatchItem.test.tsx (1)
15-15: Mock updated to new field — looks consistent.
The mock aligns with the PastBroadcast change and other tests in this PR.If BatchItem renders delivery metrics in its expanded view, consider adding expectations for “Delivered successfully” and “Failed to deliver” to improve coverage.
src/pages/broadcast/PastBroadcastsSection.test.tsx (1)
43-43: Updated mock fields are correct; consider asserting delivery metrics if rendered.
Mocks now use successfullyDelivered, consistent with the API change. If PastBroadcastsSection displays these delivery counts, add assertions to ensure the UI reflects the new field.Also applies to: 58-58
src/pages/broadcast/LastBatchSection.test.tsx (1)
48-48: Tests correctly reflect the new label/field; add a negative-path test for robustness.
The expectations for “Delivered successfully” and values (95, 950,000) are consistent. Consider adding a test where successfullyDelivered is missing to ensure the component doesn’t throw and shows a fallback (e.g., '—').Example:
it('handles missing successfullyDelivered gracefully', async () => { (useBroadcastsQuery as any).mockReturnValue({ data: { past: [{ id: 1, runAt: 1619827200, totalFirstSent: 1, totalSecondSent: 1, failedDelivered: 0, totalUnsubscribed: 0 }] }, isLoading: false, isError: false, }); render(<LastBatchSection />); expect(await screen.findByText('Delivered successfully')).toBeInTheDocument(); expect(await screen.findByText('—')).toBeInTheDocument(); // or your chosen fallback });Also applies to: 79-80, 95-95, 112-112
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/apis/broadcasts.ts(1 hunks)src/pages/broadcast/BatchItem.test.tsx(1 hunks)src/pages/broadcast/LastBatchSection.test.tsx(4 hunks)src/pages/broadcast/LastBatchSection.tsx(1 hunks)src/pages/broadcast/PastBroadcastsSection.test.tsx(2 hunks)
🔇 Additional comments (1)
src/apis/broadcasts.ts (1)
22-22: Confirm broadcast field rename complete; optional back-compat only if neededAll occurrences of
totalRepliesin the broadcasts feature have been replaced bysuccessfullyDelivered. A global search shows only two remainingtotalRepliesreferences—insrc/apis/campaigns.tsandsrc/pages/campaign/CampaignHistory.tsx—which belong to the separate campaigns feature and aren’t affected by this change.Since every broadcasts model, UI component, and test now uses
successfullyDelivered, it’s safe to remove the old field. If you still need a short migration window for downstream clients, you can add this toPastBroadcast:/** @deprecated Use successfullyDelivered instead */ totalReplies?: number;Otherwise, you can proceed without any additional back-compat.
| <span>Delivered successfully</span> | ||
| <span className="font-medium tabular-nums text-green-600 dark:text-green-400"> | ||
| {lastBatch?.totalReplies.toLocaleString()} | ||
| {lastBatch?.successfullyDelivered.toLocaleString()} |
There was a problem hiding this comment.
Guard against undefined values to avoid runtime crashes with partial rollouts.
If the backend response is missing successfullyDelivered (e.g., during phased rollout or for legacy records), calling toLocaleString on undefined will throw. Add a null-safe guard.
Apply this diff:
- <span className="font-medium tabular-nums text-green-600 dark:text-green-400">
- {lastBatch?.successfullyDelivered.toLocaleString()}
- </span>
+ <span className="font-medium tabular-nums text-green-600 dark:text-green-400">
+ {typeof lastBatch?.successfullyDelivered === 'number'
+ ? lastBatch.successfullyDelivered.toLocaleString()
+ : '—'}
+ </span>Optionally, for short-term back-compat while services roll out, map old -> new at the API boundary (in getBroadcasts) so the UI always has a number:
// inside getBroadcasts after receiving response.data
const data = response.data;
data.past = data.past.map((p: any) => ({
...p,
successfullyDelivered:
typeof p.successfullyDelivered === 'number'
? p.successfullyDelivered
: typeof p.totalReplies === 'number'
? p.totalReplies
: 0,
}));
return data;🤖 Prompt for AI Agents
In src/pages/broadcast/LastBatchSection.tsx around lines 69 to 71, the code
calls toLocaleString() on lastBatch?.successfullyDelivered which can be
undefined and crash; update the rendering to guard against undefined (e.g.,
derive a safe number via nullish coalescing or a conditional: use
(lastBatch?.successfullyDelivered ?? 0) before calling toLocaleString, or render
a fallback like "-" when undefined). Optionally implement the suggested API-side
mapping in getBroadcasts to normalize legacy records to always provide a numeric
successfullyDelivered value so the UI can safely assume a number.
1 participant
Important
Replace
totalReplieswithsuccessfullyDeliveredin broadcast data model and update related tests and UI components.totalReplieswithsuccessfullyDeliveredinPastBroadcastinterface inbroadcasts.ts.BatchItem.test.tsx,LastBatchSection.test.tsx, andPastBroadcastsSection.test.tsxto usesuccessfullyDeliveredinstead oftotalReplies.LastBatchSection.tsxand update the corresponding data field.This description was created by
for 3643677. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Tests