-
Notifications
You must be signed in to change notification settings - Fork 5.6k
fix(wordpress_org): Replace unreliable WPAPI.discover with explicit endpoint initialization #19665
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?
fix(wordpress_org): Replace unreliable WPAPI.discover with explicit endpoint initialization #19665
Conversation
…ndpoint initialization
- Modified getClient() to use new WPAPI({ endpoint: ... }) instead of WPAPI.discover()
- This fixes API discovery failures on certain WordPress configurations
- Updated create-post action to add tags prop and properly parse integer IDs
- Bumped create-post version to 0.0.6
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
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:
|
WalkthroughAdds tag support and a tags-listing method, changes post creation props (adds excerpt, tags, featuredMedia; removes media; status defaults to "publish"), refactors run to build conditional Changes
Sequence Diagram(s)sequenceDiagram
actor Runner
participant Action as CreatePost Action
participant App as WordPress App (getClient/listTags)
participant WPAPI as WPAPI Client
participant WP as WordPress REST API
Runner->>Action: run(props)
Action->>Action: normalize inputs, build conditional postData
Action->>WPAPI: instantiate with endpoint `${normalizedUrl}/wp-json`
Action->>WPAPI: auth(username, applicationPassword)
Action->>WP: POST /wp/v2/posts with postData
WP-->>Action: return created post
Action-->>Runner: return full response (summary includes title)
Runner->>App: request tag options (async options)
App->>WPAPI: tags().perPage(...).page(n)
WP-->>App: return tags list
App-->>Runner: supply tag options
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. 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.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @components/wordpress_org/actions/create-post/create-post.mjs:
- Around line 102-112: The Array.isArray checks in the create-post logic are
redundant because this.categories and this.tags are already typed as string[];
simplify by directly mapping parseInt over this.categories and this.tags and
assigning the resulting number arrays to postData.categories and postData.tags
(replace the conditional branches with direct mappings), ensuring you still
guard with the existing presence checks (if (this.categories &&
this.categories.length > 0) / if (this.tags && this.tags.length > 0)) and use
the same identifiers postData, this.categories, and this.tags so behavior is
unchanged.
- Around line 102-116: The parseInt calls for this.categories, this.tags, and
this.featuredMedia should use a radix and validate results to avoid NaN being
sent to the API: update the mapping in the postData assignment to call
parseInt(value, 10) for categories and tags and parseInt(this.featuredMedia, 10)
for featured_media, then filter out any values where Number.isNaN(parsed) (or
throw/return a clear validation error if any invalid input is present) so only
valid integers are assigned to postData.categories, postData.tags and
postData.featured_media.
- Line 1: Remove the unused import symbol "axios" from create-post.mjs; delete
the leftover import statement importing axios from "@pipedream/platform" and
verify there are no remaining references to axios in the file so the module no
longer includes an unused dependency.
- Around line 31-43: The status property was changed from a dynamic
propDefinition to hardcoded options, breaking custom statuses; revert it to use
propDefinition: set status to use propDefinition: { key: "listStatuses", method:
listStatuses } (or whatever pattern your app uses to call listStatuses()) so
available statuses come from listStatuses(), keep optional: true and default:
"publish" as a fallback, and ensure the propDefinition returns values like
"future" and any plugin-registered statuses.
In @components/wordpress_org/wordpress_org.app.mjs:
- Around line 180-186: The endpoint construction always prefixes `https://`
which can produce malformed URLs or disallow http; update where WPAPI is
instantiated (the `WPAPI` call that sets `endpoint`) to normalize the incoming
`url` before building the endpoint: detect and strip any existing protocol
(`http://` or `https://`) or, if `url` already contains a protocol, use it
as-is; then construct the endpoint using the cleaned/validated scheme and host
(e.g., preserve http when provided or default to https when none provided).
Ensure the normalized value is used in the `endpoint` property passed to `WPAPI`
and keep the subsequent `wp.auth({...})` call unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/wordpress_org/actions/create-post/create-post.mjscomponents/wordpress_org/wordpress_org.app.mjs
🔇 Additional comments (1)
components/wordpress_org/actions/create-post/create-post.mjs (1)
83-87: Verify intended behavior change for status field.The
statusfield now has a default value of"publish"(line 36), which means it will always be included in thepostDataeven when the user doesn't explicitly set it. This differs from the previous behavior where it was likely only sent if specified.While this provides better UX (posts are published by default), confirm this behavior change is intentional and won't cause issues for workflows expecting different default behavior.
luancazarine
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 @Melodymaifafa, I just added a small suggestion.
| annotations: { | ||
| destructiveHint: false, | ||
| openWorldHint: true, | ||
| readOnlyHint: false, | ||
| }, |
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 keep the annotations
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.
Done! Added the annotations block back. Thanks for the review!
- Remove unused axios import - Revert status to use propDefinition for dynamic status options - Simplify Array.isArray checks (redundant for typed string[]) - Add radix (10) to parseInt calls - Filter out NaN values from categories, tags, and featuredMedia
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: 3
🤖 Fix all issues with AI agents
In @components/wordpress_org/actions/create-post/create-post.mjs:
- Around line 113-121: The tags handling is over-guarded and parseInt calls lack
a radix; remove the unnecessary Array.isArray check and assign postData.tags =
this.tags.map(tag => parseInt(tag, 10)) (assuming tags is string[]), and update
the featured media conversion to postData.featured_media =
parseInt(this.featuredMedia, 10); ensure both parseInt calls include radix 10
and preserve behavior when tags/featuredMedia are undefined.
- Around line 107-111: The categories handling is overly defensive and uses
parseInt without a radix; simplify the block by assuming this.categories is an
array (remove the Array.isArray conditional) and set postData.categories =
this.categories.map(cat => parseInt(cat, 10)); update references to
this.categories and postData.categories accordingly and ensure parseInt includes
radix 10.
- Line 1: Remove the unused axios import at the top of the file: delete the line
"import { axios } from \"@pipedream/platform\";" in
components/wordpress_org/actions/create-post/create-post.mjs because all HTTP
calls use this.wordpress.createPost() and there are no other references to
axios; after removal, run lint/type-check to ensure no remaining unused-import
warnings.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/wordpress_org/actions/create-post/create-post.mjs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T22:01:11.472Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 18362
File: components/leonardo_ai/actions/generate-image/generate-image.mjs:103-105
Timestamp: 2025-09-15T22:01:11.472Z
Learning: In Pipedream components, pipedream/platform's axios implementation automatically excludes undefined values from HTTP requests, so there's no need to manually check for truthiness before including properties in request payloads.
Applied to files:
components/wordpress_org/actions/create-post/create-post.mjs
🔇 Additional comments (5)
components/wordpress_org/actions/create-post/create-post.mjs (5)
8-9: LGTM!The description is now more specific and accurate, and the version bump is appropriate for the changes made.
30-48: LGTM!The new
excerptprop and the explicit definition ofstatusprop with valid WordPress post status options are well-implemented.
67-78: LGTM!The new
tagsprop and the renamedfeaturedMediaprop are correctly typed and properly configured.
129-129: No changes needed — response.title.rendered access is safe.The WordPress REST API guarantees that successful POST /posts responses include the
title.renderedproperty. The WPAPI library throws exceptions on API errors, so line 129 is only reached after a successful API call with the expected response structure. This pattern aligns with the codebase's own prop definitions (e.g.,post?.title?.rendered), which expect this structure from successful API responses.Likely an incorrect or invalid review comment.
87-126: Verify ifcreatePost()uses Pipedream's axios before simplifying conditional checks.The code includes extensive conditional checks (lines 95–125) before adding optional fields. While Pipedream's axios implementation automatically excludes undefined values from JSON requests, this optimization only applies if
this.wordpress.createPost()internally uses Pipedream's axios client.Additionally, note that some conditionals check array length (
this.categories.length > 0,this.tags.length > 0), which prevents sending empty arrays. This may be intentional depending on the WordPress REST API's expectations.Before refactoring, confirm:
- Whether
createPost()uses Pipedream's axios wrapper- Whether the API accepts or rejects empty arrays for
categoriesandtagsIf both checks pass, the conditionals can be simplified.
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: 2
🤖 Fix all issues with AI agents
In @components/wordpress_org/actions/create-post/create-post.mjs:
- Around line 129-133: The code assumes response.title.rendered exists after
calling this.wordpress.createPost; change the summary construction to
defensively access the title using optional chaining or presence checks on
response and response.title (e.g., inspect the createPost response and use
something like response?.title?.rendered or a fallback like response?.title ||
response?.name || 'untitled') before calling $.export, and ensure createPost's
returned value (response) is validated so the exported summary never throws when
the API returns an unexpected shape.
In @components/wordpress_org/wordpress_org.app.mjs:
- Around line 181-189: The trailing slash removal is only applied when the URL
already includes a protocol; update the normalization so trailing slashes are
removed consistently by trimming the input before or after protocol handling
(e.g., operate on url or always call replace(/\/$/, "") when assigning
normalizedUrl). Ensure you modify the normalization logic around the
url.startsWith(...) branch that sets normalizedUrl so both branches produce a
URL without a trailing slash.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/wordpress_org/actions/create-post/create-post.mjscomponents/wordpress_org/wordpress_org.app.mjs
🔇 Additional comments (4)
components/wordpress_org/actions/create-post/create-post.mjs (3)
7-8: LGTM!Version bump and description update accurately reflect the changes to use the REST API directly with explicit endpoint initialization.
29-71: LGTM!The new props (excerpt, tags, commentStatus, featuredMedia) are properly defined, with appropriate use of propDefinitions where available. The status default of "publish" is a reasonable choice for post creation.
80-128: LGTM!The payload construction is well-structured:
- Conditional field inclusion keeps the payload clean
- Integer parsing with radix 10 and NaN filtering is correct for categories, tags, and featuredMedia
- WordPress REST API field naming conventions (comment_status, featured_media) are properly followed
- The logic correctly handles all optional fields
components/wordpress_org/wordpress_org.app.mjs (1)
191-197: Explicit endpoint initialization is properly supported in wpapi 1.2.2.The pattern
new WPAPI({ endpoint })followed by.auth({ username, password })is fully supported in wpapi v1.x. This approach correctly replaces the discovery mechanism and is the right fix for the identified issues.However, note that this assumes the WordPress REST API is at the standard
/wp-jsonpath. Sites using custom REST API base paths via the WordPressrest_url_prefixfilter will still fail, as the code does not account for this configuration option.
| // Normalize URL: preserve existing protocol or default to https:// | ||
| let normalizedUrl = url; | ||
| if (url.startsWith("http://") || url.startsWith("https://")) { | ||
| // URL already has protocol, use as-is | ||
| normalizedUrl = url.replace(/\/$/, ""); // Remove trailing slash if present | ||
| } else { | ||
| // No protocol provided, default to https:// | ||
| normalizedUrl = `https://${url}`; | ||
| } |
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.
Ensure consistent trailing slash removal across both branches.
The trailing slash removal only occurs in the branch where a protocol already exists. In the else branch (line 188), when https:// is prepended, a trailing slash in the original URL would persist (e.g., "example.com/" → "https://example.com/"). Consider normalizing by removing the trailing slash before prepending the protocol.
🔧 Proposed fix for consistent trailing slash handling
// Normalize URL: preserve existing protocol or default to https://
let normalizedUrl = url;
if (url.startsWith("http://") || url.startsWith("https://")) {
// URL already has protocol, use as-is
normalizedUrl = url.replace(/\/$/, ""); // Remove trailing slash if present
} else {
// No protocol provided, default to https://
- normalizedUrl = `https://${url}`;
+ normalizedUrl = `https://${url.replace(/\/$/, "")}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Normalize URL: preserve existing protocol or default to https:// | |
| let normalizedUrl = url; | |
| if (url.startsWith("http://") || url.startsWith("https://")) { | |
| // URL already has protocol, use as-is | |
| normalizedUrl = url.replace(/\/$/, ""); // Remove trailing slash if present | |
| } else { | |
| // No protocol provided, default to https:// | |
| normalizedUrl = `https://${url}`; | |
| } | |
| // Normalize URL: preserve existing protocol or default to https:// | |
| let normalizedUrl = url; | |
| if (url.startsWith("http://") || url.startsWith("https://")) { | |
| // URL already has protocol, use as-is | |
| normalizedUrl = url.replace(/\/$/, ""); // Remove trailing slash if present | |
| } else { | |
| // No protocol provided, default to https:// | |
| normalizedUrl = `https://${url.replace(/\/$/, "")}`; | |
| } |
🤖 Prompt for AI Agents
In @components/wordpress_org/wordpress_org.app.mjs around lines 181 - 189, The
trailing slash removal is only applied when the URL already includes a protocol;
update the normalization so trailing slashes are removed consistently by
trimming the input before or after protocol handling (e.g., operate on url or
always call replace(/\/$/, "") when assigning normalizedUrl). Ensure you modify
the normalization logic around the url.startsWith(...) branch that sets
normalizedUrl so both branches produce a URL without a trailing slash.
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/wordpress_org/wordpress_org.app.mjs (2)
199-273: Consider memoizing the authenticated WPAPI client to reduce repeated auth calls.All
async options()helpers callgetClient(); if that re-authenticates every time, it can add latency and rate-limit pressure. A simple per-instance cache (e.g.,this._wpClient ??= ...) would help.
174-198: Fix URL normalization to avoid//wp-jsonand/wp-json/wp-jsonendpoints.The current normalization has three bugs:
url="example.com/"produceshttps://example.com//wp-json(double slash)url="https://example.com/wp-json"produceshttps://example.com/wp-json/wp-json(duplicate path)- Protocol detection is case-sensitive, so
HTTP://example.combecomeshttps://HTTP://example.com/wp-json(malformed)Use case-insensitive protocol matching, properly strip all trailing slashes, and remove existing
/wp-jsonpaths before appending.Proposed fix
async getClient() { const { url, username, application_password: applicationPassword, } = this.$auth; - // Normalize URL: preserve existing protocol or default to https:// - let normalizedUrl = url; - if (url.startsWith("http://") || url.startsWith("https://")) { - // URL already has protocol, use as-is - normalizedUrl = url.replace(/\/$/, ""); // Remove trailing slash if present - } else { - // No protocol provided, default to https:// - normalizedUrl = `https://${url}`; - } + // Normalize base URL: trim, ensure protocol, remove trailing slashes, and prevent double `/wp-json` + let normalizedUrl = String(url ?? "").trim(); + normalizedUrl = normalizedUrl.replace(/\/+$/, ""); + if (!/^https?:\/\//i.test(normalizedUrl)) { + normalizedUrl = `https://${normalizedUrl}`; + } + normalizedUrl = normalizedUrl.replace(/\/wp-json\/?$/i, ""); const wp = new WPAPI({ endpoint: `${normalizedUrl}/wp-json`, }); return wp.auth({ username, password: applicationPassword, }); }
🤖 Fix all issues with AI agents
In @components/wordpress_org/actions/create-post/create-post.mjs:
- Around line 60-65: The current action schema for "tags" requires raw tag IDs
which are hard to discover; add a propDefinition named "tags" and implement a
listTags() helper to fetch available tags, then change the tags field in
create-post.mjs to reference propDefinitions.tags (so the UI shows selectable
tag options) while keeping the existing optional string[] type; update or add
function listTags() to call the WordPress API and return id/label pairs for the
propDefinition to use.
- Around line 79-134: The run method builds postData but doesn't normalize
this.author to an integer and directly accesses response.title.rendered; update
the author handling in the postData construction to parseInt(this.author, 10)
and only set postData.author when the parsed value is a valid number (mirroring
the categories/tags/featured_media logic), and change the $.export summary to
safely read the response (e.g., use optional chaining/fallback like
response?.title?.rendered || response?.id) so it won't throw if title is
missing; keep references to postData, this.author, this.wordpress.createPost,
response, and $.export when making the changes.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/wordpress_org/actions/create-post/create-post.mjscomponents/wordpress_org/wordpress_org.app.mjs
🔇 Additional comments (1)
components/wordpress_org/actions/create-post/create-post.mjs (1)
35-71: propDefinition correctly propagates optional properties—no action needed.Both
commentStatusandfeaturedMediainheritoptional: truefrom their source propDefinitions in wordpress_org.app.mjs and remain optional in the action, as confirmed by the conditional checks in the run method.
| tags: { | ||
| type: "string[]", | ||
| label: "Tags", | ||
| description: "Array of tag IDs to assign to the post", | ||
| optional: true, | ||
| }, |
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.
🧹 Nitpick | 🔵 Trivial
Optional: consider async options for tags (IDs are hard to discover).
Right now users must supply raw tag IDs; if you want parity with categories, adding an app propDefinitions.tags backed by a listTags() method would improve UX.
🤖 Prompt for AI Agents
In @components/wordpress_org/actions/create-post/create-post.mjs around lines 60
- 65, The current action schema for "tags" requires raw tag IDs which are hard
to discover; add a propDefinition named "tags" and implement a listTags() helper
to fetch available tags, then change the tags field in create-post.mjs to
reference propDefinitions.tags (so the UI shows selectable tag options) while
keeping the existing optional string[] type; update or add function listTags()
to call the WordPress API and return id/label pairs for the propDefinition to
use.
… use optional chaining - Added tags propDefinition with async options to fetch available tags - Added listTags() method to wordpress_org.app.mjs - Updated create-post to use tags propDefinition for UI tag selection - Added parseInt validation for author (mirroring categories/tags/featured_media) - Added optional chaining for response summary to prevent errors if title is missing
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: 2
🤖 Fix all issues with AI agents
In @components/wordpress_org/actions/create-post/create-post.mjs:
- Around line 92-97: The code silently drops non-numeric author values
(parseInt(this.author, 10) → NaN) which can confuse callers; update the block
that reads this.author (the parseInt/Number.isNaN branch) to emit a warning when
parsed value is NaN (include the original this.author value and that
postData.author will be omitted) instead of failing silently, using the module
logger if available (or console.warn) and keep the existing behavior of only
setting postData.author when parsedAuthor is a valid number; also add/update a
brief comment or docstring noting that non-numeric author inputs are ignored.
In @components/wordpress_org/wordpress_org.app.mjs:
- Around line 193-206: The URL normalization misses trimming a trailing slash
when adding the default protocol, causing duplicate slashes before /wp-json;
ensure normalizedUrl is always stripped of any trailing slash regardless of
branch (either remove trailing slash in the else branch or run a single
normalization step after the if/else), then use that cleaned normalizedUrl for
the WPAPI endpoint construction (referencing the normalizedUrl variable and the
WPAPI({ endpoint: `${normalizedUrl}/wp-json` }) usage).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/wordpress_org/actions/create-post/create-post.mjscomponents/wordpress_org/wordpress_org.app.mjs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-20T01:01:02.970Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 18744
File: components/slack_v2/actions/send-large-message/send-large-message.mjs:49-64
Timestamp: 2025-10-20T01:01:02.970Z
Learning: In components/slack_v2/actions/send-large-message/send-large-message.mjs, the metadata_event_payload prop is typed as string, so the code only needs to handle string-to-JSON parsing and does not need to handle object inputs.
Applied to files:
components/wordpress_org/actions/create-post/create-post.mjs
📚 Learning: 2024-07-04T18:11:59.822Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-07-04T18:11:59.822Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Applied to files:
components/wordpress_org/actions/create-post/create-post.mjs
🔇 Additional comments (9)
components/wordpress_org/wordpress_org.app.mjs (3)
129-141: LGTM - Tags prop definition follows existing patterns.The
tagsprop is implemented consistently with the existingcategoriesprop, using the same pagination pattern and optional chaining for safe property access.
204-210: Good change: Explicit endpoint initialization improves reliability.Replacing
WPAPI.discover()with directnew WPAPI({ endpoint })instantiation addresses the auto-discovery issues with custom CORS, redirects, and non-standard REST API paths on self-hosted sites.
227-231: LGTM - listTags method follows existing patterns.The implementation is consistent with
listCategoriesand other list methods.components/wordpress_org/actions/create-post/create-post.mjs (6)
7-8: LGTM - Version bump and description update are appropriate.The description accurately reflects the implementation change, and the version is correctly incremented.
29-47: LGTM - New props are correctly defined.The
excerpt,statuswith default, andcommentStatusprops are properly referenced from the app's propDefinitions.
60-71: LGTM - Tags and featuredMedia props are correctly defined.Props correctly reference the app's propDefinitions.
103-119: LGTM - Integer parsing for categories and tags is robust.The parsing logic correctly filters out
NaNvalues and only includes the field when valid IDs remain. This handles edge cases where string values might be passed.
132-136: LGTM - Response handling with safe access.The summary message correctly uses optional chaining to safely access
response?.title?.rendered, falling back toresponse?.idand finally a static string. This handles API responses that might not include the rendered title.
128-130: No action needed—utils.parseObjalready handles both edge cases.The utility correctly handles both JSON strings and objects: it parses strings via
JSON.parse()and returns objects as-is. Since themetaprop is typed as"object"in the app definition,this.metawill be an object at runtime, andparseObjwill return it unchanged—the correct behavior.
| if (this.author) { | ||
| const parsedAuthor = parseInt(this.author, 10); | ||
| if (!Number.isNaN(parsedAuthor)) { | ||
| postData.author = parsedAuthor; | ||
| } | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Consider silently dropping invalid author values.
When parseInt fails (returns NaN), the author field is silently omitted. If a non-numeric string is passed, the user may not realize their input was ignored. Consider logging a warning or documenting this behavior.
🤖 Prompt for AI Agents
In @components/wordpress_org/actions/create-post/create-post.mjs around lines 92
- 97, The code silently drops non-numeric author values (parseInt(this.author,
10) → NaN) which can confuse callers; update the block that reads this.author
(the parseInt/Number.isNaN branch) to emit a warning when parsed value is NaN
(include the original this.author value and that postData.author will be
omitted) instead of failing silently, using the module logger if available (or
console.warn) and keep the existing behavior of only setting postData.author
when parsedAuthor is a valid number; also add/update a brief comment or
docstring noting that non-numeric author inputs are ignored.
|
|
||
| // Normalize URL: preserve existing protocol or default to https:// | ||
| let normalizedUrl = url; | ||
| if (url.startsWith("http://") || url.startsWith("https://")) { | ||
| // URL already has protocol, use as-is | ||
| normalizedUrl = url.replace(/\/$/, ""); // Remove trailing slash if present | ||
| } else { | ||
| // No protocol provided, default to https:// | ||
| normalizedUrl = `https://${url}`; | ||
| } | ||
| return site.auth({ | ||
|
|
||
| const wp = new WPAPI({ | ||
| endpoint: `${normalizedUrl}/wp-json`, | ||
| }); |
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.
URL normalization has an edge case with trailing slashes.
The normalization logic only removes trailing slashes when a protocol is already present (line 198), but URLs without a protocol (line 201) don't get trailing slash removal. This could result in https://example.com//wp-json.
Proposed fix
// Normalize URL: preserve existing protocol or default to https://
let normalizedUrl = url;
if (url.startsWith("http://") || url.startsWith("https://")) {
// URL already has protocol, use as-is
normalizedUrl = url.replace(/\/$/, ""); // Remove trailing slash if present
} else {
// No protocol provided, default to https://
- normalizedUrl = `https://${url}`;
+ normalizedUrl = `https://${url.replace(/\/$/, "")}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Normalize URL: preserve existing protocol or default to https:// | |
| let normalizedUrl = url; | |
| if (url.startsWith("http://") || url.startsWith("https://")) { | |
| // URL already has protocol, use as-is | |
| normalizedUrl = url.replace(/\/$/, ""); // Remove trailing slash if present | |
| } else { | |
| // No protocol provided, default to https:// | |
| normalizedUrl = `https://${url}`; | |
| } | |
| return site.auth({ | |
| const wp = new WPAPI({ | |
| endpoint: `${normalizedUrl}/wp-json`, | |
| }); | |
| // Normalize URL: preserve existing protocol or default to https:// | |
| let normalizedUrl = url; | |
| if (url.startsWith("http://") || url.startsWith("https://")) { | |
| // URL already has protocol, use as-is | |
| normalizedUrl = url.replace(/\/$/, ""); // Remove trailing slash if present | |
| } else { | |
| // No protocol provided, default to https:// | |
| normalizedUrl = `https://${url.replace(/\/$/, "")}`; | |
| } | |
| const wp = new WPAPI({ | |
| endpoint: `${normalizedUrl}/wp-json`, | |
| }); |
🤖 Prompt for AI Agents
In @components/wordpress_org/wordpress_org.app.mjs around lines 193 - 206, The
URL normalization misses trimming a trailing slash when adding the default
protocol, causing duplicate slashes before /wp-json; ensure normalizedUrl is
always stripped of any trailing slash regardless of branch (either remove
trailing slash in the else branch or run a single normalization step after the
if/else), then use that cleaned normalizedUrl for the WPAPI endpoint
construction (referencing the normalizedUrl variable and the WPAPI({ endpoint:
`${normalizedUrl}/wp-json` }) usage).
luancazarine
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 @Melodymaifafa, LGTM! Ready for QA!
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
Summary
Fixes an issue where WordPress.org API calls fail due to
WPAPI.discover()being unreliable on certain WordPress configurations (e.g., sites with custom CORS settings, redirects, or non-standard REST API paths).Problem
The current implementation uses
WPAPI.discover()which attempts to auto-discover the WordPress REST API endpoint. This frequently fails on self-hosted WordPress sites, causing all WordPress actions (create-post, update-post, etc.) to throw errors.Solution
new WPAPI({ endpoint: \https://${url}/wp-json` })instead ofWPAPI.discover()`Additional Changes
tagsprop tocreate-postactioncategories,tags, andfeatured_mediaIDscreate-postversion to0.0.6Testing
Tested against a self-hosted WordPress site where
WPAPI.discover()previously failed. All API calls now succeed.Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.