CLI-1731: ACLI documentation expose MEO commands that should be hidden#1970
CLI-1731: ACLI documentation expose MEO commands that should be hidden#1970jigar-shah-acquia merged 10 commits intoacquia:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses CLI-1731 by ensuring hidden commands are excluded from generated ACLI documentation, and adds targeted unit tests to lock in the intended “hidden in docs” and API namespace visibility behavior.
Changes:
- Update docs generation to skip commands marked as hidden in the Symfony JSON descriptor output.
- Refactor API list-command generation to use a helper for “namespace has at least one visible command”.
- Add/extend PHPUnit coverage to verify hidden-command exclusion and API namespace list-command creation behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Command/Self/MakeDocsCommand.php |
Filters out hidden commands during JSON docs dump via isCommandHiddenInDocs() helper. |
tests/phpunit/src/Commands/Self/MakeDocsCommandTest.php |
Adds tests to validate hidden-command handling and exclusion from dumped docs/index. |
src/Command/Api/ApiCommandHelper.php |
Refactors namespace visibility check into namespaceHasVisibleCommand() and gates list-command creation on visibility. |
tests/phpunit/src/Commands/Api/ApiCommandHelperTest.php |
Adds new tests for list-command creation rules based on namespace visibility/hidden state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1970 +/- ##
============================================
+ Coverage 92.30% 92.33% +0.02%
- Complexity 1921 1932 +11
============================================
Files 122 122
Lines 7009 7035 +26
============================================
+ Hits 6470 6496 +26
Misses 539 539 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/1970/acli.phar |
anujkaushal
left a comment
There was a problem hiding this comment.
LGTM
➜ cli git:(CLI-1731) ./bin/acli self:make-docs --dump=/tmp/acli-docs/1
➜ cli git:(CLI-1731) cd /tmp/acli-docs/1
➜ 1 ls -l | grep -i v3
➜ 1 ls -l | grep -i site-instances
➜ 1
src/Command/Api/ApiCommandHelper.php
Outdated
| private function namespaceHasHidddenCommand(array $apiCommands, string $namespace): bool | ||
| { | ||
| // If namespace is in array sites-instance,sites,environment-v3 then only return true if the command is not hidden and the command name starts with the namespace. | ||
| if (in_array($namespace, ['site-instances', 'site-instance', 'sites', 'environment-v3', 'environments-v3'])) { |
There was a problem hiding this comment.
This seems brittle; I foresee a time when we have to add or remove commands and no one thinks to look here to hide/unhide them.
We already track which individual commands are hidden. It seems like you could simply hide any namespace that has no visible commands.
I leave it to you to decide, maybe that's too much effort, given how we build the command list iteratively. If so, I don't want to hold up this ticket.
There was a problem hiding this comment.
@danepowell I have updated the logic for the namespaceHasHiddenCommand function as suggested. Please take a look so I can proceed with merging.
Motivation
Fixes #CLI-1731 ACLI documentation expose MEO commands that should be hidden
Proposed changes
src/Command/Api/ApiCommandHelper.phpgenerateApiListCommands(): Replaced the inner loop that usedbreakwith a call to a new helper. Visibility is now computed vianamespaceHasVisibleCommand().namespaceHasVisibleCommand(array $apiCommands, string $namespace): bool: Encapsulates “does this namespace have at least one non-hidden command?” and uses earlyreturn trueinstead ofbreak, removing the Break_ mutation at that site.src/Command/Self/MakeDocsCommand.phpexecute(): Replaced inline$command['hidden'] ?? falsewithself::isCommandHiddenInDocs($command).isCommandHiddenInDocs(array $command): bool: Public static helper that returns$command['hidden'] ?? falseso the “hidden in docs” rule is testable and the FalseValue/Coalesce mutations can be killed by direct unit tests.hiddenkey are still included; commands withhidden => trueare still excluded.Alternatives considered
N/A
Testing steps
./bin/acli ckc./vendor/bin/phpunit./vendor/bin/phpunit tests/phpunit/src/Commands/Api/ApiCommandHelperTest.php tests/phpunit/src/Commands/Api/ApiListCommandTest.php./vendor/bin/phpunit tests/phpunit/src/Commands/Self/MakeDocsCommandTest.php./bin/acli apiand./bin/acli api:accounts(or another namespace) and confirm list output and subcommands are unchanged.ApiCommandHelper.php(e.g. LogicalAnd, ConcatOperandRemoval, Break_) andMakeDocsCommand.php(FalseValue, Coalesce) are killed../bin/acli self:make-docs --dump=/tmp/acli-docsand inspectindex.jsonand presence/absence ofself:make-docs.json.