Skip to content

Commit fe77f51

Browse files
hariombalharahariombalhara
committed
Add more tests
1 parent 8a7b758 commit fe77f51

File tree

7 files changed

+723
-70
lines changed

7 files changed

+723
-70
lines changed

‎apps/api/v2/src/modules/organizations/memberships/organizations-membership.controller.e2e-spec.ts

Lines changed: 62 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { GetOrgMembership } from "@/modules/organizations/memberships/outputs/ge
99
import { OrgUserAttribute } from "@/modules/organizations/memberships/outputs/organization-membership.output";
1010
import { UpdateOrgMembership } from "@/modules/organizations/memberships/outputs/update-membership.output";
1111
import { PrismaModule } from "@/modules/prisma/prisma.module";
12-
import { TeamMembershipOutput } from "@/modules/teams/memberships/outputs/team-membership.output";
1312
import { TokensModule } from "@/modules/tokens/tokens.module";
1413
import { UsersModule } from "@/modules/users/users.module";
1514
import { INestApplication } from "@nestjs/common";
@@ -47,17 +46,12 @@ describe("Organizations Memberships Endpoints", () => {
4746
let org: Team;
4847
let membership: Membership;
4948
let membership2: Membership;
50-
let membershipCreatedViaApi: TeamMembershipOutput;
5149

5250
const userEmail = `organizations-memberships-admin-${randomString()}@api.com`;
5351
const userEmail2 = `organizations-memberships-member-${randomString()}@api.com`;
54-
const invitedUserEmail = `organizations-memberships-invited-${randomString()}@api.com`;
55-
5652
let user: User;
5753
let user2: User;
5854

59-
let userToInviteViaApi: User;
60-
6155
let textAttribute: Attribute;
6256
let multiSelectAttribute: Attribute;
6357
let numberAttribute: Attribute;
@@ -104,13 +98,6 @@ describe("Organizations Memberships Endpoints", () => {
10498
metadata,
10599
});
106100

107-
userToInviteViaApi = await userRepositoryFixture.create({
108-
email: invitedUserEmail,
109-
username: invitedUserEmail,
110-
bio,
111-
metadata,
112-
});
113-
114101
org = await organizationsRepositoryFixture.create({
115102
name: `organizations-memberships-organization-${randomString()}`,
116103
isOrganization: true,
@@ -379,40 +366,69 @@ describe("Organizations Memberships Endpoints", () => {
379366
});
380367

381368
it("should create the membership of the org", async () => {
369+
// Create test user for this specific test
370+
const testUserForCreate = await userRepositoryFixture.create({
371+
email: `test-create-${randomString()}@api.com`,
372+
username: `test-create-${randomString()}`,
373+
bio,
374+
metadata,
375+
});
376+
382377
return request(app.getHttpServer())
383378
.post(`/v2/organizations/${org.id}/memberships`)
384379
.send({
385-
userId: userToInviteViaApi.id,
380+
userId: testUserForCreate.id,
386381
accepted: true,
387382
role: "MEMBER",
388383
} satisfies CreateOrgMembershipDto)
389384
.expect(201)
390-
.then((response) => {
385+
.then(async (response) => {
391386
const responseBody: CreateOrgMembershipOutput = response.body;
392387
expect(responseBody.status).toEqual(SUCCESS_STATUS);
393-
membershipCreatedViaApi = responseBody.data;
394-
expect(membershipCreatedViaApi.teamId).toEqual(org.id);
395-
expect(membershipCreatedViaApi.role).toEqual("MEMBER");
396-
expect(membershipCreatedViaApi.userId).toEqual(userToInviteViaApi.id);
397-
expect(membershipCreatedViaApi.user.bio).toEqual(bio);
398-
expect(membershipCreatedViaApi.user.metadata).toEqual(metadata);
399-
expect(membershipCreatedViaApi.user.email).toEqual(userToInviteViaApi.email);
400-
expect(membershipCreatedViaApi.user.username).toEqual(userToInviteViaApi.username);
388+
const createdMembership = responseBody.data;
389+
expect(createdMembership.teamId).toEqual(org.id);
390+
expect(createdMembership.role).toEqual("MEMBER");
391+
expect(createdMembership.userId).toEqual(testUserForCreate.id);
392+
expect(createdMembership.user.bio).toEqual(bio);
393+
expect(createdMembership.user.metadata).toEqual(metadata);
394+
expect(createdMembership.user.email).toEqual(testUserForCreate.email);
395+
expect(createdMembership.user.username).toEqual(testUserForCreate.username);
396+
397+
// Clean up
398+
await membershipRepositoryFixture.delete(createdMembership.id);
399+
await userRepositoryFixture.deleteByEmail(testUserForCreate.email);
401400
});
402401
});
403402

404403
it("should update the membership of the org", async () => {
404+
// Create test user and membership for this specific test
405+
const testUserForUpdate = await userRepositoryFixture.create({
406+
email: `test-update-${randomString()}@api.com`,
407+
username: `test-update-${randomString()}`,
408+
});
409+
410+
const membershipToUpdate = await membershipRepositoryFixture.create({
411+
role: "MEMBER",
412+
user: { connect: { id: testUserForUpdate.id } },
413+
team: { connect: { id: org.id } },
414+
accepted: true,
415+
});
416+
405417
return request(app.getHttpServer())
406-
.patch(`/v2/organizations/${org.id}/memberships/${membershipCreatedViaApi.id}`)
418+
.patch(`/v2/organizations/${org.id}/memberships/${membershipToUpdate.id}`)
407419
.send({
408420
role: "OWNER",
409421
} satisfies UpdateOrgMembershipDto)
410422
.expect(200)
411-
.then((response) => {
423+
.then(async (response) => {
412424
const responseBody: UpdateOrgMembership = response.body;
413425
expect(responseBody.status).toEqual(SUCCESS_STATUS);
414-
membershipCreatedViaApi = responseBody.data;
415-
expect(membershipCreatedViaApi.role).toEqual("OWNER");
426+
const updatedMembership = responseBody.data;
427+
expect(updatedMembership.role).toEqual("OWNER");
428+
429+
// Clean up
430+
await membershipRepositoryFixture.delete(membershipToUpdate.id);
431+
await userRepositoryFixture.deleteByEmail(testUserForUpdate.email);
416432
});
417433
});
418434

@@ -576,13 +592,29 @@ describe("Organizations Memberships Endpoints", () => {
576592
});
577593

578594
it("should delete the membership of the org we created via api", async () => {
595+
// Create a new membership for this test to ensure independence
596+
const testUserForDeletion = await userRepositoryFixture.create({
597+
email: `test-deletion-${randomString()}@api.com`,
598+
username: `test-deletion-${randomString()}`,
599+
});
600+
601+
const membershipToDelete = await membershipRepositoryFixture.create({
602+
role: "MEMBER",
603+
user: { connect: { id: testUserForDeletion.id } },
604+
team: { connect: { id: org.id } },
605+
accepted: true,
606+
});
607+
579608
return request(app.getHttpServer())
580-
.delete(`/v2/organizations/${org.id}/memberships/${membershipCreatedViaApi.id}`)
609+
.delete(`/v2/organizations/${org.id}/memberships/${membershipToDelete.id}`)
581610
.expect(200)
582-
.then((response) => {
611+
.then(async (response) => {
583612
const responseBody: DeleteOrgMembership = response.body;
584613
expect(responseBody.status).toEqual(SUCCESS_STATUS);
585-
expect(responseBody.data.id).toEqual(membershipCreatedViaApi.id);
614+
expect(responseBody.data.id).toEqual(membershipToDelete.id);
615+
616+
// Clean up
617+
await userRepositoryFixture.deleteByEmail(testUserForDeletion.email);
586618
});
587619
});
588620

@@ -676,32 +708,6 @@ describe("Organizations Memberships Endpoints", () => {
676708
await userRepositoryFixture.deleteByEmail(testUser.email);
677709
});
678710

679-
it("should fail to get the membership of the org we just deleted", async () => {
680-
// This test depends on the previous test "should delete the membership of the org we created via api"
681-
// If membershipCreatedViaApi is not defined, we can't run this test
682-
if (!membershipCreatedViaApi || !membershipCreatedViaApi.id) {
683-
console.log("Skipping test: membershipCreatedViaApi is not defined. This test depends on previous tests.");
684-
return;
685-
}
686-
687-
// First, let's verify the membership was actually deleted
688-
const deletedMembership = await membershipRepositoryFixture.findById(membershipCreatedViaApi.id);
689-
690-
// If the membership still exists in the database, that explains why we're getting 200 instead of 404
691-
if (deletedMembership) {
692-
console.log("WARNING: Membership was not actually deleted from database:", deletedMembership);
693-
// For now, let's adjust the test to match current behavior
694-
return request(app.getHttpServer())
695-
.get(`/v2/organizations/${org.id}/memberships/${membershipCreatedViaApi.id}`)
696-
.expect(200);
697-
} else {
698-
// If it was deleted, we should get 404
699-
return request(app.getHttpServer())
700-
.get(`/v2/organizations/${org.id}/memberships/${membershipCreatedViaApi.id}`)
701-
.expect(404);
702-
}
703-
});
704-
705711
it("should fail if the membership does not exist", async () => {
706712
return request(app.getHttpServer())
707713
.get(`/v2/organizations/${org.id}/memberships/123132145`)
@@ -711,7 +717,6 @@ describe("Organizations Memberships Endpoints", () => {
711717
afterAll(async () => {
712718
await userRepositoryFixture.deleteByEmail(user.email);
713719
await userRepositoryFixture.deleteByEmail(user2.email);
714-
await userRepositoryFixture.deleteByEmail(userToInviteViaApi.email);
715720
await organizationsRepositoryFixture.delete(org.id);
716721
await app.close();
717722
});
@@ -725,7 +730,6 @@ describe("Organizations Memberships Endpoints", () => {
725730
let userRepositoryFixture: UserRepositoryFixture;
726731
let organizationsRepositoryFixture: OrganizationRepositoryFixture;
727732
let membershipRepositoryFixture: MembershipRepositoryFixture;
728-
let attributesRepositoryFixture: AttributeRepositoryFixture;
729733

730734
let org: Team;
731735
let membership: Membership;
@@ -744,7 +748,6 @@ describe("Organizations Memberships Endpoints", () => {
744748
userRepositoryFixture = new UserRepositoryFixture(moduleRef);
745749
organizationsRepositoryFixture = new OrganizationRepositoryFixture(moduleRef);
746750
membershipRepositoryFixture = new MembershipRepositoryFixture(moduleRef);
747-
attributesRepositoryFixture = new AttributeRepositoryFixture(moduleRef);
748751

749752
user = await userRepositoryFixture.create({
750753
email: userEmail,

‎apps/api/v2/src/modules/organizations/memberships/services/organizations-membership.service.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,6 @@ export class OrganizationsMembershipService {
7676
);
7777
}
7878

79-
// Use TeamService.removeMembers which handles:
80-
// - Profile deletion
81-
// - Sub-team membership removal
82-
// - Event type and host cleanup
83-
// - Workflow reminder cleanup
84-
// - Billing updates
85-
// - Username conflict resolution
8679
await TeamService.removeMembers([organizationId], [membership.userId], true);
8780

8881
return this.organizationsMembershipOutputService.getOrgMembershipOutput(membership);

‎apps/api/v2/src/modules/organizations/teams/memberships/organizations-teams-memberships.controller.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,6 @@ export class OrganizationsTeamsMembershipsController {
127127
membershipId
128128
);
129129

130-
await this.teamsEventTypesService.deleteUserTeamEventTypesAndHosts(membership.userId, teamId);
131-
132130
return {
133131
status: SUCCESS_STATUS,
134132
data: plainToClass(TeamMembershipOutput, membership, { strategy: "excludeAll" }),

‎apps/api/v2/src/modules/organizations/teams/memberships/services/organizations-teams-memberships.service.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { CreateOrgTeamMembershipDto } from "@/modules/organizations/teams/member
22
import { UpdateOrgTeamMembershipDto } from "@/modules/organizations/teams/memberships/inputs/update-organization-team-membership.input";
33
import { OrganizationsTeamsMembershipsRepository } from "@/modules/organizations/teams/memberships/organizations-teams-memberships.repository";
44
import { Injectable, NotFoundException } from "@nestjs/common";
5+
import { TeamService } from "@calcom/platform-libraries";
56

67
@Injectable()
78
export class OrganizationsTeamsMembershipsService {
@@ -63,6 +64,7 @@ export class OrganizationsTeamsMembershipsService {
6364
teamId,
6465
membershipId
6566
);
67+
await TeamService.removeMembers([teamId], [teamMembership.userId]);
6668
return teamMembership;
6769
}
6870
}

‎packages/features/ee/dsync/lib/removeUserFromOrg.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import removeMember from "@calcom/features/ee/teams/lib/removeMember";
22

33
const removeUserFromOrg = async ({ userId, orgId }: { userId: number; orgId: number }) => {
4+
// TODO: Shouldn't we call TeamService.removeMembers instead, which also updates billing too?
45
return removeMember({
56
memberId: userId,
67
teamId: orgId,

‎packages/features/ee/teams/lib/removeMember.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,15 @@ const removeMember = async ({
100100
});
101101
}
102102

103+
const newUsername = foundUser.username != null ? `${foundUser.username}-${foundUser.id}` : null;
104+
103105
await prisma.$transaction([
104106
prisma.user.update({
105107
where: { id: membership.userId },
106-
data: {
108+
data: {
107109
organizationId: null,
108110
// Update username to avoid conflicts with users outside organizations
109-
username: foundUser.username ? `${foundUser.username}-${membership.userId}` : null
111+
username: newUsername,
110112
},
111113
}),
112114
ProfileRepository.delete({
@@ -147,6 +149,13 @@ const removeMember = async ({
147149

148150
await deleteWorkfowRemindersOfRemovedMember(team, memberId, isOrg);
149151

152+
// Finally, delete the membership itself
153+
await prisma.membership.delete({
154+
where: {
155+
userId_teamId: { userId: memberId, teamId: teamId },
156+
},
157+
});
158+
150159
return { membership };
151160
};
152161

0 commit comments

Comments
 (0)