-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[Components] Gmail: Update find-email action to v0.1.2 #17039
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
@hubertpysklo is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe update increments the version from 0.1.1 to 0.1.2 and refines the Gmail "find-email" action by extracting additional headers ("from", "to", "subject"), formatting the internal date as ISO string, and restructuring payload processing based on the Changes
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/gmail/actions/find-email/find-email.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ 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 (
|
|
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:
|
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: 1
🧹 Nitpick comments (1)
components/gmail/actions/find-email/find-email.mjs (1)
64-95: Consider refactoring repetitive header extraction logic.The header extraction logic is repetitive across multiple blocks. Consider refactoring to improve maintainability:
+ // Helper function to extract header value + const getHeaderValue = (headerName) => { + const header = message.payload?.headers?.find( + (h) => h.name.toLowerCase() === headerName.toLowerCase() + ); + return header?.value; + }; + + if (this.withTextPayload) { - const messageIdHeader = message.payload?.headers?.find( - (h) => h.name.toLowerCase() === "message-id", - ); - if (messageIdHeader) { - message.message_id = messageIdHeader.value; - } + const messageId = getHeaderValue("message-id"); + if (messageId) { + message.message_id = messageId; + } // Add the internal date as a human-readable string if (message.internalDate) { message.date = new Date(parseInt(message.internalDate)).toISOString(); } - const senderHeader = message.payload?.headers?.find( - (h) => h.name.toLowerCase() === "from", - ); - if (senderHeader) { - message.sender = senderHeader.value; - } + const sender = getHeaderValue("from"); + if (sender) { + message.sender = sender; + } - const recipientHeader = message.payload?.headers?.find( - (h) => h.name.toLowerCase() === "to", - ); - if (recipientHeader) { - message.recipient = recipientHeader.value; - } + const recipient = getHeaderValue("to"); + if (recipient) { + message.recipient = recipient; + } - const subjectHeader = message.payload?.headers?.find( - (h) => h.name.toLowerCase() === "subject", - ); - if (subjectHeader) { - message.subject = subjectHeader.value; - } + const subject = getHeaderValue("subject"); + if (subject) { + message.subject = subject; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/gmail/actions/find-email/find-email.mjs(2 hunks)
🔇 Additional comments (2)
components/gmail/actions/find-email/find-email.mjs (2)
9-9: Version bump looks good.The version increment from 0.1.1 to 0.1.2 is appropriate for the new functionality being added.
63-96: Conditional header extraction introduces breaking change - ensure proper documentation.This change introduces a breaking change where
message.message_idis no longer extracted whenwithTextPayloadis false. While this aligns with the PR objectives, ensure this is properly documented in release notes and migration guides.The logic correctly groups metadata extraction with the
withTextPayloadoption and uses case-insensitive header matching as intended.
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 @hubertpysklo thanks for this great contribution! I've just left a minor suggestion and also please increment the version in components/gmail/package.json to "version": "0.3.5",
Right after you changes in your branch you can follow this commands
git add .
git commit --amend --no-edit
git push --force-with-lease origin new-find-email-propsThis will update your existing commit rather than adding new ones, keeping the PR history clean.
Thanks again for the contribution!
7e11b28 to
eef17ee
Compare
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
🔭 Outside diff range comments (1)
components/gmail/actions/find-email/find-email.mjs (1)
63-68:⚠️ Potential issue
message.message_idis extracted unconditionally – contradicts stated behaviorPR description says
message.message_idshould only be added whenwithTextPayloadistrue, but the extraction occurs before the flag check, so it runs for every message. Either the doc is wrong or this is an implementation slip.- const messageIdHeader = message.payload?.headers?.find( - (h) => h.name.toLowerCase() === "message-id", - ); - if (messageIdHeader) { - message.message_id = messageIdHeader.value.replace(/[<>]/g, ""); - } + if (this.withTextPayload) { + const messageIdHeader = message.payload?.headers?.find( + (h) => h.name.toLowerCase() === "message-id", + ); + if (messageIdHeader) { + message.message_id = messageIdHeader.value.replace(/[<>]/g, ""); + } + }Aligning code with the announced breaking-change avoids unexpected field presence for existing consumers.
♻️ Duplicate comments (1)
components/gmail/actions/find-email/find-email.mjs (1)
70-72:⚠️ Potential issueStill missing validation for
internalDate→ potentialInvalid Datevalues
Same issue flagged in earlier review:parseInt(message.internalDate)may yieldNaN, producing"Invalid Date"ISO strings.- if (message.internalDate) { - message.date = new Date(parseInt(message.internalDate)).toISOString(); - } + if (message.internalDate) { + const ts = parseInt(message.internalDate, 10); + if (!Number.isNaN(ts)) { + message.date = new Date(ts).toISOString(); + } + }
🧹 Nitpick comments (1)
components/gmail/actions/find-email/find-email.mjs (1)
60-62:newPayloadvariable created even when not used – minor cleanup
let newPayload = ""is declared before thewithTextPayloadcheck, yet remains unused in thefalsebranch. Declaring it inside thetruebranch removes a redundant allocation and clarifies intent.- let newPayload = ""; - - ... - - if (this.withTextPayload) { - if (message.payload?.body?.data && !Array.isArray(message.payload.parts)) { - const decodedBody = utils.decodeBase64Url(message.payload.body.data); - newPayload += convert(decodedBody); - } + if (this.withTextPayload) { + let newPayload = ""; + if (message.payload?.body?.data && !Array.isArray(message.payload.parts)) { + const decodedBody = utils.decodeBase64Url(message.payload.body.data); + newPayload += convert(decodedBody); + }Also applies to: 95-106
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/gmail/actions/find-email/find-email.mjs(2 hunks)components/gmail/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/gmail/package.json
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 @hubertpysklo lgtm! Ready for QA!
|
Hi everyone, all test cases are passed! Ready for release! |
eef17ee to
6b3431c
Compare
|
/approve |
6b3431c to
e70a1ed
Compare
Summary
Adds email header extraction to provide essential email metadata for all messages, improving integration with LLMs and downstream processing.
Changes
0.1.2withTextPayloadsetting):message.message_id: Extracted fromMessage-IDheader (now strips<>brackets)message.sender: Extracted fromFromheadermessage.recipient: Extracted fromToheadermessage.subject: Extracted fromSubjectheadermessage.date: ConvertsinternalDatefrom Unix timestamp to ISO 8601 string format for all messages0.3.5Why
toLowerCase()for header name comparison since email headers are case-insensitive per RFC specifications, ensuring reliable extraction regardless of how Gmail returns header names.<>brackets from Message-ID makes it cleaner for downstream processingHow to test
1. Publish the component:
2. Test with
withTextPayload = false:message.message_id(without<>brackets)message.sendermessage.recipientmessage.subjectmessage.date(in ISO 8601 format)3. Test with
withTextPayload = true:message.payloadcontains the concatenated plain text4. Edge cases:
internalDatevalues<>brackets (should remain unchanged)Summary by CodeRabbit