ciphera-net/pulse
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 15 Wiki Activity

[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
Conversation 27 Commits 12 Files Changed 7 +803 -5
7 changed files with 803 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit ceb668890b - Show all commits

26
app/sites/[id]/funnels/[funnelId]/page.tsx
View File

@@ -2,6 +2,7 @@
import { useCallback, useEffect, useMemo, useState } from 'react'
greptile-apps[bot] commented 2026-02-04 22:35:37 +00:00 (Migrated from github.com)
Review
Copy Link

Unused auth import

useAuth is imported here but never referenced, which will fail lint/typecheck in projects enforcing no-unused-vars. Remove the import to avoid CI failures.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: app/sites/[id]/funnels/[funnelId]/page.tsx
Line: 1:3

Comment:
**Unused auth import**

`useAuth` is imported here but never referenced, which will fail lint/typecheck in projects enforcing `no-unused-vars`. Remove the import to avoid CI failures.

How can I resolve this? If you propose a fix, please make it concise.
**Unused auth import** `useAuth` is imported here but never referenced, which will fail lint/typecheck in projects enforcing `no-unused-vars`. Remove the import to avoid CI failures. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: app/sites/[id]/funnels/[funnelId]/page.tsx Line: 1:3 Comment: **Unused auth import** `useAuth` is imported here but never referenced, which will fail lint/typecheck in projects enforcing `no-unused-vars`. Remove the import to avoid CI failures. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-04 22:38:27 +00:00 (Migrated from github.com)
Review
Copy Link

Already fixed.

Already fixed.
import { useParams, useRouter } from 'next/navigation'
import { ApiError } from '@/lib/api/client'
import { getFunnel, getFunnelStats, deleteFunnel, type Funnel, type FunnelStats } from '@/lib/api/funnels'
import { toast, LoadingOverlay, Select, DatePicker, ChevronLeftIcon, ArrowRightIcon, TrashIcon, useTheme } from '@ciphera-net/ui'
import Link from 'next/link'
@@ -45,8 +46,10 @@ export default function FunnelReportPage() {
const [dateRange, setDateRange] = useState(getDateRange(30))
const [datePreset, setDatePreset] = useState<'7' | '30' | 'custom'>('30')
const [isDatePickerOpen, setIsDatePickerOpen] = useState(false)
const [loadError, setLoadError] = useState<'not_found' | 'error' | null>(null)
const loadData = useCallback(async () => {
setLoadError(null)
try {
setLoading(true)
const [funnelData, statsData] = await Promise.all([
@@ -56,7 +59,9 @@ export default function FunnelReportPage() {
setFunnel(funnelData)
setStats(statsData)
} catch (error) {
toast.error('Failed to load funnel data')
const is404 = error instanceof ApiError && error.status === 404
setLoadError(is404 ? 'not_found' : 'error')
if (!is404) toast.error('Failed to load funnel data')
} finally {
setLoading(false)
}
@@ -88,6 +93,25 @@ export default function FunnelReportPage() {
return <LoadingOverlay logoSrc="/pulse_icon_no_margins.png" title="Pulse" />
}
if (loadError === 'not_found' || (!funnel && !stats && !loadError)) {
return (
<div className="w-full max-w-6xl mx-auto px-4 sm:px-6 py-8">
greptile-apps[bot] commented 2026-02-04 23:08:05 +00:00 (Migrated from github.com)
Review
Copy Link

Shows "Funnel not found" for all errors (404, 403, 500, network failures). For transient errors, this suggests the funnel doesn't exist when it might just be a temporary issue. Consider tracking error type separately to show appropriate messages (e.g., "Unable to load funnel" for 500/network, "Access denied" for 403).

Prompt To Fix With AI 
This is a comment left during a code review.
Path: app/sites/[id]/funnels/[funnelId]/page.tsx
Line: 91:96

Comment:
Shows "Funnel not found" for all errors (404, 403, 500, network failures). For transient errors, this suggests the funnel doesn't exist when it might just be a temporary issue. Consider tracking error type separately to show appropriate messages (e.g., "Unable to load funnel" for 500/network, "Access denied" for 403).

How can I resolve this? If you propose a fix, please make it concise.
Shows "Funnel not found" for all errors (404, 403, 500, network failures). For transient errors, this suggests the funnel doesn't exist when it might just be a temporary issue. Consider tracking error type separately to show appropriate messages (e.g., "Unable to load funnel" for 500/network, "Access denied" for 403). <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: app/sites/[id]/funnels/[funnelId]/page.tsx Line: 91:96 Comment: Shows "Funnel not found" for all errors (404, 403, 500, network failures). For transient errors, this suggests the funnel doesn't exist when it might just be a temporary issue. Consider tracking error type separately to show appropriate messages (e.g., "Unable to load funnel" for 500/network, "Access denied" for 403). How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-04 23:09:39 +00:00 (Migrated from github.com)
Review
Copy Link

Change:
loadError is now 'not_found' | 'forbidden' | 'error' | null.
In loadData catch: status === 404 → setLoadError('not_found'); status === 403 → setLoadError('forbidden'); otherwise → setLoadError('error'). Toast only for non-404, non-403.
Render:
loadError === 'not_found' (or fallback) → "Funnel not found" (unchanged).
loadError === 'forbidden' → "Access denied" and a "Back to Funnels" link.
loadError === 'error' → "Unable to load funnel" and "Try again" button that calls loadData().
Why: 404 stays “Funnel not found”, 403 shows “Access denied” with a way back, and 500/network show “Unable to load funnel” with retry so transient errors are no longer treated as “not found”.

Change: loadError is now 'not_found' | 'forbidden' | 'error' | null. In loadData catch: status === 404 → setLoadError('not_found'); status === 403 → setLoadError('forbidden'); otherwise → setLoadError('error'). Toast only for non-404, non-403. Render: loadError === 'not_found' (or fallback) → "Funnel not found" (unchanged). loadError === 'forbidden' → "Access denied" and a "Back to Funnels" link. loadError === 'error' → "Unable to load funnel" and "Try again" button that calls loadData(). Why: 404 stays “Funnel not found”, 403 shows “Access denied” with a way back, and 500/network show “Unable to load funnel” with retry so transient errors are no longer treated as “not found”.
<p className="text-neutral-600 dark:text-neutral-400">Funnel not found</p>
</div>
)
}
if (loadError === 'error') {
return (
<div className="w-full max-w-6xl mx-auto px-4 sm:px-6 py-8">
<p className="text-neutral-600 dark:text-neutral-400 mb-4">Failed to load funnel data</p>
<button type="button" onClick={() => loadData()} className="btn-primary">
Try again
</button>
</div>
)
}
if (!funnel || !stats) {
return (
<div className="w-full max-w-6xl mx-auto px-4 sm:px-6 py-8">

5
app/sites/[id]/funnels/new/page.tsx
View File

@@ -53,6 +53,11 @@ export default function CreateFunnelPage() {
return
}
if (steps.some(s => !s.name.trim())) {
toast.error('Please enter a name for all steps')
return
}
greptile-apps[bot] commented 2026-02-04 23:08:04 +00:00 (Migrated from github.com)
Review
Copy Link

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
Copy Link

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.value.trim())) {
greptile-apps[bot] commented 2026-02-04 22:45:41 +00:00 (Migrated from github.com)
Review
Copy Link

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
Copy Link

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('Please enter a path for all steps')
return