migrated test/rotes/graphql/Mutation/updateChatMessage to integration test #3344
Conversation
|
Warning
|
| File(s) | Change Summary |
|---|---|
schema.graphql, src/graphql/inputs/MutationCreateChatMessageInput.ts, test/routes/graphql/gql.tada.d.ts |
Changed the parentMessageId field from non-nullable to nullable and updated the required attribute accordingly in the MutationCreateChatMessageInput type. |
src/graphql/types/Mutation/updateChatMessage.ts |
Refactored the update chat message logic by removing the standalone resolver and integrating it directly into a mutation field using builder.mutationField, while preserving authentication, validation, and error handling. |
test/routes/graphql/Mutation/updateChatMessage.test.ts |
Rewrote the test suite for the updateChatMessage mutation to include comprehensive tests for authentication, authorization, invalid arguments, non-existing messages, unexpected errors, and successful updates. |
test/routes/graphql/documentNodes.ts, test/routes/graphql/gql.tada-cache.d.ts |
Added new GraphQL mutation definitions related to chat operations, including creating chats, chat messages, updating messages, and creating chat memberships. |
Sequence Diagram(s)
sequenceDiagram
participant C as Client
participant G as GraphQL Server
participant V as Validator
participant DB as Database
C->>G: Execute updateChatMessage mutation with input
G->>G: Verify authentication (isAuthenticated)
G->>V: Validate input arguments
V-->>G: Return validated input / error
G->>DB: Fetch current user and chat message
DB-->>G: Provide user and message data
G->>G: Check authorization for update
G->>DB: Update chat message details
DB-->>G: Return updated message
G-->>C: Return updated message or error
Possibly related PRs
- Added mutations for directChat and groupChat and fix subscriptions #2407: Related changes to input types, specifically making fields optional, similar to the modification of the
parentMessageId. - Update chat message test #3174: Involves updates to the
updateChatMessagemutation, affecting the handling of chat message relationships, aligning with the current changes.
Suggested reviewers
- palisadoes
- disha1202
🐰 In the chat where messages flow,
The parent ID can now let go.
With updates made and tests anew,
Our chats will thrive, and friendships too!
Hooray for changes, let’s all cheer,
For clearer paths, we hold so dear! 🌟
✨ Finishing Touches
- 📝 Generate Docstrings
🪧 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
@coderabbitaiin 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
@coderabbitaiin 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 pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate docstringsto generate docstrings for this PR.@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere 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.
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
|
GlenDsza
added
the
ignore-sensitive-files-pr
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3344 +/- ##
====================================================
+ Coverage 50.58% 51.10% +0.52%
====================================================
Files 466 466
Lines 35030 35025 -5
Branches 1101 1136 +35
====================================================
+ Hits 17720 17900 +180
+ Misses 17308 17123 -185
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Please make a minor commit. The sensitive file check should then pass |
I think it's passing now all checks succeeded after re run |
|
@disha1202 @meetulr @GlenDsza PTAL |
Piyushk8
force-pushed
the
updateChatMessage
branch
from
bc9580d to
28cb9a2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
test/routes/graphql/documentNodes.ts (1)
725-737: Include role inMutation_createChatMembershipreturn data.Since roles are often important for verifying membership level, returning the
rolealongsideidand timestamps may streamline client usage.Example diff adding
role:createChatMembership(input: $input) { id createdAt creator { id name } updatedAt + role }test/routes/graphql/Mutation/updateChatMessage.test.ts (3)
21-136: Consolidate repeated test setup logic for unauthenticated scenarios.This block thoroughly checks unauthenticated states. However, the steps for signing in as an administrator, creating users, and executing mutations appear multiple times. Consider extracting them into shared test utilities to reduce duplication and improve maintainability.
304-346: Extend validation to additional missing resources, if needed.This test ensures an error is raised for a non-existing message. If the code also handles other missing resources (e.g., missing chat), consider adding parallel checks to maximize coverage.
517-646: Break down overly large success-path test for clarity.The final success test is comprehensive but somewhat lengthy, making it harder to maintain. Consider factoring out repeated steps into helper functions or test fixtures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
schema.graphql(1 hunks)src/graphql/inputs/MutationCreateChatMessageInput.ts(1 hunks)src/graphql/types/Mutation/updateChatMessage.ts(1 hunks)test/routes/graphql/Mutation/updateChatMessage.test.ts(1 hunks)test/routes/graphql/documentNodes.ts(1 hunks)test/routes/graphql/gql.tada-cache.d.ts(1 hunks)test/routes/graphql/gql.tada.d.ts(1 hunks)test/routes/graphql/documentNodes.ts(1 hunks)test/routes/graphql/gql.tada-cache.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/graphql/inputs/MutationCreateChatMessageInput.ts
- schema.graphql
- test/routes/graphql/gql.tada.d.ts
- src/graphql/types/Mutation/updateChatMessage.ts
🧰 Additional context used
🧠 Learnings (2)
test/routes/graphql/Mutation/updateChatMessage.test.ts (2)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3174
File: test/routes/graphql/Mutation/updateChatMessage.test.ts:6-16
Timestamp: 2025-02-10T13:05:53.364Z
Learning: For test files in development environment, it's acceptable to define simplified interfaces locally instead of importing production types, as it's more pragmatic and includes only the fields needed for testing.
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3174
File: src/graphql/types/Mutation/updateChatMessage.ts:124-141
Timestamp: 2025-02-10T10:03:45.272Z
Learning: When reviewing test-focused PRs in talawa-api, suggestions should be limited to test implementation aspects unless the changes directly impact test coverage. Modifications to existing business logic should be handled in separate PRs to maintain clear scope boundaries.
test/routes/graphql/gql.tada-cache.d.ts (1)
Learnt from: yugal07
PR: PalisadoesFoundation/talawa-api#3218
File: test/routes/graphql/gql.tada-cache.d.ts:61-62
Timestamp: 2025-02-18T06:09:11.711Z
Learning: The file `test/routes/graphql/gql.tada-cache.d.ts` is auto-generated from GraphQL schema/queries and should not be modified directly.
⏰ 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)
test/routes/graphql/Mutation/updateChatMessage.test.ts (4)
1-20: No issues identified with imports and initial setup.These import statements and helper references are consistent and clear.
140-301: Authorization check is well-covered.The tests accurately validate the
unauthorized_action_on_arguments_associated_resourcescode path when the user lacks necessary membership. The approach is solid, and the expectations are correct.
348-386: Robust invalid argument handling.Using an empty message body to trigger an
invalid_argumentserror is a good approach. The expectations align well with typical GraphQL validation errors.
388-515: Good use of mocking to simulate an unexpected DB state.This test cleverly mocks the database update to return no rows, capturing the
unexpectederror scenario. The approach is effective for verifying fallback error handling.test/routes/graphql/gql.tada-cache.d.ts (2)
81-82: Confirm addition offileUrlinMutation_createPresignedUrl.Adding
fileUrlto the presigned URL response can be helpful, but confirm it exists in the actual schema. This file is auto-generated, so any discrepancies should be corrected at the schema level.
83-94: Validate newly added chat-related definitions.The new Chat queries and mutations (
CreateChat,CreateChatMessage,UpdateChatMessage,CreateChatMembership, etc.) appear consistent with the rest of the schema. However, remember that these definitions will be overwritten ifgql.tadais regenerated, so ensure the backing schema reflects these changes permanently.
What kind of change does this PR introduce?
Migrated updateChatMessage test from unit (mock) to integration (real schema) with 100% coverage.
Fixed
parentMessageIdinMutattonCreateChatMessageInputin filesrc/graphql/input/MutattonCreateChatMessageInputfrom required (true) to required (false) for replies/threads.Made changes the schema for the same by
pnpm generate_graphql_sdl_fileandpnpm generate_gql_tadaIssue Number:
Fixes #3274
Snapshots/Videos:
Summary
Migrated test to integration for real schema testing of mutations/queries.
Fixed
parentMessageIdinMutattonCreateChatMessageInputin filesrc/graphql/input/MutattonCreateChatMessageInputfrom required (true) to required (false) for replies/threads.due to unexpected behavior on creating a new message as a standalone message required parentMessageID which needs to undefined but was required true.
Does this PR introduce a breaking change?
No.
Checklist
CodeRabbit AI Review
Test Coverage
Summary by CodeRabbit
updateChatMessagemutation, ensuring various scenarios are properly handled.