Skip to content

Conversation

@gandalf-repo
Copy link

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

User description

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.


PR Type

Enhancement, Tests


Description

Major expansion of Todoist node functionality - Added support for new resources: Projects, Sections, Comments, and Labels with full CRUD operations
Enhanced task operations - Added QuickAdd functionality, duration tracking, assignee management, and deadline support
Comprehensive type validation system - Implemented robust parameter validation with new assertion utilities (assertIsString, assertIsNumber, assertIsNodeParameters)
Complete test coverage - Added extensive test suites for operation handlers, node integration tests, and type assertion utilities
Improved API flexibility - Enhanced sync request function to support configurable endpoints beyond the default /sync
Better UI/UX - Added color selection options and comprehensive field configurations for all new resources


Diagram Walkthrough

flowchart LR
  A["Todoist Node v2"] --> B["New Resources"]
  A --> C["Enhanced Operations"]
  A --> D["Type Validation"]
  A --> E["Test Coverage"]
  
  B --> B1["Projects"]
  B --> B2["Sections"]
  B --> B3["Comments"]
  B --> B4["Labels"]
  
  C --> C1["QuickAdd Tasks"]
  C --> C2["CRUD Operations"]
  C --> C3["Archive/Unarchive"]
  
  D --> D1["Parameter Assertions"]
  D --> D2["Type Guards"]
  
  E --> E1["Operation Handler Tests"]
  E --> E2["Node Integration Tests"]
  E --> E3["Workflow Tests"]
Loading

File Walkthrough

Relevant files
Tests
4 files
OperationHandler.test.ts
Add comprehensive test suite for Todoist operation handlers

packages/nodes-base/nodes/Todoist/v2/test/OperationHandler.test.ts

• Added comprehensive test suite for all Todoist operation handlers
including tasks, projects, sections, comments, and labels

Implemented mock functions for todoistApiRequest and
todoistSyncRequest with Jest
• Created test cases covering CRUD
operations, error handling, and type validation for all resources

Added tests for special operations like QuickAdd and Sync handlers
with various parameter combinations

+1232/-0
TodoistV2.node.test.ts
Add integration tests for TodoistV2 node functionality     

packages/nodes-base/nodes/Todoist/v2/test/TodoistV2.node.test.ts

• Created integration tests for the TodoistV2 node using
NodeTestHarness
• Added mock data for all resources (projects,
sections, tasks, labels, comments) with realistic sample data

Implemented nock HTTP mocking for all API endpoints and operations

Added comprehensive workflow test data covering all supported
operations and their expected outputs

+347/-0 
types.test.ts
Add comprehensive test suite for type assertion utilities

packages/nodes-base/utils/tests/types.test.ts

• Added comprehensive test suite for type assertion functions
• Tests
cover assertIsNodeParameters, assertIsString, assertIsNumber, and
assertIsArray functions
• Includes positive and negative test cases
for various parameter types and validation scenarios
• Tests complex
nested scenarios, array validation, resource-locator types, and error
handling

+388/-0 
workflow.json
Add comprehensive workflow test configuration for Todoist operations

packages/nodes-base/nodes/Todoist/v2/test/workflow.json

• Added comprehensive workflow test configuration for Todoist node

Includes test nodes for all resource types: tasks, projects, sections,
comments, and labels
• Covers all CRUD operations and special
operations like archive/unarchive, quick add
• Contains node
connections and parameter configurations for end-to-end testing

+947/-0 
Enhancement
5 files
OperationHandler.ts
Expand operation handlers with new resources and type validation

packages/nodes-base/nodes/Todoist/v2/OperationHandler.ts

• Added type validation using assertIsString, assertIsNumber,
assertIsStringOrNumber, and assertIsNodeParameters functions

Expanded CreateTaskRequest interface with new fields like assignee_id,
duration, duration_unit, deadline_date
• Added comprehensive operation
handlers for projects, sections, comments, and labels (create, read,
update, delete operations)
• Enhanced command types with additional
Todoist sync API commands and improved error handling

+658/-47
TodoistV2.node.ts
Add new resources and operations to Todoist node interface

packages/nodes-base/nodes/Todoist/v2/TodoistV2.node.ts

• Added support for new resources: Project, Section, Comment, and
Label with their respective operations
• Implemented comprehensive UI
fields for all new resources including creation, update, and filter
options
• Added TODOIST_COLOR_OPTIONS constant for color selection
across resources
• Enhanced task operations with QuickAdd
functionality and additional fields like duration, assignee, and
deadline

