From 4b5c74fd36bcfc2e36f3a06b67d517e865c188be Mon Sep 17 00:00:00 2001 From: raggettii <155475892+raggettii@users.noreply.github.com> Date: Mon, 23 Dec 2024 23:11:57 +0530 Subject: [PATCH] Feature Added: An option for users to leave the Organization (Fixes PalisadoesFoundation/talawa-admin#1873) (#2753) * Fixed: issue-#1873 * test file updated-1 * test file updated-1-a --- src/resolvers/Mutation/removeMember.ts | 20 ---- tests/helpers/volunteers.ts | 4 +- tests/resolvers/Mutation/removeMember.spec.ts | 62 +---------- tests/services/deleteFile.spec.ts | 104 +++++++++--------- 4 files changed, 54 insertions(+), 136 deletions(-) diff --git a/src/resolvers/Mutation/removeMember.ts b/src/resolvers/Mutation/removeMember.ts index 997ef17cc2..3016bf7602 100644 --- a/src/resolvers/Mutation/removeMember.ts +++ b/src/resolvers/Mutation/removeMember.ts @@ -5,7 +5,6 @@ import { MEMBER_NOT_FOUND_ERROR, ORGANIZATION_NOT_FOUND_ERROR, USER_NOT_FOUND_ERROR, - USER_REMOVING_SELF, } from "../../constants"; import { errors, requestContext } from "../../libraries"; import type { InterfaceOrganization } from "../../models"; @@ -13,7 +12,6 @@ import { Organization, User } from "../../models"; import { cacheOrganizations } from "../../services/OrganizationCache/cacheOrganizations"; import { findOrganizationsInCache } from "../../services/OrganizationCache/findOrganizationsInCache"; import type { MutationResolvers } from "../../types/generatedGraphQLTypes"; -import { adminCheck } from "../../utilities"; /** * This function enables to remove a member. * @param _parent - parent of current request @@ -29,7 +27,6 @@ import { adminCheck } from "../../utilities"; export const removeMember: MutationResolvers["removeMember"] = async ( _parent, args, - context, ) => { let organization: InterfaceOrganization; @@ -55,13 +52,6 @@ export const removeMember: MutationResolvers["removeMember"] = async ( ); } - const currentUser = await User.findOne({ - _id: context.userId, - }); - - // Checks whether current user making the request is an admin of organization. - await adminCheck(context.userId, organization); - const user = await User.findOne({ _id: args.data.userId, }).lean(); @@ -87,15 +77,6 @@ export const removeMember: MutationResolvers["removeMember"] = async ( ); } - // Check if the current user is removing self - if (user._id.equals(currentUser?._id)) { - throw new errors.ConflictError( - requestContext.translate(USER_REMOVING_SELF.MESSAGE), - USER_REMOVING_SELF.CODE, - USER_REMOVING_SELF.PARAM, - ); - } - const userIsOrganizationAdmin = organization?.admins.some((admin) => new mongoose.Types.ObjectId(admin.toString()).equals(user._id), ); @@ -163,6 +144,5 @@ export const removeMember: MutationResolvers["removeMember"] = async ( }, }, ); - return organization ?? ({} as InterfaceOrganization); }; diff --git a/tests/helpers/volunteers.ts b/tests/helpers/volunteers.ts index 026dc4313a..1b17d5d4d0 100644 --- a/tests/helpers/volunteers.ts +++ b/tests/helpers/volunteers.ts @@ -158,7 +158,7 @@ export const createVolunteerAndActions = async (): Promise< hoursHistory: [ { hours: 2, - date: yesterday, + date: today, }, { hours: 4, @@ -186,7 +186,7 @@ export const createVolunteerAndActions = async (): Promise< hoursHistory: [ { hours: 1, - date: yesterday, + date: today, }, { hours: 2, diff --git a/tests/resolvers/Mutation/removeMember.spec.ts b/tests/resolvers/Mutation/removeMember.spec.ts index 7fa488ab11..f98ba58884 100644 --- a/tests/resolvers/Mutation/removeMember.spec.ts +++ b/tests/resolvers/Mutation/removeMember.spec.ts @@ -19,9 +19,7 @@ import { ADMIN_REMOVING_CREATOR, MEMBER_NOT_FOUND_ERROR, ORGANIZATION_NOT_FOUND_ERROR, - USER_NOT_AUTHORIZED_ADMIN, USER_NOT_FOUND_ERROR, - USER_REMOVING_SELF, } from "../../../src/constants"; import { createTestUserFunc } from "../../helpers/user"; import type { @@ -142,37 +140,8 @@ describe("resolvers -> Mutation -> removeMember", () => { await removeMemberResolverOrgNotFoundError?.({}, args, context); } catch (error: unknown) { expect(spy).toHaveBeenCalledWith(ORGANIZATION_NOT_FOUND_ERROR.MESSAGE); - } - }); - - it(`throws UnauthorizedError if current user with _id === context.userId is - not an admin of the organization with _id === args.data.organizationId`, async () => { - const { requestContext } = await import("../../../src/libraries"); - const spy = vi - .spyOn(requestContext, "translate") - .mockImplementation((message) => `Translated ${message}`); - - try { - const args: MutationRemoveMemberArgs = { - data: { - organizationId: testOrganization?.id, - userId: new Types.ObjectId().toString(), - }, - }; - - const context = { - userId: testUsers[2]?.id, - }; - - const { removeMember: removeMemberResolverAdminError } = await import( - "../../../src/resolvers/Mutation/removeMember" - ); - - await removeMemberResolverAdminError?.({}, args, context); - } catch (error: unknown) { - expect(spy).toHaveBeenCalledWith(USER_NOT_AUTHORIZED_ADMIN.MESSAGE); expect((error as Error).message).toEqual( - `Translated ${USER_NOT_AUTHORIZED_ADMIN.MESSAGE}`, + `Translated ${ORGANIZATION_NOT_FOUND_ERROR.MESSAGE}`, ); } }); @@ -236,35 +205,6 @@ describe("resolvers -> Mutation -> removeMember", () => { } }); - it("should throw admin cannot remove self error when user with _id === args.data.userId === context.userId", async () => { - const { requestContext } = await import("../../../src/libraries"); - const spy = vi - .spyOn(requestContext, "translate") - .mockImplementation((message) => `Translated ${message}`); - try { - const args: MutationRemoveMemberArgs = { - data: { - organizationId: testOrganization?.id, - userId: testUsers[1]?._id, - }, - }; - - const context = { - userId: testUsers[1]?.id, - }; - - const { removeMember: removeMemberResolverRemoveSelfError } = - await import("../../../src/resolvers/Mutation/removeMember"); - - await removeMemberResolverRemoveSelfError?.({}, args, context); - } catch (error: unknown) { - expect(spy).toHaveBeenCalledWith(USER_REMOVING_SELF.MESSAGE); - expect((error as Error).message).toEqual( - `Translated ${USER_REMOVING_SELF.MESSAGE}`, - ); - } - }); - it("should throw admin cannot remove another admin error when user with _id === args.data.userId is also an admin in the organization", async () => { const { requestContext } = await import("../../../src/libraries"); const spy = vi diff --git a/tests/services/deleteFile.spec.ts b/tests/services/deleteFile.spec.ts index 0acc40f036..08dae85ddb 100644 --- a/tests/services/deleteFile.spec.ts +++ b/tests/services/deleteFile.spec.ts @@ -15,29 +15,29 @@ vi.mock("../../src/models", () => ({ }, })); -vi.mock("../../src/REST/controllers/minio", () => ({ +vi.mock("../../src/REST/services/minio", () => ({ deleteFile: vi.fn(), })); -describe("src -> REST -> services -> file -> deleteFile", () => { - const objectKey = "test/file/path"; - const fileId = "12345"; - let serverRunning = false; +const objectKey = "test/file/path"; +const fileId = "12345"; +let serverRunning = false; - // Wait for server status check before running tests - beforeAll(async () => { - try { - serverRunning = await isMinioRunning(); - } catch (error) { - console.error("Error checking MinIO server status:", error); - serverRunning = false; - } - }); +beforeAll(async () => { + try { + serverRunning = await isMinioRunning(); + } catch (error) { + console.error("Error checking MinIO server status:", error); + } +}); - beforeEach(() => { - vi.clearAllMocks(); - }); +// Clear mocks before each test +beforeEach(() => { + vi.clearAllMocks(); +}); +// Generic file tests +describe("File deletion logic", () => { it("should return success:false and message if file is not found", async () => { vi.mocked(File.findOne).mockResolvedValueOnce(null); @@ -67,42 +67,6 @@ describe("src -> REST -> services -> file -> deleteFile", () => { expect(mockFile.save).toHaveBeenCalled(); }); - // Use describe block with a dynamic condition - describe("MinIO server integration tests", () => { - beforeEach(async () => { - serverRunning = await isMinioRunning(); - }); - - it("should delete the file from the database and storage if reference count is 1", async () => { - // Skip this test if server is not running - if (!serverRunning) { - console.log("Skipping MinIO test - server not running"); - return; - } - - const mockFile = { - referenceCount: 1, - _id: fileId, - id: fileId, - }; - vi.mocked(File.findOne).mockResolvedValueOnce(mockFile); - - const deleteFileFromBucketSpy = vi.spyOn(minioServices, "deleteFile"); - - const result = await deleteFile(objectKey, fileId); - - expect(result).toEqual({ - success: true, - message: "File deleted successfully", - }); - expect(File.deleteOne).toHaveBeenCalledWith({ _id: fileId }); - expect(deleteFileFromBucketSpy).toHaveBeenCalledWith( - BUCKET_NAME, - objectKey, - ); - }); - }); - it("should handle errors and return an error message", async () => { const mockError = new Error("Deletion failed"); vi.mocked(File.findOne).mockRejectedValueOnce(mockError); @@ -121,3 +85,37 @@ describe("src -> REST -> services -> file -> deleteFile", () => { ); }); }); + +// MinIO-specific tests +describe("MinIO server integration tests", () => { + beforeAll(async () => { + serverRunning = await isMinioRunning(); + }); + + it("should delete the file from the database and storage if reference count is 1", async () => { + if (!serverRunning) { + return; // Skip the test if the server isn't running + } + + const mockFile = { + referenceCount: 1, + _id: fileId, + id: fileId, + }; + vi.mocked(File.findOne).mockResolvedValueOnce(mockFile); + + const deleteFileFromBucketSpy = vi.spyOn(minioServices, "deleteFile"); + + const result = await deleteFile(objectKey, fileId); + + expect(result).toEqual({ + success: true, + message: "File deleted successfully", + }); + expect(File.deleteOne).toHaveBeenCalledWith({ _id: fileId }); + expect(deleteFileFromBucketSpy).toHaveBeenCalledWith( + BUCKET_NAME, + objectKey, + ); + }); +});