Skip to content

Conversation

@Aunshon
Copy link
Collaborator

@Aunshon Aunshon commented Sep 29, 2025

All Submissions:

  • My code follow the WordPress' coding standards
  • My code satisfies feature requirements
  • My code is tested
  • [] My code passes the PHPCS tests
  • My code has proper inline documentation
  • I've included related pull request(s) (optional)
  • I've included developer documentation (optional)
  • I've added proper labels to this pull request

Closes

How to test the changes in this Pull Request:

  • Test as mentioned in the issue.

Changelog entry

No needed

Summary by CodeRabbit

  • Bug Fixes
    • Invalid or non-seller store pages now signal and return a proper 404 status earlier, improving SEO, caching, and user clarity.
    • Breadcrumbs consistently include the store listing link and now show a clear “Error 404” entry when seller info is missing.
    • Consolidated validation ensures the 404 template is shown reliably for invalid store pages.

@Aunshon Aunshon self-assigned this Sep 29, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 29, 2025

Walkthrough

store_template now signals WordPress 404 (sets global query and header) for invalid or non-existent store users before returning the 404 template. store_page_breadcrumb prepopulates the store listing crumb and returns an explicit "Error 404" crumb when no seller is found.

Changes

Cohort / File(s) Summary
Store 404 handling & breadcrumbs
includes/Rewrites.php
Consolidated validation in store_template to set is_404 and send a 404 header for missing/invalid sellers (vendor_staff/non-seller) before returning the 404 template; store_page_breadcrumb now assigns the store listing crumb early and adds an explicit Error 404 crumb when seller lookup fails.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant WP as WordPress Router
  participant R as includes/Rewrites.php:store_template
  participant Q as WP_Query
  participant T as Theme

  U->>WP: Request /store/{slug}
  WP->>R: Route to store_template
  R->>R: Validate store_user (exists, not vendor_staff, is seller)
  alt Invalid store
    R->>Q: set is_404 = true
    R->>WP: status_header(404)
    WP->>T: Load 404 template
    T-->>U: Render 404 page (is_404()=true)
  else Valid store
    R->>T: Continue normal store template
    T-->>U: Render store page
  end
Loading
sequenceDiagram
  autonumber
  participant C as includes/Rewrites.php:store_page_breadcrumb
  participant S as Seller Lookup
  participant B as Breadcrumb Array

  C->>B: crumbs[1] = Store Listing (formatted)
  C->>S: Find seller by slug
  alt Seller not found
    C->>B: crumbs[2] = "Error 404"
    C-->>B: Return crumbs
  else Seller found
    C->>B: Populate seller-specific crumbs
    C-->>B: Return crumbs
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

:\:tada\: QA Approved, Dev Review Done

Suggested reviewers

  • mrabbani

Poem

I hopped to a shop that wasn’t there,
Breadcrumbs set, a polite 404 flair.
I sniffed the trail, it led to none,
WP raised the flag — the fix is done.
A twitch of whiskers, a carrot-spun run. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description includes checklist items, linked issue closure, testing instructions, and a changelog note but omits the required “Changes proposed in this Pull Request” section and lacks detail on the specific code changes, before/after behavior, and related pull request links as outlined in the repository template. Please add a “### Changes proposed in this Pull Request” section that summarizes the code modifications, include any relevant before/after behavior details or screenshots, and populate any other template sections (such as related PR links) to fully align with the repository’s PR description template.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly communicates the primary change of ensuring non-existing store pages return a 404 error, directly aligning with the pull request’s main objective.
Linked Issues Check ✅ Passed The changes introduce a consolidated validation block in store_template to mark non-existent stores as 404 via the global query and status header and ensure the 404 template is returned, and the breadcrumb function now handles missing seller info with a 404 crumb rather than exiting early, meeting the issue’s requirements that is_404() returns true and the error404 body class appears for non-existent store pages (#4889).
Out of Scope Changes Check ✅ Passed All code modifications are confined to the store_page_breadcrumb and store_template functions to implement proper 404 handling for non-existent store pages, and no unrelated files or features have been altered beyond the scope defined in the linked issue.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/set-nonexisting-store-page-as-404

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dea4e5d and 67e5967.

📒 Files selected for processing (1)
  • includes/Rewrites.php (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • includes/Rewrites.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: e2e tests (2, 3)
  • GitHub Check: e2e tests (1, 3)
  • GitHub Check: e2e tests (3, 3)
  • GitHub Check: api tests (1, 1)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b58a11 and dea4e5d.

📒 Files selected for processing (1)
  • includes/Rewrites.php (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
includes/Rewrites.php (1)
includes/functions.php (3)
  • dokan_get_option (1014-1024)
  • dokan_get_store_url (1082-1110)
  • dokan_is_user_seller (73-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: e2e tests (2, 3)
  • GitHub Check: e2e tests (3, 3)
  • GitHub Check: e2e tests (1, 3)
  • GitHub Check: api tests (1, 1)

@Aunshon Aunshon added Needs: Testing This requires further testing Needs: Dev Review It requires a developer review and approval labels Sep 29, 2025
@mrabbani mrabbani added Dev Review Done and removed Needs: Dev Review It requires a developer review and approval labels Sep 30, 2025
@dev-shahed
Copy link
Member

dev-shahed commented Oct 13, 2025

DO NOT MERGE

Fatal error on this branch..

[13-Oct-2025 09:23:21 UTC] PHP Fatal error: Uncaught Error: Class "WeDevs\Dokan\Intelligence\Services\Provider" not found in /Users/wedevs/Documents/Sites/dokantesting/wp-content/plugins/dokan-pro/includes/Intelligence/Provider/BriaAi.php:16
Stack trace:
#0 phar:///Applications/Herd.app/Contents/Resources/valet/dump.phar/vendor/composer/ClassLoader.php(576): include()
#1 phar:///Applications/Herd.app/Contents/Resources/valet/dump.phar/vendor/composer/ClassLoader.php(427): {closure:Composer\Autoload\ClassLoader::initializeIncludeClosure():575}('/Users/wedevs/D...')

@Aunshon Aunshon removed the Type: Bug label Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants