From 72e0b36070e7fcf561b4a458c1c5c638b352fc99 Mon Sep 17 00:00:00 2001 From: Hariom Date: Mon, 6 Jan 2025 16:34:25 +0530 Subject: [PATCH 1/2] Make it easier to manage perf and also improve perf --- apps/web/public/static/locales/en/common.json | 3 + .../settings/attributes/AttributesForm.tsx | 297 +++++++++++------- 2 files changed, 194 insertions(+), 106 deletions(-) diff --git a/apps/web/public/static/locales/en/common.json b/apps/web/public/static/locales/en/common.json index ec7885d63aca4c..7bd416fb7f6493 100644 --- a/apps/web/public/static/locales/en/common.json +++ b/apps/web/public/static/locales/en/common.json @@ -2904,5 +2904,8 @@ "dry_run_mode_active": "You are doing a test booking.", "icon_showcase": "Icon showcase", "icons_showcase": "Icons showcase", + "choose_an_option": "Choose an option", + "enter_option_value": "Enter option value", + "remove_option": "Remove option", "ADD_NEW_STRINGS_ABOVE_THIS_LINE_TO_PREVENT_MERGE_CONFLICTS": "↑↑↑↑↑↑↑↑↑↑↑↑↑ Add your new strings above here ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑" } \ No newline at end of file diff --git a/packages/features/ee/organizations/pages/settings/attributes/AttributesForm.tsx b/packages/features/ee/organizations/pages/settings/attributes/AttributesForm.tsx index 96abe8d37ccc7a..aae1e6c0516bec 100644 --- a/packages/features/ee/organizations/pages/settings/attributes/AttributesForm.tsx +++ b/packages/features/ee/organizations/pages/settings/attributes/AttributesForm.tsx @@ -1,11 +1,9 @@ -import { useAutoAnimate } from "@formkit/auto-animate/react"; import { zodResolver } from "@hookform/resolvers/zod"; import React, { useState } from "react"; import { Controller, useForm, useFieldArray } from "react-hook-form"; import { z } from "zod"; import { useLocale } from "@calcom/lib/hooks/useLocale"; -import { entries } from "@calcom/prisma/zod-utils"; import { Button, Form, @@ -119,6 +117,178 @@ export function getGroupOptionUpdate({ } } +type AttributeOption = { + value: string; + id?: string; + assignedUsers?: number; + isGroup?: boolean; + contains?: string[]; + attributeOptionId?: string; +}; + +const NonGroupOption = ({ + option, + index, + form, + remove, + setDeleteOptionDialog, +}: { + option: AttributeOption; + index: number; + form: any; + remove: (index: number) => void; + setDeleteOptionDialog: (value: { id: number; open: boolean }) => void; +}) => { + const { t } = useLocale(); + return ( +
+
+
+ +
+
+ +
+ ); +}; + +const getNonGroupOptionsForSelect = (nonGroupOptions: AttributeOption[]) => { + return nonGroupOptions + .map((f) => ({ + label: f.value, + value: f.attributeOptionId, + })) + .filter((option): option is { label: string; value: string } => !!option.value); +}; + +const getSelectedNonGroupOptions = (option: AttributeOption, nonGroupOptions: AttributeOption[]) => { + return option.contains + ?.map((containedId: string) => { + const nonGroupOption = nonGroupOptions.find((opt) => opt.attributeOptionId === containedId); + if (!nonGroupOption?.value || !nonGroupOption?.attributeOptionId) return null; + return { + label: nonGroupOption.value, + value: nonGroupOption.attributeOptionId, + }; + }) + .filter((val): val is { label: string; value: string } => val !== null); +}; + +const GroupOption = ({ + option, + index, + form, + remove, + setDeleteOptionDialog, +}: { + option: AttributeOption; + index: number; + form: any; + remove: (index: number) => void; + setDeleteOptionDialog: (value: { id: number; open: boolean }) => void; +}) => { + const { t } = useLocale(); + const watchedOptions = form.getValues("options") as AttributeOption[]; + if (!watchedOptions) return null; + + const watchedNonGroupOptions = watchedOptions.filter((field): field is AttributeOption => !field.isGroup); + if (!watchedNonGroupOptions.length) return null; + + const nonGroupOptionsSelectFieldOptions = getNonGroupOptionsForSelect(watchedNonGroupOptions); + const nonGroupOptionsSelectFieldSelectedValue = getSelectedNonGroupOptions(option, watchedNonGroupOptions); + + return ( +
+
+
+ + { + const newContains = chosenNonGroupOptions.map((opt) => opt.value); + form.setValue(`options.${index}.contains`, newContains); + }} + containerClassName="w-full" + /> +
+
+ +
+ ); +}; + +const GroupOptions = ({ + fields, + watchedOptions, + form, + remove, + setDeleteOptionDialog, +}: { + fields: any[]; + watchedOptions: AttributeOption[]; + form: any; + remove: (index: number) => void; + setDeleteOptionDialog: (value: { id: number; open: boolean }) => void; +}) => { + const { t } = useLocale(); + return ( + <> + +
+ {fields.map((option, index) => { + const isAGroupOption = option.isGroup; + if (!isAGroupOption) return null; + return ( + + ); + })} +
+ + ); +}; + export function AttributeForm({ initialValues, onSubmit, header }: AttributeFormProps) { const { t } = useLocale(); const [deleteOptionDialog, setDeleteOptionDialog] = useState<{ @@ -128,10 +298,6 @@ export function AttributeForm({ initialValues, onSubmit, header }: AttributeForm id: undefined, open: false, }); - const [listRef] = useAutoAnimate({ - duration: 300, - easing: "ease-in-out", - }); // Needed because useFieldArray overrides the id field const initialValuesEnsuringThatOptionIdIsNotInId = initialValues @@ -161,8 +327,7 @@ export function AttributeForm({ initialValues, onSubmit, header }: AttributeForm name: "options", }); - const watchedOptions = form.watch("options"); - const watchedGroupOptions = watchedOptions.filter((field) => field.isGroup); + const watchedOptions = form.watch("options") as AttributeOption[]; const watchedType = form.watch("type"); return (
-
- {watchedOptions.map((nonGroupOption, index) => { - const isAGroupOption = nonGroupOption.isGroup; +
+ {fields.map((option, index) => { + const isAGroupOption = option.isGroup; if (isAGroupOption) return null; - const groupOptionsSelectFieldSelectedValue = watchedGroupOptions - ?.filter( - ({ contains }) => - nonGroupOption.attributeOptionId && - contains?.includes(nonGroupOption.attributeOptionId) - ) - .map((groupOption) => ({ - label: groupOption.value, - value: groupOption.attributeOptionId, - })); - - const groupOptionsSelectFieldOptions = watchedGroupOptions - .map((f) => ({ - label: f.value, - value: f.attributeOptionId, - })) - .filter((option): option is { label: string; value: string } => !!option.value); - return ( - <> -
-
- - { - // subOption is the option that is being added to groups - const subOption = nonGroupOption; - const optionsUpdate = getGroupOptionUpdate({ - // Spread makes it non-readonly - newGroupOptions: [...chosenGroupOptions], - previousGroupOptions: groupOptionsSelectFieldSelectedValue, - subOption, - allOptions: watchedOptions, - }); - entries(optionsUpdate).forEach(([index, value]) => { - form.setValue(`options.${index}.contains`, value); - }); - }} - className="min-w-64" - /> -
-
- - + ); })}
@@ -308,38 +418,13 @@ export function AttributeForm({ initialValues, onSubmit, header }: AttributeForm
- -
- {watchedOptions.map((watchedOption, index) => { - const isAGroupOption = watchedOption.isGroup; - if (!isAGroupOption) return null; - return ( - <> -
-
- -
-
- - - ); - })} -
+ } + /> + ); + }, + + addOptionToGroup: async (user: ReturnType) => { + const selects = await screen.findAllByText("choose_an_option"); + await user.click(selects[0]); + }, + + selectOption: async (user: ReturnType, optionText: string) => { + const option = await screen.findByText(optionText); + await user.click(option); + }, + + submitForm: async (user: ReturnType) => { + const submitButton = screen.getByRole("button", { name: "Save" }); + await user.click(submitButton); + }, + + deleteOption: async (user: ReturnType, index: number) => { + const deleteButtons = screen.getAllByTitle("remove_option"); + await user.click(deleteButtons[index]); + }, + + expectGroupContainsOptions: async (mockOnSubmit: Mock, groupValue: string, containsIds: string[]) => { + await waitFor(() => { + expect(mockOnSubmit).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.arrayContaining([ + expect.objectContaining({ + value: groupValue, + isGroup: true, + contains: expect.arrayContaining(containsIds), + }), + ]), + }) + ); + }); + }, + + expectGroupNotContainsOptions: async (mockOnSubmit: Mock, groupValue: string, notContainsIds: string[]) => { + await waitFor(() => { + expect(mockOnSubmit).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.arrayContaining([ + expect.objectContaining({ + value: groupValue, + isGroup: true, + contains: expect.not.arrayContaining(notContainsIds), + }), + ]), + }) + ); + }); + }, + + expectDeleteConfirmationDialog: () => { + expect(screen.getByText("delete_attribute")).toBeInTheDocument(); + expect(screen.getByText("delete_attribute_description")).toBeInTheDocument(); + }, + + expectOptionToBeDeleted: async (mockOnSubmit: Mock, optionValue: string) => { + await waitFor(() => { + expect(mockOnSubmit).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.not.arrayContaining([ + expect.objectContaining({ + value: optionValue, + }), + ]), + }) + ); + }); + }, +}; + +const initialOptionsWithAGroupHavingOptions: InitialOption[] = [ + { id: "1", value: "Engineering", isGroup: false, attributeOptionId: "1" }, + { id: "2", value: "Design", isGroup: false, attributeOptionId: "2" }, + { id: "3", value: "Product", isGroup: false, attributeOptionId: "3" }, + { + id: "4", + value: "Tech Teams", + isGroup: true, + contains: ["1", "3"], + attributeOptionId: "4", + }, +]; + +const initialOptionsWithAGroupHavingNoOptions: InitialOption[] = [ + { id: "1", value: "Engineering", isGroup: false, attributeOptionId: "1" }, + { id: "2", value: "Design", isGroup: false, attributeOptionId: "2" }, + { id: "3", value: "Product", isGroup: false, attributeOptionId: "3" }, + { + id: "4", + value: "Tech Teams", + isGroup: true, + contains: [], + attributeOptionId: "4", + }, +]; + +describe("AttributeForm", () => { + describe("Group Options Handling", () => { + it("should handle adding non-group options to existing group", async () => { + const { user, mockOnSubmit } = AttributeFormActions.setup(); + AttributeFormActions.render(initialOptionsWithAGroupHavingNoOptions, mockOnSubmit); + + await AttributeFormActions.addOptionToGroup(user); + await AttributeFormActions.selectOption(user, "Design"); + await AttributeFormActions.submitForm(user); + + await AttributeFormActions.expectGroupContainsOptions(mockOnSubmit, "Tech Teams", ["2"]); + }); + + it("should handle removing options from group when the option is deleted", async () => { + const { user, mockOnSubmit } = AttributeFormActions.setup(); + AttributeFormActions.render(initialOptionsWithAGroupHavingOptions, mockOnSubmit); + + await AttributeFormActions.deleteOption(user, 0); + await AttributeFormActions.submitForm(user); + await AttributeFormActions.expectGroupNotContainsOptions(mockOnSubmit, "Tech Teams", ["1"]); + await AttributeFormActions.expectOptionToBeDeleted(mockOnSubmit, "Engineering"); + }); + + it("should take confirmation before deleting option with assigned users", async () => { + const { user, mockOnSubmit } = AttributeFormActions.setup(); + const optionsWithAssignedUsers = initialOptionsWithAGroupHavingOptions.map((opt) => + opt.value === "Engineering" ? { ...opt, assignedUsers: 5 } : opt + ); + + AttributeFormActions.render(optionsWithAssignedUsers, mockOnSubmit); + await AttributeFormActions.deleteOption(user, 0); + AttributeFormActions.expectDeleteConfirmationDialog(); + const confirmationButton = screen.getByTestId("dialog-confirmation"); + await user.click(confirmationButton); + await AttributeFormActions.submitForm(user); + await AttributeFormActions.expectGroupNotContainsOptions(mockOnSubmit, "Tech Teams", ["1"]); + }); + }); +});