Skip to content

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

Open
wants to merge 13 commits into
base: develop-postgres
Choose a base branch
from

Conversation

NishantSinghhhhh
Copy link

@NishantSinghhhhh NishantSinghhhhh commented Apr 22, 2025

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

    • Added support for creating and updating action item categories via new GraphQL mutations.
    • Introduced paginated queries for fetching action items and action item categories by organization.
    • Action items now include an "allotted hours" field to represent estimated completion time.
    • Action item and category types now expose related entities (e.g., organization, creator, updater) as nested objects instead of flat ID fields.
    • Added detailed error handling and authorization checks to resolvers for action items and categories.
    • Added a new resolver to fetch action item categories associated with action items.
  • Bug Fixes

    • Corrected and unified references to action item and category tables throughout the application.
  • Refactor

    • Replaced scalar ID fields in GraphQL types with object references for improved clarity and data access.
    • Updated input types and schemas for mutations and queries to support new features and relationships.
    • Extracted resolver logic into standalone functions with explicit typing and improved error handling.
    • Simplified error handling in assignee resolver to return null instead of throwing.
    • Reorganized imports and exports to remove redundant re-exports and improve clarity.
  • Chores

    • Updated environment configuration for Redis connection handling.
    • Migrated and enhanced sample data and test data to align with new schema.
    • Updated GraphQL document nodes and type declarations to reflect schema changes.
  • Tests

    • Added comprehensive unit and integration tests for new resolvers, mutations, and pagination logic.
    • Removed obsolete tests related to deprecated queries and input types.
    • Added tests covering authentication, authorization, error handling, and data retrieval for action items and categories.

Copy link

coderabbitai bot commented Apr 22, 2025

Walkthrough

This 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 allottedHours field to action items, and introduces new input types and mutations for managing action item categories. Pagination is implemented for queries returning action items and categories by organization. The codebase also updates related resolvers, authorization logic, and tests to align with the new structure, while removing obsolete queries and tests related to the previous schema.

Changes

File(s) / Group Change Summary
drizzle_migrations/20250422182535_moaning_pandemic.sql, drizzle_migrations/meta/_journal.json Added migration to introduce allotted_hours column to actions table and updated migration journal.
schema.graphql Refactored ActionItem and ActionItemCategory types to use object references instead of ID fields, added allottedHours, new input types, mutations, and paginated queries for action items/categories.
scripts/dbManagement/helpers.ts, scripts/dbManagement/sample_data/action_items.json, test/scripts/dbManagement/helpers.test.ts Updated references and sample data to use new table names and fields (actionItemsTable, actionItemCategoriesTable, allottedHours).
src/drizzle/schema.ts, src/drizzle/tables/actionItemCategories.ts, src/drizzle/tables/actionItems.ts, src/drizzle/tables/organizations.ts, src/drizzle/tables/users.ts Renamed tables and relations from actionsTable/actionCategoriesTable to actionItemsTable/actionItemCategoriesTable, updated relations, and added allottedHours field.
src/graphql/inputs/MutationCreateActionItem.ts, src/graphql/inputs/MutationCreateActionItemCategory.ts, src/graphql/inputs/MutationDeleteActionItemInput.ts, src/graphql/inputs/MutationUpdateActionItemCategory.ts, src/graphql/inputs/MutationUpdateActionItemInput.ts, src/graphql/inputs/QueryActionCategoriesByOrganizationInput.ts, src/graphql/inputs/QueryActionItemInput.ts Added and updated input validation schemas and GraphQL input types for new/modified mutations and queries.
src/graphql/types/ActionItem/actionItem.ts, src/graphql/types/ActionItem/actionItemCategory.ts, src/graphql/types/ActionItem/assignee.ts, src/graphql/types/ActionItem/createdAt.ts, src/graphql/types/ActionItem/creator.ts, src/graphql/types/ActionItem/event.ts, src/graphql/types/ActionItem/organization.ts, src/graphql/types/ActionItem/updatedAt.ts, src/graphql/types/ActionItem/updater.ts Refactored resolvers and GraphQL fields to match new schema, removed ID fields, added object references and allottedHours, updated error handling and authorization.
src/graphql/types/ActionItem/index.ts Removed re-exports; now only imports modules for side effects.
src/graphql/types/ActionItemCategory/actionItemCategory.ts, src/graphql/types/ActionItemCategory/createdAt.ts, src/graphql/types/ActionItemCategory/creator.ts, src/graphql/types/ActionItemCategory/organization.ts, src/graphql/types/ActionItemCategory/updatedAt.ts, src/graphql/types/ActionItemCategory/index.ts Updated category resolvers to use new table, removed ID fields, added object references, and implemented new resolvers for organization and creator.
src/graphql/types/Mutation/createActionItem.ts, src/graphql/types/Mutation/createActionItemCategory.ts, src/graphql/types/Mutation/deleteActionItem.ts, src/graphql/types/Mutation/index.ts, src/graphql/types/Mutation/updateActionItem.ts, src/graphql/types/Mutation/updateActionItemCategory.ts Refactored mutations to use new input types, updated table references, added/updated mutations for action item categories, and improved validation and authorization.
src/graphql/types/Query/ActionItem.ts Removed old actionItemsByOrganization query.
src/graphql/types/Query/ActionItemCategorybyOrganization.ts, src/graphql/types/Query/ActionItembyOrganization.ts Added new paginated queries for fetching action item categories and items by organization with Relay-style connections and authorization checks.
src/graphql/types/Query/index.ts Updated imports to reference new query modules for action items and categories by organization.
src/graphql/types/Query/organizationUsers.ts Removed eventsByOrganizationId query and related types.
src/createServer.ts, src/envConfigSchema.ts Removed commented Redis URI plugin registration; commented out Redis URI env var schema property.
test/graphql/types/ActionItem/assignee.test.ts Removed test for error-throwing on missing assignee.
test/graphql/types/ActionItem/categoryId.test.ts, test/graphql/types/ActionItem/createdAt.test.ts, test/graphql/types/ActionItem/creator.test.ts, test/graphql/types/ActionItem/event.test.ts, test/graphql/types/ActionItem/organization.test.ts Added/updated unit tests for new/updated ActionItem resolvers.
test/graphql/types/ActionItemCategory/createdAt.test.ts, test/graphql/types/ActionItemCategory/creator.test.ts, test/graphql/types/ActionItemCategory/organization.test.ts Added/updated tests for ActionItemCategory resolvers.
test/graphql/types/Mutation/createActionItemCategory.test.ts Added integration tests for createActionItemCategory mutation covering authentication and authorization.
test/graphql/types/Query/ActionCategory.test.ts, test/graphql/types/Query/ActionItem.test.ts, test/graphql/types/Query/Events.test.ts Removed obsolete test suites for old queries.
test/graphql/types/Query/organizationUsers.test.ts Removed tests related to deleted eventsByOrganizationId query.
test/graphql/types/documentNodes.ts, test/graphql/types/gql.tada-cache.d.ts, test/graphql/types/gql.tada.d.ts Updated GraphQL operation documents and type declarations to match new schema, input types, and nested object references.

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)
Loading

