Skip to content
Open
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
118 changes: 71 additions & 47 deletions src/components/therapy/TreatmentPlanManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ const TreatmentPlanManager: FC = () => {
const [error, setError] = useState<string | null>(null)

const [isCreateModalOpen, setIsCreateModalOpen] = useState(false)
const [newPlanData, setNewPlanData] = useState<FormNewPlanData>(
JSON.parse(JSON.stringify(initialNewPlanData) as unknown),
)
const [newPlanData, setNewPlanData] = useState<FormNewPlanData>({
...initialNewPlanData,
goals: [...initialNewPlanData.goals],
} as FormNewPlanData)
Comment on lines +121 to +124
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

initialNewPlanData is instantiated once at module load time (it contains startDate: new Date()...). Resetting form state by spreading initialNewPlanData means the β€œdefault” startDate can become stale (e.g., leaving the tab open overnight, then opening the create modal).

Also, the repeated as FormNewPlanData assertions make it easier to accidentally drift from the real shape without TypeScript helping you.

Suggestion

Introduce a factory function and reuse it for initialization + resets so startDate is always fresh and you can avoid as:

const makeInitialNewPlanData = (): FormNewPlanData => ({
  ...initialNewPlanData,
  startDate: new Date().toISOString().split('T')[0],
  goals: [],
})

const [newPlanData, setNewPlanData] = useState<FormNewPlanData>(makeInitialNewPlanData)

Then replace the reset call sites with setNewPlanData(makeInitialNewPlanData()).

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.


const [planToDelete, setPlanToDelete] = useState<TreatmentPlan | null>(null)
const [isEditModalOpen, setIsEditModalOpen] = useState(false)
Expand Down Expand Up @@ -257,6 +258,7 @@ const TreatmentPlanManager: FC = () => {
// --- End Goal Management ---

// --- Objective Management Functions ---
// ⚑ Bolt: Replace JSON.parse/stringify with shallow copy for performance
const addObjective = (goalIndex: number, isEdit = false) => {
const newObjective: ClientSideNewObjective = {
description: '',
Expand All @@ -265,27 +267,29 @@ const TreatmentPlanManager: FC = () => {
}

if (isEdit && editingPlanData) {
const updatedGoals = JSON.parse(
JSON.stringify(editingPlanData.goals || []),
) as EditableGoal[]
const updatedGoals = [...(editingPlanData.goals || [])]
if (updatedGoals[goalIndex]) {
updatedGoals[goalIndex].objectives = [
...(updatedGoals[goalIndex].objectives || []),
newObjective,
]
updatedGoals[goalIndex] = {
...updatedGoals[goalIndex],
objectives: [
...(updatedGoals[goalIndex].objectives || []),
newObjective,
],
}
setEditingPlanData((prev: FormUpdatePlanData | null) =>
prev ? { ...prev, goals: updatedGoals } : null,
)
}
} else {
const updatedGoals = JSON.parse(
JSON.stringify(newPlanData.goals),
) as ClientSideNewGoal[]
const updatedGoals = [...newPlanData.goals]
if (updatedGoals[goalIndex]) {
updatedGoals[goalIndex].objectives = [
...updatedGoals[goalIndex].objectives,
newObjective,
]
updatedGoals[goalIndex] = {
...updatedGoals[goalIndex],
objectives: [
...updatedGoals[goalIndex].objectives,
newObjective,
],
}
setNewPlanData((prev: FormNewPlanData) => ({
...prev,
goals: updatedGoals,
Expand All @@ -302,33 +306,41 @@ const TreatmentPlanManager: FC = () => {
isEdit = false,
) => {
if (isEdit && editingPlanData) {
const updatedGoals = JSON.parse(
JSON.stringify(editingPlanData.goals || []),
) as EditableGoal[]
const updatedGoals = [...(editingPlanData.goals || [])]
if (
updatedGoals[goalIndex] &&
updatedGoals[goalIndex].objectives &&
updatedGoals[goalIndex].objectives[objIndex]
Comment on lines 310 to 313
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): Guarding on objectives presence may hide inconsistent state without recovery

In updateObjectiveField, the new guard updatedGoals[goalIndex].objectives && updatedGoals[goalIndex].objectives[objIndex] means that if objectives is unexpectedly undefined, the function will silently no-op. If objectives should always exist in this code path, consider normalizing it to an empty array when constructing/updating goals (as when mapping from plan.goals) so this check only needs to validate the index and inconsistent state becomes visible instead of silently ignored.

Suggested implementation:

    isEdit = false,
  ) => {
    if (isEdit && editingPlanData) {
      const updatedGoals = (editingPlanData.goals || []).map((goal) => ({
        ...goal,
        objectives: goal.objectives || [],
      }))
      if (updatedGoals[goalIndex]?.objectives[objIndex]) {
        const updatedObjectives = [...updatedGoals[goalIndex].objectives]

To fully enforce the invariant that objectives is always an array and avoid future silent no-ops, you should also:

  1. Normalize objectives to [] anywhere goals are constructed or updated (e.g., when mapping from plan.goals into editingPlanData or newPlanData).
  2. Ensure any goal-creation helpers or initial state definitions set objectives: [] instead of leaving it undefined.
    This keeps updateObjectiveField simple (only index checks) and ensures inconsistent state is surfaced earlier in the data flow.

Fix in Cursor

) {
;(updatedGoals[goalIndex].objectives[objIndex])[
field as keyof EditableObjective
] = value as never
const updatedObjectives = [...updatedGoals[goalIndex].objectives]
updatedObjectives[objIndex] = {
...updatedObjectives[objIndex],
[field]: value,
}
updatedGoals[goalIndex] = {
...updatedGoals[goalIndex],
objectives: updatedObjectives,
}
setEditingPlanData((prev: FormUpdatePlanData | null) =>
prev ? { ...prev, goals: updatedGoals } : null,
)
}
} else {
const updatedNewGoals = JSON.parse(
JSON.stringify(newPlanData.goals),
) as ClientSideNewGoal[]
const updatedNewGoals = [...newPlanData.goals]
if (
updatedNewGoals[goalIndex] &&
updatedNewGoals[goalIndex].objectives &&
updatedNewGoals[goalIndex].objectives[objIndex]
) {
;(
updatedNewGoals[goalIndex].objectives[
objIndex
]
)[field] = value as never
const updatedObjectives = [...updatedNewGoals[goalIndex].objectives]
updatedObjectives[objIndex] = {
...updatedObjectives[objIndex],
[field]: value,
}
updatedNewGoals[goalIndex] = {
...updatedNewGoals[goalIndex],
objectives: updatedObjectives,
}
setNewPlanData((prev: FormNewPlanData) => ({
...prev,
goals: updatedNewGoals,
Expand All @@ -343,21 +355,27 @@ const TreatmentPlanManager: FC = () => {
isEdit = false,
) => {
if (isEdit && editingPlanData) {
const updatedGoals = JSON.parse(
JSON.stringify(editingPlanData.goals || []),
) as EditableGoal[]
const updatedGoals = [...(editingPlanData.goals || [])]
if (updatedGoals[goalIndex] && updatedGoals[goalIndex].objectives) {
updatedGoals[goalIndex].objectives.splice(objIndex, 1)
const updatedObjectives = [...updatedGoals[goalIndex].objectives]
updatedObjectives.splice(objIndex, 1)
updatedGoals[goalIndex] = {
...updatedGoals[goalIndex],
objectives: updatedObjectives,
}
setEditingPlanData((prev: FormUpdatePlanData | null) =>
prev ? { ...prev, goals: updatedGoals } : null,
)
}
} else {
const updatedNewGoals = JSON.parse(
JSON.stringify(newPlanData.goals),
) as ClientSideNewGoal[]
const updatedNewGoals = [...newPlanData.goals]
if (updatedNewGoals[goalIndex] && updatedNewGoals[goalIndex].objectives) {
updatedNewGoals[goalIndex].objectives.splice(objIndex, 1)
const updatedObjectives = [...updatedNewGoals[goalIndex].objectives]
updatedObjectives.splice(objIndex, 1)
updatedNewGoals[goalIndex] = {
...updatedNewGoals[goalIndex],
objectives: updatedObjectives,
}
setNewPlanData((prev: FormNewPlanData) => ({
...prev,
goals: updatedNewGoals,
Expand Down Expand Up @@ -409,7 +427,10 @@ const TreatmentPlanManager: FC = () => {
}
await fetchPlans()
setIsCreateModalOpen(false)
setNewPlanData(JSON.parse(JSON.stringify(initialNewPlanData) as unknown))
setNewPlanData({
...initialNewPlanData,
goals: [...initialNewPlanData.goals],
} as FormNewPlanData)
toast.success('Treatment plan created successfully!')
} catch (err: unknown) {
const errorMessage =
Expand Down Expand Up @@ -501,18 +522,21 @@ const TreatmentPlanManager: FC = () => {
? new Date(plan.startDate).toISOString().split('T')[0]
: '',
goals: plan.goals
? JSON.parse(
JSON.stringify(
plan.goals.map((g) => ({ ...g, objectives: g.objectives || [] })),
),
)
: [], // Deep copy goals, ensure objectives is array
? plan.goals.map((g) => ({
...g,
objectives: g.objectives ? [...g.objectives] : [],
Comment on lines 524 to +527
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

question (bug_risk): plan.goals mapping now uses a shallow copy of objectives; ensure this matches expectations

This used to deep-clone each goal and objective; now only goals and their objectives arrays are copied, while the objective objects themselves are shared. That’s a behavior change: if any consumer mutates objective objects in place (outside the controlled update paths that spread them on edit), those mutations will now affect both plan.goals and editingPlanData.goals. Please confirm no code depends on the previous full detachment.

Fix in Cursor

}))
: [], // Shallow copy goals, ensure objectives is array
} as FormUpdatePlanData) // Cast to ensure type compatibility
setIsEditModalOpen(true)
}

const openCreateModal = () => {
setNewPlanData(JSON.parse(JSON.stringify(initialNewPlanData) as unknown)) // Reset with deep copy
// ⚑ Bolt: Replace JSON.parse/stringify with shallow copy for performance
setNewPlanData({
...initialNewPlanData,
goals: [...initialNewPlanData.goals],
} as FormNewPlanData) // Reset with shallow copy
setIsCreateModalOpen(true)
}

Expand Down
Loading