Skip to content
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

Migrated to Integration-Test : Test/routes/graphql/Mutation/deletePost : #3289

Merged
merged 14 commits into from
Feb 25, 2025

Conversation

iamanishx
Copy link

@iamanishx iamanishx commented Feb 24, 2025

What kind of change does this PR introduce?

Issue Number: #3275

Fixes #
N/A

Snapshots/Videos:

If relevant, did you update the documentation?

Summary
created createRegularUserUsingAdmin so that this can be used in every integration-test
as a helper function

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

  • New Features

    • Introduced enhanced post management capabilities, enabling reliable creation and deletion of posts along with associated attachments.
  • Refactor

    • Streamlined the deletion process to improve clarity, maintainability, and robust handling of authorization and error scenarios.
  • Tests

    • Expanded test coverage for post operations, validating authentication, authorization, input integrity, and transaction handling.
    • Added support for administrative workflows in testing to simplify creating standard user accounts.

Copy link

coderabbitai bot commented Feb 24, 2025

Walkthrough

This pull request refactors the GraphQL API's deletePost mutation by embedding the resolver logic directly within the mutation field definition. It also restructures the test suite for deletePost by replacing mocked contexts with real GraphQL API operations using the mercuriusClient. Additionally, a helper function for creating a regular user via admin credentials has been added, and new GraphQL document nodes and type definitions for both deletePost and createPost mutations have been introduced.

Changes

File(s) Change Summary
src/graphql/types/Mutation/deletePost.ts Refactored the deletePost resolver by removing the separate deletePostResolver and integrating its logic within builder.mutationField for improved organization.
test/routes/graphql/Mutation/deletePost.test.ts Reorganized test suite to use mercuriusClient for real API interactions; enhanced tests for authentication, post existence, authorization, invalid arguments, and transaction handling.
test/routes/graphql/createRegularUserUsingAdmin.ts Added a new async function to create a regular user using admin credentials, performing sign-in and user creation mutations with validation of tokens and IDs.
test/routes/graphql/documentNodes.ts, test/routes/graphql/gql.tada-cache.d.ts Introduced new GraphQL mutation definitions for deletePost and createPost to expand API functionality and enforce type safety.

Possibly related PRs

  • wrote Test: src/graphql/types/Mutation/deletePost.ts #3182: The changes in the main PR involve restructuring the deletePost mutation resolver directly within the mutation field definition, while the retrieved PR focuses on extracting the resolver logic into a separate function, indicating a direct modification of the same functionality but in opposite directions.

Suggested labels

ignore-sensitive-files-pr

Suggested reviewers

  • palisadoes

📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 64da513 and 5ab24da.

📒 Files selected for processing (1)
  • test/routes/graphql/Mutation/deletePost.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
test/routes/graphql/Mutation/deletePost.test.ts (8)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: test/routes/graphql/Mutation/deletePost.test.ts:315-331
Timestamp: 2025-02-24T06:22:14.006Z
Learning: For test utilities that are only used once, prefer keeping them inline rather than extracting to separate helper files to maintain code locality.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:25-50
Timestamp: 2025-02-09T08:28:18.161Z
Learning: In test files for talawa-api, using type assertions (e.g., `as GraphQLContext`) for mock objects is acceptable and preferred over implementing complete type interfaces, as it reduces boilerplate and only mocks the properties that are actually used in the tests.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:131-156
Timestamp: 2025-02-24T05:36:27.896Z
Learning: When working with test files in the Talawa API project, use the original server code implementation without modifications to ensure tests accurately reflect the actual server behavior.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: test/routes/graphql/Mutation/deletePost.test.ts:80-100
Timestamp: 2025-02-24T06:40:09.248Z
Learning: In integration tests for Talawa API, each test case should create organizations with unique details (name, description, etc.) to maintain test isolation and aid debugging. Avoid reusing organization details across test cases.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:112-128
Timestamp: 2025-02-09T10:01:17.775Z
Learning: In the deletePost mutation, post deletion permissions are strictly controlled:
1. Non-admin post owners cannot delete posts without a membership record
2. A valid membership record with admin rights is required for post deletion, regardless of post ownership
3. The membership check is performed via existingPost.organization.membershipsWhereOrganization[0]
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: src/graphql/types/Mutation/deletePost.ts:120-125
Timestamp: 2025-02-09T13:40:01.937Z
Learning: The deletePost mutation's primary error handling focus is on authentication, authorization, and data validation. Additional error handling for MinIO operations can be improved in a future PR.
⏰ 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 (3)
test/routes/graphql/Mutation/deletePost.test.ts (3)