Possibly related PRs

  • No mongo action items #3385: The main PR adds a new numeric column allotted_hours to the actions table (renamed to actionItemsTable), updates the GraphQL schema to include allottedHours in ActionItem, and refactors related table and resolver references accordingly, while the retrieved PR modifies the completion_at column in the same actions table to allow null values and introduces the initial GraphQL type and resolvers for ActionItem without the allottedHours field; thus, both PRs modify the same database table and GraphQL type but focus on different schema fields and aspects of the ActionItem entity.

Suggested labels

Ready 4 PR Review

Suggested reviewers

  • meetulr
  • palisadoes
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Our Pull Request Approval Process

Thanks for contributing!

Testing Your Code

Remember, your PRs won't be reviewed until these criteria are met:

  1. We don't merge PRs with poor code quality.
    1. Follow coding best practices such that CodeRabbit.ai approves your PR.
  2. We don't merge PRs with failed tests.
    1. When tests fail, click on the Details link to learn more.
    2. Write sufficient tests for your changes (CodeCov Patch Test). Your testing level must be better than the target threshold of the repository
    3. Tests may fail if you edit sensitive files. Ask to add the ignore-sensitive-files-pr label if the edits are necessary.
  3. We cannot merge PRs with conflicting files. These must be fixed.

Our policies make our code better.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

  1. PRs with issues not assigned to you will be closed by the reviewer
  2. Fix the first comment in the PR so that each issue listed automatically closes

Other

  1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
  2. Read the CONTRIBUTING.md file make

Copy link

codecov bot commented Apr 22, 2025

Codecov Report

Attention: Patch coverage is 62.29692% with 673 lines in your changes missing coverage. Please review.

Project coverage is 59.35%. Comparing base (33d5b7b) to head (19cc704).

Files with missing lines Patch % Lines
...ql/types/Query/ActionItemCategorybyOrganization.ts 25.85% 152 Missing ⚠️
...rc/graphql/types/Query/ActionItembyOrganization.ts 26.36% 148 Missing ⚠️
...graphql/types/Mutation/updateActionItemCategory.ts 20.00% 88 Missing ⚠️
src/graphql/types/ActionItemCategory/updatedAt.ts 2.77% 70 Missing ⚠️
src/graphql/types/ActionItem/updater.ts 17.94% 64 Missing ⚠️
src/graphql/types/Mutation/createActionItem.ts 56.94% 62 Missing ⚠️
...c/graphql/types/Query/ActionItemCategoriesByIds.ts 43.85% 32 Missing ⚠️
...graphql/types/Mutation/createActionItemCategory.ts 84.95% 17 Missing ⚠️
src/graphql/types/ActionItem/actionItem.ts 80.39% 10 Missing ⚠️
src/graphql/types/Mutation/updateActionItem.ts 38.46% 8 Missing ⚠️
... and 8 more
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitai coderabbitai bot left a 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 issue

Pipeline 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 suggestion

Remove 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 issue

File has incorrect double .ts.ts extension.

The filename has a duplicated .ts extension, which should be corrected to just categoryId.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 tests

src/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 in MutationUpdateActionItemInput
See the earlier comment on lines 10–11 regarding the mismatch between Float in the schema and Int in the input.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 33d5b7b and c58fdca.

📒 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 model

This 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.5

The 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 file

The 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 added

