A campaign might be linked to a list of labels#43
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CampaignPage
participant CampaignAPI
User->>CampaignPage: Load Campaigns
CampaignPage->>CampaignAPI: Fetch campaigns (with labelIds/labelNames arrays)
CampaignAPI-->>CampaignPage: Return campaigns (labelIds[], campaignLabelNames[])
CampaignPage->>CampaignPage: Map labelIds to label names
CampaignPage->>User: Render campaign cards with multiple labels (icons, styling)
Possibly related PRs
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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.
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!
Summary of Changes
This pull request modifies the campaign data structure and related components to support multiple labels per campaign. Previously, a campaign could only be associated with a single label. The changes involve updating the Campaign interface to use arrays for labelIds and campaignLabelNames, and modifying the CampaignHistory and UpcomingCampaigns components to handle the display of multiple labels.
Highlights
- Data Structure Update: The
Campaigninterface is updated to uselabelIds?: string[]andcampaignLabelNames?: string[]to support multiple labels per campaign. - CampaignHistory Component: The
CampaignHistorycomponent is modified to display multiple labels associated with a campaign, using a flex-wrap layout for the labels. - UpcomingCampaigns Component: The
UpcomingCampaignscomponent is updated to handle multiple labels when initializing theeditedCampaignsstate and when rendering the labels in the UI.
Changelog
- src/apis/campaigns.ts
- Updated the
Campaigninterface to uselabelIds?: string[]andcampaignLabelNames?: string[]instead of singlelabelIdandcampaignLabelName.
- Updated the
- src/pages/campaign/CampaignHistory.tsx
- Modified the component to map over
campaign.labelIdsto find the corresponding label names. - Updated the UI to display multiple labels using a flex-wrap layout.
- Modified the component to map over
- src/pages/campaign/UpcomingCampaigns.tsx
- Modified the component to handle multiple labels when initializing the
editedCampaignsstate. - Updated the UI to display multiple labels.
- Modified the component to handle multiple labels when initializing the
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 is currently in preview and 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 to provide feedback.
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.
A label's gentle touch,
Now many, meaning so much,
Campaigns bloom bright.
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
The pull request introduces the ability to link campaigns to a list of labels, enhancing campaign organization and management. The changes involve modifying the campaign data structure and updating the UI components to handle multiple labels. Overall, the code is well-structured and addresses the intended functionality. However, there are a few areas that could be improved for better clarity and efficiency.
Summary of Findings
- Performance Optimization: Consider using a
Mapfor faster label lookups inCampaignHistory.tsxandUpcomingCampaigns.tsx, especially ifmissiveLabelsis large. - Code Readability: Add comments to explain the purpose of
filter(Boolean)and the conditioncampaign.labelIds && campaign.labelIds.length > 0for better code understanding. - Code Reusability: Extract the repeated code block for rendering campaign labels into a separate component or function to improve maintainability and reduce redundancy.
Merge Readiness
The pull request is well-structured and addresses the intended functionality. However, addressing the performance optimization and code readability suggestions would improve the overall quality of the code. I am unable to approve this pull request, and recommend that others review and approve this code before merging. I recommend addressing the performance and readability suggestions before merging.
| {editedCampaign.campaignLabelNames && editedCampaign.campaignLabelNames.length > 0 && ( | ||
| <> | ||
| <Tag className="h-3 w-3 ml-2 mr-1" /> | ||
| {editedCampaign.campaignLabelName} | ||
| {editedCampaign.campaignLabelNames.map((labelName, idx) => ( | ||
| <span key={idx} className="ml-2 inline-flex items-center bg-muted rounded px-1.5 py-0.5 text-xs"> | ||
| <Tag className="h-3 w-3 mr-1" /> | ||
| {labelName} | ||
| </span> | ||
| ))} | ||
| </> |
There was a problem hiding this comment.
Consider extracting this repeated code block into a separate component or function to improve maintainability and reduce redundancy. This will make the code easier to update and test in the future.
{renderCampaignLabels(editedCampaign.campaignLabelNames)}![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 5f72763 in 1 minute and 30 seconds. Click for details.
- Reviewed
105lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3draft 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/campaigns.ts:27
- Draft comment:
The Campaign interface now uses multiple labels, but CampaignPayload is still using a single label field. Consider updating CampaignPayload for consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. src/pages/campaign/UpcomingCampaigns.tsx:95
- Draft comment:
Similar to CampaignHistory, consider caching missiveLabels in a dictionary to avoid repeated linear searches when mapping labelIds to label names. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The suggestion is valid - using Array.find() inside a loop is O(n^2) when it could be O(n) with a Map. However, this code only runs once during initialization, not on every render. The number of labels and campaigns is likely small enough that the performance impact is negligible. The suggestion is technically correct but may be premature optimization. I may be underestimating the scale - if there are hundreds of campaigns and labels, the performance impact could be noticeable during page load. The optimization is also very simple to implement. While the optimization is easy to implement, without evidence of actual performance issues, this feels like premature optimization that adds complexity for minimal benefit. The code is more readable in its current form. The comment should be deleted. While technically correct, it suggests an optimization that likely provides minimal benefit in practice and would make the code slightly more complex.
3. src/pages/campaign/UpcomingCampaigns.tsx:436
- Draft comment:
Using index as key for label spans. Consider using a unique identifier if label order can change. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_MXa3MSqsWZhkybhv
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: 0
🔭 Outside diff range comments (2)
src/apis/campaigns.ts (2)
58-59:⚠️ Potential issueUpdate CampaignPayload to support multiple labels.
While the
Campaigninterface was updated to support multiple labels withlabelIdsandcampaignLabelNamesas arrays, theCampaignPayloadinterface still only supports a single label withcampaignLabelNameas a string. This could cause issues when creating or updating campaigns with multiple labels.export interface CampaignPayload { title?: string; firstMessage: string; secondMessage?: string | null; segments?: { included: Array<Segment | Segment[]>; excluded?: Array<Segment | Segment[]> | null; }; delay?: number; runAt: number; - campaignLabelName?: string; + campaignLabelNames?: string[]; + labelIds?: string[]; }
66-66:⚠️ Potential issueUpdate CreateCampaignFormData to support multiple labels.
The
CreateCampaignFormDatainterface needs to be updated to handle multiple campaign labels.export interface CreateCampaignFormData extends FormData { // This is just for TypeScript to understand what fields might be in the FormData - append(name: 'file' | 'title' | 'firstMessage' | 'secondMessage' | 'delay' | 'runAt' | 'campaignLabelName', value: string | Blob): void; + append(name: 'file' | 'title' | 'firstMessage' | 'secondMessage' | 'delay' | 'runAt' | 'campaignLabelNames' | 'labelIds', value: string | Blob): void; }
🧹 Nitpick comments (1)
src/pages/campaign/UpcomingCampaigns.tsx (1)
92-104: Consider refactoring duplicate label mapping logic.The label mapping logic is duplicated between
CampaignHistory.tsxandUpcomingCampaigns.tsx. Consider extracting this into a shared utility function or custom hook to maintain DRY principles.// Create a new file: src/utils/campaignLabels.ts export const mapLabelIdsToNames = (campaign: Campaign, missiveLabels: { id: string; name: string; parent_id: string }[]) => { if (campaign.labelIds && campaign.labelIds.length > 0) { const labelNames = campaign.labelIds .map(labelId => { const label = missiveLabels.find(l => l.id === labelId); return label?.name; }) .filter(Boolean) as string[]; return { ...campaign, campaignLabelNames: labelNames }; } return campaign; };Then in each component:
// In both files where the label mapping occurs: import { mapLabelIdsToNames } from '@/utils/campaignLabels'; // Then replace the mapping logic: const updatedCampaigns = campaigns.map(campaign => mapLabelIdsToNames(campaign, missiveLabels) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/apis/campaigns.ts(1 hunks)src/pages/campaign/CampaignHistory.tsx(2 hunks)src/pages/campaign/UpcomingCampaigns.tsx(2 hunks)
🔇 Additional comments (5)
src/apis/campaigns.ts (1)
27-28: Good implementation of multiple campaign labels.The changes to make
campaignLabelNamesandlabelIdsas arrays allow for multiple labels per campaign, which matches the PR objective.src/pages/campaign/CampaignHistory.tsx (2)
52-64: Well implemented multiple label name mapping.This code correctly maps multiple label IDs to their corresponding names, with proper error handling and filtering of null values.
178-186: Good implementation of multiple label UI display.The UI has been well-updated to display multiple labels with proper styling. Using flex-wrap and gap styling ensures good layout with any number of labels.
src/pages/campaign/UpcomingCampaigns.tsx (2)
95-104: Multiple label mapping implementation looks good.This code correctly handles multiple label IDs, fetching their names and storing them in the campaign object.
437-445: Good implementation of multiple label UI in campaign card header.The UI has been properly updated to render multiple labels in the campaign card header.
🧰 Tools
🪛 Biome (1.9.4)
[error] 437-444: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 8d403a0 in 1 minute and 25 seconds. Click for details.
- Reviewed
59lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft 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/pages/campaign/CampaignHistory.tsx:51
- Draft comment:
Good performance improvement using a lookup object. Consider extracting the label lookup creation to a shared helper to reduce duplication with UpcomingCampaigns. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While code reuse is generally good, I can't verify the duplication claim since I can't see UpcomingCampaigns.tsx. The comment requires cross-file context which violates the rules. Even if there is duplication, this is a relatively simple 5-line helper that may not warrant extraction depending on the full context. I might be overly strict - code duplication is a real issue and extracting shared code is a good practice. The comment seems well-intentioned. However, the rules explicitly state to ignore cross-file issues and to only keep comments when we have strong evidence. We can't verify the duplication claim without seeing other files. Delete the comment since it requires cross-file context that we can't verify, even though the suggestion itself may be valid.
2. src/pages/campaign/UpcomingCampaigns.tsx:92
- Draft comment:
Similar lookup logic is introduced here. Extracting this into a common utility function could reduce duplication and ease future maintenance. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% 1. The code is straightforward and contained within a single function. 2. I don't see evidence of duplication in the provided file. 3. The comment is speculative about future maintenance benefits. 4. Without seeing other files or evidence of duplication, extracting this into a utility seems premature. 5. The comment isn't pointing out a clear problem that needs fixing. I could be missing other files in the codebase where this logic is duplicated. The suggestion to extract reusable utilities is generally good practice. While utility extraction can be good practice, comments should point to concrete issues. Without evidence of actual duplication, this comment is speculative and not actionable enough. Delete this comment. It's speculative about potential future benefits without evidence of current duplication or concrete problems to solve.
Workflow ID: wflow_r8eEC80nRX6ozJp3
You can customize
1 participant
Important
Support for multiple labels per campaign added, updating
Campaigninterface and UI components to handle and display these labels inCampaignHistory.tsxandUpcomingCampaigns.tsx.Campaigninterface incampaigns.tsupdated to support multiple labels withcampaignLabelNamesandlabelIdsarrays.CampaignHistory.tsxandUpcomingCampaigns.tsxupdated to fetch, map, and display multiple labels for each campaign.CampaignHistory.tsxandUpcomingCampaigns.tsxto handle multiple labels with improved layout.CampaignHistory.tsxandUpcomingCampaigns.tsx.labelLookupto maplabelIdstocampaignLabelNames.This description was created by
for 8d403a0. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Style