LO-048: Interactive Campaign Sequence Flow Visualization#308
Open
Anushreer22 wants to merge 1 commit into
Open
LO-048: Interactive Campaign Sequence Flow Visualization#308Anushreer22 wants to merge 1 commit into
Anushreer22 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe campaign builder page gains interactive Mermaid.js flowchart visualization: the Mermaid CDN script and container CSS are added to the ChangesMermaid Flowchart Visualization in Campaign Builder
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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: 4
🤖 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 `@frontend/campaign-builder.html`:
- Around line 2659-2681: The renderFlowchart function has multiple issues that
prevent proper execution. First, wrap the graph string construction in backticks
to create a proper template literal since it uses ${} syntax for variable
interpolution. Second, change the data source from window.campaignSteps to the
module-scoped steps variable that already exists, or alternatively expose the
steps variable as window.campaignSteps around line 1240. Third, update the
condition type check to match the actual step types used in the codebase by
replacing the check for 'condition' or 'CONDITION' with checks for
'CONDITION_OPEN', 'CONDITION_REPLY', or 'CONDITION_CLICK'. Finally, replace the
deprecated mermaid.init() call at the end with mermaid.run() as required by
Mermaid v10+.
- Around line 2683-2689: The openStepDetail function calls an undefined editStep
function that doesn't exist, preventing node clicks from working. Replace the
editStep call in the openStepDetail function with the existing pattern by
setting selectedNodeIndex to the stepIndex value and calling renderEditor() to
display the editor. Additionally, expose selectedNodeIndex, renderCanvas, and
renderEditor to the global window object so they are accessible for use in the
HTML event handlers and other global contexts.
- Around line 954-959: The CSS rules for `#flowchart-container` and .mermaid
elements are defined but the corresponding HTML elements do not exist in the
DOM, and the Mermaid CDN script URL lacks version pinning which risks breaking
changes on updates. Add a missing div container with id="flowchart-container"
inside the canvas-area element (around the canvas area, approximately line
995-997), with a nested child div having id="mermaidFlowchart" and
class="mermaid" to match the CSS selectors. Additionally, pin the Mermaid CDN
script version by specifying a specific version number (e.g., mermaid@10.6.1) in
the src URL instead of using an unpinned path.
- Around line 2645-2658: The mermaid.initialize() call has securityLevel set to
'loose' which allows HTML rendering in node labels, creating an XSS
vulnerability for user-controlled step names. Change securityLevel to 'strict'
or 'antiscript' to prevent this. Additionally, the event listener selectors are
broken: document.getElementById('canvasTab') references a non-existent ID, and
document.getElementById('') uses an empty string. Update these selectors to
properly reference the tab elements using their data-tab attributes instead of
IDs, and ensure all element references match the actual DOM structure where tabs
use data-tab attributes.
🪄 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: ce8091f9-21ff-4231-a4a7-66157e8143d0
📒 Files selected for processing (1)
frontend/campaign-builder.html
Comment on lines
+954
to
+959
| <script src="https://cdn.jsdelivr.net/npm/mermaid/dist/mermaid.min.js"></script> | ||
| <style> | ||
| #flowchart-container { display: none; padding: 15px; border: 1px solid #ccc; margin-top: 10px; } | ||
| .mermaid { text-align: center; } | ||
| .node rect, .node circle, .node polygon { cursor: pointer; } | ||
| </style> |
There was a problem hiding this comment.
Missing flowchart container element and unpinned CDN version.
The CSS references #flowchart-container and .mermaid elements, but neither exists in the HTML. Additionally, the Mermaid CDN script lacks version pinning, which risks breaking changes.
Add the missing container element and pin the version:
🛠️ Suggested fix
-<script src="https://cdn.jsdelivr.net/npm/mermaid/dist/mermaid.min.js"></script>
+<script src="https://cdn.jsdelivr.net/npm/mermaid@10.9.1/dist/mermaid.min.js"></script>Add the container inside the canvas area (around line 995-997):
<div class="canvas-area" id="canvas">
<!-- Rendered dynamically -->
<div id="flowchart-container">
<div id="mermaidFlowchart" class="mermaid"></div>
</div>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <script src="https://cdn.jsdelivr.net/npm/mermaid/dist/mermaid.min.js"></script> | |
| <style> | |
| #flowchart-container { display: none; padding: 15px; border: 1px solid #ccc; margin-top: 10px; } | |
| .mermaid { text-align: center; } | |
| .node rect, .node circle, .node polygon { cursor: pointer; } | |
| </style> | |
| <script src="https://cdn.jsdelivr.net/npm/mermaid@10.9.1/dist/mermaid.min.js"></script> | |
| <style> | |
| `#flowchart-container` { display: none; padding: 15px; border: 1px solid `#ccc`; margin-top: 10px; } | |
| .mermaid { text-align: center; } | |
| .node rect, .node circle, .node polygon { cursor: pointer; } | |
| </style> |
🤖 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 954 - 959, The CSS rules for
`#flowchart-container` and .mermaid elements are defined but the corresponding
HTML elements do not exist in the DOM, and the Mermaid CDN script URL lacks
version pinning which risks breaking changes on updates. Add a missing div
container with id="flowchart-container" inside the canvas-area element (around
the canvas area, approximately line 995-997), with a nested child div having
id="mermaidFlowchart" and class="mermaid" to match the CSS selectors.
Additionally, pin the Mermaid CDN script version by specifying a specific
version number (e.g., mermaid@10.6.1) in the src URL instead of using an
unpinned path.
Comment on lines
+2645
to
+2658
| <script> | ||
| mermaid.initialize({ startOnLoad: true, securityLevel: 'loose' }); | ||
|
|
||
| document.getElementById('canvasTab').addEventListener('click', function() { | ||
| document.getElementById('flowchart-container').style.display = 'block'; | ||
| document.getElementById('').style.display = 'none'; | ||
| renderFlowchart(); | ||
| }); | ||
|
|
||
| document.getElementById('').addEventListener('click', function() { | ||
| document.getElementById('flowchart-container').style.display = 'none'; | ||
| document.getElementById('').style.display = 'block'; | ||
| }); | ||
|
|
There was a problem hiding this comment.
XSS risk from securityLevel: 'loose' and broken element selectors.
- Security:
securityLevel: 'loose'allows HTML in node labels. Since step names/subjects are user-controlled, this enables stored XSS attacks. - Runtime errors:
document.getElementById('canvasTab')anddocument.getElementById('')reference non-existent elements. The existing tabs usedata-tabattributes, not IDs.
🔒 Suggested fix
- mermaid.initialize({ startOnLoad: true, securityLevel: 'loose' });
+ mermaid.initialize({ startOnLoad: false, securityLevel: 'strict' });
- document.getElementById('canvasTab').addEventListener('click', function() {
- document.getElementById('flowchart-container').style.display = 'block';
- document.getElementById('').style.display = 'none';
- renderFlowchart();
- });
-
- document.getElementById('').addEventListener('click', function() {
- document.getElementById('flowchart-container').style.display = 'none';
- document.getElementById('').style.display = 'block';
- });
+ // Hook into existing tab system - add a "Visual Canvas" tab in the tab-pills div
+ // and listen for its activation via the existing switchTab() function🤖 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 2645 - 2658, The
mermaid.initialize() call has securityLevel set to 'loose' which allows HTML
rendering in node labels, creating an XSS vulnerability for user-controlled step
names. Change securityLevel to 'strict' or 'antiscript' to prevent this.
Additionally, the event listener selectors are broken:
document.getElementById('canvasTab') references a non-existent ID, and
document.getElementById('') uses an empty string. Update these selectors to
properly reference the tab elements using their data-tab attributes instead of
IDs, and ensure all element references match the actual DOM structure where tabs
use data-tab attributes.
Comment on lines
+2659
to
+2681
| function renderFlowchart() { | ||
| const steps = window.campaignSteps; | ||
| if (!steps || steps.length === 0) return; | ||
|
|
||
| let graph = 'graph TD\n'; | ||
| steps.forEach((step, i) => { | ||
| const nodeId = step${i}; | ||
| if (step.type === 'condition' || step.type === 'CONDITION') { | ||
| graph += ${nodeId}{${step.name || 'Condition'}}\n; | ||
| } else { | ||
| graph += ${nodeId}[${step.name || 'Action'}]\n; | ||
| } | ||
| if (i < steps.length - 1) { | ||
| graph += ${nodeId} --> step${i+1}\n; | ||
| } | ||
| }); | ||
| steps.forEach((step, i) => { | ||
| graph += click step${i} openStepDetail\n; | ||
| }); | ||
|
|
||
| document.getElementById('mermaidFlowchart').textContent = graph; | ||
| mermaid.init(undefined, document.getElementById('mermaidFlowchart')); | ||
| } |
There was a problem hiding this comment.
Syntax errors and undefined data source break flowchart rendering.
- Syntax errors: Lines use template literal syntax (
${...}) without backticks. This causes a parse error and prevents script execution. - Undefined data source:
window.campaignStepsdoesn't exist. The existing code stores steps in module-scopedstepsvariable (line 1236). - Type mismatch: Condition check uses
'condition'/'CONDITION'but actual types are'CONDITION_OPEN','CONDITION_REPLY','CONDITION_CLICK'. - Deprecated API:
mermaid.init()is deprecated in Mermaid v10+; usemermaid.run().
🐛 Suggested fix
function renderFlowchart() {
- const steps = window.campaignSteps;
+ // Access the steps from the module scope by exposing it
+ const steps = window.campaignSteps; // Requires: window.campaignSteps = steps; in the module
if (!steps || steps.length === 0) return;
let graph = 'graph TD\n';
steps.forEach((step, i) => {
- const nodeId = step${i};
- if (step.type === 'condition' || step.type === 'CONDITION') {
- graph += ${nodeId}{${step.name || 'Condition'}}\n;
+ const nodeId = `step${i}`;
+ const label = (step.subject || step.description || step.type || 'Step').replace(/["\[\]{}]/g, '');
+ if (step.type && step.type.startsWith('CONDITION_')) {
+ graph += ` ${nodeId}{${label}}\n`;
} else {
- graph += ${nodeId}[${step.name || 'Action'}]\n;
+ graph += ` ${nodeId}[${label}]\n`;
}
if (i < steps.length - 1) {
- graph += ${nodeId} --> step${i+1}\n;
+ graph += ` ${nodeId} --> step${i+1}\n`;
}
});
steps.forEach((step, i) => {
- graph += click step${i} openStepDetail\n;
+ graph += ` click step${i} openStepDetail\n`;
});
document.getElementById('mermaidFlowchart').textContent = graph;
- mermaid.init(undefined, document.getElementById('mermaidFlowchart'));
+ mermaid.run({ nodes: [document.getElementById('mermaidFlowchart')] });
}Also expose steps from the module by adding this inside the module script (around line 1240):
window.campaignSteps = steps;🤖 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 2659 - 2681, The renderFlowchart
function has multiple issues that prevent proper execution. First, wrap the
graph string construction in backticks to create a proper template literal since
it uses ${} syntax for variable interpolution. Second, change the data source
from window.campaignSteps to the module-scoped steps variable that already
exists, or alternatively expose the steps variable as window.campaignSteps
around line 1240. Third, update the condition type check to match the actual
step types used in the codebase by replacing the check for 'condition' or
'CONDITION' with checks for 'CONDITION_OPEN', 'CONDITION_REPLY', or
'CONDITION_CLICK'. Finally, replace the deprecated mermaid.init() call at the
end with mermaid.run() as required by Mermaid v10+.
Comment on lines
+2683
to
+2689
| function openStepDetail(stepId) { | ||
| console.log('Clicked node:', stepId); | ||
| const stepIndex = parseInt(stepId.replace('step', '')); | ||
| if (typeof editStep === 'function') { | ||
| editStep(stepIndex); | ||
| } | ||
| } |
There was a problem hiding this comment.
editStep function is undefined; node clicks won't work.
The function calls editStep(stepIndex) but this function doesn't exist. The existing code pattern uses selectedNodeIndex and renderEditor() to edit steps.
🛠️ Suggested fix
function openStepDetail(stepId) {
console.log('Clicked node:', stepId);
const stepIndex = parseInt(stepId.replace('step', ''));
- if (typeof editStep === 'function') {
- editStep(stepIndex);
- }
+ if (!isNaN(stepIndex) && stepIndex >= 0) {
+ // Access module-scoped variables - requires exposing them on window
+ window.selectedNodeIndex = stepIndex;
+ if (typeof window.renderCanvas === 'function') window.renderCanvas();
+ if (typeof window.renderEditor === 'function') window.renderEditor();
+ }
}This also requires exposing selectedNodeIndex, renderCanvas, and renderEditor from the module script:
window.selectedNodeIndex = selectedNodeIndex;
window.renderCanvas = renderCanvas;
window.renderEditor = renderEditor;🤖 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 2683 - 2689, The openStepDetail
function calls an undefined editStep function that doesn't exist, preventing
node clicks from working. Replace the editStep call in the openStepDetail
function with the existing pattern by setting selectedNodeIndex to the stepIndex
value and calling renderEditor() to display the editor. Additionally, expose
selectedNodeIndex, renderCanvas, and renderEditor to the global window object so
they are accessible for use in the HTML event handlers and other global
contexts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #194
Summary by CodeRabbit