Skip to content

Conversation

@gandalf-repo
Copy link

@gandalf-repo gandalf-repo commented Aug 5, 2025

Cloned Pull Request

This PR is a clone of the original PR: n8n-io/n8n#17925

Original Author: mutdmour
Original PR Number: #17925
Original Repository: n8n-io/n8n


Original Description:

Summary

Add more resources and operations to Todoist node

Screen.Recording.2025-08-03.at.15.40.57.mov

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

Files Changed:

  • modified: packages/nodes-base/nodes/Todoist/GenericFunctions.ts
  • modified: packages/nodes-base/nodes/Todoist/v2/OperationHandler.ts
  • modified: packages/nodes-base/nodes/Todoist/v2/Service.ts
  • modified: packages/nodes-base/nodes/Todoist/v2/TodoistV2.node.ts
  • added: packages/nodes-base/nodes/Todoist/v2/test/OperationHandler.test.ts
  • added: packages/nodes-base/nodes/Todoist/v2/test/TodoistV2.node.test.ts
  • added: packages/nodes-base/nodes/Todoist/v2/test/workflow.json
  • added: packages/nodes-base/utils/__tests__/types.test.ts
  • modified: packages/nodes-base/utils/types.ts

This PR was automatically cloned for AI code review benchmarking purposes.

Summary by Bito

This pull request significantly enhances the Todoist node by expanding resource support to include projects, sections, comments, labels, and quick add functionality. The changes improve API endpoint handling, add stronger type validations, and incorporate additional Todoist command operations, collectively enhancing reliability and flexibility of task management across the node.

Original PR: n8n-io/n8n#17925
Author: mutdmour

## Summary

Add more resources and operations to Todoist node

https://github.com/user-attachments/assets/4bbee79f-ea0e-4c20-9e34-e00add862cd9

<!--
Describe what the PR does and how to test.
Photos and videos are recommended.
-->

## Related Linear tickets, Github issues, and Community forum posts

<!--
Include links to **Linear ticket** or Github issue or Community forum post.
Important in order to close *automatically* and provide context to reviewers.
https://linear.app/n8n/issue/
-->
<!-- Use "closes #<issue-number>", "fixes #<issue-number>", or "resolves #<issue-number>" to automatically close issues when the PR is merged. -->

## Review / Merge checklist

- [ ] PR title and summary are descriptive. ([conventions](../blob/master/.github/pull_request_title_conventions.md)) <!--
   **Remember, the title automatically goes into the changelog.
   Use `(no-changelog)` otherwise.**
-->
- [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created.
- [ ] Tests included. <!--
   A bug is not considered fixed, unless a test is added to prevent it from happening again.
   A feature is not complete without tests.
-->
- [ ] PR Labeled with `release/backport` (if the PR is an urgent fix that needs to be backported)

---
This is a cloned PR for AI code review benchmarking.
@bito-code-review
Copy link

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
Feature Improvement - API and Operations Enhancements

GenericFunctions.ts - Updated function signature with dynamic endpoint capability for better API integration.

OperationHandler.ts - Enhanced Todoist operations by adding new handler classes for various operations and refining parameter validations.

New Feature - Enhanced Resource Integration

Service.ts - Integrated resource-specific handlers into the service execution flow to improve overall orchestration.

TodoistV2.node.ts - Expanded resource operations and UI parameters with detailed options for projects, sections, comments, labels, and quick add, enhancing overall functionality.

Testing - New Test Coverage for Todoist Node

OperationHandler.test.ts - Added and expanded test cases for quick add operations with variations for note, reminder, auto_reminder, sync commands, and error handling, ensuring comprehensive validation for Todoist operations.

TodoistV2.node.test.ts - Enhanced tests to cover extensive scenarios for projects, sections, tasks, labels, comments, and quick add functionality, reinforcing API interaction validations.

workflow.json - Included a sample workflow for integration testing.

types.test.ts - Introduced comprehensive tests for type assertions covering valid scenarios and error cases for various input types.

Other Improvements - Type Definitions Update

types.ts - Updated and refined type definitions and assertion utilities to improve type safety, consistency, and integration across the system.

Copy link

@bito-code-review bito-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Agent Run #c2f4b0

Actionable Suggestions - 4
  • packages/nodes-base/utils/types.ts - 2
    • Generic type parameter mismatch in assertIsType · Line 3-9
    • Incorrect type guard in createElementValidator · Line 83-85
  • packages/nodes-base/nodes/Todoist/v2/test/TodoistV2.node.test.ts - 1
    • Duplicate nock interceptors causing test failures · Line 276-277
  • packages/nodes-base/nodes/Todoist/v2/TodoistV2.node.ts - 1
Additional Suggestions - 7
  • packages/nodes-base/nodes/Todoist/v2/OperationHandler.ts - 2
    • Unused parameter in ProjectGetAllHandler class · Line 651-651
      The parameter `_itemIndex` in `ProjectGetAllHandler.handleOperation()` is defined but never used. Consider removing the underscore prefix if you plan to use it, or remove the parameter if it's not needed.
      Code suggestion
       @@ -651,4 +651,4 @@
        export class ProjectGetAllHandler implements OperationHandler {
      -	async handleOperation(ctx: Context, _itemIndex: number): Promise<TodoistResponse> {
      +	async handleOperation(ctx: Context): Promise<TodoistResponse> {
        		const data = await todoistApiRequest.call(ctx, 'GET', '/projects');
    • Unused parameter in LabelGetAllHandler class · Line 915-915
      The parameter `_itemIndex` in `LabelGetAllHandler.handleOperation()` is defined but never used. Consider removing the underscore prefix if you plan to use it, or remove the parameter if it's not needed.
      Code suggestion
       @@ -915,4 +915,4 @@
        export class LabelGetAllHandler implements OperationHandler {
      -	async handleOperation(ctx: Context, _itemIndex: number): Promise<TodoistResponse> {
      +	async handleOperation(ctx: Context): Promise<TodoistResponse> {
        		const data = await todoistApiRequest.call(ctx, 'GET', '/labels');
  • packages/nodes-base/nodes/Todoist/v2/TodoistV2.node.ts - 4
    • Unsupported ESLint rule in comment · Line 104-104
      The ESLint rule `n8n-nodes-base/node-param-options-type-unsorted-items` is referenced in a comment but appears to be undefined in your ESLint configuration. Consider removing the comment or ensuring the rule is properly configured.
      Code suggestion
       @@ -104,1 +104,1 @@
      -			// eslint-disable-next-line n8n-nodes-base/node-param-options-type-unsorted-items
      +			// Options are intentionally not sorted
    • Unsupported ESLint rule in comment · Line 518-519
      The ESLint rule `n8n-nodes-base/node-param-display-name-wrong-for-dynamic-multi-options` is referenced in a comment but appears to be undefined. Similar issues exist on lines 930-931.
      Code suggestion
       @@ -518,2 +518,2 @@
      -			// eslint-disable-next-line n8n-nodes-base/node-param-display-name-wrong-for-dynamic-multi-options
      -			displayName: 'Label Names',
      +			// Using 'Label Names' for dynamic multi-options
      +			displayName: 'Label Names',
    • Unsupported ESLint rule in comment · Line 602-604
      The ESLint rule `n8n-nodes-base/node-param-description-boolean-without-whether` is referenced in a comment but appears to be undefined in your ESLint configuration.
      Code suggestion
       @@ -602,3 +602,3 @@
      -					// eslint-disable-next-line n8n-nodes-base/node-param-description-boolean-without-whether
      -					description:
      -						'When this option is enabled, the default reminder will be added to the new item if it has a due date with time set',
      +					// Boolean parameter description
      +					description:
      +						'Whether the default reminder should be added to the new item if it has a due date with time set',
    • Unsupported ESLint rule in comment · Line 930-932
      The ESLint rule `n8n-nodes-base/node-param-display-name-wrong-for-dynamic-multi-options` is referenced in a comment but appears to be undefined in your ESLint configuration.
      Code suggestion
       @@ -930,3 +930,3 @@
      -					// eslint-disable-next-line n8n-nodes-base/node-param-display-name-wrong-for-dynamic-multi-options
      -					displayName: 'Label Names',
      -					name: 'labels',
      +					// Using 'Label Names' for dynamic multi-options
      +					displayName: 'Label Names',
      +					name: 'labels',
  • packages/nodes-base/nodes/Todoist/v2/Service.ts - 1
    • Unused variable used only as type · Line 135-135
      The `TASK_OPERATIONS` constant is assigned a value but only used as a type. Consider using `as const` with a type declaration or export the constant if it's needed elsewhere.
      Code suggestion
       @@ -134,13 +134,7 @@
        // Define operations as const arrays - source of truth
      -const TASK_OPERATIONS = [
      -	'create',
      -	'close',
      -	'delete',
      -	'get',
      -	'getAll',
      -	'reopen',
      -	'update',
      -	'move',
      -	'sync',
      -	'quickAdd',
      -] as const;
      +export type TaskOperationType = 'create' | 'close' | 'delete' | 'get' | 'getAll' | 'reopen' | 'update' | 'move' | 'sync' | 'quickAdd';
       
        const PROJECT_OPERATIONS = [
        	'create',
       @@ -165,7 +159,6 @@
        const LABEL_OPERATIONS = ['create', 'delete', 'get', 'getAll', 'update'] as const;
       
        // Derive types from arrays
      -export type TaskOperationType = (typeof TASK_OPERATIONS)[number];
        export type ProjectOperationType = (typeof PROJECT_OPERATIONS)[number];
        export type SectionOperationType = (typeof SECTION_OPERATIONS)[number];
        export type CommentOperationType = (typeof COMMENT_OPERATIONS)[number];
Review Details
  • Files reviewed - 8 · Commit Range: e3b26d2..e3b26d2
    • packages/nodes-base/nodes/Todoist/GenericFunctions.ts
    • packages/nodes-base/nodes/Todoist/v2/OperationHandler.ts
    • packages/nodes-base/nodes/Todoist/v2/Service.ts
    • packages/nodes-base/nodes/Todoist/v2/TodoistV2.node.ts
    • packages/nodes-base/nodes/Todoist/v2/test/OperationHandler.test.ts
    • packages/nodes-base/nodes/Todoist/v2/test/TodoistV2.node.test.ts
    • packages/nodes-base/utils/__tests__/types.test.ts
    • packages/nodes-base/utils/types.ts
  • Files skipped - 1
    • packages/nodes-base/nodes/Todoist/v2/test/workflow.json - Reason: Filter setting
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

Comment on lines +3 to +9
function assertIsType<T>(
parameterName: string,
value: unknown,
type: 'string' | 'number' | 'boolean',
): asserts value is T {
assert(typeof value === type, `Parameter "${parameterName}" is not ${type}`);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generic type parameter mismatch in assertIsType

The generic function assertIsType<T> has a type mismatch issue. The function accepts a generic type parameter T but the type parameter is constrained to only 'string' | 'number' | 'boolean'. This creates a disconnect between the generic type T and the actual runtime type checking, which could lead to incorrect type assertions. Fix: Remove the generic type parameter and use a concrete return type based on the type parameter.

Code suggestion
Check the AI-generated fix before applying
Suggested change
function assertIsType<T>(
parameterName: string,
value: unknown,
type: 'string' | 'number' | 'boolean',
): asserts value is T {
assert(typeof value === type, `Parameter "${parameterName}" is not ${type}`);
}
function assertIsType(
parameterName: string,
value: unknown,
type: 'string' | 'number' | 'boolean',
): asserts value is string | number | boolean {
assert(typeof value === type, `Parameter "${parameterName}" is not ${type}`);
}

Code Review Run #c2f4b0


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

Comment on lines +83 to +85
function createElementValidator(elementType: 'string' | 'number' | 'boolean') {
return (val: unknown): val is string | number | boolean => typeof val === elementType;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect type guard in createElementValidator

The createElementValidator function returns an incorrect type guard. It returns val is string | number | boolean but should return the specific type being validated (e.g., val is string when elementType is 'string'). This causes type safety issues in array validation. Fix: Add function overloads to return the correct specific type guard.

Code suggestion
Check the AI-generated fix before applying
Suggested change
function createElementValidator(elementType: 'string' | 'number' | 'boolean') {
return (val: unknown): val is string | number | boolean => typeof val === elementType;
}
function createElementValidator(elementType: 'string'): (val: unknown) => val is string;
function createElementValidator(elementType: 'number'): (val: unknown) => val is number;
function createElementValidator(elementType: 'boolean'): (val: unknown) => val is boolean;
function createElementValidator(elementType: 'string' | 'number' | 'boolean') {
return (val: unknown): val is any => typeof val === elementType;
}

Code Review Run #c2f4b0


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

Comment on lines +276 to +277
todoistNock.post('/rest/v2/tasks').reply(200, taskData);
todoistNock.post('/rest/v2/tasks').reply(200, taskData2);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate nock interceptors causing test failures

Duplicate nock interceptors for the same POST endpoint will cause test failures. The second interceptor on line 277 will be ignored by nock, causing tests expecting taskData2 to fail. Fix by combining into a single interceptor with .times(2) and conditional response logic.

Code suggestion
Check the AI-generated fix before applying
Suggested change
todoistNock.post('/rest/v2/tasks').reply(200, taskData);
todoistNock.post('/rest/v2/tasks').reply(200, taskData2);
todoistNock
.post('/rest/v2/tasks')
.times(2)
.reply((uri, requestBody) => [200, requestBody?.content === 'Another sample task' ? taskData2 : taskData]);

Code Review Run #c2f4b0


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

Comment on lines 951 to 1007
},
{
displayName: 'Order',
name: 'order',
type: 'number',
default: 0,
description: 'Non-zero integer used to sort tasks under the same parent',
},
{
displayName: 'Due Date',
name: 'dueDate',
type: 'string',
default: '',
placeholder: 'YYYY-MM-DD',
description: 'Specific date in YYYY-MM-DD format',
},
{
displayName: 'Assignee ID',
name: 'assigneeId',
type: 'string',
default: '',
description: 'Responsible user ID (for shared tasks)',
},
{
displayName: 'Duration',
name: 'duration',
type: 'number',
default: 0,
description: 'Positive integer for task duration (must be used with Duration Unit)',
},
{
displayName: 'Duration Unit',
name: 'durationUnit',
type: 'options',
options: [
{
name: 'Minute',
value: 'minute',
},
{
name: 'Day',
value: 'day',
},
],
default: 'minute',
description: 'Unit of time for duration (must be used with Duration)',
},
{
displayName: 'Deadline Date',
name: 'deadlineDate',
type: 'string',
default: '',
placeholder: 'YYYY-MM-DD',
description: 'Specific deadline date in YYYY-MM-DD format',
},
],
},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate field definitions in update options

Duplicate field definitions detected in task update options. Fields like order, dueDate, assigneeId, duration, durationUnit, and deadlineDate are being added to the update options but already exist in the create options. This will cause n8n configuration errors. Remove these duplicate field definitions.

Code suggestion
Check the AI-generated fix before applying
Suggested change
},
{
displayName: 'Order',
name: 'order',
type: 'number',
default: 0,
description: 'Non-zero integer used to sort tasks under the same parent',
},
{
displayName: 'Due Date',
name: 'dueDate',
type: 'string',
default: '',
placeholder: 'YYYY-MM-DD',
description: 'Specific date in YYYY-MM-DD format',
},
{
displayName: 'Assignee ID',
name: 'assigneeId',
type: 'string',
default: '',
description: 'Responsible user ID (for shared tasks)',
},
{
displayName: 'Duration',
name: 'duration',
type: 'number',
default: 0,
description: 'Positive integer for task duration (must be used with Duration Unit)',
},
{
displayName: 'Duration Unit',
name: 'durationUnit',
type: 'options',
options: [
{
name: 'Minute',
value: 'minute',
},
{
name: 'Day',
value: 'day',
},
],
default: 'minute',
description: 'Unit of time for duration (must be used with Duration)',
},
{
displayName: 'Deadline Date',
name: 'deadlineDate',
type: 'string',
default: '',
placeholder: 'YYYY-MM-DD',
description: 'Specific deadline date in YYYY-MM-DD format',
},
],
},
},
],
},

Code Review Run #c2f4b0


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

@gandalf-repo
Copy link
Author

/review

Please review this code for potential issues, security vulnerabilities, and improvements.

Review triggered at: 2025-08-05T10:44:26.667Z

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.

2 participants