Codeqr new actions #17060
Codeqr new actions #17060
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
@deusdete is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis update introduces the CodeQR integration module, enabling dynamic QR code and short link management via a standardized API client. It adds actions for creating, retrieving, and deleting QR codes and links, centralizes API communication using Axios, and documents the module's capabilities and use cases. Constants and dependencies are also defined. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ActionModule
participant CodeQRApp
participant CodeQRAPI
User->>ActionModule: Provide action inputs
ActionModule->>CodeQRApp: Call method (e.g., createLink, createQrcode)
CodeQRApp->>CodeQRAPI: Send HTTP request (Axios)
CodeQRAPI-->>CodeQRApp: Return response
CodeQRApp-->>ActionModule: Return processed data
ActionModule-->>User: Output result
Poem
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 (
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
components/codeqr/common/constants.mjs (1)
1-5: Consider making the base URL configurableHard-coding the API root prevents users from easily switching between staging, sandbox, or self-hosted environments. Exporting the string as the default is fine, but additionally reading from
process.env.CODEQR_BASE_URL(and falling back to the hard-coded value) would give consumers more flexibility without breaking existing code.-const BASE_URL = "https://api.codeqr.io"; +const BASE_URL = process.env.CODEQR_BASE_URL ?? "https://api.codeqr.io";components/codeqr/README.md (1)
1-4: Remove stray control characters and fix minor style issuesThere’s an artefact (
fileciteturn3file0) embedded after “2,500 apps”, and LanguageTool flagged wordiness around “a new record”. These don’t affect code but will show up in rendered docs.-… 2,500 apps. fileciteturn3file0 +… 2,500 apps.Optional tidy-up:
-When a new record is added … +When a record is added …Also applies to: 16-18
components/codeqr/actions/get-link-info/get-link-info.mjs (1)
5-9: Rename for grammatical clarity“Get a Link Info” reads awkwardly in the UI. Dropping the article is enough.
-name: "Get a Link Info", +name: "Get Link Info",components/codeqr/actions/get-qrcode-info/get-qrcode-info.mjs (3)
40-47: Clarify validation error messageThe check correctly blocks calls where neither
qrcodeId,externalId, nor the(domain && key)pair is supplied, but the thrown message does not explain that bothdomainandkeyare required when that lookup mode is used.
A more explicit message prevents user confusion.- "Please provide qrcodeId, externalId, or both domain and key.", + "Provide one of: qrcodeId, externalId, OR the combination of domain and key.",
48-52: Payload assembly can be reduced to a one-linerThe four conditional assignments are repetitive. Using
Object.fromEntriesor a spread with conditional property inclusion makes the intent clearer and scales better if parameters grow.-const params = {}; -qrcodeId && (params.qrcodeId = qrcodeId); -externalId && (params.externalId = externalId); -domain && (params.domain = domain); -key && (params.key = key); +const params = Object.fromEntries( + Object.entries({ qrcodeId, externalId, domain, key }) + .filter(([, v]) => v != null), +);
56-62: Nested ternary hurts readabilityThe summary construction is correct but the triple-nest ternary is hard to scan. Consider an
if/elseblock or a helper that derives the “source” string.components/codeqr/actions/create-qrcode/create-qrcode.mjs (2)
17-24:staticidentifier may surprise consumersWhile
this.staticworks in JS,staticis a reserved keyword in class bodies and highlighting/linters often flag it. Renaming toisStatic(and mapping tostaticwhen building the payload) avoids false-positive lint errors.
111-131: Loop already solves conditional assignment – duplicate code elsewhereHere you iterate over a white-list to build the payload; replicating the verbose
&&pattern in the link action (see below) is inconsistent. Consider extracting this helper to the app file so all actions share one approach.components/codeqr/actions/create-link/create-link.mjs (1)
200-226: Repetitive conditional assignments hurt maintainabilityThe long block of
field && (payload.field = field)is error-prone and diverges from the cleaner pattern used in the QR-code action. A single helper keeps things DRY:-const payload = { url }; -key && (payload.key = key); -// … 20 more lines +const optional = { + key, domain, externalId, password, flexible, + title, description, image, video, proxy, + preRedirection, pageId, pageUrl, rewrite, + ios, android, doIndex, comments, expiresAt, + expiredUrl, geo, publicStats, +}; +const payload = { url, ...Object.fromEntries( + Object.entries(optional).filter(([, v]) => v != null), +)};components/codeqr/codeqr.app.mjs (1)
67-72: Method name typo breaks import consistencyEverywhere else the camel-cased acronym is “Qrcode”, but here the deleter is
deleteQRCode(uppercase C). This inconsistency will surprise action authors.-async deleteQRCode(identifier) { +async deleteQrcode(identifier) {(and update callers)
📜 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/codeqr/README.md(1 hunks)components/codeqr/actions/create-link/create-link.mjs(1 hunks)components/codeqr/actions/create-qrcode/create-qrcode.mjs(1 hunks)components/codeqr/actions/delete-link/delete-link.mjs(1 hunks)components/codeqr/actions/delete-qrcode/delete-qrcode.mjs(1 hunks)components/codeqr/actions/get-link-info/get-link-info.mjs(1 hunks)components/codeqr/actions/get-qrcode-info/get-qrcode-info.mjs(1 hunks)components/codeqr/codeqr.app.mjs(1 hunks)components/codeqr/common/constants.mjs(1 hunks)components/codeqr/package.json(2 hunks)
🧰 Additional context used
🪛 LanguageTool
components/codeqr/README.md
[style] ~11-~11: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...th Pre-Redirect Lead Capture** When a new record is added to Airtable or a Google Sheet ...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
🔇 Additional comments (6)
components/codeqr/package.json (2)
3-3: Bump version to 0.1.0
This aligns with the introduction of the new CodeQR actions. Ensure your CHANGELOG or release notes reflect this version bump.
14-17: Add@pipedream/platformdependency
Including the Pipedream SDK is necessary for these new action components to function correctly.components/codeqr/actions/delete-qrcode/delete-qrcode.mjs (1)
36-38: Double-check helper method nameThe call uses
deleteQrcode, but the PR summary mentionsdeleteQRCode. If the helper incodeqr.app.mjsis camel-cased with a capital “C”, this will throwis not a functionat runtime.Please verify the method spelling is consistent across the app client and all action files.
components/codeqr/actions/delete-link/delete-link.mjs (1)
30-34: Good defensive validationEarly guard against missing identifiers avoids unnecessary API calls and gives clear feedback. 👍
components/codeqr/actions/create-link/create-link.mjs (2)
171-198: Lack of upfront validation for required vs. incompatible fieldsSome combinations are mutually exclusive or required together:
preRedirection === trueshould probably requirepageIdorpageUrl.externalIdmust start withext_(per docs).- If
expiresAtis set,expiredUrlis highly recommended.Adding light validation (or at least warnings) will save users failed runs.
226-228: Tag parameters typed asstring[]but coerced to plain arrayGood that you only add them when length > 0. However, the Props definition uses
"string[]", which Pipedream currently serialises as a CSV string, not a real array. Confirm this matches the API expectation; you might need to split the string first.
vunguyenhung
moved this to Ready for PR Review
in Component (Source and Action) Backlog
michelle0927
moved this from Ready for PR Review
to In Review
in Component (Source and Action) Backlog
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/codeqr/codeqr.app.mjs (2)
42-46: Auth header still hard-codesoauth_access_token
The previous review already flagged this but it remains unresolved. If the user configures the component with an API key (the primary method CodeQR advertises) requests will 401.
56-61:datais forwarded on GET / DELETE requests
The config blindly spreadsopts, sodatais sent even for methods that do not have a body – a repeat of the issue called out in the earlier review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
components/codeqr/actions/create-link/create-link.mjs(1 hunks)components/codeqr/actions/create-qrcode/create-qrcode.mjs(1 hunks)components/codeqr/actions/delete-link/delete-link.mjs(1 hunks)components/codeqr/actions/delete-qrcode/delete-qrcode.mjs(1 hunks)components/codeqr/actions/get-link-info/get-link-info.mjs(1 hunks)components/codeqr/actions/get-qrcode-info/get-qrcode-info.mjs(1 hunks)components/codeqr/codeqr.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- components/codeqr/actions/get-link-info/get-link-info.mjs
- components/codeqr/actions/get-qrcode-info/get-qrcode-info.mjs
- components/codeqr/actions/delete-link/delete-link.mjs
- components/codeqr/actions/delete-qrcode/delete-qrcode.mjs
- components/codeqr/actions/create-qrcode/create-qrcode.mjs
- components/codeqr/actions/create-link/create-link.mjs
|
@vunguyenhung Based on some errors I got while testing, I think we need to add scopes |
michelle0927
moved this from In Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to In QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from In 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: 3
🧹 Nitpick comments (2)
components/codeqr/actions/create-link/create-link.mjs (2)
175-196: Consider extracting payload construction into a shared helperThere is a long series of
foo && (payload.foo = foo)statements.
This pattern is repeated in the other actions of this PR and will be hard to maintain.A helper like
buildPayload(this, ["url", "key", …])or a simpleObject.fromEntriesfilter would make the code shorter and reduce the risk of omitting a future field.
138-143: Array prop should be typed asstring[]andoptional: trueis redundantPipedream automatically treats array props as optional unless
required: trueis set. You can drop theoptionalflag or, if you keep it, ensure consistency with other props.
📜 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 (2)
components/codeqr/actions/create-link/create-link.mjs(1 hunks)components/codeqr/package.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/codeqr/package.json
Thanks @michelle0927, I've added the 2 scopes to our official oauth app |
|
@michelle0927 AND @vunguyenhung I removed some unused props in the Create Link action and updated the Create QR Code action with new correct properties. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/codeqr/actions/create-qrcode/create-qrcode.mjs (2)
30-41:⚠️ Potential issueStill missing the “at least one content field” guard
Previous review already highlighted that a request with all content fields empty is invalid. The guard is still absent, so users can submit an empty payload and receive a runtime 400 from the API.
@@ const payload = {}; for (const key of [ @@ "expiredUrl", ]) { if (this[key] != null) payload[key] = this[key]; } + + // Guard: ensure at least one content field is supplied + if (!payload.url && !payload.text) { + throw new Error( + "Please provide at least one content field (`url` or `text`).", + ); + }Also applies to: 100-119
16-19: 🛠️ Refactor suggestionExpand
typeenum to cover all supported QR-code content typesOnly
"url"and"text"are allowed, but the API also supports"email","phone","vcard","wifi","pix", etc. End-users lose functionality and will assume the action is broken when they try any officially-documented value outside this list.
Add the full set of valid options (or source it dynamically from the app file) to prevent avoidable 4xx errors.options: [ "url", "text", + "email", + "phone", + "vcard", + "wifi", + "pix", ],
🧹 Nitpick comments (2)
components/codeqr/actions/create-qrcode/create-qrcode.mjs (2)
22-29: Rename/clarifystaticprop to avoid ambiguity
staticis a reserved keyword in JS classes and the current label (“Static/Dynamic”) inverses the usual truthy meaning (true= static). Consider renaming toisStatic(ordynamic, defaultfalse) and updating the description accordingly. This increases readability and avoids accidental misuse.
86-92: Copy-paste description mentions “short link”The
expiresAtdescription refers to “the short link”, which is misleading for a QR-code action. Replace with “QR Code” to avoid confusion.- "The date and time when the short link will expire (ISO 8601). Only available for dynamic QR Codes.", + "The date and time when the QR Code will expire (ISO 8601). Only available for dynamic QR Codes.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/codeqr/actions/create-qrcode/create-qrcode.mjs(1 hunks)components/codeqr/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/codeqr/package.json
vunguyenhung
moved this from Changes Required
to Ready for PR Review
in Component (Source and Action) Backlog
michelle0927
moved this from Ready for PR Review
to In Review
in Component (Source and Action) Backlog
michelle0927
moved this from In Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to In QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from In QA
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to In QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from In 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 |
@vunguyenhung i decided to remove the tagNames field for now. I believe that now the "Create Link" action should work. I have a question, in the reference link you provided there is a topic "Create Qrcode - Failed, Improvement needed" but when viewing the print the QR Code was created successfully. Could you provide more details about which improvements are needed? |
|
Hey @deusdete
Got it, I'll test again.
Actually this is a cache issue on Notion. Kindly ignore it. You can refresh the test report page to see the up-to-date content |
vunguyenhung
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 |
github-project-automation
bot
moved this from Ready for Release
to Done
in Component (Source and Action) Backlog
6 participants
WHY
By adding these actions, users can fully automate CodeQR workflows in Pipedream—creating, retrieving, and deleting both links and QR codes—directly from their existing integrations. This expands the automation possibilities for marketing campaigns, feedback collection, analytics reporting, and more.
Description
This PR adds the following Pipedream actions to support CodeQR link and QR code management:
Create a Link (
create-link.mjs)codeqr-create-linkGet a Link (
get-link-info.mjs)codeqr-get-linklinkId,externalId, ordomain+keyvia query parameters.Delete a Link (
delete-link.mjs)codeqr-delete-linklinkIdorexternalId.Create a QR Code (
create-qrcode.js)codeqr-create-qrcodeDelete a QR Code (
delete-qrcode.js)codeqr-delete-qrcodeqrcodeIdorexternalId.Get QR Code Info (
get-qrcode-info.js)codeqr-get-qrcode-infoqrcodeId,externalId, ordomain+keyvia query parameters.Summary by CodeRabbit