-
Notifications
You must be signed in to change notification settings - Fork 11
Node Organization & Category-Based Interface #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Consolidate separate properties/functions files into organized index.ts modules - Adopt official n8n starter template structure with proper resource separation - Improve module organization with clean category-based operation dispatch
|
Warning Rate limit exceeded@fllppi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
WalkthroughRefactors the Postiz node from a monolithic per-operation switch to category-based modules (integrations, posts, media); adds new resource modules and routing. Fixes a trailing-comma syntax in credentials and clarifies a baseURL type assertion in generic functions. Subtitle now shows "category: operation". Changes
Sequence DiagramsequenceDiagram
participant User
participant PostizNode as Postiz.node
participant Module as Category Module\n(integrations / posts / media)
participant API as Postiz API
User->>PostizNode: Select category + operation
PostizNode->>PostizNode: Determine category
rect rgb(230,245,255)
PostizNode->>Module: execute(operation, context, itemIndex)
Module->>Module: Route to operation handler
Module->>API: postizApiRequest(endpoint, payload)
API-->>Module: Response / Error
Module-->>PostizNode: Result / Error
end
PostizNode-->>User: Output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
nodes/Postiz/resources/posts/index.ts (1)
485-489: Silent JSON parsing may hide user input errors.The try-catch block silently falls back to the raw string value when JSON parsing fails. This could hide malformed JSON input from users, making debugging difficult.
Consider logging a warning or providing user feedback:
case 'json': try { value = JSON.parse(setting.jsonValue); } catch { + // Log or warn about invalid JSON, still use raw value as fallback value = setting.jsonValue; } break;nodes/Postiz/resources/media/index.ts (1)
177-181: Silent JSON parsing may hide user input errors.Similar to the posts module, the try-catch block silently falls back to the raw string value when JSON parsing fails. This could make it difficult for users to diagnose malformed JSON in custom parameters.
Consider logging a warning:
try { body.customParams[param.key] = JSON.parse(param.value); } catch { + // Log or warn about invalid JSON, still use raw value as fallback body.customParams[param.key] = param.value; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
credentials/PostizApi.credentials.ts(1 hunks)nodes/Postiz/GenericFunctions.ts(2 hunks)nodes/Postiz/Postiz.node.ts(4 hunks)nodes/Postiz/resources/integrations/index.ts(1 hunks)nodes/Postiz/resources/media/index.ts(1 hunks)nodes/Postiz/resources/posts/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
nodes/Postiz/resources/integrations/index.ts (2)
nodes/Postiz/GenericFunctions.ts (1)
postizApiRequest(13-42)nodes/Postiz/Postiz.node.ts (1)
execute(82-129)
nodes/Postiz/Postiz.node.ts (3)
nodes/Postiz/resources/integrations/index.ts (1)
operation(4-23)nodes/Postiz/resources/media/index.ts (1)
operation(4-29)nodes/Postiz/resources/posts/index.ts (1)
operation(4-35)
nodes/Postiz/resources/media/index.ts (2)
nodes/Postiz/GenericFunctions.ts (1)
postizApiRequest(13-42)nodes/Postiz/Postiz.node.ts (1)
execute(82-129)
nodes/Postiz/resources/posts/index.ts (2)
nodes/Postiz/GenericFunctions.ts (1)
postizApiRequest(13-42)nodes/Postiz/Postiz.node.ts (1)
execute(82-129)
🔇 Additional comments (8)
nodes/Postiz/GenericFunctions.ts (1)
25-25: LGTM! Type assertion properly scoped.The type assertion now correctly applies to the entire concatenated URL string rather than just the host portion, improving type safety.
credentials/PostizApi.credentials.ts (1)
36-36: LGTM! Proper trailing comma formatting.The trailing comma follows JavaScript/TypeScript best practices and reduces future diff noise.
nodes/Postiz/resources/integrations/index.ts (1)
27-29: LGTM! Clean API request implementation.The getIntegrations function correctly uses the centralized postizApiRequest utility.
nodes/Postiz/Postiz.node.ts (2)
15-35: LGTM! Well-structured category selector.The category property provides clear organization with user-friendly names (Channel, Post, Media) mapped to internal values.
61-61: LGTM! Improved subtitle visibility.The subtitle now displays both category and operation, providing better context in the workflow UI.
nodes/Postiz/resources/posts/index.ts (2)
432-444: LGTM! Clean query parameter handling.The getPosts function correctly builds the query object with conditional customer parameter inclusion.
446-532: Complex but well-structured post creation logic.The createPost function handles nested transformations correctly, mapping the n8n UI structure to the Postiz API format. The value type switching and content item processing are properly implemented.
nodes/Postiz/resources/media/index.ts (1)
139-160: LGTM! Proper binary data handling with validation.The uploadFile function correctly validates binary data presence and throws a descriptive NodeOperationError when validation fails. The Blob and FormData construction is appropriate for file uploads.
…dia and posts nodes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
nodes/Postiz/resources/posts/index.ts (1)
188-274: Reorder valueType field to improve UX.The
valueTypeselector (lines 250-274) controls which value field is displayed (stringValue, numberValue, booleanValue, or jsonValue), but it's defined after all the value fields. This creates a confusing experience where users must scroll down to select the type before the appropriate field appears. Following n8n conventions, the controlling field should be first.Apply this diff to reorder the fields:
displayName: 'Setting', values: [ + { + displayName: 'Value Type', + name: 'valueType', + type: 'options', + options: [ + { + name: 'String', + value: 'string', + }, + { + name: 'Number', + value: 'number', + }, + { + name: 'Boolean', + value: 'boolean', + }, + { + name: 'JSON', + value: 'json', + }, + ], + default: 'string', + required: true, + description: 'Type of the setting value', + }, { displayName: 'Key', name: 'key', type: 'string', default: '', required: true, description: 'Setting key', }, { displayName: 'Value', name: 'stringValue', type: 'string', displayOptions: { show: { valueType: ['string'], }, }, default: '', required: true, description: 'String value', }, { displayName: 'Value', name: 'numberValue', type: 'number', displayOptions: { show: { valueType: ['number'], }, }, default: 0, required: true, description: 'Number value', }, { displayName: 'Value', name: 'booleanValue', type: 'boolean', displayOptions: { show: { valueType: ['boolean'], }, }, default: false, required: true, description: 'Whether the setting is enabled', }, { displayName: 'Value', name: 'jsonValue', type: 'json', displayOptions: { show: { valueType: ['json'], }, }, default: '', required: true, description: 'JSON value (object or array)', }, - { - displayName: 'Value Type', - name: 'valueType', - type: 'options', - options: [ - { - name: 'String', - value: 'string', - }, - { - name: 'Number', - value: 'number', - }, - { - name: 'Boolean', - value: 'boolean', - }, - { - name: 'JSON', - value: 'json', - }, - ], - default: 'string', - required: true, - description: 'Type of the setting value', - }, ],nodes/Postiz/resources/media/index.ts (1)
48-61: Consider validating video type values.The
videoTypeparameter accepts any string value, but the description suggests specific valid types (e.g., "image-text-slides, veo3"). If the valid video types are fixed, consider using anoptionstype instead to provide validation and better UX. If video types are dynamic or the API accepts arbitrary types, the current implementation is appropriate.If valid video types are fixed, apply this diff:
export const generateVideoType: INodeProperties = { displayName: 'Video Type', name: 'videoType', - type: 'string', + type: 'options', displayOptions: { show: { category: ['media'], operation: ['generateVideo'], }, }, + options: [ + { + name: 'Image Text Slides', + value: 'image-text-slides', + }, + { + name: 'Veo3', + value: 'veo3', + }, + // Add other valid video types + ], default: 'image-text-slides', required: true, description: 'Type of video to generate', };Otherwise, if video types are dynamic, consider documenting this in a comment or verifying the API's expectations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
nodes/Postiz/resources/integrations/index.ts(1 hunks)nodes/Postiz/resources/media/index.ts(1 hunks)nodes/Postiz/resources/posts/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- nodes/Postiz/resources/integrations/index.ts
🧰 Additional context used
🧬 Code graph analysis (2)
nodes/Postiz/resources/posts/index.ts (2)
nodes/Postiz/GenericFunctions.ts (1)
postizApiRequest(13-42)nodes/Postiz/Postiz.node.ts (1)
execute(82-129)
nodes/Postiz/resources/media/index.ts (2)
nodes/Postiz/GenericFunctions.ts (1)
postizApiRequest(13-42)nodes/Postiz/Postiz.node.ts (1)
execute(82-129)
🔇 Additional comments (4)
nodes/Postiz/resources/posts/index.ts (2)
1-1: LGTM! Past review concern addressed.The import of
NodeOperationErroris correct and properly used throughout the module (line 552), addressing the previous review feedback.
539-554: LGTM! Previous review feedback properly implemented.The execute function now correctly uses
NodeOperationErrorfor unknown operations (line 552), addressing the previous review comment. The error handling is consistent with n8n conventions and provides proper error context.nodes/Postiz/resources/media/index.ts (2)
1-1: LGTM! Past review concern addressed.The import of
NodeOperationErroris correct and properly used in the execute function (line 199), addressing the previous review feedback.
188-201: LGTM! Previous review feedback properly implemented.The execute function now correctly uses
NodeOperationErrorfor unknown operations (line 199), addressing the previous review comment. The error handling is consistent with the validation error inuploadFile(lines 146-150) and aligns with n8n best practices.
…ter for integration operations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nodes/Postiz/Postiz.node.ts (1)
37-51: Consider simplifying module wrappers.The module wrapper objects provide clear organization but could be streamlined. Since the imported modules already export both properties and functions, you could directly reference them:
-const integrationsModule = { - properties: integrations.properties, - functions: integrations, -};Then use
integrations.propertiesandintegrations.execute()directly. However, the current approach is not incorrect and does provide explicit contracts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
nodes/Postiz/Postiz.node.ts(4 hunks)nodes/Postiz/resources/integrations/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
nodes/Postiz/resources/integrations/index.ts (2)
nodes/Postiz/GenericFunctions.ts (1)
postizApiRequest(13-42)nodes/Postiz/Postiz.node.ts (1)
execute(82-129)
nodes/Postiz/Postiz.node.ts (3)
nodes/Postiz/resources/integrations/index.ts (1)
operation(4-23)nodes/Postiz/resources/media/index.ts (1)
operation(4-29)nodes/Postiz/resources/posts/index.ts (1)
operation(4-35)
🔇 Additional comments (8)
nodes/Postiz/resources/integrations/index.ts (4)
4-23: LGTM! Clean operation definition.The operation property is well-structured with appropriate conditional display logic tied to the integrations category. The naming and descriptions are clear and user-friendly.
25-25: LGTM! Properties correctly exported.The properties array properly aggregates the operation configuration for consumption by the main node.
27-29: LGTM! Consistent function signature maintained.The function correctly retrieves all connected channels. The
itemIndexparameter maintains API consistency across category modules even though it's not needed for this non-item-specific operation.
31-42: LGTM! Operation routing with proper error handling.The execute function correctly routes operations and uses
NodeOperationErrorfor unknown operations, which aligns with n8n best practices. The switch-based dispatch pattern makes it easy to add new operations in the future.nodes/Postiz/Postiz.node.ts (4)
11-35: LGTM! Excellent modular architecture.The category-based organization with clean module imports represents a significant improvement over monolithic operation handling. The user-friendly category names (Channel, Post, Media) provide clear navigation, and setting 'posts' as the default is a sensible choice.
61-61: LGTM! Clear subtitle formatting.The subtitle expression effectively communicates both the category and operation, providing users with clear context about the node's current configuration.
75-78: LGTM! Properties array correctly structured.The properties array properly positions the category selector first, followed by operation-specific properties from each module. This creates an intuitive user experience where selecting a category reveals its relevant operations.
88-105: LGTM! Category-based dispatch with consistent API.The refactored execute method successfully implements category-based routing with all module execute calls consistently passing
itemIndex(lines 95, 98, 101). This addresses the previous review concern about inconsistent function signatures and provides a clean, maintainable architecture that scales well as new categories and operations are added.
✅ Actions performedFull review triggered. |
Overview
This PR introduces a major reorganization of the Postiz n8n node with a new category-based interface and enhanced functionality, following official n8n best practices.
New Features
Category-Based Operation Selection
Channel,Post, andMediacategoriesArchitectural Improvements
Code Structure Overhaul
index.tsper resource instead of separateproperties.ts+functions.tsFile Organization
Summary by CodeRabbit
Bug Fixes
New Features
UX