-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Test to src/graphql/types/Mutation/deleteAgendaItem.ts #3256
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 to src/graphql/types/Mutation/deleteAgendaItem.ts #3256
Conversation
WalkthroughThe changes standardize the naming convention for the delete agenda item GraphQL input schema by renaming the exported variable from Changes
Possibly related PRs
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
|
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: 2
🔭 Outside diff range comments (1)
src/graphql/types/Mutation/deleteAgendaItem.ts (1)
1-11
:⚠️ Potential issueSort import statements.
The pipeline indicates that import statements need to be sorted.
Apply this diff to sort the imports:
+import { eq } from "drizzle-orm"; +import { z } from "zod"; +import type { GraphQLContext } from "~/src/graphql/context"; +import { agendaItemsTable } from "~/src/drizzle/tables/agendaItems"; +import { builder } from "~/src/graphql/builder"; +import { + MutationDeleteAgendaItemInput, + MutationDeleteAgendaItemInputSchema, +} from "~/src/graphql/inputs/MutationDeleteAgendaItemInput"; +import { AgendaItem } from "~/src/graphql/types/AgendaItem/AgendaItem"; +import { TalawaGraphQLError } from "~/src/utilities/TalawaGraphQLError";🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 3-11: Import statements could be sorted.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/graphql/inputs/MutationDeleteAgendaItemInput.ts
(1 hunks)src/graphql/types/Mutation/deleteAgendaItem.ts
(1 hunks)test/graphql/types/Mutation/deleteAgendaItem.test.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
test/graphql/types/Mutation/deleteAgendaItem.test.ts (5)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:33-49
Timestamp: 2025-02-10T18:57:27.311Z
Learning: In test/graphql/types/Mutation/updateAgendaItem.test.ts, while a global mock setup is used for the drizzle client, an alternative approach is to use specialized mocks initialized in beforeEach for each test scenario to reduce global state and ensure test isolation. However, this requires careful type handling to maintain type safety.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:62-79
Timestamp: 2025-02-10T18:48:15.739Z
Learning: When mocking GraphQLContext in tests, keep the minio property with minimal implementation to satisfy TypeScript types, even if minio operations aren't used in the specific resolver being tested.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:1-2
Timestamp: 2025-02-09T04:24:57.021Z
Learning: In test files for Talawa API, create a TestContext interface that extends GraphQLContext but overrides specific fields needed for mocking. Use the `satisfies` operator to ensure type safety of the mock implementations.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:46-46
Timestamp: 2025-02-10T17:58:51.912Z
Learning: When mocking objects in TypeScript tests, prefer explicit type annotations (e.g., `const mock: ExpectedType = {...}`) over type assertions with `as unknown as` to maintain type safety and catch potential type mismatches at compile time.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:8-24
Timestamp: 2025-02-10T18:40:01.230Z
Learning: In test files for Talawa API, mock interfaces should include explicit return type definitions that match the expected database schema to ensure type safety and catch potential issues during testing.
src/graphql/types/Mutation/deleteAgendaItem.ts (1)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: src/graphql/types/Mutation/updateAgendaItem.ts:46-84
Timestamp: 2025-02-08T11:15:47.878Z
Learning: When creating GraphQL mutation resolvers in Talawa API, ensure the resolver function's type signature matches the GraphQL schema types exactly, especially for nullable fields. The context parameter should be typed as `GraphQLContext` imported from "~/src/graphql/context".
🪛 GitHub Actions: Pull request workflow
test/graphql/types/Mutation/deleteAgendaItem.test.ts
[error] 6-41: Formatter would have printed the following content: Incorrect formatting in MockDrizzleClient interface.
src/graphql/types/Mutation/deleteAgendaItem.ts
[error] 3-11: Import statements could be sorted.
[error] 17-25: Formatter would have printed the following content: Incorrect formatting in function deleteAgendaItemResolver.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (3)
src/graphql/inputs/MutationDeleteAgendaItemInput.ts (1)
5-11
: LGTM! Improved naming convention.The renaming to PascalCase for the exported schema aligns with TypeScript naming conventions for types and schemas.
src/graphql/types/Mutation/deleteAgendaItem.ts (1)
158-171
: LGTM! Well-structured mutation field registration.The mutation field registration is clear and properly typed.
test/graphql/types/Mutation/deleteAgendaItem.test.ts (1)
84-366
: LGTM! Comprehensive test coverage.The test suite thoroughly covers various scenarios:
- Authentication checks
- Input validation
- Resource existence validation
- Authorization checks
- Successful deletion
- Error handling
The test implementation follows best practices from retrieved learnings:
- Uses TestContext interface
- Properly mocks GraphQLContext
- Maintains test isolation with beforeEach
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 #3256 +/- ##
====================================================
+ Coverage 45.48% 45.83% +0.35%
====================================================
Files 455 455
Lines 33935 33940 +5
Branches 757 770 +13
====================================================
+ Hits 15435 15558 +123
+ Misses 18496 18378 -118
Partials 4 4 ☔ View full report in Codecov by Sentry. |
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: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/graphql/types/Mutation/deleteAgendaItem.ts
(1 hunks)test/graphql/types/Mutation/deleteAgendaItem.test.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
test/graphql/types/Mutation/deleteAgendaItem.test.ts (5)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:33-49
Timestamp: 2025-02-10T18:57:27.311Z
Learning: In test/graphql/types/Mutation/updateAgendaItem.test.ts, while a global mock setup is used for the drizzle client, an alternative approach is to use specialized mocks initialized in beforeEach for each test scenario to reduce global state and ensure test isolation. However, this requires careful type handling to maintain type safety.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:62-79
Timestamp: 2025-02-10T18:48:15.739Z
Learning: When mocking GraphQLContext in tests, keep the minio property with minimal implementation to satisfy TypeScript types, even if minio operations aren't used in the specific resolver being tested.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:1-2
Timestamp: 2025-02-09T04:24:57.021Z
Learning: In test files for Talawa API, create a TestContext interface that extends GraphQLContext but overrides specific fields needed for mocking. Use the `satisfies` operator to ensure type safety of the mock implementations.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:8-24
Timestamp: 2025-02-10T18:40:01.230Z
Learning: In test files for Talawa API, mock interfaces should include explicit return type definitions that match the expected database schema to ensure type safety and catch potential issues during testing.
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: test/graphql/types/Mutation/updateAgendaItem.test.ts:46-46
Timestamp: 2025-02-10T17:58:51.912Z
Learning: When mocking objects in TypeScript tests, prefer explicit type annotations (e.g., `const mock: ExpectedType = {...}`) over type assertions with `as unknown as` to maintain type safety and catch potential type mismatches at compile time.
src/graphql/types/Mutation/deleteAgendaItem.ts (1)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3175
File: src/graphql/types/Mutation/updateAgendaItem.ts:46-84
Timestamp: 2025-02-08T11:15:47.878Z
Learning: When creating GraphQL mutation resolvers in Talawa API, ensure the resolver function's type signature matches the GraphQL schema types exactly, especially for nullable fields. The context parameter should be typed as `GraphQLContext` imported from "~/src/graphql/context".
⏰ 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 (15)
src/graphql/types/Mutation/deleteAgendaItem.ts (9)
5-14
: Imports and schema usage look correct.Changing the input schema reference to
MutationDeleteAgendaItemInputSchema
and importingGraphQLContext
are consistent with the GraphQL-based approach.
17-32
: Function signature aligns well with GraphQL context and usage.The
deleteAgendaItemResolver
signature matches the GraphQL schema and context type consistently. The unauthenticated check is clear and straightforward.
34-50
: Zod parsing is well-implemented.Using
z.object(...).safeParse(args)
and providing structured error details is a robust way to handle invalid inputs.
52-98
: Parallel queries for user and agenda item are efficient.Executing both queries with
Promise.all
is a good performance choice, minimizing round-trip times to the database. The error handling patterns remain consistent.
100-106
: Clear handling of missing user scenario.Throwing a
TalawaGraphQLError
for an undefined user is consistent with the existing error-handling pattern.
108-119
: Proper error response when agenda item is not found.The code thoroughly covers a missing agenda item, providing a clear error code and relevant details.
142-154
: Deletion logic and checks are well-handled.The Drizzle ORM-based deletion, combined with a check of the return value, ensures reliable detection of unexpected cases.
156-157
: Clear and consistent return of the deleted agenda item.Returning the item provides useful context for the client after a successful deletion.
159-172
: Mutation field registration follows best practices.Defining the
deleteAgendaItem
field in the builder with explicit args and resolver cleanly ties the schema to the resolver.test/graphql/types/Mutation/deleteAgendaItem.test.ts (6)
1-7
: Imports are correctly organized.The test relies on Vitest, mock utilities, and relevant GraphQL resolver references, aligning with Talawa's testing conventions.
70-99
: Unauthenticated user test covers the essential error-throw scenario.The check for
extensions.code
and the thrownunauthenticated
error reflects desired security constraints.
100-114
: Invalid UUID test is accurate.Validating the UUID format ensures early detection of incorrect arguments.
116-262
: Comprehensive coverage of error conditions.These tests systematically handle missing users, non-existent agenda items, and insufficient roles, demonstrating robust coverage of edge cases.
264-307
: Successful deletion test is thorough.Verifying the database mock return value and ensuring the resolver response match fosters confidence in the happy path outcome.
309-355
: Failure scenario test ensures complete coverage.Testing when the deletion operation returns an empty array helps confirm that unexpected outcomes are handled gracefully.
dee12a8
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Issue Number:
Fixes #3010
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Summary by CodeRabbit