Skip to content

Test to src/graphql/types/Mutation/deleteAgendaItem.ts #3256

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

Merged
merged 2 commits into from
Feb 19, 2025

Conversation

im-vedant
Copy link

@im-vedant im-vedant commented Feb 19, 2025

What kind of change does this PR introduce?

Issue Number:

Fixes #3010

Snapshots/Videos:

If relevant, did you update the documentation?

Summary

Does this PR introduce a breaking change?

Checklist

CodeRabbit AI Review

  • I have reviewed and addressed all critical issues flagged by CodeRabbit AI
  • I have implemented or provided justification for each non-critical suggestion
  • I have documented my reasoning in the PR comments where CodeRabbit AI suggestions were not implemented

Test Coverage

  • I have written tests for all new changes/features
  • I have verified that test coverage meets or exceeds 95%
  • I have run the test suite locally and all tests pass

Other information

Have you read the contributing guide?

Summary by CodeRabbit

  • Refactor
    • Enhanced agenda item deletion functionality with consistent naming conventions and improved modularity in resolver logic.
  • Tests
    • Expanded test coverage for the deletion resolver, validating various scenarios including authentication, input errors, and successful deletions.

Copy link

coderabbitai bot commented Feb 19, 2025

Walkthrough

The changes standardize the naming convention for the delete agenda item GraphQL input schema by renaming the exported variable from mutationDeleteAgendaItemInputSchema to MutationDeleteAgendaItemInputSchema. This update is reflected in both the input definition and the mutation resolver, where the inline logic is extracted into a new asynchronous function (deleteAgendaItemResolver). Additionally, a comprehensive test suite has been introduced to validate the resolver’s behavior under various scenarios, including authentication, input validation, and error handling.

Changes

File(s) Change Summary
src/graphql/inputs/.../MutationDeleteAgendaItemInput.ts
src/graphql/types/Mutation/deleteAgendaItem.ts
Renamed mutationDeleteAgendaItemInputSchema to MutationDeleteAgendaItemInputSchema and updated its reference. Added deleteAgendaItemResolver function to handle the deleteAgendaItem mutation logic.
test/graphql/types/Mutation/deleteAgendaItem.test.ts Introduced tests for deleteAgendaItemResolver, covering scenarios for authentication, input validation, and deletion success/failure using mocks.

Possibly related PRs

Suggested reviewers

  • palisadoes
  • gautam-divyanshu
✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Our Pull Request Approval Process

Thanks for contributing!

Testing Your Code

Remember, your PRs won't be reviewed until these criteria are met:

  1. We don't merge PRs with poor code quality.
    1. Follow coding best practices such that CodeRabbit.ai approves your PR.
  2. We don't merge PRs with failed tests.
    1. When tests fail, click on the Details link to learn more.
    2. Write sufficient tests for your changes (CodeCov Patch Test). Your testing level must be better than the target threshold of the repository
    3. Tests may fail if you edit sensitive files. Ask to add the ignore-sensitive-files-pr label if the edits are necessary.
  3. We cannot merge PRs with conflicting files. These must be fixed.

Our policies make our code better.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

  1. PRs with issues not assigned to you will be closed by the reviewer
  2. Fix the first comment in the PR so that each issue listed automatically closes

Other

  1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
  2. Read the CONTRIBUTING.md file make

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (1)
src/graphql/types/Mutation/deleteAgendaItem.ts (1)

1-11: ⚠️ Potential issue

Sort import statements.

The pipeline indicates that import statements need to be sorted.

Apply this diff to sort the imports:

+import { eq } from "drizzle-orm";
+import { z } from "zod";
+import type { GraphQLContext } from "~/src/graphql/context";
+import { agendaItemsTable } from "~/src/drizzle/tables/agendaItems";
+import { builder } from "~/src/graphql/builder";
+import {
+  MutationDeleteAgendaItemInput,
+  MutationDeleteAgendaItemInputSchema,
+} from "~/src/graphql/inputs/MutationDeleteAgendaItemInput";
+import { AgendaItem } from "~/src/graphql/types/AgendaItem/AgendaItem";
+import { TalawaGraphQLError } from "~/src/utilities/TalawaGraphQLError";
🧰 Tools
🪛 GitHub Actions: Pull request workflow

[error] 3-11: Import statements could be sorted.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 84562a3 and 6029923.

📒 Files selected for processing (3)
  • src/graphql/inputs/MutationDeleteAgendaItemInput.ts (1 hunks)
  • src/graphql/types/Mutation/deleteAgendaItem.ts (1 hunks)
  • test/graphql/types/Mutation/deleteAgendaItem.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