These 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 resolvers

The 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 resolvers

The imports have been updated to include three new resolvers for relationship fields:

  • updater: Resolves the user who last updated the action item
  • categoryId.ts: Resolves the category of the action item
  • organization: Resolves the organization the action item belongs to

These 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 actionItemsTable

Good update to the relation definition, referencing the renamed actionItemsTable while preserving the existing relation name for database compatibility.


142-144: Relation updated to reference actionItemCategoriesTable

Good 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 previous actionCategoriesTable. 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 via rg "actionsTable" --type ts src/graphql/types/Query/, confirming the query resolver correctly uses actionItemsTable.

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 and actionsTable to actionItemCategoriesTable and actionItemsTable 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 new actionItemCategoriesTable in the insertCollections 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 new actionItemsTable in the insertCollections 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:

  1. Verifies user authentication
  2. Checks if the user is a global administrator
  3. 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:

  1. Tests that the correct error type is thrown
  2. Verifies the error message and code match expectations
  3. 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:

  1. An appropriate error is thrown when organizationId is null
  2. The error has the expected "unexpected" code
  3. The correct error log is generated
  4. No database query is attempted

This comprehensively covers the null input scenario.


54-72: Comprehensive test for organization not found scenario.

The test verifies:

  1. The correct error type is thrown
  2. The error has the expected code
  3. Proper error logging with the specific organizationId
  4. 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 renaming

The import path has been correctly updated to align with the table renaming from actions to actionItems, maintaining consistency with the broader refactoring.


5-7: Improved documentation with numeric labels

Adding the numeric emoji (1️⃣) improves readability and creates a clear visual hierarchy in the documentation.


12-14: Enhanced documentation with numeric labels

Adding the numeric emoji (2️⃣) continues the documentation pattern established earlier, improving readability.


29-34: New wrapper schema standardizes input argument structure

This 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 imports

The 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 mocks

The 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 mocking

The 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 tested

This test correctly verifies that the resolver rejects unauthenticated requests with the appropriate error code.


53-59: User record validation properly tested

This test correctly verifies that the resolver rejects requests when the user record cannot be found.


61-71: Authorization validation properly tested

This 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 tested

This 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 tested

This test verifies that:

  1. The resolver successfully returns an event when all checks pass
  2. 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 renaming

The import has been correctly updated from ./actions to ./actionItems to maintain consistency with the table renaming.


16-16: Table name updated for consistency

The table export has been renamed from actionCategoriesTable to actionItemCategoriesTable for better naming consistency and to reflect the relationship with action items more clearly.


68-70: Relations export renamed for consistency

The relations export has been updated from actionCategoriesRelations to actionItemCategoriesRelations to match the table renaming.


71-73: Relation field updated to match renamed table

The relation reference has been updated from actionsTable to actionItemsTable while maintaining the original database relation name.


75-77: Field reference updated to use renamed table

The fields property now correctly references actionItemCategoriesTable.creatorId instead of the previous table name.


81-83: Field reference updated to use renamed table

The fields property now correctly references actionItemCategoriesTable.organizationId instead of the previous table name.


87-89: Field reference updated to use renamed table

The fields property now correctly references actionItemCategoriesTable.updaterId instead of the previous table name.

src/drizzle/schema.ts (1)

5-7: Table and relations exports renamed for consistency

The exports have been updated from actionCategoriesTable and actionCategoriesRelations to actionItemCategoriesTable and actionItemCategoriesRelations 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 obsolete actionCategoriesTable.


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 scalar organizationId 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
The resolveOrganization resolver is already exercised in test/graphql/types/ActionItem/organization.test.ts. That suite mocks the Drizzle client’s findFirst method to cover:

  • organizationId null → throws TalawaGraphQLError
  • valid organizationId but no record → throws TalawaGraphQLError
  • valid organizationId and record found → returns the expected organization

No 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 tests

