From 4796bcd579ca67e876771daa0e2e702bbd158967 Mon Sep 17 00:00:00 2001 From: Piyush Gupta Date: Thu, 20 Nov 2025 22:50:10 +0530 Subject: [PATCH] fix: review comments --- backend/src/ee/routes/v1/group-router.ts | 49 +++++++++++++++++-- backend/src/ee/services/group/group-dal.ts | 23 ++++++--- .../src/ee/services/group/group-service.ts | 6 ++- backend/src/ee/services/group/group-types.ts | 8 ++- backend/src/lib/api-docs/constants.ts | 4 +- frontend/src/hooks/api/groups/queries.tsx | 27 ++++++++-- .../components/AddGroupProjectModal.tsx | 13 +++-- .../GroupMembersSection.tsx | 2 +- .../GroupProjectsSection/GroupProjectRow.tsx | 6 ++- .../GroupProjectsSection.tsx | 8 +-- .../GroupProjectsTable.tsx | 44 +++++------------ 11 files changed, 127 insertions(+), 63 deletions(-) diff --git a/backend/src/ee/routes/v1/group-router.ts b/backend/src/ee/routes/v1/group-router.ts index 2186438ca..4696bef26 100644 --- a/backend/src/ee/routes/v1/group-router.ts +++ b/backend/src/ee/routes/v1/group-router.ts @@ -1,8 +1,14 @@ import { z } from "zod"; import { GroupsSchema, OrgMembershipRole, ProjectsSchema, UsersSchema } from "@app/db/schemas"; -import { EFilterReturnedProjects, EFilterReturnedUsers } from "@app/ee/services/group/group-types"; +import { + EFilterReturnedProjects, + EFilterReturnedUsers, + EGroupProjectsOrderBy +} from "@app/ee/services/group/group-types"; import { ApiDocsTags, GROUPS } from "@app/lib/api-docs"; +import { OrderByDirection } from "@app/lib/types"; +import { readLimit, writeLimit } from "@app/server/config/rateLimiter"; import { slugSchema } from "@app/server/lib/schemas"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { AuthMode } from "@app/services/auth/auth-type"; @@ -11,6 +17,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { server.route({ url: "/", method: "POST", + config: { + rateLimit: writeLimit + }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), schema: { hide: false, @@ -40,6 +49,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { server.route({ url: "/:id", method: "GET", + config: { + rateLimit: readLimit + }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), schema: { hide: false, @@ -69,6 +81,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { server.route({ url: "/", method: "GET", + config: { + rateLimit: readLimit + }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), schema: { hide: false, @@ -93,6 +108,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { server.route({ url: "/:id", method: "PATCH", + config: { + rateLimit: writeLimit + }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), schema: { hide: false, @@ -128,6 +146,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { server.route({ url: "/:id", method: "DELETE", + config: { + rateLimit: writeLimit + }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), schema: { hide: false, @@ -155,6 +176,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { server.route({ method: "GET", url: "/:id/users", + config: { + rateLimit: readLimit + }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), schema: { hide: false, @@ -163,7 +187,7 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { id: z.string().trim().describe(GROUPS.LIST_USERS.id) }), querystring: z.object({ - offset: z.coerce.number().min(0).max(100).default(0).describe(GROUPS.LIST_USERS.offset), + offset: z.coerce.number().min(0).default(0).describe(GROUPS.LIST_USERS.offset), limit: z.coerce.number().min(1).max(100).default(10).describe(GROUPS.LIST_USERS.limit), username: z.string().trim().optional().describe(GROUPS.LIST_USERS.username), search: z.string().trim().optional().describe(GROUPS.LIST_USERS.search), @@ -206,6 +230,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { server.route({ method: "GET", url: "/:id/projects", + config: { + rateLimit: readLimit + }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), schema: { hide: false, @@ -214,10 +241,18 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { id: z.string().trim().describe(GROUPS.LIST_PROJECTS.id) }), querystring: z.object({ - offset: z.coerce.number().min(0).max(100).default(0).describe(GROUPS.LIST_PROJECTS.offset), + offset: z.coerce.number().min(0).default(0).describe(GROUPS.LIST_PROJECTS.offset), limit: z.coerce.number().min(1).max(100).default(10).describe(GROUPS.LIST_PROJECTS.limit), search: z.string().trim().optional().describe(GROUPS.LIST_PROJECTS.search), - filter: z.nativeEnum(EFilterReturnedProjects).optional().describe(GROUPS.LIST_PROJECTS.filterProjects) + filter: z.nativeEnum(EFilterReturnedProjects).optional().describe(GROUPS.LIST_PROJECTS.filterProjects), + orderBy: z + .nativeEnum(EGroupProjectsOrderBy) + .default(EGroupProjectsOrderBy.Name) + .describe(GROUPS.LIST_PROJECTS.orderBy), + orderDirection: z + .nativeEnum(OrderByDirection) + .default(OrderByDirection.ASC) + .describe(GROUPS.LIST_PROJECTS.orderDirection) }), response: { 200: z.object({ @@ -255,6 +290,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { server.route({ method: "POST", url: "/:id/users/:username", + config: { + rateLimit: writeLimit + }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), schema: { hide: false, @@ -290,6 +328,9 @@ export const registerGroupRouter = async (server: FastifyZodProvider) => { server.route({ method: "DELETE", url: "/:id/users/:username", + config: { + rateLimit: writeLimit + }, onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]), schema: { hide: false, diff --git a/backend/src/ee/services/group/group-dal.ts b/backend/src/ee/services/group/group-dal.ts index 9c5fe5d3a..ced8410b7 100644 --- a/backend/src/ee/services/group/group-dal.ts +++ b/backend/src/ee/services/group/group-dal.ts @@ -4,8 +4,9 @@ import { TDbClient } from "@app/db"; import { AccessScope, TableName, TGroups } from "@app/db/schemas"; import { DatabaseError } from "@app/lib/errors"; import { buildFindFilter, ormify, selectAllTableCols, TFindFilter, TFindOpt } from "@app/lib/knex"; +import { OrderByDirection } from "@app/lib/types"; -import { EFilterReturnedProjects, EFilterReturnedUsers } from "./group-types"; +import { EFilterReturnedProjects, EFilterReturnedUsers, EGroupProjectsOrderBy } from "./group-types"; export type TGroupDALFactory = ReturnType; @@ -172,7 +173,9 @@ export const groupDALFactory = (db: TDbClient) => { offset, limit, search, - filter + filter, + orderBy, + orderDirection }: { orgId: string; groupId: string; @@ -180,10 +183,10 @@ export const groupDALFactory = (db: TDbClient) => { limit?: number; search?: string; filter?: EFilterReturnedProjects; + orderBy?: EGroupProjectsOrderBy; + orderDirection?: OrderByDirection; }) => { try { - const normalizedSearch = search?.trim(); - const query = db .replicaNode()(TableName.Project) .where(`${TableName.Project}.orgId`, orgId) @@ -201,17 +204,23 @@ export const groupDALFactory = (db: TDbClient) => { db.ref("createdAt").withSchema(TableName.Membership).as("joinedGroupAt"), db.raw(`count(*) OVER() as "totalCount"`) ) - .orderBy(`${TableName.Project}.name`, "asc") .offset(offset ?? 0); + if (orderBy) { + void query.orderByRaw( + `LOWER(${TableName.Project}.??) ${orderDirection === OrderByDirection.ASC ? "asc" : "desc"}`, + [orderBy] + ); + } + if (limit) { void query.limit(limit); } - if (normalizedSearch && normalizedSearch.toLowerCase() !== "undefined") { + if (search) { void query.andWhereRaw( `CONCAT_WS(' ', "${TableName.Project}"."name", "${TableName.Project}"."slug", "${TableName.Project}"."description") ilike ?`, - [`%${normalizedSearch}%`] + [`%${search}%`] ); } diff --git a/backend/src/ee/services/group/group-service.ts b/backend/src/ee/services/group/group-service.ts index dcd06a4b9..1a6a046a6 100644 --- a/backend/src/ee/services/group/group-service.ts +++ b/backend/src/ee/services/group/group-service.ts @@ -381,6 +381,8 @@ export const groupServiceFactory = ({ limit, search, filter, + orderBy, + orderDirection, actor, actorId, actorAuthMethod, @@ -414,7 +416,9 @@ export const groupServiceFactory = ({ offset, limit, search, - filter + filter, + orderBy, + orderDirection }); return { projects, totalCount }; diff --git a/backend/src/ee/services/group/group-types.ts b/backend/src/ee/services/group/group-types.ts index d48083442..335b6d72b 100644 --- a/backend/src/ee/services/group/group-types.ts +++ b/backend/src/ee/services/group/group-types.ts @@ -2,7 +2,7 @@ import { Knex } from "knex"; import { TGroups } from "@app/db/schemas"; import { TUserGroupMembershipDALFactory } from "@app/ee/services/group/user-group-membership-dal"; -import { TGenericPermission } from "@app/lib/types"; +import { OrderByDirection, TGenericPermission } from "@app/lib/types"; import { TMembershipGroupDALFactory } from "@app/services/membership-group/membership-group-dal"; import { TOrgDALFactory } from "@app/services/org/org-dal"; import { TProjectDALFactory } from "@app/services/project/project-dal"; @@ -48,6 +48,8 @@ export type TListGroupProjectsDTO = { limit: number; search?: string; filter?: EFilterReturnedProjects; + orderBy?: EGroupProjectsOrderBy; + orderDirection?: OrderByDirection; } & TGenericPermission; export type TListProjectGroupUsersDTO = TListGroupUsersDTO & { @@ -124,3 +126,7 @@ export enum EFilterReturnedProjects { ASSIGNED_PROJECTS = "assignedProjects", UNASSIGNED_PROJECTS = "unassignedProjects" } + +export enum EGroupProjectsOrderBy { + Name = "name" +} diff --git a/backend/src/lib/api-docs/constants.ts b/backend/src/lib/api-docs/constants.ts index bbbcb3a04..970f9a1a0 100644 --- a/backend/src/lib/api-docs/constants.ts +++ b/backend/src/lib/api-docs/constants.ts @@ -112,7 +112,9 @@ export const GROUPS = { limit: "The number of projects to return.", search: "The text string that project name or slug will be filtered by.", filterProjects: - "Whether to filter the list of returned projects. 'assignedProjects' will only return projects assigned to the group, 'unassignedProjects' will only return projects not assigned to the group, undefined will return all projects in the organization." + "Whether to filter the list of returned projects. 'assignedProjects' will only return projects assigned to the group, 'unassignedProjects' will only return projects not assigned to the group, undefined will return all projects in the organization.", + orderBy: "The column to order projects by.", + orderDirection: "The direction to order projects in." }, ADD_USER: { id: "The ID of the group to add the user to.", diff --git a/frontend/src/hooks/api/groups/queries.tsx b/frontend/src/hooks/api/groups/queries.tsx index 334c12844..2e5b9f684 100644 --- a/frontend/src/hooks/api/groups/queries.tsx +++ b/frontend/src/hooks/api/groups/queries.tsx @@ -2,6 +2,7 @@ import { useQuery } from "@tanstack/react-query"; import { apiRequest } from "@app/config/request"; +import { OrderByDirection } from "../generic/types"; import { EFilterReturnedProjects, EFilterReturnedUsers, @@ -55,14 +56,22 @@ export const groupKeys = { offset, limit, search, - filter + filter, + orderBy, + orderDirection }: { groupId: string; offset: number; limit: number; search: string; filter?: EFilterReturnedProjects; - }) => [...groupKeys.forGroupProjects(groupId), { offset, limit, search, filter }] as const + orderBy?: string; + orderDirection?: OrderByDirection; + }) => + [ + ...groupKeys.forGroupProjects(groupId), + { offset, limit, search, filter, orderBy, orderDirection } + ] as const }; export const useGetGroupById = (groupId: string) => { @@ -175,12 +184,16 @@ export const useListGroupProjects = ({ offset = 0, limit = 10, search, - filter + filter, + orderBy, + orderDirection }: { id: string; offset: number; limit: number; search: string; + orderBy?: string; + orderDirection?: OrderByDirection; filter?: EFilterReturnedProjects; }) => { return useQuery({ @@ -189,7 +202,9 @@ export const useListGroupProjects = ({ offset, limit, search, - filter + filter, + orderBy, + orderDirection }), enabled: Boolean(id), placeholderData: (previousData) => previousData, @@ -198,7 +213,9 @@ export const useListGroupProjects = ({ offset: String(offset), limit: String(limit), search, - ...(filter && { filter }) + ...(filter && { filter }), + ...(orderBy && { orderBy }), + ...(orderDirection && { orderDirection }) }); const { data } = await apiRequest.get<{ projects: TGroupProject[]; totalCount: number }>( diff --git a/frontend/src/pages/organization/GroupDetailsByIDPage/components/AddGroupProjectModal.tsx b/frontend/src/pages/organization/GroupDetailsByIDPage/components/AddGroupProjectModal.tsx index aeaa39d6c..3c4dc93d5 100644 --- a/frontend/src/pages/organization/GroupDetailsByIDPage/components/AddGroupProjectModal.tsx +++ b/frontend/src/pages/organization/GroupDetailsByIDPage/components/AddGroupProjectModal.tsx @@ -21,12 +21,14 @@ import { Tr } from "@app/components/v2"; import { OrgPermissionGroupActions, OrgPermissionSubjects } from "@app/context"; +import { getProjectTitle } from "@app/helpers/project"; import { useDebounce, useResetPageHelper } from "@app/hooks"; import { useAddGroupToWorkspace as useAddProjectToGroup, useListGroupProjects } from "@app/hooks/api"; import { EFilterReturnedProjects } from "@app/hooks/api/groups/types"; +import { ProjectType } from "@app/hooks/api/projects/types"; import { UsePopUpState } from "@app/hooks/usePopUp"; type Props = { @@ -95,7 +97,7 @@ export const AddGroupProjectModal = ({ popUp, handlePopUpToggle }: Props) => { handlePopUpToggle("addGroupProjects", isOpen); }} > - + setSearchProjectFilter(e.target.value)} @@ -117,16 +119,16 @@ export const AddGroupProjectModal = ({ popUp, handlePopUpToggle }: Props) => { data?.projects?.map((project) => { return ( - +

{project.name}

{project.description && (

{project.description}

)} -

{project.type?.replace("-", " ") || "-"}

+

{getProjectTitle(project.type as ProjectType)}

- + { {(isAllowed) => { return (