test/graphql/types/Mutation/deleteAgendaItem.test.ts (5)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:33-49
Timestamp: 2025-02-10T18:57:27.311Z
Learning: In test/graphql/types/Mutation/updateAgendaItem.test.ts, while a global mock setup is used for the drizzle client, an alternative approach is to use specialized mocks initialized in beforeEach for each test scenario to reduce global state and ensure test isolation. However, this requires careful type handling to maintain type safety.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:62-79
Timestamp: 2025-02-10T18:48:15.739Z
Learning: When mocking GraphQLContext in tests, keep the minio property with minimal implementation to satisfy TypeScript types, even if minio operations aren't used in the specific resolver being tested.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:1-2
Timestamp: 2025-02-09T04:24:57.021Z
Learning: In test files for Talawa API, create a TestContext interface that extends GraphQLContext but overrides specific fields needed for mocking. Use the `satisfies` operator to ensure type safety of the mock implementations.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:46-46
Timestamp: 2025-02-10T17:58:51.912Z
Learning: When mocking objects in TypeScript tests, prefer explicit type annotations (e.g., `const mock: ExpectedType = {...}`) over type assertions with `as unknown as` to maintain type safety and catch potential type mismatches at compile time.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:8-24
Timestamp: 2025-02-10T18:40:01.230Z
Learning: In test files for Talawa API, mock interfaces should include explicit return type definitions that match the expected database schema to ensure type safety and catch potential issues during testing.
src/graphql/types/Mutation/deleteAgendaItem.ts (1)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: src/graphql/types/Mutation/updateAgendaItem.ts:46-84
Timestamp: 2025-02-08T11:15:47.878Z
Learning: When creating GraphQL mutation resolvers in Talawa API, ensure the resolver function's type signature matches the GraphQL schema types exactly, especially for nullable fields. The context parameter should be typed as `GraphQLContext` imported from "~/src/graphql/context".
🪛 GitHub Actions: Pull request workflow
test/graphql/types/Mutation/deleteAgendaItem.test.ts

[error] 6-41: Formatter would have printed the following content: Incorrect formatting in MockDrizzleClient interface.

src/graphql/types/Mutation/deleteAgendaItem.ts

[error] 3-11: Import statements could be sorted.


[error] 17-25: Formatter would have printed the following content: Incorrect formatting in function deleteAgendaItemResolver.

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (3)
src/graphql/inputs/MutationDeleteAgendaItemInput.ts (1)

5-11: LGTM! Improved naming convention.

The renaming to PascalCase for the exported schema aligns with TypeScript naming conventions for types and schemas.

src/graphql/types/Mutation/deleteAgendaItem.ts (1)

158-171: LGTM! Well-structured mutation field registration.

The mutation field registration is clear and properly typed.

test/graphql/types/Mutation/deleteAgendaItem.test.ts (1)

84-366: LGTM! Comprehensive test coverage.

The test suite thoroughly covers various scenarios:

  • Authentication checks
  • Input validation
  • Resource existence validation
  • Authorization checks
  • Successful deletion
  • Error handling

The test implementation follows best practices from retrieved learnings:

  • Uses TestContext interface
  • Properly mocks GraphQLContext
  • Maintains test isolation with beforeEach

Copy link

Our Pull Request Approval Process

Thanks for contributing!

Testing Your Code

Remember, your PRs won't be reviewed until these criteria are met:

  1. We don't merge PRs with poor code quality.
    1. Follow coding best practices such that CodeRabbit.ai approves your PR.
  2. We don't merge PRs with failed tests.
    1. When tests fail, click on the Details link to learn more.
    2. Write sufficient tests for your changes (CodeCov Patch Test). Your testing level must be better than the target threshold of the repository
    3. Tests may fail if you edit sensitive files. Ask to add the ignore-sensitive-files-pr label if the edits are necessary.
  3. We cannot merge PRs with conflicting files. These must be fixed.

Our policies make our code better.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

  1. PRs with issues not assigned to you will be closed by the reviewer
  2. Fix the first comment in the PR so that each issue listed automatically closes

Other

  1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
  2. Read the CONTRIBUTING.md file make

Copy link

codecov bot commented Feb 19, 2025

Codecov Report

Attention: Patch coverage is 98.31933% with 2 lines in your changes missing coverage. Please review.

Project coverage is 45.83%. Comparing base (84562a3) to head (7c9569f).
Report is 1 commits behind head on develop-postgres.

Files with missing lines Patch % Lines
src/graphql/types/Mutation/deleteAgendaItem.ts 98.29% 2 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                  @@
##           develop-postgres    #3256      +/-   ##
====================================================
+ Coverage             45.48%   45.83%   +0.35%     
====================================================
  Files                   455      455              
  Lines                 33935    33940       +5     
  Branches                757      770      +13     
