Skip to content

Commit cb8ae81

Browse files
rorarclaude
andcommitted
fix: review round — 11 findings fixed (event propagation, race condition, i18n, types)
HIGH: DropdownMenuItem handlers prevent Link navigation via stopPropagation MEDIUM: Create & Run Now uses useRef to avoid stale closure race MEDIUM: Resume Combobox uses r.id as value + keywords prop for dedup safety MEDIUM: AddContactInfo i18n — 12 hardcoded English strings replaced (16 new keys, 4 locales) MEDIUM: Detail page as-any casts replaced with proper typing LOW: Removed getLocationCodes wrapper, cn() for LocationBadge, added comments 69 suites, 1481 tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 830bc15 commit cb8ae81

7 files changed

Lines changed: 124 additions & 55 deletions

File tree

src/actions/automation.actions.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ export async function getAutomationsList(
7474
prisma.automation.count({ where: { userId: user.id } }),
7575
]);
7676

77-
// Soft warning when automation count exceeds user's configured threshold
77+
// Soft warning when automation count exceeds user's configured threshold.
78+
// Convention: result.message is overloaded — a "performanceWarning:<count>" prefix
79+
// signals a non-error warning to the UI layer, which checks for the prefix in
80+
// AutomationWizard.onSubmit to show a separate toast. This avoids adding a
81+
// dedicated "warnings" field to ActionResult for a single use case.
7882
let warning: string | undefined;
7983
const automationSettings = await getAutomationSettingsForUser(user.id);
8084
if (
@@ -199,7 +203,8 @@ export async function createAutomation(
199203
},
200204
});
201205

202-
// Soft warning when automation count exceeds user's configured threshold
206+
// Soft warning when automation count exceeds user's configured threshold.
207+
// See getAutomationsList for the "performanceWarning:" prefix convention.
203208
let warning: string | undefined;
204209
const automationCount = await prisma.automation.count({ where: { userId: user.id } });
205210
const automationSettings = await getAutomationSettingsForUser(user.id);

src/app/dashboard/automations/[id]/page.tsx

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import type {
3838
AutomationRun,
3939
DiscoveredJob,
4040
} from "@/models/automation.model";
41+
import type { Resume } from "@/models/profile.model";
4142
import type { JobMatchResponse } from "@/models/ai.schemas";
4243
import { DiscoveredJobsList } from "@/components/automations/DiscoveredJobsList";
4344
import { DiscoveredJobDetail } from "@/components/automations/DiscoveredJobDetail";
@@ -55,9 +56,9 @@ export default function AutomationDetailPage() {
5556
const { locale } = useTranslations();
5657
const automationId = params.id as string;
5758

58-
const [automation, setAutomation] = useState<AutomationWithResume | null>(
59-
null,
60-
);
59+
const [automation, setAutomation] = useState<
60+
(AutomationWithResume & { runs?: AutomationRun[] }) | null
61+
>(null);
6162
const [runs, setRuns] = useState<AutomationRun[]>([]);
6263
const [jobs, setJobs] = useState<DiscoveredJob[]>([]);
6364
const [loading, setLoading] = useState(true);
@@ -82,8 +83,8 @@ export default function AutomationDetailPage() {
8283
]);
8384

8485
if (automationResult.success && automationResult.data) {
85-
setAutomation(automationResult.data as any);
86-
setRuns((automationResult.data as any).runs || []);
86+
setAutomation(automationResult.data);
87+
setRuns(automationResult.data.runs || []);
8788
} else {
8889
toast({
8990
title: "Error",
@@ -95,15 +96,16 @@ export default function AutomationDetailPage() {
9596
}
9697

9798
if (runsResult.success && runsResult.data) {
98-
setRuns(runsResult.data as any);
99+
setRuns(runsResult.data);
99100
}
100101

101102
if (jobsResult.success && jobsResult.data) {
102-
setJobs(jobsResult.data as any);
103+
// StagedVacancyWithAutomation is structurally compatible with DiscoveredJob at runtime
104+
setJobs(jobsResult.data as unknown as DiscoveredJob[]);
103105
}
104106

105107
if (resumeResult.success && resumeResult.data) {
106-
setResumes(resumeResult.data.map((r: any) => ({ id: r.id, title: r.title })));
108+
setResumes(resumeResult.data.map((r: Resume) => ({ id: r.id, title: r.title })));
107109
}
108110
} catch (error) {
109111
toast({
@@ -184,9 +186,10 @@ export default function AutomationDetailPage() {
184186
const handleViewJobDetails = async (job: DiscoveredJob) => {
185187
const result = await getDiscoveredJobById(job.id);
186188
if (result.success && result.data) {
187-
setSelectedJob(result.data as any);
189+
// StagedVacancyWithAutomation is structurally compatible with DiscoveredJob at runtime
190+
setSelectedJob(result.data as unknown as DiscoveredJob);
188191
setSelectedJobMatchData(
189-
(result.data as any).parsedMatchData as JobMatchResponse | null,
192+
result.data.parsedMatchData as JobMatchResponse | null,
190193
);
191194
setDetailOpen(true);
192195
} else {

src/components/automations/AutomationList.tsx

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,6 @@ const PAUSE_REASON_KEYS: Record<AutomationPauseReason, string> = {
6666
cb_escalation: "automations.pauseReasonCbEscalation",
6767
};
6868

69-
/** Parse location string into raw codes for LocationBadge rendering */
70-
function getLocationCodes(location: string): string[] {
71-
if (!location) return [];
72-
return parseLocations(location);
73-
}
74-
7569
export function AutomationList({
7670
automations,
7771
onEdit,
@@ -157,7 +151,7 @@ export function AutomationList({
157151
const isLoading = loadingAction === automation.id;
158152
const resumeMissing = !automation.resume;
159153
const keywordChips = parseKeywords(automation.keywords);
160-
const locationCodes = getLocationCodes(automation.location);
154+
const locationCodes = parseLocations(automation.location);
161155
const isEures = automation.jobBoard === "eures" || automation.jobBoard === "arbeitsagentur";
162156

163157
return (
@@ -268,27 +262,27 @@ export function AutomationList({
268262
</DropdownMenuTrigger>
269263
<DropdownMenuContent align="end">
270264
{automation.status === "active" ? (
271-
<DropdownMenuItem onClick={() => handlePause(automation.id)}>
265+
<DropdownMenuItem onClick={(e) => { e.preventDefault(); e.stopPropagation(); handlePause(automation.id); }}>
272266
<Pause className="h-4 w-4 mr-2" />
273267
{t("automations.pause")}
274268
</DropdownMenuItem>
275269
) : (
276270
<DropdownMenuItem
277-
onClick={() => handleResume(automation.id)}
271+
onClick={(e) => { e.preventDefault(); e.stopPropagation(); handleResume(automation.id); }}
278272
disabled={resumeMissing}
279273
>
280274
<Play className="h-4 w-4 mr-2" />
281275
{t("automations.resume")}
282276
</DropdownMenuItem>
283277
)}
284-
<DropdownMenuItem onClick={() => onEdit(automation)}>
278+
<DropdownMenuItem onClick={(e) => { e.preventDefault(); e.stopPropagation(); onEdit(automation); }}>
285279
<Pencil className="h-4 w-4 mr-2" />
286280
{t("automations.edit")}
287281
</DropdownMenuItem>
288282
<DropdownMenuSeparator />
289283
<DropdownMenuItem
290284
className="text-destructive"
291-
onClick={() => setDeleteId(automation.id)}
285+
onClick={(e) => { e.preventDefault(); e.stopPropagation(); setDeleteId(automation.id); }}
292286
>
293287
<Trash2 className="h-4 w-4 mr-2" />
294288
{t("automations.delete")}

src/components/automations/AutomationWizard.tsx

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"use client";
22

3-
import { useState, useEffect } from "react";
3+
import { useState, useEffect, useRef } from "react";
44
import { useForm } from "react-hook-form";
55
import { zodResolver } from "@hookform/resolvers/zod";
66
import {
@@ -123,6 +123,7 @@ export function AutomationWizard({
123123
const [scheduleFrequency, setScheduleFrequency] = useState<ScheduleFrequency>("daily");
124124
const [resumePopoverOpen, setResumePopoverOpen] = useState(false);
125125
const [runAfterCreate, setRunAfterCreate] = useState(false);
126+
const runAfterCreateRef = useRef(false);
126127

127128
const form = useForm<CreateAutomationInput>({
128129
resolver: zodResolver(CreateAutomationSchema),
@@ -258,7 +259,9 @@ export function AutomationWizard({
258259
const onSubmit = async (data: CreateAutomationInput) => {
259260
setIsSubmitting(true);
260261
try {
261-
// Ensure schedule frequency is in connectorParams before submit
262+
// Ensure schedule frequency is in connectorParams before submit.
263+
// Only persist non-daily frequencies: "daily" is the server-side default,
264+
// so omitting it keeps connectorParams lean for the common case.
262265
const currentParams = tryParseConnectorParams(data.connectorParams) ?? {};
263266
if (scheduleFrequency !== "daily") {
264267
data.connectorParams = JSON.stringify({ ...currentParams, scheduleFrequency });
@@ -269,13 +272,15 @@ export function AutomationWizard({
269272
: await createAutomation(data);
270273

271274
if (result.success) {
272-
// If "Create & Run Now" was clicked, trigger a manual run
273-
if (runAfterCreate && result.data?.id) {
275+
// If "Create & Run Now" was clicked, trigger a manual run.
276+
// Read from ref to avoid stale closure over state set just before handleSubmit.
277+
if (runAfterCreateRef.current && result.data?.id) {
274278
try {
275279
await fetch(`/api/automations/${result.data.id}/run`, { method: "POST" });
276280
} catch {
277281
// Non-blocking — automation was created, run is best-effort
278282
}
283+
runAfterCreateRef.current = false;
279284
setRunAfterCreate(false);
280285
}
281286

@@ -572,20 +577,16 @@ export function AutomationWizard({
572577
</FormControl>
573578
</PopoverTrigger>
574579
<PopoverContent className="w-[--radix-popover-trigger-width] p-0" align="start">
575-
<Command filter={(value, search) => {
576-
// Custom filter: cmdk strips special chars by default.
577-
// We need exact substring match for titles like "[MOCK_DATA] Jane Smith"
578-
if (!search) return 1;
579-
return value.toLowerCase().includes(search.toLowerCase()) ? 1 : 0;
580-
}}>
580+
<Command>
581581
<CommandInput placeholder={t("automations.searchResume")} />
582582
<CommandList>
583583
<CommandEmpty>{t("automations.noResumeFound")}</CommandEmpty>
584584
<CommandGroup>
585585
{resumes.map((r) => (
586586
<CommandItem
587587
key={r.id}
588-
value={r.title}
588+
value={r.id}
589+
keywords={[r.title]}
589590
onSelect={() => {
590591
field.onChange(r.id);
591592
setResumePopoverOpen(false);
@@ -847,6 +848,7 @@ export function AutomationWizard({
847848
type="button"
848849
disabled={isSubmitting}
849850
onClick={async () => {
851+
runAfterCreateRef.current = true;
850852
setRunAfterCreate(true);
851853
form.handleSubmit(onSubmit)();
852854
}}

src/components/profile/AddContactInfo.tsx

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ function AddContactInfo({
4646
const { t } = useTranslations();
4747

4848
const pageTitle = contactInfoToEdit
49-
? "Edit Contact Info"
50-
: "Add Contact Info";
49+
? t("profile.editContactInfo")
50+
: t("profile.addContactInfo");
5151

5252
const form = useForm<z.infer<typeof AddContactInfoFormSchema>>({
5353
resolver: zodResolver(AddContactInfoFormSchema),
@@ -119,9 +119,9 @@ function AddContactInfo({
119119
setDialogOpen(false);
120120
toast({
121121
variant: "success",
122-
description: `Contact Info has been ${
123-
contactInfoToEdit ? "updated" : "created"
124-
} successfully`,
122+
description: contactInfoToEdit
123+
? t("profile.contactInfoUpdated")
124+
: t("profile.contactInfoCreated"),
125125
});
126126
}
127127
});
@@ -135,7 +135,7 @@ function AddContactInfo({
135135
<DialogHeader>
136136
<DialogTitle>{pageTitle}</DialogTitle>
137137
<DialogDescription className="sr-only">
138-
{contactInfoToEdit ? "Edit contact information" : "Add contact information"}
138+
{contactInfoToEdit ? t("profile.editContactInfo") : t("profile.addContactInfo")}
139139
</DialogDescription>
140140
</DialogHeader>
141141
<Form {...form}>
@@ -150,9 +150,9 @@ function AddContactInfo({
150150
name="firstName"
151151
render={({ field }) => (
152152
<FormItem>
153-
<FormLabel>First Name</FormLabel>
153+
<FormLabel>{t("profile.firstName")}</FormLabel>
154154
<FormControl>
155-
<Input placeholder="e.g. John" {...field} />
155+
<Input placeholder={t("profile.firstNamePlaceholder")} {...field} />
156156
</FormControl>
157157
<FormMessage />
158158
</FormItem>
@@ -167,9 +167,9 @@ function AddContactInfo({
167167
name="lastName"
168168
render={({ field }) => (
169169
<FormItem>
170-
<FormLabel>Last Name</FormLabel>
170+
<FormLabel>{t("profile.lastName")}</FormLabel>
171171
<FormControl>
172-
<Input placeholder="e.g. Doe" {...field} />
172+
<Input placeholder={t("profile.lastNamePlaceholder")} {...field} />
173173
</FormControl>
174174
<FormMessage />
175175
</FormItem>
@@ -184,9 +184,9 @@ function AddContactInfo({
184184
name="headline"
185185
render={({ field }) => (
186186
<FormItem>
187-
<FormLabel>Headline</FormLabel>
187+
<FormLabel>{t("profile.headline")}</FormLabel>
188188
<FormControl>
189-
<Input placeholder="e.g. Senior Software Engineer" {...field} />
189+
<Input placeholder={t("profile.headlinePlaceholder")} {...field} />
190190
</FormControl>
191191
<FormMessage />
192192
</FormItem>
@@ -201,9 +201,9 @@ function AddContactInfo({
201201
name="email"
202202
render={({ field }) => (
203203
<FormItem>
204-
<FormLabel>Email</FormLabel>
204+
<FormLabel>{t("profile.email")}</FormLabel>
205205
<FormControl>
206-
<Input placeholder="e.g. john@example.com" {...field} />
206+
<Input placeholder={t("profile.emailPlaceholder")} {...field} />
207207
</FormControl>
208208
<FormMessage />
209209
</FormItem>
@@ -218,9 +218,9 @@ function AddContactInfo({
218218
name="phone"
219219
render={({ field }) => (
220220
<FormItem>
221-
<FormLabel>Phone</FormLabel>
221+
<FormLabel>{t("profile.phone")}</FormLabel>
222222
<FormControl>
223-
<Input placeholder="e.g. +49 123 456789" {...field} />
223+
<Input placeholder={t("profile.phonePlaceholder")} {...field} />
224224
</FormControl>
225225
<FormMessage />
226226
</FormItem>
@@ -235,9 +235,9 @@ function AddContactInfo({
235235
name="address"
236236
render={({ field }) => (
237237
<FormItem>
238-
<FormLabel>Address</FormLabel>
238+
<FormLabel>{t("profile.address")}</FormLabel>
239239
<FormControl>
240-
<Input placeholder="e.g. Berlin, Germany" {...field} value={field.value ?? ""} />
240+
<Input placeholder={t("profile.addressPlaceholder")} {...field} value={field.value ?? ""} />
241241
</FormControl>
242242
<FormMessage />
243243
</FormItem>
@@ -255,11 +255,11 @@ function AddContactInfo({
255255
className="mt-2 md:mt-0 w-full"
256256
onClick={closeDialog}
257257
>
258-
Cancel
258+
{t("common.cancel")}
259259
</Button>
260260
</div>
261261
<Button type="submit" disabled={!formState.isDirty}>
262-
Save
262+
{t("common.save")}
263263
{isPending && <Loader className="h-4 w-4 shrink-0 spinner" />}
264264
</Button>
265265
</DialogFooter>

src/components/ui/location-badge.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import Image from "next/image";
44
import { Badge } from "@/components/ui/badge";
5+
import { cn } from "@/lib/utils";
56
import { getLocationLabel, getCountryCode } from "@/lib/connector/job-discovery/modules/eures/countries";
67

78
interface LocationBadgeProps {
@@ -20,7 +21,7 @@ export function LocationBadge({ code, resolve = true, className }: LocationBadge
2021
const countryCode = getCountryCode(code);
2122

2223
return (
23-
<Badge variant="secondary" className={`text-xs gap-1 ${className ?? ""}`}>
24+
<Badge variant="secondary" className={cn("text-xs gap-1", className)}>
2425
{countryCode && (
2526
<Image
2627
src={`/flags/${countryCode.toLowerCase()}.svg`}

0 commit comments

Comments
 (0)