-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Test: src/graphql/types/FundCampaignPledge/updater.ts #3190
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
Test: src/graphql/types/FundCampaignPledge/updater.ts #3190
Conversation
unit test
WalkthroughThis pull request extracts the resolver logic for the FundCampaignPledge type into a new function named resolveUpdater. The function enforces authentication, performs concurrent queries for the current user and related fund campaign data, and includes comprehensive error handling and role authorization checks. A new ContextType is defined to support the updated context. A corresponding test suite has been added using Vitest, covering scenarios such as unauthenticated access, missing campaign or user data, null updater IDs, and proper user lookup and authorization. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Resolver as resolveUpdater
participant UserDB
participant CampaignDB
Client->>Resolver: Call resolveUpdater(parent, _args, ctx)
Resolver->>Resolver: Check if ctx.currentClient.isAuthenticated
alt Not authenticated
Resolver-->>Client: Throw TalawaGraphQLError
else Authenticated
Resolver->>UserDB: Query for current user and, if needed, updater user concurrently
Resolver->>CampaignDB: Query for existing fund campaign (using parent.campaignId)
alt updaterId is null
Resolver-->>Client: Return null
else updaterId equals currentUserId
Resolver-->>Client: Return currentUser
else
Resolver->>UserDB: Fetch user by updaterId
alt User found
Resolver-->>Client: Return fetched updater
else User not found
Resolver-->>Client: Log error and throw TalawaGraphQLError
end
end
end
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🔇 Additional comments (13)
✨ 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/graphql/types/FundCampaignPledge/updater.ts
(1 hunks)test/graphql/types/FundCampaignPledge/FundCampaignPledge.test.ts
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Pull request workflow
src/graphql/types/FundCampaignPledge/updater.ts
[warning] 1-1: Import statements could be sorted.
[warning] 4-4: Formatter would have printed the following content.
test/graphql/types/FundCampaignPledge/FundCampaignPledge.test.ts
[warning] 1-1: Import statements could be sorted.
[warning] 1-1: Formatter would have printed the following content.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (4)
src/graphql/types/FundCampaignPledge/updater.ts (2)
9-77
: Well-structured resolver with proper error handling and concurrent data fetching!The implementation shows good practices:
- Concurrent data fetching using Promise.all
- Descriptive error messages
- Type-safe context handling
94-131
: Efficient updater resolution logic with proper error handling!The implementation shows good practices:
- Early return for null updaterId
- Optimization by returning currentUser when possible
- Proper error handling for non-existent updater
test/graphql/types/FundCampaignPledge/FundCampaignPledge.test.ts (2)
1-52
: Well-structured test setup with comprehensive mocks!The test setup shows good practices:
- Complete mock coverage of dependencies
- Realistic test data
- Clear type definitions
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[warning] 1-1: Import statements could be sorted.
[warning] 1-1: Formatter would have printed the following content.
132-214
: Thorough query validation tests!The tests effectively verify:
- Correct query parameters
- Proper membership validation
- Query construction accuracy
test/graphql/types/FundCampaignPledge/FundCampaignPledge.test.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/graphql/types/FundCampaignPledge/updater.ts
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Pull request workflow
src/graphql/types/FundCampaignPledge/updater.ts
[warning] 1-1: Import statements could be sorted.
[warning] 1-1: Formatter would have printed the following content.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (6)
src/graphql/types/FundCampaignPledge/updater.ts (6)
1-4
: Sort import statements to address pipeline warning.
This suggestion has already been made in a previous review. Below is the same diff for reference:-import { User } from "~/src/graphql/types/User/User"; -import { TalawaGraphQLError } from "~/src/utilities/TalawaGraphQLError"; -import { FundCampaignPledge } from "./FundCampaignPledge"; -import type { ExplicitGraphQLContext, ImplicitMercuriusContext } from '~/src/graphql/context'; +import type { ExplicitGraphQLContext, ImplicitMercuriusContext } from '~/src/graphql/context'; +import { FundCampaignPledge } from "./FundCampaignPledge"; +import { TalawaGraphQLError } from "~/src/utilities/TalawaGraphQLError"; +import { User } from "~/src/graphql/types/User/User";🧰 Tools
🪛 GitHub Actions: Pull request workflow
[warning] 1-1: Import statements could be sorted.
[warning] 1-1: Formatter would have printed the following content.
6-6
: Type alias for combined context looks good.
Defining and using a unified context type helps ensure type safety and clarity across resolvers.
9-57
: Clean resolver structure and concurrent data fetching.
Your approach using Promise.all() to fetch both the current user and campaign simultaneously is efficient. The surrounding conditions (e.g., checking authentication) are logically sound.
82-92
: Enhance error message for unauthorized access.
As mentioned in a prior review, consider adding a descriptive message in addition to the error code to clarify why the request was denied.
94-100
: Simple and intuitive updaterId check.
Returning null when no updater exists, or the current user object when IDs match, is clear and logical.
123-131
: Nicely integrated resolver assignment.
Registering the new resolver function within the FundCampaignPledge type is straightforward and fits well with your project’s patterns.
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@palisadoes I haven't removed any code, just shifted a block of code above the component and called it back in, the comparison git is making looks messed up, but the code is same FundCampaignPledge.implement({ this resolveUpdater is extracted from pledge and called back here and also in test file. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3190 +/- ##
====================================================
+ Coverage 42.08% 42.38% +0.29%
====================================================
Files 455 455
Lines 33647 33648 +1
Branches 563 578 +15
====================================================
+ Hits 14161 14261 +100
+ Misses 19486 19387 -99 ☔ View full report in Codecov by Sentry. |
d3415dd
into
PalisadoesFoundation:develop-postgres
Goal
This PR improves the code coverage for the file src/graphql/types/FundCampaignPledge/updater.ts, ensuring 100% coverage across lines, functions, statements, and branches.
Tasks Completed
Issue
This PR addresses #3036
Note
Resolver was extracted carefully to make it available in test file, it wouldn't cause breaking changes.
Summary by CodeRabbit
New Features
Tests