Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions client/src/api/analytics/endpoints/goals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { GetSessionsResponse } from "./sessions";
export interface Goal {
goalId: number;
name: string | null;
goalType: "path" | "event";
goalType: "path" | "event" | "outbound";
config: {
pathPattern?: string;
eventName?: string;
Expand All @@ -18,6 +18,7 @@ export interface Goal {
key: string;
value: string | number | boolean;
}>;
outboundUrlPattern?: string;
};
createdAt: string;
total_conversions: number;
Expand Down Expand Up @@ -51,7 +52,7 @@ export interface GoalSessionsParams extends CommonApiParams, PaginationParams {

export interface CreateGoalParams {
name?: string;
goalType: "path" | "event";
goalType: "path" | "event" | "outbound";
config: {
pathPattern?: string;
eventName?: string;
Expand Down
3 changes: 2 additions & 1 deletion client/src/api/analytics/hooks/goals/useCreateGoal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import { createGoal } from "../../endpoints";
export interface CreateGoalRequest {
siteId: number;
name?: string;
goalType: "path" | "event";
goalType: "path" | "event" | "outbound";
config: {
pathPattern?: string;
eventName?: string;
eventPropertyKey?: string;
eventPropertyValue?: string | number | boolean;
outboundUrlPattern?: string;
};
}

Expand Down
3 changes: 2 additions & 1 deletion client/src/api/analytics/hooks/goals/useUpdateGoal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export interface UpdateGoalRequest {
goalId: number;
siteId: number;
name?: string;
goalType: "path" | "event";
goalType: "path" | "event" | "outbound";
config: {
pathPattern?: string;
eventName?: string;
Expand All @@ -16,6 +16,7 @@ export interface UpdateGoalRequest {
key: string;
value: string | number | boolean;
}>;
outboundUrlPattern?: string;
};
}

Expand Down
19 changes: 16 additions & 3 deletions client/src/app/[site]/goals/components/GoalCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useState } from "react";
import { useDeleteGoal } from "../../../../api/analytics/hooks/goals/useDeleteGoal";
import { Goal } from "../../../../api/analytics/endpoints";
import { useGetGoalSessions } from "../../../../api/analytics/hooks/goals/useGetGoalSessions";
import { EventIcon, PageviewIcon } from "../../../../components/EventIcons";
import { EventIcon, OutboundIcon, PageviewIcon } from "../../../../components/EventIcons";
import { SessionsList } from "../../../../components/Sessions/SessionsList";
import {
AlertDialog,
Expand Down Expand Up @@ -86,7 +86,7 @@ export default function GoalCard({ goal, siteId }: GoalCardProps) {
<p>Page Goal</p>
</TooltipContent>
</Tooltip>
) : (
) : goal.goalType === "event" ? (
<Tooltip>
<TooltipTrigger asChild>
<EventIcon />
Expand All @@ -95,14 +95,27 @@ export default function GoalCard({ goal, siteId }: GoalCardProps) {
<p>Event Goal</p>
</TooltipContent>
</Tooltip>
) : (
<Tooltip>
<TooltipTrigger asChild>
<OutboundIcon />
</TooltipTrigger>
<TooltipContent>
<p>Outbound Goal</p>
</TooltipContent>
</Tooltip>
)}
{goal.name || `Goal #${goal.goalId}`}
</h3>

<div className="mt-1">
<span className="text-xs text-neutral-500 dark:text-neutral-400 mr-2">Pattern:</span>
<code className="text-xs bg-neutral-100 dark:bg-neutral-800 px-1 py-0.5 rounded">
{goal.goalType === "path" ? goal.config.pathPattern : goal.config.eventName}
{goal.goalType === "path"
? goal.config.pathPattern
: goal.goalType === "event"
? goal.config.eventName
: goal.config.outboundUrlPattern}
</code>

{goal.goalType === "event" && goal.config.eventPropertyKey && (
Expand Down
113 changes: 99 additions & 14 deletions client/src/app/[site]/goals/components/GoalFormModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import * as z from "zod";
import { useCreateGoal } from "../../../../api/analytics/hooks/goals/useCreateGoal";
import { Goal } from "../../../../api/analytics/endpoints";
import { useUpdateGoal } from "../../../../api/analytics/hooks/goals/useUpdateGoal";
import { useGetOutboundLinks } from "../../../../api/analytics/hooks/events/useGetOutboundLinks";
import { useMetric } from "../../../../api/analytics/hooks/useGetMetric";
import { EventIcon, PageviewIcon } from "../../../../components/EventIcons";
import { EventIcon, OutboundIcon, PageviewIcon } from "../../../../components/EventIcons";
import { Button } from "../../../../components/ui/button";
import {
Dialog,
Expand All @@ -26,11 +27,10 @@ import { Switch } from "../../../../components/ui/switch";
import { cn } from "../../../../lib/utils";
import { Plus, X } from "lucide-react";

// Define form schema
const formSchema = z
.object({
name: z.string().optional(),
goalType: z.enum(["path", "event"]),
goalType: z.enum(["path", "event", "outbound"]),
config: z.object({
pathPattern: z.string().optional(),
eventName: z.string().optional(),
Expand All @@ -44,6 +44,7 @@ const formSchema = z
})
)
.optional(),
outboundUrlPattern: z.string().optional(),
}),
})
.refine(
Expand All @@ -52,6 +53,8 @@ const formSchema = z
return !!data.config.pathPattern;
} else if (data.goalType === "event") {
return !!data.config.eventName;
} else if (data.goalType === "outbound") {
return !!data.config.outboundUrlPattern;
}
return false;
},
Expand All @@ -65,9 +68,9 @@ type FormValues = z.infer<typeof formSchema>;

interface GoalFormModalProps {
siteId: number;
goal?: Goal; // Optional goal for editing mode
goal?: Goal;
trigger: React.ReactNode;
isCloneMode?: boolean; // Optional clone mode flag
isCloneMode?: boolean;
}

export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = false }: GoalFormModalProps) {
Expand Down Expand Up @@ -101,7 +104,8 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
useFilters: false,
});

