-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[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
base: master
Are you sure you want to change the base?
[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. |
WalkthroughThis set of changes updates several WordPress.com integration components. It increments the version numbers and refines summary messages for the "Create Post," "Delete Post," and "Upload Media" actions to provide more informative feedback. The sources for "New Post," "New Comment," and "New Follower" are enhanced with a timer property for polling, modularized data fetching methods, and an activation hook for initialization and cursor management. Additionally, two utility methods, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Source (e.g. New Post)
participant WordPress App
participant Database
User->>Source: Activate source
Source->>WordPress App: getWordpressPosts($)
WordPress App-->>Source: Posts array
Source->>WordPress App: initialize(posts, db, "lastPostId")
WordPress App->>Database: Store first post ID as cursor
Database-->>WordPress App: Ack
WordPress App-->>Source: Initialization result
sequenceDiagram
participant User
participant Action (Create Post)
participant WordPress API
User->>Action: Trigger create-post
Action->>WordPress API: Create post with title, content, etc.
WordPress API-->>Action: Post ID and details
Action-->>User: Summary message with post title and ID
Assessment against linked issues
Possibly related PRs
Suggested labels
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/wordpress_com/actions/create-post/create-post.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/actions/delete-post/delete-post.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/actions/upload-media/upload-media.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 (6)
🚧 Files skipped from review as they are similar to previous changes (6)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ 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 |
@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