Skip to content

Merge extract operation into media resource - #4

Merged
rafalzawadzki merged 1 commit into
masterfrom
rafalzawadzki/merge-extract-dropdown
Feb 23, 2026
Merged

rafalzawadzki merged 1 commit into
masterfrom
rafalzawadzki/merge-extract-dropdown

Conversation

@rafalzawadzki

Copy link
Copy Markdown

Summary

  • Move 'Extract Structured Data' from a separate resource category into the Media operations dropdown
  • Now appears alongside 'Get Transcript' and 'Get Metadata' for a cleaner UI
  • Bump version to 0.5.0

Test plan

  • Select Media resource in n8n
  • Verify the operations dropdown shows: Extract Structured Data, Get Metadata, Get Transcript
  • Verify Extract resource no longer appears in the top-level resource dropdown
  • Test Extract Structured Data functionality with prompt and schema inputs

Move the 'Extract Structured Data' action from a separate resource category into the Media operations dropdown alongside 'Get Transcript' and 'Get Metadata' for a cleaner UI. Bump version to 0.5.0.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Feb 23, 2026 •

Copy link
Copy Markdown

Claude finished @rafalzawadzki's task —— View job


Code Review Complete ✅

I've thoroughly reviewed the merged PR that consolidates the 'Extract Structured Data' operation into the Media resource dropdown. Here's my comprehensive analysis:

Code Review Checklist:

  • Read and analyze changed files
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Evaluate test coverage
  • Provide comprehensive feedback

✅ Code Quality & Best Practices

The refactoring demonstrates excellent code organization:

  • Clean UI consolidation: Moving extractData from a separate resource to the Media operations dropdown improves UX by grouping related functionality
  • Consistent parameter naming: Extract fields use proper prefixes (extractUrl, extractInputMode, extractPrompt, extractSchema) avoiding parameter name conflicts
  • Well-structured conditionals: The displayOptions.show logic properly controls field visibility based on resource/operation selection
  • Proper TypeScript usage: Correct typing with IDataObject, IHttpRequestMethods, and proper type assertions

⚠️ Potential Issues

1. JSON Schema Parsing Risk (Supadata.node.ts:524-528)

const schemaValue = this.getNodeParameter('extractSchema', i);
body.schema = typeof schemaValue === 'string'
    ? JSON.parse(schemaValue)
    : schemaValue;

Issue: No error handling around JSON.parse() - invalid JSON will crash the execution
Recommendation: Wrap in try-catch and provide user-friendly error message

2. Inconsistent Parameter Validation (Supadata.node.ts:514-528)

The extractUrl parameter has required: true but no runtime validation, while schema parsing lacks error handling. Consider adding URL format validation.

3. Duplicate Field Definitions

Fields like text, lang, and mode are duplicated between Media and YouTube resources (lines 221-276 and 337-392). While functionally correct, this increases maintenance burden.

🚀 Performance Considerations

  • Efficient polling: The extract operation uses configurable polling with reasonable defaults (5s interval, 300s max wait)
  • Minimal API calls: Good use of query parameters vs body data for different HTTP methods
  • Clean resource handling: No memory leaks or resource cleanup issues identified

🔒 Security Assessment

  • API credentials: Properly uses n8n's credential system via supadataApi credential type
  • Input handling: Parameters are properly typed and passed through n8n's parameter system
  • No sensitive data exposure: No hardcoded secrets or logging of sensitive information

🧪 Test Coverage Concerns

Missing test scenarios:

  • JSON schema parsing errors
  • Invalid URL formats
  • API timeout handling
  • Polling job failures
  • Parameter validation edge cases

The PR test plan focuses on UI validation but lacks backend functionality testing.

📋 Summary

This is a well-executed refactoring that improves user experience by consolidating related operations. The code follows n8n best practices and maintains backward compatibility.

Priority fixes needed:

  1. Add error handling for JSON schema parsing
  2. Consider URL validation for extract operations

Optional improvements:

  • Deduplicate common field definitions
  • Add comprehensive error messaging
  • Consider adding integration tests

Overall: Good code quality with minor security/robustness improvements needed.

@rafalzawadzki
rafalzawadzki merged commit 56823e0 into master Feb 23, 2026
1 check passed
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.

1 participant