-
Notifications
You must be signed in to change notification settings - Fork 12k
feat: add email filtering to team memberships endpoint #23923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5e04066
8326658
913906e
91a886f
983d42b
15c1cc2
5a5e054
d6b0a60
ebc9f01
d9c2b48
dca53f1
86e8727
d9d0ba7
9ec09b4
922bb7c
2faa2af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -328,6 +328,131 @@ describe("Teams Memberships Endpoints", () => { | |
| return request(app.getHttpServer()).get(`/v2/teams/${team.id}/memberships/123132145`).expect(404); | ||
| }); | ||
|
|
||
| it("should filter memberships by single email", async () => { | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/teams/${team.id}/memberships?emails=${teamAdminEmail}`) | ||
| .expect(200) | ||
| .then((response) => { | ||
| const responseBody: GetTeamMembershipsOutput = response.body; | ||
| expect(responseBody.status).toEqual(SUCCESS_STATUS); | ||
| expect(responseBody.data.length).toEqual(1); | ||
| expect(responseBody.data[0].user.email).toEqual(teamAdminEmail); | ||
| expect(responseBody.data[0].userId).toEqual(teamAdmin.id); | ||
| expect(responseBody.data[0].role).toEqual("ADMIN"); | ||
| }); | ||
| }); | ||
|
|
||
| it("should filter memberships by multiple emails", async () => { | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/teams/${team.id}/memberships?emails=${teamAdminEmail},${teamMemberEmail}`) | ||
| .expect(200) | ||
| .then((response) => { | ||
| const responseBody: GetTeamMembershipsOutput = response.body; | ||
| expect(responseBody.status).toEqual(SUCCESS_STATUS); | ||
| expect(responseBody.data.length).toEqual(2); | ||
|
|
||
| const emails = responseBody.data.map((membership) => membership.user.email); | ||
| expect(emails).toContain(teamAdminEmail); | ||
| expect(emails).toContain(teamMemberEmail); | ||
|
|
||
| const adminMembership = responseBody.data.find((m) => m.user.email === teamAdminEmail); | ||
| const memberMembership = responseBody.data.find((m) => m.user.email === teamMemberEmail); | ||
|
|
||
| expect(adminMembership).toBeDefined(); | ||
| expect(memberMembership).toBeDefined(); | ||
| expect(adminMembership?.role).toEqual("ADMIN"); | ||
| expect(memberMembership?.role).toEqual("MEMBER"); | ||
| }); | ||
| }); | ||
|
Comment on lines
+331
to
+366
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 🧩 Analysis chainTests clearly specify comma-separated and multi-email behavior. These will pass once the emails transform splits and normalizes values as suggested in GetUsersInput. Run to confirm: 🏁 Script executed: #!/bin/bash
rg -n "Transform\\(\\(\\{ value" apps/api/v2/src/modules/users/inputs/get-users.input.ts -n -C3Length of output: 811 Update GetUsersInput email Transform to split and normalize comma-separated emails Current @Transform in apps/api/v2/src/modules/users/inputs/get-users.input.ts only wraps a single string into an array; replace it with a transform that, for string values, splits on ',', trims entries, filters out empties, and normalizes (e.g., toLowerCase()) so comma-separated email queries in tests pass. 🤖 Prompt for AI Agents |
||
|
|
||
| it("should return empty array when filtering by non-existent email", async () => { | ||
| const nonExistentEmail = `nonexistent-${randomString()}@test.com`; | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/teams/${team.id}/memberships?emails=${nonExistentEmail}`) | ||
| .expect(200) | ||
| .then((response) => { | ||
| const responseBody: GetTeamMembershipsOutput = response.body; | ||
| expect(responseBody.status).toEqual(SUCCESS_STATUS); | ||
| expect(responseBody.data.length).toEqual(0); | ||
| }); | ||
| }); | ||
|
|
||
| it("should return partial results when filtering by mix of existing and non-existent emails", async () => { | ||
| const nonExistentEmail = `nonexistent-${randomString()}@test.com`; | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/teams/${team.id}/memberships?emails=${teamAdminEmail},${nonExistentEmail}`) | ||
| .expect(200) | ||
| .then((response) => { | ||
| const responseBody: GetTeamMembershipsOutput = response.body; | ||
| expect(responseBody.status).toEqual(SUCCESS_STATUS); | ||
| expect(responseBody.data.length).toEqual(1); | ||
| expect(responseBody.data[0].user.email).toEqual(teamAdminEmail); | ||
| }); | ||
| }); | ||
|
|
||
| it("should work with pagination and email filtering combined", async () => { | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/teams/${team.id}/memberships?emails=${teamAdminEmail},${teamMemberEmail}&skip=1&take=1`) | ||
| .expect(200) | ||
| .then((response) => { | ||
| const responseBody: GetTeamMembershipsOutput = response.body; | ||
| expect(responseBody.status).toEqual(SUCCESS_STATUS); | ||
| expect(responseBody.data.length).toEqual(1); | ||
| const returnedEmail = responseBody.data[0].user.email; | ||
| expect([teamAdminEmail, teamMemberEmail]).toContain(returnedEmail); | ||
| }); | ||
| }); | ||
|
|
||
| it("should handle empty emails array gracefully", async () => { | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/teams/${team.id}/memberships?emails=`) | ||
| .expect(200) | ||
| .then((response) => { | ||
| const responseBody: GetTeamMembershipsOutput = response.body; | ||
| expect(responseBody.status).toEqual(SUCCESS_STATUS); | ||
| expect(responseBody.data.length).toEqual(2); | ||
| }); | ||
| }); | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| it("should handle URL encoded email addresses in filter", async () => { | ||
| const encodedEmail = encodeURIComponent(teamAdminEmail); | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/teams/${team.id}/memberships?emails=${encodedEmail}`) | ||
| .expect(200) | ||
| .then((response) => { | ||
| const responseBody: GetTeamMembershipsOutput = response.body; | ||
| expect(responseBody.status).toEqual(SUCCESS_STATUS); | ||
| expect(responseBody.data.length).toEqual(1); | ||
| expect(responseBody.data[0].user.email).toEqual(teamAdminEmail); | ||
| }); | ||
| }); | ||
|
|
||
| it("should filter by email and maintain all user properties", async () => { | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/teams/${team.id}/memberships?emails=${teamMemberEmail}`) | ||
| .expect(200) | ||
| .then((response) => { | ||
| const responseBody: GetTeamMembershipsOutput = response.body; | ||
| expect(responseBody.status).toEqual(SUCCESS_STATUS); | ||
| expect(responseBody.data.length).toEqual(1); | ||
| const membership = responseBody.data[0]; | ||
| expect(membership.user.email).toEqual(teamMemberEmail); | ||
| expect(membership.user.bio).toEqual(teamMember.bio); | ||
| expect(membership.user.metadata).toEqual(teamMember.metadata); | ||
| expect(membership.user.username).toEqual(teamMember.username); | ||
| expect(membership.teamId).toEqual(team.id); | ||
| expect(membership.userId).toEqual(teamMember.id); | ||
| expect(membership.role).toEqual("MEMBER"); | ||
| }); | ||
| }); | ||
|
|
||
| it("should validate email array size limits", async () => { | ||
| const tooManyEmails = Array.from({ length: 21 }, (_, i) => `test${i}@example.com`).join(","); | ||
| return request(app.getHttpServer()) | ||
| .get(`/v2/teams/${team.id}/memberships?emails=${tooManyEmails}`) | ||
| .expect(400); | ||
| }); | ||
|
|
||
| afterAll(async () => { | ||
| await userRepositoryFixture.deleteByEmail(teamAdmin.email); | ||
| await userRepositoryFixture.deleteByEmail(teammateInvitedViaApi.email); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { Roles } from "@/modules/auth/decorators/roles/roles.decorator"; | |
| import { ApiAuthGuard } from "@/modules/auth/guards/api-auth/api-auth.guard"; | ||
| import { RolesGuard } from "@/modules/auth/guards/roles/roles.guard"; | ||
| import { CreateTeamMembershipInput } from "@/modules/teams/memberships/inputs/create-team-membership.input"; | ||
| import { GetTeamMembershipsInput } from "@/modules/teams/memberships/inputs/get-team-memberships.input"; | ||
| import { UpdateTeamMembershipInput } from "@/modules/teams/memberships/inputs/update-team-membership.input"; | ||
| import { CreateTeamMembershipOutput } from "@/modules/teams/memberships/outputs/create-team-membership.output"; | ||
| import { DeleteTeamMembershipOutput } from "@/modules/teams/memberships/outputs/delete-team-membership.output"; | ||
|
|
@@ -32,7 +33,6 @@ import { plainToClass } from "class-transformer"; | |
|
|
||
| import { SUCCESS_STATUS } from "@calcom/platform-constants"; | ||
| import { updateNewTeamMemberEventTypes } from "@calcom/platform-libraries/event-types"; | ||
| import { SkipTakePagination } from "@calcom/platform-types"; | ||
|
|
||
| @Controller({ | ||
| path: "/v2/teams/:teamId/memberships", | ||
|
|
@@ -85,16 +85,20 @@ export class TeamsMembershipsController { | |
| } | ||
|
|
||
| @Get("/") | ||
| @ApiOperation({ summary: "Get all memberships" }) | ||
| @ApiOperation({ | ||
| summary: "Get all memberships", | ||
| description: "Retrieve team memberships with optional filtering by email addresses. Supports pagination.", | ||
| }) | ||
| @Roles("TEAM_ADMIN") | ||
| @HttpCode(HttpStatus.OK) | ||
| async getTeamMemberships( | ||
| @Param("teamId", ParseIntPipe) teamId: number, | ||
| @Query() queryParams: SkipTakePagination | ||
| @Query() queryParams: GetTeamMembershipsInput | ||
| ): Promise<GetTeamMembershipsOutput> { | ||
| const { skip, take } = queryParams; | ||
| const { skip, take, emails } = queryParams; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now extracting emails also for the filtering. |
||
| const orgTeamMemberships = await this.teamsMembershipsService.getPaginatedTeamMemberships( | ||
| teamId, | ||
| emails, | ||
| skip ?? 0, | ||
| take ?? 250 | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { GetUsersInput } from "@/modules/users/inputs/get-users.input"; | ||
| import { ApiPropertyOptional } from "@nestjs/swagger"; | ||
| import { Transform } from "class-transformer"; | ||
| import { ArrayMaxSize, ArrayNotEmpty, IsEmail, IsOptional } from "class-validator"; | ||
|
|
||
| export class GetTeamMembershipsInput extends GetUsersInput { | ||
| @IsOptional() | ||
| @Transform(({ value }) => { | ||
| if (value == null) return undefined; | ||
| const rawValues = (Array.isArray(value) ? value : [value]).flatMap((entry) => | ||
| typeof entry === "string" ? entry.split(",") : [] | ||
| ); | ||
| const normalized = rawValues | ||
| .map((email) => email.trim()) | ||
| .filter((email) => email.length > 0) | ||
| .map((email) => email.toLowerCase()); | ||
| const deduplicated = [...new Set(normalized)]; | ||
| return deduplicated.length > 0 ? deduplicated : undefined; | ||
| }) | ||
| @ArrayNotEmpty({ message: "emails cannot be empty." }) | ||
| @ArrayMaxSize(20, { | ||
| message: "emails array cannot contain more than 20 email addresses for team membership filtering", | ||
| }) | ||
| @IsEmail({}, { each: true, message: "Each email must be a valid email address" }) | ||
| @ApiPropertyOptional({ | ||
Devanshusharma2005 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| type: [String], | ||
| description: | ||
| "Filter team memberships by email addresses. If you want to filter by multiple emails, separate them with a comma (max 20 emails for performance).", | ||
| example: "?emails=user1@example.com,user2@example.com", | ||
| }) | ||
| emails?: string[]; | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,10 @@ import { Injectable } from "@nestjs/common"; | |||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| import type { Prisma } from "@calcom/prisma/client"; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| export interface TeamMembershipFilters { | ||||||||||||||||||||||||||||||||||||||||
| emails?: string[]; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| export const MembershipUserSelect: Prisma.UserSelect = { | ||||||||||||||||||||||||||||||||||||||||
| username: true, | ||||||||||||||||||||||||||||||||||||||||
| email: true, | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -41,6 +45,30 @@ export class TeamsMembershipsRepository { | |||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| async findTeamMembershipsPaginatedWithFilters( | ||||||||||||||||||||||||||||||||||||||||
| teamId: number, | ||||||||||||||||||||||||||||||||||||||||
| filters: TeamMembershipFilters, | ||||||||||||||||||||||||||||||||||||||||
| skip: number, | ||||||||||||||||||||||||||||||||||||||||
| take: number | ||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||
| const whereClause: Prisma.MembershipWhereInput = { | ||||||||||||||||||||||||||||||||||||||||
| teamId: teamId, | ||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if (filters.emails && filters.emails.length > 0) { | ||||||||||||||||||||||||||||||||||||||||
| whereClause.user = { | ||||||||||||||||||||||||||||||||||||||||
| email: { in: filters.emails }, | ||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+58
to
+62
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prisma relation filter is incomplete; use
Apply this diff: - if (filters.emails && filters.emails.length > 0) {
- whereClause.user = {
- email: { in: filters.emails },
- };
- }
+ if (filters.emails && filters.emails.length > 0) {
+ const emails = Array.from(new Set(filters.emails.map((e) => e.toLowerCase())));
+ whereClause.user = {
+ is: { email: { in: emails } },
+ };
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| return await this.dbRead.prisma.membership.findMany({ | ||||||||||||||||||||||||||||||||||||||||
| where: whereClause, | ||||||||||||||||||||||||||||||||||||||||
| include: { user: { select: MembershipUserSelect } }, | ||||||||||||||||||||||||||||||||||||||||
| skip, | ||||||||||||||||||||||||||||||||||||||||
| take, | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+64
to
+69
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use Follow “select only what you need; never use include”. Also add Apply this diff: - return await this.dbRead.prisma.membership.findMany({
- where: whereClause,
- include: { user: { select: MembershipUserSelect } },
- skip,
- take,
- });
+ return await this.dbRead.prisma.membership.findMany({
+ where: whereClause,
+ select: {
+ id: true,
+ teamId: true,
+ userId: true,
+ role: true,
+ user: { select: MembershipUserSelect },
+ },
+ skip,
+ take,
+ orderBy: { id: "asc" },
+ });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| async findTeamMembership(teamId: number, membershipId: number) { | ||||||||||||||||||||||||||||||||||||||||
| return this.dbRead.prisma.membership.findUnique({ | ||||||||||||||||||||||||||||||||||||||||
| where: { | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice tests!