LO-041: Dynamic Merge Tag Validation on Campaign Launch#307
LO-041: Dynamic Merge Tag Validation on Campaign Launch#307Anushreer22 wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthrough
ChangesMerge Tag Validation on Campaign Launch
Sequence Diagram(s)sequenceDiagram
participant User
participant CampaignBuilderUI
participant ValidateLaunchAPI
participant get_all_step_merge_tags
participant LeadDB
User->>CampaignBuilderUI: Click "Launch Campaign"
CampaignBuilderUI->>ValidateLaunchAPI: GET /api/v1/campaigns/{id}/validate-launch/
ValidateLaunchAPI->>get_all_step_merge_tags: campaign
get_all_step_merge_tags-->>ValidateLaunchAPI: set of required lead fields
ValidateLaunchAPI->>LeadDB: Query enrolled leads for empty fields
LeadDB-->>ValidateLaunchAPI: missing field → lead ID mapping
alt Missing fields found
ValidateLaunchAPI-->>CampaignBuilderUI: can_launch: false, total_incomplete_leads, missing_fields
CampaignBuilderUI->>User: Show mergeWarningModal with message
User->>CampaignBuilderUI: Click "Launch Anyway" or "Review Leads"
else No missing fields
ValidateLaunchAPI-->>CampaignBuilderUI: can_launch: true
CampaignBuilderUI->>User: Proceed with launch
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/campaign-builder.html (1)
2547-2567:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWire the validation modal into the launch path.
The launch branch still calls
launchCampaign()directly, and the new modal/buttons have no event listeners. As written, users never see the merge-tag warning before launch.🔧 Proposed integration sketch
+ async function validateLaunch() { + if (!campaignId) throw new Error('Save campaign before validation.'); + const res = await fetchWithAuth(`/campaigns/${campaignId}/validate-launch/`, { + method: 'GET', + timeoutMs: 60000, + }); + const data = await res.json().catch(() => ({})); + if (!res.ok) { + throw new Error(data.error || data.detail || 'Failed to validate campaign launch.'); + } + return data; + } + + function showMergeWarning(validation) { + return new Promise(resolve => { + const modal = document.getElementById('mergeWarningModal'); + const msg = document.getElementById('mergeWarningMsg'); + const launchAnywayBtn = document.getElementById('launchAnywayBtn'); + const reviewBtn = document.getElementById('reviewBtn'); + const closeBtn = modal.querySelector('.close'); + + const fields = Object.keys(validation.missing_fields || {}).join(', '); + msg.textContent = `${validation.total_incomplete_leads} leads are missing data for: ${fields}.`; + modal.style.display = 'block'; + + const cleanup = () => { + modal.style.display = 'none'; + launchAnywayBtn.onclick = null; + reviewBtn.onclick = null; + closeBtn.onclick = null; + }; + + launchAnywayBtn.onclick = () => { + cleanup(); + resolve(true); + }; + reviewBtn.onclick = () => { + cleanup(); + switchTab('leads'); + resolve(false); + }; + closeBtn.onclick = () => { + cleanup(); + resolve(false); + }; + }); + } + document.getElementById('next-step-btn').addEventListener('click', async () => { @@ } else { await saveCampaign(); await enrollLeads(); + const validation = await validateLaunch(); + if (!validation.can_launch) { + const proceed = await showMergeWarning(validation); + if (!proceed) return; + } await launchCampaign(); alert('Campaign launched and processing triggered.'); window.location.href = '/campaigns.html'; }Also applies to: 2640-2649
🤖 Prompt for 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. In `@frontend/campaign-builder.html` around lines 2547 - 2567, The launch branch in the next-step-btn event listener currently calls launchCampaign() directly without displaying the merge-tag validation modal to users. Instead of directly calling launchCampaign() in the else clause, show the validation modal first and attach event listeners to the modal's confirmation button to trigger launchCampaign() only after the user acknowledges the warning. This ensures users see the merge-tag warning modal before the campaign is actually launched.
🤖 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 `@backend/campaigns/utils.py`:
- Line 22: The `get_all_step_merge_tags()` function attempts to access campaign
steps using `campaign.sequence_steps.all()`, but this does not match the
related_name defined in the SequenceStep model. Change the attribute name from
`sequence_steps` to `steps` to align with the model's related_name definition
and match the prefetch configuration used in CampaignViewSet.
- Around line 2-25: The functions generate_unsubscribe_token and
verify_unsubscribe_token have been removed from backend/campaigns/utils.py but
are still actively imported and used in backend/campaigns/tests.py,
backend/campaigns/gmail_service.py, and backend/campaigns/views.py, which will
cause ImportError failures. Restore both the generate_unsubscribe_token and
verify_unsubscribe_token functions to backend/campaigns/utils.py (they should be
added after the existing extract_merge_tags and get_all_step_merge_tags
functions) with their complete implementation to resolve the import errors in
the dependent modules.
In `@backend/campaigns/views.py`:
- Around line 666-672: The total_incomplete count is incrementing for each
missing field per lead, which means a lead missing multiple fields is counted
multiple times. To fix this, instead of adding len(missing) to total_incomplete
in each iteration of the loop over lead_fields, use a set to collect all unique
lead IDs that are missing any field across all iterations, then set
total_incomplete to the size of that set at the end. This will ensure each lead
is only counted once regardless of how many fields it's missing.
- Around line 654-683: The validate_launch method is incorrectly nested inside
an unterminated f-string for a CSRF token that needs to be closed. First,
properly close the f-string containing the CSRF token at line 654, then move the
validate_launch method as a proper decorated method of the CampaignViewSet
class. Next, add the missing imports for get_all_step_merge_tags and
MERGE_TAG_FIELD_MAP at the top of the file so these utilities are available when
the method executes. Finally, fix the total_incomplete counting logic by
replacing the current approach of incrementing total_incomplete by len(missing)
for each field with a set-based approach that tracks unique lead IDs across all
fields, ensuring each lead is counted only once even if it's missing data for
multiple merge tags.
---
Outside diff comments:
In `@frontend/campaign-builder.html`:
- Around line 2547-2567: The launch branch in the next-step-btn event listener
currently calls launchCampaign() directly without displaying the merge-tag
validation modal to users. Instead of directly calling launchCampaign() in the
else clause, show the validation modal first and attach event listeners to the
modal's confirmation button to trigger launchCampaign() only after the user
acknowledges the warning. This ensures users see the merge-tag warning modal
before the campaign is actually launched.
🪄 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: 83bfae5c-4906-4015-a34c-792652beeb1e
📒 Files selected for processing (3)
backend/campaigns/utils.pybackend/campaigns/views.pyfrontend/campaign-builder.html
| import re | ||
|
|
||
| def generate_unsubscribe_token(lead_id): | ||
| return signer.sign(str(lead_id)) | ||
| # Mapping from common merge tags to Lead model fields | ||
| MERGE_TAG_FIELD_MAP = { | ||
| 'firstName': 'first_name', | ||
| 'lastName': 'last_name', | ||
| 'company': 'company', | ||
| 'email': 'email', | ||
| 'jobTitle': 'job_title', | ||
| 'phone': 'phone', | ||
| # add more as needed | ||
| } | ||
|
|
||
| def verify_unsubscribe_token(token): | ||
| try: | ||
| return signer.unsign(token) | ||
| except BadSignature: | ||
| return None No newline at end of file | ||
| def extract_merge_tags(text): | ||
| """Return a list of tag names inside {{ }} in text (e.g., ['firstName', 'company']).""" | ||
| return re.findall(r'\{\{\s*(\w+)\s*\}\}', text) | ||
|
|
||
| def get_all_step_merge_tags(campaign): | ||
| """Return set of all merge tag field names used in a campaign's steps.""" | ||
| tags = set() | ||
| for step in campaign.sequence_steps.all(): | ||
| tags.update(extract_merge_tags(step.template_subject or '')) | ||
| tags.update(extract_merge_tags(step.template_body or '')) | ||
| return tags |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm unsubscribe token helper definitions and call sites still line up.
rg -n -C3 '\b(generate_unsubscribe_token|verify_unsubscribe_token)\b|from\s+\.utils\s+import' --type=pyRepository: Kuldeeep18/LeadOrbit
Length of output: 3058
🏁 Script executed:
cat -n backend/campaigns/utils.pyRepository: Kuldeeep18/LeadOrbit
Length of output: 1023
Restore the unsubscribe token helpers to backend/campaigns/utils.py.
The functions generate_unsubscribe_token and verify_unsubscribe_token have been removed from this module, but they are still imported and actively used in three locations:
backend/campaigns/tests.py(line 19)backend/campaigns/gmail_service.py(line 55)backend/campaigns/views.py(line 610)
This will cause immediate ImportError failures when those modules are loaded.
🔧 Proposed fix
import re
+from django.core.signing import BadSignature, Signer
# Mapping from common merge tags to Lead model fields
MERGE_TAG_FIELD_MAP = {
@@
}
+
+
+def generate_unsubscribe_token(lead_id):
+ return Signer().sign(str(lead_id))
+
+
+def verify_unsubscribe_token(token):
+ try:
+ return Signer().unsign(token)
+ except BadSignature:
+ return None🤖 Prompt for 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.
In `@backend/campaigns/utils.py` around lines 2 - 25, The functions
generate_unsubscribe_token and verify_unsubscribe_token have been removed from
backend/campaigns/utils.py but are still actively imported and used in
backend/campaigns/tests.py, backend/campaigns/gmail_service.py, and
backend/campaigns/views.py, which will cause ImportError failures. Restore both
the generate_unsubscribe_token and verify_unsubscribe_token functions to
backend/campaigns/utils.py (they should be added after the existing
extract_merge_tags and get_all_step_merge_tags functions) with their complete
implementation to resolve the import errors in the dependent modules.
| def get_all_step_merge_tags(campaign): | ||
| """Return set of all merge tag field names used in a campaign's steps.""" | ||
| tags = set() | ||
| for step in campaign.sequence_steps.all(): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the SequenceStep/Campaign relationship and any prefetches.
# Expectation: the related manager used by get_all_step_merge_tags matches the model's related_name.
rg -n -C4 'class\s+SequenceStep|ForeignKey\(.*Campaign|related_name\s*=|prefetch_related\(' --type=pyRepository: Kuldeeep18/LeadOrbit
Length of output: 13198
🏁 Script executed:
cat -n backend/campaigns/utils.pyRepository: Kuldeeep18/LeadOrbit
Length of output: 1023
🏁 Script executed:
rg 'sequence_steps|\.steps' backend/campaigns/utils.py --type=pyRepository: Kuldeeep18/LeadOrbit
Length of output: 111
Fix attribute name mismatch in get_all_step_merge_tags().
The function uses campaign.sequence_steps but the SequenceStep model defines related_name='steps'. This will raise AttributeError at runtime. Change campaign.sequence_steps.all() to campaign.steps.all() to match the model's related_name and the prefetch in CampaignViewSet.
🤖 Prompt for 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.
In `@backend/campaigns/utils.py` at line 22, The `get_all_step_merge_tags()`
function attempts to access campaign steps using
`campaign.sequence_steps.all()`, but this does not match the related_name
defined in the SequenceStep model. Change the attribute name from
`sequence_steps` to `steps` to align with the model's related_name definition
and match the prefetch configuration used in CampaignViewSet.
| f'<input type="hidden" name="csrfmiddlewaretoken" value="{csrf_token | ||
| @action(detail=True, methods=['get'], url_path='validate-launch') | ||
| def validate_launch(self, request, pk=None): | ||
| campaign = self.get_object() | ||
| # Get all merge tags used in the campaign | ||
| tags = get_all_step_merge_tags(campaign) # from utils | ||
| # Map to actual lead fields | ||
| lead_fields = [MERGE_TAG_FIELD_MAP.get(t, t) for t in tags] | ||
|
|
||
| # Find enrolled leads missing any of those fields | ||
| enrolled_leads = campaign.enrolled_leads.select_related('lead') | ||
| missing_by_field = {} | ||
| total_incomplete = 0 | ||
| for field in lead_fields: | ||
| missing = [clead.lead.id for clead in enrolled_leads | ||
| if getattr(clead.lead, field, None) in (None, '')] | ||
| if missing: | ||
| missing_by_field[field] = missing | ||
| total_incomplete += len(missing) | ||
|
|
||
| if total_incomplete > 0: | ||
| # Return warning data | ||
| return Response({ | ||
| 'can_launch': False, | ||
| 'total_incomplete_leads': total_incomplete, | ||
| 'missing_fields': missing_by_field, | ||
| 'message': f"{total_incomplete} leads are missing data for merge tags." | ||
| }) | ||
| return Response({'can_launch': True, 'message': 'All merge fields are present.'}) | ||
| }">' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Parse the file with Python's AST module; this reads source only and does not execute it.
python - <<'PY'
from pathlib import Path
import ast
path = Path('backend/campaigns/views.py')
try:
ast.parse(path.read_text(), filename=str(path))
except SyntaxError as exc:
print(f'{path}:{exc.lineno}:{exc.offset}: {exc.msg}')
raise SystemExit(1)
print('syntax ok')
PYRepository: Kuldeeep18/LeadOrbit
Length of output: 150
🏁 Script executed:
# Check the actual content around lines 654-683 to understand the context
sed -n '650,690p' backend/campaigns/views.py | cat -nRepository: Kuldeeep18/LeadOrbit
Length of output: 2172
🏁 Script executed:
# Find imports at the top of the file to check for missing imports
head -50 backend/campaigns/views.py | cat -nRepository: Kuldeeep18/LeadOrbit
Length of output: 2225
🏁 Script executed:
# Search for CampaignViewSet to understand where validate_launch should be placed
rg -n "class CampaignViewSet" backend/campaigns/views.pyRepository: Kuldeeep18/LeadOrbit
Length of output: 113
🏁 Script executed:
# Check if MERGE_TAG_FIELD_MAP and get_all_step_merge_tags are imported or defined
rg -n "MERGE_TAG_FIELD_MAP|get_all_step_merge_tags" backend/campaigns/views.pyRepository: Kuldeeep18/LeadOrbit
Length of output: 203
Move validate_launch out of the unsubscribe form string and close the CSRF f-string; import missing utilities.
The new action is embedded in the middle of an unterminated f-string at line 654, so this file does not parse. After fixing the string, place validate_launch inside CampaignViewSet and add imports for MERGE_TAG_FIELD_MAP and get_all_step_merge_tags (used at lines 659, 661 but not imported), otherwise DRF will not route the action and the code will fail at runtime.
Additionally, the total_incomplete count is incorrect: the loop increments by len(missing) for each field, so a lead missing multiple fields gets counted multiple times. Use a set of unique lead IDs instead.
🧰 Tools
🪛 Ruff (0.15.17)
[warning] 655-655: f-string: unterminated string
(invalid-syntax)
[warning] 656-656: f-string: expecting }
(invalid-syntax)
[warning] 659-659: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 661-661: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 664-664: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 665-665: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 666-666: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 667-667: Compound statements are not allowed on the same line as simple statements
(invalid-syntax)
[warning] 670-670: Expected else, found :
(invalid-syntax)
[warning] 672-672: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 674-674: Expected else, found :
(invalid-syntax)
[warning] 682-682: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 683-683: Expected a statement
(invalid-syntax)
[warning] 683-683: missing closing quote in string literal
(invalid-syntax)
🤖 Prompt for 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.
In `@backend/campaigns/views.py` around lines 654 - 683, The validate_launch
method is incorrectly nested inside an unterminated f-string for a CSRF token
that needs to be closed. First, properly close the f-string containing the CSRF
token at line 654, then move the validate_launch method as a proper decorated
method of the CampaignViewSet class. Next, add the missing imports for
get_all_step_merge_tags and MERGE_TAG_FIELD_MAP at the top of the file so these
utilities are available when the method executes. Finally, fix the
total_incomplete counting logic by replacing the current approach of
incrementing total_incomplete by len(missing) for each field with a set-based
approach that tracks unique lead IDs across all fields, ensuring each lead is
counted only once even if it's missing data for multiple merge tags.
Source: Linters/SAST tools
| total_incomplete = 0 | ||
| for field in lead_fields: | ||
| missing = [clead.lead.id for clead in enrolled_leads | ||
| if getattr(clead.lead, field, None) in (None, '')] | ||
| if missing: | ||
| missing_by_field[field] = missing | ||
| total_incomplete += len(missing) |
There was a problem hiding this comment.
Count unique incomplete leads, not field misses.
A lead missing both firstName and company is counted twice, so total_incomplete_leads and the warning message overstate the number of affected leads.
🔧 Proposed fix
- missing_by_field = {}
- total_incomplete = 0
+ missing_by_field = {}
+ incomplete_lead_ids = set()
+ enrolled_leads = list(enrolled_leads)
for field in lead_fields:
- missing = [clead.lead.id for clead in enrolled_leads
- if getattr(clead.lead, field, None) in (None, '')]
+ missing = []
+ for clead in enrolled_leads:
+ value = getattr(clead.lead, field, None)
+ is_missing = value is None or (isinstance(value, str) and value.strip() == '')
+ if is_missing:
+ missing.append(clead.lead.id)
+ incomplete_lead_ids.add(clead.lead.id)
if missing:
missing_by_field[field] = missing
- total_incomplete += len(missing)
+ total_incomplete = len(incomplete_lead_ids)🧰 Tools
🪛 Ruff (0.15.17)
[warning] 666-666: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 667-667: Compound statements are not allowed on the same line as simple statements
(invalid-syntax)
[warning] 670-670: Expected else, found :
(invalid-syntax)
[warning] 672-672: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
🤖 Prompt for 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.
In `@backend/campaigns/views.py` around lines 666 - 672, The total_incomplete
count is incrementing for each missing field per lead, which means a lead
missing multiple fields is counted multiple times. To fix this, instead of
adding len(missing) to total_incomplete in each iteration of the loop over
lead_fields, use a set to collect all unique lead IDs that are missing any field
across all iterations, then set total_incomplete to the size of that set at the
end. This will ensure each lead is only counted once regardless of how many
fields it's missing.
1 participant
Summary
Closes #187
Implements validation of merge tags before launching a campaign.
/api/v1/campaigns/{id}/validate-launch/checks enrolled leads for missing field values.Changes
campaigns/utils.pywithextract_merge_tags,get_all_step_merge_tags, and field mapping.validate_launchaction to CampaignViewSet (campaigns/views.py).campaign-builder.html.Summary by CodeRabbit