-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Added queries and mutations for action items #3458
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
base: develop-postgres
Are you sure you want to change the base?
Added queries and mutations for action items #3458
Conversation
Signed-off-by: NishantSinghhhhh <[email protected]>
WalkthroughThis change is a comprehensive refactor and enhancement of the action item and action item category features across the database schema, GraphQL API, and related server code. It renames and restructures tables and types, replaces ID references with direct object references in GraphQL types, adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GraphQLServer
participant DB
Client->>GraphQLServer: createActionItemCategory(input)
GraphQLServer->>DB: Validate user & org, check admin, insert category
DB-->>GraphQLServer: New ActionItemCategory
GraphQLServer-->>Client: ActionItemCategory object (with org & creator)
Client->>GraphQLServer: actionItemsByOrganization(organizationId, pagination)
GraphQLServer->>DB: Validate user, check admin/member, fetch paginated items
DB-->>GraphQLServer: ActionItem list (with nested relations)
GraphQLServer-->>Client: Paginated ActionItemConnection (edges, nodes, pageInfo)
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
Signed-off-by: NishantSinghhhhh <[email protected]>
…hhhh/talawa-api into actionitems-nomongo
Signed-off-by: NishantSinghhhhh <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3458 +/- ##
====================================================
- Coverage 59.60% 59.35% -0.26%
====================================================
Files 523 537 +14
Lines 40782 41627 +845
Branches 1786 1830 +44
====================================================
+ Hits 24307 24706 +399
- Misses 16464 16910 +446
Partials 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 50
🔭 Outside diff range comments (5)
src/graphql/types/Query/index.ts (2)
1-1
:⚠️ Potential issuePipeline blocked due to unauthorized import adjustments.
CI has flagged unauthorized changes in this sensitive file. Please apply the
ignore-sensitive-files-pr
label to override, or confirm with maintainers to approve these import updates.🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
10-10
: 🛠️ Refactor suggestionRemove deprecated
event
query import.The
event
query import remains even though event-related GraphQL queries have been removed. This orphan import may register an unused resolver and cause confusion or runtime errors. Remove the line:-import "./event";
src/graphql/types/ActionItem/categoryId.ts.ts (1)
1-55
:⚠️ Potential issueFile has incorrect double .ts.ts extension.
The filename has a duplicated
.ts
extension, which should be corrected to justcategoryId.ts
.Rename the file to use the correct extension:
git mv src/graphql/types/ActionItem/categoryId.ts.ts src/graphql/types/ActionItem/categoryId.ts🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 20-20: src/graphql/types/ActionItem/categoryId.ts.ts#L20
Added line #L20 was not covered by testssrc/graphql/types/Mutation/updateActionItem.ts (1)
190-196
: 🧹 Nitpick (assertive)Minor: avoid accidental field leaks with object spread
fieldsToUpdate
may still contain properties you filtered out of the GraphQL input later (e.g., unknown keys if validation schema drifts). Consider a whitelist instead of spread to reduce attack surface.src/drizzle/tables/actionItems.ts (1)
137-139
: 🧹 Nitpick (assertive)Rename insert schema to reflect the new table name
actionsTableInsertSchema
still references the old table. This is only cosmetic but can confuse future maintainers and code‑gen.-export const actionsTableInsertSchema = createInsertSchema(actionItemsTable); +export const actionItemsTableInsertSchema = + createInsertSchema(actionItemsTable);
♻️ Duplicate comments (1)
schema.graphql (1)
2459-2461
: Duplicate:allottedHours
type mismatch inMutationUpdateActionItemInput
See the earlier comment on lines 10–11 regarding the mismatch betweenFloat
in the schema andInt
in the input.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (61)
drizzle_migrations/20250422182535_moaning_pandemic.sql
(1 hunks)drizzle_migrations/meta/_journal.json
(1 hunks)schema.graphql
(9 hunks)scripts/dbManagement/helpers.ts
(3 hunks)scripts/dbManagement/sample_data/action_items.json
(165 hunks)src/drizzle/schema.ts
(1 hunks)src/drizzle/tables/actionItemCategories.ts
(2 hunks)src/drizzle/tables/actionItems.ts
(2 hunks)src/drizzle/tables/organizations.ts
(2 hunks)src/drizzle/tables/users.ts
(2 hunks)src/graphql/inputs/MutationCreateActionItem.ts
(1 hunks)src/graphql/inputs/MutationCreateActionItemCategory.ts
(1 hunks)src/graphql/inputs/MutationDeleteActionItemInput.ts
(2 hunks)src/graphql/inputs/MutationUpdateActionItemCategory.ts
(1 hunks)src/graphql/inputs/MutationUpdateActionItemInput.ts
(3 hunks)src/graphql/inputs/QueryActionCategoriesByOrganizationInput.ts
(1 hunks)src/graphql/inputs/QueryActionItemInput.ts
(2 hunks)src/graphql/types/ActionItem/actionItem.ts
(3 hunks)src/graphql/types/ActionItem/assignee.ts
(2 hunks)src/graphql/types/ActionItem/categoryId.ts.ts
(1 hunks)src/graphql/types/ActionItem/createdAt.ts
(1 hunks)src/graphql/types/ActionItem/creator.ts
(1 hunks)src/graphql/types/ActionItem/event.ts
(1 hunks)src/graphql/types/ActionItem/index.ts
(1 hunks)src/graphql/types/ActionItem/organization.ts
(1 hunks)src/graphql/types/ActionItem/updatedAt.ts
(1 hunks)src/graphql/types/ActionItem/updater.ts
(1 hunks)src/graphql/types/ActionItemCategory/actionItemCategory.ts
(2 hunks)src/graphql/types/ActionItemCategory/createdAt.ts
(1 hunks)src/graphql/types/ActionItemCategory/creator.ts
(1 hunks)src/graphql/types/ActionItemCategory/index.ts
(1 hunks)src/graphql/types/ActionItemCategory/organization.ts
(1 hunks)src/graphql/types/ActionItemCategory/updatedAt.ts
(1 hunks)src/graphql/types/Mutation/createActionItem.ts
(1 hunks)src/graphql/types/Mutation/createActionItemCategory.ts
(1 hunks)src/graphql/types/Mutation/deleteActionItem.ts
(3 hunks)src/graphql/types/Mutation/index.ts
(1 hunks)src/graphql/types/Mutation/updateActionItem.ts
(6 hunks)src/graphql/types/Mutation/updateActionItemCategory.ts
(1 hunks)src/graphql/types/Query/ActionCategorybyOrg.ts
(4 hunks)src/graphql/types/Query/ActionItem.ts
(2 hunks)src/graphql/types/Query/index.ts
(1 hunks)src/graphql/types/Query/organizationUsers.ts
(0 hunks)test/graphql/types/ActionItem/assignee.test.ts
(0 hunks)test/graphql/types/ActionItem/categoryId.test.ts
(1 hunks)test/graphql/types/ActionItem/createdAt.test.ts
(1 hunks)test/graphql/types/ActionItem/creator.test.ts
(1 hunks)test/graphql/types/ActionItem/event.test.ts
(1 hunks)test/graphql/types/ActionItem/organization.test.ts
(1 hunks)test/graphql/types/ActionItemCategory/createdAt.test.ts
(1 hunks)test/graphql/types/ActionItemCategory/creator.test.ts
(1 hunks)test/graphql/types/ActionItemCategory/organization.test.ts
(1 hunks)test/graphql/types/Mutation/createActionItemCategory.test.ts
(1 hunks)test/graphql/types/Query/ActionCategory.test.ts
(0 hunks)test/graphql/types/Query/ActionItem.test.ts
(0 hunks)test/graphql/types/Query/Events.test.ts
(0 hunks)test/graphql/types/Query/organizationUsers.test.ts
(0 hunks)test/graphql/types/documentNodes.ts
(3 hunks)test/graphql/types/gql.tada-cache.d.ts
(3 hunks)test/graphql/types/gql.tada.d.ts
(5 hunks)test/scripts/dbManagement/helpers.test.ts
(4 hunks)
💤 Files with no reviewable changes (6)
- test/graphql/types/ActionItem/assignee.test.ts
- test/graphql/types/Query/ActionItem.test.ts
- test/graphql/types/Query/organizationUsers.test.ts
- src/graphql/types/Query/organizationUsers.ts
- test/graphql/types/Query/Events.test.ts
- test/graphql/types/Query/ActionCategory.test.ts
🧰 Additional context used
🧠 Learnings (4)
test/graphql/types/ActionItem/createdAt.test.ts (1)
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.
test/graphql/types/ActionItemCategory/organization.test.ts (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: tests/resolvers/Query/actionItemsByOrganization.spec.ts:51-82
Timestamp: 2024-11-02T06:36:31.866Z
Learning: When reviewing `tests/resolvers/Query/actionItemsByOrganization.spec.ts`, if the existing test cases already provide 100% coverage, avoid suggesting additional test cases.
test/graphql/types/ActionItem/organization.test.ts (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: tests/resolvers/Query/actionItemsByOrganization.spec.ts:51-82
Timestamp: 2024-11-02T06:36:31.866Z
Learning: When reviewing `tests/resolvers/Query/actionItemsByOrganization.spec.ts`, if the existing test cases already provide 100% coverage, avoid suggesting additional test cases.
src/graphql/types/ActionItem/event.ts (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: src/resolvers/Event/actionItems.ts:18-18
Timestamp: 2024-10-27T14:33:40.803Z
Learning: In `src/resolvers/Event/actionItems.ts`, the `event` field is required and should be used instead of `eventId` as per the schema.
🧬 Code Graph Analysis (22)
src/drizzle/tables/organizations.ts (2)
src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(18-102)src/drizzle/tables/actionItemCategories.ts (1)
actionItemCategoriesTable
(16-66)
src/graphql/types/Mutation/deleteActionItem.ts (1)
src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(18-102)
src/drizzle/tables/users.ts (3)
src/drizzle/schema.ts (2)
actionItemsTable
(8-8)actionItemCategoriesTable
(5-5)src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(18-102)src/drizzle/tables/actionItemCategories.ts (1)
actionItemCategoriesTable
(16-66)
src/graphql/types/ActionItemCategory/updatedAt.ts (2)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)
src/graphql/inputs/QueryActionCategoriesByOrganizationInput.ts (1)
src/graphql/builder.ts (1)
builder
(10-20)
src/drizzle/tables/actionItemCategories.ts (3)
src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(18-102)src/drizzle/tables/users.ts (1)
usersTable
(59-216)src/drizzle/tables/organizations.ts (1)
organizationsTable
(31-128)
src/graphql/inputs/MutationCreateActionItemCategory.ts (1)
src/graphql/builder.ts (1)
builder
(10-20)
src/graphql/inputs/MutationUpdateActionItemCategory.ts (1)
src/graphql/builder.ts (1)
builder
(10-20)
src/graphql/types/ActionItem/categoryId.ts.ts (4)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/graphql/types/ActionItemCategory/actionItemCategory.ts (2)
ActionItemCategory
(6-8)ActionItemCategory
(10-11)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/graphql/types/ActionItem/actionItem.ts (2)
ActionItem
(4-4)ActionItem
(6-6)
src/graphql/types/Query/ActionCategorybyOrg.ts (2)
src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/graphql/inputs/QueryActionCategoriesByOrganizationInput.ts (1)
queryActionCategoriesByOrganizationArgumentsSchema
(37-39)
src/graphql/inputs/MutationUpdateActionItemInput.ts (1)
src/drizzle/tables/actionItems.ts (1)
actionsTableInsertSchema
(138-138)
test/graphql/types/ActionItem/categoryId.test.ts (3)
src/graphql/context.ts (1)
GraphQLContext
(55-55)test/_Mocks_/drizzleClientMock.ts (1)
createMockDrizzleClient
(31-51)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)
src/graphql/types/ActionItem/organization.ts (3)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/graphql/types/ActionItem/actionItem.ts (2)
ActionItem
(4-4)ActionItem
(6-6)
test/graphql/types/ActionItemCategory/organization.test.ts (4)
src/graphql/context.ts (1)
GraphQLContext
(55-55)test/_Mocks_/drizzleClientMock.ts (1)
createMockDrizzleClient
(31-51)src/graphql/types/ActionItemCategory/organization.ts (1)
resolveCategoryOrganization
(11-43)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)
src/drizzle/tables/actionItems.ts (3)
src/drizzle/tables/users.ts (1)
usersTable
(59-216)src/drizzle/tables/actionItemCategories.ts (1)
actionItemCategoriesTable
(16-66)src/drizzle/tables/organizations.ts (1)
organizationsTable
(31-128)
test/graphql/types/ActionItem/organization.test.ts (4)
src/graphql/context.ts (1)
GraphQLContext
(55-55)test/_Mocks_/drizzleClientMock.ts (1)
createMockDrizzleClient
(31-51)src/graphql/types/ActionItem/organization.ts (1)
resolveOrganization
(6-38)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)
src/graphql/types/Mutation/createActionItemCategory.ts (5)
src/graphql/builder.ts (1)
builder
(10-20)src/graphql/types/ActionItemCategory/actionItemCategory.ts (2)
ActionItemCategory
(6-8)ActionItemCategory
(10-11)src/graphql/inputs/MutationCreateActionItemCategory.ts (2)
MutationCreateActionItemCategoryInput
(17-42)mutationCreateActionItemCategoryArgumentsSchema
(47-49)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/drizzle/tables/actionItemCategories.ts (1)
actionItemCategoriesTable
(16-66)
src/graphql/inputs/QueryActionItemInput.ts (1)
src/drizzle/tables/actionItems.ts (1)
actionsTableInsertSchema
(138-138)
src/graphql/types/ActionItem/event.ts (3)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/graphql/types/ActionItem/actionItem.ts (2)
ActionItem
(4-4)ActionItem
(6-6)
src/graphql/types/ActionItemCategory/creator.ts (3)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/graphql/types/ActionItemCategory/actionItemCategory.ts (2)
ActionItemCategory
(6-8)ActionItemCategory
(10-11)
src/graphql/inputs/MutationDeleteActionItemInput.ts (1)
src/drizzle/tables/actionItems.ts (1)
actionsTableInsertSchema
(138-138)
src/graphql/inputs/MutationCreateActionItem.ts (1)
src/graphql/builder.ts (1)
builder
(10-20)
🪛 GitHub Actions: Pull request workflow
drizzle_migrations/20250422182535_moaning_pandemic.sql
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
drizzle_migrations/meta/_journal.json
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Query/index.ts
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Query/ActionItem.ts
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Mutation/deleteActionItem.ts
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Query/ActionCategorybyOrg.ts
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Mutation/updateActionItem.ts
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Mutation/updateActionItemCategory.ts
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Mutation/index.ts
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Mutation/createActionItemCategory.ts
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Mutation/createActionItem.ts
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
schema.graphql
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
🪛 GitHub Check: codecov/patch
src/graphql/types/ActionItem/assignee.ts
[warning] 22-22: src/graphql/types/ActionItem/assignee.ts#L22
Added line #L22 was not covered by tests
[warning] 25-25: src/graphql/types/ActionItem/assignee.ts#L25
Added line #L25 was not covered by tests
src/graphql/types/ActionItem/categoryId.ts.ts
[warning] 20-20: src/graphql/types/ActionItem/categoryId.ts.ts#L20
Added line #L20 was not covered by tests
src/graphql/types/ActionItem/organization.ts
[warning] 23-23: src/graphql/types/ActionItem/organization.ts#L23
Added line #L23 was not covered by tests
src/graphql/types/ActionItem/updater.ts
[warning] 14-20: src/graphql/types/ActionItem/updater.ts#L14-L20
Added lines #L14 - L20 were not covered by tests
[warning] 22-22: src/graphql/types/ActionItem/updater.ts#L22
Added line #L22 was not covered by tests
[warning] 25-36: src/graphql/types/ActionItem/updater.ts#L25-L36
Added lines #L25 - L36 were not covered by tests
[warning] 39-45: src/graphql/types/ActionItem/updater.ts#L39-L45
Added lines #L39 - L45 were not covered by tests
[warning] 47-48: src/graphql/types/ActionItem/updater.ts#L47-L48
Added lines #L47 - L48 were not covered by tests
[warning] 51-61: src/graphql/types/ActionItem/updater.ts#L51-L61
Added lines #L51 - L61 were not covered by tests
[warning] 64-66: src/graphql/types/ActionItem/updater.ts#L64-L66
Added lines #L64 - L66 were not covered by tests
[warning] 69-71: src/graphql/types/ActionItem/updater.ts#L69-L71
Added lines #L69 - L71 were not covered by tests
[warning] 73-73: src/graphql/types/ActionItem/updater.ts#L73
Added line #L73 was not covered by tests
[warning] 76-80: src/graphql/types/ActionItem/updater.ts#L76-L80
Added lines #L76 - L80 were not covered by tests
[warning] 83-92: src/graphql/types/ActionItem/updater.ts#L83-L92
Added lines #L83 - L92 were not covered by tests
[warning] 94-95: src/graphql/types/ActionItem/updater.ts#L94-L95
Added lines #L94 - L95 were not covered by tests
src/graphql/types/ActionItem/creator.ts
[warning] 26-26: src/graphql/types/ActionItem/creator.ts#L26
Added line #L26 was not covered by tests
[warning] 45-45: src/graphql/types/ActionItem/creator.ts#L45
Added line #L45 was not covered by tests
[warning] 65-67: src/graphql/types/ActionItem/creator.ts#L65-L67
Added lines #L65 - L67 were not covered by tests
src/graphql/types/ActionItem/actionItem.ts
[warning] 43-43: src/graphql/types/ActionItem/actionItem.ts#L43
Added line #L43 was not covered by tests
[warning] 46-49: src/graphql/types/ActionItem/actionItem.ts#L46-L49
Added lines #L46 - L49 were not covered by tests
[warning] 51-53: src/graphql/types/ActionItem/actionItem.ts#L51-L53
Added lines #L51 - L53 were not covered by tests
[warning] 55-56: src/graphql/types/ActionItem/actionItem.ts#L55-L56
Added lines #L55 - L56 were not covered by tests
src/graphql/types/ActionItem/event.ts
[warning] 26-26: src/graphql/types/ActionItem/event.ts#L26
Added line #L26 was not covered by tests
[warning] 52-52: src/graphql/types/ActionItem/event.ts#L52
Added line #L52 was not covered by tests
[warning] 58-58: src/graphql/types/ActionItem/event.ts#L58
Added line #L58 was not covered by tests
src/graphql/types/ActionItemCategory/actionItemCategory.ts
[warning] 53-54: src/graphql/types/ActionItemCategory/actionItemCategory.ts#L53-L54
Added lines #L53 - L54 were not covered by tests
[warning] 57-57: src/graphql/types/ActionItemCategory/actionItemCategory.ts#L57
Added line #L57 was not covered by tests
[warning] 60-67: src/graphql/types/ActionItemCategory/actionItemCategory.ts#L60-L67
Added lines #L60 - L67 were not covered by tests
[warning] 69-70: src/graphql/types/ActionItemCategory/actionItemCategory.ts#L69-L70
Added lines #L69 - L70 were not covered by tests
🔇 Additional comments (136)
src/graphql/types/ActionItem/createdAt.ts (1)
3-3
: Import path corrected and commented code removed.The import of
ActionItem
from"./actionItem"
aligns with the renamed module, and the removal of the outdated comment improves clarity. No functional changes here.scripts/dbManagement/sample_data/action_items.json (3)
5-5
: Field renamed from "actorId" to "assigneeId" to match updated data modelThis change consistently renames the field across all action items in the sample data, aligning with the migration from Express endpoints to GraphQL schema where the relationship models have been updated.
Also applies to: 22-22, 39-39, 56-56, 73-73, 90-90, 107-107, 124-124, 141-141, 158-158, 175-175, 192-192, 209-209, 226-226, 243-243, 260-260, 277-277, 294-294, 311-311, 328-328, 345-345, 362-362, 379-379, 396-396, 413-413, 430-430, 447-447, 464-464, 481-481, 498-498, 515-515, 532-532, 549-549, 566-566, 583-583, 600-600, 617-617, 634-634, 651-651, 668-668, 685-685, 702-702, 719-719, 736-736, 753-753, 770-770, 787-787, 804-804, 821-821, 838-838, 855-855, 872-872, 889-889, 906-906, 923-923, 940-940, 957-957, 974-974, 991-991, 1008-1008, 1025-1025, 1042-1042, 1059-1059, 1076-1076, 1093-1093, 1110-1110, 1127-1127, 1144-1144, 1161-1161, 1178-1178, 1195-1195, 1212-1212, 1229-1229, 1246-1246, 1263-1263, 1280-1280, 1297-1297, 1314-1314, 1331-1331, 1348-1348, 1365-1365, 1382-1382, 1399-1399, 1416-1416, 1433-1433, 1450-1450, 1467-1467, 1484-1484, 1501-1501, 1518-1518, 1535-1535, 1552-1552, 1569-1569, 1586-1586, 1603-1603, 1620-1620, 1637-1637, 1654-1654, 1671-1671, 1688-1688, 1705-1705, 1722-1722, 1739-1739, 1756-1756, 1773-1773, 1790-1790, 1807-1807, 1824-1824, 1841-1841, 1858-1858, 1875-1875, 1892-1892, 1909-1909, 1926-1926, 1943-1943, 1960-1960, 1977-1977, 1994-1994, 2011-2011, 2028-2028, 2045-2045, 2062-2062, 2079-2079, 2096-2096, 2113-2113, 2130-2130, 2147-2147, 2164-2164, 2181-2181, 2198-2198, 2215-2215, 2232-2232, 2249-2249, 2266-2266, 2283-2283, 2300-2300, 2317-2317, 2334-2334, 2351-2351, 2368-2368, 2385-2385, 2402-2402, 2419-2419, 2436-2436, 2453-2453, 2470-2470, 2487-2487, 2504-2504, 2521-2521, 2538-2538, 2555-2555, 2572-2572, 2589-2589, 2606-2606, 2623-2623, 2640-2640, 2657-2657, 2674-2674, 2691-2691, 2708-2708, 2725-2725, 2742-2742, 2759-2759, 2776-2776, 2793-2793
11-11
: Added "allottedHours" field with consistent value of 5.5The new field has been added consistently across all action items with the same value (5.5 hours). This supports the new database column and GraphQL field for tracking estimated time for action items.
Also applies to: 28-28, 45-45, 62-62, 79-79, 96-96, 110-110, 130-130, 147-147, 164-164, 181-181, 198-198, 215-215, 232-232, 249-249, 266-266, 283-283, 300-300, 317-317, 334-334, 351-351, 368-368, 385-385, 402-402, 419-419, 436-436, 453-453, 470-470, 487-487, 504-504, 521-521, 538-538, 555-555, 572-572, 589-589, 606-606, 623-623, 640-640, 657-657, 674-674, 691-691, 708-708, 725-725, 742-742, 759-759, 776-776, 793-793, 810-810, 827-827, 844-844, 861-861, 878-878, 895-895, 912-912, 929-929, 946-946, 963-963, 980-980, 997-997, 1014-1014, 1031-1031, 1048-1048, 1065-1065, 1082-1082, 1099-1099, 1116-1116, 1133-1133, 1150-1150, 1167-1167, 1184-1184, 1201-1201, 1218-1218, 1235-1235, 1252-1252, 1269-1269, 1286-1286, 1303-1303, 1320-1320, 1337-1337, 1354-1354, 1371-1371, 1388-1388, 1405-1405, 1422-1422, 1439-1439, 1456-1456, 1473-1473, 1490-1490, 1507-1507, 1524-1524, 1541-1541, 1558-1558, 1575-1575, 1592-1592, 1609-1609, 1626-1626, 1643-1643, 1660-1660, 1677-1677, 1694-1694, 1711-1711, 1728-1728, 1745-1745, 1762-1762, 1779-1779, 1796-1796, 1813-1813, 1830-1830, 1847-1847, 1864-1864, 1881-1881, 1898-1898, 1915-1915, 1932-1932, 1949-1949, 1966-1966, 1983-1983, 2000-2000, 2017-2017, 2034-2034, 2051-2051, 2068-2068, 2085-2085, 2102-2102, 2119-2119, 2136-2136, 2153-2153, 2170-2170, 2187-2187, 2204-2204, 2221-2221, 2238-2238, 2255-2255, 2272-2272, 2289-2289, 2306-2306, 2323-2323, 2340-2340, 2357-2357, 2374-2374, 2391-2391, 2408-2408, 2425-2425, 2442-2442, 2459-2459, 2476-2476, 2493-2493, 2510-2510, 2527-2527, 2544-2544, 2561-2561, 2578-2578, 2595-2595, 2612-2612, 2629-2629, 2646-2646, 2663-2663, 2680-2680, 2697-2697, 2714-2714, 2731-2731, 2748-2748, 2765-2765, 2782-2782, 2799-2799
1-2807
: Consistent formatting across the entire sample data fileThe sample data has been updated consistently with the new field structure. This approach helps ensure a smooth migration to the new schema and provides good test data for the GraphQL resolvers.
src/graphql/types/Mutation/index.ts (1)
80-81
: New mutation resolvers for action item categories addedThese new imports add support for creating and updating action item categories via GraphQL, which aligns with the PR objective of migrating action item category handling from Express endpoints to GraphQL.
This file has a pipeline failure indicating unauthorized changes. Please apply the 'ignore-sensitive-files-pr' label as mentioned in the error message, as these changes are required for the migration work described in the PR objectives.
src/graphql/types/ActionItemCategory/index.ts (1)
1-5
: Enhanced ActionItemCategory type with field-specific resolversThe imports have been expanded to include dedicated resolvers for timestamp fields (
createdAt
,updatedAt
) and relationship fields (organization
,creator
). This modular approach improves code organization and maintainability.src/graphql/types/ActionItem/index.ts (1)
1-9
: Enhanced ActionItem type with relationship resolversThe imports have been updated to include three new resolvers for relationship fields:
updater
: Resolves the user who last updated the action itemcategoryId.ts
: Resolves the category of the action itemorganization
: Resolves the organization the action item belongs toThese changes support the migration from flat ID fields to object references in the GraphQL schema, providing a more relational data model.
src/drizzle/tables/organizations.ts (3)
14-15
: Import paths updated to reflect renamed tables.The imports have been updated to use the new table names, which is consistent with the migration of action items to a more descriptive naming convention. This change maintains codebase consistency.
136-138
: Relation updated to reference actionItemsTableGood update to the relation definition, referencing the renamed
actionItemsTable
while preserving the existing relation name for database compatibility.
142-144
: Relation updated to reference actionItemCategoriesTableGood update to the relation definition, referencing the renamed
actionItemCategoriesTable
while preserving the existing relation name for database compatibility.src/graphql/types/ActionItem/assignee.ts (1)
3-3
: Import path corrected to match component file naming.The import path for ActionItem has been updated to match the lowercase filename convention.
test/scripts/dbManagement/helpers.test.ts (3)
330-346
: Table references updated to use renamed actionItemCategoriesTable.The test has been updated to use the renamed
actionItemCategoriesTable
instead of the previousactionCategoriesTable
. This maintains consistency with the schema changes.
364-369
: Type definition updated to use actionItemsTable.$inferInsert.The type inference for captured data has been updated to reference the renamed table, ensuring type safety during testing.
383-387
: Test updated to use renamed actionItemsTable.The table reference in the test has been updated to use the renamed
actionItemsTable
, ensuring the tests properly validate the new schema.src/graphql/types/Query/ActionItem.ts (3)
4-7
: Import paths updated to match restructured inputs and types.The imports have been updated to align with the renamed and restructured GraphQL inputs and types. The direct import of
queryActionItemsByOrganizationArgumentsSchema
from QueryActionItemInput and the corrected path for ActionItem are good refactoring changes.
1-161
: Pipeline failure detected for this file.The pipeline indicates this is a sensitive file that requires special approval. Consider applying the 'ignore-sensitive-files-pr' label if these changes are intended as part of the migration.
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
66-66
:✅ Verification successful
Query updated to use renamed actionItemsTable.
The query resolver now correctly uses
actionItemsTable
instead of the old table name, aligning with the schema refactoring.
🏁 Script executed:
#!/bin/bash # Check for any missed references to the old table name in query resolvers rg "actionsTable" --type ts src/graphql/types/Query/Length of output: 52
Ready to merge – no remaining
actionsTable
references found
Verified viarg "actionsTable" --type ts src/graphql/types/Query/
, confirming the query resolver correctly usesactionItemsTable
.src/drizzle/tables/users.ts (3)
22-23
: Import updates align with action item schema refactoring.These import changes correctly reflect the renamed tables from
actionCategoriesTable
andactionsTable
toactionItemCategoriesTable
andactionItemsTable
respectively, aligning with the broader schema refactoring for action items.
222-236
: Table references in actionsWhere relations properly updated.*The relation definitions between users and action items have been correctly updated to reference the renamed
actionItemsTable
. The relation names themselves remain unchanged, maintaining backward compatibility with existing code while adapting to the new table structure.
240-248
: Correct update of actionCategoriesWhere relations.*The actionCategoriesWhereCreator and actionCategoriesWhereUpdater relations now properly reference the renamed
actionItemCategoriesTable
while maintaining the same relation names for compatibility.scripts/dbManagement/helpers.ts (3)
562-568
: Updated type assertions and table references for action categories.The code correctly updates the type assertion and table references from the old
actionCategoriesTable
to the newactionItemCategoriesTable
in theinsertCollections
function. This ensures sample data insertion will work with the refactored schema.
612-618
: Updated type assertions and table references for action items.The code correctly updates the type assertion and table references from the old
actionsTable
to the newactionItemsTable
in theinsertCollections
function, ensuring data insertion compatibility with the refactored schema.
670-672
: Updated table references in checkDataSize function.The table references in the
checkDataSize
function have been correctly updated to point to the renamed tables, ensuring proper record counting and reporting during data import operations.test/graphql/types/ActionItemCategory/creator.test.ts (6)
1-8
: Well-structured test setup with proper imports.The test file properly imports all necessary dependencies including the resolver being tested (
resolveCategoryCreator
), test utilities, and mock objects. The file header and imports are well-organized.
9-33
: Comprehensive test setup with proper context mocking.The test setup creates a properly mocked GraphQL context with authentication state and mock implementations of the database client. The use of
beforeEach
to reset the mocks between tests follows testing best practices for isolation.
35-57
: Proper authentication error tests.These tests correctly verify that the resolver throws authentication errors when the user is not signed in or when the current user can't be found in the database, which are important security checks.
59-73
: Authorization check validation.This test verifies that unauthorized users cannot access creator information, ensuring proper permission enforcement for action item categories which aligns with the PR's security goals.
75-104
: Proper null handling and self-reference tests.These tests verify two important cases: returning null when the creatorId is null, and correctly returning the current user when the creatorId is the current user. Both are important edge cases to handle.
106-149
: Comprehensive error handling and creator resolution tests.These tests verify that the resolver can correctly find creators other than the current user and that it handles errors appropriately when a creator can't be found despite having a non-null ID. The error logging verification is particularly good.
test/graphql/types/ActionItem/createdAt.test.ts (3)
1-14
: Well-structured test setup with proper test interface.The test file properly imports all necessary dependencies and defines a clean interface for the test subject. The FakeActionItem interface provides a clear contract for the test data.
45-113
: Comprehensive authentication and authorization tests.The tests cover all important scenarios: unauthenticated users, users not found in the database, users without admin privileges, and users with different types of admin access (global admin vs organization admin). The assertions are clear and specific to each test case.
15-43
: 🧹 Nitpick (assertive)Efficient test context setup with proper mocking.
The test setup creates a properly mocked GraphQL context and database client. The beforeEach hook ensures each test starts with a clean state, following the test isolation principle.
Based on the retrieved learning from shivasankaran18, consider using specialized mocks initialized in beforeEach instead of relying on global mock state to further improve test isolation. Would you like me to suggest a specific implementation for this approach?
⛔ Skipped due to learnings
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: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: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.
src/graphql/types/Mutation/deleteActionItem.ts (3)
2-2
: Updated import paths to align with new naming conventions.The import changes correctly reflect the renamed tables and models. This is consistent with the PR objective of migrating ActionItem Category handling to GraphQL.
Also applies to: 4-4, 7-7
58-64
: Updated table reference to the renamed actionItemsTable.The query now correctly uses
actionItemsTable
instead of the previous name, maintaining consistency with the schema changes.
1-106
: Pipeline failure requires ignore-sensitive-files-pr label.The GitHub Actions pipeline indicates that this file is considered sensitive, and changes require the 'ignore-sensitive-files-pr' label to be applied to the PR.
Please apply the 'ignore-sensitive-files-pr' label to the PR to address the pipeline failure.
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/ActionItemCategory/createdAt.ts (3)
1-9
: Well-documented resolver with clear purpose.The file header comments clearly explain the purpose of this resolver and its authorization requirements.
10-65
: Proper authentication and authorization checks implemented.The resolver correctly implements a multi-layered security approach:
- Verifies user authentication
- Checks if the user is a global administrator
- Alternatively checks if the user has administrator privileges in the specific organization
This follows security best practices by enforcing the principle of least privilege.
67-76
: Implementation of GraphQL field with proper resolver.The field is correctly wired to the ActionItemCategory type with an appropriate description and resolver function.
test/graphql/types/ActionItem/categoryId.test.ts (4)
1-36
: Comprehensive test setup with proper mocking.The test file correctly sets up the testing environment with:
- Appropriate imports
- Mock database client with typed mock functions
- Mock GraphQL context with necessary properties
This provides a solid foundation for testing the resolver functionality.
37-41
: Test case for null categoryId scenario.This test correctly verifies that the resolver returns null when categoryId is null and doesn't attempt unnecessary database queries.
43-65
: Thorough error case testing for missing category.The test comprehensively verifies error handling when a category is not found:
- Tests that the correct error type is thrown
- Verifies the error message and code match expectations
- Ensures proper error logging occurs
This is a well-structured test that covers both the thrown error and side effects.
67-78
: Success path verification for category resolution.The test correctly verifies that when a category exists, it's properly returned by the resolver without any error logging.
test/graphql/types/ActionItemCategory/organization.test.ts (4)
1-36
: Well-structured test setup with typed mocks.The test setup includes:
- A properly defined fake category interface
- Typed mock functions for database queries
- A mock GraphQL context with necessary properties
This follows testing best practices and ensures type safety.
38-52
: Error case testing for null organizationId.This test properly verifies that:
- An appropriate error is thrown when organizationId is null
- The error has the expected "unexpected" code
- The correct error log is generated
- No database query is attempted
This comprehensively covers the null input scenario.
54-72
: Comprehensive test for organization not found scenario.The test verifies:
- The correct error type is thrown
- The error has the expected code
- Proper error logging with the specific organizationId
- The database was queried with appropriate parameters
This thoroughly tests the error handling when an organization can't be found.
74-84
: Successful organization resolution test.This test verifies the happy path where an organization is successfully found and returned, with no error logging and proper database query execution.
src/graphql/inputs/QueryActionItemInput.ts (4)
2-2
: Import updated to reflect table renamingThe import path has been correctly updated to align with the table renaming from
actions
toactionItems
, maintaining consistency with the broader refactoring.
5-7
: Improved documentation with numeric labelsAdding the numeric emoji (1️⃣) improves readability and creates a clear visual hierarchy in the documentation.
12-14
: Enhanced documentation with numeric labelsAdding the numeric emoji (2️⃣) continues the documentation pattern established earlier, improving readability.
29-34
: New wrapper schema standardizes input argument structureThis new top-level schema standardizes the GraphQL query argument structure by wrapping the input in an
input
object, which aligns with GraphQL best practices and creates consistency with other mutations and queries in the codebase.test/graphql/types/ActionItem/event.test.ts (8)
1-8
: Good test file organization and importsThe test file is well-organized with clear imports and appropriate mocking setup. The comments provide helpful context about the purpose of the interfaces.
9-18
: Well-defined types for test mocksThe interfaces and types defined here properly model the expected data structures, which will make the tests more robust and maintainable.
19-44
: Comprehensive test setup with proper mockingThe test setup is thorough, with appropriate mocking of the Drizzle client, user data, and event data. The use of
beforeEach
ensures a clean state for each test.
46-51
: Authentication check properly testedThis test correctly verifies that the resolver rejects unauthenticated requests with the appropriate error code.
53-59
: User record validation properly testedThis test correctly verifies that the resolver rejects requests when the user record cannot be found.
61-71
: Authorization validation properly testedThis test correctly verifies that the resolver rejects requests from users without appropriate permissions (system admin or organization admin).
73-87
: Resource not found error properly testedThis test correctly verifies that the resolver throws an appropriate error when the requested event cannot be found, and that it logs the error.
89-121
: Successful event resolution properly testedThis test verifies that:
- The resolver successfully returns an event when all checks pass
- The resolver correctly maps attachments from the raw database format to the expected GraphQL format
The test data setup is comprehensive and appropriate for testing the resolver's functionality.
src/drizzle/tables/actionItemCategories.ts (7)
12-12
: Import updated to reflect table renamingThe import has been correctly updated from
./actions
to./actionItems
to maintain consistency with the table renaming.
16-16
: Table name updated for consistencyThe table export has been renamed from
actionCategoriesTable
toactionItemCategoriesTable
for better naming consistency and to reflect the relationship with action items more clearly.
68-70
: Relations export renamed for consistencyThe relations export has been updated from
actionCategoriesRelations
toactionItemCategoriesRelations
to match the table renaming.
71-73
: Relation field updated to match renamed tableThe relation reference has been updated from
actionsTable
toactionItemsTable
while maintaining the original database relation name.
75-77
: Field reference updated to use renamed tableThe
fields
property now correctly referencesactionItemCategoriesTable.creatorId
instead of the previous table name.
81-83
: Field reference updated to use renamed tableThe
fields
property now correctly referencesactionItemCategoriesTable.organizationId
instead of the previous table name.
87-89
: Field reference updated to use renamed tableThe
fields
property now correctly referencesactionItemCategoriesTable.updaterId
instead of the previous table name.src/drizzle/schema.ts (1)
5-7
: Table and relations exports renamed for consistencyThe exports have been updated from
actionCategoriesTable
andactionCategoriesRelations
toactionItemCategoriesTable
andactionItemCategoriesRelations
with the correct import path, maintaining consistency with the broader refactoring effort.test/graphql/types/ActionItem/organization.test.ts (1)
1-84
: Clean, comprehensive test implementation for the organization resolver.The test file provides thorough coverage of the
resolveOrganization
function with tests for all edge cases: null organizationId, organization not found, and successful organization retrieval. The mocks and assertions are well structured and accurately verify the resolver's behavior.I particularly like how you verify both the error instances and their specific code properties, and check whether database queries are made appropriately in each scenario.
src/graphql/types/Query/ActionCategorybyOrg.ts (7)
2-7
: Updated imports to use new schema and type definitions.The imports have been correctly updated to reflect the new naming conventions for action items and categories.
28-33
: Improved code documentation with numbered steps.Adding the comment to identify this as the authentication check step improves code readability and maintainability.
35-41
: Updated input validation to use the shared schema.The code now correctly uses the imported shared Zod schema for input validation, which improves consistency across the codebase.
56-70
: Updated database query to use renamed table and added comments.The database query has been correctly updated to use
actionItemCategoriesTable
instead of the obsoleteactionCategoriesTable
.
73-76
: Simplified error extension object for unauthenticated errors.The error extension has been simplified for better consistency.
94-94
: Simplified return statement for better readability.The return statement has been improved to handle the null case more elegantly.
1-98
: Please address the pipeline failure for this file.The GitHub Actions pull request workflow indicates unauthorized changes were made to this file. This might need special approval with the 'ignore-sensitive-files-pr' label.
Can you check with your team whether this file requires special permission for modifications?
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 1-1: Unauthorized changes were made in this file. To override this, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/ActionItem/categoryId.ts.ts (1)
45-54
: Good implementation of the ActionItem field with appropriate nullability.The implementation correctly defines the
category
field on the ActionItem type, making it nullable and providing a good description.src/graphql/inputs/MutationDeleteActionItemInput.ts (3)
3-3
: Import path correctly updated to reflect table renaming.The import path has been updated from
"~/src/drizzle/tables/actions"
to"~/src/drizzle/tables/actionItems"
, which properly reflects the table renaming mentioned in the PR description. This ensures consistency with the broader refactoring effort.
6-8
: Well-structured code organization with clear documentation.The schema layering is clearly documented with numbered comments, making the code's organization immediately apparent to developers. This pattern enhances maintainability.
30-35
: Consistent argument wrapping improves schema standardization.Adding this top-level arguments schema that wraps the input under an
input
property follows a standardized pattern for GraphQL mutations, making the codebase more consistent and easier to understand.src/graphql/types/ActionItem/organization.ts (5)
1-10
: Well-structured resolver implementation with proper type imports.The resolver function imports all necessary dependencies and properly types its parameters and return value, which ensures type safety throughout the GraphQL resolving process.
11-17
: Good error handling for missing organization ID.The resolver properly checks for the existence of an organization ID before proceeding, throwing an appropriate error with logging when it's missing.
26-34
: Thorough error handling for missing organization.The resolver includes proper error handling when the organization is not found, with detailed error logging that helps with debugging. The error message includes the specific organization ID that wasn't found.
40-49
: Appropriate implementation of object reference replacing scalar ID.The
organization
field is correctly implemented as a non-nullable object reference, replacing the scalarorganizationId
field. This aligns with GraphQL best practices for modeling relationships and enables efficient nested queries.
19-24
:✅ Verification successful
Missing test coverage for database query operation.
Line 23 with the database query condition
operators.eq(fields.id, parent.organizationId as string)
is flagged as not covered by tests according to static analysis. This critical part of the query logic should be tested.Consider adding a test case that verifies the database query operation by checking that the correct SQL is generated, or by mocking the query to return different results and asserting on the resolver's behavior.
🏁 Script executed:
#!/bin/bash # Check for existing test coverage of the organization resolver rg -A 2 -B 2 "resolveOrganization" test/Length of output: 6451
Existing test coverage confirmed for
resolveOrganization
TheresolveOrganization
resolver is already exercised intest/graphql/types/ActionItem/organization.test.ts
. That suite mocks the Drizzle client’sfindFirst
method to cover:
organizationId
null → throwsTalawaGraphQLError
- valid
organizationId
but no record → throwsTalawaGraphQLError
- valid
organizationId
and record found → returns the expected organizationNo additional tests are required for the database query logic here.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 23-23: src/graphql/types/ActionItem/organization.ts#L23
Added line #L23 was not covered by teststest/graphql/types/ActionItemCategory/createdAt.test.ts (4)
1-14
: Well-structured test setup with clear mocking interfaces.The test file correctly imports necessary dependencies and defines a clear interface for the fake category object used in tests. The mock structure is well-designed to represent the actual data model.
15-49
: Comprehensive test context setup.The
beforeEach
function properly sets up a fresh testing context with mocked dependencies for each test, ensuring test isolation. The mocking of the database client and logger is thorough and realistic.
51-61
: Complete authentication verification.This test thoroughly verifies that the resolver enforces authentication by checking that unauthenticated requests receive the appropriate error code.
63-104
: Thorough authorization testing for different user roles.The tests comprehensively verify the authorization logic by testing multiple scenarios:
- Non-existent user
- Regular user with no admin privileges
- User with global administrator role
- User with organization administrator membership
This ensures that the authorization boundaries are properly enforced.
src/graphql/inputs/MutationCreateActionItem.ts (4)
4-12
: Robust validation schema with appropriate constraints.The Zod schema properly validates all input fields with appropriate type constraints:
- UUID validation for ID fields
- Optional fields marked correctly
- Non-negative integer validation for
allottedHours
This ensures data integrity at the API boundary.
14-58
: Well-documented GraphQL input type with detailed field descriptions.Each field in the GraphQL input type has a detailed and clear description explaining its purpose, constraints, and usage. The required/optional flags match the schema definitions, ensuring consistency between validation and GraphQL schema.
52-56
: Properly implemented allottedHours field aligns with schema updates.The
allottedHours
field is correctly implemented as an optional non-negative integer, which aligns with the database schema updates mentioned in the PR summary (newallotted_hours
numeric column).
60-62
: Consistent argument schema structure.The top-level arguments schema follows the same pattern as other mutations, wrapping the input schema under an
input
property. This consistency improves developer experience and code maintainability.src/graphql/types/ActionItem/updater.ts (1)
3-3
: Verify import path spelling to avoid runtime module‑resolution errors
graphqLimits
is imported here and in other files (e.g.builder.ts
).
If the actual file on disk is namedgraphqlLimits.ts
, module resolution will fail at runtime.
Please confirm the filename and adjust the import if necessary.src/graphql/types/ActionItem/actionItem.ts (1)
38-57
:allottedHours
resolver looks good—consider unit tests for edge‑casesThe NaN‑guard and string‑to‑float parsing are solid.
Please add tests for:
- numeric column returned as string (
"3.5"
➜3.5
)- explicit
0
➜0
- invalid string (
"abc"
) ➜null
null
➜null
This will lock in the intended behaviour and address the uncovered‑lines alert from Codecov.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 43-43: src/graphql/types/ActionItem/actionItem.ts#L43
Added line #L43 was not covered by tests
[warning] 46-49: src/graphql/types/ActionItem/actionItem.ts#L46-L49
Added lines #L46 - L49 were not covered by tests
[warning] 51-53: src/graphql/types/ActionItem/actionItem.ts#L51-L53
Added lines #L51 - L53 were not covered by tests
[warning] 55-56: src/graphql/types/ActionItem/actionItem.ts#L55-L56
Added lines #L55 - L56 were not covered by testssrc/graphql/inputs/QueryActionCategoriesByOrganizationInput.ts (3)
9-13
: Well-structured validation schema with clear error messaging.The Zod validation schema is properly defined with UUID validation and a helpful error message, enforcing proper data format for the organizationId parameter.
18-31
: Well-defined GraphQL input type with comprehensive documentation.The input type definition includes detailed descriptions for both the type itself and its field. Making the organizationId a required field ensures the query will always have the necessary data to execute properly.
37-39
: Good pattern for arguments consistency.Wrapping the input schema in an arguments schema follows a consistent pattern that makes resolvers easier to maintain and understand. This approach aligns well with GraphQL best practices for structuring mutation and query arguments.
src/graphql/inputs/MutationCreateActionItemCategory.ts (3)
8-12
: Well-designed input validation schema with appropriate constraints.The schema properly validates that the name is non-empty, the organizationId is a UUID, and the isDisabled flag is optional, ensuring data integrity for new action item categories.
17-42
: Excellent GraphQL input type implementation with detailed field descriptions.The input type definition uses builder.inputRef with the Zod schema's inferred type, creating a strongly-typed GraphQL input. Each field has a clear description and appropriate required flags matching the validation schema.
47-49
: Consistent argument structure for GraphQL mutations.Wrapping the input in an arguments schema under the "input" field provides a consistent pattern for mutations throughout the GraphQL API, making the codebase more maintainable.
src/graphql/types/ActionItemCategory/organization.ts (1)
46-56
: Well-structured field implementation for the ActionItemCategory type.The implementation correctly wires the resolver to the ActionItemCategory type with appropriate nullability settings and a clear description for the field.
test/graphql/types/ActionItem/creator.test.ts (6)
8-29
: Good test setup with proper mocking.The test setup effectively creates mock objects and functions needed for testing the resolver, with clear variable names and proper typing.
31-52
: Thorough authentication testing.The tests properly verify that the resolver throws the correct authentication errors when a user is not signed in or not found.
54-68
: Comprehensive authorization testing.The test effectively verifies that users without proper roles or memberships cannot access the creator information.
70-99
: Proper handling of edge cases and matching creator scenarios.These tests ensure the resolver correctly handles null creator IDs and cases where the creator is the current user, following the expected behavior in each scenario.
101-120
: Effective testing of successful creator resolution.The test properly verifies that when the creator ID is different from the current user and the creator exists, the resolver returns the correct user record.
122-145
: Thorough error handling test for missing creators.This test effectively verifies that when a creator cannot be found, the resolver logs an appropriate error message and throws an unexpected error.
src/graphql/inputs/MutationUpdateActionItemInput.ts (3)
4-6
: Corrected import path & Zod alias look good
The switch to~/src/drizzle/tables/actionItems
and thez as zod
alias keeps the import list tight and prevents name‑clashes.
20-22
:allottedHours
is allowed to benull
andundefined
– verify intent
Because the column is nullable, acceptingnull
is fine, but acceptingundefined
as well makes three distinct states (missing / null / number). Make sure callers really need that extra degree of freedom; otherwise consider using.nullable()
without.optional()
so the API surface is simpler.
63-65
: 👍 Wrapped‑argument schema keeps a consistent mutation pattern
Using{ input: … }
mirrors the pattern adopted by the other mutations and keeps validation code uniform.src/graphql/types/Mutation/updateActionItem.ts (1)
59-86
: Heads‑up: CI marks this file as “sensitive”
The pull‑request workflow failed with “Unauthorized changes were made in this file”. If this mutation is expected to live under the protected‑files list, remember to apply theignore-sensitive-files-pr
label or update the protection rules before merging.src/drizzle/tables/actionItems.ts (1)
68-69
:numeric
is typed as string in Drizzle – mind GraphQL scalar mismatches
allottedHours
will serialise as a string (per Drizzle typing) while GraphQL clients will likely expect aFloat
. Either:
- declare the column with explicit precision/scale and cast to number when exposing via GraphQL, or
- store as
integer
/double precision
if partial hours are not required.Failing to convert can lead to silent type coercion bugs on the client side.
src/graphql/types/Mutation/createActionItemCategory.ts (1)
14-30
: Pipeline warns about unauthorised changesCI flagged this file as “Unauthorized changes were made in this file”.
Ensure theignore-sensitive-files-pr
label is applied or move the logic into an approved directory to unblock the pipeline.src/graphql/types/ActionItem/creator.ts (2)
82-91
: Clear implementation of creator field on ActionItem typeThe implementation correctly adds the creator field to the ActionItem GraphQL type with appropriate nullable configuration and description. This supports the schema update to replace flat ID references with object relations.
6-80
:✅ Verification successful
Well-structured resolver with comprehensive authentication and authorization
The
resolveCreator
function is well-implemented with a clear step-by-step approach:
- Authentication check to verify the client is authenticated
- Fetching the current user and their membership in the organization
- Authorization check to ensure the user is an administrator (global or organization-level)
- Handling null creatorId case
- Performance optimization by returning the current user if they are the creator
- Fallback to fetch the creator user by ID
This implementation properly supports the GraphQL schema migration from flat ID references to object references.
🏁 Script executed:
#!/bin/bash # Check test coverage for resolveCreator function # Find the test file for creator resolver echo "Looking for test file for ActionItem creator resolver..." find test -name "creator.test.ts" | grep "ActionItem" # Check which test cases are already implemented echo "Checking existing test cases:" grep -A 2 "it(" test/graphql/types/ActionItem/creator.test.ts 2>/dev/null || echo "No test file found" # Check lines flagged as uncovered by CodeCov echo "Checking lines flagged as uncovered (lines 26, 45, 65-67):" sed -n '26p;45p;65,67p' src/graphql/types/ActionItem/creator.tsLength of output: 1741
Coverage Verified: All branches of
resolveCreator
are exercised
- Located
test/graphql/types/ActionItem/creator.test.ts
covering:
- Unauthenticated if not signed in or user not found
- Unauthorized when neither global nor org‑level admin
creatorId === null
returnsnull
creatorId === currentUserId
returns the fetched current user- Fetching and returning a different existing user
- Logging an error and throwing when the creator lookup returns
undefined
All logical paths (authentication, membership checks, null guard, identity shortcut, and fallback fetch) are tested. CodeCov’s reported gaps appear to be configuration artifacts. No additional changes are required.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 26-26: src/graphql/types/ActionItem/creator.ts#L26
Added line #L26 was not covered by tests
[warning] 45-45: src/graphql/types/ActionItem/creator.ts#L45
Added line #L45 was not covered by tests
[warning] 65-67: src/graphql/types/ActionItem/creator.ts#L65-L67
Added lines #L65 - L67 were not covered by teststest/graphql/types/documentNodes.ts (6)
883-903
: Updated CreateActionItem mutation with nested object referencesThe mutation document has been successfully updated to:
- Use the new input type
MutationCreateActionItemInput
- Include the new
allottedHours
field in the selection set- Replace flat ID fields with nested object references for related entities (category, assignee, event, organization, creator, updater)
This aligns with the PR objective of migrating ActionItem category handling to GraphQL.
905-920
: Updated UpdateActionItem mutation with nested objects and allottedHours fieldThe mutation now correctly includes:
- The new
allottedHours
field- Nested object references instead of flat IDs
- Proper selection of the updatedAt field
This change is consistent with the schema migration described in the PR objectives.
922-938
: Updated DeleteActionItem mutation with nested object referencesThe mutation has been correctly updated to include:
- The new
allottedHours
field- Nested object references for category, assignee, and organization
- Proper structure for selection sets
This change is well-aligned with the overall schema updates.
940-951
: New ActionItemsByOrganization query for categoriesThis new query supports fetching action item categories by organization, including:
- Basic category fields (id, createdAt)
- Nested object references for organization and creator
This is a critical addition for the migration of action item category handling to GraphQL.
974-995
: Comprehensive ActionItemsByOrganization query with nested objectsThis updated query now includes:
- Complete field selection for action items (isCompleted, notes, timestamps, etc.)
- The new
allottedHours
field- Nested object references for all related entities
This query provides the necessary data access for the migrated functionality.
1045-1059
: New CreateActionItemCategory mutationThis new mutation supports the creation of action item categories with:
- Basic category fields (name, isDisabled)
- Appropriate timestamps
- Nested object references for organization and creator
This is a key addition for migrating category management from Express endpoints to GraphQL.
test/graphql/types/gql.tada-cache.d.ts (7)
117-118
: Updated type declaration for CreateActionItem mutationThe type declaration now properly reflects:
- The correct input type
MutationCreateActionItemInput
- The inclusion of the new
allottedHours
field as a nullable number- Nested object types for related entities instead of flat IDs
This update ensures type safety for tests using the CreateActionItem mutation.
119-120
: Updated type declaration for UpdateActionItem mutationThe type definition correctly includes:
- The new
allottedHours
field- Nested object types for category, assignee, and updater
- Proper nullable configuration
This ensures consistent type checking for tests using this mutation.
121-122
: Updated type declaration for DeleteActionItem mutationThe type declaration properly includes:
- The
allottedHours
field- Nested object types for related entities
- Consistent null handling
This completes the type safety for action item mutation operations.
123-124
: Added type declaration for ActionItemsByOrganization queryThe type declaration supports fetching action item categories by organization with:
- Basic fields like id and createdAt
- Nested object types for organization and creator
This ensures type safety for the new action item categories functionality.
129-130
: Comprehensive type declaration for ActionItemsByOrganization queryThe type declaration includes:
- All action item fields including the new
allottedHours
- Nested object types for all related entities
- Proper nullable configurations
This provides complete type safety for the query used to fetch action items.
141-142
: Added type declaration for CreateActionItemCategory mutationThis new declaration supports:
- The input type for creating action item categories
- Return type with category fields and nested organization and creator objects
- Appropriate nullable configurations
This completes the type safety for the action item category management functionality.
152-152
: Fixed type declaration for advertisement queryThe type declaration has been corrected to use proper field types, fixing the
isCompleted
filter in the where clause for the advertisements query.schema.graphql (9)
42-43
: Approve: Addedorganization
reference toActionItem
Introducing theorganization: Organization!
field correctly models the relationship and is non-nullable, matching the requirement that every action item belongs to an organization.
54-55
: Approve: Addedupdater
reference toActionItem
The newupdater: User
field will allow clients to fetch who last modified the action item.
59-59
: Approve: DocumentedcreatedAt
onActionItemCategory
Adding the timestamp helps clients audit category creation.
74-75
: Approve: Addedorganization
reference toActionItemCategory
Marking the category’s organization with a non-nullableOrganization!
field is consistent and necessary.
1462-1465
: Approve: IntroducedcreateActionItem
mutation
The newcreateActionItem(input: MutationCreateActionItemInput!): ActionItem
mutation aligns with the GraphQL API’s pattern and uses the new input type.
1467-1473
: Approve: IntroducedcreateActionItemCategory
mutation
AddingcreateActionItemCategory(input: MutationCreateActionItemCategoryInput!): ActionItemCategory
completes the CRUD surface for categories.
1649-1654
: Approve: IntroducedupdateActionItemCategory
mutation
TheupdateActionItemCategory(input: MutationUpdateActionItemCategoryInput!): ActionItemCategory
mutation correctly complements the create and delete operations.
1742-1758
: Approve:MutationCreateActionItemCategoryInput
is well-defined
Input fields (name: String!
,organizationId: ID!
, optionalisDisabled
) cover all necessary parameters for creating a category.
2444-2456
: Approve:MutationUpdateActionItemCategoryInput
is clear
The input allows updating the category name and disabled state, withcategoryId: ID!
to identify the target.
Signed-off-by: NishantSinghhhhh <[email protected]>
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
test/graphql/types/Query/ActionItem.test.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
test/graphql/types/Query/ActionItem.test.ts (2)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: tests/resolvers/Query/actionItemsByOrganization.spec.ts:51-82
Timestamp: 2024-11-02T06:36:31.866Z
Learning: When reviewing `tests/resolvers/Query/actionItemsByOrganization.spec.ts`, if the existing test cases already provide 100% coverage, avoid suggesting additional test cases.
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.
🧬 Code Graph Analysis (1)
test/graphql/types/Query/ActionItem.test.ts (4)
test/graphql/types/client.ts (1)
mercuriusClient
(7-9)test/graphql/types/documentNodes.ts (5)
Query_signIn
(191-218)Mutation_createUser
(12-40)Mutation_createOrganization
(400-408)Mutation_createOrganizationMembership
(437-442)Query_actionItemsByOrganization
(974-995)test/server.ts (1)
server
(12-27)src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(18-102)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run tests for talawa api
🔇 Additional comments (7)
test/graphql/types/Query/ActionItem.test.ts (7)
1-5
: Updates to imports and file structure look good.The file properly imports the renamed
actionItemsTable
from the new path, which aligns with the table renaming from "actions" to "actionItemsTable" mentioned in the PR. The import organization and vitest test structure follow good practices.
20-44
: Improved authentication handling with proper environment variable validation.The
globalSignInAndGetToken
function now properly validates administrator credentials through environment variables, which is a security improvement. The error handling is thorough and provides clear error messages if environment configurations are missing.
46-76
: User creation helper is well-structured with proper type safety.The
createUser
function now has a cleaner implementation with better type handling and parameter defaults. The non-null assertions are appropriately wrapped in self-executing functions for better error isolation.
110-132
: Action item creation properly includes the newallottedHours
field.The
seedActionItem
function has been updated to include the newallottedHours
field, which aligns with the database table changes where this field was added. This ensures tests create data that matches the new schema.
135-173
: Test setup provides good isolation with multiple organizations and user roles.The test setup is robust, creating two organizations and users with different roles to properly test authorization scenarios. The cleanup mechanism is well-implemented and the seed data is properly initialized.
175-182
: Clean implementation of test teardown.The afterEach block properly cleans up by executing functions in reverse order (proper stack unwinding), with error handling to ensure cleanup continues even if individual operations fail. This prevents test interference.
184-210
: Authentication and authorization tests are comprehensive.The test cases cover important security scenarios:
- Unauthenticated access (no token)
- Empty results when appropriate
- Unauthorized access for non-members
These tests ensure the GraphQL resolvers properly enforce access controls for the refactored action items API.
Please fix the failing test. |
@meetulr sir, @palisadoes sir should I add any more changes in here ?? |
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.
I've highlighted these before. Please take a thorough look yourself and identify such errors.
I am looking at the whole PR again and will write all changes I make in here |
Signed-off-by: NishantSinghhhhh <[email protected]>
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: 8
♻️ Duplicate comments (13)
src/graphql/types/ActionItem/index.ts (1)
1-1
: Import statement includes file extensionThe import for
actionItem.ts
includes the file extension, which is inconsistent with the project's import style based on past reviews.-import "./actionItem"; +import "./actionItem";src/graphql/types/Mutation/updateActionItem.ts (1)
193-206
:null
cannot be written back – loss of informationWhen the client explicitly sets
allottedHours
tonull
(to clear it), the current logic turns it intoundefined
, which Drizzle interprets as "leave the column untouched".- allottedHours: - fieldsToUpdate.allottedHours != null - ? fieldsToUpdate.allottedHours.toString() - : undefined, + allottedHours: + fieldsToUpdate.allottedHours === undefined + ? undefined // field omitted + : fieldsToUpdate.allottedHours === null + ? null // explicit null + : fieldsToUpdate.allottedHours.toString(),This preserves the three states: unset, cleared, and updated.
src/graphql/types/Mutation/createActionItem.ts (3)
12-13
: Rename exported constant to match its purposeThe exported constant name
createActionItemCategoryMutation
doesn't match the mutation field namecreateActionItem
it implements, creating confusion.-export const createActionItemCategoryMutation = builder.mutationField( +export const createActionItemMutation = builder.mutationField(
145-145
:completionAt
is pre-filled althoughisCompleted
isfalse
Creating a new action item with
isCompleted: false
shouldn't have acompletionAt
timestamp. This creates an inconsistent state.-completionAt: new Date(), +completionAt: null,
151-154
: NumericallottedHours
is stored as a string – add an explicit cast for clarityWhile
numeric
columns map tostring
in Drizzle, use an explicit conversion for clarity.allottedHours: parsedArgs.input.allottedHours !== undefined - ? parsedArgs.input.allottedHours.toString() + ? String(parsedArgs.input.allottedHours) : "0",src/graphql/inputs/MutationUpdateActionItemInput.ts (1)
53-56
: Type mismatch between GraphQL & DB (Int
vsnumeric
)The database column is declared as
numeric
, but the GraphQL input type usest.int()
. If you need to support fractional hours, uset.float()
instead.-allottedHours: t.int({ +allottedHours: t.float({ description: "Number of hours allotted for completion.", required: false, })src/drizzle/tables/actionItems.ts (2)
104-104
: Typo in exported constant name
actionsItemRelations
mixes plural/singular and deviates from the surrounding naming scheme (actionItemsTable
). PreferactionItemRelations
oractionItemsRelations
for clarity.-export const actionsItemRelations = relations(actionItemsTable, ({ one }) => ({ +export const actionItemRelations = relations(actionItemsTable, ({ one }) => ({
108-108
:⚠️ Potential issue
relationName
does not match the physical column nameThe
relationName
still refers toassignee_id
but the relation mapping is fromactor_id
.-relationName: "actions.assignee_id:users.id", +relationName: "actions.actor_id:users.id",test/graphql/types/Query/ActionItem.test.ts (1)
210-211
: Add a test for successful retrieval of action itemsThe test suite is missing a positive case that verifies authorized members can successfully retrieve action items.
test("returns action items for authorized member", async () => { // Seed a couple of items in orgId for regular user await seedActionItem(orgId, regular.userId); await seedActionItem(orgId, regular.userId); const res = await mercuriusClient.query( Query_actionItemsByOrganization, { headers: { authorization: `bearer ${regular.authToken}` }, variables: { input: { organizationId: orgId } }, } ); expect(res.errors).toBeUndefined(); expect(res.data?.actionItemsByOrganization).toHaveLength(2); expect(res.data?.actionItemsByOrganization.map(item => item.id)).toBeTruthy(); });schema.graphql (4)
10-11
: Schema/Input Type Mismatch for allottedHours
TheActionItem.allottedHours
field is declared asFloat
, but the corresponding input types (MutationCreateActionItemInput
andMutationUpdateActionItemInput
) useInt
. This breaks type consistency and prevents fractional hours.
Please unify by changing the input types toFloat
.
19-20
: Optional: Makecategory
non-nullable
Since everyActionItem
should belong to a category, consider markingcategory: ActionItemCategory!
to enforce this constraint at the schema level.
62-63
: Optional: Makecreator
non-nullable
If every category is created by a user, enforcingcreator: User!
can improve schema robustness.
1760-1790
: Duplicate: Type mismatches inMutationCreateActionItemInput
The input definesallottedHours: Int
andassignedAt: String
, which conflicts withallottedHours: Float
andassignedAt: DateTime
inActionItem
.
Please update the input fields toFloat
andDateTime
respectively.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
schema.graphql
(9 hunks)src/createServer.ts
(0 hunks)src/drizzle/tables/actionItems.ts
(2 hunks)src/envConfigSchema.ts
(0 hunks)src/graphql/inputs/MutationUpdateActionItemInput.ts
(2 hunks)src/graphql/types/ActionItem/actionItemCategory.ts
(1 hunks)src/graphql/types/ActionItem/index.ts
(1 hunks)src/graphql/types/Mutation/createActionItem.ts
(1 hunks)src/graphql/types/Mutation/updateActionItem.ts
(6 hunks)test/graphql/types/ActionItem/categoryId.test.ts
(1 hunks)test/graphql/types/Query/ActionItem.test.ts
(2 hunks)test/graphql/types/gql.tada-cache.d.ts
(3 hunks)
💤 Files with no reviewable changes (2)
- src/envConfigSchema.ts
- src/createServer.ts
🧰 Additional context used
🧠 Learnings (1)
test/graphql/types/Query/ActionItem.test.ts (2)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: tests/resolvers/Query/actionItemsByOrganization.spec.ts:51-82
Timestamp: 2024-11-02T06:36:31.866Z
Learning: When reviewing `tests/resolvers/Query/actionItemsByOrganization.spec.ts`, if the existing test cases already provide 100% coverage, avoid suggesting additional test cases.
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.
🧬 Code Graph Analysis (5)
test/graphql/types/ActionItem/categoryId.test.ts (4)
src/graphql/context.ts (1)
GraphQLContext
(55-55)test/_Mocks_/drizzleClientMock.ts (1)
createMockDrizzleClient
(31-51)src/graphql/types/ActionItem/actionItemCategory.ts (1)
resolveCategory
(5-29)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)
src/drizzle/tables/actionItems.ts (3)
src/drizzle/tables/users.ts (1)
usersTable
(59-216)src/drizzle/tables/actionItemCategories.ts (1)
actionItemCategoriesTable
(16-66)src/drizzle/tables/organizations.ts (1)
organizationsTable
(31-128)
src/graphql/inputs/MutationUpdateActionItemInput.ts (1)
src/drizzle/tables/actionItems.ts (1)
actionsTableInsertSchema
(138-138)
test/graphql/types/Query/ActionItem.test.ts (4)
test/graphql/types/client.ts (1)
mercuriusClient
(7-9)test/graphql/types/documentNodes.ts (5)
Query_signIn
(191-218)Mutation_createUser
(12-40)Mutation_createOrganization
(400-408)Mutation_createOrganizationMembership
(437-442)Query_actionItemsByOrganization
(974-995)test/server.ts (1)
server
(12-27)src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(18-102)
src/graphql/types/Mutation/updateActionItem.ts (2)
src/utilities/isNotNullish.ts (1)
isNotNullish
(12-14)src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(18-102)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run tests for talawa api
🔇 Additional comments (25)
src/graphql/types/ActionItem/index.ts (1)
1-10
: LGTM: Imports correctly modularizedThe file has been properly updated to import all necessary modules without re-exporting them, which aligns with the migration of ActionItem category handling to GraphQL. The imports reflect the new resolvers for category, organization, and updater fields.
test/graphql/types/ActionItem/categoryId.test.ts (4)
10-36
: LGTM: Test setup with appropriate mocksThe test setup is well-structured with proper mocking of the GraphQL context, Drizzle client, and logger. The type definition for
findFirstMock
is detailed and correct.
38-46
: LGTM: Null category ID handling testThis test case correctly verifies that the resolver returns null when the category ID is null, without unnecessarily calling the database.
48-70
: LGTM: Error handling test is comprehensiveThis test properly verifies that the resolver throws a TalawaGraphQLError with the correct message, error code, and issues when a category is not found. It also verifies that the error is logged correctly.
72-87
: LGTM: Successful resolution testThis test case properly verifies that the resolver returns the category when it is found in the database, and confirms that no errors are logged.
src/graphql/types/ActionItem/actionItemCategory.ts (2)
1-29
: LGTM: Effective category resolver implementationThe
resolveCategory
function is well-implemented with proper handling for:
- Null category IDs
- Database queries with appropriate filtering
- Error handling with detailed error messages
- Logging when the category is not found
The implementation aligns with the PR objective of migrating ActionItem category handling to GraphQL.
31-40
: LGTM: Clean field implementation on ActionItem typeThe ActionItem type is properly extended with a category field that:
- Returns the appropriate type (ActionItemCategory)
- Is correctly marked as nullable
- Has a clear description
- Uses the resolveCategory function for resolution
This completes the GraphQL schema connection for the category relationship.
src/graphql/types/Mutation/updateActionItem.ts (5)
1-10
: LGTM: Imports updated for action item renamesThe imports have been properly updated to reflect the renamed tables and types, which aligns with the PR objective of migrating ActionItem category handling to GraphQL.
59-65
: LGTM: Database query updated for renamed tablesThe database query has been correctly updated to use
actionItemsTable
instead ofactionsTable
, with proper column references including the renamedactionItemCategoryId
.
143-167
: LGTM: Category validation logic updatedThe category validation logic has been properly updated to use
actionItemCategoryId
instead ofcategoryId
, with appropriate database queries to the renamedactionItemCategoriesTable
.
246-249
: LGTM: Mark pending query updatedThe database query to fetch an existing action item for the "mark as pending" mutation has been correctly updated to use
actionItemsTable
.
276-285
: LGTM: Mark pending update operation updatedThe database update operation has been correctly updated to use
actionItemsTable
for the "mark as pending" mutation.test/graphql/types/gql.tada-cache.d.ts (6)
117-118
: Well-structured CreateActionItem mutation with comprehensive field coverage.The CreateActionItem mutation is well-structured with proper fields like
allottedHours
which aligns with the PR's objective of migrating ActionItem Category handling to GraphQL. The nested object references for category, assignee, event, organization, creator, and updater represent good GraphQL schema design by returning entity relationships instead of just IDs.
119-120
: UpdateActionItem mutation looks good with appropriate field subset.The UpdateActionItem mutation correctly includes only the fields that make sense to update, following the principle of providing only what's needed in the interface. The mutation properly returns the updated entity with its relationships.
121-122
: DeleteActionItem mutation properly returns the full object.The DeleteActionItem mutation follows best practices by returning the complete deleted object and its relationships, which allows clients to have the full context of what was removed. This is especially useful for undo operations or displaying confirmation messages.
123-124
: Two variants of ActionItemsByOrganization with different detail levels.The implementation provides two versions of the ActionItemsByOrganization query - a simplified one (123-124) and a more detailed one (129-130). This gives API consumers flexibility depending on their data needs. The detailed version includes all fields including the new
allottedHours
field consistent with the other mutations.Also applies to: 129-130
141-142
: CreateActionItemCategory mutation follows consistent pattern.The CreateActionItemCategory mutation completes the CRUD operations for action item categories. It follows the same pattern as other entity creation mutations with appropriate fields and nested relationships. The inclusion of the
isDisabled
boolean provides good flexibility for category management.
152-152
:❌ Incorrect review comment
Verify the where condition change in OrganizationAdvertisements query.
The
isCompleted
filter in the where clause of OrganizationAdvertisements query seems unrelated to advertisements. This might be a copy-paste error or a mistake in the type definition.
🏁 Script executed:
#!/bin/bash # Look for other instances of AdvertisementWhereInput to confirm expected properties rg -A 3 -B 3 "AdvertisementWhereInput" --type tsLength of output: 5813
🏁 Script executed:
#!/bin/bash # Display the implementation of AdvertisementWhereInput to verify its fields rg -A 30 -n "AdvertisementWhereInput" src/graphql/inputs/QueryOrganizationInput.tsLength of output: 490
Ignore the
isCompleted
filter concern
TheisCompleted
field is intentionally defined onAdvertisementWhereInput
insrc/graphql/inputs/QueryOrganizationInput.ts
(lines 69–73) and is used to filter advertisements by completion status. No change is required.Likely an incorrect or invalid review comment.
schema.graphql (7)
42-43
: Approve: Non-nullable organization field
Definingorganization: Organization!
ensures that anActionItem
always has an organization, which aligns with the domain model.
54-55
: Approve:updater
field addition
Addingupdater: User
provides traceability for who last updated the action item.
59-59
: Approve: Timestamp documentation
The new doc string forcreatedAt
inActionItemCategory
clarifies the field's purpose.
74-75
: Approve: Non-nullable organization on category
Requiringorganization: Organization!
ensures consistency and integrity of category assignments.
1462-1466
: Approve: NewcreateActionItem
mutation
The mutation signature and input type align correctly with the schema refactor.
1467-1473
: Approve: NewcreateActionItemCategory
mutation
The addition ofcreateActionItemCategory
extends the API surface as intended and uses the new input type.
1742-1758
: Approve:MutationCreateActionItemCategoryInput
definition
The input type clearly documents required fields and optional flags for creating an action item category.
This comment was marked as off-topic.
This comment was marked as off-topic.
Signed-off-by: NishantSinghhhhh <[email protected]>
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: 16
🔭 Outside diff range comments (1)
schema.graphql (1)
3536-3539
:⚠️ Potential issueCritical:
organizationId
input type mismatch and unused input
- The
QueryActionItemsByOrganizationInput
usesorganizationId: String!
instead ofID!
.- This input type is currently unused by the
actionItemsByOrganization
query.
Please update the scalar toID!
and ensure the query signature consumes this input.
♻️ Duplicate comments (5)
schema.graphql (2)
10-11
:⚠️ Potential issueCritical: Mismatch between schema field and input type for
allottedHours
TheActionItem.allottedHours
is declared asFloat
, but inMutationCreateActionItemInput
andMutationUpdateActionItemInput
it’s defined asInt
. This prevents clients from specifying fractional hours and is inconsistent with the schema.
Please unify these types toFloat
.Example diff:
# In input types: input MutationCreateActionItemInput { - allottedHours: Int + allottedHours: Float ... } input MutationUpdateActionItemInput { - allottedHours: Int + allottedHours: Float ... }
1760-1791
:⚠️ Potential issueCritical: Input type mismatch for
assignedAt
andallottedHours
assignedAt
is declared asString
, but the schema field usesDateTime
.allottedHours
is declared asInt
, but the schema field usesFloat
.
These inconsistencies can lead to runtime errors and type coercion issues.
Please update the input type to useDateTime
forassignedAt
andFloat
forallottedHours
.Example diff:
input MutationCreateActionItemInput { - allottedHours: Int + allottedHours: Float - assignedAt: String + assignedAt: DateTime ... }test/graphql/types/gql.tada.d.ts (3)
77-78
:allottedHours
/assignedAt
scalar mismatches persistThe input still declares
•allottedHours: Int
(schema usesFloat
)
•assignedAt: String
(schema usesDateTime
)This exact issue was raised in the previous review and remains unresolved. Please regenerate the introspection file after aligning your GraphQL input definitions with the object field types.
129-129
: Update mutation remains rigid & type-inconsistent
MutationUpdateActionItemInput
still forces
•isCompleted: NON_NULL Boolean
– prevents PATCH-style updates
•allottedHours: Int
– should beFloat
Both concerns were flagged earlier. Adjust the schema and regenerate the typings.
195-195
:organizationId
scalar inconsistency not fixed
QueryActionItemsByOrganizationInput.organizationId
continues to useString
, whereas the new categories query correctly usesID
. This inconsistency was highlighted before and can confuse API consumers & code-gen tools.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
schema.graphql
(10 hunks)src/createServer.ts
(1 hunks)src/envConfigSchema.ts
(1 hunks)src/graphql/types/Query/ActionItem.ts
(0 hunks)src/graphql/types/Query/ActionItemCategorybyOrganization.ts
(1 hunks)src/graphql/types/Query/ActionItembyOrganization.ts
(1 hunks)src/graphql/types/Query/index.ts
(1 hunks)test/graphql/types/Query/ActionItem.test.ts
(0 hunks)test/graphql/types/documentNodes.ts
(2 hunks)test/graphql/types/gql.tada-cache.d.ts
(3 hunks)test/graphql/types/gql.tada.d.ts
(5 hunks)
💤 Files with no reviewable changes (2)
- src/graphql/types/Query/ActionItem.ts
- test/graphql/types/Query/ActionItem.test.ts
🧰 Additional context used
🧠 Learnings (1)
schema.graphql (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: src/typeDefs/inputs.ts:80-80
Timestamp: 2024-11-01T10:39:50.333Z
Learning: In `src/typeDefs/inputs.ts`, the `orgId` field in `ActionItemWhereInput` is intentionally optional (`ID`) and should not be changed to non-nullable (`ID!`).
🪛 GitHub Actions: Pull request workflow
src/createServer.ts
[error] 15-15: Error: env must have required property 'API_REDIS_URI'. The environment variable 'API_REDIS_URI' is missing, causing the server initialization to fail.
src/graphql/types/Query/index.ts
[error] 1-1: Unauthorized changes detected in this file. To override, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Query/ActionItembyOrganization.ts
[error] 1-1: Unauthorized changes detected in this file. To override, apply the 'ignore-sensitive-files-pr' label.
src/graphql/types/Query/ActionItemCategorybyOrganization.ts
[error] 1-1: Unauthorized changes detected in this file. To override, apply the 'ignore-sensitive-files-pr' label.
schema.graphql
[error] 1-1: Unauthorized changes detected in this file. To override, apply the 'ignore-sensitive-files-pr' label.
🔇 Additional comments (21)
src/graphql/types/Query/ActionItembyOrganization.ts (3)
60-67
: Unauthenticated guard is solid but remember the PR pipeline flagLogic is correct; however, GitHub Actions reported “Unauthorized changes detected in this file”.
If this file is intentionally modified, please add theignore-sensitive-files-pr
label or update the sensitive-file list to unblock the pipeline.
188-200
:with:
selection ‑ confirm that every relation exists in the Drizzle schema
assignee
,category
,creator
,event
,organization
,updater
are eagerly loaded.
If any of these relations were renamed during the refactor (creator
→createdBy
, etc.) the query will fail at runtime. Re-run type-checking or integration tests to guarantee alignment.
68-72
:❌ Incorrect review comment
queryActionItemsByOrganizationArgumentsSchema
likely expects a flat object – validateYou wrap
organizationId
inside aninput
property that the original Express version used:queryActionItemsByOrganizationArgumentsSchema.safeParse({ input: { organizationId: args.organizationId }, });If the Zod schema was updated for GraphQL (i.e.,
organizationId
now lives at the root), this will always fail and force the error path. Double-check the schema signature and drop theinput:
wrapper if unnecessary.- queryActionItemsByOrganizationArgumentsSchema.safeParse({ - input: { organizationId: args.organizationId }, - }); + queryActionItemsByOrganizationArgumentsSchema.safeParse({ + organizationId: args.organizationId, + });
This will help verify whether the schema expects
{ input: { organizationId } }
or a flat{ organizationId }
shape.
🏁 Script executed:
#!/bin/bash set -e # Search for the definition and usage of the Zod schema in the codebase rg "queryActionItemsByOrganizationArgumentsSchema" -nLength of output: 380
🏁 Script executed:
#!/bin/bash set -e echo "=== Zod Schema Definition (QueryActionItemInput.ts) ===" sed -n '1,80p' src/graphql/inputs/QueryActionItemInput.ts echo "" echo "=== Resolver Usage (ActionItem.ts) ===" sed -n '1,80p' src/graphql/types/Query/ActionItem.tsLength of output: 3576
No change needed—
input
wrapper is correctThe
queryActionItemsByOrganizationArgumentsSchema
is defined as:export const queryActionItemsByOrganizationArgumentsSchema = z.object({ input: queryActionItemsByOrgInputSchema, });…and the GraphQL field’s
args
shape is{ input: { organizationId: string } }
. Dropping theinput
wrapper would break the validation.Likely an incorrect or invalid review comment.
src/graphql/types/Query/ActionItemCategorybyOrganization.ts (1)
70-75
: Auth guard passes butcurrentClient.user
may still beundefined
Immediately after the guard you access
ctx.currentClient.user.id
. Ensure the context always hydrates theuser
object onceisAuthenticated
is true, or add a null check to prevent a crash on malformed tokens.src/graphql/types/Query/index.ts (1)
24-25
: Module registration looks goodThe new paginated queries are correctly registered in the root Query index. No further action required.
schema.graphql (14)
42-43
: Approve: Added non-nullableorganization
reference
Theorganization: Organization!
field correctly replaces the ID reference with a direct object and enforces its presence.
54-55
: Approve: Addedupdater
field toActionItem
Providing theupdater: User
relation aligns with other timestamp fields and enhances auditability.
59-63
: Approve: Documentation and creator relation forActionItemCategory
The added docstring forcreatedAt
and thecreator: User
field improve clarity and consistency with other types.
74-75
: Approve: Non-nullableorganization
onActionItemCategory
Markingorganization: Organization!
ensures every category is scoped to an organization, aligning with the data model.
1461-1466
: Approve:createActionItem
mutation signature
The newcreateActionItem(input: MutationCreateActionItemInput!): ActionItem
mutation correctly uses the consolidated input type for cleaner API design.
1467-1473
: Approve:createActionItemCategory
mutation added
Enabling category management viacreateActionItemCategory(input: …): ActionItemCategory
is a valuable addition to the GraphQL API.
1649-1654
: Approve:updateActionItemCategory
mutation added
Supporting updates to action item categories withupdateActionItemCategory(input: …): ActionItemCategory
completes the CRUD operations surface.
1742-1758
: Approve:MutationCreateActionItemCategoryInput
definition
The input type withname: String!
,organizationId: ID!
, and optionalisDisabled
is clear and enforces required metadata.
3379-3388
: Approve: PaginatedactionCategoriesByOrganization
query
The switch to Relay-style pagination withinput: QueryActionCategoriesByOrganizationInput!
and the connection pattern is consistent and scalable.
3505-3508
: Approve: Connection type for paginated category results
QueryActionCategoriesByOrganizationConnection
follows the Relay spec and integrates well with the query.
3510-3513
: Approve: Edge type for paginated category results
QueryActionCategoriesByOrganizationConnectionEdge
is correctly structured withcursor
andnode
.
3518-3523
: Approve: Input for paginated category queries
QueryActionCategoriesByOrganizationInput
with a non-nullableorganizationId: ID!
neatly encapsulates business arguments.
3525-3528
: Approve: Connection type for paginated action items
QueryActionItemsByOrganizationConnection
is correctly defined to support cursor-based pagination.
3530-3533
: Approve: Edge type for paginated action items
QueryActionItemsByOrganizationConnectionEdge
correctly provides thecursor
andnode
fields.test/graphql/types/gql.tada-cache.d.ts (1)
141-142
: MissingupdatedAt
increateActionItemCategory
return typeDown-stream consumers often rely on
createdAt
andupdatedAt
. You already exposecreatedAt
; returningupdatedAt
keeps the payload symmetrical with other mutations (e.g.updateActionItem
).
[ suggest_nitpick ]test/graphql/types/documentNodes.ts (1)
985-1016
: Pagination arguments default to Relay spec – document your expectations
ActionItemCategoriesByOrganization
forwards$first/$last/$after/$before
, but many tests passnull
. For stricter test coverage pass concrete values or document which combination is allowed (e.g., forbid mixingfirst
&last
).
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
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: 5
♻️ Duplicate comments (5)
test/graphql/types/gql.tada.d.ts (1)
76-76
:allottedHours
scalar mismatch – inputs stillInt
while output isFloat
Same issue flagged in a previous review; the generator has not been updated. This will coerce or fail depending on resolver logic.Please regenerate the schema / update the inputs:
- allotedHours: { kind: 'SCALAR'; name: 'Int'; + allotedHours: { kind: 'SCALAR'; name: 'Float';Also check
assignedAt
(String
vsDateTime
) and optionality ofisCompleted
.Also applies to: 128-128
schema.graphql (4)
19-20
: Optional: Enforce non-nullcategory
onActionItem
.
Since every action item must belong to a category, consider changingcategory: ActionItemCategory
tocategory: ActionItemCategory!
to guarantee data integrity at the schema level.
3386-3396
: Refactor: Standardize pagination input foractionItemsByOrganization
.
TheactionItemsByOrganization
query uses inlineorganizationId: ID!
instead of aninput
object, while the connection types follow the paginated pattern. For API consistency and future filters, consider:actionItemsByOrganization( after: String before: String first: Int last: Int input: QueryActionItemsByOrganizationInput! ): QueryActionItemsByOrganizationConnectionAnd update
QueryActionItemsByOrganizationInput
toorganizationId: ID!
.Also applies to: 3521-3524, 3526-3529
2440-2453
: Optional Refactor: RenamecategoryId
toid
inMutationUpdateActionItemCategoryInput
.
For consistency with other update mutations (which useid: ID!
), consider renaming the field fromcategoryId
toid
.
1756-1787
:⚠️ Potential issueCritical: Inconsistent types for
allottedHours
andassignedAt
inputs.
The schema'sallottedHours
field isFloat
, but the input usesInt
, andassignedAt
isString
while the schema expectsDateTime
. Mismatched types can cause runtime errors and validation gaps.Apply this diff:
input MutationCreateActionItemInput { - allottedHours: Int + allottedHours: Float ... - assignedAt: String + assignedAt: DateTime }Ensure resolvers and Zod schemas are updated accordingly.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
schema.graphql
(12 hunks)src/createServer.ts
(0 hunks)src/envConfigSchema.ts
(0 hunks)src/graphql/types/ActionItemCategory/actionItemCategory.ts
(1 hunks)src/graphql/types/Query/ActionItemCategoriesByIds.ts
(1 hunks)src/graphql/types/Query/index.ts
(1 hunks)test/graphql/types/gql.tada.d.ts
(5 hunks)
💤 Files with no reviewable changes (2)
- src/createServer.ts
- src/envConfigSchema.ts
🧰 Additional context used
🧠 Learnings (1)
schema.graphql (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: src/typeDefs/inputs.ts:80-80
Timestamp: 2024-11-01T10:39:50.333Z
Learning: In `src/typeDefs/inputs.ts`, the `orgId` field in `ActionItemWhereInput` is intentionally optional (`ID`) and should not be changed to non-nullable (`ID!`).
⏰ 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 (11)
schema.graphql (11)
10-11
: Approve the newallottedHours
field onActionItem
.
This aligns with the database schema and supports fractional hour estimates, enhancing scheduling accuracy.
42-43
: Approve theorganization
association onActionItem
.
The non-nullableorganization: Organization!
field cleanly exposes the multi-tenant relationship in the GraphQL schema.
54-55
: Approve the addition ofupdater
onActionItem
.
Addingupdater: User
matches the audit pattern for other types and completes the update-tracking fields.
59-63
: Approve audit fields onActionItemCategory
.
IncludingcreatedAt: DateTime
andcreator: User
provides consistent creation metadata and aligns with patterns in other types.
74-75
: Approve theorganization
association onActionItemCategory
.
Linking toOrganization!
ensures proper multi-tenant separation and is consistent with other connected types.
1458-1462
: ApprovecreateActionItem
mutation signature.
Switching toMutationCreateActionItemInput
standardizes the API and integrates with the new resolver layer.
1463-1470
: Approve newcreateActionItemCategory
mutation.
AddingcreateActionItemCategory(input: MutationCreateActionItemCategoryInput!): ActionItemCategory
provides full CRUD support for categories in the GraphQL schema.
1645-1650
: Approve newupdateActionItemCategory
mutation.
IntroducingupdateActionItemCategory(input: MutationUpdateActionItemCategoryInput!): ActionItemCategory
completes the category management endpoints.
1738-1754
: ReviewMutationCreateActionItemCategoryInput
fields.
The input definesname: String!
, optionalisDisabled
, andorganizationId: ID!
. Ensure server-side validation enforces non-emptyname
and a valid UUID format fororganizationId
.
3375-3385
: Approve paginatedactionCategoriesByOrganization
query.
The Relay-style pagination usingfirst
,last
,before
,after
, combined withQueryActionCategoriesByOrganizationInput
and its connection/edge types, follows best practices and matches other paginated fields.Also applies to: 3501-3504, 3506-3509, 3514-3519
3409-3411
: ApprovecategoriesByIds
query and input.
The newcategoriesByIds(input: QueryCategoriesByIdsInput!): [ActionItemCategory!]
field, backed by the corresponding input type, efficiently supports batch-fetching of categories.Also applies to: 3571-3573
Signed-off-by: NishantSinghhhhh <[email protected]>
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: 17
🔭 Outside diff range comments (1)
src/drizzle/tables/actionItemCategories.ts (1)
107-148
: 🧹 Nitpick (assertive)Clear relation definitions with proper documentation.
The relations are well-defined with descriptive JSDoc comments, clearly explaining the one-to-many and many-to-one relationships. The field references are properly updated to use
actionItemCategoriesTable
instead of the previous naming.One inconsistency to note: the relation named
actionsWhereCategory
still uses the term "actions" while the rest of the codebase is moving to "actionItems" terminology.Consider renaming
actionsWhereCategory
toactionItemsWhereCategory
for complete naming consistency across the codebase:- actionsWhereCategory: many(actionItemsTable, { + actionItemsWhereCategory: many(actionItemsTable, {
♻️ Duplicate comments (35)
src/envConfigSchema.ts (1)
243-246
:⚠️ Potential issueEnvironment variable
API_REDIS_URI
needs to be made optional to prevent pipeline failuresThe change has reactivated
API_REDIS_URI
as a required environment variable, but based on the pipeline failures, this variable isn't set in all environments, causing the server initialization to fail.Make
API_REDIS_URI
optional until all environments are updated:- API_REDIS_URI: Type.String({ + API_REDIS_URI: Type.Optional(Type.String({ format: "uri", pattern: "^redis://.*", - }), + })),src/createServer.ts (2)
85-88
:⚠️ Potential issueAdd a fallback mechanism for Redis connection when URI is not available
The Redis connection now exclusively uses the
API_REDIS_URI
environment variable, but pipeline failures show this variable is missing in some environments. You need to implement a fallback to maintain backward compatibility.Implement a fallback to use host/port when URI is not available:
fastify.register(fastifyRedis, { - url: fastify.envConfig.API_REDIS_URI, + ...(fastify.envConfig.API_REDIS_URI + ? { url: fastify.envConfig.API_REDIS_URI } + : { + host: fastify.envConfig.API_REDIS_HOST, + port: fastify.envConfig.API_REDIS_PORT + }), closeClient: true, });
79-83
: 🧹 Nitpick (assertive)Remove commented-out code
The commented-out Redis configuration using host and port is no longer needed and should be removed to avoid confusion.
-// fastify.register(fastifyRedis, { -// host: fastify.envConfig.API_REDIS_HOST, -// port: fastify.envConfig.API_REDIS_PORT, -// closeClient: true, -// });src/graphql/inputs/MutationUpdateActionItemCategory.ts (4)
1-1
: Header comment doesn't match the actual filenameThe header comment says
// src/graphql/inputs/MutationUpdateActionItemCategoryInput.ts
but the actual file isMutationUpdateActionItemCategory.ts
.
Keeping these in sync avoids confusion during code search.
15-19
: Strengthen validation forname
to avoid empty or blank category namesUsers could currently pass
""
or whitespace.
Consider trimming and enforcing a minimum length:-name: z.string().optional(), // Optional: new name for the category +name: z.string().trim().min(1).optional(), // Optional: new name for the categoryThis keeps empty updates disallowed while still permitting omission.
40-44
: Align GraphQL field nullability with Zod schemaThe
name
field is optional in the Zod schema. For absolute consistency, make that explicit:name: t.string({ description: "Optional new name for the category.", required: false, + nullable: true, }),
Although Pothos treats non-required input fields as nullable, adding
nullable: true
improves SDL readability.
47-51
: Align GraphQL field nullability with Zod schemaThe
isDisabled
field is optional in the Zod schema. For absolute consistency, make that explicit:isDisabled: t.boolean({ description: "Optional flag to mark the category as disabled or enabled.", required: false, + nullable: true, }),
Although Pothos treats non-required input fields as nullable, adding
nullable: true
improves SDL readability.src/graphql/types/ActionItem/assignee.ts (1)
31-37
: Add test for missing assignee in resolverThe error handling has been simplified. Instead of throwing a
TalawaGraphQLError
when an assignee is not found, the code now logs a warning and returnsnull
. This is a more appropriate approach for GraphQL resolvers where null values are preferred over errors for missing references.However, according to Codecov, this code path is not covered by tests. Please add a test in
test/graphql/types/ActionItem/assignee.test.ts
, for example:test("Returns null and logs warning if assignee not found", async () => { const { ctx, mocks } = createMockCtx(); mocks.findFirst.mockResolvedValue(null); const result = await resolveAssignee( { assigneeId: "nonexistent" }, {}, ctx, ); expect(result).toBeNull(); expect(ctx.log.warn).toHaveBeenCalledWith( `Assignee with ID nonexistent not found for ActionItem.`, ); });This will ensure coverage of the new
ctx.log.warn(...)
+return null
path.src/graphql/types/ActionItemCategory/actionItemCategory.ts (2)
11-12
: Type alias and value share the same identifier – consider renaming the const
export type ActionItemCategory
(type space) and
export const ActionItemCategory
(value space) compile, but tooling & stack-traces can become confusing. Renaming the runtime reference keeps intent crystal-clear:-export const ActionItemCategory = - builder.objectRef<ActionItemCategory>("ActionItemCategory"); +export const ActionItemCategoryRef = + builder.objectRef<ActionItemCategory>("ActionItemCategory");Remember to update downstream imports (
ActionItemCategory
→ActionItemCategoryRef
) if you adopt this.
15-34
: Expose timestamps for full parity with DB model (optional)The DB table likely carries
createdAt
/updatedAt
. If these fields are intentionally withheld for privacy that's fine—otherwise consider exposing them; clients often need them for sorting or caches.(Simple add under
fields
)fields: (t) => ({ // ... existing fields + createdAt: t.expose("createdAt", { type: "DateTime" }), + updatedAt: t.expose("updatedAt", { type: "DateTime" }), }),src/graphql/types/ActionItem/updatedAt.ts (1)
2-2
: Unify GraphQLContext import path across resolvers.In this file you import
GraphQLContext
using a relative path (../../context
), whereas other resolvers in the codebase use the alias import ("~/src/graphql/context"
). For consistency and to leverage the path alias, consider updating this to:-import type { GraphQLContext } from "../../context"; +import type { GraphQLContext } from "~/src/graphql/context";src/graphql/types/ActionItemCategory/updatedAt.ts (1)
37-51
: 🧹 Nitpick (assertive)Consider optimizing the database query.
The database query fetches more data than strictly needed for the authorization check. Since you're only checking user roles, you could optimize the query by adding a limit to the organization memberships:
const currentUser = await ctx.drizzleClient.query.usersTable.findFirst({ columns: { role: true, }, with: { organizationMembershipsWhereMember: { columns: { role: true, }, where: (fields, operators) => operators.eq(fields.organizationId, parent.organizationId), + limit: 1, // We only need one record if we're just checking if any exist with admin role }, }, where: (fields, operators) => operators.eq(fields.id, currentUserId), });
However, if you implement the previous suggestion to check all memberships for the admin role, you should remove this limit.
src/graphql/types/ActionItem/updater.ts (2)
51-58
: Guard against missingorganizationMembershipsWhereMember
You’re still indexing[0]
without a safety check, which will throw when the user
has no membership in the target organisation. Wrap it with optional
chaining as previously suggested.-const currentUserOrganizationMembership = - currentUser.organizationMembershipsWhereMember[0]; +const currentUserOrganizationMembership = + currentUser.organizationMembershipsWhereMember?.[0];
88-90
: Log message omits the offendingupdaterId
Including the ID dramatically speeds up production debugging.
-ctx.log.error( - "Postgres select operation returned an empty array for an action item's updater id that isn't null.", -); +ctx.log.error( + `No user found for updaterId=${updaterId} while resolving ActionItem.updater`, +);src/graphql/types/Mutation/updateActionItem.ts (1)
208-211
:⚠️ Potential issue
null
cannot be written back – loss of informationWhen the client explicitly sets
allottedHours
tonull
(to clear it), the current logic turns it intoundefined
, which Drizzle interprets as "leave the column untouched".- allottedHours: - fieldsToUpdate.allottedHours != null - ? fieldsToUpdate.allottedHours.toString() - : undefined, + allottedHours: + fieldsToUpdate.allottedHours === undefined + ? undefined // field omitted + : fieldsToUpdate.allottedHours === null + ? null // explicit null + : fieldsToUpdate.allottedHours.toString(),This preserves the three states: unset, cleared, and updated.
src/graphql/types/Mutation/createActionItemCategory.ts (3)
86-90
: 🛠️ Refactor suggestionUse composable expression helpers instead of raw
sql
template for safetyEmbedding two
operators.eq
calls inside a raw SQL template string negates Drizzle's automatic parameterization and increases the chance of subtle SQL-injection / syntax issues.-where: (fields, operators) => - sql`${operators.eq(fields.memberId, currentUserId)} AND ${operators.eq( - fields.organizationId, - parsedArgs.input.organizationId, - )}`, +where: (fields, operators) => + operators.and( + operators.eq(fields.memberId, currentUserId), + operators.eq(fields.organizationId, parsedArgs.input.organizationId), + ),
101-115
: 🛠️ Refactor suggestionGlobal administrators locked out of the mutation
Only organization-level admins can create categories, but a site-wide administrator (
ctx.currentClient.user.role === "administrator"
) is denied.-if (userMembership.role !== "administrator") { +if ( + ctx.currentClient.user.role !== "administrator" && + userMembership.role !== "administrator" +) {This keeps current behavior while empowering trusted super-admins.
117-137
:⚠️ Potential issueUnique-constraint collisions not handled – surface a friendly error
Inserting a category with an existing
(name, organizationId)
pair will raise a Postgres error bubbling up as500
. The database schema has a unique index on these fields (seeuniqueIndex().on(self.name, self.organizationId)
in the actionItemCategories table definition), but the mutation doesn't handle this case.Wrap the insert in a try/catch block and translate
unique_violation
into aTalawaGraphQLError
with code"conflict_duplicate_resource"
so the client can react sensibly:+ try { const [createdCategory] = await ctx.drizzleClient .insert(actionItemCategoriesTable) .values({ id: uuidv7(), name: parsedArgs.input.name, organizationId: parsedArgs.input.organizationId, creatorId: currentUserId, isDisabled: parsedArgs.input.isDisabled ?? false, // Default to enabled createdAt: new Date(), updatedAt: new Date(), }) .returning(); // Step 7: Validate that the insert was successful if (!createdCategory) { ctx.log.error( "Postgres insert operation unexpectedly returned an empty array.", ); throw new TalawaGraphQLError({ extensions: { code: "unexpected" } }); } // Step 8: Return the newly created category return createdCategory; + } catch (error) { + // Check for unique constraint violation (name + organizationId) + if (error.code === '23505') { + throw new TalawaGraphQLError({ + message: "A category with this name already exists in the organization.", + extensions: { + code: "conflict_duplicate_resource", + issues: [{ argumentPath: ["input", "name"] }] + } + }); + } + // Re-throw other errors + throw error; + }src/graphql/types/ActionItem/createdAt.ts (1)
22-31
: 🧹 Nitpick (assertive)Repeated auth / role-check logic – extract to a shared helper
The authentication + “is admin (global OR org)” check in this block is identical to what we now have in several resolvers (
ActionItemCategory.createdAt
,ActionItem.event
, the new paginated queries, etc.). Copy-pasting the same logic increases maintenance cost and the risk of future drift.Consider moving the check into a utility such as
assertIsAdminForOrg(ctx, orgId)
that throws the appropriateTalawaGraphQLError
.
This keeps resolvers focused on business logic and makes unit-testing the rule far easier.src/graphql/types/ActionItem/event.ts (2)
82-85
:⚠️ Potential issueLeaking internal relation field (
attachmentsWhereEvent
) in GraphQL payload
Object.assign(existingEvent, { attachments: existingEvent.attachmentsWhereEvent })
adds the friendlyattachments
alias but leaves the originalattachmentsWhereEvent
property on the returned object.
That field is an internal ORM relation and should never reach the client; besides bloating the response, it exposes implementation details.- return Object.assign(existingEvent, { - attachments: existingEvent.attachmentsWhereEvent, - }); +const { attachmentsWhereEvent, ...eventWithoutRelation } = existingEvent; +return { + ...eventWithoutRelation, + attachments: attachmentsWhereEvent, +};
45-53
: 🧹 Nitpick (assertive)Auth helper duplication
Same admin-role check duplicated here – see earlier note in
createdAt.ts
. Extracting this toassertIsAdminForOrg
(or similar) would DRY-up the codebase.src/graphql/types/Query/ActionItembyOrganization.ts (1)
100-110
: 🛠️ Refactor suggestionCursor decoding can throw – wrap JSON.parse in try/catch
An invalid Base64/JSON cursor currently bubbles a raw
SyntaxError
, leaking stack traces. Guard it and surface a typed GraphQL error instead, as previously suggested in earlier reviews.src/graphql/types/Mutation/createActionItem.ts (3)
16-18
: 🧹 Nitpick (assertive)Rename constant to match the mutation it registers
createActionItemCategoryMutation
actually maps to thecreateActionItem
field, which is confusing for readers and IDE symbol search.-export const createActionItemCategoryMutation = builder.mutationField( +export const createActionItemMutation = builder.mutationField(
152-156
:⚠️ Potential issue
completionAt
initialised whileisCompleted
isfalse
– inconsistent stateCreating an incomplete action item with a non-null
completionAt
timestamp breaks “open vs. done” logic and future SLAs.- completionAt: new Date(), + completionAt: null,
158-161
: 🧹 Nitpick (assertive)Explicitly cast numeric
allottedHours
to string for clarityDrizzle maps
numeric
→string
. An explicit cast makes intent obvious and avoids locale traps:- ? parsedArgs.input.allottedHours.toString() + ? String(parsedArgs.input.allottedHours)src/graphql/types/ActionItemCategory/creator.ts (3)
23-31
: 🧹 Nitpick (assertive)Select only the columns you really need
Fetching the entire user row loads unnecessary PII into memory. Restrict the projection (e.g.
{ id: true, role: true }
) to minimise risk and improve performance.
42-45
: 🧹 Nitpick (assertive)Hard-coded
"administrator"
string – centralise role constantsThe literal role string is repeated across resolvers. Extracting a shared enum/const (e.g.
UserRole.ADMIN
) avoids typos and eases future policy tweaks.
70-74
: 🧹 Nitpick (assertive)Misleading log message & overly generic error
The error states “creator id that isn't null” even though
creatorId
is known. Use a clearer message and expose a dedicatedresource_not_found
error code so clients can react gracefully.src/drizzle/tables/actionItems.ts (1)
170-170
: Typo in exported constant name – plural/singular mix-up
actionsItemRelations
does not follow the surrounding naming convention
(actionItemsTable
). It should beactionItemRelations
or
actionItemsRelations
for clarity and IDE discoverability.Renaming now avoids a breaking change later.
src/graphql/inputs/MutationUpdateActionItemInput.ts (2)
5-5
: Inconsistent Zod import aliasUsing
import { z as zod }
is uncommon and was previously flagged. Prefer the
canonicalimport { z } from "zod"
(and drop the alias) orimport * as zod
for consistency across input files.
31-33
: Type mismatch:numeric
↔︎Int
—decimal hours will be truncatedThe DB layer allows fractional hours (
numeric
+zod.number()
), but the
GraphQL layer exposest.int()
. Clients sending1.5
will get a validation
error or lose precision.- allottedHours: t.int({ + allottedHours: t.float({ description: "Number of hours allotted for completion.", required: false, })Keep the scalar in sync with DB semantics and update downstream resolvers.
Also applies to: 78-81
src/graphql/types/Query/ActionItemCategorybyOrganization.ts (2)
122-133
: Cursor decoding still uses manual replace/"base64"
instead ofbase64url
A prior review suggested switching to the built-in
"base64url"
variant (Node
≥ 20) or extracting a shared helper. The manual character swap is brittle and
easy to forget in other resolvers.- Buffer.from( - (parsed.cursor as unknown as string) - .replace(/-/g, "+") - .replace(/_/g, "/"), - "base64", - ) + Buffer.from(parsed.cursor as string, "base64url")If you have to support older Node versions, factor this into a reusable utility
to keep both resolvers consistent.
190-210
:exists
sub-query adds cost without benefitThe extra
exists (…)
merely checks that the cursor row exists; if it doesn’t,
the outerselect
will return zero rows and the later guard already throws
arguments_associated_resources_not_found
.Removing the sub-query simplifies SQL and improves planner performance:
- whereCond = and( - base, - exists(/* redundant */), - or(/* cmp logic */) - ) + whereCond = and( + base, + or(/* cmp logic */) + )src/graphql/types/Query/ActionItemCategoriesByIds.ts (2)
32-36
:⚠️ Potential issueOnly authentication checked – missing authorization allows cross-org data leak
A signed-in user can fetch any category ID, even from an organisation they don't belong to. Add an authorization guard after authentication to ensure users can only access categories from organizations they belong to.
if (!ctx.currentClient.isAuthenticated) { throw new TalawaGraphQLError({ extensions: { code: "unauthenticated" }, }); } +// Authorization check +const userOrgs = ctx.currentClient.organizationIds; +if (!categories.every(c => userOrgs.includes(c.organizationId))) { + throw new TalawaGraphQLError({ + extensions: { code: "forbidden" }, + }); +}
52-52
: 🧹 Nitpick (assertive)Deduplicate IDs to avoid exceeding SQL placeholder limits
Postgres caps
IN (…)
to 32k placeholders; in practice, far fewer is wise. Remove duplicates before the query to shrink the array:-const ids = parsed.data.ids; +const ids = Array.from(new Set(parsed.data.ids)); // O(n)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (31)
src/createServer.ts
(1 hunks)src/drizzle/tables/actionItemCategories.ts
(4 hunks)src/drizzle/tables/actionItems.ts
(1 hunks)src/envConfigSchema.ts
(1 hunks)src/graphql/inputs/MutationCreateActionItem.ts
(1 hunks)src/graphql/inputs/MutationCreateActionItemCategory.ts
(1 hunks)src/graphql/inputs/MutationUpdateActionItemCategory.ts
(1 hunks)src/graphql/inputs/MutationUpdateActionItemInput.ts
(1 hunks)src/graphql/inputs/QueryActionCategoriesByOrganizationInput.ts
(1 hunks)src/graphql/inputs/QueryActionItemInput.ts
(2 hunks)src/graphql/types/ActionItem/actionItem.ts
(1 hunks)src/graphql/types/ActionItem/actionItemCategory.ts
(1 hunks)src/graphql/types/ActionItem/assignee.ts
(1 hunks)src/graphql/types/ActionItem/createdAt.ts
(2 hunks)src/graphql/types/ActionItem/creator.ts
(1 hunks)src/graphql/types/ActionItem/event.ts
(1 hunks)src/graphql/types/ActionItem/organization.ts
(1 hunks)src/graphql/types/ActionItem/updatedAt.ts
(6 hunks)src/graphql/types/ActionItem/updater.ts
(1 hunks)src/graphql/types/ActionItemCategory/actionItemCategory.ts
(1 hunks)src/graphql/types/ActionItemCategory/createdAt.ts
(1 hunks)src/graphql/types/ActionItemCategory/creator.ts
(1 hunks)src/graphql/types/ActionItemCategory/updatedAt.ts
(1 hunks)src/graphql/types/Mutation/createActionItem.ts
(1 hunks)src/graphql/types/Mutation/createActionItemCategory.ts
(1 hunks)src/graphql/types/Mutation/deleteActionItem.ts
(7 hunks)src/graphql/types/Mutation/updateActionItem.ts
(15 hunks)src/graphql/types/Query/ActionItemCategoriesByIds.ts
(1 hunks)src/graphql/types/Query/ActionItemCategorybyOrganization.ts
(1 hunks)src/graphql/types/Query/ActionItembyOrganization.ts
(1 hunks)src/graphql/types/Query/organizationUsers.ts
(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
src/graphql/types/ActionItem/event.ts (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: src/resolvers/Event/actionItems.ts:18-18
Timestamp: 2024-10-27T14:33:40.803Z
Learning: In `src/resolvers/Event/actionItems.ts`, the `event` field is required and should be used instead of `eventId` as per the schema.
src/graphql/types/ActionItem/createdAt.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".
src/graphql/types/ActionItem/updatedAt.ts (1)
Learnt from: shivasankaran18
PR: PalisadoesFoundation/talawa-api#3205
File: src/graphql/types/FundCampaign/updatedAt.ts:86-99
Timestamp: 2025-02-14T09:21:49.509Z
Learning: In role-based authorization checks for FundCampaign's updatedAt resolver, check all organization memberships for administrator role using Array.some() instead of accessing only the first membership.
src/graphql/types/ActionItem/actionItemCategory.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".
🧬 Code Graph Analysis (18)
src/graphql/types/ActionItem/assignee.ts (1)
src/graphql/context.ts (1)
GraphQLContext
(55-55)
src/graphql/inputs/MutationUpdateActionItemCategory.ts (1)
src/graphql/builder.ts (1)
builder
(10-20)
src/graphql/types/ActionItem/event.ts (3)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/graphql/types/ActionItem/actionItem.ts (2)
ActionItem
(8-8)ActionItem
(14-14)
src/drizzle/tables/actionItemCategories.ts (3)
src/drizzle/tables/users.ts (1)
usersTable
(59-216)src/drizzle/tables/organizations.ts (1)
organizationsTable
(31-128)src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(29-164)
src/graphql/types/ActionItemCategory/updatedAt.ts (2)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)
src/graphql/types/ActionItem/updater.ts (2)
src/graphql/types/ActionItem/actionItem.ts (2)
ActionItem
(8-8)ActionItem
(14-14)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)
src/graphql/inputs/QueryActionCategoriesByOrganizationInput.ts (1)
src/graphql/builder.ts (1)
builder
(10-20)
src/graphql/types/ActionItem/createdAt.ts (3)
src/graphql/types/ActionItemCategory/createdAt.ts (1)
resolveCreatedAt
(10-70)src/graphql/context.ts (1)
GraphQLContext
(55-55)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)
src/graphql/inputs/MutationCreateActionItemCategory.ts (1)
src/graphql/builder.ts (1)
builder
(10-20)
src/graphql/inputs/QueryActionItemInput.ts (1)
src/drizzle/tables/actionItems.ts (1)
actionsTableInsertSchema
(225-225)
src/graphql/types/Mutation/createActionItemCategory.ts (5)
src/graphql/builder.ts (1)
builder
(10-20)src/graphql/types/ActionItemCategory/actionItemCategory.ts (2)
ActionItemCategory
(6-8)ActionItemCategory
(11-12)src/graphql/inputs/MutationCreateActionItemCategory.ts (2)
MutationCreateActionItemCategoryInput
(25-55)mutationCreateActionItemCategoryArgumentsSchema
(61-63)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/drizzle/tables/actionItemCategories.ts (1)
actionItemCategoriesTable
(25-105)
src/graphql/types/ActionItemCategory/creator.ts (3)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/graphql/types/ActionItemCategory/actionItemCategory.ts (2)
ActionItemCategory
(6-8)ActionItemCategory
(11-12)
src/graphql/inputs/MutationCreateActionItem.ts (1)
src/graphql/builder.ts (1)
builder
(10-20)
src/graphql/types/Mutation/deleteActionItem.ts (3)
src/graphql/builder.ts (1)
builder
(10-20)src/graphql/inputs/MutationDeleteActionItemInput.ts (1)
mutationDeleteActionItemArgumentsSchema
(33-35)src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(29-164)
src/drizzle/tables/actionItems.ts (4)
src/drizzle/schema.ts (6)
actionItemsTable
(8-8)usersTable
(94-94)actionItemCategoriesTable
(5-5)eventsTable
(55-55)organizationsTable
(75-75)actionsItemRelations
(8-8)src/drizzle/tables/users.ts (1)
usersTable
(59-216)src/drizzle/tables/actionItemCategories.ts (1)
actionItemCategoriesTable
(25-105)src/drizzle/tables/organizations.ts (1)
organizationsTable
(31-128)
src/graphql/types/ActionItem/actionItemCategory.ts (4)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/graphql/types/ActionItemCategory/actionItemCategory.ts (2)
ActionItemCategory
(6-8)ActionItemCategory
(11-12)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/graphql/types/ActionItem/actionItem.ts (2)
ActionItem
(8-8)ActionItem
(14-14)
src/graphql/types/Query/ActionItemCategoriesByIds.ts (3)
src/graphql/builder.ts (1)
builder
(10-20)src/graphql/types/ActionItemCategory/actionItemCategory.ts (2)
ActionItemCategory
(6-8)ActionItemCategory
(11-12)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)
src/graphql/types/ActionItem/creator.ts (3)
src/graphql/context.ts (1)
GraphQLContext
(55-55)src/utilities/TalawaGraphQLError.ts (1)
TalawaGraphQLError
(264-277)src/graphql/types/ActionItem/actionItem.ts (2)
ActionItem
(8-8)ActionItem
(14-14)
🪛 GitHub Actions: Pull request workflow
src/createServer.ts
[error] 15-15: Error: env must have required property 'API_REDIS_URI'. The environment variable 'API_REDIS_URI' is missing, causing the server initialization to fail.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (34)
src/graphql/inputs/MutationUpdateActionItemCategory.ts (2)
25-53
: The implementation of the GraphQL input type looks goodThe structure and implementation of the GraphQL input type are well-organized with clear descriptions for each field.
60-62
: The arguments schema wrapper is correctly structuredThe mutation arguments schema correctly wraps the input schema to match the GraphQL mutation argument format.
src/graphql/types/ActionItem/assignee.ts (3)
8-8
: Good: Updated import path correctly uses lowercase filenameThe import path for ActionItem has been corrected to use the lowercase filename convention.
10-14
: Improved documentation with clear purpose and functionality descriptionThe added JSDoc comments clearly explain the purpose and functionality of the resolver, which improves code maintainability.
43-57
: Enhanced implementation of ActionItem with clear field extensionThe implementation of the ActionItem type extension is well-structured with improved documentation and a clear field description.
src/graphql/types/ActionItemCategory/actionItemCategory.ts (1)
15-34
: Well-structured GraphQL type definition with clear descriptionsThe implementation of the ActionItemCategory GraphQL object is well-organized with clear descriptions for each field, making the schema self-documenting.
src/graphql/types/ActionItemCategory/createdAt.ts (2)
10-70
: Excellent implementation of authentication and authorization in resolverThe resolver implementation includes:
- Proper authentication check for logged-in users
- Thorough authorization check for administrator roles
- Clear separation of concerns with stepped logic
- Comprehensive error handling
- Well-commented code explaining the purpose of each step
This follows best practices for GraphQL resolver security.
73-81
: Well-defined GraphQL field implementationThe GraphQL field implementation is properly typed with a clear description and correctly uses the resolver function. The DateTime type is appropriately specified.
src/graphql/types/ActionItem/updatedAt.ts (1)
15-79
: Comprehensive step-by-step comments improve code readability.The detailed step-by-step comments effectively document each part of the resolver's logic, making the code more maintainable and easier to understand. This approach helps future developers quickly grasp the authentication and authorization flow.
src/graphql/types/ActionItemCategory/updatedAt.ts (1)
5-95
: Well-structured resolver with comprehensive authentication and authorization checks.The resolver follows best practices by:
- Checking for required fields
- Verifying authentication
- Implementing proper authorization rules
- Handling errors appropriately with clear steps
The step-by-step comments also greatly improve code readability and maintainability.
src/graphql/types/ActionItem/actionItemCategory.ts (3)
18-48
: Well-implemented resolver with proper error handling.The
resolveCategory
resolver is well-implemented with:
- Clear type definitions
- Proper handling of null category IDs
- Comprehensive error handling with detailed error messages and logging
- Appropriate use of structured GraphQL errors with the correct error code
The resolver correctly follows the pattern of returning
null
for missing IDs and throwing appropriate errors when a category can't be found, which is a good practice.
55-64
: GraphQL type extension adds relationship between ActionItem and ActionItemCategory.The implementation properly extends the ActionItem GraphQL type with a category field that uses the resolver. The field is correctly marked as nullable with a descriptive comment, which aligns with GraphQL best practices.
13-17
: Clear documentation with JSDoc comments.The JSDoc comments for the resolver function clearly explain its purpose, including what it resolves and how it looks up the associated category. This makes the code more maintainable and helps other developers understand the function's role.
src/graphql/inputs/QueryActionItemInput.ts (3)
4-5
: Correctly updated import path for actionsTableInsertSchema.You've correctly updated the import path from "
/src/drizzle/tables/actions" to "/src/drizzle/tables/actionItems", which aligns with the table renaming in the database schema.
37-43
: Added wrapper schema for GraphQL query arguments convention.The new wrapper schema
queryActionItemsByOrganizationArgumentsSchema
follows the common GraphQL convention of wrapping input objects in aninput
field. This makes the schema more maintainable and consistent with GraphQL best practices.
10-40
: Enhanced documentation with numbered labels improves readability.The addition of detailed comments with numbered labels (1️⃣, 2️⃣, 3️⃣) for each component of the input schema structure is excellent. This makes it easy to understand the relationship between the Zod validation schema, Pothos GraphQL input type, and the wrapper schema.
src/drizzle/tables/actionItemCategories.ts (2)
1-20
: Good job on the clear import and documentation structure!The imports are well-organized with explanatory comments, properly separating Drizzle ORM utilities, UUIDv7, and related table imports. The renaming from
actionsTable
toactionItemsTable
in the imports aligns well with the overall migration of action item handling to GraphQL.
21-105
: Well-structured table definition with comprehensive JSDoc documentation.The table definition maintains the database table name "action_categories" while the export variable uses the more consistent name
actionItemCategoriesTable
. Each field has detailed JSDoc comments explaining its purpose, which greatly improves code maintainability. The primary key using UUIDv7 and index definitions with comments are well-implemented.src/graphql/types/ActionItem/actionItem.ts (3)
1-14
: Good type definition structure with clear documentation.The file properly imports the necessary types and defines
ActionItem
type by inferring it from the Drizzle table schema, which ensures type safety. The descriptive comments and object reference creation follow best practices.
16-60
: Well-structured GraphQL object implementation with comprehensive field descriptions.The implementation properly exposes various fields from the database with clear descriptions. All fields have appropriate types, nullability settings, and explanatory descriptions which improves API documentation for clients.
61-89
: Robust implementation of the allottedHours resolver.The custom resolver for
allottedHours
handles multiple edge cases well:
- Properly converts string values to floats
- Returns null for NaN values and invalid types
- Validates numeric inputs
This ensures consistent and safe handling of the allotted hours data regardless of how it's stored in the database.
src/graphql/inputs/QueryActionCategoriesByOrganizationInput.ts (3)
1-17
: Well-implemented input validation schema with proper error message.The Zod schema correctly validates that the
organizationId
is a valid UUID string and provides a clear error message. This ensures robust input validation for the GraphQL query.
19-38
: Clear GraphQL input type definition with appropriate field descriptions.The input type is well-defined using the builder pattern with descriptive comments. The
organizationId
field is correctly marked as required and has a detailed description explaining its purpose.
39-47
: Standard argument schema wrapper follows GraphQL conventions.The wrapper schema correctly follows GraphQL conventions by placing the input schema under a top-level
input
field, ensuring consistency with standard GraphQL mutation and query argument patterns.src/graphql/inputs/MutationCreateActionItemCategory.ts (3)
1-20
: Well-defined validation schema with appropriate constraints.The Zod schema properly validates:
name
as a non-empty string with a meaningful error messageorganizationId
as a valid UUIDisDisabled
as an optional booleanThis ensures robust input validation for the mutation.
21-55
: Comprehensive GraphQL input type implementation with detailed field descriptions.The input type is well-implemented using Pothos's inputRef pattern with thorough documentation. Each field has appropriate type definitions, required flags, and clear descriptions that help API consumers understand the expected inputs.
57-64
: Standard argument schema wrapper follows GraphQL conventions.The wrapper schema correctly follows GraphQL conventions by placing the input schema under a top-level
input
field, ensuring consistency with standard GraphQL mutation argument patterns.src/graphql/types/Query/organizationUsers.ts (1)
1-109
: Clean code refactoring with improved error handling and validationThe refactoring provides enhanced documentation, proper authentication checks, and structured error handling using Zod for input validation. Both queries now follow a consistent pattern and include helpful comments explaining each step of the process.
src/graphql/types/Mutation/updateActionItem.ts (2)
2-3
: Good replacement of table references to align with new schemaThe import statements correctly reference the renamed tables and types, maintaining consistency with the schema changes throughout the codebase.
Also applies to: 4-4
150-175
: Proper renaming from categoryId to actionItemCategoryIdThe code now correctly references
actionItemCategoryId
instead ofcategoryId
, aligning with the schema changes made throughout the codebase.src/graphql/inputs/MutationCreateActionItem.ts (1)
1-104
: Well-structured input type with comprehensive validationThe new input type for creating action items is well-designed with:
- Clear documentation for each field
- Proper validation rules using Zod (e.g., UUID format, non-negative integers)
- Descriptive GraphQL field definitions with appropriate required flags
- Consistent organization following the established pattern in the codebase
The
allottedHours
field is properly validated as a non-negative integer with appropriate description.src/graphql/types/Query/ActionItembyOrganization.ts (1)
239-241
:❌ Incorrect review comment
Complexity multiplier can explode
(args.first ?? args.last ?? 1) + 1
allows clients to bypass the server-sidelimit
cap indefaultGraphQLConnectionArgumentsSchema
by requesting a hugefirst
.
Consider clamping the multiplier to the same maximum allowedlimit
to defend against denial-of-service queries.
🏁 Script executed:
#!/bin/bash # Locate the definition of defaultGraphQLConnectionArgumentsSchema rg "defaultGraphQLConnectionArgumentsSchema" -n # Show surrounding lines to inspect limit enforcement rg "defaultGraphQLConnectionArgumentsSchema" -A10Length of output: 63221
🏁 Script executed:
#!/bin/bash # Print the first 100 lines of defaultGraphQLConnection.ts to inspect schema constraints sed -n '1,100p' src/utilities/defaultGraphQLConnection.tsLength of output: 3100
Zod schema already caps “first”/“last” at 32, so multiplier can’t explode
ThedefaultGraphQLConnectionArgumentsSchema
enforces.max(32)
on bothfirst
andlast
, meaning(args.first ?? args.last ?? 1) + 1
will never exceed 33. No additional clamping is needed.Likely an incorrect or invalid review comment.
src/graphql/types/ActionItem/creator.ts (1)
15-23
:❌ Incorrect review comment
Defensive check for
ctx.currentClient.user
is missing
isAuthenticated
almost always implies thatctx.currentClient.user
is set, but the contract is not enforced in the type system. Accessingctx.currentClient.user.id
could throw if a future middleware inadvertently setsisAuthenticated = true
without a user object (or with a partially-hydrated one coming from a cache).- const currentUserId = ctx.currentClient.user.id; + // Fail fast if the user object is unexpectedly undefined + if (!ctx.currentClient.user) { + throw new TalawaGraphQLError({ + extensions: { code: "unauthenticated" }, + }); + } + + const currentUserId = ctx.currentClient.user.id;
I’ll locate how
ctx.currentClient
and itsuser
field are typed to confirm whether a runtime null-check is needed. Running some searches now.
Running searches to locate the context and
CurrentClient
definitions for verification:
🏁 Script executed:
#!/bin/bash # Locate GraphQL context interface rg "export interface .*Context" -n src # Locate CurrentClient type/interface rg "export (interface|type) CurrentClient" -n srcLength of output: 146
🏁 Script executed:
#!/bin/bash # Show surrounding lines of CurrentClient definition rg "export type CurrentClient" -A 10 -B 5 src/graphql/context.tsLength of output: 394
Gathering the full
CurrentClient
definition and any references touser
:
🏁 Script executed:
#!/bin/bash # Print full CurrentClient union rg -n -A 200 "export type CurrentClient" src/graphql/context.ts # Find any 'user' references in the context file rg -n "user" src/graphql/context.tsLength of output: 1507
Ignore unnecessary runtime null-check for
ctx.currentClient.user
The
CurrentClient
type is a discriminated union—after yourif (!ctx.currentClient.isAuthenticated) throw…
check, TypeScript narrows to the branch whereisAuthenticated: true
anduser
is guaranteed. Adding a redundant runtime guard isn’t needed, and it’s inconsistent with how other resolvers rely on TS for this assurance.Likely an incorrect or invalid review comment.
src/graphql/types/Query/ActionItemCategoriesByIds.ts (1)
15-74
: Comprehensive GraphQL resolver implementationThis implementation follows good practices by:
- Including thorough input validation
- Using proper error handling with specific error codes
- Providing clear type definitions and descriptions
- Following the project's established resolver patterns
The query fits well with the broader refactor of action item handling described in the PR.
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
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
🔭 Outside diff range comments (3)
src/graphql/types/Mutation/updateActionItem.ts (2)
116-140
:⚠️ Potential issuePost-completion notes check uses the old completion status
validateActionItemTypeConstraints
is invoked withexistingActionItem.isCompleted
; therefore, if the client flipsisCompleted
fromfalse → true
in the same request, post-completion notes are not enforced. This allows creating “completed” items without mandatory notes.- const validationIssues = validateActionItemTypeConstraints( - existingActionItem.isCompleted, - parsedArgs.input, - ); +const newIsCompleted = + isNotNullish(parsedArgs.input.isCompleted) + ? parsedArgs.input.isCompleted + : existingActionItem.isCompleted; + +const validationIssues = validateActionItemTypeConstraints( + newIsCompleted, + parsedArgs.input, +);Without this fix, business rules for completed items can be bypassed.
165-174
: 🧹 Nitpick (assertive)Wrong
argumentPath
when category is missingThe error object still reports
["input", "categoryId"]
, but the field was renamed toactionItemCategoryId
. This misleads API consumers and automated clients.- argumentPath: ["input", "categoryId"], + argumentPath: ["input", "actionItemCategoryId"],schema.graphql (1)
3525-3528
:⚠️ Potential issueFix
QueryActionItemsByOrganizationInput
type mismatchThe field
organizationId
is declared asString!
but should beID!
to match other query inputs and the underlying model.input QueryActionItemsByOrganizationInput { - organizationId: String! + organizationId: ID! }
♻️ Duplicate comments (6)
src/graphql/types/Mutation/updateActionItem.ts (1)
208-211
:null
cannot be cleared – same issue raised earlierThe
allottedHours
mapping still collapses an explicitnull
toundefined
, making it impossible for a client to clear the value (see prior review).test/graphql/types/gql.tada.d.ts (3)
75-75
:allottedHours
should beFloat
, notInt
The runtime field accepts decimals (
numeric
in DB /Float
in schema). KeepingInt
here can break clients that send fractional hours.
127-127
:MutationUpdateActionItemInput.allottedHours
also usesInt
Same scalar mismatch as above; both create & update inputs must align with
Float
.
187-194
: 🛠️ Refactor suggestionInconsistent ID scalar for organization in action-item query
QueryActionItemsByOrganizationInput.organizationId
is stillString
, while the new category query correctly usesID
(lines 190-191). Standardise onID
to avoid confusion and coercion issues.- 'organizationId'; type: { kind: 'NON_NULL'; ... 'String' + 'organizationId'; type: { kind: 'NON_NULL'; ... 'ID'schema.graphql (2)
62-63
: Optional: Makecreator
non-nullable for ActionItemCategoryTo guarantee every category has an author, you could enforce
creator: User!
instead of allowing null.
1749-1780
: Critical: Inconsistent input types forallottedHours
andassignedAt
The
MutationCreateActionItemInput
defines:
allottedHours: Int
but the schema type isFloat
assignedAt: String
but the schema type isDateTime
These mismatches will cause type conflicts. Please update the input fields to match the schema (e.g.,
allottedHours: Float
,assignedAt: DateTime
).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
schema.graphql
(12 hunks)src/createServer.ts
(0 hunks)src/envConfigSchema.ts
(0 hunks)src/graphql/types/Mutation/updateActionItem.ts
(11 hunks)test/graphql/types/gql.tada.d.ts
(5 hunks)
💤 Files with no reviewable changes (2)
- src/createServer.ts
- src/envConfigSchema.ts
🧰 Additional context used
🧠 Learnings (1)
schema.graphql (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: src/typeDefs/inputs.ts:80-80
Timestamp: 2024-11-01T10:39:50.333Z
Learning: In `src/typeDefs/inputs.ts`, the `orgId` field in `ActionItemWhereInput` is intentionally optional (`ID`) and should not be changed to non-nullable (`ID!`).
🧬 Code Graph Analysis (1)
src/graphql/types/Mutation/updateActionItem.ts (4)
src/graphql/inputs/MutationUpdateActionItemInput.ts (1)
mutationUpdateActionItemArgumentsSchema
(89-91)src/utilities/isNotNullish.ts (1)
isNotNullish
(12-14)src/drizzle/tables/actionItems.ts (1)
actionItemsTable
(29-164)src/drizzle/schema.ts (1)
actionItemsTable
(8-8)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Check App Startup and Health
- GitHub Check: Test Deployment to https://docs-api.talawa.io
🔇 Additional comments (14)
schema.graphql (14)
10-11
: Use Float forallottedHours
– VerifiedThe new
allottedHours: Float
field onActionItem
correctly supports fractional hour estimates as intended.
42-43
: Good: Non-nullable organization referenceThe
organization: Organization!
field correctly enforces that eachActionItem
belongs to an organization.
54-55
: Approveupdater
reference on ActionItemAdding the
updater: User
field enhances auditability by tracking who last modified the action item.
74-75
: Good: Non-nullable organization on ActionItemCategoryThe
organization: Organization!
field ensures categories are scoped to an organization, preventing orphaned entries.
1454-1457
: ApprovecreateActionItem
mutation signatureThe signature
createActionItem(input: MutationCreateActionItemInput!): ActionItem
follows the established input-object pattern and aligns with other mutations.
1459-1466
: ApprovecreateActionItemCategory
mutationThe new
createActionItemCategory(input: MutationCreateActionItemCategoryInput!): ActionItemCategory
mutation is well-structured and consistent with GraphQL conventions.
1731-1747
: ApproveMutationCreateActionItemCategoryInput
The input object includes required
name
andorganizationId
fields and an optionalisDisabled
flag—this design covers the mutation requirements.
3368-3378
: ApproveactionCategoriesByOrganization
queryThe paginated
actionCategoriesByOrganization(input: QueryActionCategoriesByOrganizationInput!)
query follows Relay-style conventions and cleanly encapsulates the organization filter.
3402-3404
: ApprovecategoriesByIds
queryThe new bulk-fetch query
categoriesByIds(input: QueryCategoriesByIdsInput!): [ActionItemCategory!]
is useful and adheres to the input-object style.
3494-3497
: ApproveQueryActionCategoriesByOrganizationConnection
typeThe connection type with
edges
andpageInfo
is correctly defined for paginated category results.
3499-3502
: ApproveQueryActionCategoriesByOrganizationConnectionEdge
The edge type properly exposes
cursor: String!
andnode: ActionItemCategory
, fitting Relay conventions.
3504-3512
: ApproveQueryActionCategoriesByOrganizationInput
The input object with a non-nullable
organizationId: ID!
cleanly encapsulates the filter criteria.
3514-3517
: ApproveQueryActionItemsByOrganizationConnection
typeThis paginated connection type accurately defines
edges
andpageInfo
for action items.
3519-3522
: ApproveQueryActionItemsByOrganizationConnectionEdge
The edge type correctly includes
cursor: String!
andnode: ActionItem
.
Please fix the failing tests. |
@meetulr sir Changed file names ActionItemCategory ----> ActionItemCategorybyOrganization.ts also converted these queries into paginated queries Added comments in all files I have added or edited I have made sure that this PR follow the way we write code in other queries and mutation |
What kind of change does this PR introduce?
Feature: Migrates ActionItem Category handling from Express endpoints to GraphQL schema and resolvers.
Issue Number: #3320
Snapshots/Videos:
N/A (No UI changes; these changes affect backend GraphQL schemas and resolvers.)
If relevant, did you update the documentation?
I have added/updated relevant documentation within the code and README where needed.
Summary
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Tests