test/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:

  1. Non-existent user
  2. Regular user with no admin privileges
  3. User with global administrator role
  4. 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 (new allotted_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 named graphqlLimits.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‑cases

The NaN‑guard and string‑to‑float parsing are solid.
Please add tests for:

  • numeric column returned as string ("3.5"3.5)
  • explicit 00
  • invalid string ("abc") ➜ null
  • nullnull

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 tests

src/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 the z as zod alias keeps the import list tight and prevents name‑clashes.


20-22: allottedHours is allowed to be null and undefined – verify intent
Because the column is nullable, accepting null is fine, but accepting undefined 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 the ignore-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 a Float. Either:

  1. declare the column with explicit precision/scale and cast to number when exposing via GraphQL, or
  2. 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 changes

CI flagged this file as “Unauthorized changes were made in this file”.
Ensure the ignore-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 type

The 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:

  1. Authentication check to verify the client is authenticated
  2. Fetching the current user and their membership in the organization
  3. Authorization check to ensure the user is an administrator (global or organization-level)
  4. Handling null creatorId case
  5. Performance optimization by returning the current user if they are the creator
  6. 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.ts

Length 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 returns null
    • 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 tests

test/graphql/types/documentNodes.ts (6)

883-903: Updated CreateActionItem mutation with nested object references

The mutation document has been successfully updated to:

  1. Use the new input type MutationCreateActionItemInput
  2. Include the new allottedHours field in the selection set
  3. 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 field

The mutation now correctly includes:

  1. The new allottedHours field
  2. Nested object references instead of flat IDs
  3. 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 references

The mutation has been correctly updated to include:

  1. The new allottedHours field
  2. Nested object references for category, assignee, and organization
  3. Proper structure for selection sets

This change is well-aligned with the overall schema updates.


940-951: New ActionItemsByOrganization query for categories

This new query supports fetching action item categories by organization, including:

  1. Basic category fields (id, createdAt)
  2. 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 objects

This updated query now includes:

  1. Complete field selection for action items (isCompleted, notes, timestamps, etc.)
  2. The new allottedHours field
  3. Nested object references for all related entities

This query provides the necessary data access for the migrated functionality.


1045-1059: New CreateActionItemCategory mutation

This new mutation supports the creation of action item categories with:

  1. Basic category fields (name, isDisabled)
  2. Appropriate timestamps
  3. 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 mutation

The type declaration now properly reflects:

  1. The correct input type MutationCreateActionItemInput
  2. The inclusion of the new allottedHours field as a nullable number
  3. 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 mutation

The type definition correctly includes:

  1. The new allottedHours field
  2. Nested object types for category, assignee, and updater
  3. Proper nullable configuration

This ensures consistent type checking for tests using this mutation.


121-122: Updated type declaration for DeleteActionItem mutation

The type declaration properly includes:

  1. The allottedHours field
  2. Nested object types for related entities
  3. Consistent null handling

This completes the type safety for action item mutation operations.


123-124: Added type declaration for ActionItemsByOrganization query

The type declaration supports fetching action item categories by organization with:

  1. Basic fields like id and createdAt
  2. 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 query

The type declaration includes:

  1. All action item fields including the new allottedHours
  2. Nested object types for all related entities
  3. Proper nullable configurations

This provides complete type safety for the query used to fetch action items.


141-142: Added type declaration for CreateActionItemCategory mutation

This new declaration supports:

  1. The input type for creating action item categories
  2. Return type with category fields and nested organization and creator objects
  3. Appropriate nullable configurations

This completes the type safety for the action item category management functionality.


152-152: Fixed type declaration for advertisement query

The 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: Added organization reference to ActionItem
Introducing the organization: Organization! field correctly models the relationship and is non-nullable, matching the requirement that every action item belongs to an organization.


54-55: Approve: Added updater reference to ActionItem
The new updater: User field will allow clients to fetch who last modified the action item.


59-59: Approve: Documented createdAt on ActionItemCategory
Adding the timestamp helps clients audit category creation.


74-75: Approve: Added organization reference to ActionItemCategory
Marking the category’s organization with a non-nullable Organization! field is consistent and necessary.


1462-1465: Approve: Introduced createActionItem mutation
The new createActionItem(input: MutationCreateActionItemInput!): ActionItem mutation aligns with the GraphQL API’s pattern and uses the new input type.


1467-1473: Approve: Introduced createActionItemCategory mutation
Adding createActionItemCategory(input: MutationCreateActionItemCategoryInput!): ActionItemCategory completes the CRUD surface for categories.


1649-1654: Approve: Introduced updateActionItemCategory mutation
The updateActionItemCategory(input: MutationUpdateActionItemCategoryInput!): ActionItemCategory mutation correctly complements the create and delete operations.


1742-1758: Approve: MutationCreateActionItemCategoryInput is well-defined
Input fields (name: String!, organizationId: ID!, optional isDisabled) cover all necessary parameters for creating a category.


2444-2456: Approve: MutationUpdateActionItemCategoryInput is clear
The input allows updating the category name and disabled state, with categoryId: ID! to identify the target.

coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 22, 2025
Signed-off-by: NishantSinghhhhh <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between c58fdca and c7e8f3f.

📒 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 new allottedHours field.

The seedActionItem function has been updated to include the new allottedHours 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:

  1. Unauthenticated access (no token)
  2. Empty results when appropriate
  3. Unauthorized access for non-members

These tests ensure the GraphQL resolvers properly enforce access controls for the refactored action items API.

coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 22, 2025
@mpdesai90
Copy link

Please fix the failing test.

@meetulr meetulr self-requested a review April 23, 2025 13:50
@NishantSinghhhhh
Copy link
Author

@meetulr sir, @palisadoes sir should I add any more changes in here ??

Copy link
Contributor

@meetulr meetulr left a 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.

@NishantSinghhhhh
Copy link
Author

I am looking at the whole PR again and will write all changes I make in here

Signed-off-by: NishantSinghhhhh <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a 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 extension

The 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 information

When the client explicitly sets allottedHours to null (to clear it), the current logic turns it into undefined, 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 purpose

The exported constant name createActionItemCategoryMutation doesn't match the mutation field name createActionItem it implements, creating confusion.

-export const createActionItemCategoryMutation = builder.mutationField(
+export const createActionItemMutation = builder.mutationField(

145-145: completionAt is pre-filled although isCompleted is false

Creating a new action item with isCompleted: false shouldn't have a completionAt timestamp. This creates an inconsistent state.

-completionAt: new Date(),
+completionAt: null,

151-154: Numeric allottedHours is stored as a string – add an explicit cast for clarity

While numeric columns map to string 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 vs numeric)

The database column is declared as numeric, but the GraphQL input type uses t.int(). If you need to support fractional hours, use t.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). Prefer actionItemRelations or actionItemsRelations 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 name

The relationName still refers to assignee_id but the relation mapping is from actor_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 items

The 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
The ActionItem.allottedHours field is declared as Float, but the corresponding input types (MutationCreateActionItemInput and MutationUpdateActionItemInput) use Int. This breaks type consistency and prevents fractional hours.
Please unify by changing the input types to Float.


19-20: Optional: Make category non-nullable
Since every ActionItem should belong to a category, consider marking category: ActionItemCategory! to enforce this constraint at the schema level.


62-63: Optional: Make creator non-nullable
If every category is created by a user, enforcing creator: User! can improve schema robustness.


1760-1790: Duplicate: Type mismatches in MutationCreateActionItemInput
The input defines allottedHours: Int and assignedAt: String, which conflicts with allottedHours: Float and assignedAt: DateTime in ActionItem.
Please update the input fields to Float and DateTime respectively.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7e8f3f and 1d56bf6.

📒 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 modularized

The 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 mocks

The 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 test

This 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 comprehensive

This 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 test

This 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 implementation

The resolveCategory function is well-implemented with proper handling for:

  1. Null category IDs
  2. Database queries with appropriate filtering
  3. Error handling with detailed error messages
  4. 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 type

The ActionItem type is properly extended with a category field that:

  1. Returns the appropriate type (ActionItemCategory)
  2. Is correctly marked as nullable
  3. Has a clear description
  4. 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 renames

The 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 tables

The database query has been correctly updated to use actionItemsTable instead of actionsTable, with proper column references including the renamed actionItemCategoryId.


143-167: LGTM: Category validation logic updated

The category validation logic has been properly updated to use actionItemCategoryId instead of categoryId, with appropriate database queries to the renamed actionItemCategoriesTable.


246-249: LGTM: Mark pending query updated

The 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 updated

The 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 ts

Length 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.ts

Length of output: 490


Ignore the isCompleted filter concern
The isCompleted field is intentionally defined on AdvertisementWhereInput in src/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
Defining organization: Organization! ensures that an ActionItem always has an organization, which aligns with the domain model.


54-55: Approve: updater field addition
Adding updater: User provides traceability for who last updated the action item.


59-59: Approve: Timestamp documentation
The new doc string for createdAt in ActionItemCategory clarifies the field's purpose.


74-75: Approve: Non-nullable organization on category
Requiring organization: Organization! ensures consistency and integrity of category assignments.


1462-1466: Approve: New createActionItem mutation
The mutation signature and input type align correctly with the schema refactor.


1467-1473: Approve: New createActionItemCategory mutation
The addition of createActionItemCategory 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.

coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 30, 2025
@NishantSinghhhhh

This comment was marked as off-topic.

Signed-off-by: NishantSinghhhhh <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a 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 issue

Critical: organizationId input type mismatch and unused input

  • The QueryActionItemsByOrganizationInput uses organizationId: String! instead of ID!.
  • This input type is currently unused by the actionItemsByOrganization query.
    Please update the scalar to ID! and ensure the query signature consumes this input.
♻️ Duplicate comments (5)
schema.graphql (2)

10-11: ⚠️ Potential issue

Critical: Mismatch between schema field and input type for allottedHours
The ActionItem.allottedHours is declared as Float, but in MutationCreateActionItemInput and MutationUpdateActionItemInput it’s defined as Int. This prevents clients from specifying fractional hours and is inconsistent with the schema.
Please unify these types to Float.

Example diff:

# In input types:
input MutationCreateActionItemInput {
-  allottedHours: Int
+  allottedHours: Float
  ...
}

input MutationUpdateActionItemInput {
-  allottedHours: Int
+  allottedHours: Float
  ...
}

1760-1791: ⚠️ Potential issue

Critical: Input type mismatch for assignedAt and allottedHours

  • assignedAt is declared as String, but the schema field uses DateTime.
  • allottedHours is declared as Int, but the schema field uses Float.
    These inconsistencies can lead to runtime errors and type coercion issues.
    Please update the input type to use DateTime for assignedAt and Float for allottedHours.

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 persist

The input still declares
allottedHours: Int (schema uses Float)
assignedAt: String (schema uses DateTime)

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 be Float

Both concerns were flagged earlier. Adjust the schema and regenerate the typings.


195-195: organizationId scalar inconsistency not fixed

QueryActionItemsByOrganizationInput.organizationId continues to use String, whereas the new categories query correctly uses ID. This inconsistency was highlighted before and can confuse API consumers & code-gen tools.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d56bf6 and 5b3cf47.

📒 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 flag

Logic is correct; however, GitHub Actions reported “Unauthorized changes detected in this file”.
If this file is intentionally modified, please add the ignore-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 (creatorcreatedBy, 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 – validate

You wrap organizationId inside an input 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 the input: 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" -n

Length 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.ts

Length of output: 3576


No change needed—input wrapper is correct

The queryActionItemsByOrganizationArgumentsSchema is defined as:

export const queryActionItemsByOrganizationArgumentsSchema = z.object({
  input: queryActionItemsByOrgInputSchema,
});

…and the GraphQL field’s args shape is { input: { organizationId: string } }. Dropping the input wrapper would break the validation.

Likely an incorrect or invalid review comment.

src/graphql/types/Query/ActionItemCategorybyOrganization.ts (1)

70-75: Auth guard passes but currentClient.user may still be undefined

Immediately after the guard you access ctx.currentClient.user.id. Ensure the context always hydrates the user object once isAuthenticated 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 good

The new paginated queries are correctly registered in the root Query index. No further action required.

schema.graphql (14)

42-43: Approve: Added non-nullable organization reference
The organization: Organization! field correctly replaces the ID reference with a direct object and enforces its presence.


54-55: Approve: Added updater field to ActionItem
Providing the updater: User relation aligns with other timestamp fields and enhances auditability.


59-63: Approve: Documentation and creator relation for ActionItemCategory
The added docstring for createdAt and the creator: User field improve clarity and consistency with other types.


74-75: Approve: Non-nullable organization on ActionItemCategory
Marking organization: Organization! ensures every category is scoped to an organization, aligning with the data model.


1461-1466: Approve: createActionItem mutation signature
The new createActionItem(input: MutationCreateActionItemInput!): ActionItem mutation correctly uses the consolidated input type for cleaner API design.


1467-1473: Approve: createActionItemCategory mutation added
Enabling category management via createActionItemCategory(input: …): ActionItemCategory is a valuable addition to the GraphQL API.


1649-1654: Approve: updateActionItemCategory mutation added
Supporting updates to action item categories with updateActionItemCategory(input: …): ActionItemCategory completes the CRUD operations surface.


1742-1758: Approve: MutationCreateActionItemCategoryInput definition
The input type with name: String!, organizationId: ID!, and optional isDisabled is clear and enforces required metadata.


3379-3388: Approve: Paginated actionCategoriesByOrganization query
The switch to Relay-style pagination with input: 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 with cursor and node.


3518-3523: Approve: Input for paginated category queries
QueryActionCategoriesByOrganizationInput with a non-nullable organizationId: 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 the cursor and node fields.

test/graphql/types/gql.tada-cache.d.ts (1)

141-142: Missing updatedAt in createActionItemCategory return type

Down-stream consumers often rely on createdAt and updatedAt. You already expose createdAt; returning updatedAt 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 pass null. For stricter test coverage pass concrete values or document which combination is allowed (e.g., forbid mixing first & last).

coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 30, 2025
Copy link

@coderabbitai coderabbitai bot left a 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 still Int while output is Float
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 vs DateTime) and optionality of isCompleted.

Also applies to: 128-128

schema.graphql (4)

19-20: Optional: Enforce non-null category on ActionItem.
Since every action item must belong to a category, consider changing category: ActionItemCategory to category: ActionItemCategory! to guarantee data integrity at the schema level.


3386-3396: Refactor: Standardize pagination input for actionItemsByOrganization.
The actionItemsByOrganization query uses inline organizationId: ID! instead of an input 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!
): QueryActionItemsByOrganizationConnection

And update QueryActionItemsByOrganizationInput to organizationId: ID!.

Also applies to: 3521-3524, 3526-3529


2440-2453: Optional Refactor: Rename categoryId to id in MutationUpdateActionItemCategoryInput.
For consistency with other update mutations (which use id: ID!), consider renaming the field from categoryId to id.


1756-1787: ⚠️ Potential issue

Critical: Inconsistent types for allottedHours and assignedAt inputs.
The schema's allottedHours field is Float, but the input uses Int, and assignedAt is String while the schema expects DateTime. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5b3cf47 and dd446df.

📒 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 new allottedHours field on ActionItem.
This aligns with the database schema and supports fractional hour estimates, enhancing scheduling accuracy.


42-43: Approve the organization association on ActionItem.
The non-nullable organization: Organization! field cleanly exposes the multi-tenant relationship in the GraphQL schema.


54-55: Approve the addition of updater on ActionItem.
Adding updater: User matches the audit pattern for other types and completes the update-tracking fields.


59-63: Approve audit fields on ActionItemCategory.
Including createdAt: DateTime and creator: User provides consistent creation metadata and aligns with patterns in other types.


74-75: Approve the organization association on ActionItemCategory.
Linking to Organization! ensures proper multi-tenant separation and is consistent with other connected types.


