No mongo action items #3385
No mongo action items #3385
Conversation
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (300)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request migrates ActionItem handling from Express endpoints to a GraphQL-based approach. It updates the database schema (dropping a NOT NULL constraint on the completion field and updating the migration journal) and modifies various configuration parameters in the project (e.g. Redis URI and debugger port). In addition, it introduces new GraphQL types, resolvers, mutations, and queries for ActionItem and ActionItemCategory, while also extending DB management scripts and sample data to support these new collections. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GraphQLAPI
participant Resolver
participant Database
Client->>GraphQLAPI: Send createActionItem mutation with input data
GraphQLAPI->>Resolver: Validate authentication and input fields
Resolver->>Database: Query for organization, category, and assignee existence
Database-->>Resolver: Return query results
Resolver->>Database: Insert new ActionItem record (with nullable completionAt)
Database-->>Resolver: Confirm insertion
Resolver->>GraphQLAPI: Return created ActionItem data
GraphQLAPI-->>Client: Deliver ActionItem response
Assessment against linked issues
Suggested labels
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
|
I will make sure to merge this PR as soon as possible , it has been 2 weeks since it is dormant |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 36
🔭 Outside diff range comments (1)
src/graphql/types/ActionItem/assignee.ts (1)
53-54: 🧹 Nitpick (assertive)Apply consistent formatting.
A trailing parenthesis issue appears to prompt the formatter complaint. A quick format pass (similar to the other files) should address it.🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 53-54: Formatter would have printed the following content: - }); + });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
drizzle_migrations/20250321223020_crazy_avengers.sql(1 hunks)drizzle_migrations/meta/_journal.json(1 hunks)package.json(4 hunks)schema.graphql(17 hunks)scripts/dbManagement/addSampleData.ts(3 hunks)scripts/dbManagement/helpers.ts(4 hunks)scripts/dbManagement/sample_data/action_categories.json(1 hunks)scripts/dbManagement/sample_data/action_items.json(1 hunks)scripts/dbManagement/sample_data/events.json(1 hunks)src/createServer.ts(1 hunks)src/drizzle/tables/actions.ts(3 hunks)src/envConfigSchema.ts(1 hunks)src/graphql/inputs/MutationDeleteActionItemInput.ts(1 hunks)src/graphql/inputs/MutationUpdateActionItemInput.ts(1 hunks)src/graphql/inputs/QueryActionItemInput.ts(1 hunks)src/graphql/types/ActionItem/ActionItem.ts(1 hunks)src/graphql/types/ActionItem/assignee.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/updatedAt.ts(1 hunks)src/graphql/types/ActionItemCategory/ActionItemCategory.ts(1 hunks)src/graphql/types/ActionItemCategory/index.ts(1 hunks)src/graphql/types/Mutation/createActionItem.ts(1 hunks)src/graphql/types/Mutation/deleteActionItem.ts(1 hunks)src/graphql/types/Mutation/index.ts(1 hunks)src/graphql/types/Mutation/updateActionItem.ts(1 hunks)src/graphql/types/Query/ActionCategory.ts(1 hunks)src/graphql/types/Query/ActionItem.ts(1 hunks)src/graphql/types/Query/Events.ts(1 hunks)src/graphql/types/Query/index.ts(1 hunks)src/graphql/types/Query/organizationUsers.ts(1 hunks)src/graphql/types/index.ts(1 hunks)test/graphql/types/gql.tada-cache.d.ts(1 hunks)test/graphql/types/gql.tada.d.ts(10 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/graphql/types/ActionItem/assignee.ts (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: src/resolvers/Mutation/createActionItem.ts:75-94
Timestamp: 2025-03-20T00:05:12.821Z
Learning: In `src/resolvers/Mutation/createActionItem.ts`, the assignee validation logic within the `createActionItem` function is intentionally kept inline and is not to be extracted into a helper function.
src/graphql/types/Mutation/createActionItem.ts (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-api#2615
File: src/resolvers/Mutation/createActionItem.ts:75-94
Timestamp: 2025-03-20T00:05:12.821Z
Learning: In `src/resolvers/Mutation/createActionItem.ts`, the assignee validation logic within the `createActionItem` function is intentionally kept inline and is not to be extracted into a helper function.
🧬 Code Definitions (11)
src/graphql/types/ActionItem/event.ts (1)
src/graphql/types/ActionItem/ActionItem.ts (2) (2)
ActionItem(4-4)ActionItem(6-6)
src/graphql/inputs/QueryActionItemInput.ts (1)
src/drizzle/tables/actions.ts (1) (1)
actionsTableInsertSchema(120-120)
src/graphql/types/ActionItem/createdAt.ts (1)
src/graphql/types/ActionItem/ActionItem.ts (2) (2)
ActionItem(4-4)ActionItem(6-6)
src/graphql/types/Mutation/deleteActionItem.ts (3)
src/graphql/inputs/MutationDeleteActionItemInput.ts (2) (2)
mutationDeleteActionItemInputSchema(5-7)MutationDeleteActionItemInput(9-21)src/drizzle/tables/actions.ts (1) (1)
actionsTable(17-84)src/graphql/types/ActionItem/ActionItem.ts (2) (2)
ActionItem(4-4)ActionItem(6-6)
src/graphql/types/Query/ActionCategory.ts (1)
src/graphql/types/ActionItemCategory/ActionItemCategory.ts (2) (2)
ActionItemCategory(6-6)ActionItemCategory(8-9)
src/graphql/types/ActionItem/assignee.ts (1)
src/graphql/types/ActionItem/ActionItem.ts (2) (2)
ActionItem(4-4)ActionItem(6-6)
src/graphql/types/ActionItem/creator.ts (1)
src/graphql/types/ActionItem/ActionItem.ts (2) (2)
ActionItem(4-4)ActionItem(6-6)
src/graphql/types/Mutation/createActionItem.ts (2)
src/graphql/types/ActionItem/ActionItem.ts (2) (2)
ActionItem(4-4)ActionItem(6-6)src/drizzle/tables/actions.ts (1) (1)
actionsTable(17-84)
src/graphql/inputs/MutationUpdateActionItemInput.ts (1)
src/drizzle/tables/actions.ts (1) (1)
actionsTableInsertSchema(120-120)
src/graphql/types/Mutation/updateActionItem.ts (3)
src/graphql/inputs/MutationUpdateActionItemInput.ts (2) (2)
MutationUpdateActionItemInputSchema(5-15)MutationUpdateActionItemInput(17-44)src/drizzle/tables/actions.ts (1) (1)
actionsTable(17-84)src/graphql/types/ActionItem/ActionItem.ts (2) (2)
ActionItem(4-4)ActionItem(6-6)
src/graphql/types/Query/ActionItem.ts (2)
src/graphql/inputs/QueryActionItemInput.ts (2) (2)
queryActionItemsByOrgInputSchema(8-10)QueryActionItemsByOrganizationInput(15-27)src/graphql/types/ActionItem/ActionItem.ts (2) (2)
ActionItem(4-4)ActionItem(6-6)
🪛 GitHub Actions: Pull request workflow
src/graphql/types/ActionItemCategory/index.ts
[error] 1-2: Formatter would have printed the following content: - import·"./ActionItemCategory"; + import·"./ActionItemCategory";
src/createServer.ts
[error] 82-83: Formatter would have printed the following content: - → ··}); + → });
src/graphql/types/ActionItem/event.ts
[error] 42-43: Formatter would have printed the following content: - }); + });
src/graphql/inputs/MutationDeleteActionItemInput.ts
[error] 21-22: Formatter would have printed the following content: - → }); + → });
src/graphql/inputs/QueryActionItemInput.ts
[error] 27-28: Formatter would have printed the following content: - → }); + → });
src/graphql/types/ActionItem/createdAt.ts
[error] 77-78: Formatter would have printed the following content: - }); + });
src/graphql/types/Mutation/deleteActionItem.ts
[error] 8-9: Import statements could be sorted.
src/graphql/types/ActionItem/index.ts
[error] 6-7: Formatter would have printed the following content: - export··from·"./event"; + export··from·"./event";
src/drizzle/tables/actions.ts
[error] 120-121: Formatter would have printed the following content: - export·const·actionsTableInsertSchema·=·createInsertSchema(actionsTable); + export·const·actionsTableInsertSchema·=·createInsertSchema(actionsTable);
src/graphql/types/ActionItem/assignee.ts
[error] 53-54: Formatter would have printed the following content: - }); + });
scripts/dbManagement/addSampleData.ts
[error] 74-75: Formatter would have printed the following content: - } + }
src/graphql/types/ActionItem/creator.ts
[error] 89-90: Formatter would have printed the following content: - }); + });
src/graphql/types/Mutation/createActionItem.ts
[error] 163-164: Formatter would have printed the following content: - ); + );
[error] 8-9: Import statements could be sorted.
src/graphql/types/ActionItem/updatedAt.ts
[error] 81-82: Formatter would have printed the following content: - }; + };
src/graphql/types/ActionItemCategory/ActionItemCategory.ts
[error] 82-83: Formatter would have printed the following content: - ); + );
src/graphql/inputs/MutationUpdateActionItemInput.ts
[error] 44-45: Formatter would have printed the following content: - → }); + → });
src/graphql/types/ActionItem/ActionItem.ts
[error] 64-65: Formatter would have printed the following content: - }); + });
src/envConfigSchema.ts
[error] 246-246: Formatter would have printed the following content: - → → format:·'uri',·//·optional·if·your·validator·supports·URI·format·validation + → → format:·"uri",·//·optional·if·your·validator·supports·URI·format·validation
scripts/dbManagement/helpers.ts
[error] 675-675: Formatter would have printed the following content: - } + }
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (66)
src/graphql/types/ActionItemCategory/index.ts (1)
1-1:✅ Verification successful
Imported the ActionItemCategory module successfully.
This import is correctly set up to include the ActionItemCategory module in the GraphQL schema. The pipeline failure indicates a minor formatting issue that doesn't affect functionality.
🏁 Script executed:
#!/bin/bash # Verify that the ActionItemCategory definition exists fd ActionItemCategory.ts --type fLength of output: 94
Verified Import of ActionItemCategory Module
- The file
src/graphql/types/ActionItemCategory/ActionItemCategory.tswas located successfully, confirming that the import statement insrc/graphql/types/ActionItemCategory/index.tsis correct.- The minor formatting issue noted in the pipeline does not impact functionality.
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 1-2: Formatter would have printed the following content: - import·"./ActionItemCategory"; + import·"./ActionItemCategory";
drizzle_migrations/20250321223020_crazy_avengers.sql (1)
1-1: Migration correctly drops NOT NULL constraint for action completion.This SQL migration makes the "completion_at" column nullable, which aligns with the PR objective of migrating ActionItem handling to GraphQL. Making this field optional provides more flexibility for action items that haven't been completed yet.
drizzle_migrations/meta/_journal.json (1)
11-18: Journal entry properly records the new migration.The journal entry correctly tracks the new migration for altering the "completion_at" column with appropriate metadata, including the timestamp and migration tag that matches the SQL file name.
src/graphql/types/index.ts (1)
1-2:✅ Verification successful
New ActionItem types are correctly registered in the GraphQL schema.
These imports incorporate the new ActionItem and ActionItemCategory modules into the main GraphQL types system, supporting the migration from Express endpoints to GraphQL as intended in the PR objectives.
🏁 Script executed:
#!/bin/bash # Verify that both ActionItem and ActionItemCategory are properly structured echo "Checking ActionItem implementation:" fd --type f ActionItem/ echo -e "\nChecking ActionItemCategory implementation:" fd --type f ActionItemCategory/ echo -e "\nVerifying GraphQL schema updates for ActionItem:" fd schema.graphql --type f | xargs grep -A 10 -B 2 "ActionItem"Length of output: 5780
GraphQL Schema Registration Verified
The imports in
src/graphql/types/index.tscorrectly include both the ActionItem and ActionItemCategory modules. Verification shows that:
- The ActionItem implementation is present.
- The ActionItemCategory implementation is detected.
- The GraphQL schema file includes proper type definitions for both ActionItem and ActionItemCategory.
The migration from Express endpoints to GraphQL appears to be supported as intended.
src/graphql/types/Query/index.ts (1)
23-26: Good addition of new query types!The new imports for ActionCategory, ActionItem, organizationUsers, and Events align with the PR objective of migrating ActionItem handling to GraphQL.
Consider standardizing the casing convention between your imports. For example, line 10 uses "event" (camelCase) while line 26 introduces "Events" (PascalCase). Similarly, "ActionCategory" and "ActionItem" use PascalCase while "organizationUsers" uses camelCase.
src/envConfigSchema.ts (1)
270-275: Good documentation addition.Adding a descriptive comment for the rate limit refill rate parameter improves code clarity.
src/drizzle/tables/actions.ts (3)
10-10: Good import addition for schema creation.Adding the
createInsertSchemaimport from drizzle-zod supports the new schema export at the end of the file.
33-37: Well-considered change to make completionAt nullable.Removing the
.notNull()constraint from thecompletionAtfield makes sense for action items that haven't been completed yet. This aligns well with the PR objective of enhancing the flexibility of the ActionItem model.
119-120: Good export of insert schema.Exporting
actionsTableInsertSchemacreated withcreateInsertSchemaenables validation for action item insertions and is used by the new GraphQL input types.src/graphql/inputs/QueryActionItemInput.ts (2)
1-10: Well-structured schema definition.Good job creating a Zod validation schema that reuses the organization ID field from the actions table insert schema. This ensures consistent validation across the application.
12-27: Well-implemented GraphQL input type.The GraphQL input type definition follows the builder pattern used elsewhere in the codebase and includes helpful descriptions. This input type will make it easy for clients to query action items by organization.
Make sure that appropriate tests are in place to verify that this new input type works correctly with the GraphQL resolver.
scripts/dbManagement/sample_data/action_categories.json (1)
1-137: Sample data structure looks good.The action categories JSON data is well-structured with consistent field types, providing good test data across multiple organizations. Each category has a proper UUID, sensible timestamps, and appropriate organization references.
src/graphql/types/ActionItem/index.ts (2)
1-6: LGTM: Well-structured index file for ActionItem typesThis index file correctly exports all necessary ActionItem-related modules, making them easily accessible throughout the codebase.
6-6:✅ Verification successful
Fix formatting issue
The pipeline failure indicates a formatting issue with line 6. Please ensure there's no extra whitespace or newline at the end of the file.
🏁 Script executed:
#!/bin/bash # Check for formatting issues in the file cat src/graphql/types/ActionItem/index.ts | wc -lLength of output: 53
Formatting issue resolved
The file now has 5 lines, which indicates that the extra newline/whitespace issue previously reported at line 6 has been removed. No further changes are needed in this file.
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 6-7: Formatter would have printed the following content: - export··from·"./event"; + export··from·"./event";
scripts/dbManagement/addSampleData.ts (2)
13-16: LGTM: Collection type properly extendedThe Collection type definition is appropriately extended to include new collection types needed for the ActionItem GraphQL implementation.
28-30: LGTM: Collections array updated accordinglyThe collections array is updated to include the new collection types, ensuring they'll be properly processed by the insertCollections function.
src/graphql/inputs/MutationDeleteActionItemInput.ts (3)
1-7: LGTM: Well-structured input schemaThe input schema for deleting action items is correctly defined using Zod, with proper validation for the id field.
9-21: LGTM: GraphQL input reference properly implementedThe GraphQL input reference is well-structured with appropriate descriptions and field definitions.
21-21:✅ Verification successful
Fix formatting issue
The pipeline failure indicates a formatting issue with line 21. Please ensure there's no extra whitespace or newline at the end of the file.
🏁 Script executed:
#!/bin/bash # Check for formatting issues in the file cat src/graphql/inputs/MutationDeleteActionItemInput.ts | tail -n 3Length of output: 87
Formatting Verification:
After reviewing the tail output of the file, the last three lines are as follows:
}),}),});There isn’t an obvious extra blank line after the closing brace. However, please double-check that none of these lines contain any trailing whitespace (e.g., spaces or tabs after the final visible character) or an unintended extra newline beyond what your formatter or linter expects. Once you confirm that all trailing whitespace is removed and the file ends exactly with the final closing line, the formatting issue should be resolved.
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 21-22: Formatter would have printed the following content: - → }); + → });
src/graphql/types/ActionItem/ActionItem.ts (1)
1-5: Imports and type definitions look good.No issues detected with importing
actionsTableor definingActionItemfrom its inferred type. This setup is clean and straightforward.scripts/dbManagement/sample_data/action_items.json (1)
24-24: Confirm handling of empty strings for “updatedAt.”Several entries use an empty string (
"") forupdatedAt. If your schema or data import expectsnullfor unknown or no updates, consider usingnullinstead of an empty string to avoid confusion or parsing errors.Also applies to: 56-56, 88-88, 104-104, 136-136, 152-152, 184-184, 200-200, 216-216, 232-232
src/graphql/types/Mutation/deleteActionItem.ts (1)
108-119: Overall deletion logic looks correct.
The database deletion and return logic is straightforward, with proper error handling if the deletion fails. Great job ensuring an unexpected error is thrown when no item is deleted.src/graphql/types/ActionItemCategory/ActionItemCategory.ts (1)
45-81:❓ Verification inconclusive
Consider implementing access control for organization-specific data.
The query currently allows any authenticated user to fetch categories by ID, regardless of organization membership. If categories are intended to be organization-restricted data, you may need to validate the requester's membership or role.You can confirm whether membership checks are required by searching for existing patterns in the repository:
🏁 Script executed:
#!/bin/bash # Searching for membership validation references in queries or resolvers rg -A 10 "membership"Length of output: 87847
Action Required: Confirm Organization-Specific Access Control
The resolver in
src/graphql/types/ActionItemCategory/ActionItemCategory.ts(lines 45–81) currently returns action item categories for any authenticated user without checking if the user belongs to the organization that should have access. We’ve observed multiple patterns across the codebase (e.g., in various organization and membership-related queries/mutations) where a membership or role check is enforced to restrict access to organization-specific data.
- If these action item categories are intended to be organization-restricted, please add a check to ensure the requester’s organization membership or proper role before fetching categories.
- Otherwise, if these categories are meant to be globally accessible, please confirm and document why additional access controls are not necessary.
src/graphql/types/ActionItem/assignee.ts (1)
44-53: Great resolver implementation.
Resolving the assignee field by checkingassigneeIdand throwing a precise error when the user is not found is solid and consistent with your error-handling approach.src/graphql/types/ActionItem/createdAt.ts (2)
1-66: Effective Access Control ChecksThe resolver comprehensively verifies user authentication and administrator-level permissions before exposing the
createdAtfield. This approach helps protect sensitive data and aligns well with best practices for field-level access control.
68-76: Field Implementation is CorrectWiring the
createdAtresolver into theActionItemtype is properly done. Defining the field description and return type ensures that consumers of the GraphQL schema clearly understand thecreatedAtfield's purpose and data type.src/graphql/types/Query/Events.ts (1)
60-96: Confirm Membership-Based Access or Make It ExplicitCurrently, the query retrieves and returns events based solely on matching IDs, without an explicit check on membership roles contained in
organization.membershipsWhereOrganization. If events are supposed to be accessible only to those within a specific organization or role, consider filtering these results accordingly or confirming that all authenticated users may view them.Do you want me to generate a script to search for any membership-based access checks in the codebase to confirm there is no oversight?
src/graphql/inputs/MutationUpdateActionItemInput.ts (1)
1-16: Logical Use of Base SchemaPicking the necessary fields (
postCompletionNotes,preCompletionNotes,categoryId,assigneeId,isCompleted) and then extending withidis a clear approach for an update schema. This ensures any extraneous fields inactionsTableInsertSchemaare excluded for updates.test/graphql/types/gql.tada-cache.d.ts (1)
95-96: Mutation entry looks correct.
The newly added mutation definition forMutation_joinPublicOrganizationappears well-structured and consistent with the surrounding typed document nodes.src/graphql/types/Query/ActionItem.ts (5)
1-8: Imports and setup are cohesive.
The import statements and initial setup lines are clear and properly reference the necessary modules.
10-12: Schema definition is explicit.
The Zod object schema for the query arguments is concise and provides a strong foundation for validating the input structure.
31-39: Good authentication guard.
Throwing an exception when the user is not authenticated protects the action items from unauthorized access.
41-58: Robust argument validation.
Using Zod’ssafeParseand mapping error messages to theissuesarray helps keep error handling consistent and transparent.
160-164: Return value is clear.
Returning all action items in an array aligns with common query patterns. No problems spotted.src/graphql/types/Query/ActionCategory.ts (6)
10-14: Zod schema for organization ID is well-defined.
Requiring a valid UUID ensures better data integrity and less confusion when handling incorrect IDs.
16-23: Input type generation is straightforward.
MappingorganizationIdto a required string field cleanly integrates with the Zod schema.
45-51: Authentication guard is in place.
Ensuring that the current client is authenticated prevents unauthorized queries.
95-109: Organization existence check is appropriate.
Returning a clear error when the organization does not exist helps the client handle missing or invalid references.
111-113: Empty array return pattern is consistent.
Providing an empty list when no categories are found avoids null checks on the client side.
115-116: Final return logic is sound.
Returns the queried categories with no extraneous transformations.package.json (2)
35-35:✅ Verification successful
Consider verifying whether both “uuidv7” and “uuid” dependencies are needed.
There may be overlapping functionality with “uuidv7” and “uuid”. If only one is needed, removing the other could simplify maintenance.
To confirm usage of both, run a code search to see how they are referenced:
🏁 Script executed:
#!/bin/bash # Searching for references to "uuid" and "uuidv7" throughout the codebase fd '\.(js|ts|json)$' | xargs rg -A 3 'uuid\(' fd '\.(js|ts|json)$' | xargs rg -A 3 'uuidv7\('Length of output: 65984
Actionable: No dependency removal required
After verifying the codebase, both the “uuid” and “uuidv7” tokens are used distinctly. The “uuid” dependency (version ^11.1.0 in package.json) appears throughout the schema definitions and various modules (e.g., in drizzle table definitions for foreign keys), while “uuidv7” is specifically used as the default generator for primary key columns and in test cases. There is no functional overlap that would allow consolidation of these dependencies without affecting intended behavior.
• Recommendation: Leave both dependencies as they are, but consider adding documentation or comments to clarify the distinct roles of “uuid” versus “uuidv7” for future maintainability.
79-79:❓ Verification inconclusive
Check references to the old debug port.
Lines 79, 91, 103, and 105 now rely on port 9230 instead of 9229. Please confirm that all references (documentation, scripts, etc.) to the old port are updated to avoid confusion.
Also applies to: 91-91, 103-103, 105-105
🏁 Script executed:
#!/bin/bash # Search for references to the old port number 9229 in the entire repo rg '9229'Length of output: 9
Verify that all debugging references have been fully updated.
Our automated search for "9229" usingrgdid not return any results, which suggests that the old debug port is no longer present in the repository. In package.json, lines 79, 91, 103, and 105 now correctly reference port 9230. However, to be completely confident that no documentation, configuration, or auxiliary scripts still mention port 9229, please perform a manual verification of all related files.
- Confirm that no markdown, configuration, or additional script files reference port 9229.
- Double-check any external documentation or setup instructions to ensure they have also been updated to port 9230.
src/graphql/types/Mutation/updateActionItem.ts (2)
222-296:❓ Verification inconclusive
Confirm authorization for “markActionItemAsPending”.
This mutation does not check administrative or ownership credentials. Verify if all authenticated users should have the ability to reset completed items to pending.
🏁 Script executed:
#!/bin/bash # Searching for any related role checks in the code rg -A 5 'markActionItemAsPending'Length of output: 809
Authorization Confirmation Required for markActionItemAsPending
The mutation still only verifies that the client is authenticated, without any additional checks for administrative rights or ownership. Please confirm whether allowing any authenticated user to reset a completed action item to pending is the intended behavior or if additional authorization checks (e.g., role or ownership validation) are required.
198-203:✅ Verification successful
Verify automatic “updatedAt” handling.
Since
updatedAtuses$onUpdatein the schema, ensure Drizzle automatically applies it without explicitly settingupdatedAt. Otherwise, consider adding it here for clarity.
🏁 Script executed:
#!/bin/bash # Searching references to updatedAt in the table definitions rg -A 5 'updatedAt' # Checking if $onUpdate is triggered automatically in Drizzle # (No direct test possible here, but the search above helps confirm usage.)Length of output: 75130
Action: Confirming automatic “updatedAt” functionality
The investigation shows that our schema consistently configures the “updatedAt” column using a default function (which in our context implements the $onUpdate behavior) rather than updating it manually. In the updateActionItem mutation, we’re correctly omitting an explicit “updatedAt” set since Drizzle is expected to update it automatically upon record modification.
No changes are required in this area, but please verify that your environment continues to honor the automated updatedAt behavior as defined in our schema.
src/graphql/types/Mutation/createActionItem.ts (3)
20-39: Overall mutation definition looks solid.The structure for
createActionItemis clean and well-typed. Good use of descriptive arguments and builder inputs.
40-131: Inline membership and resource checks appear consistent.This aligns with the retrieved learnings indicating such validations should remain inline. The checks for organization, membership, category, and assignee are thorough.
142-142: Question the default completion date on creation.Currently,
completionAt: new Date()sets the item as “completed at creation,” which might conflict with the usual flow if the item is not actually completed yet. Consider setting this tonullor a future date logic.- completionAt: new Date(), + completionAt: null,src/graphql/types/Query/organizationUsers.ts (2)
93-125: Confirm permission requirements forusersByOrganizationId.This query fetches all users in an organization but does not explicitly check if the caller is authenticated or authorized. If public access is not intended, consider adding an authentication and membership check.
127-189: Query for events includes authentication checks.The additional authentication gating in
eventsByOrganizationIdis consistent with best practices, helping safeguard sensitive event data.scripts/dbManagement/helpers.ts (4)
17-17: UUID import is appropriate for fallback generation.The use of
uuidv4here is logical for IDs whose length is not standard. No concerns noted.
510-537: Action categories insertion logic is clean.Handling
createdAtandupdatedAtwith fallbacks to the current date looks good.
539-587: Checkidlength for action items is beneficial.Replacing invalid IDs with a new UUID ensures data consistency. This is a good safeguard.
632-633: Including “action_items” and “events” in record count checks.Tracking these tables in
checkDataSizeis consistent with the new insertion logic.schema.graphql (9)
280-282: New input type 'CategoriesByIdsInput'.
This input type correctly enforces non-nullability on the list elements by using[ID!]!. It is straightforward and looks good.
607-610: New 'EventsByOrganizationIdInput' input type.
The input is concise and correctly requires anorganizationIdof typeID!.
1389-1391: New 'MarkActionItemAsPendingInput' input type.
This minimal input type (requiring only the action itemid) appears effective for marking an action item as pending.
1590-1592: 'markActionItemAsPending' mutation added.
This mutation is designed to reset a completed action item back to a pending state. Please verify that the associated resolver sets or clears the appropriate fields (for example, ensuring that completion-related timestamps or notes are reset correctly) according to your business logic.
1602-1607: 'updateActionItem' mutation added.
This mutation facilitates updating key fields of an action item (such asassigneeId,categoryId,isCompleted, and related notes). It’s important that the resolver for this mutation performs the necessary validations—such as ensuring valid state transitions—and handles relational updates appropriately.
3210-3212: Query field for fetching action item categories by IDs added.
ThecategoriesByIdsquery uses theCategoriesByIdsInputto return an array of non-nullActionItemCategoryobjects. This is clear and consistent with the input definition.
3351-3354: New 'QueryEventsByIdsInput' input type added.
This input type enforces that a list of event IDs (all non-null) must be provided, which should help ensure robust query behavior.
3701-3704: New 'UsersByIdsInput' input type added.
This is a clean and well-defined input for batch-fetching users by their IDs.
3182-3197: New query fields for fetching action item categories and action items by organization.
The queriesactionCategoriesByOrganizationandactionItemsByOrganizationprovide flexible ways to fetch action item–related data linked to a specific organization. Both fields are well documented, and the use of input types helps enforce the required filtering criteria. One suggestion is to ensure that, in the case of no matching records, the resolvers return an empty list rather than null to simplify client-side handling.test/graphql/types/gql.tada.d.ts (4)
5-5: Well-structured ActionItem GraphQL typeThe ActionItem type includes all necessary fields for comprehensive action item management, including relationships with users (assignee, creator), events, and categorization. The inclusion of separate fields for pre and post-completion notes provides flexibility for action item tracking through its full lifecycle.
6-6: Well-defined ActionItemCategory typeThe ActionItemCategory type appropriately supports organization scoping and includes the isDisabled flag, which provides for soft deletion or temporary deactivation of categories without removing them from the database.
177-177: Complete CRUD operations for ActionItemThe implementation properly provides a full set of CRUD operations for ActionItems through both Query and Mutation types. The query operations appropriately support organization-scoped retrieval, which maintains proper data isolation in a multi-tenant context.
Also applies to: 69-69
22-22: Efficient batch fetching with IDs input typesThe implementation of CategoriesByIdsInput and UsersByIdsInput enables efficient batch fetching of related entities, which is important for optimizing resolver performance when retrieving assignees, creators, and categories for action items.
Also applies to: 222-222
|
@NishantSinghhhhh Please make sure it gets linked to the original issue. Please fix the failed tests and get code rabbits approval. |
Signed-off-by: NishantSinghhhhh <[email protected]>
|
@palisadoes sir , @aashimawadhwa ma'am, some 7 testing files are failing, should I open new issues for them or solve them in my PR only ??? |
Signed-off-by: NishantSinghhhhh <[email protected]>
|
All tests are passing for deleteActionItem |
|
Solve the failures in this PR. We cannot merge failing code |
Signed-off-by: NishantSinghhhhh <[email protected]>
|
@palisadoes sir , there is some problem in the CI/CD Workflow , and we are currently solving it |
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
|
@palisadoes sir, @DMills27 sir , @aashimawadhwa ma'am , I have added tests for all the files that i created for action Items, The CI/CD pipline is causing errors right now so, I will be correcting it now |
|
CI/CD is working good. If you see the logs, it's failing because of one related testcase.
I have raised on PR with sample change and it passed correctly. |
|
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
Signed-off-by: NishantSinghhhhh <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3385 +/- ##
====================================================
+ Coverage 57.02% 57.28% +0.26%
====================================================
Files 498 517 +19
Lines 38420 39985 +1565
Branches 1512 1600 +88
====================================================
+ Hits 21909 22906 +997
- Misses 16502 17070 +568
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@palisadoes sir , @meetulr sir ,can you review this PR |
|
Please fix the failing tests. |
|
Please fix the conflicting files |
Signed-off-by: NishantSinghhhhh <[email protected]>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
palisadoes
merged commit eec3734
into
PalisadoesFoundation:develop-postgres
|
@NishantSinghhhhh hey, did you introduce any mutation for create actionCategory and tell me how did you test this please? |
|
@Piyushk8 Are you saying that action item categories is missing in the DB? |
The database table exists but I didn't find any Graphql mutations to create any instance of it, Just wanted to know how the author of the PR handled it |
|
@Piyushk8 Create an issue to fix this. We need to restore the API functionality as it used to be. |
|
It seems this PR was merged prematurely... There was much to be corrected here. |
|
Unfortunately it cannot be merged automatically. |
|
@Piyushk8 Do you want to work on the reopened issue? |
To implement this in the Frontend, events need to handled first so should I go with the basic event functionality that is in API already , (no recurring events support) Otherwise it will need further changes depending on events which is still in discussion |
|
The basic functionality. Please ask to be assigned the issue, unless you have too many already |
There was a problem hiding this comment.
Add field resolvers to the Organization schema for actionItemCategories and actionItems.
Add a field resolver to the ActionItemCategory schema for actionItems.
Name the tables actionItemCategories, and actionItems.
Please maintain clear code separation and formatting, and follow the practices of the codebase... Proper table, schema, query names. Keep inputs, queries, mutations... where they belong, etc.
Fix all these in your current api pr or in a new pr. Take a thorough look, if there are things I've missed, fix them as well.
|
Sir I have added all these changes, and now will be adding tests for the file |
What kind of change does this PR introduce?
Feature: Migrates ActionItem handling from Express endpoints to GraphQL schema and resolvers.
Issue Number: Fixes #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
Motivation: We previously handled ActionItem CRUD using Express routes. This PR migrates those routes into a new GraphQL schema (ActionItem.ts) with related resolvers (e.g., creator.ts, assignee.ts, createdAt.ts, etc.). This ensures more flexible querying and improves consistency with other GraphQL-based parts of the codebase.
Changes:
Created a new ActionItem.ts GraphQL type exposing core fields (e.g., id, createdAt, organizationId, etc.).
Added separate files (creator.ts, assignee.ts, etc.) to modularize logic for fetching related objects.
Updated Drizzle ORM queries to handle relationships using findFirst and relevant where clauses.
Ensured ActionItemCategory.ts is properly related to ActionItem via categoryId.
Addressed TypeScript type issues by explicitly casting return types and adding null checks.
Does this PR introduce a breaking change?
Potentially no: Existing data is unaffected, but any service depending on the old Express routes must switch to using the GraphQL endpoints.
Checklist
CodeRabbit AI Review
I have reviewed and addressed all critical issues flagged by CodeRabbit AI.
I have implemented or provided justification for each non-critical suggestion.
I have documented my reasoning in the PR comments where CodeRabbit AI suggestions were not implemented.
Test Coverage
I have written or updated tests for all new GraphQL fields and resolvers.
I have verified that the test coverage meets or exceeds 95%.
I have run the test suite locally, and all tests pass.
Other Information
This PR updates Drizzle migrations to ensure that tables related to ActionItem are consistent with GraphQL fields.
Any front-end or service code referencing the old Express routes should begin using the GraphQL queries and mutations.
Have you read the contributing guide? Yes
Summary by CodeRabbit
New Features
Refactor/Chores