+960/-43
GenericFunctions.ts
Enhance sync request function with configurable endpoints

packages/nodes-base/nodes/Todoist/GenericFunctions.ts

• Modified todoistSyncRequest function to accept an optional endpoint
parameter
• Updated the function to support different sync API
endpoints beyond the default /sync
• Enhanced flexibility for calling
various Todoist sync API endpoints like /quick/add

+2/-1     
Service.ts
Expand Todoist service to support multiple resource types

packages/nodes-base/nodes/Todoist/v2/Service.ts

• Refactored service to support multiple resource types (tasks,
projects, sections, comments, labels)
• Added separate execution
methods for each resource type with dedicated handlers
• Introduced
type-safe operation types and type guards for different resources

Added QuickAddHandler for tasks and comprehensive handlers for all new
resource operations

+177/-13
types.ts
Add type assertion utilities for parameter validation       

packages/nodes-base/utils/types.ts

• Added new type assertion utility functions for parameter validation

• Implemented assertIsNodeParameters for complex object validation
with type definitions
• Added basic type assertions for strings,
numbers, arrays, and string/number combinations
• Includes support for
optional parameters, resource-locator types, and multi-type validation

+167/-0 

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.
@qodo-merge-pro qodo-merge-pro bot changed the title [Cloned] feat(Todoist Node): Add more resources and operations [Cloned] feat(Todoist Node): Add more resources and operations Aug 5, 2025
@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Aug 5, 2025

PR Reviewer Guide 🔍

(Review updated until commit 32d24a1)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Type Safety

The new type assertion system using assertIsString, assertIsNumber, etc. replaces direct type casting but may introduce runtime overhead. The validation logic should be reviewed to ensure it properly handles edge cases and provides meaningful error messages without impacting performance.

import {
	assertIsString,
	assertIsNumber,
	assertIsStringOrNumber,
	assertIsNodeParameters,
} from '../../../utils/types';
Complex Validation

The assertIsNodeParameters function with complex nested type definitions may be overly restrictive or miss edge cases. The validation schemas should be tested thoroughly to ensure they don't reject valid inputs or accept invalid ones.

assertIsNodeParameters<{
	description?: string;
	dueDateTime?: string;
	dueString?: string;
	section?: string | number;
	dueLang?: string;
	parentId?: string | number;
	priority?: string | number;
	order?: number;
	dueDate?: string;
	assigneeId?: string;
	duration?: number;
	durationUnit?: string;
	deadlineDate?: string;
}>(options, {
	description: { type: 'string', optional: true },
	dueDateTime: { type: 'string', optional: true },
	dueString: { type: 'string', optional: true },
	section: { type: ['string', 'number'], optional: true },
	dueLang: { type: 'string', optional: true },
	parentId: { type: ['string', 'number'], optional: true },
	priority: { type: ['string', 'number'], optional: true },
	order: { type: 'number', optional: true },
	dueDate: { type: 'string', optional: true },
	assigneeId: { type: 'string', optional: true },
	duration: { type: 'number', optional: true },
	durationUnit: { type: 'string', optional: true },
	deadlineDate: { type: 'string', optional: true },
});
API Consistency

The new handlers follow different patterns for parameter handling and response formatting. Some return { success: true } while others return { data }. This inconsistency should be reviewed for API uniformity and proper error handling across all operations.

// Project Handlers
export class ProjectCreateHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const name = ctx.getNodeParameter('name', itemIndex);
		assertIsString('name', name);

		const options = ctx.getNodeParameter('projectOptions', itemIndex) as IDataObject;
		assertIsNodeParameters<{
			color?: string;
			is_favorite?: boolean;
			parent_id?: string;
			view_style?: string;
		}>(options, {
			color: { type: 'string', optional: true },
			is_favorite: { type: 'boolean', optional: true },
			parent_id: { type: 'string', optional: true },
			view_style: { type: 'string', optional: true },
		});

		const body: IDataObject = {
			name,
			...options,
		};

		const data = await todoistApiRequest.call(ctx, 'POST', '/projects', body);
		return { data };
	}
}

export class ProjectDeleteHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('projectId', itemIndex);
		assertIsStringOrNumber('projectId', id);

		await todoistApiRequest.call(ctx, 'DELETE', `/projects/${id}`);
		return { success: true };
	}
}