1-25: LGTM! Well-structured test setup.

The imports are well-organized, and the admin authentication setup is properly reused across test cases.


27-471: Excellent test coverage and organization!

The test suites comprehensively cover all critical scenarios:

  • Authentication and authorization
  • Resource existence validation
  • Error handling for invalid inputs
  • Transaction failures
  • MinIO integration failures
  • Successful deletion path

The test cases are well-structured with clear setup, action, and assertion phases.


122-122: Remove debug console.log statements.

These console.log statements should be removed as they were likely used during development.

Also applies to: 127-127, 134-134, 431-431, 454-454, 473-473

✨ 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

@iamanishx iamanishx changed the title Pre commit Added Integration-Test for est/routes/graphql/Mutation/deletePost : Feb 24, 2025
Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.56%. Comparing base (2438e61) to head (5ab24da).
Report is 2 commits behind head on develop-postgres.

Additional details and impacted files
@@                 Coverage Diff                  @@
##           develop-postgres    #3289      +/-   ##
====================================================
+ Coverage             48.36%   48.56%   +0.19%     
====================================================
  Files                   458      458              
  Lines                 34516    34533      +17     
  Branches                966      976      +10     
====================================================
+ Hits                  16695    16772      +77     
+ Misses                17821    17761      -60     

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

@iamanishx iamanishx changed the title Added Integration-Test for est/routes/graphql/Mutation/deletePost : Migreated to Integration-Test : Test/routes/graphql/Mutation/deletePost : Feb 24, 2025
@iamanishx iamanishx changed the title Migreated to Integration-Test : Test/routes/graphql/Mutation/deletePost : Migrated to Integration-Test : Test/routes/graphql/Mutation/deletePost : Feb 24, 2025
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: 7

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0aa74c2 and f52366f.

📒 Files selected for processing (5)
  • src/graphql/types/Mutation/deletePost.ts (1 hunks)
  • test/routes/graphql/Mutation/deletePost.test.ts (1 hunks)
  • test/routes/graphql/createRegularUserUsingAdmin.ts (1 hunks)
  • test/routes/graphql/documentNodes.ts (1 hunks)
  • test/routes/graphql/gql.tada-cache.d.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
test/routes/graphql/Mutation/deletePost.test.ts (3)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:25-50
Timestamp: 2025-02-09T08:28:18.161Z
Learning: In test files for talawa-api, using type assertions (e.g., `as GraphQLContext`) for mock objects is acceptable and preferred over implementing complete type interfaces, as it reduces boilerplate and only mocks the properties that are actually used in the tests.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:55-58
Timestamp: 2025-02-09T07:56:50.950Z
Learning: When testing GraphQL resolvers in TypeScript, using type casting (as unknown as) is an acceptable pattern for creating invalid arguments to test error scenarios, especially when the schema requires mandatory fields.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:112-128
Timestamp: 2025-02-09T10:01:17.775Z
Learning: In the deletePost mutation, post deletion permissions are strictly controlled:
1. Non-admin post owners cannot delete posts without a membership record
2. A valid membership record with admin rights is required for post deletion, regardless of post ownership
3. The membership check is performed via existingPost.organization.membershipsWhereOrganization[0]
test/routes/graphql/gql.tada-cache.d.ts (1)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:25-50
Timestamp: 2025-02-09T08:28:18.161Z
Learning: In test files for talawa-api, using type assertions (e.g., `as GraphQLContext`) for mock objects is acceptable and preferred over implementing complete type interfaces, as it reduces boilerplate and only mocks the properties that are actually used in the tests.
src/graphql/types/Mutation/deletePost.ts (2)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:55-58
Timestamp: 2025-02-09T07:56:50.950Z
Learning: When testing GraphQL resolvers in TypeScript, using type casting (as unknown as) is an acceptable pattern for creating invalid arguments to test error scenarios, especially when the schema requires mandatory fields.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: src/graphql/types/Mutation/deletePost.ts:120-125
Timestamp: 2025-02-09T13:40:01.937Z
Learning: The deletePost mutation's primary error handling focus is on authentication, authorization, and data validation. Additional error handling for MinIO operations can be improved in a future PR.
⏰ 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 (7)
test/routes/graphql/documentNodes.ts (1)

529-558: LGTM! Well-structured mutation definitions.

The new Mutation_deletePost and Mutation_createPost mutations are properly defined with appropriate return types and input validation. The comment clarifying the test purpose is helpful.

test/routes/graphql/gql.tada-cache.d.ts (1)

71-74: LGTM! Well-defined type definitions.

The new mutation types are properly defined with correct input validation and return types, maintaining consistency with the existing type system.

test/routes/graphql/Mutation/deletePost.test.ts (3)

27-199: LGTM! Comprehensive test coverage for authentication and error scenarios.

The test cases thoroughly verify authentication requirements and error handling.


279-418: LGTM! Thorough error handling tests.

The test cases for transaction and minio failures are well-implemented and cover important edge cases.


419-488: LGTM! Positive case testing is comprehensive.

The successful deletion test properly verifies both the deletion and the returned data structure.

src/graphql/types/Mutation/deletePost.ts (2)

26-33: LGTM! Authentication check is properly implemented.

The authentication check is performed early in the resolver and uses appropriate error handling.


35-51: LGTM! Input validation is thorough and well-structured.

The implementation uses zod for schema validation and provides detailed error messages with argument paths.

@palisadoes
Copy link
Contributor

  1. Please make coderabbit.ai approves your work.
  2. Make sure all tests pass and are valid.
  3. Ensure the test code coverage for your patch reaches as close to 100% as possible.

@iamanishx
Copy link
Author

  1. Please make coderabbit.ai approves your work.
  2. Make sure all tests pass and are valid.
  3. Ensure the test code coverage for your patch reaches as close to 100% as possible.

on it sir pls allow me few minutes

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 24, 2025
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 f52366f and 4c4dad3.

📒 Files selected for processing (3)
  • test/routes/graphql/Mutation/deletePost.test.ts (1 hunks)
  • test/routes/graphql/documentNodes.ts (1 hunks)
  • test/routes/graphql/gql.tada-cache.d.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
test/routes/graphql/documentNodes.ts (3)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:131-156
Timestamp: 2025-02-24T05:36:27.896Z
Learning: When working with test files in the Talawa API project, use the original server code implementation without modifications to ensure tests accurately reflect the actual server behavior.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
test/routes/graphql/gql.tada-cache.d.ts (4)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:25-50
Timestamp: 2025-02-09T08:28:18.161Z
Learning: In test files for talawa-api, using type assertions (e.g., `as GraphQLContext`) for mock objects is acceptable and preferred over implementing complete type interfaces, as it reduces boilerplate and only mocks the properties that are actually used in the tests.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:131-156
Timestamp: 2025-02-24T05:36:27.896Z
Learning: When working with test files in the Talawa API project, use the original server code implementation without modifications to ensure tests accurately reflect the actual server behavior.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
test/routes/graphql/Mutation/deletePost.test.ts (7)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:25-50
Timestamp: 2025-02-09T08:28:18.161Z
Learning: In test files for talawa-api, using type assertions (e.g., `as GraphQLContext`) for mock objects is acceptable and preferred over implementing complete type interfaces, as it reduces boilerplate and only mocks the properties that are actually used in the tests.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:131-156
Timestamp: 2025-02-24T05:36:27.896Z
Learning: When working with test files in the Talawa API project, use the original server code implementation without modifications to ensure tests accurately reflect the actual server behavior.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:112-128
Timestamp: 2025-02-09T10:01:17.775Z
Learning: In the deletePost mutation, post deletion permissions are strictly controlled:
1. Non-admin post owners cannot delete posts without a membership record
2. A valid membership record with admin rights is required for post deletion, regardless of post ownership
3. The membership check is performed via existingPost.organization.membershipsWhereOrganization[0]
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:55-58
Timestamp: 2025-02-09T07:56:50.950Z
Learning: When testing GraphQL resolvers in TypeScript, using type casting (as unknown as) is an acceptable pattern for creating invalid arguments to test error scenarios, especially when the schema requires mandatory fields.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: src/graphql/types/Mutation/deletePost.ts:120-125
Timestamp: 2025-02-09T13:40:01.937Z
Learning: The deletePost mutation's primary error handling focus is on authentication, authorization, and data validation. Additional error handling for MinIO operations can be improved in a future PR.
⏰ 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 (5)
test/routes/graphql/documentNodes.ts (1)

581-591: LGTM! Well-structured GraphQL mutations.

The mutations are properly typed and follow the project's patterns for GraphQL document nodes.

Also applies to: 594-609

test/routes/graphql/gql.tada-cache.d.ts (1)

73-76: LGTM! Well-defined type definitions.

The type definitions for both mutations are thorough and properly handle nullability.

test/routes/graphql/Mutation/deletePost.test.ts (3)

1-26: LGTM! Clean setup with proper imports.

The test setup is well-organized with clear imports and initial admin authentication.


27-50: LGTM! Thorough authentication testing.

The unauthenticated test case properly verifies error codes and response structure.


27-480: LGTM! Comprehensive test coverage.

The test suite provides excellent coverage of various scenarios including:

  • Authentication and authorization
  • Resource existence
  • Invalid arguments
  • Transaction handling
  • MinIO integration
  • Success cases

The tests follow best practices by:

  • Using real GraphQL operations instead of mocks
  • Properly cleaning up resources
  • Verifying error codes and messages
  • Testing edge cases and error scenarios

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 24, 2025
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 4c4dad3 and 3651286.

📒 Files selected for processing (1)
  • test/routes/graphql/Mutation/deletePost.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
test/routes/graphql/Mutation/deletePost.test.ts (6)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: test/routes/graphql/Mutation/deletePost.test.ts:315-331
Timestamp: 2025-02-24T06:22:14.006Z
Learning: For test utilities that are only used once, prefer keeping them inline rather than extracting to separate helper files to maintain code locality.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:25-50
Timestamp: 2025-02-09T08:28:18.161Z
Learning: In test files for talawa-api, using type assertions (e.g., `as GraphQLContext`) for mock objects is acceptable and preferred over implementing complete type interfaces, as it reduces boilerplate and only mocks the properties that are actually used in the tests.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:131-156
Timestamp: 2025-02-24T05:36:27.896Z
Learning: When working with test files in the Talawa API project, use the original server code implementation without modifications to ensure tests accurately reflect the actual server behavior.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:112-128
Timestamp: 2025-02-09T10:01:17.775Z
Learning: In the deletePost mutation, post deletion permissions are strictly controlled:
1. Non-admin post owners cannot delete posts without a membership record
2. A valid membership record with admin rights is required for post deletion, regardless of post ownership
3. The membership check is performed via existingPost.organization.membershipsWhereOrganization[0]
⏰ 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 (8)
test/routes/graphql/Mutation/deletePost.test.ts (8)

1-6: No issues found.
The imports and helper references are clearly structured, and there are no obvious problems here.


28-49: Thorough coverage for the unauthenticated scenario.
These assertions correctly validate the error code and path, ensuring the GraphQL API rejects unauthenticated requests.


