feat: add shared WP-CLI command registry for wp dokan#3301
Open
MdAsifHossainNadim wants to merge 5 commits into
Open
feat: add shared WP-CLI command registry for wp dokan#3301MdAsifHossainNadim wants to merge 5 commits into
wp dokan#3301MdAsifHossainNadim wants to merge 5 commits into
Conversation
Introduce WeDevs\Dokan\CLI\Manager to own the `wp dokan` command namespace and expose the `dokan_cli_commands` filter. Dokan and its extensions (e.g. Dokan Pro) contribute their commands through this single registry, so free-version commands can be added here later. The Manager is instantiated only under WP-CLI (from init_classes) and registers commands on `init` priority 99, so every extension has already added its commands to the filter by then. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds WP-CLI service wiring for Dokan, registers a new CLI manager through the container, and conditionally loads it during Dokan initialization when WP-CLI is active. ChangesWP-CLI Command Registration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@includes/CLI/Manager.php`:
- Around line 38-45: Add defensive checks in register_commands() before calling
\WP_CLI::add_command(): ensure apply_filters( 'dokan_cli_commands', [] ) returns
an array, and skip or ignore any entry whose handler is not a valid, loadable
command class or callable. Mirror the validation approach used in the Captcha
Manager by verifying the filtered result with is_array and confirming each
$handler is safe to register (for example with class_exists and a
callable/instance check) so malformed filter values do not trigger warnings or
fatal errors.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 609137f1-f51a-4cc9-96c4-57fa4724561b
📒 Files selected for processing (2)
dokan-class.phpincludes/CLI/Manager.php
Load the WP-CLI registry through a dedicated CliServiceProvider that tags the Manager as `cli-service`, resolved from init_classes() only under WP-CLI — mirroring how the ajax-service group is loaded. This keeps CLI registration inside Dokan's service container while avoiding a no-op Manager instantiation on non-CLI requests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
includes/DependencyManagement/Providers/CliServiceProvider.php (1)
29-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
register()hardcodesManager::classinstead of iterating$this->services.The sibling
ServiceProvider::register()loops over its$servicesmap to register every declared service. Here,$servicesdeclaresManager::classbutregister()ignores it and hardcodes the class directly — if a second CLI-related service is ever added to$services, it silently won't be registered unlessregister()is also updated.♻️ Proposed refactor to keep `register()` in sync with `$services`
public function register(): void { - $this->add_tags( - $this->getContainer()->addShared( Manager::class ), - $this->tags - ); + foreach ( $this->services as $class_name ) { + $this->add_tags( + $this->getContainer()->addShared( $class_name ), + $this->tags + ); + } }Also applies to: 38-43
🤖 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/DependencyManagement/Providers/CliServiceProvider.php` around lines 29 - 31, `CliServiceProvider::register()` is hardcoding `Manager::class` instead of using the `$services` list, so any future entries in `$services` would not be registered. Update `register()` to iterate over `$this->services` the same way the sibling `ServiceProvider::register()` does, and register each declared service from that array rather than referencing `Manager::class` directly.
🤖 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.
Nitpick comments:
In `@includes/DependencyManagement/Providers/CliServiceProvider.php`:
- Around line 29-31: `CliServiceProvider::register()` is hardcoding
`Manager::class` instead of using the `$services` list, so any future entries in
`$services` would not be registered. Update `register()` to iterate over
`$this->services` the same way the sibling `ServiceProvider::register()` does,
and register each declared service from that array rather than referencing
`Manager::class` directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89378a12-0f9e-4881-b57c-2331d5f15677
📒 Files selected for processing (3)
dokan-class.phpincludes/DependencyManagement/Providers/CliServiceProvider.phpincludes/DependencyManagement/Providers/ServiceProvider.php
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Guard the `dokan_cli_commands` filter result: bail if it is not an array, and skip any entry whose name is not a non-empty string or whose handler is not a callable/loadable class, so a misbehaving filter cannot trigger WP-CLI warnings or fatals. Mirrors the validation the Captcha Manager already applies to its provider filter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Simplify register_commands(): cast the `dokan_cli_commands` result to an array (covering a misbehaving non-array filter) and let WP-CLI validate each name/handler itself. Drops the verbose per-entry checks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MdAsifHossainNadim
added
Needs: Testing
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:
Changes proposed in this Pull Request:
Adds a shared WP-CLI command registry to Dokan (free) so the
wp dokancommand namespace has a single owner in Lite, and any extension — starting with Dokan Pro — registers its commands under it from one place. This PR ships no user-facing commands on its own; it is the registration seam that Dokan Pro (and future free-tier commands) plug into.What it does
WeDevs\Dokan\CLI\Managerowns thewp dokannamespace.dokan_cli_commandsfilter — extensions add a[ 'dokan <command>' => HandlerClass::class ]map, and the Manager registers each entry withWP_CLI::add_command().CliServiceProvider(tags the Managercli-service) and resolved frominit_classes()only under WP-CLI — mirroring the existingajax-service/DOING_AJAXpattern, so nothing is instantiated on normal web requests.initpriority99, so every extension has already contributed its commands to the filter by the time registration runs.Why
Previously Dokan Pro registered its
wp dokan …commands directly, so thewp dokannamespace had no owner in the free plugin. Moving the registry into Lite means:wp dokan vendor,wp dokan withdraw, …) can be added right here.dokan_rest_api_class_map).Developer usage
Register a command from any plugin that loads after Dokan:
HandlerClassis a standardWP_CLI_Commandsubclass — the registry just forwards it toWP_CLI::add_command().Implementation notes
includes/CLI/Manager.php— owns the namespace, applies thedokan_cli_commandsfilter, and registers each command with WP-CLI.includes/DependencyManagement/Providers/CliServiceProvider.php— tags the Managercli-service, mirroringAjaxServiceProvider.dokan-class.php—init_classes()resolvescli-serviceonly under WP-CLI, right next to the existingajax-serviceline.Related Pull Request(s)
wp dokan licenseandwp dokan modulethrough the new filter. This PR should merge first, since Pro bails when the registry is absent.How to test the changes in this Pull Request:
With Dokan Pro checked out on the companion branch (
feature/dokan-cli-license-modules):wp dokan— lists thelicenseandmodulesubcommands.wp dokan module list --status=active— lists active modules.wp dokan license status— prints the current license status.Without Dokan Pro (or with an older build) the
dokan_cli_commandsfilter returns an empty map and no commands are registered.Changelog entry
Title: Add a shared WP-CLI command registry for the
wp dokannamespaceDokan now owns the
wp dokanWP-CLI namespace and exposes adokan_cli_commandsfilter so Dokan and its extensions can register commands from a single place. Previously there was no CLI registry in Dokan Lite. No user-facing commands are added by this PR on its own.Before Changes
Dokan Lite had no WP-CLI registry; the
wp dokannamespace was owned ad-hoc by Dokan Pro.After Changes
Dokan Lite owns the
wp dokannamespace and exposesdokan_cli_commands; extensions register commands through the filter, and the registry loads only under WP-CLI.Summary by CodeRabbit
New Features
Bug Fixes