====================================================
+ Hits                  15435    15558     +123     
+ Misses                18496    18378     -118     
  Partials                  4        4              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6029923 and 7c9569f.

📒 Files selected for processing (2)
  • src/graphql/types/Mutation/deleteAgendaItem.ts (1 hunks)
  • test/graphql/types/Mutation/deleteAgendaItem.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
test/graphql/types/Mutation/deleteAgendaItem.test.ts (5)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:33-49
Timestamp: 2025-02-10T18:57:27.311Z
Learning: In test/graphql/types/Mutation/updateAgendaItem.test.ts, while a global mock setup is used for the drizzle client, an alternative approach is to use specialized mocks initialized in beforeEach for each test scenario to reduce global state and ensure test isolation. However, this requires careful type handling to maintain type safety.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:62-79
Timestamp: 2025-02-10T18:48:15.739Z
Learning: When mocking GraphQLContext in tests, keep the minio property with minimal implementation to satisfy TypeScript types, even if minio operations aren't used in the specific resolver being tested.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:1-2
Timestamp: 2025-02-09T04:24:57.021Z
Learning: In test files for Talawa API, create a TestContext interface that extends GraphQLContext but overrides specific fields needed for mocking. Use the `satisfies` operator to ensure type safety of the mock implementations.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:8-24
Timestamp: 2025-02-10T18:40:01.230Z
Learning: In test files for Talawa API, mock interfaces should include explicit return type definitions that match the expected database schema to ensure type safety and catch potential issues during testing.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:46-46
Timestamp: 2025-02-10T17:58:51.912Z
Learning: When mocking objects in TypeScript tests, prefer explicit type annotations (e.g., `const mock: ExpectedType = {...}`) over type assertions with `as unknown as` to maintain type safety and catch potential type mismatches at compile time.
src/graphql/types/Mutation/deleteAgendaItem.ts (1)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: src/graphql/types/Mutation/updateAgendaItem.ts:46-84
Timestamp: 2025-02-08T11:15:47.878Z
Learning: When creating GraphQL mutation resolvers in Talawa API, ensure the resolver function's type signature matches the GraphQL schema types exactly, especially for nullable fields. The context parameter should be typed as `GraphQLContext` imported from "~/src/graphql/context".
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Run tests for talawa api
  • GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (15)
src/graphql/types/Mutation/deleteAgendaItem.ts (9)

5-14: Imports and schema usage look correct.

Changing the input schema reference to MutationDeleteAgendaItemInputSchema and importing GraphQLContext are consistent with the GraphQL-based approach.


17-32: Function signature aligns well with GraphQL context and usage.

The deleteAgendaItemResolver signature matches the GraphQL schema and context type consistently. The unauthenticated check is clear and straightforward.


34-50: Zod parsing is well-implemented.

Using z.object(...).safeParse(args) and providing structured error details is a robust way to handle invalid inputs.


52-98: Parallel queries for user and agenda item are efficient.

Executing both queries with Promise.all is a good performance choice, minimizing round-trip times to the database. The error handling patterns remain consistent.


100-106: Clear handling of missing user scenario.

Throwing a TalawaGraphQLError for an undefined user is consistent with the existing error-handling pattern.


108-119: Proper error response when agenda item is not found.

The code thoroughly covers a missing agenda item, providing a clear error code and relevant details.


142-154: Deletion logic and checks are well-handled.

The Drizzle ORM-based deletion, combined with a check of the return value, ensures reliable detection of unexpected cases.


156-157: Clear and consistent return of the deleted agenda item.

Returning the item provides useful context for the client after a successful deletion.


159-172: Mutation field registration follows best practices.

Defining the deleteAgendaItem field in the builder with explicit args and resolver cleanly ties the schema to the resolver.

test/graphql/types/Mutation/deleteAgendaItem.test.ts (6)

1-7: Imports are correctly organized.

The test relies on Vitest, mock utilities, and relevant GraphQL resolver references, aligning with Talawa's testing conventions.


70-99: Unauthenticated user test covers the essential error-throw scenario.

The check for extensions.code and the thrown unauthenticated error reflects desired security constraints.


100-114: Invalid UUID test is accurate.

Validating the UUID format ensures early detection of incorrect arguments.


116-262: Comprehensive coverage of error conditions.

These tests systematically handle missing users, non-existent agenda items, and insufficient roles, demonstrating robust coverage of edge cases.


264-307: Successful deletion test is thorough.

Verifying the database mock return value and ensuring the resolver response match fosters confidence in the happy path outcome.


309-355: Failure scenario test ensures complete coverage.

Testing when the deletion operation returns an empty array helps confirm that unexpected outcomes are handled gracefully.

@palisadoes palisadoes merged commit dee12a8 into PalisadoesFoundation:develop-postgres Feb 19, 2025
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants