From ed91d696585dd3c387c8e628a6dd38b0e61c680a Mon Sep 17 00:00:00 2001 From: Sahil <135227614+Sahi1l-Kumar@users.noreply.github.com> Date: Wed, 17 Apr 2024 04:02:43 +0530 Subject: [PATCH 1/2] Fix: Event details cannot be updated by Admins (#2217) --- src/resolvers/Mutation/updateEvent.ts | 23 +++++--- tests/resolvers/Mutation/updateEvent.spec.ts | 59 +++++++++++++++++--- 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/resolvers/Mutation/updateEvent.ts b/src/resolvers/Mutation/updateEvent.ts index d0fcf21a1f2..58ef19bd1b4 100644 --- a/src/resolvers/Mutation/updateEvent.ts +++ b/src/resolvers/Mutation/updateEvent.ts @@ -108,16 +108,25 @@ export const updateEvent: MutationResolvers["updateEvent"] = async ( ); } - const currentUserIsEventAdmin = event.admins.some( - (admin) => - admin === context.userID || - new Types.ObjectId(admin).equals(context.userId), + // Boolean to determine whether user is an admin of organization. + const currentUserIsOrganizationAdmin = currentUserAppProfile.adminFor.some( + (organization) => + organization && + new Types.ObjectId(organization.toString()).equals(event?.organization), ); - // checks if current user is an admin of the event with _id === args.id + // Boolean to determine whether user is an admin of event. + const currentUserIsEventAdmin = event.admins.some((admin) => + admin.equals(currentUser?._id), + ); + + // Checks whether currentUser cannot update event. if ( - currentUserIsEventAdmin === false && - currentUserAppProfile.isSuperAdmin === false + !( + currentUserIsOrganizationAdmin || + currentUserIsEventAdmin || + currentUserAppProfile.isSuperAdmin + ) ) { throw new errors.UnauthorizedError( requestContext.translate(USER_NOT_AUTHORIZED_ERROR.MESSAGE), diff --git a/tests/resolvers/Mutation/updateEvent.spec.ts b/tests/resolvers/Mutation/updateEvent.spec.ts index c826d0e591a..cac49fa52a7 100644 --- a/tests/resolvers/Mutation/updateEvent.spec.ts +++ b/tests/resolvers/Mutation/updateEvent.spec.ts @@ -7,6 +7,7 @@ import { EventAttendee, AppUserProfile, } from "../../../src/models"; +import type { InterfaceAppUserProfile } from "../../../src/models"; import type { MutationUpdateEventArgs } from "../../../src/types/generatedGraphQLTypes"; import { connect, @@ -40,6 +41,7 @@ import { convertToUTCDate } from "../../../src/utilities/recurrenceDatesUtil"; import { addWeeks } from "date-fns"; import { RecurrenceRule } from "../../../src/models/RecurrenceRule"; import { fail } from "assert"; +import { cacheAppUserProfile } from "../../../src/services/AppUserProfileCache/cacheAppUserProfile"; let MONGOOSE_INSTANCE: typeof mongoose; let testUser: TestUserType; @@ -157,14 +159,36 @@ describe("resolvers -> Mutation -> updateEvent", () => { } }); - it(`throws UnauthorizedError if current user with _id === context.userId is - not an admin of event with _id === args.id`, async () => { + it(`throws UnauthorizedError if user with _id === context.userId is neither an + admin of organization with _id === event.organization for event with _id === args.id + or an admin for event with _id === args.id`, async () => { const { requestContext } = await import("../../../src/libraries"); const spy = vi .spyOn(requestContext, "translate") - .mockImplementation((message) => `Translated ${message}`); - + .mockImplementationOnce((message) => message); try { + await AppUserProfile.updateOne( + { + userId: testUser?._id, + }, + { + $set: { + adminFor: [], + }, + }, + ); + + await Event.updateOne( + { + _id: testEvent?._id, + }, + { + $set: { + admins: [], + }, + }, + ); + const args: MutationUpdateEventArgs = { id: testEvent?._id, }; @@ -179,11 +203,9 @@ describe("resolvers -> Mutation -> updateEvent", () => { await updateEventResolver?.({}, args, context); } catch (error: unknown) { - expect(spy).toHaveBeenCalledWith(USER_NOT_AUTHORIZED_ERROR.MESSAGE); + expect(spy).toBeCalledWith(USER_NOT_AUTHORIZED_ERROR.MESSAGE); if (error instanceof Error) { - expect(error.message).toEqual( - `Translated ${USER_NOT_AUTHORIZED_ERROR.MESSAGE}`, - ); + USER_NOT_AUTHORIZED_ERROR.MESSAGE; } else { fail(`Expected UnauthorizedError, but got ${error}`); } @@ -191,6 +213,26 @@ describe("resolvers -> Mutation -> updateEvent", () => { }); it(`updates the event with _id === args.id and returns the updated event`, async () => { + const updatedTestUserAppProfile = await AppUserProfile.findOneAndUpdate( + { + userId: testUser?._id, + }, + { + $push: { + adminFor: testOrganization?._id, + }, + }, + { + new: true, + }, + ).lean(); + + if (updatedTestUserAppProfile !== null) { + await cacheAppUserProfile([ + updatedTestUserAppProfile as InterfaceAppUserProfile, + ]); + } + const updatedEvent = await Event.findOneAndUpdate( { _id: testEvent?._id, @@ -1397,7 +1439,6 @@ describe("resolvers -> Mutation -> updateEvent", () => { recurring: false, startDate: "Tue Feb 14 2023", startTime: "", - title: "Random", }, }; From 8f0e5979a46ac8b35558a2defc3b2c8c353ab43f Mon Sep 17 00:00:00 2001 From: Atharva Kanherkar <142440039+Atharva-Kanherkar@users.noreply.github.com> Date: Thu, 18 Apr 2024 01:04:36 +0530 Subject: [PATCH 2/2] Test : Tests for src/resolvers/Mutation/updateAdvertisement.ts (#2219) * Added a unit test for app user profile * Updated 100% coverage for updateAdvertisement * ran prettier * fixed linting error * Ran prettier again * removed redundant code * Ran prettier --- src/resolvers/Mutation/updateAdvertisement.ts | 38 ++--- .../Mutation/updateAdvertisement.spec.ts | 160 +++++++++++++++++- 2 files changed, 170 insertions(+), 28 deletions(-) diff --git a/src/resolvers/Mutation/updateAdvertisement.ts b/src/resolvers/Mutation/updateAdvertisement.ts index 94f06a6bedc..b4b1992ba61 100644 --- a/src/resolvers/Mutation/updateAdvertisement.ts +++ b/src/resolvers/Mutation/updateAdvertisement.ts @@ -9,7 +9,11 @@ import { USER_NOT_FOUND_ERROR, } from "../../constants"; import { errors, requestContext } from "../../libraries"; -import type { InterfaceAppUserProfile, InterfaceUser } from "../../models"; +import type { + InterfaceAdvertisement, + InterfaceAppUserProfile, + InterfaceUser, +} from "../../models"; import { Advertisement, AppUserProfile, User } from "../../models"; import { cacheAppUserProfile } from "../../services/AppUserProfileCache/cacheAppUserProfile"; import { findAppUserProfileCache } from "../../services/AppUserProfileCache/findAppUserProfileCache"; @@ -163,27 +167,21 @@ export const updateAdvertisement: MutationResolvers["updateAdvertisement"] = }, ).lean(); - if (!updatedAdvertisement) { - throw new errors.NotFoundError( - requestContext.translate(ADVERTISEMENT_NOT_FOUND_ERROR.MESSAGE), - ADVERTISEMENT_NOT_FOUND_ERROR.CODE, - ADVERTISEMENT_NOT_FOUND_ERROR.PARAM, - ); - } - const updatedAdvertisementPayload = { - _id: updatedAdvertisement._id.toString(), // Ensure _id is converted to String as per GraphQL schema - name: updatedAdvertisement.name, - organizationId: updatedAdvertisement.organizationId, - mediaUrl: updatedAdvertisement.mediaUrl, - type: updatedAdvertisement.type, - startDate: updatedAdvertisement.startDate, - endDate: updatedAdvertisement.endDate, - createdAt: updatedAdvertisement.createdAt, - updatedAt: updatedAdvertisement.updatedAt, - creatorId: updatedAdvertisement.creatorId, + _id: updatedAdvertisement?._id?.toString(), // Ensure _id is converted to String as per GraphQL schema + name: updatedAdvertisement?.name, + organizationId: updatedAdvertisement?.organizationId, + mediaUrl: updatedAdvertisement?.mediaUrl, + type: updatedAdvertisement?.type, + startDate: updatedAdvertisement?.startDate, + endDate: updatedAdvertisement?.endDate, + createdAt: updatedAdvertisement?.createdAt, + updatedAt: updatedAdvertisement?.updatedAt, + creatorId: updatedAdvertisement?.creatorId, }; return { - advertisement: { ...updatedAdvertisementPayload }, + advertisement: { + ...updatedAdvertisementPayload, + } as InterfaceAdvertisement, }; }; diff --git a/tests/resolvers/Mutation/updateAdvertisement.spec.ts b/tests/resolvers/Mutation/updateAdvertisement.spec.ts index a5701dec5bb..5f4128296e4 100644 --- a/tests/resolvers/Mutation/updateAdvertisement.spec.ts +++ b/tests/resolvers/Mutation/updateAdvertisement.spec.ts @@ -12,7 +12,11 @@ import { USER_NOT_FOUND_ERROR, } from "../../../src/constants"; import { ApplicationError } from "../../../src/libraries/errors"; -import { Advertisement } from "../../../src/models"; +import { + Advertisement, + AppUserProfile, + Organization, +} from "../../../src/models"; import { updateAdvertisement as updateAdvertisementResolver } from "../../../src/resolvers/Mutation/updateAdvertisement"; import type { MutationUpdateAdvertisementArgs } from "../../../src/types/generatedGraphQLTypes"; import * as uploadEncodedImage from "../../../src/utilities/encodedImageStorage/uploadEncodedImage"; @@ -23,18 +27,49 @@ import { type TestSuperAdminType, } from "../../helpers/advertisement"; import { connect, disconnect } from "../../helpers/db"; -import { createTestUser, type TestUserType } from "../../helpers/userAndOrg"; - +import type { + TestOrganizationType, + TestUserType, +} from "../../helpers/userAndOrg"; +import { createTestUser } from "../../helpers/userAndOrg"; let MONGOOSE_INSTANCE: typeof mongoose; let testUser: TestUserType; let testAdvertisement: TestAdvertisementType; let testSuperAdmin: TestSuperAdminType; +let testAdminUser: TestUserType; +let randomUser: TestUserType; +let testAdminUser2: TestUserType; +let testOrganization: TestOrganizationType; +let testAdvertisement2: TestAdvertisementType; beforeAll(async () => { MONGOOSE_INSTANCE = await connect(); testUser = await createTestUser(); + randomUser = await createTestUser(); + testAdminUser = await createTestUser(); testSuperAdmin = await createTestSuperAdmin(); testAdvertisement = await createTestAdvertisement(); + testAdminUser2 = await createTestUser(); + testOrganization = await Organization.create({ + name: "name", + description: "description", + isPublic: true, + creator: testAdminUser?._id, + admins: [testAdminUser?._id], + members: [testUser?._id, testAdminUser?._id], + creatorId: testAdminUser?._id, + }); + testAdvertisement2 = await Advertisement.create({ + name: "Test Advertisement", + mediaUrl: "data:image/png;base64,bWVkaWEgY29udG", + type: "POPUP", + startDate: "2023-01-01", + endDate: "2023-01-31", + organizationId: testOrganization?._id, + createdAt: "2024-01-13T18:23:00.316Z", + updatedAt: "2024-01-13T20:28:21.292Z", + creatorId: testUser?._id, + }); }); afterAll(async () => { @@ -101,7 +136,6 @@ describe("resolvers -> Mutation -> updateAdvertisement", () => { expect(spy).toHaveBeenLastCalledWith(USER_NOT_AUTHORIZED_ERROR.MESSAGE); } }); - it(`throws NotFoundError if no advertisement exists with _id === args.id `, async () => { const { requestContext } = await import("../../../src/libraries"); @@ -136,12 +170,23 @@ describe("resolvers -> Mutation -> updateAdvertisement", () => { } }); - it.skip(`updates the advertisement with _id === args.id and returns it`, async () => { + it(`updates the advertisement with _id === args.id and returns it`, async () => { const { requestContext } = await import("../../../src/libraries"); vi.spyOn(requestContext, "translate").mockImplementationOnce( (message: string) => `Translated ${message}`, ); + const superAdminTestUser = await AppUserProfile.findOneAndUpdate( + { + userId: randomUser?._id, + }, + { + isSuperAdmin: true, + }, + { + new: true, + }, + ); const args: MutationUpdateAdvertisementArgs = { input: { _id: testAdvertisement._id, @@ -157,7 +202,7 @@ describe("resolvers -> Mutation -> updateAdvertisement", () => { }, }; - const context = { userId: testSuperAdmin?._id }; + const context = { userId: superAdminTestUser?.userId }; const updateAdvertisementPayload = await updateAdvertisementResolver?.( {}, @@ -190,13 +235,79 @@ describe("resolvers -> Mutation -> updateAdvertisement", () => { } expect(advertisement).toEqual({ advertisement: expectedAdvertisement }); }); + it(`updates the advertisement with _id === args.id and returns it as an org admin`, async () => { + const { requestContext } = await import("../../../src/libraries"); + + vi.spyOn(requestContext, "translate").mockImplementationOnce( + (message: string) => `Translated ${message}`, + ); + + await AppUserProfile.findOneAndUpdate( + { + userId: testAdminUser?._id, + }, + { + adminFor: [testOrganization?._id], + }, + { + new: true, + }, + ); + const args: MutationUpdateAdvertisementArgs = { + input: { + _id: testAdvertisement2._id, + name: "New Advertisement Name", + mediaFile: "data:image/png;base64,bWaWEgY29udGVudA==", + type: "POPUP", + startDate: new Date(new Date().getFullYear() + 0, 11, 31) + .toISOString() + .split("T")[0], + endDate: new Date(new Date().getFullYear() + 1, 11, 31) + .toISOString() + .split("T")[0], + }, + }; + + const context = { userId: testAdminUser?.id }; + + const updateAdvertisementPayload = await updateAdvertisementResolver?.( + {}, + args, + context, + ); + const advertisement = updateAdvertisementPayload || {}; + + const updatedTestAdvertisement = await Advertisement.findOne({ + _id: testAdvertisement2._id, + }).lean(); - it.skip(`updates the advertisement media and returns it`, async () => { + let expectedAdvertisement; + + if (!updatedTestAdvertisement) { + console.error("Updated advertisement not found in the database"); + } else { + expectedAdvertisement = { + _id: updatedTestAdvertisement._id.toString(), // Ensure _id is converted to String as per GraphQL schema + name: updatedTestAdvertisement.name, + organizationId: updatedTestAdvertisement.organizationId, + mediaUrl: updatedTestAdvertisement.mediaUrl, + type: updatedTestAdvertisement.type, + startDate: updatedTestAdvertisement.startDate, + endDate: updatedTestAdvertisement.endDate, + createdAt: updatedTestAdvertisement.createdAt, + updatedAt: updatedTestAdvertisement.updatedAt, + creatorId: updatedTestAdvertisement.creatorId, + }; + } + expect(advertisement).toEqual({ advertisement: expectedAdvertisement }); + }); + it(`updates the advertisement media and returns it`, async () => { const { requestContext } = await import("../../../src/libraries"); vi.spyOn(requestContext, "translate").mockImplementationOnce( (message: string) => `Translated ${message}`, ); + const args: MutationUpdateAdvertisementArgs = { input: { _id: testAdvertisement._id, @@ -206,7 +317,7 @@ describe("resolvers -> Mutation -> updateAdvertisement", () => { }, }; - const context = { userId: testSuperAdmin?._id }; + const context = { userId: testSuperAdmin?.id }; const updateAdvertisementPayload = await updateAdvertisementResolver?.( {}, @@ -395,4 +506,37 @@ describe("resolvers -> Mutation -> updateAdvertisement", () => { ); } }); + it("throws UnauthorizedError if the user does not have an app profile", async () => { + await AppUserProfile.deleteOne({ userId: testAdminUser2?._id }); + + try { + const { requestContext } = await import("../../../src/libraries"); + + vi.spyOn(requestContext, "translate").mockImplementationOnce( + (message: string) => `Translated ${message}`, + ); + const args: MutationUpdateAdvertisementArgs = { + input: { + _id: testAdvertisement._id, + name: "New Advertisement Name", + mediaFile: "data:image/png;base64,bWaWEgY29udGVudA==", + type: "POPUP", + startDate: new Date(new Date().getFullYear() + 0, 11, 31) + .toISOString() + .split("T")[0], + endDate: new Date(new Date().getFullYear() + 1, 11, 31) + .toISOString() + .split("T")[0], + }, + }; + + const context = { userId: testAdminUser2?._id }; + + await updateAdvertisementResolver?.({}, args, context); + } catch (error: unknown) { + expect((error as Error).message).toEqual( + `Translated ${USER_NOT_AUTHORIZED_ERROR.MESSAGE}`, + ); + } + }); });