From d164ddd39891f7f067cd118544e156d64d7c18af Mon Sep 17 00:00:00 2001 From: Aaradhy Sharma Date: Wed, 24 Jul 2024 11:13:10 +0530 Subject: [PATCH 1/4] fix: issue #2382 by upgrading graphql version - Updated the `graphql` dependency in `package.json` from version `^16.8.1` to `^16.9.0` - Regenerated `package-lock.json` to reflect the changes in dependencies This change addresses issue #2382 by ensuring compatibility with the latest features and bug fixes in the `graphql` package. Closes #2382 --- package-lock.json | 9 +++++---- package.json | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5f635ee91f..4c8b9083f9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30,7 +30,7 @@ "express": "^4.19.2", "express-mongo-sanitize": "^2.2.0", "express-rate-limit": "^7.3.1", - "graphql": "^16.8.1", + "graphql": "^16.9.0", "graphql-depth-limit": "^1.1.0", "graphql-scalars": "^1.20.1", "graphql-subscriptions": "^2.0.0", @@ -10087,9 +10087,10 @@ "dev": true }, "node_modules/graphql": { - "version": "16.8.1", - "resolved": "https://registry.npmjs.org/graphql/-/graphql-16.8.1.tgz", - "integrity": "sha512-59LZHPdGZVh695Ud9lRzPBVTtlX9ZCV150Er2W43ro37wVof0ctenSaskPPjN7lVTIN8mSZt8PHUNKZuNQUuxw==", + "version": "16.9.0", + "resolved": "https://registry.npmjs.org/graphql/-/graphql-16.9.0.tgz", + "integrity": "sha512-GGTKBX4SD7Wdb8mqeDLni2oaRGYQWjWHGKPQ24ZMnUtKfcsVoiv4uX8+LJr1K6U5VW2Lu1BwJnj7uiori0YtRw==", + "license": "MIT", "engines": { "node": "^12.22.0 || ^14.16.0 || ^16.0.0 || >=17.0.0" } diff --git a/package.json b/package.json index 50bec11467..c43575a2b5 100644 --- a/package.json +++ b/package.json @@ -64,7 +64,7 @@ "express": "^4.19.2", "express-mongo-sanitize": "^2.2.0", "express-rate-limit": "^7.3.1", - "graphql": "^16.8.1", + "graphql": "^16.9.0", "graphql-depth-limit": "^1.1.0", "graphql-scalars": "^1.20.1", "graphql-subscriptions": "^2.0.0", From 998f844c7a18b156f72b7eae8042f7cea166790c Mon Sep 17 00:00:00 2001 From: Aaradhy Sharma Date: Fri, 26 Jul 2024 12:11:15 +0530 Subject: [PATCH 2/4] test: Add tests for 100% coverage of src/resolvers/User/posts.ts - Implemented comprehensive test cases for `posts.ts` to ensure 100% code coverage. - Added tests for various pagination arguments (`first`, `last`, `before`, `after`). - Included edge cases for no posts and invalid filters. - Verified error handling for invalid arguments and cursor values. Fixes #1928 --- tests/resolvers/User/post.spec.ts | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/resolvers/User/post.spec.ts b/tests/resolvers/User/post.spec.ts index efb1b1f706..6b5de69087 100644 --- a/tests/resolvers/User/post.spec.ts +++ b/tests/resolvers/User/post.spec.ts @@ -1,4 +1,5 @@ // Replace with the correct path + import { GraphQLError } from "graphql"; import type mongoose from "mongoose"; import { Types } from "mongoose"; @@ -62,6 +63,7 @@ describe("resolvers -> User -> post", () => { } } }); + it(`returns the expected connection object`, async () => { const parent = testUser as InterfaceUser; const connection = await postResolver?.( @@ -92,7 +94,49 @@ describe("resolvers -> User -> post", () => { expect(connection?.pageInfo.startCursor).toEqual(testPost2?._id.toString()); expect(connection?.totalCount).toEqual(totalCount); }); + + it("returns an empty connection object if no posts are found", async () => { + await Post.deleteMany({ creatorId: testUser?._id }); + const parent = testUser as InterfaceUser; + const connection = await postResolver?.(parent, { first: 2 }, {}); + + expect(connection?.edges).toHaveLength(0); + expect(connection?.totalCount).toEqual(0); + expect(connection?.pageInfo.endCursor).toBeNull(); + expect(connection?.pageInfo.startCursor).toBeNull(); + expect(connection?.pageInfo.hasNextPage).toBe(false); + expect(connection?.pageInfo.hasPreviousPage).toBe(false); + }); + + it("handles different pagination arguments correctly", async () => { + // Recreate posts for pagination testing + testPost = await Post.create({ + text: `text${nanoid().toLowerCase()}`, + creatorId: testUser?._id, + organization: testOrganization?._id, + pinned: false, + }); + testPost2 = await Post.create({ + text: `text${nanoid().toLowerCase()}`, + creatorId: testUser?._id, + organization: testOrganization?._id, + pinned: false, + }); + + const parent = testUser as InterfaceUser; + + const connectionFirst = await postResolver?.(parent, { first: 1 }, {}); + expect(connectionFirst?.edges).toHaveLength(1); + expect(connectionFirst?.pageInfo.hasNextPage).toBe(true); + expect(connectionFirst?.pageInfo.hasPreviousPage).toBe(false); + + const connectionLast = await postResolver?.(parent, { last: 1 }, {}); + expect(connectionLast?.edges).toHaveLength(1); + expect(connectionLast?.pageInfo.hasNextPage).toBe(false); + expect(connectionLast?.pageInfo.hasPreviousPage).toBe(true); + }); }); + describe("parseCursor function", () => { it("returns failure state if argument cursorValue is an invalid cursor", async () => { const result = await parseCursor({ @@ -121,4 +165,18 @@ describe("parseCursor function", () => { expect(result.parsedCursor).toEqual(testPost?._id.toString()); } }); + + it("returns failure state if creatorId is invalid", async () => { + const result = await parseCursor({ + cursorName: "after", + cursorPath: ["after"], + cursorValue: testPost?._id.toString() as string, + creatorId: new Types.ObjectId().toString(), + }); + + expect(result.isSuccessful).toEqual(false); + if (result.isSuccessful === false) { + expect(result.errors.length).toBeGreaterThan(0); + } + }); }); From 352fe69f9eb34c45473cb26f449a16843dbcb305 Mon Sep 17 00:00:00 2001 From: Aaradhy Sharma Date: Fri, 26 Jul 2024 21:40:50 +0530 Subject: [PATCH 3/4] test: Add tests for 100% coverage of src/resolvers/User/posts.ts update-2 --- tests/resolvers/User/post.spec.ts | 43 +++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/resolvers/User/post.spec.ts b/tests/resolvers/User/post.spec.ts index 6b5de69087..f13970be8e 100644 --- a/tests/resolvers/User/post.spec.ts +++ b/tests/resolvers/User/post.spec.ts @@ -56,9 +56,9 @@ describe("resolvers -> User -> post", () => { await postResolver?.(parent, {}, {}); } catch (error) { if (error instanceof GraphQLError) { - expect(error.extensions.code).toEqual("INVALID_ARGUMENTS"); + expect(error.extensions?.code).toEqual("INVALID_ARGUMENTS"); expect( - (error.extensions.errors as DefaultGraphQLArgumentError[]).length, + (error.extensions?.errors as DefaultGraphQLArgumentError[]).length, ).toBeGreaterThan(0); } } @@ -135,6 +135,45 @@ describe("resolvers -> User -> post", () => { expect(connectionLast?.pageInfo.hasNextPage).toBe(false); expect(connectionLast?.pageInfo.hasPreviousPage).toBe(true); }); + + // Additional tests to cover parseCursor in postResolver + it("throws an error for invalid cursor value", async () => { + const parent = testUser as InterfaceUser; + const args = { after: "invalidCursor" }; // Simulate an invalid cursor value + try { + await postResolver?.(parent, args, {}); + } catch (error) { + if (error instanceof GraphQLError) { + expect(error.extensions?.code).toEqual("INVALID_ARGUMENTS"); + } else { + throw error; // Re-throw if not a GraphQLError + } + } + }); + + it("handles valid cursor value", async () => { + const parent = testUser as InterfaceUser; + const args = { after: testPost?._id.toString() }; // Use a valid cursor value + const connection = await postResolver?.(parent, args, {}); + expect(connection).toBeDefined(); + expect(connection?.edges.length).toBeGreaterThan(0); // Check if posts are returned + }); + + it("handles missing cursor value gracefully", async () => { + const parent = testUser as InterfaceUser; + const args = {}; // No cursor provided + const connection = await postResolver?.(parent, args, {}); + expect(connection).toBeDefined(); + expect(connection?.edges.length).toBeGreaterThan(0); // Check if posts are returned + }); + + it("handles cursor value with pagination arguments", async () => { + const parent = testUser as InterfaceUser; + const args = { after: testPost?._id.toString(), first: 2 }; // Valid cursor with pagination + const connection = await postResolver?.(parent, args, {}); + expect(connection).toBeDefined(); + expect(connection?.edges.length).toBeLessThanOrEqual(2); // Check if pagination works + }); }); describe("parseCursor function", () => { From e382af52b77208ddda63c1027622fd0c07ca9a57 Mon Sep 17 00:00:00 2001 From: Aaradhy Sharma Date: Sat, 27 Jul 2024 11:44:51 +0530 Subject: [PATCH 4/4] Fix and improve User post resolver tests - Update 'handles valid cursor value' test to be more flexible - Ensure test passes with variable post order - Maintain test coverage and functionality verification --- tests/resolvers/User/post.spec.ts | 119 ++++++++++++++++++++++-------- 1 file changed, 88 insertions(+), 31 deletions(-) diff --git a/tests/resolvers/User/post.spec.ts b/tests/resolvers/User/post.spec.ts index f13970be8e..d5efab53d4 100644 --- a/tests/resolvers/User/post.spec.ts +++ b/tests/resolvers/User/post.spec.ts @@ -27,6 +27,7 @@ let MONGOOSE_INSTANCE: typeof mongoose; let testUser: TestUserType; let testPost: TestPostType; let testPost2: TestPostType; +let testPost3: TestPostType; let testOrganization: TestOrganizationType; beforeAll(async () => { @@ -42,6 +43,12 @@ beforeAll(async () => { organization: testOrganization?._id, pinned: false, }); + testPost3 = await Post.create({ + text: `text${nanoid().toLowerCase()}`, + creatorId: testUser?._id, + organization: testOrganization?._id, + pinned: false, + }); }); afterAll(async () => { @@ -69,11 +76,10 @@ describe("resolvers -> User -> post", () => { const connection = await postResolver?.( parent, { - first: 2, + first: 3, }, {}, ); - console.log(connection, testPost2?._id, testPost?._id); const totalCount = await Post.find({ creatorId: testUser?._id, }).countDocuments(); @@ -83,15 +89,18 @@ describe("resolvers -> User -> post", () => { // Check individual properties // console.log(connection?.edges[0]); expect((connection?.edges[0] as unknown as PostEdge).cursor).toEqual( - testPost2?._id.toString(), + testPost3?._id.toString(), ); expect((connection?.edges[1] as unknown as PostEdge).cursor).toEqual( + testPost2?._id.toString(), + ); + expect((connection?.edges[2] as unknown as PostEdge).cursor).toEqual( testPost?._id.toString(), ); expect(connection?.pageInfo.endCursor).toEqual(testPost?._id.toString()); expect(connection?.pageInfo.hasNextPage).toBe(false); expect(connection?.pageInfo.hasPreviousPage).toBe(false); - expect(connection?.pageInfo.startCursor).toEqual(testPost2?._id.toString()); + expect(connection?.pageInfo.startCursor).toEqual(testPost3?._id.toString()); expect(connection?.totalCount).toEqual(totalCount); }); @@ -122,6 +131,12 @@ describe("resolvers -> User -> post", () => { organization: testOrganization?._id, pinned: false, }); + testPost3 = await Post.create({ + text: `text${nanoid().toLowerCase()}`, + creatorId: testUser?._id, + organization: testOrganization?._id, + pinned: false, + }); const parent = testUser as InterfaceUser; @@ -136,46 +151,45 @@ describe("resolvers -> User -> post", () => { expect(connectionLast?.pageInfo.hasPreviousPage).toBe(true); }); - // Additional tests to cover parseCursor in postResolver it("throws an error for invalid cursor value", async () => { const parent = testUser as InterfaceUser; - const args = { after: "invalidCursor" }; // Simulate an invalid cursor value - try { - await postResolver?.(parent, args, {}); - } catch (error) { - if (error instanceof GraphQLError) { - expect(error.extensions?.code).toEqual("INVALID_ARGUMENTS"); - } else { - throw error; // Re-throw if not a GraphQLError - } - } + const args = { after: "invalidCursor", first: 10 }; + await expect(postResolver?.(parent, args, {})).rejects.toThrow(); }); it("handles valid cursor value", async () => { const parent = testUser as InterfaceUser; - const args = { after: testPost?._id.toString() }; // Use a valid cursor value + const args = { after: testPost2?._id.toString(), first: 10 }; const connection = await postResolver?.(parent, args, {}); expect(connection).toBeDefined(); - expect(connection?.edges.length).toBeGreaterThan(0); // Check if posts are returned - }); + expect(connection?.edges.length).toBeGreaterThan(0); - it("handles missing cursor value gracefully", async () => { - const parent = testUser as InterfaceUser; - const args = {}; // No cursor provided - const connection = await postResolver?.(parent, args, {}); - expect(connection).toBeDefined(); - expect(connection?.edges.length).toBeGreaterThan(0); // Check if posts are returned - }); + const allPostIds = [testPost, testPost2, testPost3].map((post) => + post?._id.toString(), + ); - it("handles cursor value with pagination arguments", async () => { - const parent = testUser as InterfaceUser; - const args = { after: testPost?._id.toString(), first: 2 }; // Valid cursor with pagination - const connection = await postResolver?.(parent, args, {}); - expect(connection).toBeDefined(); - expect(connection?.edges.length).toBeLessThanOrEqual(2); // Check if pagination works + const returnedCursor = (connection?.edges[0] as unknown as PostEdge).cursor; + expect(allPostIds).toContain(returnedCursor); + expect(returnedCursor).not.toEqual(testPost2?._id.toString()); }); }); +it("handles missing cursor value gracefully", async () => { + const parent = testUser as InterfaceUser; + const args = { first: 10 }; + const connection = await postResolver?.(parent, args, {}); + expect(connection).toBeDefined(); + expect(connection?.edges.length).toBeGreaterThan(0); +}); + +it("handles cursor value with pagination arguments", async () => { + const parent = testUser as InterfaceUser; + const args = { after: testPost?._id.toString(), first: 2 }; + const connection = await postResolver?.(parent, args, {}); + expect(connection).toBeDefined(); + expect(connection?.edges.length).toBeLessThanOrEqual(2); +}); + describe("parseCursor function", () => { it("returns failure state if argument cursorValue is an invalid cursor", async () => { const result = await parseCursor({ @@ -218,4 +232,47 @@ describe("parseCursor function", () => { expect(result.errors.length).toBeGreaterThan(0); } }); + + it("handles empty string cursor value", async () => { + try { + await parseCursor({ + cursorName: "after", + cursorPath: ["after"], + cursorValue: "", + creatorId: testUser?._id.toString() as string, + }); + } catch (error) { + expect(error).toBeDefined(); + expect((error as Error).message).toContain("Cast to ObjectId failed"); + } + }); + + it("handles invalid ObjectId string as cursor value", async () => { + try { + await parseCursor({ + cursorName: "after", + cursorPath: ["after"], + cursorValue: "invalidObjectId", + creatorId: testUser?._id.toString() as string, + }); + } catch (error) { + expect(error).toBeDefined(); + expect((error as Error).message).toContain("Cast to ObjectId failed"); + } + }); + + it("handles non-existent ObjectId as cursor value", async () => { + const nonExistentId = new Types.ObjectId().toString(); + const result = await parseCursor({ + cursorName: "after", + cursorPath: ["after"], + cursorValue: nonExistentId, + creatorId: testUser?._id.toString() as string, + }); + + expect(result.isSuccessful).toEqual(false); + if (result.isSuccessful === false) { + expect(result.errors.length).toBeGreaterThan(0); + } + }); });