improvements: address feedback

backend/src/server/routes/v2/group-project-router.ts

@@ -308,6 +308,9 @@ export const registerGroupProjectRouter = async (server: FastifyZodProvider) =>
     method: "GET",
     url: "/:projectId/groups/:groupId/users",
     onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]),
+    config: {
+      rateLimit: readLimit
+    },
     schema: {
       hide: false,
       tags: [ApiDocsTags.ProjectGroups],

frontend/src/pages/project/AccessControlPage/components/GroupsTab/components/GroupsSection/GroupRoles.tsx

@@ -29,6 +29,7 @@ import { formatProjectRoleName } from "@app/helpers/roles";
 import { usePopUp } from "@app/hooks";
 import { useGetProjectRoles, useUpdateGroupWorkspaceRole } from "@app/hooks/api";
 import { TGroupMembership } from "@app/hooks/api/groups/types";
+import { TProjectRole } from "@app/hooks/api/roles/types";
 import { ProjectUserMembershipTemporaryMode } from "@app/hooks/api/workspace/types";
 import { groupBy } from "@app/lib/fn/array";
 
@@ -204,17 +205,22 @@ export type TMemberRolesProp = {
 
 const MAX_ROLES_TO_BE_SHOWN_IN_TABLE = 2;
 
-export const GroupRoles = ({
+type FormProps = {
-  roles = [],
+  projectRoles: Omit<TProjectRole, "permissions">[] | undefined;
-  disableEdit = false,
+  roles: TGroupMembership["roles"];
-  groupId,
+  groupId: string;
-  className,
+  onClose: VoidFunction;
-  popperContentProps
+};
-}: TMemberRolesProp) => {
+
+const GroupRolesForm = ({ projectRoles, roles, groupId, onClose }: FormProps) => {
   const { currentWorkspace } = useWorkspace();
-  const { popUp, handlePopUpToggle } = usePopUp(["editRole"] as const);
+
   const [searchRoles, setSearchRoles] = useState("");
 
+  const userRolesGroupBySlug = groupBy(roles, ({ customRoleSlug, role }) => customRoleSlug || role);
+
+  const updateGroupWorkspaceRole = useUpdateGroupWorkspaceRole();
+
   const {
     handleSubmit,
     control,
@@ -224,13 +230,6 @@ export const GroupRoles = ({
     resolver: zodResolver(formSchema)
   });
 
-  const { data: projectRoles, isPending: isRolesLoading } = useGetProjectRoles(
-    currentWorkspace?.id ?? ""
-  );
-  const userRolesGroupBySlug = groupBy(roles, ({ customRoleSlug, role }) => customRoleSlug || role);
-
-  const updateGroupWorkspaceRole = useUpdateGroupWorkspaceRole();
-
   const handleRoleUpdate = async (data: TForm) => {
     const selectedRoles = Object.keys(data)
       .filter((el) => Boolean(data[el].isChecked))
@@ -261,13 +260,126 @@ export const GroupRoles = ({
         roles: selectedRoles
       });
       createNotification({ text: "Successfully updated group role", type: "success" });
-      handlePopUpToggle("editRole");
+      onClose();
       setSearchRoles("");
     } catch {
       createNotification({ text: "Failed to update group role", type: "error" });
     }
   };
 
+  return (
+    <form onSubmit={handleSubmit(handleRoleUpdate)} id="role-update-form">
+      <div className="thin-scrollbar max-h-80 space-y-4 overflow-y-auto">
+        {projectRoles
+          ?.filter(
+            ({ name, slug }) =>
+              name.toLowerCase().includes(searchRoles.toLowerCase()) ||
+              slug.toLowerCase().includes(searchRoles.toLowerCase())
+          )
+          ?.map(({ id, name, slug }) => {
+            const userProjectRoleDetails = userRolesGroupBySlug?.[slug]?.[0];
+
+            return (
+              <div key={id} className="flex items-center space-x-4">
+                <div className="flex-grow">
+                  <Controller
+                    control={control}
+                    defaultValue={Boolean(userProjectRoleDetails?.id)}
+                    name={`${slug}.isChecked`}
+                    render={({ field }) => (
+                      <Checkbox
+                        id={slug}
+                        isChecked={field.value}
+                        onCheckedChange={(isChecked) => {
+                          field.onChange(isChecked);
+                          setValue(`${slug}.temporaryAccess`, false);
+                        }}
+                      >
+                        {name}
+                      </Checkbox>
+                    )}
+                  />
+                </div>
+                <div>
+                  <Controller
+                    control={control}
+                    name={`${slug}.temporaryAccess`}
+                    defaultValue={
+                      userProjectRoleDetails?.isTemporary
+                        ? {
+                            isTemporary: true,
+                            temporaryAccessStartTime:
+                              userProjectRoleDetails.temporaryAccessStartTime as string,
+                            temporaryRange: userProjectRoleDetails.temporaryRange as string,
+                            temporaryAccessEndTime: userProjectRoleDetails.temporaryAccessEndTime
+                          }
+                        : false
+                    }
+                    render={({ field }) => (
+                      <IdentityTemporaryRoleForm
+                        temporaryConfig={
+                          typeof field.value === "boolean"
+                            ? { isTemporary: field.value }
+                            : field.value
+                        }
+                        onSetTemporary={(data) => {
+                          setValue(`${slug}.isChecked`, true, { shouldDirty: true });
+                          field.onChange({ isTemporary: true, ...data });
+                        }}
+                        onRemoveTemporary={() => {
+                          setValue(`${slug}.isChecked`, false, { shouldDirty: true });
+                          field.onChange(false);
+                        }}
+                      />
+                    )}
+                  />
+                </div>
+              </div>
+            );
+          })}
+      </div>
+      <div className="mt-3 flex items-center space-x-2 border-t border-t-gray-700 pt-3">
+        <div>
+          <Input
+            className="w-full p-1.5 pl-8"
+            size="xs"
+            value={searchRoles}
+            onChange={(el) => setSearchRoles(el.target.value)}
+            leftIcon={<FontAwesomeIcon icon={faSearch} />}
+            placeholder="Search roles.."
+          />
+        </div>
+        <div>
+          <Button
+            size="xs"
+            type="submit"
+            form="role-update-form"
+            leftIcon={<FontAwesomeIcon icon={faCheck} />}
+            isDisabled={!isDirty || isSubmitting}
+            isLoading={isSubmitting}
+          >
+            Save
+          </Button>
+        </div>
+      </div>
+    </form>
+  );
+};
+
+export const GroupRoles = ({
+  roles = [],
+  disableEdit = false,
+  groupId,
+  className,
+  popperContentProps
+}: TMemberRolesProp) => {
+  const { currentWorkspace } = useWorkspace();
+  const { popUp, handlePopUpToggle } = usePopUp(["editRole"] as const);
+
+  const { data: projectRoles, isPending: isRolesLoading } = useGetProjectRoles(
+    currentWorkspace?.id ?? ""
+  );
+
   return (
     <div className={twMerge("flex items-center space-x-1", className)}>
       {roles
@@ -353,103 +465,12 @@ export const GroupRoles = ({
                 <Spinner />
               </div>
             ) : (
-              <form onSubmit={handleSubmit(handleRoleUpdate)} id="role-update-form">
+              <GroupRolesForm
-                <div className="thin-scrollbar max-h-80 space-y-4 overflow-y-auto">
+                projectRoles={projectRoles}
-                  {projectRoles
+                groupId={groupId}
-                    ?.filter(
+                roles={roles}
-                      ({ name, slug }) =>
+                onClose={() => handlePopUpToggle("editRole")}
-                        name.toLowerCase().includes(searchRoles.toLowerCase()) ||
+              />
-                        slug.toLowerCase().includes(searchRoles.toLowerCase())
-                    )
-                    ?.map(({ id, name, slug }) => {
-                      const userProjectRoleDetails = userRolesGroupBySlug?.[slug]?.[0];
-
-                      return (
-                        <div key={id} className="flex items-center space-x-4">
-                          <div className="flex-grow">
-                            <Controller
-                              control={control}
-                              defaultValue={Boolean(userProjectRoleDetails?.id)}
-                              name={`${slug}.isChecked`}
-                              render={({ field }) => (
-                                <Checkbox
-                                  id={slug}
-                                  isChecked={field.value}
-                                  onCheckedChange={(isChecked) => {
-                                    field.onChange(isChecked);
-                                    setValue(`${slug}.temporaryAccess`, false);
-                                  }}
-                                >
-                                  {name}
-                                </Checkbox>
-                              )}
-                            />
-                          </div>
-                          <div>
-                            <Controller
-                              control={control}
-                              name={`${slug}.temporaryAccess`}
-                              defaultValue={
-                                userProjectRoleDetails?.isTemporary
-                                  ? {
-                                      isTemporary: true,
-                                      temporaryAccessStartTime:
-                                        userProjectRoleDetails.temporaryAccessStartTime as string,
-                                      temporaryRange:
-                                        userProjectRoleDetails.temporaryRange as string,
-                                      temporaryAccessEndTime:
-                                        userProjectRoleDetails.temporaryAccessEndTime
-                                    }
-                                  : false
-                              }
-                              render={({ field }) => (
-                                <IdentityTemporaryRoleForm
-                                  temporaryConfig={
-                                    typeof field.value === "boolean"
-                                      ? { isTemporary: field.value }
-                                      : field.value
-                                  }
-                                  onSetTemporary={(data) => {
-                                    setValue(`${slug}.isChecked`, true, { shouldDirty: true });
-                                    field.onChange({ isTemporary: true, ...data });
-                                  }}
-                                  onRemoveTemporary={() => {
-                                    setValue(`${slug}.isChecked`, false, { shouldDirty: true });
-                                    field.onChange(false);
-                                  }}
-                                />
-                              )}
-                            />
-                          </div>
-                        </div>
-                      );
-                    })}
-                </div>
-                <div className="mt-3 flex items-center space-x-2 border-t border-t-gray-700 pt-3">
-                  <div>
-                    <Input
-                      className="w-full p-1.5 pl-8"
-                      size="xs"
-                      value={searchRoles}
-                      onChange={(el) => setSearchRoles(el.target.value)}
-                      leftIcon={<FontAwesomeIcon icon={faSearch} />}
-                      placeholder="Search roles.."
-                    />
-                  </div>
-                  <div>
-                    <Button
-                      size="xs"
-                      type="submit"
-                      form="role-update-form"
-                      leftIcon={<FontAwesomeIcon icon={faCheck} />}
-                      isDisabled={!isDirty || isSubmitting}
-                      isLoading={isSubmitting}
-                    >
-                      Save
-                    </Button>
-                  </div>
-                </div>
-              </form>
             )}
           </PopoverContent>
         </Popover>

frontend/src/pages/project/GroupDetailsByIDPage/GroupDetailsByIDPage.tsx

@@ -54,7 +54,7 @@ export const GroupDetailsByIDPage = () => {
   return (
     <>
       <Helmet>
-        <title>{t("common.head-title", { title: t("settings.members.title") })}</title>
+        <title>{t("common.head-title", { title: "Project Group" })}</title>
         <link rel="icon" href="/infisical.ico" />
       </Helmet>
       <ProjectPermissionCan

frontend/src/pages/project/GroupDetailsByIDPage/components/GroupDetailsSection.tsx

@@ -41,7 +41,7 @@ export const GroupDetailsSection = ({ groupMembership }: Props) => {
       });
 
       createNotification({
-        text: "Successfully removed identity from project",
+        text: "Successfully removed group from project",
         type: "success"
       });