Screenshotbase - New Action #18752
Screenshotbase - New Action #18752
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a ScreenshotBase app client with prop definitions and HTTP methods, a Take Screenshot action that validates inputs, calls the app to capture an image, writes it to a temp file, returns filename and path, and bumps package version while adding @pipedream/platform dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as Trigger
participant A as Take Screenshot Action
participant APP as ScreenshotBase App
participant API as ScreenshotBase API
participant FS as Temp Filesystem
U->>A: Invoke run()
A->>A: Validate format/quality
A->>APP: takeScreenshot(options)
APP->>APP: _makeRequest({ path: "/take", responseType: "arraybuffer" })
APP->>API: POST /v1/take with API key
API-->>APP: ArrayBuffer (image data)
APP-->>A: ArrayBuffer
A->>FS: Write temp file from buffer
A-->>U: [filename, filepath]
rect rgba(220, 235, 255, 0.25)
note over A,APP: New/changed: validation, HTTP request (arraybuffer), temp file write
end
alt Error
API-->>APP: Error
APP-->>A: Throw error
A-->>U: Error with contextual message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (3)
components/screenshotbase/actions/take-screenshot/take-screenshot.mjs(1 hunks)components/screenshotbase/package.json(2 hunks)components/screenshotbase/screenshotbase.app.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (7)
components/screenshotbase/package.json (1)
3-3: LGTM!The version bump to 0.1.0 is appropriate for the first functional release, and the @pipedream/platform dependency aligns with the axios import used in the app module.
Based on learnings: @pipedream/platform version 3.1.0 is the latest published release and is suitable for this integration.
Also applies to: 15-17
components/screenshotbase/actions/take-screenshot/take-screenshot.mjs (2)
1-15: LGTM!The imports are appropriate and the action metadata is well-structured with clear naming and annotations.
67-69: LGTM!The validation correctly enforces that quality is only applicable to jpg/jpeg formats with a clear error message.
components/screenshotbase/screenshotbase.app.mjs (4)
1-1: LGTM!The axios import from @pipedream/platform is the correct approach for Pipedream components.
Based on learnings: @pipedream/platform provides the axios client used for HTTP requests in Pipedream components.
6-52: LGTM!The propDefinitions are well-structured with clear labels, appropriate types, helpful descriptions, and correct optional flags. All properties align with the ScreenshotBase API requirements.
54-67: LGTM!The HTTP client implementation follows Pipedream best practices with a clean separation of concerns: base URL, request construction, and authentication header injection.
68-74: LGTM!The
takeScreenshotmethod correctly specifiesresponseType: "arraybuffer"for receiving binary image data and cleanly forwards additional options.
components/screenshotbase/actions/take-screenshot/take-screenshot.mjs
Outdated
Show resolved
Hide resolved
michelle0927
moved this from Doing
to Ready for PR Review
in Component (Source and Action) Backlog
GTFalcao
moved this from Ready for PR Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to Changes Required
in Component (Source and Action) Backlog
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check the test report below for more information |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/screenshotbase/actions/take-screenshot/take-screenshot.mjs (1)
60-64: syncDir prop is unused.This prop is defined but never used. The screenshot is written to
/tmp/(line 86) instead of usingthis.syncDir.Either remove the unused prop or update line 86 to use it:
- const downloadedFilepath = `/tmp/${this.filename}`; + const downloadedFilepath = `${this.syncDir}/${this.filename}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/screenshotbase/actions/take-screenshot/take-screenshot.mjs(1 hunks)components/screenshotbase/screenshotbase.app.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (4)
components/screenshotbase/screenshotbase.app.mjs (1)
1-73: LGTM! Well-structured app integration.The ScreenshotBase app implementation is solid:
- Proper use of
axiosfrom@pipedream/platform- Well-defined prop definitions with appropriate types and descriptions
- Clean API client structure with
_baseUrl(),_makeRequest(), andtakeScreenshot()methods- Correct
responseType: "arraybuffer"for binary image datacomponents/screenshotbase/actions/take-screenshot/take-screenshot.mjs (3)
67-69: LGTM! Proper validation for quality parameter.The validation correctly ensures that
qualityis only allowed when the format isjpgorjpeg, matching the API constraints.
72-84: LGTM! Correct parameter mapping to API.The parameter mapping properly converts camelCase prop names to the API's expected snake_case format (
full_page,viewport_width,viewport_height), and correctly handles the boolean-to-numeric conversion forfull_page.
86-87: Good fix! Base64 conversion removed.The response is now written directly to the file without the unnecessary base64 round-trip that was flagged in the previous review. This is the correct approach since
takeScreenshotalready returns an ArrayBuffer/Buffer.
michelle0927
moved this from Changes Required
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to Ready for Release
in Component (Source and Action) Backlog
|
Hi everyone, all test cases are passed! Ready for release! Test report |
|
/approve |
github-project-automation
bot
moved this from Ready for Release
to Done
in Component (Source and Action) Backlog
3 participants
Addresses #18663
Summary by CodeRabbit