[PULSE-36] Funnels UI - builder and report #8

Merged
uz1mani merged 12 commits from staging into main 2026-02-04 23:16:04 +00:00
7 changed files with 774 additions and 5 deletions
Showing only changes of commit 0c09c5b97e - Show all commits

View File

@@ -57,8 +57,9 @@ export default function CreateFunnelPage() {
toast.error('Please enter a path for all steps')
return
}
greptile-apps[bot] commented 2026-02-04 23:08:04 +00:00 (Migrated from github.com)
Review

Validates step.value but not step.name. Empty/whitespace step names will render as blank labels throughout the UI (list page line 121, report page line 246).

    if (steps.some(s => !s.value.trim())) {
      toast.error('Please enter a path for all steps')
      return
    }
    
    if (steps.some(s => !s.name.trim())) {
      toast.error('Please enter a name for all steps')
      return
    }
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/sites/[id]/funnels/new/page.tsx
Line: 56:59

Comment:
Validates `step.value` but not `step.name`. Empty/whitespace step names will render as blank labels throughout the UI (list page line 121, report page line 246).

```suggestion
    if (steps.some(s => !s.value.trim())) {
      toast.error('Please enter a path for all steps')
      return
    }
    
    if (steps.some(s => !s.name.trim())) {
      toast.error('Please enter a name for all steps')
      return
    }
```

How can I resolve this? If you propose a fix, please make it concise.
Validates `step.value` but not `step.name`. Empty/whitespace step names will render as blank labels throughout the UI (list page line 121, report page line 246). ```suggestion if (steps.some(s => !s.value.trim())) { toast.error('Please enter a path for all steps') return } if (steps.some(s => !s.name.trim())) { toast.error('Please enter a name for all steps') return } ``` <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: app/sites/[id]/funnels/new/page.tsx Line: 56:59 Comment: Validates `step.value` but not `step.name`. Empty/whitespace step names will render as blank labels throughout the UI (list page line 121, report page line 246). ```suggestion if (steps.some(s => !s.value.trim())) { toast.error('Please enter a path for all steps') return } if (steps.some(s => !s.name.trim())) { toast.error('Please enter a name for all steps') return } ``` How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-04 23:09:55 +00:00 (Migrated from github.com)
Review

Status: Already implemented.
handleSubmit already has if (steps.some(s => !s.name.trim())) { toast.error('Please enter a name for all steps'); return } (lines 56–59), before the path check.

Status: Already implemented. handleSubmit already has if (steps.some(s => !s.name.trim())) { toast.error('Please enter a name for all steps'); return } (lines 56–59), before the path check.
if (steps.some(s => s.type === 'regex' && !isValidRegex(s.value))) {
toast.error('Invalid regex in one or more steps. Check the pattern for steps with type "regex".')
const invalidRegexStep = steps.find(s => s.type === 'regex' && !isValidRegex(s.value))
if (invalidRegexStep) {
greptile-apps[bot] commented 2026-02-04 22:45:41 +00:00 (Migrated from github.com)
Review

Step validation only checks if value is empty but doesn't validate regex syntax when type === 'regex'. Invalid regex will cause backend errors or runtime issues when matching. Consider adding client-side regex validation:

    if (steps.some(s => !s.value.trim() || (s.type === 'regex' && !isValidRegex(s.value)))) {

Add a helper:

function isValidRegex(pattern: string): boolean {
  try { new RegExp(pattern); return true } catch { return false }
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/sites/[id]/funnels/new/page.tsx
Line: 46:46

Comment:
Step validation only checks if `value` is empty but doesn't validate regex syntax when `type === 'regex'`. Invalid regex will cause backend errors or runtime issues when matching. Consider adding client-side regex validation:

```suggestion
    if (steps.some(s => !s.value.trim() || (s.type === 'regex' && !isValidRegex(s.value)))) {
```

Add a helper:
```typescript
function isValidRegex(pattern: string): boolean {
  try { new RegExp(pattern); return true } catch { return false }
}
```

How can I resolve this? If you propose a fix, please make it concise.
Step validation only checks if `value` is empty but doesn't validate regex syntax when `type === 'regex'`. Invalid regex will cause backend errors or runtime issues when matching. Consider adding client-side regex validation: ```suggestion if (steps.some(s => !s.value.trim() || (s.type === 'regex' && !isValidRegex(s.value)))) { ``` Add a helper: ```typescript function isValidRegex(pattern: string): boolean { try { new RegExp(pattern); return true } catch { return false } } ``` <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: app/sites/[id]/funnels/new/page.tsx Line: 46:46 Comment: Step validation only checks if `value` is empty but doesn't validate regex syntax when `type === 'regex'`. Invalid regex will cause backend errors or runtime issues when matching. Consider adding client-side regex validation: ```suggestion if (steps.some(s => !s.value.trim() || (s.type === 'regex' && !isValidRegex(s.value)))) { ``` Add a helper: ```typescript function isValidRegex(pattern: string): boolean { try { new RegExp(pattern); return true } catch { return false } } ``` How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-04 22:47:56 +00:00 (Migrated from github.com)
Review

Change:
Added isValidRegex(pattern: string): boolean that does try { new RegExp(pattern); return true } catch { return false }.
After the “path for all steps” check, added a check: steps.some(s => s.type === 'regex' && !isValidRegex(s.value)). If true, show toast.error('Invalid regex in one or more steps. Check the pattern for steps with type "regex".') and return without submitting.
Why: Invalid regex for type === 'regex' is caught on the client so the user gets a clear error instead of a backend/runtime failure.

Change: Added isValidRegex(pattern: string): boolean that does try { new RegExp(pattern); return true } catch { return false }. After the “path for all steps” check, added a check: steps.some(s => s.type === 'regex' && !isValidRegex(s.value)). If true, show toast.error('Invalid regex in one or more steps. Check the pattern for steps with type "regex".') and return without submitting. Why: Invalid regex for type === 'regex' is caught on the client so the user gets a clear error instead of a backend/runtime failure.
toast.error(`Invalid regex pattern in step: ${invalidRegexStep.name}`)
return
}