feature: selectable docs URL structure#310
feature: selectable docs URL structure#310sapayth wants to merge 3 commits intoweDevsOfficial:developfrom
Conversation
WalkthroughConsolidates breadcrumb construction into a new helper, updates breadcrumb rendering to use it, adds two new UI components (ListboxSelect, PreviewDropdown), exposes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. 🔧 PHPStan (2.1.51)PHP Fatal error: Uncaught Error: Undefined constant "ABSPATH" in /includes/functions.php:421 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
includes/functions.php (2)
149-175: Move the 25-char truncation out of the shared helper.
wedocs_apply_short_content( $post_title, 25 )is applied to ancestor titles inside the helper, but:
- The block renderer (
src/blocks/Breadcrumb/render.php) has its own user-configurablemaxTitleLengthand is now silently capped at 25 for ancestors.wedocs_get_breadcrumb_item()(line 251) also callswedocs_apply_short_content( $label, 25 ), so the legacy caller truncates again — harmless in practice but redundant.- The
currentcrumb is not truncated, so there's asymmetry between ancestors and the current doc.Consider returning full titles from
wedocs_get_doc_breadcrumb_trail()and letting each caller apply its own truncation policy. If you'd like to keep backward compatibility, add an opt-in arg (e.g.$args['truncate']) instead of truncating unconditionally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/functions.php` around lines 149 - 175, The breadcrumb helper wedocs_get_doc_breadcrumb_trail currently truncates ancestor titles via wedocs_apply_short_content( ..., 25 ), causing a hard 25-char cap for block renderer (src/blocks/Breadcrumb/render.php) and redundant truncation in wedocs_get_breadcrumb_item; change wedocs_get_doc_breadcrumb_trail to return full titles (remove the unconditional wedocs_apply_short_content calls for ancestors) and instead add an optional argument (e.g. $args['truncate'] or $args['maxTitleLength']) to opt in to truncation for backward compatibility; update callers (notably src/blocks/Breadcrumb/render.php and wedocs_get_breadcrumb_item) to apply their own truncation policy using wedocs_apply_short_content when needed.
133-143: Add a depth guard to the ancestor walk.The
while ( $parent_id )loop relies on a well-formed post-parent chain. If thedocshierarchy ever contains a cycle (manual DB edit, faulty import, etc.), this loop runs forever on every doc page load. Since this is now a hot path for both the block and legacy breadcrumb rendering, consider a cheap guard:- while ( $parent_id ) { - $parent = get_post( $parent_id ); - if ( ! $parent ) { - break; - } - $ancestors[] = $parent; - $parent_id = (int) $parent->post_parent; - } + $seen = []; + while ( $parent_id && ! isset( $seen[ $parent_id ] ) ) { + $seen[ $parent_id ] = true; + $parent = get_post( $parent_id ); + if ( ! $parent ) { + break; + } + $ancestors[] = $parent; + $parent_id = (int) $parent->post_parent; + }Alternatively,
get_post_ancestors( $post )already handles this internally and may be simpler than walking manually.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/functions.php` around lines 133 - 143, The manual ancestor-walk in the block (variables $ancestors, $parent_id and loop using get_post) lacks a depth guard and can infinite-loop on cyclical parent chains; replace the manual loop with WordPress's safe helper get_post_ancestors($post) or add a cheap max-iterations guard (e.g., limit iterations to a sane constant like 50) around the while ($parent_id) loop and break if exceeded, ensuring you still populate $ancestors with the found parent objects via get_post for consistency with existing code that expects post objects.src/components/Settings/GeneralSettings.js (1)
81-143: New settings block LGTM; minor wording nit.Flow, tooltip, and example-URL preview look correct, and
siteUrlnormalization handles both trailing-slash and the missing-localized-var case gracefully.Optional: the helper/class docblocks describe the segments as
{top-doc}/{section}/{article}, while the UI preview uses{doc}. Consider aligning to{top-doc}for consistency with the backend docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/GeneralSettings.js` around lines 81 - 143, The example URL preview in GeneralSettings.js currently uses the placeholder "{doc}" which is inconsistent with the backend docs that use "{top-doc}"; update the template strings where siteUrl is used (the inline IIFE that builds the example URL using generalSettings and siteUrl) to replace "{doc}" with "{top-doc}" in both the 'after_doc' and default branches so the UI preview matches the helper/class docblocks; this affects the example string shown alongside DocsUrlStructureSelect and references the generalSettings variable and siteUrl computation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@includes/Docs_URL_Structure.php`:
- Around line 430-452: The returned reserved-segment escaping currently calls
preg_quote without a delimiter, which allows '#' characters in third-party
filter values to break the regex built later; update the return in
Docs_URL_Structure (the array_map call that wraps preg_quote over
array_values($filtered)) to pass the regex delimiter '#' as the second argument
to preg_quote so segments are escaped with respect to that delimiter before
being used in the pattern.
- Around line 401-404: get_doc_by_path currently returns any WP_Post from
get_page_by_path regardless of status, which allows non-published docs to be
resolved or redirected by resolve_request() and maybe_redirect_legacy_url();
update get_doc_by_path so after confirming $post instanceof WP_Post it also
checks $post->post_status === 'publish' (mirroring the guard in
filter_doc_permalink()) and return null for any non-published status, ensuring
only published docs are returned to callers.
- Around line 103-115: The rewrite patterns in the add_rewrite_rule calls and
the resolve_request() fallback must account for the permalink "front" prefix
when the docs post type uses with_front=true; update the two add_rewrite_rule
patterns (the nested path and bare hub rules) and the resolve_request() logic to
prepend the front base (use the WP_Rewrite front value, e.g.
trim($wp_rewrite->front, '/') and preg_quote it when building the regex) so the
regex becomes '^' . (front ? preg_quote(front) . '/' : '') . '(?!(?:' .
implode('|', $reserved) . ')/)([^/]+)/' . $slug_regex . '/(.+?)/?$' (and the
shorter hub variant), and mirror the same front-aware prefix when parsing the
request in resolve_request(); alternatively, disable with_front in the post type
registration if you prefer not to include the front prefix.
- Around line 238-247: The code is using $_SERVER['REQUEST_URI'] directly which
on subdirectory installs contains the site base path and breaks matching; update
the normalization logic that computes $request_uri/$request_path by removing the
site's home path prefix (compare against parse_url( home_url(), PHP_URL_PATH ))
before trimming so patterns in get_structure() and comparisons using
$current_url and $rewrite_slug match correctly; specifically, compute the site's
base path, strip that prefix from $request_uri if present, then continue
building $request_path and $current_url so get_structure(), get_rewrite_slug(),
and subsequent 301 redirect logic operate on the site-relative path.
In `@includes/functions.php`:
- Line 114: Replace the templated placeholder "@since WEDOCS_SINCE" in the
docblock inside includes/functions.php with the actual release version (for
example "2.2.2" or the upcoming release number); locate the docblock containing
"@since WEDOCS_SINCE" (near the functions defined in this file) and update the
`@since` tag to the concrete version string so the documentation reflects the
correct release.
- Around line 148-166: When url_structure is 'after_doc' and $ancestors is empty
(current post is a top-level doc), ensure the top-level doc crumb is inserted
before the 'docs_home' crumb instead of skipping it: treat the current post as
$top_level (use the current post object used elsewhere in this function), push a
'doc_ancestor' entry for $top_level onto $trail before adding the 'docs_home'
entry, and if that produces a duplicate/undesired standalone 'docs_home' crumb
remove or skip adding the 'docs_home' entry when $top_level is the docs hub
root; update the existing 'after_doc' block that references $ancestors,
$top_level and $trail so top-level docs appear as "Top-doc › Docs" ordering (or
omit Docs when appropriate).
In `@src/blocks/Breadcrumb/render.php`:
- Around line 245-250: The ancestor titles are being pre-truncated inside
wedocs_get_doc_breadcrumb_trail via wedocs_apply_short_content(..., 25), which
causes double-truncation and ignores the block's maxTitleLength; update the
logic so the breadcrumb helper does not hard-cut ancestor titles (remove or make
the 25-char wedocs_apply_short_content call optional/opt-in), and let the
Breadcrumb block (render.php) continue to apply
wedocs_truncate_breadcrumb_title($title, $attrs['maxTitleLength']) uniformly for
both ancestors and the current doc; reference wedocs_get_doc_breadcrumb_trail,
wedocs_apply_short_content, and wedocs_truncate_breadcrumb_title when making the
change so the block can fully control truncation with the user-configured
maxTitleLength.
---
Nitpick comments:
In `@includes/functions.php`:
- Around line 149-175: The breadcrumb helper wedocs_get_doc_breadcrumb_trail
currently truncates ancestor titles via wedocs_apply_short_content( ..., 25 ),
causing a hard 25-char cap for block renderer (src/blocks/Breadcrumb/render.php)
and redundant truncation in wedocs_get_breadcrumb_item; change
wedocs_get_doc_breadcrumb_trail to return full titles (remove the unconditional
wedocs_apply_short_content calls for ancestors) and instead add an optional
argument (e.g. $args['truncate'] or $args['maxTitleLength']) to opt in to
truncation for backward compatibility; update callers (notably
src/blocks/Breadcrumb/render.php and wedocs_get_breadcrumb_item) to apply their
own truncation policy using wedocs_apply_short_content when needed.
- Around line 133-143: The manual ancestor-walk in the block (variables
$ancestors, $parent_id and loop using get_post) lacks a depth guard and can
infinite-loop on cyclical parent chains; replace the manual loop with
WordPress's safe helper get_post_ancestors($post) or add a cheap max-iterations
guard (e.g., limit iterations to a sane constant like 50) around the while
($parent_id) loop and break if exceeded, ensuring you still populate $ancestors
with the found parent objects via get_post for consistency with existing code
that expects post objects.
In `@src/components/Settings/GeneralSettings.js`:
- Around line 81-143: The example URL preview in GeneralSettings.js currently
uses the placeholder "{doc}" which is inconsistent with the backend docs that
use "{top-doc}"; update the template strings where siteUrl is used (the inline
IIFE that builds the example URL using generalSettings and siteUrl) to replace
"{doc}" with "{top-doc}" in both the 'after_doc' and default branches so the UI
preview matches the helper/class docblocks; this affects the example string
shown alongside DocsUrlStructureSelect and references the generalSettings
variable and siteUrl computation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2e07d96-92de-4633-811d-b3bf7d44c7b5
📒 Files selected for processing (9)
includes/Assets.phpincludes/Docs_URL_Structure.phpincludes/functions.phpsrc/blocks/Breadcrumb/render.phpsrc/components/DocsUrlStructureSelect.jssrc/components/ListboxSelect.jssrc/components/SelectBox.jssrc/components/Settings/GeneralSettings.jswedocs.php
| // Nested paths: /{top}/{slug}/{rest}/ | ||
| add_rewrite_rule( | ||
| '^(?!(?:' . implode( '|', $reserved ) . ')/)([^/]+)/' . $slug_regex . '/(.+?)/?$', | ||
| 'index.php?' . $this->post_type . '=$matches[1]/$matches[2]', | ||
| 'top' | ||
| ); | ||
|
|
||
| // Bare "Docs hub" landing under a top doc: /{top}/{slug}/ | ||
| add_rewrite_rule( | ||
| '^(?!(?:' . implode( '|', $reserved ) . ')/)([^/]+)/' . $slug_regex . '/?$', | ||
| 'index.php?' . $this->post_type . '=$matches[1]', | ||
| 'top' | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the docs post type can emit a rewrite front.
# Expected: either docs rewrite has with_front=false, or Docs_URL_Structure accounts for the front prefix.
rg -n -C4 --type=php "register_post_type\\s*\\(\\s*['\"]docs['\"]|with_front|rewrite"Repository: weDevsOfficial/wedocs-plugin
Length of output: 21522
🏁 Script executed:
# Check resolve_request and how it handles the front prefix
cat -n includes/Docs_URL_Structure.php | sed -n '176,215p'Repository: weDevsOfficial/wedocs-plugin
Length of output: 1674
🏁 Script executed:
# Also check if there's any handling of the front in reserved segments or elsewhere
rg -n "site_url|home_url" includes/Docs_URL_Structure.php | head -20Repository: weDevsOfficial/wedocs-plugin
Length of output: 346
🏁 Script executed:
# Verify WordPress's actual behavior with with_front by checking WordPress docs/examples
# Check if the site might have no front base (meaning with_front doesn't change behavior)
fd -e php | xargs rg -l "ABSPATH|WP_HOME" | head -5Repository: weDevsOfficial/wedocs-plugin
Length of output: 242
🏁 Script executed:
# Check WordPress's default behavior with add_rewrite_rule
# Verify if custom rewrite rules need to account for with_front
# Look for any evidence the code is aware of or handling front base in rewrite rules
rg -n "add_rewrite_rule|flush_rewrite" includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 411
🏁 Script executed:
# Check how WordPress post types with with_front=true typically work
# Look at the actual post type registration more carefully to understand the full context
cat -n includes/Post_Types.php | sed -n '43,75p'Repository: weDevsOfficial/wedocs-plugin
Length of output: 1642
🏁 Script executed:
# Check if there are any site configuration or multisite considerations
rg -n "get_option.*front|site_url|home_url" includes/ -A2 -B2 | head -40Repository: weDevsOfficial/wedocs-plugin
Length of output: 3345
🏁 Script executed:
# Let me verify the actual impact by checking WordPress's behavior
# When with_front=true, custom add_rewrite_rule patterns need to account for the front prefix
# Check if there's documentation or evidence about what the front base would be
rg -n "post_type_link|permalink_structure|get_option.*permalink" includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 242
🏁 Script executed:
# Check if the code anywhere validates or asserts the front base setting
# Look for any conditional logic around with_front or front-related settings
rg -n "with_front|front.*base|rewrite.*slug" includes/Post_Types.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 148
🏁 Script executed:
# Test the actual WordPress behavior: when with_front=true, does WordPress
# automatically prepend the front to custom rewrite rules?
# Check WordPress test cases or documentation patterns in the repo
fd -e md -e txt | xargs rg -l "rewrite|permalink" 2>/dev/null | head -3Repository: weDevsOfficial/wedocs-plugin
Length of output: 82
🏁 Script executed:
# The critical test: does resolve_request actually work if a front base exists?
# Check what pagename WordPress populates with when there's a front base
# Also check if the site could even be configured with a non-empty front base
cat -n includes/Docs_URL_Structure.php | sed -n '130,170p'Repository: weDevsOfficial/wedocs-plugin
Length of output: 1835
Align rewrite rules with with_front=true configuration.
The docs post type is registered with with_front=true (Post_Types.php:49), which means WordPress will prepend any configured permalink front base to docs permalinks. Line 146-158 correctly preserves this front base in generated URLs via home_url(). However, the rewrite rule patterns (lines 104-115) and the request fallback (lines 200-201) start matching at ^{top}/{slug}/ without accounting for the front prefix. If the site has a non-empty permalink front base, incoming requests like /front/{top}/{slug}/ won't match these patterns, causing 404s. Either disable with_front in the post type registration or update the rewrite patterns and resolve_request() to include the front base prefix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/Docs_URL_Structure.php` around lines 103 - 115, The rewrite patterns
in the add_rewrite_rule calls and the resolve_request() fallback must account
for the permalink "front" prefix when the docs post type uses with_front=true;
update the two add_rewrite_rule patterns (the nested path and bare hub rules)
and the resolve_request() logic to prepend the front base (use the WP_Rewrite
front value, e.g. trim($wp_rewrite->front, '/') and preg_quote it when building
the regex) so the regex becomes '^' . (front ? preg_quote(front) . '/' : '') .
'(?!(?:' . implode('|', $reserved) . ')/)([^/]+)/' . $slug_regex . '/(.+?)/?$'
(and the shorter hub variant), and mirror the same front-aware prefix when
parsing the request in resolve_request(); alternatively, disable with_front in
the post type registration if you prefer not to include the front prefix.
| $request_uri = wp_parse_url( esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ), PHP_URL_PATH ); | ||
| $request_path = trim( (string) $request_uri, '/' ); | ||
| if ( '' === $request_path ) { | ||
| return; | ||
| } | ||
|
|
||
| $structure = $this->get_structure(); | ||
|
|
||
| $current_url = untrailingslashit( home_url( $request_uri ) ); | ||
| $rewrite_slug = $this->get_rewrite_slug(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WordPress get_home_url source behavior when passing a path on a subdirectory installation
💡 Result:
The get_home_url function in WordPress retrieves the 'home' option from the database (or WP_HOME constant), applies the appropriate URL scheme (http/https/relative based on parameters and is_ssl), and appends the provided $path parameter by prefixing it with a '/' after trimming any leading '/' from the path. This behavior is identical regardless of whether the WordPress installation is in a subdirectory. In a subdirectory installation (where WordPress core files are in e.g. /example.com/wp/ but the site front-end appears at example.com/ via modified index.php and settings), the 'home' option is typically set to the root URL (example.com). Thus: - get_home_url returns: https://example.com - get_home_url('/my-path') returns: https://example.com/my-path The subdirectory path (/wp/) does not factor into the URL construction; it only affects the internal file paths (like ABSPATH) and siteurl (for admin/core links). The function does not parse or modify the path based on installation directory; it simply concatenates. Source code excerpt: function get_home_url( $blog_id = null, $path = '', $scheme = null ) { // ... gets $url = get_option('home') (single site) or equivalent ... $url = set_url_scheme( $url, $scheme ); if ( $path && is_string( $path ) ) { $url .= '/' . ltrim( $path, '/' ); } return apply_filters( 'home_url', $url, $path, $orig_scheme, $blog_id ); }
Citations:
- 1: https://developer.wordpress.org/reference/functions/get_home_url/
- 2: https://wp-kama.com/function/get_home_url
- 3: https://developer.wordpress.org/reference/functions/get_home_url
🏁 Script executed:
cat -n includes/Docs_URL_Structure.php | sed -n '220,280p'Repository: weDevsOfficial/wedocs-plugin
Length of output: 2798
🏁 Script executed:
cat -n includes/Docs_URL_Structure.php | grep -A 100 "function maybe_redirect_legacy_url"Repository: weDevsOfficial/wedocs-plugin
Length of output: 4652
🏁 Script executed:
head -n 300 includes/Docs_URL_Structure.php | tail -n 80Repository: weDevsOfficial/wedocs-plugin
Length of output: 3122
🏁 Script executed:
rg "REQUEST_URI" -A 3 -B 1 includes/ wp-wedocs.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 738
🏁 Script executed:
rg "wp_parse_url.*home_url|home_url.*wp_parse_url" -A 2 -B 2 includes/Repository: weDevsOfficial/wedocs-plugin
Length of output: 54
🏁 Script executed:
grep -r "subdirectory" --include="*.md" --include="*.txt" --include="*.php" . | head -20Repository: weDevsOfficial/wedocs-plugin
Length of output: 273
🏁 Script executed:
find . -name "*test*" -o -name "*spec*" | grep -i test | head -20Repository: weDevsOfficial/wedocs-plugin
Length of output: 1064
🏁 Script executed:
head -50 README.md 2>/dev/null || head -50 readme.txt 2>/dev/null || trueRepository: weDevsOfficial/wedocs-plugin
Length of output: 2614
🏁 Script executed:
python3 << 'EOF'
# Simulate subdirectory behavior
import re
# Scenario: WordPress is installed in /wp/ subdirectory
# Request: /wp/docs/my-page
REQUEST_URI = "/wp/docs/my-page" # Full server path
request_path = REQUEST_URI.strip('/') # "wp/docs/my-page"
rewrite_slug = "docs"
legacy_pattern = f'^{re.escape(rewrite_slug)}/(.+?)/?$'
print("=== Subdirectory Install (/wp/) ===")
print(f"REQUEST_URI from server: {REQUEST_URI}")
print(f"Trimmed request_path: {request_path}")
print(f"Pattern to match: {legacy_pattern}")
print(f"Pattern matches: {bool(re.match(legacy_pattern, request_path))}")
print()
# Now test home_url() behavior
home_url_result = "https://example.com/wp" # The home URL
# When you do home_url($request_uri), it concatenates:
current_url = home_url_result + REQUEST_URI.lstrip('/')
print(f"home_url() = {home_url_result}")
print(f"home_url('{REQUEST_URI}') would be: {home_url_result}/{REQUEST_URI.lstrip('/')}")
print(" → This duplicates the subdirectory path!")
print()
# On root install for comparison
print("=== Root Install ===")
REQUEST_URI_root = "/docs/my-page"
request_path_root = REQUEST_URI_root.strip('/')
print(f"REQUEST_URI from server: {REQUEST_URI_root}")
print(f"Trimmed request_path: {request_path_root}")
print(f"Pattern matches: {bool(re.match(legacy_pattern, request_path_root))}")
EOFRepository: weDevsOfficial/wedocs-plugin
Length of output: 519
🏁 Script executed:
rg "remove_query_arg|wp_parse_request" includes/ | head -20Repository: weDevsOfficial/wedocs-plugin
Length of output: 308
🏁 Script executed:
cat -n includes/Docs_URL_Structure.php | sed -n '1,120p' | grep -A 50 "add_rewrite_rule\|get_rewrite_rules"Repository: weDevsOfficial/wedocs-plugin
Length of output: 792
🏁 Script executed:
python3 << 'EOF'
# Let's verify what the actual WordPress behavior is for subdirectory installs
# WordPress core's REQUEST_URI handling
import json
info = {
"claim": "On subdirectory installs, REQUEST_URI from $_SERVER includes the full path from document root",
"wordpress_behavior": "REQUEST_URI is set by the web server BEFORE WordPress runs, relative to document root",
"implications": [
"Root install + /docs/page → REQUEST_URI = /docs/page",
"Subdirectory /wp/ + /docs/page → REQUEST_URI = /wp/docs/page",
"Code must normalize REQUEST_URI relative to home_path before pattern matching"
],
"current_code_issue": "Does not normalize, assumes REQUEST_URI is already home-relative",
"severity": "Bug on subdirectory installs - redirects will not fire"
}
print(json.dumps(info, indent=2))
EOFRepository: weDevsOfficial/wedocs-plugin
Length of output: 706
Normalize REQUEST_URI against the site home path before matching.
On subdirectory installs, $_SERVER['REQUEST_URI'] includes the home-path prefix (e.g., /wp/docs/page instead of /docs/page), causing pattern matching to fail and URL comparisons to build incorrect paths with duplicated subdirectories. This prevents the promised 301 redirects for legacy docs URLs from firing.
Proposed fix
- $request_uri = wp_parse_url( esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ), PHP_URL_PATH );
- $request_path = trim( (string) $request_uri, '/' );
+ $request_uri = wp_parse_url( esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ), PHP_URL_PATH );
+ $home_path = wp_parse_url( home_url( '/' ), PHP_URL_PATH );
+ $request_path = '/' . ltrim( (string) $request_uri, '/' );
+
+ if ( $home_path ) {
+ $home_path = '/' . trim( $home_path, '/' );
+
+ if ( '/' !== $home_path && 0 === strpos( $request_path . '/', trailingslashit( $home_path ) ) ) {
+ $request_path = substr( $request_path, strlen( $home_path ) );
+ }
+ }
+
+ $request_path = trim( $request_path, '/' );
if ( '' === $request_path ) {
return;
}
$structure = $this->get_structure();
- $current_url = untrailingslashit( home_url( $request_uri ) );
+ $current_url = untrailingslashit( home_url( '/' . $request_path . '/' ) );
$rewrite_slug = $this->get_rewrite_slug();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/Docs_URL_Structure.php` around lines 238 - 247, The code is using
$_SERVER['REQUEST_URI'] directly which on subdirectory installs contains the
site base path and breaks matching; update the normalization logic that computes
$request_uri/$request_path by removing the site's home path prefix (compare
against parse_url( home_url(), PHP_URL_PATH )) before trimming so patterns in
get_structure() and comparisons using $current_url and $rewrite_slug match
correctly; specifically, compute the site's base path, strip that prefix from
$request_uri if present, then continue building $request_path and $current_url
so get_structure(), get_rewrite_slug(), and subsequent 301 redirect logic
operate on the site-relative path.
| private function get_doc_by_path( $path ) { | ||
| $post = get_page_by_path( $path, OBJECT, $this->post_type ); | ||
|
|
||
| return $post instanceof WP_Post ? $post : null; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WordPress get_page_by_path post_status filtering behavior
💡 Result:
The WordPress function get_page_by_path does not filter by post_status. Its direct SQL query selects posts matching the path components solely based on post_name IN (...) AND post_type IN (...), without any WHERE clause for post_status = 'publish' or similar conditions. It can therefore return pages or posts with any post_status, including 'draft', 'private', 'trash', etc. The function finds a matching post ID via the custom query, then calls get_post($found_id, $output) to return the full WP_Post object (or array). The get_post function itself does not filter by status either; it retrieves the post regardless of status, though capabilities checks may affect content visibility in some contexts. To only get published pages, check the status manually after retrieval: if ($page = get_page_by_path('/path/')) { if ('publish' === $page->post_status) { // Use the page } } This behavior is confirmed by the official WordPress source code in wp-includes/post.php, visible in developer.wordpress.org/reference/functions/get_page_by_path/. An older Codex note mentions it "Returns false for 'private' pages/posts", but current code and documentation do not reflect this; the query includes all statuses.
Citations:
- 1: https://developer.wordpress.org/reference/functions/get_page_by_path/
- 2: http://codex.wordpress.org/Function_Reference/get_page_by_path
- 3: https://developer.wordpress.org/reference/functions/get_page_by_path/?output_format=md
- 4: https://wp-kama.com/function/get_page_by_path
- 5: https://wpseek.com/function/get_page_by_path
🏁 Script executed:
# First, locate and read the target file
find . -name "Docs_URL_Structure.php" -type fRepository: weDevsOfficial/wedocs-plugin
Length of output: 105
🏁 Script executed:
# Get context around the function to understand callers
rg "get_doc_by_path" -A 3 -B 3Repository: weDevsOfficial/wedocs-plugin
Length of output: 1761
🏁 Script executed:
# Check for filter_doc_permalink to verify the claimed status guard
rg "filter_doc_permalink" -A 10 -B 2Repository: weDevsOfficial/wedocs-plugin
Length of output: 1896
🏁 Script executed:
# Get the maybe_redirect_legacy_url method
rg "maybe_redirect_legacy_url" -A 20 -B 2Repository: weDevsOfficial/wedocs-plugin
Length of output: 3244
🏁 Script executed:
# Get the resolve_request method
rg "resolve_request" -A 20 -B 2Repository: weDevsOfficial/wedocs-plugin
Length of output: 3198
🏁 Script executed:
# Read the full file to see complete context
wc -l includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 107
🏁 Script executed:
# Get the complete maybe_redirect_legacy_url method
sed -n '334,380p' includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 1634
🏁 Script executed:
# Get the complete resolve_request method
sed -n '381,430p' includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 1376
🏁 Script executed:
# Find line numbers for maybe_redirect_legacy_url
rg -n "public function maybe_redirect_legacy_url" includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 125
🏁 Script executed:
# Find line numbers for resolve_request
rg -n "public function resolve_request" includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 128
🏁 Script executed:
# Get maybe_redirect_legacy_url full method (it was around line 329, let's get 329-375)
sed -n '329,375p' includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 1739
🏁 Script executed:
# Get resolve_request full method
sed -n '376,410p' includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 954
🏁 Script executed:
# Get the complete maybe_redirect_legacy_url method (line 229 onwards)
sed -n '229,280p' includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 2136
🏁 Script executed:
# Get the complete resolve_request method (line 179 onwards)
sed -n '179,228p' includes/Docs_URL_Structure.phpRepository: weDevsOfficial/wedocs-plugin
Length of output: 1747
Add a published-only status check to get_doc_by_path().
The helper returns results from get_page_by_path(), which retrieves posts regardless of publication status. Public URL handlers (resolve_request() and maybe_redirect_legacy_url()) rely on this method without verifying the post status, allowing draft, private, and trashed docs to be resolved or 301-redirected publicly. Add a status check to match the guard already present in filter_doc_permalink():
Proposed fix
private function get_doc_by_path( $path ) {
$post = get_page_by_path( $path, OBJECT, $this->post_type );
- return $post instanceof WP_Post ? $post : null;
+ if ( ! $post instanceof WP_Post || 'publish' !== $post->post_status ) {
+ return null;
+ }
+
+ return $post;
}📝 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.
| private function get_doc_by_path( $path ) { | |
| $post = get_page_by_path( $path, OBJECT, $this->post_type ); | |
| return $post instanceof WP_Post ? $post : null; | |
| private function get_doc_by_path( $path ) { | |
| $post = get_page_by_path( $path, OBJECT, $this->post_type ); | |
| if ( ! $post instanceof WP_Post || 'publish' !== $post->post_status ) { | |
| return null; | |
| } | |
| return $post; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/Docs_URL_Structure.php` around lines 401 - 404, get_doc_by_path
currently returns any WP_Post from get_page_by_path regardless of status, which
allows non-published docs to be resolved or redirected by resolve_request() and
maybe_redirect_legacy_url(); update get_doc_by_path so after confirming $post
instanceof WP_Post it also checks $post->post_status === 'publish' (mirroring
the guard in filter_doc_permalink()) and return null for any non-published
status, ensuring only published docs are returned to callers.
| $filtered = apply_filters( 'wedocs_url_structure_reserved_segments', $reserved ); | ||
|
|
||
| // Defensive: a third-party callback may return a non-array or a non-string entry. | ||
| // Coerce back to a clean list of non-empty strings before preg_quote'ing. | ||
| if ( ! \is_array( $filtered ) ) { | ||
| $filtered = $reserved; | ||
| } | ||
|
|
||
| $filtered = array_filter( | ||
| array_map( | ||
| static function ( $segment ) { | ||
| return \is_scalar( $segment ) ? trim( (string) $segment, '/' ) : ''; | ||
| }, | ||
| $filtered | ||
| ), | ||
| 'strlen' | ||
| ); | ||
|
|
||
| if ( empty( $filtered ) ) { | ||
| $filtered = $reserved; | ||
| } | ||
|
|
||
| return array_map( 'preg_quote', array_values( $filtered ) ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check its existence
find . -name "Docs_URL_Structure.php" -type fRepository: weDevsOfficial/wedocs-plugin
Length of output: 105
🏁 Script executed:
# Once found, examine lines around 430-452 to see the function
head -460 $(find . -name "Docs_URL_Structure.php" -type f) | tail -50Repository: weDevsOfficial/wedocs-plugin
Length of output: 1469
🏁 Script executed:
# Search for where reserved_top_segments or the filtered result is used
rg "array_map.*preg_quote" -A 5 -B 5Repository: weDevsOfficial/wedocs-plugin
Length of output: 487
🏁 Script executed:
# Search for usage of the return value from this function and where it's used in regex patterns
rg "#.*preg_quote|preg_match.*#" -A 3 -B 3Repository: weDevsOfficial/wedocs-plugin
Length of output: 3251
Escape reserved segments with the regex delimiter.
Filtered reserved segments are interpolated into #...# regex patterns at line ~242, so pass '#' to preg_quote() to prevent third-party filter values from breaking pattern compilation.
Proposed fix
- return array_map( 'preg_quote', array_values( $filtered ) );
+ return array_map(
+ static function ( $segment ) {
+ return preg_quote( $segment, '#' );
+ },
+ array_values( $filtered )
+ );📝 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.
| $filtered = apply_filters( 'wedocs_url_structure_reserved_segments', $reserved ); | |
| // Defensive: a third-party callback may return a non-array or a non-string entry. | |
| // Coerce back to a clean list of non-empty strings before preg_quote'ing. | |
| if ( ! \is_array( $filtered ) ) { | |
| $filtered = $reserved; | |
| } | |
| $filtered = array_filter( | |
| array_map( | |
| static function ( $segment ) { | |
| return \is_scalar( $segment ) ? trim( (string) $segment, '/' ) : ''; | |
| }, | |
| $filtered | |
| ), | |
| 'strlen' | |
| ); | |
| if ( empty( $filtered ) ) { | |
| $filtered = $reserved; | |
| } | |
| return array_map( 'preg_quote', array_values( $filtered ) ); | |
| $filtered = apply_filters( 'wedocs_url_structure_reserved_segments', $reserved ); | |
| // Defensive: a third-party callback may return a non-array or a non-string entry. | |
| // Coerce back to a clean list of non-empty strings before preg_quote'ing. | |
| if ( ! \is_array( $filtered ) ) { | |
| $filtered = $reserved; | |
| } | |
| $filtered = array_filter( | |
| array_map( | |
| static function ( $segment ) { | |
| return \is_scalar( $segment ) ? trim( (string) $segment, '/' ) : ''; | |
| }, | |
| $filtered | |
| ), | |
| 'strlen' | |
| ); | |
| if ( empty( $filtered ) ) { | |
| $filtered = $reserved; | |
| } | |
| return array_map( | |
| static function ( $segment ) { | |
| return preg_quote( $segment, '#' ); | |
| }, | |
| array_values( $filtered ) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/Docs_URL_Structure.php` around lines 430 - 452, The returned
reserved-segment escaping currently calls preg_quote without a delimiter, which
allows '#' characters in third-party filter values to break the regex built
later; update the return in Docs_URL_Structure (the array_map call that wraps
preg_quote over array_values($filtered)) to pass the regex delimiter '#' as the
second argument to preg_quote so segments are escaped with respect to that
delimiter before being used in the pattern.
| * Does NOT include the "Home" crumb — callers render that themselves so | ||
| * they can keep their own home icon/link wrapping conventions. | ||
| * | ||
| * @since WEDOCS_SINCE |
There was a problem hiding this comment.
Replace the @since WEDOCS_SINCE placeholder.
The docblock tag @since WEDOCS_SINCE appears to be a templated placeholder that should be replaced with the actual release version (e.g. 2.2.2 or the next release) before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/functions.php` at line 114, Replace the templated placeholder
"@since WEDOCS_SINCE" in the docblock inside includes/functions.php with the
actual release version (for example "2.2.2" or the upcoming release number);
locate the docblock containing "@since WEDOCS_SINCE" (near the functions defined
in this file) and update the `@since` tag to the concrete version string so the
documentation reflects the correct release.
| // In "after_doc" mode the top-level doc precedes the Docs hub crumb. | ||
| if ( 'after_doc' === $url_structure && ! empty( $ancestors ) ) { | ||
| $top_level = array_shift( $ancestors ); | ||
| $trail[] = [ | ||
| 'type' => 'doc_ancestor', | ||
| 'title' => wedocs_apply_short_content( $top_level->post_title, 25 ), | ||
| 'url' => get_permalink( $top_level->ID ), | ||
| 'post' => $top_level, | ||
| ]; | ||
| } | ||
|
|
||
| if ( $docs_home ) { | ||
| $trail[] = [ | ||
| 'type' => 'docs_home', | ||
| 'title' => __( 'Docs', 'wedocs' ), | ||
| 'url' => get_permalink( $docs_home ), | ||
| 'post' => null, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Breadcrumb ordering for after_doc on a top-level doc doesn't match the documented structure.
When url_structure === 'after_doc' and the current post has no ancestors (i.e. it is a top-level doc), $ancestors is empty, so the array_shift() block is skipped and the trail becomes:
Home › Docs › {top-doc}
However, per the PR objectives, after_doc breadcrumbs should place the top-level doc before the "Docs" hub crumb (Home › Top-doc › Docs › Section › Article). For a top-level doc, this should therefore be Home › Top-doc, with no standalone "Docs" crumb (or at minimum, "Docs" should not appear before the top-doc crumb in after_doc mode).
Consider something like:
- // In "after_doc" mode the top-level doc precedes the Docs hub crumb.
- if ( 'after_doc' === $url_structure && ! empty( $ancestors ) ) {
- $top_level = array_shift( $ancestors );
- $trail[] = [
- 'type' => 'doc_ancestor',
- 'title' => wedocs_apply_short_content( $top_level->post_title, 25 ),
- 'url' => get_permalink( $top_level->ID ),
- 'post' => $top_level,
- ];
- }
-
- if ( $docs_home ) {
+ $after_doc = 'after_doc' === $url_structure;
+ $is_top_level = empty( $ancestors );
+
+ // In "after_doc" mode the top-level doc precedes the Docs hub crumb.
+ if ( $after_doc && ! $is_top_level ) {
+ $top_level = array_shift( $ancestors );
+ $trail[] = [
+ 'type' => 'doc_ancestor',
+ 'title' => wedocs_apply_short_content( $top_level->post_title, 25 ),
+ 'url' => get_permalink( $top_level->ID ),
+ 'post' => $top_level,
+ ];
+ }
+
+ // In "after_doc" mode on the top-level doc itself, skip the Docs hub crumb
+ // so the trail matches the URL: Home › {top-doc}.
+ if ( $docs_home && ! ( $after_doc && $is_top_level && 'docs' === $post->post_type ) ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/functions.php` around lines 148 - 166, When url_structure is
'after_doc' and $ancestors is empty (current post is a top-level doc), ensure
the top-level doc crumb is inserted before the 'docs_home' crumb instead of
skipping it: treat the current post as $top_level (use the current post object
used elsewhere in this function), push a 'doc_ancestor' entry for $top_level
onto $trail before adding the 'docs_home' entry, and if that produces a
duplicate/undesired standalone 'docs_home' crumb remove or skip adding the
'docs_home' entry when $top_level is the docs hub root; update the existing
'after_doc' block that references $ancestors, $top_level and $trail so top-level
docs appear as "Top-doc › Docs" ordering (or omit Docs when appropriate).
| foreach ( wedocs_get_doc_breadcrumb_trail( $post ) as $crumb ) { | ||
| $breadcrumbs[] = [ | ||
| 'title' => __('Docs', 'wedocs'), | ||
| 'url' => get_permalink( $docs_home ) | ||
| 'title' => $crumb['title'], | ||
| 'url' => $crumb['url'], | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Ancestor titles will be double-truncated and ignore the block's maxTitleLength.
wedocs_get_doc_breadcrumb_trail() hard-truncates each ancestor title via wedocs_apply_short_content( $post_title, 25 ) before returning, while the current-doc crumb is returned untruncated. Downstream, this block applies its own wedocs_truncate_breadcrumb_title() using the user-configured maxTitleLength (default 20) attribute. Net effect:
- Ancestors: truncated to 25 with
..., then possibly re-truncated tomaxTitleLengthwith…— andmaxTitleLength > 25has no effect on ancestors. - Current: only truncated by the block's
maxTitleLength.
This is inconsistent with the pre-refactor behavior where the block owned truncation, and it silently caps the user's configured maxTitleLength at 25 for ancestor entries. Consider removing the hard-coded 25-char truncation from the shared helper (or making it opt-in) and letting the block/theme apply truncation as before.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/blocks/Breadcrumb/render.php` around lines 245 - 250, The ancestor titles
are being pre-truncated inside wedocs_get_doc_breadcrumb_trail via
wedocs_apply_short_content(..., 25), which causes double-truncation and ignores
the block's maxTitleLength; update the logic so the breadcrumb helper does not
hard-cut ancestor titles (remove or make the 25-char wedocs_apply_short_content
call optional/opt-in), and let the Breadcrumb block (render.php) continue to
apply wedocs_truncate_breadcrumb_title($title, $attrs['maxTitleLength'])
uniformly for both ancestors and the current doc; reference
wedocs_get_doc_breadcrumb_trail, wedocs_apply_short_content, and
wedocs_truncate_breadcrumb_title when making the change so the block can fully
control truncation with the user-configured maxTitleLength.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
includes/functions.php (1)
117-117:⚠️ Potential issue | 🟡 MinorReplace the
@sinceplaceholder before merge.
@since WEDOCS_SINCEshould be updated to the concrete release version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/functions.php` at line 117, Replace the placeholder `@since` annotation in the docblock (the line reading "@since WEDOCS_SINCE") with the concrete release version for this change; update that annotation to the current release tag (for example "@since 1.2.0") in the docblock above the affected function(s) so the file no longer contains the WEDOCS_SINCE placeholder.
🧹 Nitpick comments (1)
src/components/ProPreviews/common/PreviewDropdown.js (1)
16-76: Expose listbox semantics for the custom dropdown.This behaves like a select, but screen readers only see a button plus plain list items. Add
listbox/optionroles and expose selected/locked state so the preview remains understandable to assistive tech.♿ Proposed semantic baseline
<button type="button" aria-label={ ariaLabel } + aria-haspopup="listbox" aria-expanded={ isOpen } onClick={ () => setIsOpen( ( prev ) => ! prev ) } className="relative w-full cursor-pointer rounded-md border border-gray-300 bg-white py-2 pl-3 pr-10 text-left shadow-sm focus:border-indigo-500 focus:outline-none focus:ring-1 focus:ring-indigo-500 sm:text-sm" > @@ { isOpen && ( - <ul className="absolute z-10 mt-1 max-h-60 w-full overflow-auto rounded-md bg-white py-1 text-base shadow-lg ring-1 ring-black ring-opacity-5 focus:outline-none sm:text-sm"> + <ul + role="listbox" + aria-label={ ariaLabel } + className="absolute z-10 mt-1 max-h-60 w-full overflow-auto rounded-md bg-white py-1 text-base shadow-lg ring-1 ring-black ring-opacity-5 focus:outline-none sm:text-sm" + > @@ - <li key={ option?.id } className={ rowClasses }> + <li + key={ option?.id } + role="option" + aria-selected={ isSelected } + aria-disabled="true" + className={ rowClasses } + > <UpgradePopup className="flex items-center w-full">{ content }</UpgradePopup> </li> @@ - <li key={ option?.id } className={ rowClasses }> + <li + key={ option?.id } + role="option" + aria-selected={ isSelected } + className={ rowClasses } + > { content } </li>
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@includes/functions.php`:
- Around line 207-210: The current breadcrumb current-item is outputting
unescaped user-filterable data ($crumb['title']) in the foreach loop (when
'current' === $crumb['type']); update the concatenation to escape the title
using the same escaping function used for linked breadcrumb labels (e.g.,
esc_html()) so that $args['before'] . esc_html( $crumb['title'] ) .
$args['after'] is rendered instead of the raw $crumb['title'].
- Around line 138-145: The ancestor-walk while loop using $parent_id and
get_post() can infinite-loop on self-parented or cyclic post_parent chains; add
a visited-ID guard (e.g., an array or set of visited IDs) inside the loop and
before assigning $parent_id = (int) $parent->post_parent check whether that ID
is already in the visited collection and break if so (also add the current
$parent_id to visited at loop start); this prevents cycles from repeatedly
following the same IDs while preserving the existing behavior of populating
$ancestors.
In `@src/components/Settings/GeneralSettings.js`:
- Around line 88-93: In GeneralSettings component update the Badge rendered when
!wp.hooks.applyFilters('wedocs_pro_loaded', false) to use a description relevant
to the "Docs URL Structure" feature instead of the AI-screenshot copy; locate
the Badge element (props: heading and description) and replace the description
string passed to description with the correct tooltip text (e.g., a short line
about Docs URL Structure being a Pro feature or directing users to upgrade to
manage URL structure), keeping heading={__('Pro Feature', 'wedocs')} unchanged.
- Around line 119-136: The PreviewDropdown is hardcoding the selected state to
'before_doc', causing the UI not to reflect the persisted value; update the
options passed to wp.hooks.applyFilters / PreviewDropdown so that each option's
selected property is computed from the current saved value
(generalSettings.docs_url_structure) instead of a static true — e.g., remove the
hardcoded selected boolean and set selected: generalSettings.docs_url_structure
=== 'before_doc' (and similarly for 'after_doc'), leaving locked:true as-is for
the locked option so the dropdown displays the actual saved selection.
---
Duplicate comments:
In `@includes/functions.php`:
- Line 117: Replace the placeholder `@since` annotation in the docblock (the line
reading "@since WEDOCS_SINCE") with the concrete release version for this
change; update that annotation to the current release tag (for example "@since
1.2.0") in the docblock above the affected function(s) so the file no longer
contains the WEDOCS_SINCE placeholder.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 497d32aa-5c60-4304-929d-fcdf468b664b
📒 Files selected for processing (4)
includes/functions.phpsrc/components/ProPreviews/common/PreviewDropdown.jssrc/components/Settings/GeneralSettings.jswedocs.php
✅ Files skipped from review due to trivial changes (1)
- wedocs.php
| while ( $parent_id ) { | ||
| $parent = get_post( $parent_id ); | ||
| if ( ! $parent ) { | ||
| break; | ||
| } | ||
| $ancestors[] = $parent; | ||
| $parent_id = (int) $parent->post_parent; | ||
| } |
There was a problem hiding this comment.
Guard the ancestor walk against parent cycles.
A self-parented or cyclic post_parent chain can keep this loop running indefinitely and hang breadcrumb rendering. Track visited IDs before following the next parent.
Proposed cycle guard
+ $visited = [];
+
while ( $parent_id ) {
+ if ( isset( $visited[ $parent_id ] ) ) {
+ break;
+ }
+
+ $visited[ $parent_id ] = true;
$parent = get_post( $parent_id );
if ( ! $parent ) {
break;
}
$ancestors[] = $parent;📝 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.
| while ( $parent_id ) { | |
| $parent = get_post( $parent_id ); | |
| if ( ! $parent ) { | |
| break; | |
| } | |
| $ancestors[] = $parent; | |
| $parent_id = (int) $parent->post_parent; | |
| } | |
| $visited = []; | |
| while ( $parent_id ) { | |
| if ( isset( $visited[ $parent_id ] ) ) { | |
| break; | |
| } | |
| $visited[ $parent_id ] = true; | |
| $parent = get_post( $parent_id ); | |
| if ( ! $parent ) { | |
| break; | |
| } | |
| $ancestors[] = $parent; | |
| $parent_id = (int) $parent->post_parent; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/functions.php` around lines 138 - 145, The ancestor-walk while loop
using $parent_id and get_post() can infinite-loop on self-parented or cyclic
post_parent chains; add a visited-ID guard (e.g., an array or set of visited
IDs) inside the loop and before assigning $parent_id = (int)
$parent->post_parent check whether that ID is already in the visited collection
and break if so (also add the current $parent_id to visited at loop start); this
prevents cycles from repeatedly following the same IDs while preserving the
existing behavior of populating $ancestors.
| foreach ( $trail as $index => $crumb ) { | ||
| if ( 'current' === $crumb['type'] ) { | ||
| $html .= ' ' . $args['before'] . $crumb['title'] . $args['after']; | ||
| continue; |
There was a problem hiding this comment.
Escape the current crumb title before rendering.
$crumb['title'] is now filterable via wedocs_doc_breadcrumb_trail, but the current crumb is concatenated directly into HTML. Escape it consistently with linked breadcrumb labels.
Proposed escaping fix
foreach ( $trail as $index => $crumb ) {
if ( 'current' === $crumb['type'] ) {
- $html .= ' ' . $args['before'] . $crumb['title'] . $args['after'];
+ $html .= ' ' . $args['before'] . esc_html( $crumb['title'] ) . $args['after'];
continue;
}📝 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.
| foreach ( $trail as $index => $crumb ) { | |
| if ( 'current' === $crumb['type'] ) { | |
| $html .= ' ' . $args['before'] . $crumb['title'] . $args['after']; | |
| continue; | |
| foreach ( $trail as $index => $crumb ) { | |
| if ( 'current' === $crumb['type'] ) { | |
| $html .= ' ' . $args['before'] . esc_html( $crumb['title'] ) . $args['after']; | |
| continue; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@includes/functions.php` around lines 207 - 210, The current breadcrumb
current-item is outputting unescaped user-filterable data ($crumb['title']) in
the foreach loop (when 'current' === $crumb['type']); update the concatenation
to escape the title using the same escaping function used for linked breadcrumb
labels (e.g., esc_html()) so that $args['before'] . esc_html( $crumb['title'] )
. $args['after'] is rendered instead of the raw $crumb['title'].
arifulhoque7
left a comment
arifulhoque7
left a comment
There was a problem hiding this comment.
PR #310 — wedocs-plugin (free)
Positives
wedocs_get_doc_breadcrumb_trail()refactor is clean — single source of truth for breadcrumb logic, properly guarded withfunction_existswedocs_doc_breadcrumb_trailfilter is correct extension point for Pro to overrideListboxSelectextraction fromSelectBoxis good reuse- Deferred JS filter slot (
wedocs_general_settings_docs_url_structure_field) is the right pattern
Issues
🔴 Medium
1. Use @wordpress/hooks import instead of wp.hooks global — GeneralSettings.js
// Current
!wp.hooks.applyFilters('wedocs_pro_loaded', false)
wp.hooks.applyFilters('wedocs_general_settings_docs_url_structure_field', ...)
// Fix
import { applyFilters } from '@wordpress/hooks';
applyFilters('wedocs_pro_loaded', false)Accessing global wp.hooks directly in React component breaks testability and bypasses proper bundling.
2. Delimiter spacing inconsistency — includes/functions.php
$delimiter = 'doc_ancestor' === $crumb['type'] ? ' ' . $args['delimiter'] . ' ' : $args['delimiter'];docs_home crumb gets $args['delimiter'] (no surrounding spaces) while doc_ancestor gets padded delimiter. Old code was consistent. Verify rendered output matches old behavior for all crumb combinations.
🟡 Low
3. Wrong badge description text — src/components/Settings/GeneralSettings.js:94
// Current — copy-paste from AI Docs feature
description={__('Upload screenshots for AI to analyze when generating documentation', 'wedocs')}
// Fix — should describe URL structure
description={__('Control how documentation URLs and breadcrumbs will be structured', 'wedocs')}4. IIFE in JSX render — src/components/Settings/GeneralSettings.js:117
// Current
{(() => { ... })()}
// Fix — extract to named component or variable before returnLogic should be a named component or computed variable. IIFE in JSX is harder to read and test.
5. Dead $last_index check — includes/functions.php
if ( $index !== $last_index ) {
$html .= $delimiter;
}current type always uses continue and is always last, so this condition never fires for current. Either remove the check or document why it's there.
6. Missing fillRule on tooltip SVG path — src/components/Settings/GeneralSettings.js:136
// Current
<path d="..." clipRule="evenodd" />
// Fix
<path d="..." fillRule="evenodd" clipRule="evenodd" />clipRule alone does nothing without fillRule for this path. Likely copy-paste error.
There was a problem hiding this comment.
♻️ Duplicate comments (3)
includes/functions.php (2)
138-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard ancestor traversal against cyclic parent chains.
Without visited-ID tracking, malformed/self-referential
post_parentdata can trap this loop.Suggested fix
$ancestors = []; $parent_id = (int) $post->post_parent; + $visited = []; while ( $parent_id ) { + if ( isset( $visited[ $parent_id ] ) ) { + break; + } + $visited[ $parent_id ] = true; + $parent = get_post( $parent_id ); if ( ! $parent ) { break; } $ancestors[] = $parent; $parent_id = (int) $parent->post_parent; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/functions.php` around lines 138 - 145, The ancestor traversal loop using get_post and $parent_id can infinite-loop on cyclic or self-referential post_parent chains; modify the logic around the while ( $parent_id ) loop to track visited IDs (e.g., $visited = array() before the loop) and on each iteration check if $parent_id is already in $visited and break if so, otherwise mark the current $parent_id as visited before fetching the parent and continue pushing $ancestors[]; this prevents infinite loops while preserving existing behavior of get_post and $ancestors accumulation.
205-208:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEscape current breadcrumb title before HTML output.
Line 207 prints
$crumb['title']unescaped even though the trail is filterable.Suggested fix
if ( 'current' === $crumb['type'] ) { - $html .= ' ' . $args['before'] . $crumb['title'] . $args['after']; + $html .= ' ' . $args['before'] . esc_html( $crumb['title'] ) . $args['after']; continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/functions.php` around lines 205 - 208, The current breadcrumb output in the foreach over $trail prints $crumb['title'] unescaped; update the code that builds the current crumb (inside the loop where 'current' === $crumb['type']) to escape the title before concatenation by replacing direct $crumb['title'] with esc_html( $crumb['title'] ) (or another appropriate escaping function) so the HTML output is safe while keeping existing $args['before'] and $args['after'] usage.src/components/Settings/GeneralSettings.js (1)
163-173:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBind
PreviewDropdownselected state to saved settings.Line 167 hardcodes
before_docas selected, so persistedafter_doccan render inconsistently.Suggested fix
options={[ { id: 'before_doc', name: __('Before Doc', 'wedocs'), - selected: true, + selected: generalSettings?.docs_url_structure !== 'after_doc', }, { id: 'after_doc', name: __('After Doc', 'wedocs'), + selected: generalSettings?.docs_url_structure === 'after_doc', locked: true, }, ]}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/GeneralSettings.js` around lines 163 - 173, The PreviewDropdown options array currently hardcodes selected: true for the 'before_doc' option, causing saved choices (e.g., 'after_doc') to be ignored; update the options construction in GeneralSettings.js (the PreviewDropdown usage) to derive the selected flag from the persisted setting (e.g., compare each option.id to the saved setting value like previewPosition or settings.display_position) instead of hardcoding, or pass the saved value into PreviewDropdown as the selectedId/selectedValue prop if it supports that; ensure both options set selected based on that comparison so the UI reflects persisted settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@includes/functions.php`:
- Around line 138-145: The ancestor traversal loop using get_post and $parent_id
can infinite-loop on cyclic or self-referential post_parent chains; modify the
logic around the while ( $parent_id ) loop to track visited IDs (e.g., $visited
= array() before the loop) and on each iteration check if $parent_id is already
in $visited and break if so, otherwise mark the current $parent_id as visited
before fetching the parent and continue pushing $ancestors[]; this prevents
infinite loops while preserving existing behavior of get_post and $ancestors
accumulation.
- Around line 205-208: The current breadcrumb output in the foreach over $trail
prints $crumb['title'] unescaped; update the code that builds the current crumb
(inside the loop where 'current' === $crumb['type']) to escape the title before
concatenation by replacing direct $crumb['title'] with esc_html( $crumb['title']
) (or another appropriate escaping function) so the HTML output is safe while
keeping existing $args['before'] and $args['after'] usage.
In `@src/components/Settings/GeneralSettings.js`:
- Around line 163-173: The PreviewDropdown options array currently hardcodes
selected: true for the 'before_doc' option, causing saved choices (e.g.,
'after_doc') to be ignored; update the options construction in
GeneralSettings.js (the PreviewDropdown usage) to derive the selected flag from
the persisted setting (e.g., compare each option.id to the saved setting value
like previewPosition or settings.display_position) instead of hardcoding, or
pass the saved value into PreviewDropdown as the selectedId/selectedValue prop
if it supports that; ensure both options set selected based on that comparison
so the UI reflects persisted settings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11652edd-f67c-4329-9f09-162c36ca64cc
📒 Files selected for processing (3)
includes/functions.phpsrc/components/Settings/GeneralSettings.jstailwind.config.js
3 participants
feature in pro #321
Selectable Docs URL Structure
Adds an admin-configurable setting that lets site owners choose between two URL layouts for docs, with automatic 301 redirects between formats so existing links keep working.
Why
Previously the docs permalink was hard-coded to WordPress's native hierarchical form:
Teams that want the top-level doc to read as the brand/product and
docsto sit underneath it had no way to achieve that without custom rewrites. This PR exposes that layout as a first-class setting while keeping the existing behavior as the default.What changed
New setting:
docs_url_structureLives in weDocs → Settings → General, stored under
wedocs_settings.general.docs_url_structure.before_doc(default)/docs/{top-doc}/{section}/{article}/after_doc/{top-doc}/docs/{section}/{article}/The settings panel renders a live example URL using the active site URL, plus an explanation that format switches are safe because the old URLs are redirected.
Summary by CodeRabbit
New Features
Improvements