Auto save project selection bug (#134)
* fix(tailoring): remove auto-sync effect causing race conditions Remove the problematic useEffect that was syncing incoming job data automatically. The effect caused race conditions where user edits were overwritten after auto-save completed. Now, state only resets when the job ID changes (user switches to a different job). User edits persist until explicitly saved. Fixes #133 * fix(tailoring): remove auto-save from tailor mode Remove the 1500ms auto-save timeout that was causing race conditions with the state sync. Users must now explicitly save changes via the Save Selection button or finalize to persist changes. * refactor(tailoring): remove draft status state and UI Remove the draftStatus state and related UI elements that showed saving/saved/unsaved status. With auto-save removed, this status indicator is no longer needed. Users now explicitly save via buttons. * test(tailoring): remove auto-save test Remove the test that verified auto-save behavior since auto-save has been removed from the tailor mode. Users now explicitly save via the Finalize button. * refactor(tailoring): remove dead focus tracking code Remove the activeField state and all related focus/blur tracking that was orphaned after removing auto-sync. The focus tracking was only used to prevent the auto-sync effect from running while editing. Changes: - Remove TailoringActiveField type export - Remove activeField state and setActiveField from useTailoringDraft - Remove handleFieldBlur callback from TailoringWorkspace - Remove onFieldFocus/onFieldBlur props and handlers from TailoringSections 39 lines of dead code removed. * docs(tailoring): clarify save behavior comment Update comment to distinguish between editor mode (Save Selection button) and tailor mode (only persists on finalize). Addresses review feedback. * docs(tailoring): clarify useEffect dependencies Add note explaining why 'job' is included in dependencies despite the effect being guarded by job.id check. Addresses review feedback about dependency array clarity. * fix(tailoring): sync server-normalized values after save Update persistCurrent and saveChanges to use the returned job from api.updateJob and call applyIncomingDraft. This ensures local state stays in sync with server-normalized values (e.g., trimmed fields). Also removes unused markCurrentAsSaved dependency. * refactor(tailoring): simplify draft sync effect Remove unused save snapshot helpers and stop exposing them from the hook. Track the latest job in a ref and only sync drafts when the job id changes to avoid unnecessary effect runs while keeping data correctness. Addresses review feedback on dependency churn and dead API surface.
This commit is contained in:
parent
e114c5d592
commit
05f1c62de2
@ -190,45 +190,6 @@ describe("TailorMode", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("autosaves headline and skills in update payload", async () => {
|
||||
vi.mocked(api.updateJob).mockResolvedValue({} as Job);
|
||||
|
||||
render(
|
||||
<TailorMode
|
||||
job={createJob()}
|
||||
onBack={vi.fn()}
|
||||
onFinalize={vi.fn()}
|
||||
isFinalizing={false}
|
||||
/>,
|
||||
);
|
||||
await waitFor(() =>
|
||||
expect(api.getResumeProjectsCatalog).toHaveBeenCalled(),
|
||||
);
|
||||
ensureAccordionOpen("Headline");
|
||||
ensureAccordionOpen("Tailored Skills");
|
||||
ensureAccordionOpen("Core");
|
||||
|
||||
fireEvent.change(screen.getByLabelText("Tailored Headline"), {
|
||||
target: { value: "Updated headline" },
|
||||
});
|
||||
fireEvent.change(screen.getByLabelText("Keywords (comma-separated)"), {
|
||||
target: { value: "Node.js, TypeScript" },
|
||||
});
|
||||
|
||||
await waitFor(
|
||||
() =>
|
||||
expect(api.updateJob).toHaveBeenCalledWith(
|
||||
"job-1",
|
||||
expect.objectContaining({
|
||||
tailoredHeadline: "Updated headline",
|
||||
tailoredSkills:
|
||||
'[{"name":"Core","keywords":["Node.js","TypeScript"]}]',
|
||||
}),
|
||||
),
|
||||
{ timeout: 3500 },
|
||||
);
|
||||
});
|
||||
|
||||
it("hydrates headline and skills after AI draft generation", async () => {
|
||||
vi.mocked(api.summarizeJob).mockResolvedValueOnce({
|
||||
...createJob(),
|
||||
|
||||
@ -10,9 +10,6 @@ import {
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { ProjectSelector } from "../discovered-panel/ProjectSelector";
|
||||
import type { EditableSkillGroup } from "../tailoring-utils";
|
||||
import type { TailoringActiveField } from "./useTailoringDraft";
|
||||
|
||||
type FocusableField = Exclude<TailoringActiveField, null>;
|
||||
|
||||
interface TailoringSectionsProps {
|
||||
catalog: ResumeProjectCatalogItem[];
|
||||
@ -35,8 +32,6 @@ interface TailoringSectionsProps {
|
||||
) => void;
|
||||
onRemoveSkillGroup: (id: string) => void;
|
||||
onToggleProject: (id: string) => void;
|
||||
onFieldFocus: (field: FocusableField) => void;
|
||||
onFieldBlur: (field: FocusableField) => void;
|
||||
}
|
||||
|
||||
const sectionClass = "rounded-lg border border-border/60 bg-muted/20 px-0";
|
||||
@ -62,8 +57,6 @@ export const TailoringSections: React.FC<TailoringSectionsProps> = ({
|
||||
onUpdateSkillGroup,
|
||||
onRemoveSkillGroup,
|
||||
onToggleProject,
|
||||
onFieldFocus,
|
||||
onFieldBlur,
|
||||
}) => {
|
||||
return (
|
||||
<Accordion type="multiple" className="space-y-3">
|
||||
@ -80,8 +73,6 @@ export const TailoringSections: React.FC<TailoringSectionsProps> = ({
|
||||
className={`${inputClass} min-h-[120px] max-h-[250px]`}
|
||||
value={jobDescription}
|
||||
onChange={(event) => onDescriptionChange(event.target.value)}
|
||||
onFocus={() => onFieldFocus("description")}
|
||||
onBlur={() => onFieldBlur("description")}
|
||||
placeholder="The raw job description..."
|
||||
disabled={disableInputs}
|
||||
/>
|
||||
@ -99,8 +90,6 @@ export const TailoringSections: React.FC<TailoringSectionsProps> = ({
|
||||
className={`${inputClass} min-h-[120px]`}
|
||||
value={summary}
|
||||
onChange={(event) => onSummaryChange(event.target.value)}
|
||||
onFocus={() => onFieldFocus("summary")}
|
||||
onBlur={() => onFieldBlur("summary")}
|
||||
placeholder="Write a tailored summary for this role, or generate with AI..."
|
||||
disabled={disableInputs}
|
||||
/>
|
||||
@ -119,8 +108,6 @@ export const TailoringSections: React.FC<TailoringSectionsProps> = ({
|
||||
className={inputClass}
|
||||
value={headline}
|
||||
onChange={(event) => onHeadlineChange(event.target.value)}
|
||||
onFocus={() => onFieldFocus("headline")}
|
||||
onBlur={() => onFieldBlur("headline")}
|
||||
placeholder="Write a concise headline tailored to this role..."
|
||||
disabled={disableInputs}
|
||||
/>
|
||||
@ -188,8 +175,6 @@ export const TailoringSections: React.FC<TailoringSectionsProps> = ({
|
||||
event.target.value,
|
||||
)
|
||||
}
|
||||
onFocus={() => onFieldFocus("skills")}
|
||||
onBlur={() => onFieldBlur("skills")}
|
||||
placeholder="Backend, Frontend, Infrastructure..."
|
||||
disabled={disableInputs}
|
||||
/>
|
||||
@ -213,8 +198,6 @@ export const TailoringSections: React.FC<TailoringSectionsProps> = ({
|
||||
event.target.value,
|
||||
)
|
||||
}
|
||||
onFocus={() => onFieldFocus("skills")}
|
||||
onBlur={() => onFieldBlur("skills")}
|
||||
placeholder="TypeScript, Node.js, REST APIs..."
|
||||
disabled={disableInputs}
|
||||
/>
|
||||
|
||||
@ -54,8 +54,6 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
setOpenSkillGroupId,
|
||||
skillsJson,
|
||||
isDirty,
|
||||
setActiveField,
|
||||
markCurrentAsSaved,
|
||||
applyIncomingDraft,
|
||||
handleToggleProject,
|
||||
handleAddSkillGroup,
|
||||
@ -70,9 +68,6 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
const [isSummarizing, setIsSummarizing] = useState(false);
|
||||
const [isGeneratingPdf, setIsGeneratingPdf] = useState(false);
|
||||
const [isGenerating, setIsGenerating] = useState(false);
|
||||
const [draftStatus, setDraftStatus] = useState<
|
||||
"unsaved" | "saving" | "saved"
|
||||
>("saved");
|
||||
|
||||
const savePayload = useMemo(
|
||||
() => ({
|
||||
@ -86,38 +81,14 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
);
|
||||
|
||||
const persistCurrent = useCallback(async () => {
|
||||
await api.updateJob(props.job.id, savePayload);
|
||||
markCurrentAsSaved();
|
||||
if (tailorProps) {
|
||||
setDraftStatus("saved");
|
||||
}
|
||||
}, [props.job.id, savePayload, markCurrentAsSaved, tailorProps]);
|
||||
const updatedJob = await api.updateJob(props.job.id, savePayload);
|
||||
applyIncomingDraft(updatedJob);
|
||||
}, [props.job.id, savePayload, applyIncomingDraft]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!tailorProps) return;
|
||||
if (isDirty && draftStatus === "saved") {
|
||||
setDraftStatus("unsaved");
|
||||
}
|
||||
if (!isDirty && draftStatus === "unsaved") {
|
||||
setDraftStatus("saved");
|
||||
}
|
||||
}, [tailorProps, isDirty, draftStatus]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!tailorProps) return;
|
||||
if (!isDirty || draftStatus !== "unsaved") return;
|
||||
|
||||
const timeout = setTimeout(async () => {
|
||||
try {
|
||||
setDraftStatus("saving");
|
||||
await persistCurrent();
|
||||
} catch {
|
||||
setDraftStatus("unsaved");
|
||||
}
|
||||
}, 1500);
|
||||
|
||||
return () => clearTimeout(timeout);
|
||||
}, [tailorProps, isDirty, draftStatus, persistCurrent]);
|
||||
// Note: Auto-save removed.
|
||||
// Editor mode: user must explicitly save via the "Save Selection" button to persist changes.
|
||||
// Tailor mode: there is no explicit save action; changes only persist when the user finalizes
|
||||
// or otherwise completes the tailoring flow. This prevents race conditions and simplifies state.
|
||||
|
||||
const saveChanges = useCallback(
|
||||
async ({ showToast = true }: { showToast?: boolean } = {}) => {
|
||||
@ -125,8 +96,8 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
|
||||
try {
|
||||
setIsSaving(true);
|
||||
await api.updateJob(props.job.id, savePayload);
|
||||
markCurrentAsSaved();
|
||||
const updatedJob = await api.updateJob(props.job.id, savePayload);
|
||||
applyIncomingDraft(updatedJob);
|
||||
if (showToast) toast.success("Changes saved");
|
||||
await editorProps.onUpdate();
|
||||
} catch (error) {
|
||||
@ -136,7 +107,7 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
setIsSaving(false);
|
||||
}
|
||||
},
|
||||
[editorProps, props.job.id, savePayload, markCurrentAsSaved],
|
||||
[editorProps, props.job.id, savePayload, applyIncomingDraft],
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
@ -144,13 +115,6 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
editorProps.onRegisterSave(() => saveChanges({ showToast: false }));
|
||||
}, [editorProps, saveChanges]);
|
||||
|
||||
const handleFieldBlur = useCallback(
|
||||
(field: "summary" | "headline" | "description" | "skills") => {
|
||||
setActiveField((prev) => (prev === field ? null : prev));
|
||||
},
|
||||
[setActiveField],
|
||||
);
|
||||
|
||||
const handleSummarizeEditor = useCallback(async () => {
|
||||
if (!editorProps) return;
|
||||
|
||||
@ -185,7 +149,6 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
|
||||
const updatedJob = await api.summarizeJob(props.job.id, { force: true });
|
||||
applyIncomingDraft(updatedJob);
|
||||
setDraftStatus("saved");
|
||||
|
||||
toast.success("Draft generated with AI", {
|
||||
description: "Review and edit before finalizing.",
|
||||
@ -303,8 +266,6 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
onUpdateSkillGroup={handleUpdateSkillGroup}
|
||||
onRemoveSkillGroup={handleRemoveSkillGroup}
|
||||
onToggleProject={handleToggleProject}
|
||||
onFieldFocus={setActiveField}
|
||||
onFieldBlur={handleFieldBlur}
|
||||
/>
|
||||
|
||||
<div className="flex justify-end border-t pt-4">
|
||||
@ -342,24 +303,6 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
<ArrowLeft className="h-3.5 w-3.5" />
|
||||
Back to overview
|
||||
</button>
|
||||
|
||||
<div className="flex items-center gap-1.5 text-[10px] text-muted-foreground">
|
||||
{draftStatus === "saving" && (
|
||||
<>
|
||||
<Loader2 className="h-3 w-3 animate-spin" />
|
||||
Saving...
|
||||
</>
|
||||
)}
|
||||
{draftStatus === "saved" && !isDirty && (
|
||||
<>
|
||||
<Check className="h-3 w-3 text-emerald-400" />
|
||||
Saved
|
||||
</>
|
||||
)}
|
||||
{draftStatus === "unsaved" && (
|
||||
<span className="text-amber-400">Unsaved changes</span>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div className="flex-1 space-y-4 overflow-y-auto pr-1">
|
||||
@ -409,8 +352,6 @@ export const TailoringWorkspace: React.FC<TailoringWorkspaceProps> = (
|
||||
onUpdateSkillGroup={handleUpdateSkillGroup}
|
||||
onRemoveSkillGroup={handleRemoveSkillGroup}
|
||||
onToggleProject={handleToggleProject}
|
||||
onFieldFocus={setActiveField}
|
||||
onFieldBlur={handleFieldBlur}
|
||||
/>
|
||||
</div>
|
||||
|
||||
|
||||
@ -10,13 +10,6 @@ import {
|
||||
toEditableSkillGroups,
|
||||
} from "../tailoring-utils";
|
||||
|
||||
export type TailoringActiveField =
|
||||
| "summary"
|
||||
| "headline"
|
||||
| "description"
|
||||
| "skills"
|
||||
| null;
|
||||
|
||||
const parseSelectedIds = (value: string | null | undefined) =>
|
||||
new Set(value?.split(",").filter(Boolean) ?? []);
|
||||
|
||||
@ -87,8 +80,8 @@ export function useTailoringDraft({
|
||||
serializeTailoredSkills(parseTailoredSkills(job.tailoredSkills)),
|
||||
);
|
||||
|
||||
const [activeField, setActiveField] = useState<TailoringActiveField>(null);
|
||||
const lastJobIdRef = useRef(job.id);
|
||||
const jobRef = useRef(job);
|
||||
|
||||
const skillsJson = useMemo(
|
||||
() => serializeTailoredSkills(fromEditableSkillGroups(skillsDraft)),
|
||||
@ -119,42 +112,6 @@ export function useTailoringDraft({
|
||||
savedSelectedIds,
|
||||
]);
|
||||
|
||||
const syncSavedSnapshot = useCallback(
|
||||
(
|
||||
nextSummary: string,
|
||||
nextHeadline: string,
|
||||
nextDescription: string,
|
||||
nextSelectedIds: Set<string>,
|
||||
nextSkillsDraft: EditableSkillGroup[],
|
||||
) => {
|
||||
setSavedSummary(nextSummary);
|
||||
setSavedHeadline(nextHeadline);
|
||||
setSavedDescription(nextDescription);
|
||||
setSavedSelectedIds(new Set(nextSelectedIds));
|
||||
setSavedSkillsJson(
|
||||
serializeTailoredSkills(fromEditableSkillGroups(nextSkillsDraft)),
|
||||
);
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
const markCurrentAsSaved = useCallback(() => {
|
||||
syncSavedSnapshot(
|
||||
summary,
|
||||
headline,
|
||||
jobDescription,
|
||||
selectedIds,
|
||||
skillsDraft,
|
||||
);
|
||||
}, [
|
||||
syncSavedSnapshot,
|
||||
summary,
|
||||
headline,
|
||||
jobDescription,
|
||||
selectedIds,
|
||||
skillsDraft,
|
||||
]);
|
||||
|
||||
const applyIncomingDraft = useCallback((incomingJob: Job) => {
|
||||
const next = parseIncomingDraft(incomingJob);
|
||||
setSummary(next.summary);
|
||||
@ -184,28 +141,18 @@ export function useTailoringDraft({
|
||||
.catch(() => setCatalog([]));
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
jobRef.current = job;
|
||||
}, [job]);
|
||||
|
||||
// Only sync when job ID changes (user switched to a different job)
|
||||
// User edits persist until explicitly saved - no auto-sync from server
|
||||
useEffect(() => {
|
||||
if (job.id !== lastJobIdRef.current) {
|
||||
lastJobIdRef.current = job.id;
|
||||
applyIncomingDraft(job);
|
||||
return;
|
||||
applyIncomingDraft(jobRef.current);
|
||||
}
|
||||
|
||||
if (isDirty || activeField !== null) return;
|
||||
|
||||
applyIncomingDraft(job);
|
||||
}, [
|
||||
job,
|
||||
job.id,
|
||||
job.tailoredSummary,
|
||||
job.tailoredHeadline,
|
||||
job.tailoredSkills,
|
||||
job.jobDescription,
|
||||
job.selectedProjectIds,
|
||||
isDirty,
|
||||
activeField,
|
||||
applyIncomingDraft,
|
||||
]);
|
||||
}, [job.id, applyIncomingDraft]);
|
||||
|
||||
useEffect(() => {
|
||||
if (
|
||||
@ -264,11 +211,7 @@ export function useTailoringDraft({
|
||||
setOpenSkillGroupId,
|
||||
skillsJson,
|
||||
isDirty,
|
||||
activeField,
|
||||
setActiveField,
|
||||
markCurrentAsSaved,
|
||||
applyIncomingDraft,
|
||||
syncSavedSnapshot,
|
||||
handleToggleProject,
|
||||
handleAddSkillGroup,
|
||||
handleUpdateSkillGroup,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user