// Transform data into SuggestionOption format
const { data: outboundLinksData } = useGetOutboundLinks();

const pathSuggestions: SuggestionOption[] =
pathsData?.data?.map(item => ({
value: item.value,
Expand All @@ -114,6 +118,11 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
label: item.value,
})) || [];

const outboundSuggestions: SuggestionOption[] =
outboundLinksData?.map(item => ({
value: item.url,
label: item.url,
})) || [];
// Reinitialize useProperties when goal changes or modal opens
useEffect(() => {
if (isOpen && goal) {
Expand Down Expand Up @@ -142,7 +151,6 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
const createGoal = useCreateGoal();
const updateGoal = useUpdateGoal();

// Initialize form with default values or existing goal
const form = useForm<FormValues>({
resolver: zodResolver(formSchema),
defaultValues:
Expand All @@ -156,6 +164,7 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
eventPropertyKey: goal.config.eventPropertyKey || "",
eventPropertyValue:
goal.config.eventPropertyValue !== undefined ? String(goal.config.eventPropertyValue) : "",
outboundUrlPattern: goal.config.outboundUrlPattern || "",
},
}
: {
Expand All @@ -166,16 +175,48 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
eventName: "",
eventPropertyKey: "",
eventPropertyValue: "",
outboundUrlPattern: "",
},
},
});

