Skip to content

fix(settings): details and visiting date field validation #4615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(settings): details and visiting date field validation #4615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e82d139
fix(components): allow only one empty CO visit field
OneChanceLance Sep 15, 2025
183c261
CU-86c1dwh1e - [FIX] Allow only one empty CO visit field – Congregati…
OneChanceLance Oct 4, 2025
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
2 changes: 1 addition & 1 deletion .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ VITE_FIREBASE_APIKEY=organized-local-api-key
VITE_FIREBASE_AUTHDOMAIN=localhost
VITE_FIREBASE_PROJECTID=organized-local
VITE_FIREBASE_APPID=organized-local-app-id
VITE_APP_MODE=DEV
VITE_APP_MODE="TEST"
VITE_FIREBASE_AUTH_EMULATOR_HOST=http://127.0.0.1:9099
22 changes: 15 additions & 7 deletions src/components/date_picker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ const DatePicker = ({
shortDateFormat,
view,
hideNav,
error,
helperText,
}: CustomDatePickerProps) => {
const poperRef = useRef<HTMLDivElement>(null);

Expand All @@ -40,7 +42,7 @@ const DatePicker = ({

const [height, setHeight] = useState(240); // Initial height
const [open, setOpen] = useState(false);
const [valueTmp, setValueTmp] = useState<Date>(value);
const [valueTmp, setValueTmp] = useState<Date | null>(value ?? null);

const slotFieldProps =
view === 'button' ? { field: ButtonField } : { textField: InputTextField };
Expand All @@ -56,18 +58,18 @@ const DatePicker = ({
const handleKeyDown = (e: KeyboardEvent<Element>) => {
if (e.key !== 'Enter') return;

const isValidDate = isValid(valueTmp);
const isValidDate = valueTmp instanceof Date && isValid(valueTmp);

if (!isValidDate) return;

onChange?.(valueTmp);
setOpen(false);
};

const handleValueChange = (value: Date) => {
const handleValueChange = (value: Date | null) => {
setValueTmp(value);

const isValidDate = isValid(value);
const isValidDate = value instanceof Date && isValid(value);

onChange?.(value);

Expand All @@ -77,15 +79,19 @@ const DatePicker = ({
};

useEffect(() => {
if (getWeeksInMonth(valueTmp, { weekStartsOn: 0 }) === 6) {
setHeight(290);
if (valueTmp) {
if (getWeeksInMonth(valueTmp, { weekStartsOn: 0 }) === 6) {
setHeight(290);
} else {
setHeight(240);
}
} else {
setHeight(240);
}
}, [valueTmp]);

useEffect(() => {
setValueTmp(value);
setValueTmp(value ?? null);
}, [value]);

return (
Expand Down Expand Up @@ -160,6 +166,8 @@ const DatePicker = ({
},
onKeyDown: handleKeyDown,
ref: poperRef,
error,
helperText,
},
}}
/>
Expand Down
16 changes: 10 additions & 6 deletions src/components/date_picker/index.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ export type CustomDatePickerView = 'button' | 'input';
* Props for the CustomDatePicker component.
*/
export interface CustomDatePickerProps {
/**
* The selected date value.
*/
value?: Date;
/**
* The selected date value.
*/
value?: Date | null;

/**
* The view type of the date picker.
Expand Down Expand Up @@ -40,8 +40,8 @@ export interface CustomDatePickerProps {
/**
* Function called when the selected date changes.
* @param value - The new selected date value.
*/
onChange?: (value: Date) => void | Promise<void>;
*/
onChange?: (value: Date | null) => void | Promise<void>;

/**
* The minimum selectable date.
Expand All @@ -56,4 +56,8 @@ export interface CustomDatePickerProps {
readOnly?: boolean;

hideNav?: boolean;

error?: boolean;

helperText?: string;
}
3 changes: 2 additions & 1 deletion src/components/date_picker/slots/toolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import { useAppTranslation } from '@hooks/index';
import { formatLongDateWithShortVars, isValidDate } from '@utils/date';
import Typography from '@components/typography';

type ToolbarProps = { selected: Date };
type ToolbarProps = { selected: Date | null };

const Toolbar = ({ selected }: ToolbarProps) => {
const { t } = useAppTranslation();

const value = useMemo(() => {
if (!selected) return '***';
if (!isValidDate(selected)) return '***';

return formatLongDateWithShortVars(selected);
Expand Down
119 changes: 90 additions & 29 deletions src/features/congregation/settings/circuit_overseer/useCircuitOverseer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import { dbAppSettingsUpdate } from '@services/dexie/settings';
import { generateDisplayName } from '@utils/common';

const useCircuitOverseer = () => {
const timer = useRef<NodeJS.Timeout>();
type FieldKey = 'firstname' | 'lastname' | 'displayname';

const saveTimers = useRef<Partial<Record<FieldKey, ReturnType<typeof setTimeout>>>>({});

const settings = useAtomValue(settingsState);
const fullnameOption = useAtomValue(fullnameOptionState);
Expand All @@ -18,70 +20,116 @@ const useCircuitOverseer = () => {
const [firstname, setFirstname] = useState('');
const [lastname, setLastname] = useState('');
const [displayname, setDisplayname] = useState('');
const [editing, setEditing] = useState<Record<FieldKey, boolean>>({
firstname: false,
lastname: false,
displayname: false,
});

const clearTimer = (key: FieldKey) => {
const timer = saveTimers.current[key];
if (timer) {
clearTimeout(timer);
saveTimers.current[key] = undefined;
}
};

const scheduleSave = (key: FieldKey, fn: () => Promise<void>, markCompleteKeys: FieldKey[]) => {
clearTimer(key);

saveTimers.current[key] = setTimeout(async () => {
saveTimers.current[key] = undefined;
await fn();
setEditing((prev) => {
const next = { ...prev };
for (const field of markCompleteKeys) {
next[field] = false;
}
return next;
});
}, 1000);
};
Comment on lines +37 to +51
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Race condition when firstname and lastname are edited in quick succession.

When a user edits firstname and then lastname before the firstname timer fires:

  1. Both fields mark displayname as editing
  2. Firstname timer completes after 1s, marks firstname and displayname as not editing
  3. At this point, displayname is no longer protected from external updates (line 154), even though lastname is still pending
  4. The useEffect may sync displayname from settings, potentially overwriting the lastname-derived value
  5. Lastname timer completes, but the derived displayname may have been lost

Consider tracking whether any name field is being edited before clearing displayname's editing state:

-  const scheduleSave = (key: FieldKey, fn: () => Promise<void>, markCompleteKeys: FieldKey[]) => {
+  const scheduleSave = (key: FieldKey, fn: () => Promise<void>) => {
     clearTimer(key);

     saveTimers.current[key] = setTimeout(async () => {
       saveTimers.current[key] = undefined;
       await fn();
       setEditing((prev) => {
         const next = { ...prev };
-        for (const field of markCompleteKeys) {
-          next[field] = false;
-        }
+        next[key] = false;
+        // Only mark displayname as not editing if neither firstname nor lastname is still editing
+        if (key === 'firstname' || key === 'lastname') {
+          if (!next.firstname && !next.lastname) {
+            next.displayname = false;
+          }
+        }
         return next;
       });
     }, 1000);
   };

Then update the call sites to remove the markCompleteKeys parameter (lines 85, 89, 93).

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/features/congregation/settings/circuit_overseer/useCircuitOverseer.tsx
around lines 37 to 51, the scheduleSave helper currently accepts
markCompleteKeys and unconditionally clears those editing flags when each timer
fires, which causes a race where firstname finishing can clear displayname while
lastname is still pending; change scheduleSave to only clear individual field
keys it directly saved and to avoid clearing shared/derived keys like
displayname unless no other related name timers remain (e.g., track active
name-field timers or an activeNames set/count and check it before clearing
displayname), and remove the now-unused markCompleteKeys parameter from the call
sites on lines 85, 89, and 93 so callers no longer rely on scheduleSave to clear
derived editing state.


const markEditing = (keys: FieldKey[]) => {
setEditing((prev) => {
const next = { ...prev };
for (const field of keys) {
next[field] = true;
}
return next;
});
};

const handleFirstnameChange = (value: string) => {
markEditing(['firstname', 'displayname']);
setFirstname(value);

const dispName = generateDisplayName(lastname, value);
setDisplayname(dispName);
};

const handleLastnameChange = (value: string) => {
markEditing(['lastname', 'displayname']);
setLastname(value);

const dispName = generateDisplayName(value, firstname);
setDisplayname(dispName);
};

const handleDisplaynameChange = (value: string) => setDisplayname(value);
const handleDisplaynameChange = (value: string) => {
markEditing(['displayname']);
setDisplayname(value);
};

const handleFirstnameSave = () => {
if (timer.current) clearTimeout(timer.current);

timer.current = setTimeout(handleFirstnameSaveDb, 1000);
scheduleSave('firstname', handleFirstnameSaveDb, ['firstname', 'displayname']);
};

const handleLastnameSave = () => {
if (timer.current) clearTimeout(timer.current);

timer.current = setTimeout(handleLastnameSaveDb, 1000);
scheduleSave('lastname', handleLastnameSaveDb, ['lastname', 'displayname']);
};

const handleDisplaynameSave = () => {
if (timer.current) clearTimeout(timer.current);

timer.current = setTimeout(handleDisplaynameSaveDb, 1000);
scheduleSave('displayname', handleDisplaynameSaveDb, ['displayname']);
};

const handleFirstnameSaveDb = async () => {
const circuitOverseer = structuredClone(
settings.cong_settings.circuit_overseer
const firstnameField = structuredClone(
settings.cong_settings.circuit_overseer.firstname
);
const displayNameField = structuredClone(
settings.cong_settings.circuit_overseer.display_name
);

circuitOverseer.firstname.value = firstname;
circuitOverseer.firstname.updatedAt = new Date().toISOString();
firstnameField.value = firstname;
firstnameField.updatedAt = new Date().toISOString();

circuitOverseer.display_name.value = displayname;
circuitOverseer.display_name.updatedAt = new Date().toISOString();
displayNameField.value = displayname;
displayNameField.updatedAt = new Date().toISOString();

await dbAppSettingsUpdate({
'cong_settings.circuit_overseer': circuitOverseer,
'cong_settings.circuit_overseer.firstname': firstnameField,
'cong_settings.circuit_overseer.display_name': displayNameField,
});
};

const handleLastnameSaveDb = async () => {
const circuitOverseer = structuredClone(
settings.cong_settings.circuit_overseer
const lastnameField = structuredClone(
settings.cong_settings.circuit_overseer.lastname
);
const displayNameField = structuredClone(
settings.cong_settings.circuit_overseer.display_name
);

circuitOverseer.lastname.value = lastname;
circuitOverseer.lastname.updatedAt = new Date().toISOString();
lastnameField.value = lastname;
lastnameField.updatedAt = new Date().toISOString();

circuitOverseer.display_name.value = displayname;
circuitOverseer.display_name.updatedAt = new Date().toISOString();
displayNameField.value = displayname;
displayNameField.updatedAt = new Date().toISOString();

await dbAppSettingsUpdate({
'cong_settings.circuit_overseer': circuitOverseer,
'cong_settings.circuit_overseer.lastname': lastnameField,
'cong_settings.circuit_overseer.display_name': displayNameField,
});
};

Expand All @@ -101,10 +149,23 @@ const useCircuitOverseer = () => {
useEffect(() => {
const co = settings.cong_settings.circuit_overseer;

setFirstname(co.firstname.value);
setLastname(co.lastname.value);
setDisplayname(co.display_name.value);
}, [settings]);
setFirstname((prev) => (editing.firstname ? prev : co.firstname.value));
setLastname((prev) => (editing.lastname ? prev : co.lastname.value));
setDisplayname((prev) => (editing.displayname ? prev : co.display_name.value));
}, [settings, editing]);

useEffect(() => {
const timersOnUnmount = saveTimers.current;

return () => {
(Object.keys(timersOnUnmount) as FieldKey[]).forEach((key) => {
const timer = timersOnUnmount[key];
if (timer) {
clearTimeout(timer);
}
});
};
}, []);

return {
fullnameOption,
Expand Down
47 changes: 44 additions & 3 deletions src/features/congregation/settings/circuit_overseer/week_item/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,63 @@ import { DatePicker } from '@components/index';
import { WeekItemType } from './index.types';
import useWeekItem from './useWeekItem';
import IconButton from '@components/icon_button';
import { formatDate, getWeekDate } from '@utils/date';

const WeekItem = ({ visit }: WeekItemType) => {
const WeekItem = ({ visit, error, helperText, onWeekChange, onDelete }: WeekItemType) => {
const { t } = useAppTranslation();

const { isAdmin } = useCurrentUser();

const { handleDateChange, handleDeleteVisit } = useWeekItem(visit);

const computeWeekOf = (date: Date | null) => {
if (date === null) {
return '';
}

return formatDate(getWeekDate(date), 'yyyy/MM/dd');
};

const handlePickerChange = async (date: Date | null) => {
const optimisticWeekOf = computeWeekOf(date);

if (onWeekChange) {
onWeekChange(visit.id, optimisticWeekOf);
}

try {
const nextWeekOf = await handleDateChange(date);

if (onWeekChange && nextWeekOf !== optimisticWeekOf) {
onWeekChange(visit.id, nextWeekOf);
}
} catch (error) {
if (onWeekChange) {
onWeekChange(visit.id, visit.weekOf);
}

console.error(error);
}
};

const handleDeleteClick = async () => {
await handleDeleteVisit();

if (onDelete) {
onDelete(visit.id);
}
};

return (
<Box sx={{ display: 'flex', gap: '16px' }}>
<DatePicker
disablePast
label={t('tr_coNextVisitWeek')}
value={visit.weekOf === '' ? null : new Date(visit.weekOf)}
onChange={(date) => handleDateChange(date)}
onChange={handlePickerChange}
readOnly={!isAdmin}
error={error}
helperText={helperText}
/>

{isAdmin && (
Expand All @@ -31,7 +72,7 @@ const WeekItem = ({ visit }: WeekItemType) => {
width: '48px',
height: '48px',
}}
onClick={handleDeleteVisit}
onClick={handleDeleteClick}
>
<IconDelete color="var(--red-main)" />
</IconButton>
Expand Down
4 changes: 4 additions & 0 deletions src/features/congregation/settings/circuit_overseer/week_item/index.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@ import { CircuitOverseerVisitType } from '@definition/settings';

export type WeekItemType = {
visit: CircuitOverseerVisitType;
error?: boolean;
helperText?: string;
onWeekChange?: (id: string, nextWeekOf: string) => void;
onDelete?: (id: string) => void;
};
Loading