Add content moderation in search.#2397
Conversation
WalkthroughAdds a deploy hook that creates/attaches an editorial content moderation workflow to the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Deployer as Deployer
participant DeployHook as ys_search.deploy
participant WorkflowStorage as WorkflowStorage
participant WorkflowEntity as WorkflowEntity
participant TypePlugin as WorkflowTypePlugin
participant NodeBundle as NodeTypeBundle
Deployer->>DeployHook: run ys_search_deploy_add_editorial_workflow()
DeployHook->>WorkflowStorage: load('editorial')
alt workflow exists
WorkflowStorage-->>DeployHook: return WorkflowEntity
else not found
DeployHook->>WorkflowEntity: create new workflow entity
WorkflowEntity-->>DeployHook: new WorkflowEntity
end
DeployHook->>WorkflowEntity: set states (draft, published) and transitions
DeployHook->>TypePlugin: getTypePlugin('node')
TypePlugin-->>DeployHook: supports ContentModerationInterface
DeployHook->>TypePlugin: addBundle('page')
DeployHook->>WorkflowEntity: save()
DeployHook-->>Deployer: "Attached editorial workflow to page content type."
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/modules/custom/ys_search/ys_search.deploy.php`:
- Around line 20-69: The editorial workflow is only created in
ys_search_deploy_add_editorial_workflow(), which runs during deploy but not on
first install; move the workflow definition into a static configuration file
under config/install/workflows.workflow.editorial.yml OR implement
ys_search_install() that provisions the same Workflow (using Workflow::create
and the same type_settings) so fresh installs get the workflow, and keep
ys_search_deploy_add_editorial_workflow() as an upgrade path that only attaches
the bundle (using $workflow->getTypePlugin() and
addEntityTypeAndBundle('node','page')) and saves the workflow.
- Line 20: The deploy hook function ys_search_deploy_add_editorial_workflow
currently omits the required sandbox parameter; change its signature to accept
the deploy API contract by adding array &$sandbox (i.e., function
ys_search_deploy_add_editorial_workflow(array &$sandbox): string), keep the
string return type, and update any internal references or docblocks to use the
passed-in &$sandbox for progress/state so the hook supports batch passes
correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7c64215b-6753-4479-b30c-b68dc072cf06
⛔ Files ignored due to path filters (1)
.vortex/installer/tests/Fixtures/handler_process/_baseline/tests/behat/features/search.featureis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (3)
tests/behat/features/search.featureweb/modules/custom/ys_search/ys_search.deploy.phpweb/modules/custom/ys_search/ys_search.info.yml
| function ys_search_deploy_add_editorial_workflow(): string { | ||
| $workflow = Workflow::load('editorial'); | ||
|
|
||
| if (!$workflow) { | ||
| $workflow = Workflow::create([ | ||
| 'id' => 'editorial', | ||
| 'label' => 'Editorial', | ||
| 'type' => 'content_moderation', | ||
| 'type_settings' => [ | ||
| 'states' => [ | ||
| 'draft' => [ | ||
| 'label' => 'Draft', | ||
| 'published' => FALSE, | ||
| 'default_revision' => FALSE, | ||
| 'weight' => -5, | ||
| ], | ||
| 'published' => [ | ||
| 'label' => 'Published', | ||
| 'published' => TRUE, | ||
| 'default_revision' => TRUE, | ||
| 'weight' => 0, | ||
| ], | ||
| ], | ||
| 'transitions' => [ | ||
| 'create_new_draft' => [ | ||
| 'label' => 'Create New Draft', | ||
| 'from' => ['draft', 'published'], | ||
| 'to' => 'draft', | ||
| 'weight' => 0, | ||
| ], | ||
| 'publish' => [ | ||
| 'label' => 'Publish', | ||
| 'from' => ['draft', 'published'], | ||
| 'to' => 'published', | ||
| 'weight' => 1, | ||
| ], | ||
| ], | ||
| 'default_moderation_state' => 'published', | ||
| ], | ||
| ]); | ||
| } | ||
|
|
||
| $type_plugin = $workflow->getTypePlugin(); | ||
| if ($type_plugin instanceof ContentModerationInterface) { | ||
| $type_plugin->addEntityTypeAndBundle('node', 'page'); | ||
| } | ||
|
|
||
| $workflow->save(); | ||
|
|
||
| return 'Attached editorial workflow to page content type.'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all files in the ys_search module
find web/modules/custom/ys_search -type f | sortRepository: drevops/vortex
Length of output: 412
🏁 Script executed:
# Check if there are any config files that provision the workflow
find web/modules/custom/ys_search -name "*.yml" -o -name "*.yaml" | xargs -I {} sh -c 'echo "=== {} ===" && cat "{}"'Repository: drevops/vortex
Length of output: 12256
🏁 Script executed:
# Check if there's an .install file with install hooks
ls -la web/modules/custom/ys_search/*.install 2>/dev/null || echo "No .install file found"Repository: drevops/vortex
Length of output: 152
🏁 Script executed:
# Search for any references to 'editorial' workflow or workflow creation in the module
rg "editorial|Workflow" web/modules/custom/ys_search/ -A 3 -B 1Repository: drevops/vortex
Length of output: 1968
🏁 Script executed:
cat -n web/modules/custom/ys_search/ys_search.installRepository: drevops/vortex
Length of output: 392
🌐 Web query:
Drush deploy hook lifecycle when does it run install vs deploy
💡 Result:
When hook_deploy_NAME() runs (vs install)
hook_deploy_NAME()does not run during install (neither Drupal site install nor when enabling a module). It only runs when you explicitly executedrush deploy:hook(ordrush deploy, which calls it). [2], [3]
The drush deploy lifecycle / order
When you run drush deploy, Drush standardizes deployment steps in this order:
drush updatedb --no-cache-cleardrush cache:rebuilddrush config:importdrush cache:rebuilddrush deploy:hook(this is wherehook_deploy_NAME()runs) [1]
So, install-time code belongs in hook_install(), while deployment-time code that must happen after config import belongs in hook_deploy_NAME() (implemented in MODULE.deploy.php). [1], [3]
“Only once” behavior
Deploy hooks are intended to run once per site, with Drupal tracking which deploy hooks have already executed by name (so they don’t re-run every deploy). [4]
🌐 Web query:
Drupal hook_deploy_NAME documentation Drush
💡 Result:
hook_deploy_NAME() is a Drush-driven “deployment hook” intended for tasks that must run after configuration import as part of a standardized deployment. It is picked up from a MODULE.deploy.php file and executed by drush deploy:hook (and thus by drush deploy). [1]
Where it’s documented
- Drush “deploy” command docs (explains purpose, order of execution, and that
HOOK_deploy_NAME()runs after config import). [1] - Drupal.org issue threads provide historical/context notes about the hook and how Drush discovers it from
MODULE_NAME.deploy.php. [3]
How to implement
Create my_module.deploy.php and add a function like:
my_module_deploy_some_task(array &$sandbox) { ... }
Drush runs these after config import; hook_post_update_NAME() runs before config import; hook_update_N() is not used by drush deploy for this purpose. [1]
How to run it
- Run only deploy hooks:
drush deploy:hook[2] - Run the full standardized deployment sequence:
drush deploy(which includesdeploy:hook). [1]
Sources: Drush deploy command docs [1], Drush deploy:hook command docs [2], Drupal.org core issue context [3].
Provision the workflow at install time, not only during deploy.
The workflow is currently created only in the deploy hook and won't be available when the module is first installed. Fresh sites will lack the workflow until drush deploy is explicitly run. Move the workflow definition into config/install/workflows.workflow.editorial.yml or provision it in hook_install(), and use the deploy hook only as an upgrade path for existing environments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/modules/custom/ys_search/ys_search.deploy.php` around lines 20 - 69, The
editorial workflow is only created in ys_search_deploy_add_editorial_workflow(),
which runs during deploy but not on first install; move the workflow definition
into a static configuration file under
config/install/workflows.workflow.editorial.yml OR implement ys_search_install()
that provisions the same Workflow (using Workflow::create and the same
type_settings) so fresh installs get the workflow, and keep
ys_search_deploy_add_editorial_workflow() as an upgrade path that only attaches
the bundle (using $workflow->getTypePlugin() and
addEntityTypeAndBundle('node','page')) and saves the workflow.
| * workflow is attached to the content type. This deploy hook creates the | ||
| * editorial workflow if it does not exist and attaches the page content type. | ||
| */ | ||
| function ys_search_deploy_add_editorial_workflow(): string { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n web/modules/custom/ys_search/ys_search.deploy.php | head -30Repository: drevops/vortex
Length of output: 1097
🏁 Script executed:
rg 'function.*_deploy_' --type=php -A 1Repository: drevops/vortex
Length of output: 693
🏁 Script executed:
web_search
What is the Drupal/Drush deploy hook signature specification?Repository: drevops/vortex
Length of output: 151
🌐 Web query:
Drush deploy hook signature array &$sandbox documentation
💡 Result:
In Drupal, deploy hooks are designed to behave like update hooks that can run in multiple batch “passes”, so the signature includes a sandbox context passed by reference:
/**
* In MODULE.deploy.php
*/
function mymodule_deploy_some_task(array &$sandbox) {
// Initialize once.
$sandbox['progress'] ??= 0;
// Do work in chunks...
// Update progress state in $sandbox so the next pass can resume.
$sandbox['#finished'] = 1; // or a float 0..1 for partial completion
}Why &$sandbox (by reference)?
Because Drupal/Drush uses the sandbox as the Batch API context; your hook stores progress/state in $sandbox and those changes must persist across passes, which requires passing it by reference. This is the same batching pattern documented for update/post-update hooks. [1]
Where it’s used / what runs it:
drush deploy runs the deploy:hook step, which executes hook_deploy_NAME(&$sandbox) implementations found in MODULE.deploy.php files (intended to run after config import during deployments). [2][3]
Related note:
Core discussion explicitly calls out that hook_deploy_NAME(&$sandbox) behaves like hook_update_N(&$sandbox). [4]
Sources:
[1] https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension%21module.api.php/11.x
[2] https://www.drupal.org/project/drupal/issues/3206497
[3] https://www.drush.org/14.x/commands/deploy_hook/
[4] https://www.drupal.org/project/drupal/issues/2901418
🏁 Script executed:
find . -name "*.api.php" -o -name "*drush*" -type f | head -20Repository: drevops/vortex
Length of output: 302
🏁 Script executed:
rg "HOOK_deploy" --type=phpRepository: drevops/vortex
Length of output: 40
🌐 Web query:
Drush deploy hook return type string vs void specification
💡 Result:
For Drush deploy hooks (hook_deploy_NAME() in MODULE.deploy.php), the return value is optional and is treated as a status message.
- Drush executes the hook and stores whatever it returns in
results['query']: it literally does$function($context['sandbox']). (raw.githubusercontent.com) - If the returned value is not empty, Drush logs it (
notice(...)) after casting it to string. (raw.githubusercontent.com)
What to use as a PHP return type
- If you want to return a message: use
?string(or no return type), e.g.function mymodule_deploy_foo(array &$sandbox): ?string. - If you never return anything: use
: void.
Using : string is brittle because any code path that returns nothing / null will cause a PHP TypeError, while Drush itself is fine with “no message” (it just won’t log one). (raw.githubusercontent.com)
Citations:
- 1: https://raw.githubusercontent.com/drush-ops/drush/14.x/src/Commands/deploy/DeployHookCommand.php
- 2: https://raw.githubusercontent.com/drush-ops/drush/14.x/src/Commands/deploy/DeployHookCommand.php
- 3: https://raw.githubusercontent.com/drush-ops/drush/14.x/src/Commands/deploy/DeployHookCommand.php
🏁 Script executed:
rg "deploy.*string|deploy.*void" web/modules/custom --type=php -B 2Repository: drevops/vortex
Length of output: 798
Add the required array &$sandbox parameter to match the deploy hook API contract.
Deploy hooks must follow the signature function hook_deploy_NAME(array &$sandbox): string as documented in the Drush deploy hook specification. The &$sandbox parameter is passed by reference to track progress state across batch passes during deployment; omitting it violates the API contract and prevents proper batching support.
🔧 Proposed fix
-function ys_search_deploy_add_editorial_workflow(): string {
+function ys_search_deploy_add_editorial_workflow(array &$sandbox): string {📝 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.
| function ys_search_deploy_add_editorial_workflow(): string { | |
| function ys_search_deploy_add_editorial_workflow(array &$sandbox): string { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/modules/custom/ys_search/ys_search.deploy.php` at line 20, The deploy
hook function ys_search_deploy_add_editorial_workflow currently omits the
required sandbox parameter; change its signature to accept the deploy API
contract by adding array &$sandbox (i.e., function
ys_search_deploy_add_editorial_workflow(array &$sandbox): string), keep the
string return type, and update any internal references or docblocks to use the
passed-in &$sandbox for progress/state so the hook supports batch passes
correctly.
fc7e0bb to
f8100d7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
web/modules/custom/ys_search/ys_search.deploy.php (1)
20-20:⚠️ Potential issue | 🔴 CriticalFix deploy hook signature to include sandbox context.
Line 20 should accept
array &$sandbox; otherwise deploy hook invocation can fail with argument count/runtime errors.🔧 Proposed fix
-function ys_search_deploy_add_editorial_workflow(): string { +function ys_search_deploy_add_editorial_workflow(array &$sandbox): string {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/modules/custom/ys_search/ys_search.deploy.php` at line 20, The deploy hook function ys_search_deploy_add_editorial_workflow currently lacks the required sandbox parameter; update the function signature for ys_search_deploy_add_editorial_workflow to accept array &$sandbox (and return string as before) so it matches Drupal's deploy hook signature and avoids argument count/runtime errors when invoked by the deploy system.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@web/modules/custom/ys_search/ys_search.deploy.php`:
- Line 20: The deploy hook function ys_search_deploy_add_editorial_workflow
currently lacks the required sandbox parameter; update the function signature
for ys_search_deploy_add_editorial_workflow to accept array &$sandbox (and
return string as before) so it matches Drupal's deploy hook signature and avoids
argument count/runtime errors when invoked by the deploy system.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c13cc0a7-7a99-4d71-a6e6-7c42e913d571
⛔ Files ignored due to path filters (27)
.vortex/installer/tests/Fixtures/handler_process/_baseline/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/_baseline/tests/behat/features/search.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/_baseline/web/modules/custom/sw_search/sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/_baseline/web/modules/custom/sw_search/sw_search.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_no_search/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_none/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_search_without_solr/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/docroot/modules/custom/sw_search/sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/docroot/modules/custom/sw_search/sw_search.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/docroot/modules/custom/sw_search/sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/docroot/modules/custom/sw_search/sw_search.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/names/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/names/web/modules/custom/the_force_search/the_force_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/names/web/modules/custom/the_force_search/the_force_search.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_no_solr/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_none/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/theme_claro/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/theme_olivero/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/theme_stark/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme_circleci/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_eslint_no_theme/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_stylelint_no_theme/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (4)
phpunit.xmltests/behat/features/search.featureweb/modules/custom/ys_search/ys_search.deploy.phpweb/modules/custom/ys_search/ys_search.info.yml
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2397 +/- ##
==========================================
- Coverage 79.37% 78.89% -0.49%
==========================================
Files 126 119 -7
Lines 6711 6552 -159
Branches 44 0 -44
==========================================
- Hits 5327 5169 -158
+ Misses 1384 1383 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
f8100d7 to
a3d983e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
web/modules/custom/ys_search/ys_search.deploy.php (2)
20-20:⚠️ Potential issue | 🔴 CriticalDeploy hook signature should include sandbox parameter.
Line 20 still omits the deploy-hook sandbox argument; this matches a previously raised unresolved concern.
🔧 Proposed fix
-function ys_search_deploy_add_editorial_workflow(): string { +function ys_search_deploy_add_editorial_workflow(array &$sandbox): string {Drupal 11 / Drush deploy hook signature: is hook_deploy_NAME(array &$sandbox) required, and what happens if the function omits the parameter?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/modules/custom/ys_search/ys_search.deploy.php` at line 20, The deploy hook function ys_search_deploy_add_editorial_workflow currently omits the required sandbox parameter; update its signature to accept the sandbox by adding array &$sandbox (e.g., function ys_search_deploy_add_editorial_workflow(array &$sandbox): string) so Drupal/Drush can pass and use the sandbox for progress/state; ensure any references to the parameter inside the function use the new &$sandbox variable and preserve the existing return behavior.
20-64:⚠️ Potential issue | 🟠 MajorProvisioning only in deploy hook leaves fresh installs under-configured.
Line 20–Line 64 implements workflow creation only during deploy-hook execution; this repeats a previously raised install-path gap for new environments.
#!/bin/bash # Verify whether install-time provisioning exists for the workflow. fd 'ys_search.install' rg -nP 'function\s+ys_search_install\s*\(' --type=php fd 'workflows.workflow.editorial.yml' rg -nP 'editorial|content_moderation' web/modules/custom/ys_search -g '*.yml' -g '*.yaml'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/modules/custom/ys_search/ys_search.deploy.php` around lines 20 - 64, The deploy-only provisioning in ys_search_deploy_add_editorial_workflow means fresh installs won't get the editorial workflow; add equivalent install-time provisioning by moving or duplicating this logic into an install hook (e.g., implement ys_search_install) or preferably provide configuration YAML (workflows.workflow.editorial.yml and related moderation_state/transition ymls) so Workflow::load('editorial') / Workflow::create(...) runs on install as well; ensure the same state/transition/type_settings are created or exported as config so new sites receive the editorial workflow without requiring a deploy hook.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@phpunit.xml`:
- Line 111: The phpunit.xml directory entry currently uses suffix=".deploy.php"
which causes deployment hook files to be omitted from normal PHP coverage;
update the <directory> element so production PHP files are included (e.g.,
change suffix to ".php" or remove the suffix attribute) so
web/modules/custom/*.php (including deploy hooks) are counted by coverage and
the 90% threshold remains meaningful.
---
Duplicate comments:
In `@web/modules/custom/ys_search/ys_search.deploy.php`:
- Line 20: The deploy hook function ys_search_deploy_add_editorial_workflow
currently omits the required sandbox parameter; update its signature to accept
the sandbox by adding array &$sandbox (e.g., function
ys_search_deploy_add_editorial_workflow(array &$sandbox): string) so
Drupal/Drush can pass and use the sandbox for progress/state; ensure any
references to the parameter inside the function use the new &$sandbox variable
and preserve the existing return behavior.
- Around line 20-64: The deploy-only provisioning in
ys_search_deploy_add_editorial_workflow means fresh installs won't get the
editorial workflow; add equivalent install-time provisioning by moving or
duplicating this logic into an install hook (e.g., implement ys_search_install)
or preferably provide configuration YAML (workflows.workflow.editorial.yml and
related moderation_state/transition ymls) so Workflow::load('editorial') /
Workflow::create(...) runs on install as well; ensure the same
state/transition/type_settings are created or exported as config so new sites
receive the editorial workflow without requiring a deploy hook.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dbf3402f-cf6a-4354-b864-9d5b9b61b16d
⛔ Files ignored due to path filters (27)
.vortex/installer/tests/Fixtures/handler_process/_baseline/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/_baseline/tests/behat/features/search.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/_baseline/web/modules/custom/sw_search/sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/_baseline/web/modules/custom/sw_search/sw_search.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_no_search/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_none/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_search_without_solr/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/docroot/modules/custom/sw_search/sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/docroot/modules/custom/sw_search/sw_search.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/docroot/modules/custom/sw_search/sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/docroot/modules/custom/sw_search/sw_search.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/names/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/names/web/modules/custom/the_force_search/the_force_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/names/web/modules/custom/the_force_search/the_force_search.info.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_no_solr/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_none/web/modules/custom/sw_search/-sw_search.deploy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/theme_claro/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/theme_olivero/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/theme_stark/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme_circleci/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_eslint_no_theme/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_stylelint_no_theme/phpunit.xmlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (4)
phpunit.xmltests/behat/features/search.featureweb/modules/custom/ys_search/ys_search.deploy.phpweb/modules/custom/ys_search/ys_search.info.yml
| </include> | ||
| <exclude> | ||
| <directory suffix="Test.php">web/modules/custom</directory> | ||
| <directory suffix=".deploy.php">web/modules/custom</directory> |
There was a problem hiding this comment.
Do not exclude deploy hooks from coverage reporting.
Line 111 removes real production code from coverage accounting, which can hide risk in deployment logic and weaken the 90% threshold’s value. Prefer keeping .deploy.php included and adding targeted tests (or narrowing exclusions to non-production artifacts only).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@phpunit.xml` at line 111, The phpunit.xml directory entry currently uses
suffix=".deploy.php" which causes deployment hook files to be omitted from
normal PHP coverage; update the <directory> element so production PHP files are
included (e.g., change suffix to ".php" or remove the suffix attribute) so
web/modules/custom/*.php (including deploy hooks) are counted by coverage and
the 90% threshold remains meaningful.
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
Code coverage (threshold: 90%) Per-class coverage |
Summary by CodeRabbit
New Features
Tests
Chores