1458-1462: Approve createActionItem mutation signature.
Switching to MutationCreateActionItemInput standardizes the API and integrates with the new resolver layer.


1463-1470: Approve new createActionItemCategory mutation.
Adding createActionItemCategory(input: MutationCreateActionItemCategoryInput!): ActionItemCategory provides full CRUD support for categories in the GraphQL schema.


1645-1650: Approve new updateActionItemCategory mutation.
Introducing updateActionItemCategory(input: MutationUpdateActionItemCategoryInput!): ActionItemCategory completes the category management endpoints.


1738-1754: Review MutationCreateActionItemCategoryInput fields.
The input defines name: String!, optional isDisabled, and organizationId: ID!. Ensure server-side validation enforces non-empty name and a valid UUID format for organizationId.


3375-3385: Approve paginated actionCategoriesByOrganization query.
The Relay-style pagination using first, last, before, after, combined with QueryActionCategoriesByOrganizationInput and its connection/edge types, follows best practices and matches other paginated fields.

Also applies to: 3501-3504, 3506-3509, 3514-3519


3409-3411: Approve categoriesByIds query and input.
The new categoriesByIds(input: QueryCategoriesByIdsInput!): [ActionItemCategory!] field, backed by the corresponding input type, efficiently supports batch-fetching of categories.