export class ProjectGetHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('projectId', itemIndex);
		assertIsStringOrNumber('projectId', id);

		const data = await todoistApiRequest.call(ctx, 'GET', `/projects/${id}`);
		return { data };
	}
}

export class ProjectGetAllHandler implements OperationHandler {
	async handleOperation(ctx: Context, _itemIndex: number): Promise<TodoistResponse> {
		const data = await todoistApiRequest.call(ctx, 'GET', '/projects');
		return { data };
	}
}

export class ProjectUpdateHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('projectId', itemIndex);
		assertIsStringOrNumber('projectId', id);

		const updateFields = ctx.getNodeParameter('projectUpdateFields', itemIndex) as IDataObject;
		assertIsNodeParameters<{
			name?: string;
			color?: string;
			is_favorite?: boolean;
			view_style?: string;
		}>(updateFields, {
			name: { type: 'string', optional: true },
			color: { type: 'string', optional: true },
			is_favorite: { type: 'boolean', optional: true },
			view_style: { type: 'string', optional: true },
		});

		await todoistApiRequest.call(ctx, 'POST', `/projects/${id}`, updateFields);
		return { success: true };
	}
}

export class ProjectArchiveHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('projectId', itemIndex);
		assertIsStringOrNumber('projectId', id);

		await todoistApiRequest.call(ctx, 'POST', `/projects/${id}/archive`);
		return { success: true };
	}
}

export class ProjectUnarchiveHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('projectId', itemIndex);
		assertIsStringOrNumber('projectId', id);

		await todoistApiRequest.call(ctx, 'POST', `/projects/${id}/unarchive`);
		return { success: true };
	}
}

export class ProjectGetCollaboratorsHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('projectId', itemIndex);
		assertIsStringOrNumber('projectId', id);

		const data = await todoistApiRequest.call(ctx, 'GET', `/projects/${id}/collaborators`);
		return { data };
	}
}

// Section Handlers
export class SectionCreateHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const name = ctx.getNodeParameter('sectionName', itemIndex);
		assertIsString('sectionName', name);

		const projectId = ctx.getNodeParameter('sectionProject', itemIndex, undefined, {
			extractValue: true,
		});
		assertIsStringOrNumber('sectionProject', projectId);

		const options = ctx.getNodeParameter('sectionOptions', itemIndex) as IDataObject;
		assertIsNodeParameters<{
			order?: number;
		}>(options, {
			order: { type: 'number', optional: true },
		});

		const body: IDataObject = {
			name,
			project_id: projectId,
			...options,
		};

		const data = await todoistApiRequest.call(ctx, 'POST', '/sections', body);
		return { data };
	}
}

export class SectionDeleteHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('sectionId', itemIndex);
		assertIsStringOrNumber('sectionId', id);

		await todoistApiRequest.call(ctx, 'DELETE', `/sections/${id}`);
		return { success: true };
	}
}

export class SectionGetHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('sectionId', itemIndex);
		assertIsStringOrNumber('sectionId', id);

		const data = await todoistApiRequest.call(ctx, 'GET', `/sections/${id}`);
		return { data };
	}
}

export class SectionGetAllHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const filters = ctx.getNodeParameter('sectionFilters', itemIndex) as IDataObject;
		const qs: IDataObject = {};

		if (filters.project_id) {
			assertIsStringOrNumber('project_id', filters.project_id);
			qs.project_id = filters.project_id;
		}

		const data = await todoistApiRequest.call(ctx, 'GET', '/sections', {}, qs);
		return { data };
	}
}

export class SectionUpdateHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('sectionId', itemIndex);
		assertIsStringOrNumber('sectionId', id);

		const updateFields = ctx.getNodeParameter('sectionUpdateFields', itemIndex) as IDataObject;
		assertIsNodeParameters<{
			name?: string;
		}>(updateFields, {
			name: { type: 'string', optional: true },
		});

		await todoistApiRequest.call(ctx, 'POST', `/sections/${id}`, updateFields);
		return { success: true };
	}
}

// Comment Handlers
export class CommentCreateHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const taskId = ctx.getNodeParameter('commentTaskId', itemIndex);
		assertIsStringOrNumber('commentTaskId', taskId);

		const content = ctx.getNodeParameter('commentContent', itemIndex);
		assertIsString('commentContent', content);

		const body: IDataObject = {
			task_id: taskId,
			content,
		};

		const data = await todoistApiRequest.call(ctx, 'POST', '/comments', body);
		return { data };
	}
}

export class CommentDeleteHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('commentId', itemIndex);
		assertIsStringOrNumber('commentId', id);

		await todoistApiRequest.call(ctx, 'DELETE', `/comments/${id}`);
		return { success: true };
	}
}

export class CommentGetHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('commentId', itemIndex);
		assertIsStringOrNumber('commentId', id);

		const data = await todoistApiRequest.call(ctx, 'GET', `/comments/${id}`);
		return { data };
	}
}

export class CommentGetAllHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const filters = ctx.getNodeParameter('commentFilters', itemIndex) as IDataObject;
		const qs: IDataObject = {};

		if (filters.task_id) {
			assertIsStringOrNumber('task_id', filters.task_id);
			qs.task_id = filters.task_id;
		}

		if (filters.project_id) {
			assertIsStringOrNumber('project_id', filters.project_id);
			qs.project_id = filters.project_id;
		}

		const data = await todoistApiRequest.call(ctx, 'GET', '/comments', {}, qs);
		return { data };
	}
}

export class CommentUpdateHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('commentId', itemIndex);
		assertIsStringOrNumber('commentId', id);

		const updateFields = ctx.getNodeParameter('commentUpdateFields', itemIndex) as IDataObject;
		assertIsNodeParameters<{
			content?: string;
		}>(updateFields, {
			content: { type: 'string', optional: true },
		});

		await todoistApiRequest.call(ctx, 'POST', `/comments/${id}`, updateFields);
		return { success: true };
	}
}

// Label Handlers
export class LabelCreateHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const name = ctx.getNodeParameter('labelName', itemIndex);
		assertIsString('labelName', name);

		const options = ctx.getNodeParameter('labelOptions', itemIndex) as IDataObject;
		assertIsNodeParameters<{
			color?: string;
			order?: number;
			is_favorite?: boolean;
		}>(options, {
			color: { type: 'string', optional: true },
			order: { type: 'number', optional: true },
			is_favorite: { type: 'boolean', optional: true },
		});

		const body: IDataObject = {
			name,
			...options,
		};

		const data = await todoistApiRequest.call(ctx, 'POST', '/labels', body);
		return { data };
	}
}

export class LabelDeleteHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('labelId', itemIndex);
		assertIsStringOrNumber('labelId', id);

		await todoistApiRequest.call(ctx, 'DELETE', `/labels/${id}`);
		return { success: true };
	}
}

export class LabelGetHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('labelId', itemIndex);
		assertIsStringOrNumber('labelId', id);

		const data = await todoistApiRequest.call(ctx, 'GET', `/labels/${id}`);
		return { data };
	}
}

export class LabelGetAllHandler implements OperationHandler {
	async handleOperation(ctx: Context, _itemIndex: number): Promise<TodoistResponse> {
		const data = await todoistApiRequest.call(ctx, 'GET', '/labels');
		return { data };
	}
}

export class LabelUpdateHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const id = ctx.getNodeParameter('labelId', itemIndex);
		assertIsStringOrNumber('labelId', id);

		const updateFields = ctx.getNodeParameter('labelUpdateFields', itemIndex) as IDataObject;
		assertIsNodeParameters<{
			name?: string;
			color?: string;
			order?: number;
			is_favorite?: boolean;
		}>(updateFields, {
			name: { type: 'string', optional: true },
			color: { type: 'string', optional: true },
			order: { type: 'number', optional: true },
			is_favorite: { type: 'boolean', optional: true },
		});

		await todoistApiRequest.call(ctx, 'POST', `/labels/${id}`, updateFields);
		return { success: true };
	}
}

export class QuickAddHandler implements OperationHandler {
	async handleOperation(ctx: Context, itemIndex: number): Promise<TodoistResponse> {
		const text = ctx.getNodeParameter('text', itemIndex);
		assertIsString('text', text);

		const options = ctx.getNodeParameter('options', itemIndex, {}) as IDataObject;
		assertIsNodeParameters<{
			note?: string;
			reminder?: string;
			auto_reminder?: boolean;
		}>(options, {
			note: { type: 'string', optional: true },
			reminder: { type: 'string', optional: true },
			auto_reminder: { type: 'boolean', optional: true },
		});

		const body: IDataObject = { text };

		if (options.note) {
			body.note = options.note;
		}

		if (options.reminder) {
			body.reminder = options.reminder;
		}

		if (options.auto_reminder) {
			body.auto_reminder = options.auto_reminder;
		}

		const data = await todoistSyncRequest.call(ctx, body, {}, '/quick/add');

		return {
			data,
		};
	}
}

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Aug 5, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Validate resource locator property types

