fix(dashboard): stop building vendor dashboard layout config on background requests - #3336
Open
akzmoudud wants to merge 1 commit into
Open
fix(dashboard): stop building vendor dashboard layout config on background requests#3336akzmoudud wants to merge 1 commit into
akzmoudud wants to merge 1 commit into
Conversation
…round requests The full-width vendor dashboard registered its assets and built the whole layout config (vendor lookup, header nav, full dashboard nav — ~50 URL builds) on init, which runs for every request: cron, Action Scheduler pings, admin-ajax, REST and admin screens included. The inline config attached to a never-enqueued script was simply discarded on all of them. Register on wp_enqueue_scripts instead — it only fires on front-end page renders — and bail unless the request is a logged-in seller-dashboard view, the only place the config is ever consumed. The seller-dashboard check is valid here (unlike on init) because the query is parsed by the time wp_enqueue_scripts runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughChangesVendor dashboard asset scoping
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
includes/Shortcodes/FullWidthVendorLayout.php (1)
196-197: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winPrevent potential fatal error when
$vendoris falsy.The ternary operator on line 196 (
$vendor ? ...) indicates that$vendorcan evaluate to false or null. However, line 197 calls$vendor->get_avatar()unconditionally. If$vendoris indeed falsy, this will throw a fatal error.Ensure safe property access by verifying
$vendorbefore calling the method.🐛 Proposed fix
- 'name' => $vendor ? $vendor->get_shop_name() : $user_name, - 'avatar' => $vendor->get_avatar() ?? VendorUtil::get_vendor_default_avatar_url(), + 'name' => $vendor ? $vendor->get_shop_name() : $user_name, + 'avatar' => $vendor ? ( $vendor->get_avatar() ?? VendorUtil::get_vendor_default_avatar_url() ) : VendorUtil::get_vendor_default_avatar_url(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@includes/Shortcodes/FullWidthVendorLayout.php` around lines 196 - 197, Update the avatar assignment in the vendor data array to verify that $vendor is truthy before calling get_avatar(). Preserve the existing VendorUtil::get_vendor_default_avatar_url() fallback for missing vendors or missing avatar values, while leaving the name handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@includes/Shortcodes/FullWidthVendorLayout.php`:
- Around line 196-197: Update the avatar assignment in the vendor data array to
verify that $vendor is truthy before calling get_avatar(). Preserve the existing
VendorUtil::get_vendor_default_avatar_url() fallback for missing vendors or
missing avatar values, while leaving the name handling unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a8afdf2-7ce4-4660-a1a3-e830cb4efbb6
📒 Files selected for processing (1)
includes/Shortcodes/FullWidthVendorLayout.php
akzmoudud
added
Needs: Testing
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All Submissions:
Closes: https://github.com/getdokan/dokan-pro/issues/5580
Changes proposed in this Pull Request:
FullWidthVendorLayout::register_vendor_dashboard_assets()was hooked toinit, which runs on every request — WP-Cron, Action Scheduler pings (every ~30s on any WooCommerce site), admin-ajax, REST and admin screens. On each of those it built the entire vendor dashboard layout config: vendor lookup, header nav, and a fulldokan_get_dashboard_nav()pass (~50 navigation URL builds), then attached it as inline data to a script that is never enqueued outside the seller dashboard — so all of that work was discarded.Measured on a plugin-heavy dev site: ~55 wasted
dokan_get_navigation_url()calls per background request, ~150,000+ per day on an idle site. With WPML active each of those multiplies further (every URL translation re-enters the dashboard nav builder).The fix (9-line diff):
wp_enqueue_scripts(priority 5, before the enqueue callback at 10) instead ofinit— it only fires on front-end page renders, so background requests never reach the code at all.is_user_logged_in() && dokan_is_seller_dashboard()— the same condition the existing enqueue callback already uses. This check is unreliable oninit(query not parsed yet) but valid onwp_enqueue_scripts, which is what makes the hook move necessary rather than just adding a guard.Verified: zero calls from cron/AJAX/REST/admin/non-dashboard pages after the change; the dashboard page itself registers, localizes and enqueues exactly as before. Translated (WPML) dashboard pages keep working since dokan-wpml maps the page ID via the existing
dokan_get_current_page_idfilter.Related Pull Request(s)
How to test the changes in this Pull Request:
dokan_appearance.vendor_layout_style = latest).error_log()insidedokan_get_navigation_url()(or use Query Monitor's hook panel) and hitwp-cron.php,admin-ajax.phpor any/wp-json/route: before this PR each hit logs dozens of navigation URL builds, after it logs none.vendorDashboardLayoutConfigpresent in page source).Changelog entry
Stop building vendor dashboard assets and layout config on background requests
Previously the full-width vendor dashboard registered its assets and built its entire layout configuration on every request to the site, including WP-Cron, Action Scheduler, AJAX and REST API calls, where the result was always discarded. Now the work only happens on an actual logged-in seller dashboard page view, removing constant background overhead on every WooCommerce site running Dokan.
🤖 Generated with Claude Code
Summary by CodeRabbit