useEffect(() => {
if (isOpen) {
setUseProperties(!!goal?.config.eventPropertyKey && !!goal?.config.eventPropertyValue);

Comment on lines +183 to +186
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent useProperties initialization overwrites correct state.

Line 185 only checks legacy fields (eventPropertyKey && eventPropertyValue), but the earlier useEffect at lines 130-133 correctly checks both propertyFilters?.length and legacy fields. Since both effects run on isOpen/goal changes, this effect runs second and overwrites the correct value, causing the property filters UI to be hidden when editing goals that use the new propertyFilters array.

🐛 Proposed fix
   useEffect(() => {
     if (isOpen) {
-      setUseProperties(!!goal?.config.eventPropertyKey && !!goal?.config.eventPropertyValue);
+      const hasFilters = !!(
+        goal?.config.propertyFilters?.length ||
+        (goal?.config.eventPropertyKey && goal?.config.eventPropertyValue !== undefined)
+      );
+      setUseProperties(hasFilters);

       if ((isEditMode || isCloneMode) && goal) {

Alternatively, consider consolidating these two useEffects into one to avoid the race condition and reduce duplication.

📝 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
useEffect(() => {
if (isOpen) {
setUseProperties(!!goal?.config.eventPropertyKey && !!goal?.config.eventPropertyValue);
useEffect(() => {
if (isOpen) {
const hasFilters = !!(
goal?.config.propertyFilters?.length ||
(goal?.config.eventPropertyKey && goal?.config.eventPropertyValue !== undefined)
);
setUseProperties(hasFilters);
if ((isEditMode || isCloneMode) && goal) {
🤖 Prompt for AI Agents
In `@client/src/app/`[site]/goals/components/GoalFormModal.tsx around lines 183 -
186, The second useEffect that sets useProperties (triggered when isOpen/goal
changes) only checks legacy fields (goal?.config.eventPropertyKey &&
goal?.config.eventPropertyValue) and overwrites the correct state set earlier;
update this effect (or consolidate both useEffects into one) so it mirrors the
logic used earlier by checking both the new propertyFilters array
(goal?.config.propertyFilters?.length) and the legacy fields before calling
setUseProperties, ensuring useProperties is true if either the propertyFilters
array has entries or both legacy fields exist (and thus avoiding the
race/overwrite).

if ((isEditMode || isCloneMode) && goal) {
form.reset({
name: isCloneMode ? `${goal.name || `Goal #${goal.goalId}`} (Copy)` : goal.name || "",
goalType: goal.goalType,
config: {
pathPattern: goal.config.pathPattern || "",
eventName: goal.config.eventName || "",
eventPropertyKey: goal.config.eventPropertyKey || "",
eventPropertyValue:
goal.config.eventPropertyValue !== undefined ? String(goal.config.eventPropertyValue) : "",
outboundUrlPattern: goal.config.outboundUrlPattern || "",
},
});
} else if (!goal) {
form.reset({
name: "",
goalType: "path",
config: {
pathPattern: "",
eventName: "",
eventPropertyKey: "",
eventPropertyValue: "",
outboundUrlPattern: "",
},
});
}
}
}, [isOpen, goal, isCloneMode, isEditMode, form]);

const goalType = form.watch("goalType");

// Handle form submission
const onSubmit = async (values: FormValues) => {
try {
// Clean up the config based on goal type
if (values.goalType === "path") {
values.config.eventName = undefined;

Expand All @@ -189,19 +230,24 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
// Clear legacy fields
values.config.eventPropertyKey = undefined;
values.config.eventPropertyValue = undefined;
values.config.outboundUrlPattern = undefined;
} else if (values.goalType === "event") {
values.config.pathPattern = undefined;

// Set propertyFilters if using properties
values.config.outboundUrlPattern = undefined;
if (useProperties) {
const validFilters = propertyFilters.filter(f => f.key && f.value);
values.config.propertyFilters = validFilters.length > 0 ? validFilters : undefined;
} else {
values.config.propertyFilters = undefined;
}
// Clear legacy fields
values.config.eventPropertyKey = undefined;
values.config.eventPropertyValue = undefined;
} else if (values.goalType === "outbound") {
values.config.pathPattern = undefined;
values.config.eventName = undefined;
values.config.eventPropertyKey = undefined;
values.config.eventPropertyValue = undefined;
values.config.propertyFilters = undefined;
}

if (isEditMode) {
Expand All @@ -221,7 +267,6 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
});
}

// Reset form and state after successful submission
form.reset();
setUseProperties(false);

Expand Down Expand Up @@ -287,11 +332,16 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
<PageviewIcon />
<span>Page Goal</span>
</div>
) : (
) : field.value === "event" ? (
<div className="flex items-center gap-1 bg-neutral-800/50 py-2 px-3 rounded">
<EventIcon />
<span>Event Goal</span>
</div>
) : (
<div className="flex items-center gap-1 bg-neutral-800/50 py-2 px-3 rounded">
<OutboundIcon />
<span>Outbound Goal</span>
</div>
)}
</div>
) : (
Expand Down Expand Up @@ -321,6 +371,18 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
<EventIcon />
<span>Event Goal</span>
</Button>
<Button
type="button"
variant={field.value === "outbound" ? "default" : "outline"}
className={cn(
"flex-1 flex items-center justify-center gap-2",
field.value === "outbound" && "border-green-500"
)}
onClick={() => field.onChange("outbound")}
>
<OutboundIcon />
<span>Outbound Goal</span>
</Button>
</div>
</FormControl>
)}
Expand Down Expand Up @@ -494,6 +556,29 @@ export default function GoalFormModal({ siteId, goal, trigger, isCloneMode = fal
</>
)}

{goalType === "outbound" && (
<FormField
control={form.control}
name="config.outboundUrlPattern"
render={({ field }) => (
<FormItem>
<FormLabel>Outbound URL Pattern</FormLabel>
<FormControl>
<InputWithSuggestions
suggestions={outboundSuggestions}
placeholder="*stripe.com* or https://docs.example.com/**"
{...field}
/>
</FormControl>
<FormMessage />
<div className="text-xs text-neutral-500 mt-1">
Use * to match any characters. Use ** to match across path segments.
</div>
</FormItem>
)}
/>
)}

<div className="flex justify-end space-x-2">
<Button variant="outline" type="button" onClick={onClose}>
Cancel
Expand Down
6 changes: 5 additions & 1 deletion client/src/components/EventIcons.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Eye, MousePointerClick } from "lucide-react";
import { ExternalLink, Eye, MousePointerClick } from "lucide-react";
import { cn } from "../lib/utils";

export function PageviewIcon({ className }: { className?: string }) {
Expand All @@ -8,3 +8,7 @@ export function PageviewIcon({ className }: { className?: string }) {
export function EventIcon({ className }: { className?: string }) {
return <MousePointerClick className={cn("h-4 w-4 text-amber-400", className)} />;
}

export function OutboundIcon({ className }: { className?: string }) {
return <ExternalLink className={cn("h-4 w-4 text-green-400", className)} />;
}
Loading