fix: review comments

This commit is contained in:
Piyush Gupta
2025-11-20 22:50:10 +05:30
parent 69255779f1
commit 4796bcd579
11 changed files with 127 additions and 63 deletions

View File

@@ -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,

View File

@@ -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<typeof groupDALFactory>;
@@ -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}%`]
);
}

View File

@@ -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 };

View File

@@ -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"
}

View File

@@ -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.",

View File

@@ -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 }>(

View File

@@ -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);
}}
>
<ModalContent title="Add Group Projects">
<ModalContent title="Add Projects">
<Input
value={searchProjectFilter}
onChange={(e) => setSearchProjectFilter(e.target.value)}
@@ -117,16 +119,16 @@ export const AddGroupProjectModal = ({ popUp, handlePopUpToggle }: Props) => {
data?.projects?.map((project) => {
return (
<Tr className="items-center" key={`group-project-${project.id}`}>
<Td>
<Td className="w-1/3">
<p>{project.name}</p>
{project.description && (
<p className="text-sm text-mineshaft-400">{project.description}</p>
)}
</Td>
<Td>
<p className="capitalize">{project.type?.replace("-", " ") || "-"}</p>
<p>{getProjectTitle(project.type as ProjectType)}</p>
</Td>
<Td className="flex justify-end">
<Td>
<OrgPermissionCan
I={OrgPermissionGroupActions.Edit}
a={OrgPermissionSubjects.Groups}
@@ -134,6 +136,7 @@ export const AddGroupProjectModal = ({ popUp, handlePopUpToggle }: Props) => {
{(isAllowed) => {
return (
<Button
className="self-center"
isLoading={isAdding}
isDisabled={!isAllowed}
colorSchema="primary"
@@ -166,7 +169,7 @@ export const AddGroupProjectModal = ({ popUp, handlePopUpToggle }: Props) => {
title={
debouncedSearch
? "No projects match search"
: "All projects are already assigned to the group"
: "This group is already a part of all projects"
}
icon={faFolder}
/>

View File

@@ -46,7 +46,7 @@ export const GroupMembersSection = ({ groupId, groupSlug }: Props) => {
return (
<div className="w-full rounded-lg border border-mineshaft-600 bg-mineshaft-900 p-4">
<div className="flex items-center justify-between border-b border-mineshaft-400 pb-4">
<h3 className="text-lg font-medium text-mineshaft-100">Group Members</h3>
<h3 className="text-lg font-medium text-mineshaft-100">Members</h3>
<OrgPermissionCan I={OrgPermissionGroupActions.Edit} a={OrgPermissionSubjects.Groups}>
{(isAllowed) => (
<Tooltip

View File

@@ -13,7 +13,9 @@ import {
Tr
} from "@app/components/v2";
import { OrgPermissionGroupActions, OrgPermissionSubjects } from "@app/context";
import { getProjectTitle } from "@app/helpers/project";
import { TGroupProject } from "@app/hooks/api/groups/types";
import { ProjectType } from "@app/hooks/api/projects/types";
import { UsePopUpState } from "@app/hooks/usePopUp";
type Props = {
@@ -31,7 +33,7 @@ export const GroupProjectRow = ({ project, handlePopUpOpen }: Props) => {
<p>{project.name}</p>
</Td>
<Td>
<p className="capitalize">{project.type?.replace("-", " ") || "-"}</p>
<p>{getProjectTitle(project.type as ProjectType)}</p>
</Td>
<Td>
<Tooltip content={new Date(project.joinedGroupAt).toLocaleString()}>
@@ -65,7 +67,7 @@ export const GroupProjectRow = ({ project, handlePopUpOpen }: Props) => {
}
isDisabled={!isAllowed}
>
Remove Project From Group
Remove group from project
</DropdownMenuItem>
);
}}

View File

@@ -31,7 +31,7 @@ export const GroupProjectsSection = ({ groupId, groupSlug }: Props) => {
});
createNotification({
text: `Successfully removed project ${projectName} from the group`,
text: `Successfully removed group from project ${projectName}`,
type: "success"
});
@@ -41,7 +41,7 @@ export const GroupProjectsSection = ({ groupId, groupSlug }: Props) => {
return (
<div className="w-full rounded-lg border border-mineshaft-600 bg-mineshaft-900 p-4">
<div className="flex items-center justify-between border-b border-mineshaft-400 pb-4">
<h3 className="text-lg font-medium text-mineshaft-100">Group Projects</h3>
<h3 className="text-lg font-medium text-mineshaft-100">Projects</h3>
<OrgPermissionCan I={OrgPermissionGroupActions.Edit} a={OrgPermissionSubjects.Groups}>
{(isAllowed) => (
<IconButton
@@ -71,9 +71,9 @@ export const GroupProjectsSection = ({ groupId, groupSlug }: Props) => {
<AddGroupProjectModal popUp={popUp} handlePopUpToggle={handlePopUpToggle} />
<DeleteActionModal
isOpen={popUp.removeProjectFromGroup.isOpen}
title={`Are you sure you want to remove ${
title={`Are you sure you want to remove the group from ${
(popUp?.removeProjectFromGroup?.data as { projectName: string })?.projectName || ""
} from the group?`}
}?`}
onChange={(isOpen) => handlePopUpToggle("removeProjectFromGroup", isOpen)}
deleteKey="confirm"
onDeleteApproved={() => {

View File

@@ -1,4 +1,3 @@
import { useMemo } from "react";
import {
faArrowDown,
faArrowUp,
@@ -61,6 +60,7 @@ export const GroupProjectsTable = ({ groupId, groupSlug, handlePopUpOpen }: Prop
setPerPage,
offset,
orderDirection,
orderBy,
toggleOrderDirection
} = usePagination(GroupProjectsOrderBy.Name, {
initPerPage: getUserTablePreference("groupProjectsTable", PreferenceKey.PerPage, 20)
@@ -76,37 +76,17 @@ export const GroupProjectsTable = ({ groupId, groupSlug, handlePopUpOpen }: Prop
offset,
limit: perPage,
search: debouncedSearch,
orderBy,
orderDirection,
filter: EFilterReturnedProjects.ASSIGNED_PROJECTS
});
const filteredGroupProjects = useMemo(() => {
return groupMemberships && groupMemberships?.projects
? groupMemberships?.projects
?.filter((project) => {
const projectSearchString = `${project.name || ""} ${project.slug || ""} ${
project.description || ""
}`;
return projectSearchString.toLowerCase().includes(search.trim().toLowerCase());
})
.sort((a, b) => {
const [projectOne, projectTwo] =
orderDirection === OrderByDirection.ASC ? [a, b] : [b, a];
const projectOneComparisonString = projectOne.name || "";
const projectTwoComparisonString = projectTwo.name || "";
const comparison = projectOneComparisonString
.toLowerCase()
.localeCompare(projectTwoComparisonString.toLowerCase());
return comparison;
})
: [];
}, [groupMemberships, orderDirection, search]);
const totalCount = groupMemberships?.totalCount ?? 0;
const isEmpty = !isPending && totalCount === 0;
const projects = groupMemberships?.projects ?? [];
useResetPageHelper({
totalCount: filteredGroupProjects?.length,
totalCount,
offset,
setPage
});
@@ -146,7 +126,7 @@ export const GroupProjectsTable = ({ groupId, groupSlug, handlePopUpOpen }: Prop
<TBody>
{isPending && <TableSkeleton columns={4} innerKey="group-project-memberships" />}
{!isPending &&
filteredGroupProjects.slice(offset, perPage * page).map((project) => {
projects.map((project) => {
return (
<GroupProjectRow
key={`group-project-${project.id}`}
@@ -157,21 +137,21 @@ export const GroupProjectsTable = ({ groupId, groupSlug, handlePopUpOpen }: Prop
})}
</TBody>
</Table>
{Boolean(filteredGroupProjects.length) && (
{!isEmpty && (
<Pagination
count={filteredGroupProjects.length}
count={totalCount}
page={page}
perPage={perPage}
onChangePage={setPage}
onChangePerPage={handlePerPageChange}
/>
)}
{!isPending && !filteredGroupProjects?.length && (
{isEmpty && (
<EmptyState
title={
groupMemberships?.projects.length
? "No projects match this search..."
: "This group does not have any projects assigned yet"
: "This group is not a part of any projects yet"
}
icon={groupMemberships?.projects.length ? faSearch : faFolder}
/>