The resource locator validation should also check that the required properties
have valid types, not just that they exist.

packages/nodes-base/utils/types.ts [65-74]

 function assertIsResourceLocator(parameterName: string, value: unknown): void {
 	assert(
 		typeof value === 'object' &&
 			value !== null &&
 			'__rl' in value &&
 			'mode' in value &&
-			'value' in value,
+			'value' in value &&
+			typeof (value as any).__rl === 'boolean' &&
+			typeof (value as any).mode === 'string' &&
+			(typeof (value as any).value === 'string' || typeof (value as any).value === 'number'),
 		`Parameter "${parameterName}" is not a valid resource locator object`,
 	);
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that checking for property existence is insufficient and enhances the validation by also checking the types of the __rl, mode, and value properties, improving type safety.

Medium
Improve JSON parsing error handling

Add error handling for JSON parsing to provide more informative error messages
when invalid JSON is provided. This will improve debugging experience for users.

packages/nodes-base/nodes/Todoist/v2/OperationHandler.ts [519]

-const commands: Command[] = jsonParse(commandsJson);
+let commands: Command[];
+try {
+	commands = jsonParse(commandsJson);
+} catch (error) {
+	throw new ApplicationError(`Invalid JSON in commands parameter: ${error.message}`);
+}
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: This suggestion correctly proposes adding a try-catch block around jsonParse to provide a more specific error message, which improves the debugging experience for users providing invalid JSON.

Low
Fix element validator return type

The return type annotation is too broad and doesn't match the specific element
type being validated. This could lead to type safety issues.

packages/nodes-base/utils/types.ts [83-85]

-function createElementValidator(elementType: 'string' | 'number' | 'boolean') {
-	return (val: unknown): val is string | number | boolean => typeof val === elementType;
+function createElementValidator<T extends 'string' | 'number' | 'boolean'>(elementType: T) {
+	return (val: unknown): val is T extends 'string' ? string : T extends 'number' ? number : boolean => typeof val === elementType;
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the return type predicate is too broad and improves it by using generics and conditional types to provide a more specific and accurate type guard.

Low
Validate priority value range

Add validation to ensure the parsed priority value is within the valid range
(1-4). Invalid priority values could cause API errors or unexpected behavior.

packages/nodes-base/nodes/Todoist/v2/OperationHandler.ts [160-163]

-body.priority =
-	typeof options.priority === 'string'
-		? parseInt(options.priority, 10)
-		: (options.priority ?? 1);
+const priorityValue = typeof options.priority === 'string'
+	? parseInt(options.priority, 10)
+	: (options.priority ?? 1);
+body.priority = Math.min(Math.max(priorityValue, 1), 4);

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that the task priority should be within the 1-4 range and proposes clamping it, which prevents potential API errors from invalid user input.

Low
Possible issue
Use slice instead of splice

Use slice instead of splice to avoid mutating the original array. The splice
method modifies the array in place, which could cause unexpected side effects.

packages/nodes-base/nodes/Todoist/v2/OperationHandler.ts [312]

-responseData = responseData.splice(0, limit);
+responseData = responseData.slice(0, limit);
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that splice mutates the array in-place and recommends using slice for a non-mutating operation, which is a best practice to avoid side effects.

Medium
Add operation validation before handler access

Add validation to ensure the operation exists in handlers before accessing it.
This prevents runtime errors if an invalid operation is passed.

packages/nodes-base/nodes/Todoist/v2/Service.ts [45-51]

 async executeTask(
 	ctx: Context,
 	operation: TaskOperationType,
 	itemIndex: number,
 ): Promise<TodoistResponse> {
+	if (!(operation in this.handlers)) {
+		throw new Error(`Unknown task operation: ${operation}`);
+	}
 	return await this.handlers[operation].handleOperation(ctx, itemIndex);
 }
  • Apply / Chat
Suggestion importance[1-10]: 2

__

Why: The operation parameter is already strongly typed as TaskOperationType, whose values are guaranteed by TypeScript to be keys of the handlers object, making this runtime check redundant.

Low
  • More

@gandalf-repo
Copy link
Author

/review

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

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

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Aug 5, 2025

Persistent review updated to latest commit 32d24a1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants