event-creator-test #3181
event-creator-test #3181
Conversation
WalkthroughThis PR refactors the event creator resolver by extracting its logic into a dedicated asynchronous function. The new function performs authentication, authorization, and error handling before returning the creator information. In addition, a comprehensive test suite is added to validate various scenarios such as missing authentication, insufficient permissions, and database anomalies, ensuring full coverage of the resolver's behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant R as eventCreatorResolver
participant DB as Database
C->>R: Request event creator
R->>R: Check authentication
alt Not authenticated
R-->>C: Throw "unauthenticated" error
else Authenticated
R->>DB: Query current user & memberships
alt User not found
R-->>C: Throw "unauthenticated" error
else
R->>R: Verify administrator role
alt Not an administrator
R-->>C: Throw "unauthorized_action" error
else
R->>R: Check if creatorId is null or matches current user
alt creatorId is null
R-->>C: Return null
else creatorId equals current user
R-->>C: Return current user
else
R->>DB: Fetch user by creatorId
alt User not found
R-->>C: Log error and throw "unexpected" error
else
R-->>C: Return fetched user
end
end
end
end
end
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3181 +/- ##
====================================================
+ Coverage 40.33% 40.54% +0.21%
====================================================
Files 455 455
Lines 33558 33574 +16
Branches 446 465 +19
====================================================
+ Hits 13534 13613 +79
+ Misses 20024 19961 -63 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/graphql/types/Event/creator.ts(1 hunks)test/graphql/types/event/creator.test.ts(1 hunks)
⏰ 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 (17)
src/graphql/types/Event/creator.ts (7)
3-5: Imports appear correct and consistent.
No issues noted with the new type imports.
7-11: Distinct resolver function is a good design approach.
Separating the logic into its own function aids maintainability and clarity.
12-18: Clear unauthenticated check.
Throwing a TalawaGraphQLError for non-authenticated users is appropriate. Consider customizing the error message to provide more context if desired for debugging.
20-43: Robust retrieval of current user.
Accessing the database to confirm user presence is well-handled. Ensure that the role retrieval logic in lines 29-31 abides by the correct membership scoping for future expansions of the membership model.
59-65: Graceful handling of null creator ID or current user as creator.
These conditionals ensure consistent returns without unnecessary DB lookups. This is efficient and improves clarity.
86-99: Catch block ensures robust error rethrowing and logging.
Re-throwing TalawaGraphQLError preserves original error context while logging unknown errors helps with investigations. This approach is solid.
101-109: Event type implementation with the new resolver.
Registering the resolver for the “creator” field is straightforward and consistent with the extracted logic.test/graphql/types/event/creator.test.ts (10)
1-23: Mock context and imports look solid.
Using vitest mocks for the context and the Drizzle client is a clean way to isolate the business logic for testing.
24-32: Well-defined MockUser type.
Clearly outlines the structure needed for testing user data, aiding readability.
54-119: Comprehensive authentication and authorization test coverage.
Covering both unauthenticated and unauthorized users is thorough. Consider using parameterized tests to reduce duplication.
121-137: Null creator ID test is precise.
Verifies correct handling of null references, preventing NPE-like issues.
139-158: Accurate test for returning the current user as creator.
Confirms the resolver correctly identifies and returns the currentUser if IDs match.
160-188: Fetching a different user from the DB.
This test ensures the resolver retrieves a separate creator, reinforcing that the logic is not incorrectly hard-coded to the current user.
190-208: Catching the missing user scenario.
Helps confirm that the resolver raises an "unexpected" error when the selected creator doesn't exist.
210-273: Handling various database query errors.
From connection failures to constraint violations and syntax errors, these tests ensure consistent error handling and logging.
276-336: Concurrent access tests are valuable.
Simulating parallel event/creator updates or deletions helps confirm the resolver logs and returns errors consistently.
338-370: Ensuring error pass-through for TalawaGraphQLError.
This preserves the original error properties and is a good approach for layering custom error handling without inadvertently masking known exceptions.
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
test/graphql/types/Event/creator.test.ts(1 hunks)
⏰ 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 (2)
test/graphql/types/Event/creator.test.ts (2)
103-118: Address commented-out test case.The commented-out test case appears to be important for testing admin authorization. Either implement it or remove it with a clear explanation of why it's not needed.
Please clarify if this test case is still needed. If it is, I can help implement it correctly.
33-371: LGTM! Comprehensive test coverage.The test suite provides excellent coverage of various scenarios including:
- Authentication and authorization
- Error handling
- Database interactions
- Concurrent access
- Edge cases
The tests are well-organized and follow good testing practices.
@palisadoes I noticed that some tests use will creating a should make a seperate PR for that ? |
|
And I have resolved issues in this pr for now |
|
@palisadoes this one is for unit testing only with current approach |
palisadoes
merged commit 9892675
into
PalisadoesFoundation:develop-postgres
|
Please create another issue and PR to fix the factory |
What kind of change does this PR introduce?
Added tests for Event creator
Issue Number:
Fixes #3067
Snapshots/Videos:
If relevant, did you update the documentation?
no
Summary
Added test cases for event creator
Does this PR introduce a breaking change?
no
Checklist
CodeRabbit AI Review
Test Coverage
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Refactor
Tests