52-77: Good checks for a non-existent post.
The test accurately verifies that a non-existent post yields the expected error code.


79-144: Validation for unavailable roles and resources looks solid.
The scenario for unauthorized user deletion covers the intended error handling.


146-167: Straightforward invalid arguments checks.
This test scenario correctly validates GraphQL input format and error codes.


169-195: Good handling for deleted/unknown current user.
The test ensures that requests from non-existent users produce the correct unauthenticated error code.


197-272: Non-admin membership scenario is well-tested.
The test effectively verifies that only authorized roles can delete another user’s post.


414-469: Successful deletion scenario is thoroughly tested.
The suite verifies correct behavior for authorized deletes, ensuring the returned data matches expectation.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 24, 2025
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

♻️ Duplicate comments (1)
test/routes/graphql/Mutation/deletePost.test.ts (1)

16-25: 🧹 Nitpick (assertive)

Consider using consistent camelCase naming.

The variable adminauthToken should follow camelCase convention to match the codebase style.

-const adminauthToken = signInResult.data.signIn.authenticationToken;
+const adminAuthToken = signInResult.data.signIn.authenticationToken;
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 3651286 and e566a3d.

📒 Files selected for processing (1)
  • test/routes/graphql/Mutation/deletePost.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
test/routes/graphql/Mutation/deletePost.test.ts (8)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: test/routes/graphql/Mutation/deletePost.test.ts:315-331
Timestamp: 2025-02-24T06:22:14.006Z
Learning: For test utilities that are only used once, prefer keeping them inline rather than extracting to separate helper files to maintain code locality.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:25-50
Timestamp: 2025-02-09T08:28:18.161Z
Learning: In test files for talawa-api, using type assertions (e.g., `as GraphQLContext`) for mock objects is acceptable and preferred over implementing complete type interfaces, as it reduces boilerplate and only mocks the properties that are actually used in the tests.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:131-156
Timestamp: 2025-02-24T05:36:27.896Z
Learning: When working with test files in the Talawa API project, use the original server code implementation without modifications to ensure tests accurately reflect the actual server behavior.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: test/routes/graphql/Mutation/deletePost.test.ts:80-100
Timestamp: 2025-02-24T06:40:09.248Z
Learning: In integration tests for Talawa API, each test case should create organizations with unique details (name, description, etc.) to maintain test isolation and aid debugging. Avoid reusing organization details across test cases.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:112-128
Timestamp: 2025-02-09T10:01:17.775Z
Learning: In the deletePost mutation, post deletion permissions are strictly controlled:
1. Non-admin post owners cannot delete posts without a membership record
2. A valid membership record with admin rights is required for post deletion, regardless of post ownership
3. The membership check is performed via existingPost.organization.membershipsWhereOrganization[0]
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: src/graphql/types/Mutation/deletePost.ts:120-125
Timestamp: 2025-02-09T13:40:01.937Z
Learning: The deletePost mutation's primary error handling focus is on authentication, authorization, and data validation. Additional error handling for MinIO operations can be improved in a future PR.
⏰ 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 (1)
test/routes/graphql/Mutation/deletePost.test.ts (1)

1-472: Excellent test coverage and structure!

The test suite is well-organized and covers all critical scenarios:

  • Authentication and authorization
  • Resource existence validation
  • Error handling for various failure modes
  • Transaction and MinIO integration
  • Input validation

The use of real API interactions via mercuriusClient instead of mocked contexts makes the tests more reliable.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 24, 2025
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: 1

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between e566a3d and 64da513.

📒 Files selected for processing (1)
  • test/routes/graphql/Mutation/deletePost.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
test/routes/graphql/Mutation/deletePost.test.ts (8)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:55-86
Timestamp: 2025-02-24T05:59:10.768Z
Learning: PR #3275 is focused on test improvements, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests. Changes to server-side code are out of scope for this PR.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: test/routes/graphql/Mutation/deletePost.test.ts:315-331
Timestamp: 2025-02-24T06:22:14.006Z
Learning: For test utilities that are only used once, prefer keeping them inline rather than extracting to separate helper files to maintain code locality.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:25-50
Timestamp: 2025-02-09T08:28:18.161Z
Learning: In test files for talawa-api, using type assertions (e.g., `as GraphQLContext`) for mock objects is acceptable and preferred over implementing complete type interfaces, as it reduces boilerplate and only mocks the properties that are actually used in the tests.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:131-156
Timestamp: 2025-02-24T05:36:27.896Z
Learning: When working with test files in the Talawa API project, use the original server code implementation without modifications to ensure tests accurately reflect the actual server behavior.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: test/routes/graphql/Mutation/deletePost.test.ts:80-100
Timestamp: 2025-02-24T06:40:09.248Z
Learning: In integration tests for Talawa API, each test case should create organizations with unique details (name, description, etc.) to maintain test isolation and aid debugging. Avoid reusing organization details across test cases.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: test/routes/graphql/Mutation/deletePost.test.ts:112-128
Timestamp: 2025-02-09T10:01:17.775Z
Learning: In the deletePost mutation, post deletion permissions are strictly controlled:
1. Non-admin post owners cannot delete posts without a membership record
2. A valid membership record with admin rights is required for post deletion, regardless of post ownership
3. The membership check is performed via existingPost.organization.membershipsWhereOrganization[0]
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3182
File: src/graphql/types/Mutation/deletePost.ts:120-125
Timestamp: 2025-02-09T13:40:01.937Z
Learning: The deletePost mutation's primary error handling focus is on authentication, authorization, and data validation. Additional error handling for MinIO operations can be improved in a future PR.
🪛 GitHub Actions: Pull request workflow
test/routes/graphql/Mutation/deletePost.test.ts

[error] 197-197: Formatter would have printed the following content: Expected formatting changes in the test file.

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (8)
test/routes/graphql/Mutation/deletePost.test.ts (8)

1-25: LGTM! Clean test setup with proper imports and authentication.

The initial setup efficiently establishes the test environment with necessary imports and admin authentication that will be reused across test cases.


28-50: LGTM! Well-structured authentication test.

The test properly verifies both the null response and the specific error code for unauthenticated access.


52-77: LGTM! Thorough validation of non-existent resource handling.

The test effectively verifies the API's behavior when attempting to delete a non-existent post.


79-144: LGTM! Comprehensive authorization test.

The test properly sets up the scenario with unique test data and verifies unauthorized access handling.


146-167: LGTM! Good input validation test.

The test effectively verifies the API's handling of invalid input arguments.


169-195: LGTM! Good edge case coverage.

The test effectively verifies the API's behavior when a user has been deleted but their token is still used.


274-353: LGTM! Good transaction failure handling test.

The test effectively verifies transaction failures and ensures proper cleanup.


355-472: LGTM! Comprehensive coverage of MinIO failures and successful deletions.

Both test suites effectively verify their respective scenarios with proper setup and cleanup.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 24, 2025
@iamanishx
Copy link
Author

@palisadoes @xoldd sir this PR is ready for review
i don't know why the test is failing even after the approval from rabbit ai !!!!!!

@iamanishx
Copy link
Author

@coderabbitai full review

Copy link

coderabbitai bot commented Feb 24, 2025

✅ Actions performed

Full review triggered.

@disha1202 disha1202 added the ignore-sensitive-files-pr The contributor has a legitimate reason for editiing protected files label Feb 25, 2025
@im-vedant
Copy link

@iamanishx @disha1202 I think there should be cleanup functions to clean the resources which are created after each test. For example, deleting regular users once test is completed.

@palisadoes
Copy link
Contributor

@iamanishx @disha1202 I think there should be cleanup functions to clean the resources which are created after each test. For example, deleting regular users once test is completed.

The testing methodology guidelines don't require it

@palisadoes palisadoes merged commit 808ff35 into PalisadoesFoundation:develop-postgres Feb 25, 2025
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-sensitive-files-pr The contributor has a legitimate reason for editiing protected files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants