-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New Components - plainly #16571
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
New Components - plainly #16571
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThis update introduces a full integration for the Plainly video automation platform, adding new actions, event sources, utility functions, and dynamic property definitions. The changes enable creating and monitoring render jobs, listing templates, and tracking new video creations or render outcomes, with robust API interactions and configurable event emission. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant PlainlyAPI
User->>Action: Trigger Create Render Job
Action->>PlainlyAPI: POST /renders (with config)
PlainlyAPI-->>Action: Render Job Created (ID)
Action-->>User: Output summary with Render Job ID
User->>Action: Trigger Get Render Status
Action->>PlainlyAPI: GET /renders/{renderId}
PlainlyAPI-->>Action: Render Status
Action-->>User: Output summary with status
User->>Action: Trigger List Templates
Action->>PlainlyAPI: GET /projects/{projectId}
PlainlyAPI-->>Action: Project Data (templates)
Action-->>User: Output templates list
Note over Sources: For event sources (e.g., new-render-completed):<br>Polling periodically, fetching new items, emitting events.
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/plainly/actions/get-render-status/get-render-status.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/plainly/actions/create-render-job/create-render-job.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/plainly/plainly.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
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 (13)
components/plainly/actions/get-render-status/get-render-status.mjs (1)
18-25: Fix space in template string for summary.There's an extra space before the closing curly brace in the template string that may cause unexpected formatting in the output summary.
- $.export("$summary", `Retrieved status ${render.state } for render ${this.renderId}`); + $.export("$summary", `Retrieved status ${render.state} for render ${this.renderId}`);components/plainly/plainly.app.mjs (2)
81-98: Consider adding error handling to API requestsThe
_makeRequestmethod should include error handling to provide more informative error messages when API requests fail. This would improve debugging and user experience._makeRequest({ $ = this, path, ...otherOpts }) { - return axios($, { + return axios($, { url: `${this._baseUrl()}${path}`, auth: { username: this.$auth.api_key, password: "", }, ...otherOpts, + }).catch((error) => { + const status = error.response?.status; + const message = error.response?.data?.message || error.message; + throw new Error(`Plainly API error (${status}): ${message}`); }); }
121-126: Consider implementing standardized pagination handlingThe
listRendersmethod accepts pagination parameters, but there's no standardized approach to handle pagination across all listing methods. Consider implementing a consistent pagination strategy, especially for methods that might return large result sets.components/plainly/actions/list-templates/list-templates.mjs (1)
18-29: Consider adding error handlingThe
runmethod should include try/catch error handling to provide more informative error messages when the API request fails.async run({ $ }) { + try { const { templates } = await this.plainly.getProject({ $, projectId: this.projectId, }); if (templates?.length) { $.export("$summary", `Fetched ${templates.length} template${templates.length === 1 ? "" : "s"}`); } return templates; + } catch (error) { + $.error(`Error fetching templates: ${error.message}`); + throw error; + } },components/plainly/sources/new-video-created/new-video-created.mjs (1)
43-49: Ensure date parsing is reliableThe
generateMetamethod usesDate.parse(item.createdDate)which assumes the API consistently returns dates in a format that JavaScript can parse. Consider using a more robust date parsing approach or adding validation.generateMeta(item) { + // Ensure we have a valid timestamp, fallback to current time if parsing fails + const timestamp = item.createdDate ? Date.parse(item.createdDate) : null; return { id: item.id, summary: `New Video Created with ID: ${item.id}`, - ts: Date.parse(item.createdDate), + ts: timestamp || Date.now(), }; },components/plainly/sources/new-render-completed/new-render-completed.mjs (1)
48-54: Ensure date parsing is reliableSimilar to the previous source component, the
generateMetamethod usesDate.parse(item.lastModified)which assumes the API consistently returns dates in a format that JavaScript can parse. Consider using a more robust date parsing approach or adding validation.generateMeta(item) { + // Ensure we have a valid timestamp, fallback to current time if parsing fails + const timestamp = item.lastModified ? Date.parse(item.lastModified) : null; return { id: item.id, summary: `New Completed Render with ID: ${item.id}`, - ts: Date.parse(item.lastModified), + ts: timestamp || Date.now(), }; },components/plainly/sources/common/base.mjs (3)
56-56: Consider a more robust timestamp parsing approachUsing
Date.parse()directly can be problematic for certain date formats. Some APIs might return dates in formats that aren't properly parsed byDate.parse().You might want to consider a more robust approach:
- const ts = Date.parse(item[tsField]); + const ts = new Date(item[tsField]).getTime();Or using a date handling library like date-fns if dealing with complex date formats.
16-21: Missing validation in state managementThe timestamp state management methods don't validate the timestamp values. Consider adding validation to ensure the timestamp is a valid number.
_getLastTs() { - return this.db.get("lastTs") || 0; + const lastTs = this.db.get("lastTs"); + return typeof lastTs === "number" ? lastTs : 0; }, _setLastTs(lastTs) { + if (typeof lastTs !== "number" || isNaN(lastTs)) { + throw new Error("Invalid timestamp value"); + } this.db.set("lastTs", lastTs); },
71-74: Potential improvement for handling large result setsWhen limiting results with
max, you truncate the array after collecting all results. For large datasets, this could lead to unnecessary memory usage.Consider limiting results as they're collected:
- if (max && results.length > max) { - results.length = max; - } do { const items = await fn(args); for (const item of items) { const ts = Date.parse(item[tsField]); if (ts > lastTs) { results.push(item); maxTs = Math.max(ts, maxTs); + if (max && results.length >= max) { + break; + } } } total = items?.length; if (args.params) { args.params.page++; } + if (max && results.length >= max) { + break; + } } while (paginate && total === args.params.size);components/plainly/actions/create-render-job/create-render-job.mjs (4)
128-128: Fix typo in labelThere's a typo in the label for the
watermarkEncodingParamsLineproperty.- label: "Watermark Encodeing Params Line", + label: "Watermark Encoding Params Line",
274-301: Make nested object handling consistentSome nested objects like
captionsandwatermarkare conditionally set based on their required properties, but others likeintegration,projectFiles, andthumbnailsare always included even if all their properties are undefined.Consider making the handling consistent by conditionally setting all nested objects:
options: { captions: this.srtFileUrl ? { captionsPosition: this.captionsPosition, captionsStyle: this.captionsStyle, srtFileUrl: this.srtFileUrl, } : undefined, - integration: { - passthrough: this.passthrough, - skipAll: this.skipAll, - }, + integration: (this.passthrough || this.skipAll) + ? { + passthrough: this.passthrough, + skipAll: this.skipAll, + } + : undefined, - projectFiles: { - uploadEnabled: this.uploadEnabled, - }, + projectFiles: this.uploadEnabled + ? { + uploadEnabled: this.uploadEnabled, + } + : undefined, - thumbnails: { - atSeconds: this.thumbnailAtSeconds, - format: this.thumbnailFormat, - frequencySeconds: this.thumbnailFrequencySeconds, - fromEncodedVideo: this.thumbnailFromEncodedVideo, - }, + thumbnails: (this.thumbnailAtSeconds || this.thumbnailFormat || + this.thumbnailFrequencySeconds || this.thumbnailFromEncodedVideo) + ? { + atSeconds: this.thumbnailAtSeconds, + format: this.thumbnailFormat, + frequencySeconds: this.thumbnailFrequencySeconds, + fromEncodedVideo: this.thumbnailFromEncodedVideo, + } + : undefined, watermark: this.watermarkUrl ? { encodingParamsLine: this.watermarkEncodingParamsLine, url: this.watermarkUrl, } : undefined, },
302-307: Consider conditionally setting outputFormatSimilar to the other nested objects, consider conditionally setting the
outputFormatobject only if at least one of its properties is defined.- outputFormat: { - attachment: this.attachment, - attachmentFileName: this.attachmentFileName, - outputModule: this.outputModule, - settingsTemplate: this.settingsTemplate, - }, + outputFormat: (this.attachment || this.attachmentFileName || + this.outputModule || this.settingsTemplate) + ? { + attachment: this.attachment, + attachmentFileName: this.attachmentFileName, + outputModule: this.outputModule, + settingsTemplate: this.settingsTemplate, + } + : undefined,
216-219: Add URL validation for webhook URLThe action accepts a webhook URL but doesn't validate it. Consider adding validation to ensure it's a valid HTTP/HTTPS URL.
Add a validation function to the run method:
async run({ $ }) { if ((this.captionsPosition || this.captionsStyle) && !this.srtFileUrl) { throw new ConfigurationError("SRT File URL is required if setting Captions Position or Style"); } if (this.watermarkEncodingParamsLine && !this.watermarkUrl) { throw new ConfigurationError("Must specify Watermark URL if specifying Watermark Encoding Params Line"); } + if (this.webhookUrl && !this.webhookUrl.match(/^https?:\/\/.+/)) { + throw new ConfigurationError("Webhook URL must be a valid HTTP/HTTPS URL"); + } + + if (this.srtFileUrl && !this.srtFileUrl.match(/^https?:\/\/.+/)) { + throw new ConfigurationError("SRT File URL must be a valid HTTP/HTTPS URL"); + } + + if (this.watermarkUrl && !this.watermarkUrl.match(/^https?:\/\/.+/)) { + throw new ConfigurationError("Watermark URL must be a valid HTTP/HTTPS URL"); + } // Rest of the run method... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
components/plainly/actions/create-render-job/create-render-job.mjs(1 hunks)components/plainly/actions/get-render-status/get-render-status.mjs(1 hunks)components/plainly/actions/list-templates/list-templates.mjs(1 hunks)components/plainly/common/utils.mjs(1 hunks)components/plainly/package.json(2 hunks)components/plainly/plainly.app.mjs(1 hunks)components/plainly/sources/common/base.mjs(1 hunks)components/plainly/sources/new-render-completed/new-render-completed.mjs(1 hunks)components/plainly/sources/new-render-failed/new-render-failed.mjs(1 hunks)components/plainly/sources/new-video-created/new-video-created.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
🔇 Additional comments (17)
components/plainly/package.json (2)
3-3: Version increment looks appropriate.The version bump from 0.0.1 to 0.1.0 follows semantic versioning principles and appropriately reflects the addition of new features in this integration.
14-17: Dependencies look correct.The addition of
@pipedream/platformdependency is appropriate for a Pipedream integration component. The version constraint (^3.0.3) allows for compatible updates.components/plainly/common/utils.mjs (1)
1-7: Helper function implementation looks good.The
optionalParseAsJSONfunction properly handles JSON parsing with a fallback to the original value if parsing fails, which is a robust approach.components/plainly/sources/new-render-failed/new-render-failed.mjs (4)
3-11: Well-structured component definition.The component metadata is clear and follows best practices for Pipedream integrations.
11-30: Props configuration looks good.The props are properly configured with optional flags and dependencies between
templateIdandprojectId.
31-48: Method implementations follow expected patterns.The methods override the base source implementation as expected, with clear resource function, arguments, and timestamp field definitions.
48-54: Verify timestamp handling for different date formats.The
generateMetamethod usesDate.parse()which can be inconsistent across browsers for some date formats. Consider using a more robust date parsing approach or ensuring the API returns dates in a consistent ISO format.Can you confirm that the
lastModifiedfield from the Plainly API always returns a standard ISO format that will be consistently parsed byDate.parse()?components/plainly/actions/get-render-status/get-render-status.mjs (2)
3-9: Component metadata looks good.The action component has appropriate key, name, description, version, and type definitions.
9-17: Props configuration is appropriate.The props definition is correct with the required
renderIdinput.components/plainly/plainly.app.mjs (1)
1-157: Well-structured API implementation with comprehensive endpointsThe Plainly app implementation provides a complete set of API methods and dynamic property definitions, making it easy to interact with the Plainly platform. The authentication mechanism and request handling are properly implemented.
components/plainly/actions/list-templates/list-templates.mjs (1)
1-31: Well-implemented template listing actionThe implementation correctly fetches templates from a specified project and returns them with a descriptive summary. The handling of singular/plural in the summary message is a nice touch.
components/plainly/sources/new-video-created/new-video-created.mjs (1)
1-51: Well-structured source component extending common baseThe implementation correctly extends the common base source and customizes it for new video events. The component properly defines required properties and overrides necessary methods.
components/plainly/sources/new-render-completed/new-render-completed.mjs (1)
1-56: Well-structured source component for completed rendersThe implementation correctly extends the common base source and customizes it for completed render events. The component properly defines optional properties with dependencies and overrides necessary methods.
components/plainly/sources/common/base.mjs (1)
4-98: Well-structured component with good separation of concernsThe base component is well-designed with clear separation of concerns, making it easy to extend for specific use cases. The approach of providing abstract methods (
getResourceFnandgenerateMeta) that must be implemented by subclasses is a good design pattern.components/plainly/actions/create-render-job/create-render-job.mjs (3)
259-265: Good validation for dependent fieldsThe validation checks ensure that required dependencies are properly configured, preventing potential errors when using the Plainly API.
317-318: Good user feedback with $summaryThe action provides clear feedback to the user about the successful creation of the render job, which is a good practice.
1-320: Well-structured component with comprehensive configuration optionsThe action provides a comprehensive set of configuration options for creating render jobs with Plainly, with good organization of related properties and appropriate validation. The dynamic property visibility based on configuration flags is a nice touch for improving the user experience.
components/plainly/sources/new-render-completed/new-render-completed.mjs
Show resolved
Hide resolved
jcortes
left a 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.
Hi @michelle0927 lgtm! Ready for QA!
Resolves #16527
Summary by CodeRabbit
New Features
Chores