feat:test to FundCampaign/updatedAt.ts #3205
Conversation
WalkthroughThis PR refactors the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learningstest/graphql/types/FundCampaign/updatedAt.test.ts (1)src/graphql/types/FundCampaign/updatedAt.ts (2)🪛 GitHub Check: codecov/patchsrc/graphql/types/FundCampaign/updatedAt.ts[warning] 57-57: src/graphql/types/FundCampaign/updatedAt.ts#L57 🔇 Additional comments (27)
✨ 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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3205 +/- ##
====================================================
+ Coverage 42.38% 44.13% +1.75%
====================================================
Files 455 455
Lines 33648 33657 +9
Branches 578 675 +97
====================================================
+ Hits 14261 14856 +595
+ Misses 19387 18797 -590
- Partials 0 4 +4 ☔ 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/FundCampaign/updatedAt.ts(1 hunks)test/graphql/types/FundCampaign/updatedAt.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 (4)
src/graphql/types/FundCampaign/updatedAt.ts (2)
5-17: LGTM! Well-structured error handling and authentication.The function signature is well-defined with proper types, and the authentication check is implemented correctly with appropriate error handling.
52-70: LGTM! Thorough error handling with proper logging.The error handling for undefined results is comprehensive, with descriptive error messages and appropriate logging.
test/graphql/types/FundCampaign/updatedAt.test.ts (2)
22-40: LGTM! Well-structured mock context creation.The mock context creation is well-implemented with all necessary properties and proper type casting.
49-232: LGTM! Comprehensive test coverage.The test suite thoroughly covers all scenarios including:
- Authentication checks
- Authorization rules
- Error handling
- Database errors
- Concurrent access issues
The tests are well-organized and follow best practices.
|
@coderabbitai review and approve the PR |
|
@shivasankaran18 I've reviewed the changes. The PR demonstrates high-quality test implementation with comprehensive coverage of various scenarios including authentication, authorization, and error handling. The tests are well-structured and follow best practices. I'll approve the changes. ✅ Actions performedComments resolved and changes approved. Review triggered.
|
|
@palisadoes please review my PR |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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 (2)
src/graphql/types/FundCampaign/updatedAt.ts(1 hunks)test/graphql/types/FundCampaign/updatedAt.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3205
File: src/graphql/types/FundCampaign/updatedAt.ts:21-50
Timestamp: 2025-02-13T16:36:41.119Z
Learning: In the FundCampaign's updatedAt resolver, the fund query intentionally uses currentUserId in the where clause instead of parent.fundId.
src/graphql/types/FundCampaign/updatedAt.ts (1)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3205
File: src/graphql/types/FundCampaign/updatedAt.ts:21-50
Timestamp: 2025-02-13T16:36:41.119Z
Learning: In the FundCampaign's updatedAt resolver, the fund query intentionally uses currentUserId in the where clause instead of parent.fundId.
🪛 GitHub Check: codecov/patch
src/graphql/types/FundCampaign/updatedAt.ts
[warning] 43-43: src/graphql/types/FundCampaign/updatedAt.ts#L43
Added line #L43 was not covered by tests
🔇 Additional comments (2)
src/graphql/types/FundCampaign/updatedAt.ts (1)
5-100: Overall structure looks solid.The resolver logic is cohesive, with clear steps for authentication, user existence checks, fund existence checks, role verification, and error handling. Logging is also integrated appropriately. No further issues spotted.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 43-43: src/graphql/types/FundCampaign/updatedAt.ts#L43
Added line #L43 was not covered by teststest/graphql/types/FundCampaign/updatedAt.test.ts (1)
8-20: Tests are well-structured and comprehensive.Your test coverage spans authentication checks, fund existence checks, authorization checks, and error handling. The approach is well-organized, making it easy to read and maintain.
|
ok @palisadoes .. will make the changes |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/graphql/types/FundCampaign/updatedAt.ts(1 hunks)test/graphql/types/FundCampaign/updatedAt.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3205
File: src/graphql/types/FundCampaign/updatedAt.ts:21-50
Timestamp: 2025-02-13T16:36:41.119Z
Learning: In the FundCampaign's updatedAt resolver, the fund query intentionally uses currentUserId in the where clause instead of parent.fundId.
src/graphql/types/FundCampaign/updatedAt.ts (1)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3205
File: src/graphql/types/FundCampaign/updatedAt.ts:21-50
Timestamp: 2025-02-13T16:36:41.119Z
Learning: In the FundCampaign's updatedAt resolver, the fund query intentionally uses currentUserId in the where clause instead of parent.fundId.
🪛 GitHub Check: codecov/patch
src/graphql/types/FundCampaign/updatedAt.ts
[warning] 43-43: src/graphql/types/FundCampaign/updatedAt.ts#L43
Added line #L43 was not covered by tests
🔇 Additional comments (28)
src/graphql/types/FundCampaign/updatedAt.ts (10)
2-2: Good import usage.
No concerns here; importingGraphQLContextis appropriate for type safety.
10-17: Effective error handling for unauthenticated users.
Catching and throwing a custom error ensures clear feedback to the caller. Good practice.
19-19: Local variable assignment is clear.
Destructuringctx.currentClient.user.idinto a constant is concise and readable.
21-28: Concurrent query is efficient.
Fetching user data with Promise.all is a solid performance choice. No issues found.
29-50: Intentional ID usage for fund query.
Per your learnings, usingcurrentUserIdinstead ofparent.fundIdis deliberate. No change required.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 43-43: src/graphql/types/FundCampaign/updatedAt.ts#L43
Added line #L43 was not covered by tests
52-58: Clear handling of undefined user.
Returning an “unauthenticated” error is appropriate when the user record is absent.
60-70: Proper logging for missing fund scenario.
Logging the error before throwing improves traceability. Well-structured approach.
72-85: Role-based authorization logic is straightforward.
This check correctly blocks non-admin users. No duplication or complexity issues noticed.
87-99: Comprehensive error-handling in catch block.
Re-throwing known errors while wrapping unknown ones is a good pattern for consistent error reporting.
102-110: Field implementation is aligned with the new resolver.
RegisteringupdatedAtResolveris clearly defined. Everything looks good here.test/graphql/types/FundCampaign/updatedAt.test.ts (18)
1-7: Test setup imports are clear.
Using Vitest with the relevant utilities ensures a consistent testing approach.
8-20: Mock data is well structured.
ThemockFundCampaignobject covers necessary fields. Straightforward approach.
22-40: Context factory is comprehensive.
Providing mocked dependencies underdrizzleClientmakes tests more readable.
42-47: Organized test suite declaration.
ThedescribeandbeforeEachblocks are properly scoped for test state setup.
49-54: Unauthenticated user test is valid.
Verifying rejection whenctx.currentClient.isAuthenticatedis false is essential coverage.
56-63: User existence check handled correctly.
Ensuring it throws an “unauthenticated” error if the user doesn’t exist is consistent with the resolver logic.
65-76: Missing fund scenario is well tested.
The “unexpected” error path is transparent and logs appropriately.
78-96: Role check coverage for non-admin user.
Correctly enforces “unauthorized_action” when user has no admin privileges.
98-115: Admin user path is verified.
Ensures we retrieveupdatedAtsuccessfully for administrators. Nicely done.
117-134: Organization admin path is verified.
Confirms that membership with an “administrator” role grants access. Thorough approach.
136-144: Database connection error scenario.
Tests resiliency by expecting an “unexpected” error when DB connections fail. Great coverage.
146-154: Database timeout error scenario.
Handling “Query timeout” properly ensures robust error responses.
156-164: Constraint violation scenario covered.
Verifying an “unexpected” error for foreign key issues solidifies DB error handling.
166-174: SQL syntax error scenario.
This test ensures syntactical DB issues are logged and wrapped as “unexpected.” Good approach.
176-185: Concurrent updates test.
Simulating “Database lock timeout” highlights proper concurrency control in error handling.
187-198: Database error during concurrent access.
Confirms that relevant error paths are exercised with appropriate logs.
200-212: TalawaGraphQLError pass-through.
Ensuring custom errors throw without re-wrapping helps maintain error context.
214-232: Missing organization membership scenario.
Validates that membership absence triggers “unauthorized_action.” Thorough coverage.
shivasankaran18
force-pushed
the
issue-3063
branch
from
49b826e to
c329902
Compare
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)
src/graphql/types/FundCampaign/updatedAt.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3205
File: src/graphql/types/FundCampaign/updatedAt.ts:21-50
Timestamp: 2025-02-13T16:36:41.119Z
Learning: In the FundCampaign's updatedAt resolver, the fund query intentionally uses currentUserId in the where clause instead of parent.fundId.
src/graphql/types/FundCampaign/updatedAt.ts (1)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3205
File: src/graphql/types/FundCampaign/updatedAt.ts:21-50
Timestamp: 2025-02-13T16:36:41.119Z
Learning: In the FundCampaign's updatedAt resolver, the fund query intentionally uses currentUserId in the where clause instead of parent.fundId.
⏰ 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 (6)
src/graphql/types/FundCampaign/updatedAt.ts (6)
5-17: Good job providing detailed JSDoc for clarity
This doc block is thorough, describing inputs, outputs, and error scenarios. It improves maintainability.
19-31: Solid function signature and initial authentication check
The overall structure of the function is clear. Checking forctx.currentClient.isAuthenticatedearly ensures that unauthenticated requests are swiftly rejected.
36-63: Concurrency and usage ofcurrentUserId
UsingPromise.allfor parallel queries is efficient. We also note that you're intentionally usingcurrentUserIdwithinfundsTable.findFirstbased on your stored learnings. This approach is recognized and no change is needed.
66-72: Robust check for missing currentUser
Verifying the existence ofcurrentUserin the database, even after the authentication check, is prudent. This ensures that stale or invalid user references do not disrupt the workflow.
101-114: Graceful error handling
Catching and rethrowingTalawaGraphQLErrorwhile treating all other errors as "unexpected" is a clean approach. This neatly separates domain errors from system or unhandled errors.
116-120: Integration with GraphQL type
BindingupdatedAtResolverto theupdatedAtfield inFundCampaign.implementis consistent and ensures a clean separation between schema definition and resolver logic.
|
@coderabbitai full review and approve the PR |
|
@shivasankaran18 I'll perform a full review of your changes and approve them as the implementation looks solid with comprehensive test coverage. (｀･ω･´)ゞ ✅ Actions performedComments resolved and changes approved. Full review triggered. |
|
@palisadoes done with coderabbit suggestions.. reverted some as it caused errors..could you review my PR now? |
palisadoes
merged commit 4ade5dc
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Tests for src/graphql/types/FundCampaign/updatedAt.ts
Issue Number: 2922
Fixes : #3063
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Added test for updateAt.ts
Does this PR introduce a breaking change?
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Summary by CodeRabbit
Refactor
updatedAtfield for Fund Campaigns, improving error handling and ensuring that only authorized users can access this information.Tests
updatedAtResolver, validating authentication, authorization, and various error handling scenarios.