-
Notifications
You must be signed in to change notification settings - Fork 5.4k
[Sources] Wordpress_com #16500
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
[Sources] Wordpress_com #16500
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
@SokolovskyiK is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
Error: Could not generate a valid Mermaid diagram after multiple attempts. 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/wordpress_com/sources/new-follower/new-follower.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/wordpress_com/wordpress_com.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 (
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
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
🔭 Outside diff range comments (2)
components/wordpress_com/sources/new-follower/new-follower.mjs (2)
80-87
:⚠️ Potential issue
initialize
is called with the wrong cursor key ("lastPostId"
instead of"lastFollowerId"
).Passing the wrong key means the cursor for followers is never updated; every run after the first will treat all followers as “new” and emit duplicates.
- if (!lastFollowerId) await wordpress.initialize(followers, db, "lastPostId"); + if (!lastFollowerId) await wordpress.initialize(followers, db, "lastFollowerId");
90-110
: 🛠️ Refactor suggestionPersist the cursor as a number to avoid string/number mismatches.
maxFollowerIdTracker
may be set to a string when you copyfollower.ID
.
Persisting it as a string and later coercing withNumber()
→NaN
can break the “no-duplicate” guarantee.- if (Number(follower.ID) > maxFollowerIdTracker) { - maxFollowerIdTracker = follower.ID; + const followerIdNum = Number(follower.ID); + if (followerIdNum > maxFollowerIdTracker) { + maxFollowerIdTracker = followerIdNum; } … - await db.set("lastFollowerId", maxFollowerIdTracker); + await db.set("lastFollowerId", Number(maxFollowerIdTracker));
🧹 Nitpick comments (5)
components/wordpress_com/wordpress_com.app.mjs (1)
165-175
: Good implementation of the initialization logicThe
initialize
method provides a centralized way to handle cursor initialization for WordPress sources. It appropriately:
- Checks for empty arrays
- Logs informative messages
- Sets the database cursor value
- Returns appropriate Boolean results
This implementation will help prevent duplicate event emissions as mentioned in the PR objectives.
There's a small style issue with an extra space before the semicolon on line 174.
- return true ; + return true;components/wordpress_com/actions/upload-media/upload-media.mjs (1)
78-79
: Improved success message formatThe summary message has been reformatted to be more user-friendly with consistent formatting for warnings.
There's a parenthesis in the message that appears to be a typo:
- $.export("$summary", `Media ID "${media.ID})" has been successfully uploaded` + + $.export("$summary", `Media ID "${media.ID}" has been successfully uploaded` +components/wordpress_com/common/methods.mjs (1)
204-212
: Suppress dangling hyphen when no warnings are present
throwCustomError()
now safely defaultswarnings
to an empty array – great!
However, whenwarnings.length === 0
, the formatted string still appends"\n- "
which leaves a stray hyphen at the end of every error message.- throw new Error(` ${mainMessage} ( ${thrower} error ) : ${error.message}. ` + "\n- " + - warnings.join("\n- ")); + const warningsBlock = warnings.length + ? "\n- " + warnings.join("\n- ") + : ""; + + throw new Error( + ` ${mainMessage} ( ${thrower} error ) : ${error.message}. ${warningsBlock}`, + );This keeps the output tidy when no warnings exist while preserving the current format when they do.
components/wordpress_com/sources/new-follower/new-follower.mjs (2)
35-60
: Minor: use the destructureddb
consistently (or drop it).Inside
activate()
you destructuredb
but still referencethis.db
.
Pick one form to keep the code consistent and easier to read.- await this.db.set("lastFollowerId", null); //reset + await db.set("lastFollowerId", null); // reset(or remove
db
from the destructuring list).
29-33
: Consider adding sensible defaults to thetimer
prop.Without a default, users must always configure the interval manually, which can be error-prone.
Typical sources expose a reasonable default (e.g. every 15 min) while still allowing overrides.Example:
timer: { type: "$.interface.timer", label: "Timer", description: "How often to poll WordPress for new followers.", + default: { + intervalSeconds: 900, + }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
components/wordpress_com/actions/create-post/create-post.mjs
(2 hunks)components/wordpress_com/actions/delete-post/delete-post.mjs
(2 hunks)components/wordpress_com/actions/upload-media/upload-media.mjs
(2 hunks)components/wordpress_com/common/methods.mjs
(1 hunks)components/wordpress_com/sources/new-comment/new-comment.mjs
(3 hunks)components/wordpress_com/sources/new-follower/new-follower.mjs
(4 hunks)components/wordpress_com/sources/new-post/new-post.mjs
(3 hunks)components/wordpress_com/wordpress_com.app.mjs
(2 hunks)
🔇 Additional comments (6)
components/wordpress_com/wordpress_com.app.mjs (1)
48-54
: Well-implemented mock utility functionThis
_mock$
function provides a clean implementation using Proxy to intercept any property access and return a function that logs arguments. This will be useful for testing and debugging purposes.components/wordpress_com/actions/delete-post/delete-post.mjs (2)
7-7
: Version increment is appropriateThe version bump from 0.0.1 to 0.0.2 is appropriate for the message format change.
51-52
: Improved success message formatThe summary message has been reformatted to be more user-friendly with better wording and formatting.
components/wordpress_com/actions/create-post/create-post.mjs (2)
7-7
: Significant version incrementThe version has been incremented from 0.0.1 to 0.0.9, which is a substantial jump for what appears to be a relatively minor change in the summary message format.
Was this intended to be such a large version increment, or was it meant to be 0.0.2 to match the other actions?
83-85
: Enhanced success message with post titleThe summary message has been improved to include the post title in addition to the ID, making it more informative for users.
components/wordpress_com/actions/upload-media/upload-media.mjs (1)
8-8
: Version increment is appropriateThe version bump from 0.0.1 to 0.0.2 is appropriate for the message format change.
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 (2)
components/wordpress_com/sources/new-follower/new-follower.mjs (2)
35-60
: The activate hook implementation looks good but needs documentation.The activate hook properly initializes the component by resetting state and fetching initial data. This prevents emitting historical events on first run and handles parameter changes correctly.
Consider adding a brief JSDoc comment explaining the purpose of the activate hook, particularly highlighting how it handles reinitialization when parameters change.
hooks: { + /** + * Initializes the component by resetting state and fetching initial follower data. + * This ensures we only emit events for new followers after activation and + * properly reinitialize when site parameters change. + */ async activate() {
52-58
: Consider conditional initialization in activate hook.The activate hook always resets the lastFollowerId to null, which means any deployment will reset tracking. Consider if this is the intended behavior, especially for updates that don't change functionality.
You might want to use conditional logic that only resets when certain props change:
- await this.db.set("lastFollowerId", null); //reset + // Only reset if site has changed + const currentSite = await this.db.get("currentSite"); + if (currentSite !== site) { + await this.db.set("lastFollowerId", null); + await this.db.set("currentSite", site); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/airtable_oauth/package.json
(1 hunks)components/wordpress_com/actions/create-post/create-post.mjs
(2 hunks)components/wordpress_com/actions/delete-post/delete-post.mjs
(2 hunks)components/wordpress_com/actions/upload-media/upload-media.mjs
(2 hunks)components/wordpress_com/sources/new-comment/new-comment.mjs
(3 hunks)components/wordpress_com/sources/new-follower/new-follower.mjs
(4 hunks)components/wordpress_com/sources/new-post/new-post.mjs
(3 hunks)components/wordpress_com/wordpress_com.app.mjs
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/airtable_oauth/package.json
🚧 Files skipped from review as they are similar to previous changes (6)
- components/wordpress_com/actions/delete-post/delete-post.mjs
- components/wordpress_com/actions/create-post/create-post.mjs
- components/wordpress_com/actions/upload-media/upload-media.mjs
- components/wordpress_com/wordpress_com.app.mjs
- components/wordpress_com/sources/new-comment/new-comment.mjs
- components/wordpress_com/sources/new-post/new-post.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (4)
components/wordpress_com/sources/new-follower/new-follower.mjs (4)
7-7
: Version increment looks appropriate.The version bump from 0.0.1 to 0.0.4 aligns with the significant functionality changes in this component.
10-19
: Good refactoring for improved modularity.Extracting the follower fetching logic into a dedicated method improves code organization and maintainability. This encapsulation makes the code more readable and easier to maintain.
29-33
: User-configurable polling frequency is a good addition.Adding a timer property allows users to control how frequently the component checks for new followers, which improves flexibility and resource utilization.
75-75
: Code refactoring improves consistency.Using the extracted method aligns with the architectural improvements made throughout the component.
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.
Looks good, I just updated some of the versions. I know when you're testing, you probably have to increase them more. However, for publishing, we want the versions just one digit higher than they were previously published at.
@michelle0927 |
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.
Hello @SokolovskyiK, could you check my comments here?
$.export("$summary", | ||
`Post “${this.title}” is successfully created with ID “${response?.ID}”` + | ||
"\n- " + warnings.join("\n- ")); |
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.
Please remove the warnings stuff here. Just export the summary as plain text
$.export("$summary", | |
`Post “${this.title}” is successfully created with ID “${response?.ID}”` + | |
"\n- " + warnings.join("\n- ")); | |
$.export("$summary", | |
`Post “${this.title}” is successfully created with ID “${response?.ID}”`); |
$.export("$summary", `Post ID “${response?.ID}” has been successfully deleted` + | ||
"\n- " + warnings.join("\n- ")); |
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.
Please remove the warnings stuff here. Just export the summary as plain text
$.export("$summary", `Post ID “${response?.ID}” has been successfully deleted` + | |
"\n- " + warnings.join("\n- ")); | |
$.export("$summary", `Post ID “${response?.ID}” has been successfully deleted`); |
`Media ID “${media.ID}” has been successfully uploaded`; | ||
$.export("$summary", `Media ID “${media.ID}” has been successfully uploaded` + | ||
"\n- " + warnings.join("\n- ")); |
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.
Please remove the warnings stuff here. Just export the summary as plain text
`Media ID “${media.ID}” has been successfully uploaded`; | |
$.export("$summary", `Media ID “${media.ID}” has been successfully uploaded` + | |
"\n- " + warnings.join("\n- ")); | |
$.export("$summary", `Media ID “${media.ID}” has been successfully uploaded`); |
timer: { | ||
type: "$.interface.timer", | ||
label: "Timer", | ||
description: "How often to poll WordPress for new comments.", | ||
}, |
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.
Please add default timer
timer: { | |
type: "$.interface.timer", | |
label: "Timer", | |
description: "How often to poll WordPress for new comments.", | |
}, | |
timer: { | |
type: "$.interface.timer", | |
label: "Timer", | |
description: "How often to poll WordPress for new comments.", | |
default: { | |
// import { DEFAULT_POLLING_SOURCE_TIMER_INTERVAL } from "@pipedream/platform"; | |
intervalSeconds: DEFAULT_POLLING_SOURCE_TIMER_INTERVAL, | |
}, | |
}, |
timer: { | ||
type: "$.interface.timer", | ||
label: "Timer", | ||
description: "How often to poll WordPress for new followers.", | ||
}, |
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.
Please add default timer
timer: { | |
type: "$.interface.timer", | |
label: "Timer", | |
description: "How often to poll WordPress for new followers.", | |
}, | |
timer: { | |
type: "$.interface.timer", | |
label: "Timer", | |
description: "How often to poll WordPress for new followers.", | |
default: { | |
// import { DEFAULT_POLLING_SOURCE_TIMER_INTERVAL } from "@pipedream/platform"; | |
intervalSeconds: DEFAULT_POLLING_SOURCE_TIMER_INTERVAL, | |
}, | |
}, |
timer: { | ||
type: "$.interface.timer", | ||
label: "Timer", | ||
description: "How often to poll WordPress for new posts.", | ||
}, |
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.
Please add default timer
timer: { | |
type: "$.interface.timer", | |
label: "Timer", | |
description: "How often to poll WordPress for new posts.", | |
}, | |
timer: { | |
type: "$.interface.timer", | |
label: "Timer", | |
description: "How often to poll WordPress for new posts.", | |
default: { | |
// import { DEFAULT_POLLING_SOURCE_TIMER_INTERVAL } from "@pipedream/platform"; | |
intervalSeconds: DEFAULT_POLLING_SOURCE_TIMER_INTERVAL, | |
}, | |
}, |
_mock$() { | ||
return new Proxy({}, { | ||
get() { | ||
return (...args) => console.log(...args); | ||
}, | ||
}); | ||
}, |
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.
Do we need this?
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.
Just recently found out that - no, we don't=)
Will remove it
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "@pipedream/airtable_oauth", | |||
"version": "0.5.0", | |||
"version": "0.5.1", |
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.
Please revert this change, it is not related to wordpress.com app
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.
Ok
@vunguyenhung Hi. Sure, will do. |
7ce16de
to
bbf87b2
Compare
Hi. @vunguyenhung Components check did not like it |
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
♻️ Duplicate comments (1)
components/wordpress_com/sources/new-follower/new-follower.mjs (1)
47-54
: Added timer configuration as requested.The timer property has been implemented with the recommended default polling interval as requested in previous review comments.
🧹 Nitpick comments (2)
components/wordpress_com/wordpress_com.app.mjs (1)
165-175
: Well-structured initialization method for cursor management.The
initialize
method centralizes cursor initialization logic across source components, reducing code duplication and standardizing the "last seen" ID tracking process.Minor formatting issue:
- return true ; + return true;components/wordpress_com/sources/new-follower/new-follower.mjs (1)
56-72
: Effective initialization implementation.The
activate
hook properly initializes the source by resetting the cursor and fetching initial followers. This ensures the source only emits events for new followers after activation, addressing a key PR objective.Consider adding a comment explaining the reset purpose:
- await this.db.set("lastFollowerId", null); //reset + // Reset cursor when source is activated or parameters are changed + await this.db.set("lastFollowerId", null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/wordpress_com/sources/new-follower/new-follower.mjs
(2 hunks)components/wordpress_com/wordpress_com.app.mjs
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (7)
components/wordpress_com/wordpress_com.app.mjs (2)
142-142
: Improved follower API integration with type filtering.The
getWordpressFollowers
method now supports filtering followers by type through the newtype
parameter, which is properly passed to the API endpoint.Also applies to: 147-147
157-164
: Appropriately implemented FormData validation.This utility method correctly checks if an input is a FormData object by verifying it has the expected methods and properties. As confirmed in the past review comments, this replaces the previously imported common method.
components/wordpress_com/sources/new-follower/new-follower.mjs (5)
2-2
: Properly imported platform constant and incremented version.The version increment and import of the default polling interval constant are appropriate for the changes being made.
Also applies to: 8-8
11-21
: Well-organized method encapsulation.Creating a dedicated
getWordpressFollowers
method improves code organization and maintainability by centralizing the API call logic.
31-46
: Good implementation of follower type selection.The new type property with clear options and descriptions gives users flexibility to choose between WordPress.com followers and email-only followers.
74-74
: Improved run method signature and API call.Updated to use the new centralized method for fetching followers, which improves code maintainability.
Also applies to: 80-80
86-86
:✅ Verification successful
Simplified first-run logic.
The explicit first-run cursor initialization has been replaced with a single call to
wordpress.initialize
, which centralizes and standardizes this logic.
🏁 Script executed:
#!/bin/bash # Verify that all other sources use consistent initialization patterns grep -r "wordpress.initialize" --include="*.mjs" components/wordpress_com/sources/Length of output: 875
Simplified first-run logic verified
All WordPress sources now use a single call to
wordpress.initialize(...)
when theirlast*Id
is unset, centralizing cursor setup consistently:
- components/wordpress_com/sources/new-comment/new-comment.mjs
- components/wordpress_com/sources/new-post/new-post.mjs
- components/wordpress_com/sources/new-follower/new-follower.mjs
No further changes needed.
Hi @vunguyenhung, this one needs your approval to be merged. |
Hey @michelle0927 , approved |
@vunguyenhung
Fixes #15127
Fixed sources: new post, comment, and follower.
Tested via pd dev — everything works.
Please note that I added activate() so that when the source is deployed, it fetches the latest comment, post, or follower to initialize properly and emit events only for items with higher IDs.
If the user changes something like the site or type, the last comment/post/follower ID is reinitialized without emitting events.
For example, if they change the post ID to one where the last comment ID is lower, it could otherwise cause unpredictable behavior.
To prevent this, activate() reinitializes on any change.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores