feat: add spintax support for email templates#276
Conversation
|
Thank you for your contribution! We love the addition of the Spintax support, but we noticed a few critical issues during review that need to be fixed before we can merge this PR:
Next Steps: Once the PR is cleaned up, we will be happy to review it again! |
📝 WalkthroughWalkthrough
ChangesSpintax parser and email dispatch integration
Leads page filter removal and CSV upload fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/leads.html (1)
198-201:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove stale filter wiring that now references missing UI/functions.
After filter-panel removal, this file still mounts filter UI/event logic (
loadTags,#filter-panel,applyFilters,clearFilters,fetchLeadsWithFilters). If those symbols/elements are gone, Line 610+ can throw and stop remaining page initialization.Suggested fix aligned with “filter removal” scope
- <button id="filter-toggle-btn" class="btn btn-sm btn-outline-secondary btn-filter-toggle" aria-label="Toggle filters" title="Filters"> - <i class="bi bi-funnel me-1" aria-hidden="true"></i>Filters - <span class="filter-dot" id="filter-active-dot" aria-hidden="true"></span> - </button> ... - // Load tags for the filter panel - await loadTags(); ... - // Filter panel toggle - const filterPanel = document.getElementById('filter-panel'); - document.getElementById('filter-toggle-btn').addEventListener('click', () => { - const isHidden = filterPanel.style.display === 'none'; - filterPanel.style.display = isHidden ? 'block' : 'none'; - }); - - // Apply / Clear filter buttons - document.getElementById('apply-filters-btn').addEventListener('click', applyFilters); - document.getElementById('clear-filters-btn').addEventListener('click', async () => { - clearFilters(); - try { - const leads = await fetchLeadsWithFilters({}); - allLeads = leads; - renderStats(leads); - renderLeadRows(leads); - } catch (e) { - console.error('Error clearing filters:', e); - } - });Also applies to: 610-625
🤖 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/leads.html` around lines 198 - 201, Remove stale filter initialization and event handling code that references missing UI elements and functions. Locate the filter setup code around lines 610-625 in frontend/leads.html that contains event listeners and initialization for symbols like loadTags, applyFilters, clearFilters, fetchLeadsWithFilters, and references to the `#filter-panel` element. Remove or comment out all of this filter-related wiring code since the filter-panel UI component has been removed. This will prevent JavaScript errors that could interrupt page initialization and other functionality.
🤖 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`:
- Around line 23-33: The lambda function in the re.sub call splits on `|`
without filtering empty strings, allowing patterns like `{|}` or `{Hi||Hello}`
to include blank options that could be randomly selected. Modify the lambda
function to filter out empty strings from the result of split("|") before
passing it to random.choice, ensuring only non-empty choices are available for
selection.
In `@frontend/leads.html`:
- Around line 244-247: The table markup in frontend/leads.html has malformed
structure with orphan closing tags and unclosed rows. At lines 244-247, verify
that the closing tags (</td>, </tr>, </tbody>) have corresponding opening tags
in the preceding structure; if the opening tags are missing or mismatched,
remove or adjust the orphan closing tags accordingly. At lines 299-302, ensure
the loading row is properly closed with the necessary closing tag (likely </tr>)
before the </table> closing tag to complete the DOM structure. Verify the
complete table hierarchy (table > tbody > tr > td) is balanced throughout the
entire table to ensure proper rendering.
---
Outside diff comments:
In `@frontend/leads.html`:
- Around line 198-201: Remove stale filter initialization and event handling
code that references missing UI elements and functions. Locate the filter setup
code around lines 610-625 in frontend/leads.html that contains event listeners
and initialization for symbols like loadTags, applyFilters, clearFilters,
fetchLeadsWithFilters, and references to the `#filter-panel` element. Remove or
comment out all of this filter-related wiring code since the filter-panel UI
component has been removed. This will prevent JavaScript errors that could
interrupt page initialization and other functionality.
🪄 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: c9dbd6af-68e0-4e07-bd61-6e3f48d85572
📒 Files selected for processing (4)
backend/campaigns/tasks.pybackend/campaigns/utils.pyfrontend/campaign-builder.htmlfrontend/leads.html
| pattern = r"\{([^{}]+)\}" | ||
|
|
||
| while re.search(pattern, text): | ||
| text = re.sub( | ||
| pattern, | ||
| lambda match: random.choice( | ||
| match.group(1).split("|") | ||
| ), | ||
| text, | ||
| count=1, | ||
| ) |
There was a problem hiding this comment.
Empty choices in spintax patterns may select blank strings.
The current implementation splits on | without filtering empty strings. Patterns like {|} or {Hi||Hello} will include empty strings in the choice list, which could result in blank content being randomly selected.
🛡️ Proposed fix to filter empty choices
pattern = r"\{([^{}]+)\}"
while re.search(pattern, text):
text = re.sub(
pattern,
- lambda match: random.choice(
- match.group(1).split("|")
- ),
+ lambda match: random.choice(
+ [opt for opt in match.group(1).split("|") if opt.strip()]
+ ) if any(opt.strip() for opt in match.group(1).split("|")) else "",
text,
count=1,
)Note regarding static analysis warnings:
The static analysis tools flag random.choice as unsuitable for cryptographic purposes (S311, CWE-330). This is a false positive—the function generates email content variations for deliverability purposes, not cryptographic tokens or security-sensitive values. The random module is appropriate here; secrets would be unnecessary.
🧰 Tools
🪛 ast-grep (0.43.0)
[info] 27-29: use secrets package over random package
Context: random.choice(
match.group(1).split("|")
)
Note: [CWE-330].
(avoid-random-python)
🪛 Ruff (0.15.17)
[error] 28-30: Standard pseudo-random generators are not suitable for cryptographic purposes
(S311)
🤖 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 23 - 33, The lambda function in the
re.sub call splits on `|` without filtering empty strings, allowing patterns
like `{|}` or `{Hi||Hello}` to include blank options that could be randomly
selected. Modify the lambda function to filter out empty strings from the result
of split("|") before passing it to random.choice, ensuring only non-empty
choices are available for selection.
Source: Linters/SAST tools
| </td> | ||
| </tr> | ||
| </tbody> | ||
|
|
There was a problem hiding this comment.
Fix malformed table markup (orphan and missing closing tags).
Line 244–Line 246 close tags that were never opened in this visible structure, and Line 299–Line 302 leaves the loading row unclosed before </table>. This can break table rendering and DOM structure.
Suggested fix
- </td>
- </tr>
- </tbody>
-
+
...
<tbody id="leads-table-body">
<tr>
<td colspan="7" class="text-center py-4 text-muted">
<div class="spinner-border spinner-border-sm text-primary me-2" role="status" aria-label="Loading leads"></div>
Loading leads...
+ </td>
+ </tr>
+ </tbody>
</table>Also applies to: 299-302
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 244-244: Tag must be paired, no start tag: [ ]
(tag-pair)
[error] 245-245: Tag must be paired, no start tag: [ ]
(tag-pair)
[error] 246-246: Tag must be paired, no start tag: [ ]
(tag-pair)
🤖 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/leads.html` around lines 244 - 247, The table markup in
frontend/leads.html has malformed structure with orphan closing tags and
unclosed rows. At lines 244-247, verify that the closing tags (</td>, </tr>,
</tbody>) have corresponding opening tags in the preceding structure; if the
opening tags are missing or mismatched, remove or adjust the orphan closing tags
accordingly. At lines 299-302, ensure the loading row is properly closed with
the necessary closing tag (likely </tr>) before the </table> closing tag to
complete the DOM structure. Verify the complete table hierarchy (table > tbody >
tr > td) is balanced throughout the entire table to ensure proper rendering.
Source: Linters/SAST tools
🚀 PR Guidelines — Read Before Raising a PRHey contributors 👋 I’m LeadOrbit's Bot, and I’ll review every PR before it gets merged. ✅ Your PR will only be merged if:
❌ PRs that may be rejected:
Before submitting:
And if you find the project useful, consider ⭐ starring the repository — it helps the project grow and motivates further development. Quality contributions > PR count. |
2 participants
Summary
Implemented Spintax support for email templates to improve email variation and deliverability.
Closes #122
Changes Made
parse_spintax()utility to randomly select options from spintax expressions.Files Modified
backend/campaigns/utils.pybackend/campaigns/tasks.pyfrontend/campaign-builder.htmlTesting
Summary by CodeRabbit
New Features
Changes