[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 759 additions and 5 deletions
Showing only changes of commit c37c66cbf3 - Show all commits

View File

@@ -64,11 +64,27 @@ export async function deleteFunnel(siteId: string, funnelId: string): Promise<vo
})
}
const DATE_ONLY_REGEX = /^\d{4}-\d{2}-\d{2}$/
/** Normalize date-only (YYYY-MM-DD) to RFC3339 for backend funnel stats API. */
function toRFC3339Range(from: string, to: string): { from: string; to: string } {
return {
from: DATE_ONLY_REGEX.test(from) ? `${from}T00:00:00.000Z` : from,
to: DATE_ONLY_REGEX.test(to) ? `${to}T23:59:59.999Z` : to,
}
greptile-apps[bot] commented 2026-02-04 22:45:43 +00:00 (Migrated from github.com)
Review

Date normalization hardcodes T23:59:59.999Z for end dates, but this may not account for user's timezone. If to is "2024-02-04", this sets end to "2024-02-04T23:59:59.999Z" (UTC), which might exclude data from users in timezones ahead of UTC who accessed the site on Feb 4th local time. Consider if timezone handling is needed or document the UTC assumption.

Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/api/funnels.ts
Line: 72:74

Comment:
Date normalization hardcodes `T23:59:59.999Z` for end dates, but this may not account for user's timezone. If `to` is "2024-02-04", this sets end to "2024-02-04T23:59:59.999Z" (UTC), which might exclude data from users in timezones ahead of UTC who accessed the site on Feb 4th local time. Consider if timezone handling is needed or document the UTC assumption.

How can I resolve this? If you propose a fix, please make it concise.
Date normalization hardcodes `T23:59:59.999Z` for end dates, but this may not account for user's timezone. If `to` is "2024-02-04", this sets end to "2024-02-04T23:59:59.999Z" (UTC), which might exclude data from users in timezones ahead of UTC who accessed the site on Feb 4th local time. Consider if timezone handling is needed or document the UTC assumption. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: lib/api/funnels.ts Line: 72:74 Comment: Date normalization hardcodes `T23:59:59.999Z` for end dates, but this may not account for user's timezone. If `to` is "2024-02-04", this sets end to "2024-02-04T23:59:59.999Z" (UTC), which might exclude data from users in timezones ahead of UTC who accessed the site on Feb 4th local time. Consider if timezone handling is needed or document the UTC assumption. How can I resolve this? If you propose a fix, please make it concise. ````` </details>
uz1mani commented 2026-02-04 22:47:34 +00:00 (Migrated from github.com)
Review

Change: Updated the JSDoc for toRFC3339Range to: Normalize date-only (YYYY-MM-DD) to RFC3339 for backend funnel stats API. Uses UTC for boundaries (API/server timestamps are UTC).
Why: It documents that the T23:59:59.999Z end-of-day is intentional UTC and that the API/server use UTC; no code change for timezone handling.

Change: Updated the JSDoc for toRFC3339Range to: Normalize date-only (YYYY-MM-DD) to RFC3339 for backend funnel stats API. Uses UTC for boundaries (API/server timestamps are UTC). Why: It documents that the T23:59:59.999Z end-of-day is intentional UTC and that the API/server use UTC; no code change for timezone handling.
}
export async function getFunnelStats(siteId: string, funnelId: string, from?: string, to?: string): Promise<FunnelStats> {
const params = new URLSearchParams()
if (from) params.append('from', from)
if (to) params.append('to', to)
if (from && to) {
const { from: fromRfc, to: toRfc } = toRFC3339Range(from, to)
params.append('from', fromRfc)
params.append('to', toRfc)
} else if (from) {
params.append('from', DATE_ONLY_REGEX.test(from) ? `${from}T00:00:00.000Z` : from)
} else if (to) {
params.append('to', DATE_ONLY_REGEX.test(to) ? `${to}T23:59:59.999Z` : to)
}
const queryString = params.toString() ? `?${params.toString()}` : ''
return apiRequest<FunnelStats>(`/sites/${siteId}/funnels/${funnelId}/stats${queryString}`)
}