WIP Remove jumper is support of command palette #285
Conversation
WalkthroughThis pull request replaces the legacy Jumper navigation widget with WordPress Command Palette integration. Changes include removal of Jumper UI components (CSS, JavaScript, PHP class), deprecation of legacy AJAX search handlers, introduction of Command Palette core infrastructure (REST controller, API trait, manager), and integration of Command Palette capabilities across nine entity managers. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin User
participant CP as Command Palette UI
participant JS as command-palette.js
participant REST as REST Controller
participant Manager as Entity Manager
participant Entity as Entity (e.g., Product)
Admin->>CP: Triggers search (⌘K)
CP->>JS: Initializes & loads config
JS->>REST: POST /ultimate-multisite/v1/command-palette/search
activate REST
REST->>REST: Validate query & permissions
REST->>Manager: search_for_command_palette(query)
activate Manager
Manager->>Entity: Fetch matching items
Entity-->>Manager: Results array
Manager->>Manager: Format results with URLs & icons
Manager-->>REST: Formatted results
deactivate Manager
REST->>REST: Apply wu_command_palette_search_results filter
REST-->>JS: JSON results + metadata
deactivate REST
JS->>JS: Build command objects with callbacks
JS->>CP: Render results in palette
Admin->>CP: Select result
CP->>JS: Execute command callback
JS->>Admin: Navigate to result URL
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
inc/managers/class-customer-manager.php (1)
21-24: Fix misleading class docblock for Customer_ManagerThe docblock says “Handles processes related to webhooks,” but this class manages customers. Consider updating it for clarity:
- * Handles processes related to webhooks. + * Handles processes related to customers.assets/js/command-palette.js (2)
15-19: Remove or conditionally enable debug console.log statements before release.Multiple
console.logstatements are present throughout the file for debugging purposes. These should be removed or wrapped in a debug flag check before production release to avoid cluttering the browser console.Consider wrapping debug logs in a conditional:
+const DEBUG = config.debug || false; + +function log(...args) { + if (DEBUG) { + console.log('[Ultimate Multisite]', ...args); + } +} + // Then replace console.log calls with: -console.log('[Ultimate Multisite] Command palette API detected, initializing...'); +log('Command palette API detected, initializing...');Also applies to: 32-37, 82-82, 88-88, 114-114, 150-150, 174-174, 195-195
286-286: Pluralization fallback may not handle irregular plurals.The fallback
entity.label + 's'assumes regular English pluralization. Whilelabel_pluralfrom the backend should normally be provided, consider that irregular nouns (e.g., "Category" → "Categories") won't be handled correctly by the fallback.This is acceptable if the backend consistently provides
label_plural, but worth noting for edge cases or third-party extensions.inc/apis/trait-command-palette.php (1)
115-118: Simple pluralization may not handle irregular nouns.Adding
's'works for current entity types but won't handle irregular plurals correctly. Consider allowing managers to override this method or providing explicit plural forms in the configuration.Current entity names (customer, site, membership, etc.) pluralize correctly with 's', so this is acceptable for now but worth noting for future extensibility.
inc/apis/class-command-palette-rest-controller.php (2)
145-145: Remove redundant default value.The
limitparameter already has a default value of 15 defined inget_search_params()(Line 103). The?: 15fallback is unnecessary.Apply this diff:
- $limit = $request->get_param('limit') ?: 15; + $limit = $request->get_param('limit');
147-155: Consider removing duplicate validation.The minimum query length check here duplicates the validation callback defined in
get_search_params()(Line 90). The REST framework will reject requests with short queries before reaching this handler.This block can be safely removed:
- // Minimum query length - if (strlen($query) < 2) { - return rest_ensure_response( - [ - 'results' => [], - 'message' => __('Query must be at least 2 characters.', 'ultimate-multisite'), - ] - ); - } -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
assets/css/jumper.css(0 hunks)assets/js/command-palette.js(1 hunks)assets/js/jumper.js(0 hunks)inc/apis/class-command-palette-rest-controller.php(1 hunks)inc/apis/trait-command-palette.php(1 hunks)inc/class-ajax.php(0 hunks)inc/class-wp-ultimo.php(1 hunks)inc/managers/class-broadcast-manager.php(2 hunks)inc/managers/class-checkout-form-manager.php(2 hunks)inc/managers/class-customer-manager.php(2 hunks)inc/managers/class-discount-code-manager.php(2 hunks)inc/managers/class-domain-manager.php(2 hunks)inc/managers/class-membership-manager.php(2 hunks)inc/managers/class-payment-manager.php(2 hunks)inc/managers/class-product-manager.php(2 hunks)inc/managers/class-site-manager.php(2 hunks)inc/managers/class-webhook-manager.php(2 hunks)inc/ui/class-command-palette-manager.php(1 hunks)inc/ui/class-jumper.php(0 hunks)
💤 Files with no reviewable changes (4)
- assets/css/jumper.css
- inc/class-ajax.php
- assets/js/jumper.js
- inc/ui/class-jumper.php
🧰 Additional context used
🧬 Code graph analysis (10)
inc/managers/class-customer-manager.php (2)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-956)inc/apis/trait-command-palette.php (1)
enable_command_palette(42-50)
inc/managers/class-payment-manager.php (2)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-956)inc/apis/trait-command-palette.php (1)
enable_command_palette(42-50)
inc/managers/class-membership-manager.php (2)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-956)inc/apis/trait-command-palette.php (1)
enable_command_palette(42-50)
inc/managers/class-site-manager.php (2)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-956)inc/apis/trait-command-palette.php (1)
enable_command_palette(42-50)
inc/managers/class-domain-manager.php (2)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-956)inc/apis/trait-command-palette.php (1)
enable_command_palette(42-50)
inc/class-wp-ultimo.php (4)
ultimate-multisite.php (1)
WP_Ultimo(124-126)inc/ui/class-command-palette-manager.php (1)
Command_Palette_Manager(22-247)inc/deprecated/deprecated.php (1)
get_instance(353-356)inc/apis/class-command-palette-rest-controller.php (1)
Command_Palette_Rest_Controller(22-266)
assets/js/command-palette.js (1)
inc/apis/class-command-palette-rest-controller.php (2)
search(141-213)init(48-51)
inc/ui/class-command-palette-manager.php (2)
inc/apis/class-command-palette-rest-controller.php (1)
init(48-51)inc/functions/helper.php (1)
wu_get_version(21-24)
inc/managers/class-discount-code-manager.php (2)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-956)inc/apis/trait-command-palette.php (1)
enable_command_palette(42-50)
inc/apis/trait-command-palette.php (3)
inc/ui/class-command-palette-manager.php (1)
register_entity_type(108-111)inc/models/class-customer.php (2)
get_display_name(211-220)get_email_address(284-293)inc/models/class-discount-code.php (1)
get_code(219-222)
🪛 PHPMD (2.15.0)
inc/apis/class-command-palette-rest-controller.php
182-182: Avoid unused local variables such as '$config'. (undefined)
(UnusedLocalVariable)
⏰ 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). (2)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
🔇 Additional comments (28)
inc/managers/class-broadcast-manager.php (1)
31-32: Command Palette wiring for broadcasts matches the shared patternIncluding
\WP_Ultimo\Apis\Command_Paletteand invokingenable_command_palette()ininit()cleanly optsBroadcast_Managerinto the new command palette flow without changing existing behavior.Also applies to: 64-65
inc/managers/class-payment-manager.php (1)
34-35: Payments now correctly participate in the command paletteAdding the
Command_Palettetrait and callingenable_command_palette()duringinit()brings payments into the centralized search/navigation system with no impact on existing hooks.Also applies to: 68-69
inc/managers/class-domain-manager.php (1)
32-33: Domain manager command palette support is hooked up correctlyUsing the
Command_Palettetrait and enabling it ininit()is consistent with other managers and should make domains available to the command palette without affecting existing domain-mapping flows.Also applies to: 139-140
inc/managers/class-webhook-manager.php (1)
30-31: Webhook manager palette integration is consistent and non‑disruptiveTrait inclusion plus
enable_command_palette()ininit()neatly registers webhooks with the new command palette infrastructure and leaves existing webhook behavior unchanged.Also applies to: 79-80
inc/managers/class-customer-manager.php (1)
31-32: Customers are correctly opted into the command paletteAdding the
Command_Palettetrait and enabling it duringinit()aligns Customer_Manager with the other managers participating in command palette search, without altering existing login/verification logic.Also applies to: 64-65
inc/managers/class-discount-code-manager.php (1)
30-31: Discount codes now exposed to the command palette as intendedIncluding the
Command_Palettetrait and callingenable_command_palette()ininit()correctly wires Discount_Code_Manager into the shared command palette infrastructure without impacting the payment‑received listener.Also applies to: 63-64
inc/class-wp-ultimo.php (1)
453-456: Core now boots the command palette manager and REST controller in the right placeInitializing
WP_Ultimo\UI\Command_Palette_ManagerandWP_Ultimo\Apis\Command_Palette_Rest_Controllerfromload_extra_components()is consistent with how other UI and API components are loaded and cleanly replaces the legacy Jumper bootstrap.Also applies to: 457-460
inc/managers/class-checkout-form-manager.php (1)
28-29: Checkout forms are correctly registered for command palette supportUsing the
Command_Palettetrait and callingenable_command_palette()ininit()brings checkout forms into the shared palette ecosystem while keeping the manager’s responsibilities small and focused.Also applies to: 59-60
inc/managers/class-membership-manager.php (1)
30-30: LGTM! Command Palette trait integration follows established patterns.The trait usage and
enable_command_palette()call are consistent with the existingRest_Api,WP_CLI, andMCP_Abilitiestrait integrations in this manager.Also applies to: 64-65
inc/managers/class-site-manager.php (1)
32-32: LGTM! Consistent Command Palette integration.The trait integration mirrors the pattern used in other managers and follows the established conventions for enabling API capabilities.
Also applies to: 65-66
inc/managers/class-product-manager.php (1)
30-30: LGTM! Command Palette integration is consistent.The trait usage and initialization call follow the same pattern established across other manager classes.
Also applies to: 62-63
assets/js/command-palette.js (1)
47-136: LGTM! Well-structured search hook with caching and debouncing.The
useEntitySearchhook implements sensible patterns: 300ms debounce for API calls, minimum 2-character search threshold, in-memory caching, and proper cleanup of timeouts. The cache persists for the page session which is appropriate for command palette usage.inc/apis/trait-command-palette.php (4)
42-50: LGTM! Clean trait initialization pattern.The
enable_command_palette()method follows the established pattern from other API traits, with an early return guard and deferred registration via theinithook at priority 20.
254-269: LGTM! Robust title extraction with appropriate fallbacks.The method chains through common getter methods (
get_display_name,get_title,get_name) with a sensible ID-based fallback, ensuring a title is always returned regardless of entity type.
279-322: LGTM! Well-structured entity-specific subtitle generation.The method appropriately uses
method_existschecks before calling entity-specific getters, and the switch statement provides clean separation for different entity types. The bullet separator (•) provides a clean visual delimiter.
192-218: No action required. The wildcard search pattern'search' => "*{$query}*"is fully supported and is the standard convention used throughout the codebase for search queries inwu_get_*functions (e.g.,wu_get_sites(),wu_get_customers()). The implementation correctly follows established patterns.inc/apis/class-command-palette-rest-controller.php (5)
1-51: LGTM! Clean class structure.The controller follows WordPress REST API patterns with proper namespace usage and singleton initialization.
59-73: LGTM! Proper route registration.The REST route is properly configured with appropriate methods, callbacks, and parameter validation.
81-109: LGTM! Well-defined search parameters.The parameter definitions include appropriate validation, sanitization, and constraints.
225-248: LGTM! Solid delegation pattern.The method properly validates the manager class, checks capabilities, and delegates to entity-specific search logic.
258-265: LGTM! Convention-based class resolution.The method uses string manipulation to derive manager class names from entity slugs. While this relies on naming conventions, the
class_exists()check provides safety, and this pattern appears consistent with the codebase's architecture.inc/ui/class-command-palette-manager.php (7)
1-45: LGTM! Well-structured manager class.The class follows WordPress patterns with proper initialization hooks and singleton usage.
54-71: LGTM! Appropriate version detection strategy.The check for WordPress 6.4+ with JavaScript-based feature detection is a sound approach. The comment clarifies that 6.9+ has admin-wide support while 6.4+ has editor support.
79-97: LGTM! Appropriate context gating.The method properly restricts Command Palette loading to admin area with appropriate capability checks.
108-122: LGTM! Clean entity registration API.The registration methods provide a straightforward interface for managing entity types.
130-162: LGTM! Proper script enqueueing and configuration.The script is properly enqueued with all necessary dependencies and comprehensive configuration data passed to JavaScript.
170-205: LGTM! Robust custom links parsing with backward compatibility.The method properly parses custom links from the legacy
jumper_custom_linkssetting (Line 172), maintaining backward compatibility during the Jumper-to-Command Palette migration. The use ofexplode(':', $line, 2)correctly handles URLs containing colons (e.g., ports, protocols).
213-246: LGTM! Clear settings interface.The settings registration provides a clear interface for custom links configuration. The retention of the
jumper_custom_linksfield ID maintains backward compatibility during the transition.
| label: entity.label_plural || entity.label + 's', | ||
| callback: ({ close }) => { | ||
| close(); | ||
| window.location.href = networkAdminUrl + 'admin.php?page=wu-' + slug.replace('_', '-') + 's'; |
There was a problem hiding this comment.
Use replaceAll or global regex for underscore replacement.
String.replace('_', '-') only replaces the first occurrence. If an entity slug contains multiple underscores, subsequent ones won't be replaced, causing incorrect URL generation.
- window.location.href = networkAdminUrl + 'admin.php?page=wu-' + slug.replace('_', '-') + 's';
+ window.location.href = networkAdminUrl + 'admin.php?page=wu-' + slug.replaceAll('_', '-') + 's';Alternatively, use a regex with the global flag: slug.replace(/_/g, '-').
📝 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.
| window.location.href = networkAdminUrl + 'admin.php?page=wu-' + slug.replace('_', '-') + 's'; | |
| window.location.href = networkAdminUrl + 'admin.php?page=wu-' + slug.replaceAll('_', '-') + 's'; |
🤖 Prompt for AI Agents
In assets/js/command-palette.js around line 289, the code uses slug.replace('_',
'-') which only replaces the first underscore; change it to replace all
underscores by using either slug.replaceAll('_', '-') or slug.replace(/_/g, '-')
so every underscore is converted to a hyphen before building the URL.
| public function search_permissions_check($request): bool { | ||
| unset($request); | ||
|
|
||
| if (! current_user_can('manage_network')) { | ||
| return new \WP_Error( | ||
| 'rest_forbidden', | ||
| __('You do not have permission to search.', 'ultimate-multisite'), | ||
| ['status' => 403] | ||
| ); | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
Fix return type declaration.
The method declares bool as return type (Line 119) but returns \WP_Error on permission failure (Line 123). While WordPress REST API commonly uses the bool|\WP_Error pattern, the strict type hint causes a mismatch.
Apply this diff to align the return type with actual behavior:
- public function search_permissions_check($request): bool {
+ public function search_permissions_check($request) {📝 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.
| public function search_permissions_check($request): bool { | |
| unset($request); | |
| if (! current_user_can('manage_network')) { | |
| return new \WP_Error( | |
| 'rest_forbidden', | |
| __('You do not have permission to search.', 'ultimate-multisite'), | |
| ['status' => 403] | |
| ); | |
| } | |
| return true; | |
| } | |
| public function search_permissions_check($request) { | |
| unset($request); | |
| if (! current_user_can('manage_network')) { | |
| return new \WP_Error( | |
| 'rest_forbidden', | |
| __('You do not have permission to search.', 'ultimate-multisite'), | |
| ['status' => 403] | |
| ); | |
| } | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In inc/apis/class-command-palette-rest-controller.php around lines 119 to 131,
the method search_permissions_check is declared to return bool but actually
returns a \WP_Error on failure; change the method signature to return
bool|\WP_Error (or remove the strict bool type) so the return type matches
behavior, and ensure any callers or docblocks reflect the updated union return
type.
| // Search all entity types | ||
| $entity_count = count($entities); | ||
| $per_type_limit = max(1, floor($limit / $entity_count)); | ||
|
|
||
| foreach ($entities as $slug => $config) { | ||
| $entity_results = $this->search_entity_type($slug, $query, $per_type_limit); | ||
| $results = array_merge($results, $entity_results); | ||
|
|
||
| // Stop if we've reached the limit | ||
| if (count($results) >= $limit) { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Trim to limit | ||
| $results = array_slice($results, 0, $limit); | ||
| } |
There was a problem hiding this comment.
Fix unused variable flagged by static analysis.
The $config variable on Line 182 is unused. Only $slug is needed for the loop iteration.
Apply this diff:
- foreach ($entities as $slug => $config) {
+ foreach ($entities as $slug => $_config) {Or simply:
- foreach ($entities as $slug => $config) {
+ foreach (array_keys($entities) as $slug) {📝 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.
| // Search all entity types | |
| $entity_count = count($entities); | |
| $per_type_limit = max(1, floor($limit / $entity_count)); | |
| foreach ($entities as $slug => $config) { | |
| $entity_results = $this->search_entity_type($slug, $query, $per_type_limit); | |
| $results = array_merge($results, $entity_results); | |
| // Stop if we've reached the limit | |
| if (count($results) >= $limit) { | |
| break; | |
| } | |
| } | |
| // Trim to limit | |
| $results = array_slice($results, 0, $limit); | |
| } | |
| // Search all entity types | |
| $entity_count = count($entities); | |
| $per_type_limit = max(1, floor($limit / $entity_count)); | |
| foreach ($entities as $slug => $_config) { | |
| $entity_results = $this->search_entity_type($slug, $query, $per_type_limit); | |
| $results = array_merge($results, $entity_results); | |
| // Stop if we've reached the limit | |
| if (count($results) >= $limit) { | |
| break; | |
| } | |
| } | |
| // Trim to limit | |
| $results = array_slice($results, 0, $limit); | |
| } |
| // Search all entity types | |
| $entity_count = count($entities); | |
| $per_type_limit = max(1, floor($limit / $entity_count)); | |
| foreach ($entities as $slug => $config) { | |
| $entity_results = $this->search_entity_type($slug, $query, $per_type_limit); | |
| $results = array_merge($results, $entity_results); | |
| // Stop if we've reached the limit | |
| if (count($results) >= $limit) { | |
| break; | |
| } | |
| } | |
| // Trim to limit | |
| $results = array_slice($results, 0, $limit); | |
| } | |
| // Search all entity types | |
| $entity_count = count($entities); | |
| $per_type_limit = max(1, floor($limit / $entity_count)); | |
| foreach (array_keys($entities) as $slug) { | |
| $entity_results = $this->search_entity_type($slug, $query, $per_type_limit); | |
| $results = array_merge($results, $entity_results); | |
| // Stop if we've reached the limit | |
| if (count($results) >= $limit) { | |
| break; | |
| } | |
| } | |
| // Trim to limit | |
| $results = array_slice($results, 0, $limit); | |
| } |
🧰 Tools
🪛 PHPMD (2.15.0)
182-182: Avoid unused local variables such as '$config'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
inc/apis/class-command-palette-rest-controller.php around lines 178 to 194: the
foreach currently declares an unused $config variable; change the loop to
iterate only the keys (e.g., foreach (array_keys($entities) as $slug) { ... })
or use a placeholder value (e.g., foreach ($entities as $slug => $_) { ... }) so
the unused variable is removed and static analysis warning is resolved.
Summary by CodeRabbit
New Features
Deprecated
✏️ Tip: You can customize this high-level summary in your review settings.