Also applies to: 3571-3573

coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 30, 2025
Signed-off-by: NishantSinghhhhh <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a 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 to actionItemsWhereCategory for complete naming consistency across the codebase:

-		actionsWhereCategory: many(actionItemsTable, {
+		actionItemsWhereCategory: many(actionItemsTable, {
♻️ Duplicate comments (35)
src/envConfigSchema.ts (1)

243-246: ⚠️ Potential issue

Environment variable API_REDIS_URI needs to be made optional to prevent pipeline failures

The 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 issue

Add 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 filename

The header comment says
// src/graphql/inputs/MutationUpdateActionItemCategoryInput.ts
but the actual file is MutationUpdateActionItemCategory.ts.
Keeping these in sync avoids confusion during code search.


15-19: Strengthen validation for name to avoid empty or blank category names

Users 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 category

This keeps empty updates disallowed while still permitting omission.


40-44: Align GraphQL field nullability with Zod schema

The 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 schema

The 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 resolver

The error handling has been simplified. Instead of throwing a TalawaGraphQLError when an assignee is not found, the code now logs a warning and returns null. 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 (ActionItemCategoryActionItemCategoryRef) 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 missing organizationMembershipsWhereMember
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 offending updaterId

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 information

When the client explicitly sets allottedHours to null (to clear it), the current logic turns it into undefined, 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 suggestion

Use composable expression helpers instead of raw sql template for safety

Embedding 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 suggestion

Global 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 issue

Unique-constraint collisions not handled – surface a friendly error

Inserting a category with an existing (name, organizationId) pair will raise a Postgres error bubbling up as 500. The database schema has a unique index on these fields (see uniqueIndex().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 a TalawaGraphQLError 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 appropriate TalawaGraphQLError.
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 issue

Leaking internal relation field (attachmentsWhereEvent) in GraphQL payload

Object.assign(existingEvent, { attachments: existingEvent.attachmentsWhereEvent }) adds the friendly attachments alias but leaves the original attachmentsWhereEvent 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 to assertIsAdminForOrg (or similar) would DRY-up the codebase.

src/graphql/types/Query/ActionItembyOrganization.ts (1)

100-110: 🛠️ Refactor suggestion

Cursor 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 the createActionItem 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 while isCompleted is false – inconsistent state

Creating 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 clarity

Drizzle maps numericstring. 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 constants

The 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 dedicated resource_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 be actionItemRelations 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 alias

Using import { z as zod } is uncommon and was previously flagged. Prefer the
canonical import { z } from "zod" (and drop the alias) or import * as zod
for consistency across input files.


31-33: Type mismatch: numeric ↔︎ Int—decimal hours will be truncated

The DB layer allows fractional hours (numeric + zod.number()), but the
GraphQL layer exposes t.int(). Clients sending 1.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 of base64url

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 benefit

The extra exists (…) merely checks that the cursor row exists; if it doesn’t,
the outer select 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 issue

Only 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

📥 Commits

Reviewing files that changed from the base of the PR and between dd446df and ecd0337.

📒 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 good

The 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 structured

The 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 filename

The import path for ActionItem has been corrected to use the lowercase filename convention.


10-14: Improved documentation with clear purpose and functionality description

The 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 extension

The 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 descriptions

The 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 resolver

The 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 implementation

The 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:

  1. Checking for required fields
  2. Verifying authentication
  3. Implementing proper authorization rules
  4. 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:

  1. Clear type definitions
  2. Proper handling of null category IDs
  3. Comprehensive error handling with detailed error messages and logging
  4. 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 an input 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 to actionItemsTable 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 message
  • organizationId as a valid UUID
  • isDisabled as an optional boolean

This 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 validation

The 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 schema

The 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 actionItemCategoryId

The code now correctly references actionItemCategoryId instead of categoryId, aligning with the schema changes made throughout the codebase.

src/graphql/inputs/MutationCreateActionItem.ts (1)

1-104: Well-structured input type with comprehensive validation

The 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-side limit cap in defaultGraphQLConnectionArgumentsSchema by requesting a huge first.
Consider clamping the multiplier to the same maximum allowed limit 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" -A10

Length 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.ts

Length of output: 3100


Zod schema already caps “first”/“last” at 32, so multiplier can’t explode
The defaultGraphQLConnectionArgumentsSchema enforces .max(32) on both first and last, 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 that ctx.currentClient.user is set, but the contract is not enforced in the type system. Accessing ctx.currentClient.user.id could throw if a future middleware inadvertently sets isAuthenticated = 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 its user 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 src

Length of output: 146


🏁 Script executed:

#!/bin/bash
# Show surrounding lines of CurrentClient definition
rg "export type CurrentClient" -A 10 -B 5 src/graphql/context.ts

Length of output: 394


Gathering the full CurrentClient definition and any references to user:


🏁 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.ts

Length of output: 1507


Ignore unnecessary runtime null-check for ctx.currentClient.user

The CurrentClient type is a discriminated union—after your if (!ctx.currentClient.isAuthenticated) throw… check, TypeScript narrows to the branch where isAuthenticated: true and user 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 implementation

This implementation follows good practices by:

  1. Including thorough input validation
  2. Using proper error handling with specific error codes
  3. Providing clear type definitions and descriptions
  4. Following the project's established resolver patterns

The query fits well with the broader refactor of action item handling described in the PR.

coderabbitai[bot]
coderabbitai bot previously approved these changes May 1, 2025
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
@NishantSinghhhhh NishantSinghhhhh requested a review from meetulr May 1, 2025 20:49
Copy link

@coderabbitai coderabbitai bot left a 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 issue

Post-completion notes check uses the old completion status

validateActionItemTypeConstraints is invoked with existingActionItem.isCompleted; therefore, if the client flips isCompleted from false → 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 missing

The error object still reports ["input", "categoryId"], but the field was renamed to actionItemCategoryId. This misleads API consumers and automated clients.

-  argumentPath: ["input", "categoryId"],
+  argumentPath: ["input", "actionItemCategoryId"],
schema.graphql (1)

3525-3528: ⚠️ Potential issue

Fix QueryActionItemsByOrganizationInput type mismatch

The field organizationId is declared as String! but should be ID! 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 earlier

The allottedHours mapping still collapses an explicit null to undefined, 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 be Float, not Int

The runtime field accepts decimals (numeric in DB / Float in schema). Keeping Int here can break clients that send fractional hours.


127-127: MutationUpdateActionItemInput.allottedHours also uses Int

Same scalar mismatch as above; both create & update inputs must align with Float.


187-194: 🛠️ Refactor suggestion

Inconsistent ID scalar for organization in action-item query

QueryActionItemsByOrganizationInput.organizationId is still String, while the new category query correctly uses ID (lines 190-191). Standardise on ID to avoid confusion and coercion issues.

- 'organizationId'; type: { kind: 'NON_NULL'; ... 'String'
+ 'organizationId'; type: { kind: 'NON_NULL'; ... 'ID'
schema.graphql (2)

62-63: Optional: Make creator non-nullable for ActionItemCategory

To guarantee every category has an author, you could enforce creator: User! instead of allowing null.


1749-1780: Critical: Inconsistent input types for allottedHours and assignedAt

The MutationCreateActionItemInput defines:

  • allottedHours: Int but the schema type is Float
  • assignedAt: String but the schema type is DateTime

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

📥 Commits

Reviewing files that changed from the base of the PR and between ecd0337 and 19cc704.

📒 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)
🔇 Additional comments (14)
schema.graphql (14)

10-11: Use Float for allottedHours – Verified

The new allottedHours: Float field on ActionItem correctly supports fractional hour estimates as intended.


42-43: Good: Non-nullable organization reference

The organization: Organization! field correctly enforces that each ActionItem belongs to an organization.


54-55: Approve updater reference on ActionItem

Adding the updater: User field enhances auditability by tracking who last modified the action item.


74-75: Good: Non-nullable organization on ActionItemCategory

The organization: Organization! field ensures categories are scoped to an organization, preventing orphaned entries.


1454-1457: Approve createActionItem mutation signature

The signature createActionItem(input: MutationCreateActionItemInput!): ActionItem follows the established input-object pattern and aligns with other mutations.


1459-1466: Approve createActionItemCategory mutation

The new createActionItemCategory(input: MutationCreateActionItemCategoryInput!): ActionItemCategory mutation is well-structured and consistent with GraphQL conventions.


1731-1747: Approve MutationCreateActionItemCategoryInput

The input object includes required name and organizationId fields and an optional isDisabled flag—this design covers the mutation requirements.


3368-3378: Approve actionCategoriesByOrganization query

The paginated actionCategoriesByOrganization(input: QueryActionCategoriesByOrganizationInput!) query follows Relay-style conventions and cleanly encapsulates the organization filter.


3402-3404: Approve categoriesByIds query

The new bulk-fetch query categoriesByIds(input: QueryCategoriesByIdsInput!): [ActionItemCategory!] is useful and adheres to the input-object style.


3494-3497: Approve QueryActionCategoriesByOrganizationConnection type

The connection type with edges and pageInfo is correctly defined for paginated category results.


3499-3502: Approve QueryActionCategoriesByOrganizationConnectionEdge

The edge type properly exposes cursor: String! and node: ActionItemCategory, fitting Relay conventions.


3504-3512: Approve QueryActionCategoriesByOrganizationInput

The input object with a non-nullable organizationId: ID! cleanly encapsulates the filter criteria.


3514-3517: Approve QueryActionItemsByOrganizationConnection type

This paginated connection type accurately defines edges and pageInfo for action items.


3519-3522: Approve QueryActionItemsByOrganizationConnectionEdge

The edge type correctly includes cursor: String! and node: ActionItem.

@mpdesai90
Copy link

Please fix the failing tests.

@NishantSinghhhhh
Copy link
Author

@meetulr sir

Changed file names

ActionItemCategory ----> ActionItemCategorybyOrganization.ts
ActionItem.ts ----> ActionItembyOrganization.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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants