From b09b32b121adb50f903bcfdadf62a39ae1a6f68b Mon Sep 17 00:00:00 2001 From: Alex van Andel Date: Tue, 13 Aug 2024 17:44:16 +0200 Subject: [PATCH] fix: scheduleId not unset properly bug (#15993) * fix: scheduleId not unset properly bug * Refactor almost complete, couple of tests needed * More testid fixes * Add migration to fixup schedule id bug * Remove migration * fix: Refactor useLockedFieldManager for correct useState use --------- Co-authored-by: zomars --- .../eventtype/EventAvailabilityTab.tsx | 230 ++++++++---------- .../views/event-types-single-view.tsx | 19 +- packages/core/getUserAvailability.ts | 4 +- .../hooks/useLockedFieldsManager.tsx | 72 +++--- .../features/ee/teams/components/TeamList.tsx | 2 +- packages/ui/components/card/Card.tsx | 8 +- packages/ui/components/data-table/index.tsx | 2 +- 7 files changed, 155 insertions(+), 182 deletions(-) diff --git a/apps/web/components/eventtype/EventAvailabilityTab.tsx b/apps/web/components/eventtype/EventAvailabilityTab.tsx index eaa585e274..7eb97c7af8 100644 --- a/apps/web/components/eventtype/EventAvailabilityTab.tsx +++ b/apps/web/components/eventtype/EventAvailabilityTab.tsx @@ -66,26 +66,17 @@ const format = (date: Date, hour12: boolean) => }).format(new Date(dayjs.utc(date).format("YYYY-MM-DDTHH:mm:ss"))); const EventTypeScheduleDetails = memo( - ({ - isManagedEventType, - selectedScheduleValue, - }: { - isManagedEventType: boolean; - selectedScheduleValue: AvailabilityOption | undefined; - }) => { + ({ isManagedEventType, scheduleId }: { isManagedEventType: boolean; scheduleId: number | null }) => { const { data: loggedInUser } = useMeQuery(); const timeFormat = loggedInUser?.timeFormat; const { t, i18n } = useLocale(); - const { watch } = useFormContext(); - const scheduleId = watch("schedule"); const { isPending, data: schedule } = trpc.viewer.availability.schedule.get.useQuery( { - scheduleId: - scheduleId || loggedInUser?.defaultScheduleId || selectedScheduleValue?.value || undefined, + scheduleId: scheduleId || loggedInUser?.defaultScheduleId || undefined, isManagedEventType, }, - { enabled: !!scheduleId || !!loggedInUser?.defaultScheduleId || !!selectedScheduleValue } + { enabled: !!scheduleId || !!loggedInUser?.defaultScheduleId } ); const weekStart = weekStartNum(loggedInUser?.weekStart); @@ -159,117 +150,100 @@ const EventTypeSchedule = ({ eventType }: { eventType: EventTypeSetup }) => { const formMethods = useFormContext(); const { shouldLockIndicator, shouldLockDisableProps, isManagedEventType, isChildrenManagedEventType } = useLockedFieldsManager({ eventType, translate: t, formMethods }); - const { watch, setValue, getValues } = formMethods; - const watchSchedule = watch("schedule"); - const [options, setOptions] = useState([]); + const { watch, setValue } = formMethods; const { data, isPending } = trpc.viewer.availability.list.useQuery(undefined); - useEffect( - function refactorMeWithoutEffect() { - if (!data) { - return; - } - const schedules = data.schedules; - - const options = schedules.map((schedule) => ({ - value: schedule.id, - label: schedule.name, - isDefault: schedule.isDefault, - isManaged: false, - })); - - // We are showing a managed event for a team admin, so adding the option to let members choose their schedule - if (isManagedEventType) { - options.push({ - value: 0, - label: t("members_default_schedule"), - isDefault: false, - isManaged: false, - }); - } - - // We are showing a managed event for a member and team owner selected their own schedule, so adding - // the managed schedule option - if ( - isChildrenManagedEventType && - watchSchedule && - !schedules.find((schedule) => schedule.id === watchSchedule) - ) { - options.push({ - value: watchSchedule, - label: eventType.scheduleName ?? t("default_schedule_name"), - isDefault: false, - isManaged: false, - }); - } - // We push the selected schedule from the event type if it's not part of the list response. This happens if the user is an admin but not the schedule owner. - else if (eventType.schedule && !schedules.find((schedule) => schedule.id === eventType.schedule)) { - options.push({ - value: eventType.schedule, - label: eventType.scheduleName ?? t("default_schedule_name"), - isDefault: false, - isManaged: false, - }); - } - - setOptions(options); - - const scheduleId = getValues("schedule"); - const value = options.find((option) => - scheduleId - ? option.value === scheduleId - : isManagedEventType - ? option.value === 0 - : option.value === schedules.find((schedule) => schedule.isDefault)?.id - ); - - setValue("availability", value, { shouldDirty: true }); - }, - [data] - ); - const availabilityValue = watch("availability"); + const scheduleId = watch("schedule"); useEffect(() => { - if (!availabilityValue?.value) return; - setValue("schedule", availabilityValue.value, { shouldDirty: true }); - }, [availabilityValue, setValue]); + // after data is loaded. + if (data && scheduleId !== 0 && !scheduleId) { + const newValue = isManagedEventType ? 0 : data.schedules.find((schedule) => schedule.isDefault)?.id; + if (!newValue && newValue !== 0) return; + setValue("schedule", newValue, { + shouldDirty: true, + }); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [scheduleId, data]); + + if (isPending || !data) { + return ; + } + + const schedules = data.schedules; + + const options = schedules.map((schedule) => ({ + value: schedule.id, + label: schedule.name, + isDefault: schedule.isDefault, + isManaged: false, + })); + + // We are showing a managed event for a team admin, so adding the option to let members choose their schedule + if (isManagedEventType) { + options.push({ + value: 0, + label: t("members_default_schedule"), + isDefault: false, + isManaged: false, + }); + } + // We are showing a managed event for a member and team owner selected their own schedule, so adding + // the managed schedule option + if (isChildrenManagedEventType && scheduleId && !schedules.find((schedule) => schedule.id === scheduleId)) { + options.push({ + value: scheduleId, + label: eventType.scheduleName ?? t("default_schedule_name"), + isDefault: false, + isManaged: false, + }); + } + // We push the selected schedule from the event type if it's not part of the list response. This happens if the user is an admin but not the schedule owner. + else if (eventType.schedule && !schedules.find((schedule) => schedule.id === eventType.schedule)) { + options.push({ + value: eventType.schedule, + label: eventType.scheduleName ?? t("default_schedule_name"), + isDefault: false, + isManaged: false, + }); + } return (
- {isPending && } - {!isPending && ( - { - return ( - { + if (selected) onChange(selected.value); + }} + className="block w-full min-w-0 flex-1 rounded-sm text-sm" + value={optionValue} + components={{ Option, SingleValue }} + isMulti={false} + /> + ); + }} + />
- {availabilityValue?.value !== 0 ? ( + {scheduleId !== 0 ? ( ) : ( @@ -283,25 +257,27 @@ const EventTypeSchedule = ({ eventType }: { eventType: EventTypeSetup }) => { const UseCommonScheduleSettingsToggle = ({ eventType }: { eventType: EventTypeSetup }) => { const { t } = useLocale(); - const { setValue } = useFormContext(); + const { setValue, resetField, getFieldState, getValues } = useFormContext(); + + const [useHostSchedulesForTeamEvent, setUseHostSchedulesForTeamEvent] = useState( + Boolean(getFieldState("schedule").isDirty ? getValues("schedule") : eventType.schedule) + ); + return ( - ( - { - onChange(!checked); - if (!checked) { - setValue("schedule", null, { shouldDirty: true }); - } - }} - title={t("choose_common_schedule_team_event")} - description={t("choose_common_schedule_team_event_description")}> - - - )} - /> + { + setUseHostSchedulesForTeamEvent(checked); + if (checked) { + if (Boolean(eventType.schedule)) resetField("schedule"); + } else { + setValue("schedule", null, { shouldDirty: Boolean(eventType.schedule) }); + } + }} + title={t("choose_common_schedule_team_event")} + description={t("choose_common_schedule_team_event_description")}> + + ); }; diff --git a/apps/web/modules/event-types/views/event-types-single-view.tsx b/apps/web/modules/event-types/views/event-types-single-view.tsx index 2b1157d29b..e53fd01e23 100644 --- a/apps/web/modules/event-types/views/event-types-single-view.tsx +++ b/apps/web/modules/event-types/views/event-types-single-view.tsx @@ -248,21 +248,6 @@ const EventTypePage = (props: EventTypeSetupProps & { allActiveWorkflows?: Workf endDate: new Date(eventType.periodEndDate || Date.now()), }); - const metadata = eventType.metadata; - // fallback to !!eventType.schedule when 'useHostSchedulesForTeamEvent' is undefined - if (!!team && metadata !== null) { - metadata.config = { - ...metadata.config, - useHostSchedulesForTeamEvent: - typeof eventType.metadata?.config?.useHostSchedulesForTeamEvent !== "undefined" - ? eventType.metadata?.config?.useHostSchedulesForTeamEvent === true - : !!eventType.schedule, - }; - } else { - // Make sure non-team events NEVER have this config key; - delete metadata?.config?.useHostSchedulesForTeamEvent; - } - const bookingFields: Prisma.JsonObject = {}; eventType.bookingFields.forEach(({ name }) => { @@ -310,7 +295,7 @@ const EventTypePage = (props: EventTypeSetupProps & { allActiveWorkflows?: Workf requiresConfirmation: eventType.requiresConfirmation, slotInterval: eventType.slotInterval, minimumBookingNotice: eventType.minimumBookingNotice, - metadata, + metadata: eventType.metadata, hosts: eventType.hosts, successRedirectUrl: eventType.successRedirectUrl || "", forwardParamsSuccessRedirect: eventType.forwardParamsSuccessRedirect, @@ -345,7 +330,7 @@ const EventTypePage = (props: EventTypeSetupProps & { allActiveWorkflows?: Workf schedulerName: eventType.aiPhoneCallConfig?.schedulerName, }, }; - }, [eventType, periodDates, metadata]); + }, [eventType, periodDates]); const formMethods = useForm({ defaultValues, resolver: zodResolver( diff --git a/packages/core/getUserAvailability.ts b/packages/core/getUserAvailability.ts index 4cee7ac818..dbaf26325b 100644 --- a/packages/core/getUserAvailability.ts +++ b/packages/core/getUserAvailability.ts @@ -323,8 +323,7 @@ const _getUserAvailability = async function getUsersWorkingHoursLifeTheUniverseA (schedule) => !user?.defaultScheduleId || schedule.id === user?.defaultScheduleId )[0]; - const useHostSchedulesForTeamEvent = eventType?.metadata?.config?.useHostSchedulesForTeamEvent; - const schedule = !useHostSchedulesForTeamEvent && eventType?.schedule ? eventType.schedule : userSchedule; + const schedule = eventType?.schedule ? eventType.schedule : userSchedule; const isDefaultSchedule = userSchedule && userSchedule.id === schedule.id; @@ -334,7 +333,6 @@ const _getUserAvailability = async function getUsersWorkingHoursLifeTheUniverseA chosenSchedule: schedule, eventTypeSchedule: eventType?.schedule, userSchedule: userSchedule, - useHostSchedulesForTeamEvent: eventType?.metadata?.config?.useHostSchedulesForTeamEvent, }) ); diff --git a/packages/features/ee/managed-event-types/hooks/useLockedFieldsManager.tsx b/packages/features/ee/managed-event-types/hooks/useLockedFieldsManager.tsx index 0c14d27633..84fa838c99 100644 --- a/packages/features/ee/managed-event-types/hooks/useLockedFieldsManager.tsx +++ b/packages/features/ee/managed-event-types/hooks/useLockedFieldsManager.tsx @@ -15,7 +15,7 @@ import { Badge, Icon, Switch, Tooltip } from "@calcom/ui"; export const LockedSwitch = ( isManagedEventType: boolean, - [isLocked, setIsLocked]: [boolean, Dispatch>], + [fieldState, setFieldState]: [Record, Dispatch>>], fieldName: string, setUnlockedFields: (fieldName: string, val: boolean | undefined) => void, options = { simple: false } @@ -24,10 +24,13 @@ export const LockedSwitch = ( { - setIsLocked(enabled); + setFieldState({ + ...fieldState, + [fieldName]: enabled, + }); setUnlockedFields(fieldName, !enabled || undefined); }} - checked={isLocked} + checked={fieldState[fieldName]} small={!options.simple} /> ) : null; @@ -36,12 +39,13 @@ export const LockedSwitch = ( export const LockedIndicator = ( isChildrenManagedEventType: boolean, isManagedEventType: boolean, - [isLocked, setIsLocked]: [boolean, Dispatch>], + [fieldState, setFieldState]: [Record, Dispatch>>], t: TFunction, fieldName: string, setUnlockedFields: (fieldName: string, val: boolean | undefined) => void, options = { simple: false } ) => { + const isLocked = fieldState[fieldName]; const stateText = t(isLocked ? "locked" : "unlocked"); const tooltipText = t( `${isLocked ? "locked" : "unlocked"}_fields_${isManagedEventType ? "admin" : "member"}_description` @@ -67,7 +71,10 @@ export const LockedIndicator = ( { - setIsLocked(enabled); + setFieldState({ + ...fieldState, + [fieldName]: enabled, + }); setUnlockedFields(fieldName, !enabled || undefined); }} checked={isLocked} @@ -91,7 +98,7 @@ const useLockedFieldsManager = ({ formMethods: UseFormReturn; }) => { const { setValue, getValues } = formMethods; - const fieldStates: Record>]> = {}; + const [fieldStates, setFieldStates] = useState>({}); const unlockedFields = (eventType.metadata?.managedEventConfig?.unlockedFields !== undefined && eventType.metadata?.managedEventConfig?.unlockedFields) || @@ -123,28 +130,31 @@ const useLockedFieldsManager = ({ const getLockedInitState = (fieldName: string): boolean => { let locked = isManagedEventType || isChildrenManagedEventType; - const unlockedFieldList = getValues("metadata")?.managedEventConfig?.unlockedFields; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - //@ts-ignore - const fieldIsUnlocked = unlockedFieldList && unlockedFieldList[fieldName] === true; + if (fieldName.includes(".")) { locked = locked && get(unlockedFields, fieldName) === undefined; } else { + type FieldName = string; + const unlockedFieldList = getValues("metadata")?.managedEventConfig?.unlockedFields as + | Record + | undefined; + const fieldIsUnlocked = !!unlockedFieldList?.[fieldName]; locked = locked && !fieldIsUnlocked; } return locked; }; const useShouldLockIndicator = (fieldName: string, options?: { simple: true }) => { - if (!fieldStates[fieldName]) { - // eslint-disable-next-line react-hooks/rules-of-hooks - fieldStates[fieldName] = useState(getLockedInitState(fieldName)); + if (typeof fieldStates[fieldName] === "undefined") { + setFieldStates({ + ...fieldStates, + [fieldName]: getLockedInitState(fieldName), + }); } - return LockedIndicator( isChildrenManagedEventType, isManagedEventType, - fieldStates[fieldName], + [fieldStates, setFieldStates], translate, fieldName, setUnlockedFields, @@ -153,12 +163,13 @@ const useLockedFieldsManager = ({ }; const useLockedLabel = (fieldName: string, options?: { simple: true }) => { - if (!fieldStates[fieldName]) { - // eslint-disable-next-line react-hooks/rules-of-hooks - fieldStates[fieldName] = useState(getLockedInitState(fieldName)); + if (typeof fieldStates[fieldName] === "undefined") { + setFieldStates({ + ...fieldStates, + [fieldName]: getLockedInitState(fieldName), + }); } - const isLocked = fieldStates[fieldName][0]; - + const isLocked = fieldStates[fieldName]; return { disabled: !isManagedEventType && @@ -170,19 +181,22 @@ const useLockedFieldsManager = ({ }; const useLockedSwitch = (fieldName: string, options = { simple: false }) => { - if (!fieldStates[fieldName]) { - // eslint-disable-next-line react-hooks/rules-of-hooks - fieldStates[fieldName] = useState(getLockedInitState(fieldName)); + if (typeof fieldStates[fieldName] === "undefined") { + setFieldStates({ + ...fieldStates, + [fieldName]: getLockedInitState(fieldName), + }); } - return () => - LockedSwitch(isManagedEventType, fieldStates[fieldName], fieldName, setUnlockedFields, options); + LockedSwitch(isManagedEventType, [fieldStates, setFieldStates], fieldName, setUnlockedFields, options); }; const useShouldLockDisableProps = (fieldName: string, options?: { simple: true }) => { - if (!fieldStates[fieldName]) { - // eslint-disable-next-line react-hooks/rules-of-hooks - fieldStates[fieldName] = useState(getLockedInitState(fieldName)); + if (typeof fieldStates[fieldName] === "undefined") { + setFieldStates({ + ...fieldStates, + [fieldName]: getLockedInitState(fieldName), + }); } return { disabled: @@ -190,7 +204,7 @@ const useLockedFieldsManager = ({ eventType.metadata?.managedEventConfig !== undefined && unlockedFields[fieldName as keyof Omit] === undefined, LockedIcon: useShouldLockIndicator(fieldName, options), - isLocked: fieldStates[fieldName][0], + isLocked: fieldStates[fieldName], }; }; diff --git a/packages/features/ee/teams/components/TeamList.tsx b/packages/features/ee/teams/components/TeamList.tsx index dce38b05f4..63d2bdaa3c 100644 --- a/packages/features/ee/teams/components/TeamList.tsx +++ b/packages/features/ee/teams/components/TeamList.tsx @@ -75,7 +75,7 @@ export default function TeamList(props: Props) { actionButton={{ href: `/settings/organizations/new`, child: t("set_up_your_organization"), - "data-testId": "setup_your_org_action_button", + "data-testid": "setup_your_org_action_button", }} /> ) => void; - "data-testId"?: string; + "data-testid"?: string; }; learnMore?: { href: string; @@ -177,7 +177,7 @@ export function Card({ target="_blank" rel="noreferrer" href={mediaLink} - data-testId={actionButton?.["data-testId"]} + data-testid={actionButton?.["data-testid"]} className="group relative my-3 flex aspect-video items-center overflow-hidden rounded">
+ data-testid={actionButton["data-testid"]}> {actionButton?.child}
@@ -232,7 +232,7 @@ export function Card({ diff --git a/packages/ui/components/data-table/index.tsx b/packages/ui/components/data-table/index.tsx index 77cc6eef35..4314e52f73 100644 --- a/packages/ui/components/data-table/index.tsx +++ b/packages/ui/components/data-table/index.tsx @@ -113,7 +113,7 @@ export function DataTable({ tableCTA={tableCTA} />
- +
{table.getHeaderGroups().map((headerGroup) => (