[#95] Added Cloudflare integration. #97
Conversation
📝 WalkthroughWalkthroughRemoves Lagoon route/monitoring entries. Adds Cloudflare and Purge modules and related config, runtime settings for Cloudflare from an env var, and purge_control config/splits that disable purge for local/CI and exclude purge_control from exported configs. Updates tests and devel settings to append excludes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Env as Environment
participant Drupal as Drupal bootstrap
participant IncCF as settings.cloudflare.php
participant IncPurge as settings.purge_control.php
participant Settings as $settings/$config
Env->>Drupal: bootstrap (ENV vars, ENVIRONMENT const)
Drupal->>IncCF: include Cloudflare settings file
IncCF->>IncCF: read DRUPAL_CLOUDFLARE_API_TOKEN
alt token present
IncCF->>Settings: set cloudflare.settings.api_token
IncCF->>Settings: set cloudflare.settings.auth_using = "token"
else token absent
IncCF-->>Drupal: no Cloudflare config applied
end
Drupal->>IncPurge: include Purge Control settings file
IncPurge->>Settings: $settings['config_exclude_modules'][] = 'purge_control'
alt ENVIRONMENT is LOCAL or CI
IncPurge->>Settings: purge_control.settings.disable_purge = TRUE
IncPurge->>Settings: purge_control.settings.purge_auto_control = FALSE
else other env
IncPurge-->>Drupal: no purge overrides applied
end
Note over Drupal,Settings: Config splits ensure purge_control.settings included for Local/CI exports
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14> [!CAUTION]
Some comments are outside the diff and can’t be posted inline due to platform limits.
🔭 Outside diff range comments (2)
tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
498-519: Consider negative assertions for non-LOCAL/CI envsOptional hardening: add assertions that purge_control.settings is absent in preview/dev/stage/prod scenarios to prevent regressions where the runtime condition accidentally widens.
I can submit a follow-up patch adding e.g.:
- $this->assertArrayNotHasKey('purge_control.settings', $this->config);
in those env tests. Want me to prepare that?
Also applies to: 558-569, 608-619, 659-670
tests/phpunit/Drupal/SwitchableSettingsTest.php (1)
10-18: Docblock class name is out of sync with the actual classThe doc says "ToggleableSettingsTest" while the class is "SwitchableSettingsTest".
/** - * Class ToggleableSettingsTest. + * Class SwitchableSettingsTest. * * Tests for Drupal settings that can be enabled or disabled. These are "unit" * tests for the business logic of specific settings' variables. * * Tests appear in the alphabetical order as per files * in "sites/default/includes". */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.lagoon.yml(0 hunks)composer.json(3 hunks)config/ci/purge_control.settings.yml(1 hunks)config/default/cloudflare.settings.yml(1 hunks)config/default/cloudflarepurger.settings.yml(1 hunks)config/default/config_split.config_split.ci.yml(1 hunks)config/default/config_split.config_split.local.yml(1 hunks)config/default/core.extension.yml(3 hunks)config/default/purge.logger_channels.yml(1 hunks)config/default/purge.plugins.yml(1 hunks)config/default/purge_queuer_coretags.settings.yml(1 hunks)config/local/purge_control.settings.yml(1 hunks)tests/phpunit/Drupal/EnvironmentSettingsTest.php(9 hunks)tests/phpunit/Drupal/SwitchableSettingsTest.php(1 hunks)web/sites/default/includes/modules/settings.cloudflare.php(1 hunks)web/sites/default/includes/modules/settings.devel.php(1 hunks)web/sites/default/includes/modules/settings.purge_control.php(1 hunks)
💤 Files with no reviewable changes (1)
- .lagoon.yml
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
tests/phpunit/Drupal/SettingsTestCase.php (1)
assertConfig(235-241)
tests/phpunit/Drupal/SwitchableSettingsTest.php (3)
tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
DataProvider(29-36)tests/phpunit/Drupal/DatabaseSettingsTest.php (1)
DataProvider(21-28)tests/phpunit/Drupal/SettingsTestCase.php (4)
setEnvVars(134-165)requireSettingsFile(204-223)assertConfigContains(249-251)assertConfigNotContains(259-261)
🔇 Additional comments (19)
config/ci/purge_control.settings.yml (2)
1-4: CI: Purge disabled with auto-control off — LGTMMatches the intent to prevent accidental external purges in CI. Mirrors local split for parity.
1-4: CI config split already includes purge_control.settings
Verified thatconfig/default/config_split.config_split.ci.ymllists- purge_control.settingsundercomplete_list(around line 17). No further action required.config/local/purge_control.settings.yml (2)
1-4: Local: Purge disabled with auto-control off — LGTMAppropriate safeguard for local development. Parity with CI is good.
1-4: Local split already includes purge_control.settings
The Local split config (config/default/config_split.config_split.local.yml) containscomplete_list: - purge_control.settingson line 17, so
purge_control.settingswill be applied locally as intended.config/default/purge.logger_channels.yml (2)
1-23: Logger channel grants look consistent — LGTMGrant levels for purgers/queue/diagnostics and the specific Cloudflare purger channel appear coherent with Purge module practices.
19-23: Channel instance ID alignment confirmedThe
id: purger_cloudflare_946182794bin config/default/purge.logger_channels.yml (line 19) exactly matches theinstance_id: 946182794bunder the Cloudflare purger in config/default/purge.plugins.yml (line 3). No changes required.config/default/cloudflare.settings.yml (1)
1-13: Sanity pass on defaults — verify test coverage for both token-present and token-absent casesGiven the new settings are switchable via env var, ensure tests assert:
- When DRUPAL_CLOUDFLARE_API_TOKEN is absent: api_token empty, valid_credentials false, modules behave as expected.
- When the token is present: settings are populated, and any purger-related behavior is not active in CI/Local due to purge_control.
If not already covered, I can help sketch additional unit tests/assertions in SwitchableSettingsTest.
web/sites/default/includes/modules/settings.devel.php (1)
10-10: Good switch from overwrite to appendPreserving existing exclusions and appending 'devel' is the right direction for composability across settings includes.
config/default/purge_queuer_coretags.settings.yml (1)
1-11: LGTM: sensible core tag blacklist for queuerThe blacklist looks appropriate to avoid excessive queueing on volatile/core tags. Works well with purge_queuer_coretags.
config/default/core.extension.yml (3)
17-18: Cloudflare modules declared (disabled by default)Declaring cloudflare/cloudflarepurger at 0 is fine to ship config without enabling by default. Ensure enablement happens via deployment tooling or config split where appropriate.
If you intend to enable these per environment, confirm config splits include them and tests cover the activation path.
33-33: No lingeringdo_corereferences detectedAn exhaustive search across the entire codebase for the old module name in its various formats (
do_core,do-core,doCore) returned no matches. You can safely proceed with the rename.
28-28: ctools disabled: verified no transitive dependenciesI’ve scanned all module *.info.yml files and configuration for any references to “ctools” in dependencies—none were found. The only occurrences are in composer.lock (as a composer‐level package), so disabling it in core.extension.yml is safe.
- config/default/core.extension.yml:
ctools: 0- No *.info.yml files list
ctoolsunder dependencies- composer.lock includes
drupal/ctools, but no enabled module or config depends on itconfig/default/purge.plugins.yml (1)
1-5: LGTM: Cloudflare purger instance configured with deterministic idThe purger instance_id matches the logger channel naming pattern and uses a defined order_index. This is ready for activation once the related modules are enabled and credentials are present.
web/sites/default/includes/modules/settings.purge_control.php (1)
12-15: Environment constants are defined before module settings includes—no guard neededThe
ENVIRONMENT_*constants are declared inweb/sites/default/settings.php(lines 119–133) before the glob-based inclusion of module settings (lines 158–162), which loadsincludes/modules/settings.purge_control.php. As a result, referencingENVIRONMENT_LOCALandENVIRONMENT_CIhere cannot trigger an undefined-constant error under the current bootstrap order.No changes required.
config/default/config_split.config_split.ci.yml (1)
1-18: CI split aligns with purge_control strategySplit metadata, storage, and complete_list for purge_control.settings look consistent with the LOCAL split and the runtime settings. Module override purge_control: 0 also aligns with excluding the module from imports.
config/default/config_split.config_split.local.yml (1)
1-18: Local split is consistent and mirrors CIConfiguration mirrors CI with appropriate weight and folder. complete_list correctly scopes purge_control.settings for local-only overrides.
tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
372-374: Purge Control config assertions match new settings behaviorAsserting disable_purge TRUE and purge_auto_control FALSE in LOCAL/CI (+container) matches settings.purge_control.php and the per-env YAML. Good coverage.
Also applies to: 418-420, 466-468
tests/phpunit/Drupal/SwitchableSettingsTest.php (2)
561-572: Good, concise, data-driven test scaffoldThe structure aligns with the rest of the suite and uses attributes consistently.
586-587: Correct mapping of env to runtime configExpecting api_token plus auth_using = 'token' aligns with settings.cloudflare.php contract.
| "drupal/ai_provider_openai": "^1.1", | ||
| "drupal/civictheme": "~1.10.0", | ||
| "drupal/clamav": "^2.1", | ||
| "drupal/cloudflare": "^2@beta", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm appetite for pre-release deps and consider pinning
You’re pulling drupal/cloudflare from 2.x beta and adding purge-related modules. With minimum-stability set to beta this is valid, but you may want to pin to a specific tag or commit to improve reproducibility and reduce unexpected upgrades.
If the VCS repo is required to satisfy a transitive dependency (e.g., a forked SDK), consider pinning it via explicit constraint (dev-branch with commit hash) to avoid silent drift:
- Example (if directly required; adjust package name/branch/hash as needed):
"andresgmh/cloudflare-php": "dev-main#"
Please verify the necessity and provenance of the VCS fork and whether a stable tag of drupal/cloudflare 2.x is available for your target features.
Run locally:
Also applies to: 34-35
🏁 Script executed:
#!/bin/bash
# Inspect why the VCS repo is needed and whether it's pulled transitively.
rg -n -C2 'andresgmh|cloudflare-php|cloudflare' composer.json
# If composer is available locally, also run:
# composer why drupal/cloudflare
# composer why-not drupal/cloudflare:^2
# composer show drupal/cloudflare -aLength of output: 408
Ensure reproducible installs for beta and VCS-based deps
You’re pulling drupal/cloudflare at ^2@beta (composer.json: line 16) and have a VCS override for andresgmh/cloudflare-php (lines 84–88). With minimum-stability: beta plus an unpinned branch, installs can silently drift as beta releases and branch heads update.
• Target a specific 2.x tag of drupal/cloudflare (if available), e.g.:
"require": {
"drupal/cloudflare": "2.0.0-beta1"
}• Pin the VCS override to a commit to lock the SDK:
"require": {
"andresgmh/cloudflare-php": "dev-main#<commit-sha>"
}• Optionally set "prefer-stable": true alongside your beta minimum-stability to favor tagged releases.
Locking both the Drupal module and the underlying SDK to explicit versions or commits will improve reproducibility and avoid unexpected upgrades.
🤖 Prompt for AI Agents
In composer.json around line 16 and lines 84–88, the dependency on
drupal/cloudflare is specified as ^2@beta and the VCS override for
andresgmh/cloudflare-php uses an unpinned branch, which allows installs to
drift; change the drupal/cloudflare requirement to a specific 2.x tag (e.g.,
2.0.0-beta1 or a later 2.x tagged release) and update the VCS override to pin
the SDK to a specific commit (dev-branch#<commit-sha>) so both the module and
underlying SDK are locked, and optionally set "prefer-stable": true alongside
your minimum-stability to favor tagged releases.
| client_ip_restore_enabled: true | ||
| remote_addr_validate: true |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enable client IP restore only where traffic is guaranteed to come via Cloudflare
Having client_ip_restore_enabled: true in default config can be risky outside Cloudflare; although remote_addr_validate: true helps, consider enabling this only in Prod (via split) and set default to false.
Optionally adjust defaults:
-client_ip_restore_enabled: true
+client_ip_restore_enabled: falseThen enable it in the production config split.
🤖 Prompt for AI Agents
In config/default/cloudflare.settings.yml around lines 3-4, the default enables
client IP restore which is unsafe outside Cloudflare; change
client_ip_restore_enabled to false in this default file, keep
remote_addr_validate: true, and add/ensure a production config split (e.g.,
config/production/cloudflare.settings.yml or relevant split mechanism) that sets
client_ip_restore_enabled: true so only prod traffic via Cloudflare restores
client IPs; update any config comments or docs to note this behavior.
| valid_credentials: true | ||
| zone_id: | ||
| - 7303aceb6327852069d6653a5b5f2fb3 | ||
| zone_name: '' | ||
| auth_using: token | ||
| api_token: '' | ||
| apikey: '' | ||
| email: '' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid committing concrete zone_id and “valid_credentials: true” in default config
Publishing a real zone_id ties defaults to a specific tenant and setting valid_credentials: true with empty credentials is inconsistent. Prefer safe defaults and populate via settings.php or environment splits.
Apply:
-valid_credentials: true
-zone_id:
- - 7303aceb6327852069d6653a5b5f2fb3
+valid_credentials: false
+zone_id: []If you want environment-driven configuration, keep defaults blank and source api_token and zone_id via split-specific overrides or settings.php.
📝 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.
| valid_credentials: true | |
| zone_id: | |
| - 7303aceb6327852069d6653a5b5f2fb3 | |
| zone_name: '' | |
| auth_using: token | |
| api_token: '' | |
| apikey: '' | |
| email: '' | |
| valid_credentials: false | |
| zone_id: [] | |
| zone_name: '' | |
| auth_using: token | |
| api_token: '' | |
| apikey: '' | |
| email: '' |
🤖 Prompt for AI Agents
In config/default/cloudflare.settings.yml around lines 6 to 13, the file
currently contains a concrete zone_id and sets valid_credentials: true with
empty credential fields; replace those unsafe defaults by removing the
hard-coded zone_id, setting valid_credentials: false, and leaving zone_id,
api_token, apikey, and email blank; document that these values must be populated
per-environment (via settings.php or environment-specific overrides/splits) or
loaded from environment variables so no tenant-specific identifiers or
assumed-valid flags are committed in the default config.
| _core: | ||
| default_config_hash: gp3FRci__UxWFkBQ1XDZ6uolEbV0QDQkpnQ_VtrtQ1A | ||
| edge_cache_tag_header_blacklist: { } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Type check: edge_cache_tag_header_blacklist may need to be a list, not a map
{} denotes an empty map. If the schema expects a list (e.g., of header names), config import will fail or be ignored. If it should be an empty list, use [].
Run a quick docs check:
If it must be a list, apply:
-edge_cache_tag_header_blacklist: { }
+edge_cache_tag_header_blacklist: []🌐 Web query:
In Drupal's cloudflarepurger module config schema, what is the type of `edge_cache_tag_header_blacklist`? Should it be a sequence (list) or a mapping?
💡 Result:
Short answer: sequence (a list) of strings — not a mapping.
Why: the blacklist is a simple indexed list of tag prefixes (e.g. "- theme_registry", "- node_list") and the module stores/reads it as an array, so the config schema entry should be type: sequence with sequence: type: string. (drupal.org, api.druphelp.com)
Example schema snippet you can use:
cloudflarepurger.settings:
type: config_object
label: 'Cloudflare Purger settings'
mapping:
edge_cache_tag_header_blacklist:
type: sequence
label: 'Edge cache tag header blacklist'
sequence:
type: string
label: 'Cache tag prefix'
See Drupal's config schema docs for the sequence type and examples (sequence is the correct type for indexed lists of values). (drupal.org, drupal.stackexchange.com)
If you want, paste your current schema file and I can show the exact change.
Citations:
- 1: https://www.drupal.org/project/akamai/issues/3020043?utm_source=chatgpt.com
- 2: https://api.druphelp.com/api/akamai/src%21Form%21ConfigForm.php/8.3?utm_source=chatgpt.com
- 3: https://www.drupal.org/docs/drupal-apis/configuration-api/configuration-schemametadata?utm_source=chatgpt.com
- 4: https://drupal.stackexchange.com/questions/310120/properly-storing-an-array-in-a-config-object?utm_source=chatgpt.com
Update edge_cache_tag_header_blacklist to a sequence of strings
The config schema for cloudflarepurger expects edge_cache_tag_header_blacklist to be a list (sequence), not a map. Update your default config and schema as follows:
-
In
config/default/cloudflarepurger.settings.yml, change the empty map to an empty list:_core: default_config_hash: gp3FRci__UxWFkBQ1XDZ6uolEbV0QDQkpnQ_VtrtQ1A -edge_cache_tag_header_blacklist: { } +edge_cache_tag_header_blacklist: []
-
In your config schema file (e.g.
config/schema/cloudflarepurger.schema.yml), declare it as a sequence of strings:cloudflarepurger.settings: type: config_object label: 'Cloudflare Purger settings' mapping: edge_cache_tag_header_blacklist: type: sequence label: 'Edge cache tag header blacklist' sequence: type: string label: 'Cache tag prefix'
This aligns the default config with the schema and ensures Drupal will correctly read and write the blacklist as an indexed list of header prefixes.
🤖 Prompt for AI Agents
In config/default/cloudflarepurger.settings.yml around lines 1 to 3, the default
value for edge_cache_tag_header_blacklist is an empty map but the schema expects
a sequence of strings; change the YAML value from an empty mapping to an empty
sequence (i.e. replace the {} with an empty list) and also update the module
schema file (config/schema/cloudflarepurger.schema.yml) so
edge_cache_tag_header_blacklist is declared as a sequence of string entries per
the suggested schema mapping.
| public static function dataProviderCloudflare(): array { | ||
| return [ | ||
| // Test with DRUPAL_CLOUDFLARE_API_TOKEN set in local environment. | ||
| [ | ||
| [ | ||
| 'DRUPAL_CLOUDFLARE_API_TOKEN' => 'test-api-token-12345', | ||
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, | ||
| ], | ||
| [ | ||
| 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], | ||
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | ||
| ], | ||
| [], | ||
| ], | ||
| // Test without DRUPAL_CLOUDFLARE_API_TOKEN set in local environment. | ||
| [ | ||
| [ | ||
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, | ||
| ], | ||
| [ | ||
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | ||
| ], | ||
| [ | ||
| 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], | ||
| ], | ||
| ], | ||
| ]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add CI coverage and keep tests in alphabetical order per file-level guideline
- The suite states tests should follow alphabetical order by includes file. The Cloudflare block (c…) currently sits after Shield (s…). Consider moving this block above Shield to maintain consistency.
- Given the PR disables purge in local/CI, add at least one CI case to validate the same behavior under CI.
Proposed addition to data provider (append after the existing two cases):
return [
// Test with DRUPAL_CLOUDFLARE_API_TOKEN set in local environment.
[
[
'DRUPAL_CLOUDFLARE_API_TOKEN' => 'test-api-token-12345',
'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL,
],
[
'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'],
'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE],
],
[],
],
// Test without DRUPAL_CLOUDFLARE_API_TOKEN set in local environment.
[
[
'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL,
],
[
'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE],
],
[
- 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'],
+ 'cloudflare.settings' => NULL,
],
],
+ // Test with DRUPAL_CLOUDFLARE_API_TOKEN set in CI environment.
+ [
+ [
+ 'DRUPAL_CLOUDFLARE_API_TOKEN' => 'test-api-token-12345',
+ 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_CI,
+ ],
+ [
+ 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'],
+ 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE],
+ ],
+ [],
+ ],
];📝 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 static function dataProviderCloudflare(): array { | |
| return [ | |
| // Test with DRUPAL_CLOUDFLARE_API_TOKEN set in local environment. | |
| [ | |
| [ | |
| 'DRUPAL_CLOUDFLARE_API_TOKEN' => 'test-api-token-12345', | |
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, | |
| ], | |
| [ | |
| 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], | |
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | |
| ], | |
| [], | |
| ], | |
| // Test without DRUPAL_CLOUDFLARE_API_TOKEN set in local environment. | |
| [ | |
| [ | |
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, | |
| ], | |
| [ | |
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | |
| ], | |
| [ | |
| 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], | |
| ], | |
| ], | |
| ]; | |
| } | |
| public static function dataProviderCloudflare(): array { | |
| return [ | |
| // Test with DRUPAL_CLOUDFLARE_API_TOKEN set in local environment. | |
| [ | |
| [ | |
| 'DRUPAL_CLOUDFLARE_API_TOKEN' => 'test-api-token-12345', | |
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, | |
| ], | |
| [ | |
| 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], | |
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | |
| ], | |
| [], | |
| ], | |
| // Test without DRUPAL_CLOUDFLARE_API_TOKEN set in local environment. | |
| [ | |
| [ | |
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, | |
| ], | |
| [ | |
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | |
| ], | |
| [ | |
| 'cloudflare.settings' => NULL, | |
| ], | |
| ], | |
| // Test with DRUPAL_CLOUDFLARE_API_TOKEN set in CI environment. | |
| [ | |
| [ | |
| 'DRUPAL_CLOUDFLARE_API_TOKEN' => 'test-api-token-12345', | |
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_CI, | |
| ], | |
| [ | |
| 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], | |
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | |
| ], | |
| [], | |
| ], | |
| ]; | |
| } |
🤖 Prompt for AI Agents
In tests/phpunit/Drupal/SwitchableSettingsTest.php around lines 577 to 604, move
the entire Cloudflare dataProvider block so it appears alphabetically before the
Shield block, and add one CI test case to the provider to validate purge is
disabled in CI: append a third case that sets DRUPAL_ENVIRONMENT to
static::ENVIRONMENT_CI (with and/or without DRUPAL_CLOUDFLARE_API_TOKEN as
appropriate for the expected outcome) and include the expected settings
asserting purge_control.settings disables purge (and cloudflare.settings
presence/values to match the CI behavior). Ensure the new CI case follows the
same array shape as the existing two cases and keep entries in file alphabetical
order.
| [ | ||
| [ | ||
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, | ||
| ], | ||
| [ | ||
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | ||
| ], | ||
| [ | ||
| 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], | ||
| ], | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the negative assertion robust by asserting the whole Cloudflare override is absent
Asserting a specific token value is absent can still pass if a different (unexpected) value bleeds in. Assert the absence of the entire 'cloudflare.settings' override instead.
[
[
'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL,
],
[
'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE],
],
[
- 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'],
+ 'cloudflare.settings' => NULL,
],
],📝 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.
| [ | |
| [ | |
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, | |
| ], | |
| [ | |
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | |
| ], | |
| [ | |
| 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], | |
| ], | |
| ], | |
| [ | |
| [ | |
| 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, | |
| ], | |
| [ | |
| 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], | |
| ], | |
| [ | |
| 'cloudflare.settings' => NULL, | |
| ], | |
| ], |
🤖 Prompt for AI Agents
In tests/phpunit/Drupal/SwitchableSettingsTest.php around lines 592 to 602, the
test currently asserts that a specific Cloudflare API token value is not
present; change this to assert the entire 'cloudflare.settings' override is
absent. Update the test expectations/data so no 'cloudflare.settings' entry is
present and replace the token-specific negative assertion with a robust
assertion that the overrides array does not contain the 'cloudflare.settings'
key (e.g., use assertArrayNotHasKey or the test framework equivalent) so any
unexpected Cloudflare override will fail the test.
| if (!empty(getenv('DRUPAL_CLOUDFLARE_API_TOKEN'))) { | ||
| $config['cloudflare.settings']['api_token'] = getenv('DRUPAL_CLOUDFLARE_API_TOKEN'); | ||
| $config['cloudflare.settings']['auth_using'] = 'token'; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Read env var once, trim, and set config
Functionally correct. Minor readability/perf nit: avoid calling getenv twice and trim whitespace to prevent subtle auth failures.
Apply:
-if (!empty(getenv('DRUPAL_CLOUDFLARE_API_TOKEN'))) {
- $config['cloudflare.settings']['api_token'] = getenv('DRUPAL_CLOUDFLARE_API_TOKEN');
- $config['cloudflare.settings']['auth_using'] = 'token';
-}
+<?php
+$token = trim((string) (getenv('DRUPAL_CLOUDFLARE_API_TOKEN') ?: ''));
+if ($token !== '') {
+ $config['cloudflare.settings']['api_token'] = $token;
+ $config['cloudflare.settings']['auth_using'] = 'token';
+}Optional: if you foresee toggling the token at runtime across environments, consider unsetting these keys when $token is empty to avoid stale overrides from other includes. Not required for current flow.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/sites/default/includes/modules/settings.cloudflare.php around lines 10 to
13, avoid calling getenv twice and trim whitespace: read the
DRUPAL_CLOUDFLARE_API_TOKEN into a single variable, trim it, check that the
trimmed value is not empty, then set $config['cloudflare.settings']['api_token']
and $config['cloudflare.settings']['auth_using'] accordingly; optionally, if you
may toggle tokens at runtime, unset or clear those config keys when the trimmed
token is empty to prevent stale overrides.
| declare(strict_types=1); | ||
|
|
||
| $settings['config_exclude_modules'] = ['devel']; | ||
| $settings['config_exclude_modules'][] = 'devel'; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Idempotent append: initialize array and avoid duplicates
Appending blindly can create duplicates if this file is included multiple times or in varying include orders. Make the exclusion idempotent and resilient if the key was preset to a non-array.
-$settings['config_exclude_modules'][] = 'devel';
+$settings['config_exclude_modules'] = $settings['config_exclude_modules'] ?? [];
+if (!in_array('devel', $settings['config_exclude_modules'], true)) {
+ $settings['config_exclude_modules'][] = 'devel';
+}📝 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.
| $settings['config_exclude_modules'][] = 'devel'; | |
| $settings['config_exclude_modules'] = $settings['config_exclude_modules'] ?? []; | |
| if (!in_array('devel', $settings['config_exclude_modules'], true)) { | |
| $settings['config_exclude_modules'][] = 'devel'; | |
| } |
🤖 Prompt for AI Agents
In web/sites/default/includes/modules/settings.devel.php around line 10, the
current blind append "$settings['config_exclude_modules'][] = 'devel';" can
create duplicates or break if the key is not an array; ensure the key is an
array (if not, convert or initialize it to an empty array), then only append
'devel' if it is not already present (use an in-array check) so the operation is
idempotent and resilient to multiple includes or non-array existing values.
| $settings['config_exclude_modules'][] = 'purge_control'; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Append is good; defensively ensure no duplicates and initialization
Appending to config_exclude_modules is correct. To make it idempotent if this file is included multiple times or order changes, initialize the array and avoid duplicates.
Apply:
-$settings['config_exclude_modules'][] = 'purge_control';
+// Ensure the list exists and avoid duplicate entries.
+$settings['config_exclude_modules'] = $settings['config_exclude_modules'] ?? [];
+if (!in_array('purge_control', $settings['config_exclude_modules'], true)) {
+ $settings['config_exclude_modules'][] = 'purge_control';
+}📝 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.
| $settings['config_exclude_modules'][] = 'purge_control'; | |
| // Ensure the list exists and avoid duplicate entries. | |
| $settings['config_exclude_modules'] = $settings['config_exclude_modules'] ?? []; | |
| if (!in_array('purge_control', $settings['config_exclude_modules'], true)) { | |
| $settings['config_exclude_modules'][] = 'purge_control'; | |
| } |
🤖 Prompt for AI Agents
In web/sites/default/includes/modules/settings.purge_control.php around lines
10-11, the code appends 'purge_control' to $settings['config_exclude_modules']
but doesn't guard against the key being unset or duplicate entries; update the
code to ensure $settings['config_exclude_modules'] is initialized to an array if
missing and then add 'purge_control' only if it is not already present (e.g.,
check with isset/is_array and in_array or merge then deduplicate) so the
operation is idempotent when the file is included multiple times.
AlexSkrypnyk
force-pushed
the
feature/add-purger
branch
from
39feae9 to
4f37dc4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (13)
config/default/cloudflarepurger.settings.yml (1)
3-3: Fix schema type: use empty list instead of empty map for edge_cache_tag_header_blacklistThe module expects a sequence (list) of strings. An empty mapping
{}can cause config import/validation issues. Use[].Apply:
-edge_cache_tag_header_blacklist: { } +edge_cache_tag_header_blacklist: []config/default/cloudflare.settings.yml (2)
3-3: Default to safer client IP restore (disable by default)Enable IP restore only where all traffic is known to come via Cloudflare (e.g., prod). Keep validation on; flip the default to false.
-client_ip_restore_enabled: true +client_ip_restore_enabled: falseConsider enabling it in the production config split instead.
6-9: Avoid committing tenant-specific identifiers and misleading credential state
valid_credentials: truewith empty credentials is inconsistent, and committing a concretezone_idties defaults to a specific tenant. Use neutral defaults and populate via env/settings or env-specific splits.-valid_credentials: true -zone_id: - - 7303aceb6327852069d6653a5b5f2fb3 +valid_credentials: false +zone_id: []If you need environment-driven values, set them via settings.php (e.g., settings.cloudflare.php) or a production split.
web/sites/default/includes/modules/settings.cloudflare.php (1)
10-13: Read and trim env var once; then set configMinor clarity/perf improvement and avoids subtle whitespace issues.
-if (!empty(getenv('DRUPAL_CLOUDFLARE_API_TOKEN'))) { - $config['cloudflare.settings']['api_token'] = getenv('DRUPAL_CLOUDFLARE_API_TOKEN'); - $config['cloudflare.settings']['auth_using'] = 'token'; -} +<?php +$token = trim((string) (getenv('DRUPAL_CLOUDFLARE_API_TOKEN') ?: '')); +if ($token !== '') { + $config['cloudflare.settings']['api_token'] = $token; + $config['cloudflare.settings']['auth_using'] = 'token'; +}Optional: if tokens can be unset between environments, consider also clearing these keys when
$token === ''to avoid stale overrides.web/sites/default/includes/modules/settings.devel.php (1)
10-10: Make appending idempotent and resilientAvoid duplicates and ensure the key is an array if preset differently.
-$settings['config_exclude_modules'][] = 'devel'; +$settings['config_exclude_modules'] = $settings['config_exclude_modules'] ?? []; +if (!in_array('devel', $settings['config_exclude_modules'], true)) { + $settings['config_exclude_modules'][] = 'devel'; +}composer.json (1)
84-90: Good: VCS switched to HTTPS; verify that “options.reference” actually pins the transitive SDKNice improvement switching the VCS URL to HTTPS and specifying a commit reference. However, Composer does not guarantee pinning of a transitive dependency via repository options alone. If the forked SDK is required indirectly, installs can still drift unless you add an explicit constraint.
- Check whether the package is directly/indirectly required and ensure pinning:
#!/bin/bash # Inspect references to the forked SDK and any constraints rg -n -C2 'andresgmh|cloudflare-php' composer.json # If you rely on dev branches, prefer adding an explicit require with a pinned commit: # "andresgmh/cloudflare-php": "dev-main#2cb793bc18cd530e9140af308b3627c814294a7d"If you confirm it’s used transitively, add an explicit require (example):
{ "require": { "andresgmh/cloudflare-php": "dev-main#2cb793bc18cd530e9140af308b3627c814294a7d" } }config/default/core.extension.yml (1)
17-18: Cloudflare and purge modules are declared disabled by default: confirm the activation path per environmentHaving these at 0 is fine for shipping defaults. Ensure your config split or deployment steps explicitly enable them where intended (e.g., prod/preview), otherwise purge plugins/config won’t take effect.
If using config_split, verify the split enables these modules and is active in the target envs. You can search for a split toggling them:
#!/bin/bash # Look for config splits enabling cloudflare/purge-related modules rg -n -C2 'config_split|split|cloudflarepurger|cloudflare|purge_queuer_coretags|purge_ui|purge_drush|purge_processor_cron' configAlso applies to: 78-82
web/sites/default/includes/modules/settings.purge_control.php (1)
10-10: Make the exclude-append idempotent and safeRepeated includes can duplicate entries and unset key can cause notices. Initialize and dedupe.
Apply:
-$settings['config_exclude_modules'][] = 'purge_control'; +// Ensure the list exists and avoid duplicate entries. +$settings['config_exclude_modules'] = $settings['config_exclude_modules'] ?? []; +if (!in_array('purge_control', $settings['config_exclude_modules'], true)) { + $settings['config_exclude_modules'][] = 'purge_control'; +}tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
264-264: Avoid brittle equality on config_exclude_modules orderingStrict equality on ['devel', 'purge_control'] is fragile if include order changes or more excludes are added. Assert membership or ignore order for this key.
Option A (robust): Tweak the assertion helper to compare this key canonically.
Add to SettingsTestCase::assertSettings (or equivalent), before comparing arrays:
// For order-insensitive comparison of module excludes. foreach (['config_exclude_modules'] as $order_insensitive_key) { if (isset($expected[$order_insensitive_key]) && isset($this->settings[$order_insensitive_key]) && is_array($expected[$order_insensitive_key]) && is_array($this->settings[$order_insensitive_key])) { $e = $expected[$order_insensitive_key]; $a = $this->settings[$order_insensitive_key]; sort($e); sort($a); $expected[$order_insensitive_key] = $e; $this->settings[$order_insensitive_key] = $a; } }Option B (local change): If modifying the helper is not feasible, replace direct equality with set inclusion per test:
- $settings['config_exclude_modules'] = ['devel', 'purge_control']; + // Expect these modules to be excluded, order-agnostic. + $expected_excludes = ['devel', 'purge_control']; + // If assertSettings enforces strict equality, ensure both sides are sorted in the helper, + // or switch to a dedicated assertion: + $this->assertTrue(empty(array_diff($expected_excludes, $this->settings['config_exclude_modules'] ?? [])));Pick one approach and apply consistently to all occurrences noted.
Also applies to: 334-334, 380-380, 426-426, 473-473, 521-521, 571-571, 621-621, 672-672
tests/phpunit/Drupal/SwitchableSettingsTest.php (4)
561-565: Whitelist DRUPAL_CLOUDFLARE_API_TOKEN in SettingsTestCase to avoid env leakage across testsIf the token isn’t in ALLOWED_ENV_VARS, setEnvVars() may not reliably isolate it, creating order-dependent tests. Add it to the whitelist.
Run to verify whitelist and usage:
#!/bin/bash set -euo pipefail echo 'Locate ALLOWED_ENV_VARS and check for DRUPAL_CLOUDFLARE_API_TOKEN' fd -t f 'SettingsTestCase.php' | xargs -I{} rg -nC3 -e 'ALLOWED_ENV_VARS' -e '\bDRUPAL_CLOUDFLARE_API_TOKEN\b' {} echo echo 'Search for DRUPAL_CLOUDFLARE_API_TOKEN usage across repo' rg -n -C2 '\bDRUPAL_CLOUDFLARE_API_TOKEN\b' echo echo 'Check Cloudflare settings loader reads the token' rg -n -C3 'DRUPAL_CLOUDFLARE_API_TOKEN|api_token|auth_using' web/sites/default/includes/modules 2>/dev/null || truePatch to SettingsTestCase.php:
diff --git a/tests/phpunit/Drupal/SettingsTestCase.php b/tests/phpunit/Drupal/SettingsTestCase.php --- a/tests/phpunit/Drupal/SettingsTestCase.php +++ b/tests/phpunit/Drupal/SettingsTestCase.php @@ -61,6 +61,8 @@ const ALLOWED_ENV_VARS = [ // Service variables. 'DATABASE_', 'VALKEY_', + // Allow Cloudflare API token for proper test isolation. + 'DRUPAL_CLOUDFLARE_API_TOKEN', ];
561-605: Move the Cloudflare block above Shield to respect the file’s alphabetical ordering rulePer the file header, blocks should be ordered alphabetically by includes. Cloudflare (c…) currently sits after Shield (s…). Please relocate the entire Cloudflare test and its data provider so they appear above the Shield block.
592-602: Assert absence of the entire Cloudflare override, not specific valuesAsserting a specific token value is absent can miss cases where an unexpected value leaks in. Follow the suite pattern (e.g., other providers) and assert the whole override is absent with NULL.
Apply this diff:
[ [ 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, ], [ 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], ], [ - 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], + 'cloudflare.settings' => NULL, ], ],
577-604: Add CI coverage to validate purge is disabled in CI for Cloudflare scenariosThe PR disables purge in local/CI. Add a CI case to the provider to ensure expected behavior is enforced under CI as well.
Append this case to the data provider:
return [ // Test with DRUPAL_CLOUDFLARE_API_TOKEN set in local environment. [ [ 'DRUPAL_CLOUDFLARE_API_TOKEN' => 'test-api-token-12345', 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, ], [ 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], ], [], ], // Test without DRUPAL_CLOUDFLARE_API_TOKEN set in local environment. [ [ 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_LOCAL, ], [ 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], ], [ - 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], + 'cloudflare.settings' => NULL, ], ], + // Test with DRUPAL_CLOUDFLARE_API_TOKEN set in CI environment. + [ + [ + 'DRUPAL_CLOUDFLARE_API_TOKEN' => 'test-api-token-12345', + 'DRUPAL_ENVIRONMENT' => static::ENVIRONMENT_CI, + ], + [ + 'cloudflare.settings' => ['api_token' => 'test-api-token-12345', 'auth_using' => 'token'], + 'purge_control.settings' => ['disable_purge' => TRUE, 'purge_auto_control' => FALSE], + ], + [], + ], ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
composer.json(3 hunks)config/ci/purge_control.settings.yml(1 hunks)config/default/cloudflare.settings.yml(1 hunks)config/default/cloudflarepurger.settings.yml(1 hunks)config/default/config_split.config_split.ci.yml(1 hunks)config/default/config_split.config_split.local.yml(1 hunks)config/default/core.extension.yml(3 hunks)config/default/purge.logger_channels.yml(1 hunks)config/default/purge.plugins.yml(1 hunks)config/default/purge_queuer_coretags.settings.yml(1 hunks)config/local/purge_control.settings.yml(1 hunks)tests/phpunit/Drupal/EnvironmentSettingsTest.php(9 hunks)tests/phpunit/Drupal/SwitchableSettingsTest.php(1 hunks)web/sites/default/includes/modules/settings.cloudflare.php(1 hunks)web/sites/default/includes/modules/settings.devel.php(1 hunks)web/sites/default/includes/modules/settings.purge_control.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
tests/phpunit/Drupal/SettingsTestCase.php (1)
assertConfig(235-241)
🔇 Additional comments (9)
config/local/purge_control.settings.yml (1)
3-4: LGTM: Local/CI-friendly purge defaultsDisabling purge and auto-control locally is appropriate and aligns with the described splits/tests.
config/default/purge_queuer_coretags.settings.yml (1)
1-11: Config set looks correct and aligns with safe cache tag blacklistingThe blacklist entries are sensible for avoiding broad invalidations (routes, route_match, theme_registry, config:*). No issues spotted.
composer.json (1)
16-16: Consider pinning drupal/cloudflare to a specific 2.x pre-release
Using"^2@beta"can unexpectedly pull in newer beta releases. Once you’ve confirmed which beta you’ve tested, lock to that exact tag.• Verify available versions locally by running:
composer show drupal/cloudflare --all• Update your composer.json entry, for example:
"drupal/cloudflare": "^2@beta", -"drupal/cloudflare": "^2@beta", +"drupal/cloudflare": "2.0.0-beta1",config/default/core.extension.yml (1)
28-28: Verified presence ofctoolsanddo_basemodules
do_baseis defined atweb/modules/custom/do_base/do_base.info.ymlctoolsappears asdrupal/ctoolsincomposer.lockNo further changes needed here.
config/ci/purge_control.settings.yml (1)
1-4: CI-safe defaults look good (purge disabled, auto control off)Matches the intended behavior to prevent external cache mutations during CI. No changes requested.
config/default/purge.plugins.yml (1)
1-5: Confirm Cloudflare purger ordering and enablement
- Only one
purgers:block was found inconfig/default/purge.plugins.ymlwithorder_index: 2, and no otherorder_indexvalues orplugin_id: cloudflareentries exist underconfig/**. There’s no ordering conflict.- For this to have effect, make sure the Cloudflare purger plugin (e.g.
cloudflarepurger) is enabled in your target environment (e.g. in your modules/plugins config).config/default/purge.logger_channels.yml (1)
19-23: Confirmed: Cloudflare purger channel ID matches plugin instance
- In config/default/purge.plugins.yml:
• plugin_id: cloudflare
• instance_id: 946182794b- In config/default/purge.logger_channels.yml:
• id: purger_cloudflare_946182794bNo further changes needed.
config/default/config_split.config_split.ci.yml (1)
13-18: CI split configuration verified
The referencedpurge_control.settingsfile is present underconfig/ci(atconfig/ci/purge_control.settings.yml), so enabling the CI split will not produce import errors.tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
372-373: LGTM: Purge Control config expectations for Local/CIAdding purge_control.settings.disable_purge = TRUE and purge_control.settings.purge_auto_control = FALSE for Local and CI aligns with settings.purge_control.php and the new config splits.
Also applies to: 418-420, 466-468
| module: | ||
| purge_control: 0 | ||
| theme: { } | ||
| complete_list: | ||
| - purge_control.settings | ||
| partial_list: { } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify presence of Local split folder and referenced config
Same as CI: confirm ../config/local exists and contains purge_control.settings with the expected keys.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check Local split folder and purge_control settings presence"
fd -t d '^local$' config -x echo "Found Local config dir: {}" || echo "WARN: Local directory not found under config/"
fd -t f 'purge_control.settings.yml' -g 'config/**' -x echo "Found: {}" || echo "WARN: purge_control.settings.yml not found under config/**"
echo
echo "Preview file content (if present)"
fd -t f 'config/local/purge_control.settings.yml' -x sh -c 'echo "---- {} ----"; sed -n "1,120p" "$1"' sh || trueLength of output: 738
Missing local config: purge_control.settings not found
The config/local directory is present but there’s no purge_control.settings (or purge_control.settings.yml) file to match the reference in your split. Please add the local override file containing the expected keys or adjust the reference in config/default/config_split.config_split.local.yml.
Locations to address:
- config/default/config_split.config_split.local.yml (Lines 13–18)
- config/local/ ← add
purge_control.settings.ymlhere with the same keys referenced above
🤖 Prompt for AI Agents
In config/default/config_split.config_split.local.yml around lines 13 to 18, the
split references purge_control.settings but the corresponding local override
file is missing; add a file named purge_control.settings.yml under config/local
containing the expected keys and values referenced by the split (or
remove/adjust the purge_control.settings entry in the split if you intend not to
override it), ensuring the YAML keys match exactly and the file is readable by
the config loader.
| if ($settings['environment'] === ENVIRONMENT_LOCAL || $settings['environment'] === ENVIRONMENT_CI) { | ||
| $config['purge_control.settings']['disable_purge'] = TRUE; | ||
| $config['purge_control.settings']['purge_auto_control'] = FALSE; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Guard against undefined environment index and keep condition readable
If this file is included before $settings['environment'] is defined, strict array access will raise a notice. Use null coalescing. Minor readability tweak groups the check.
Apply:
-if ($settings['environment'] === ENVIRONMENT_LOCAL || $settings['environment'] === ENVIRONMENT_CI) {
+if ((($settings['environment'] ?? null) === ENVIRONMENT_LOCAL)
+ || (($settings['environment'] ?? null) === ENVIRONMENT_CI)) {
$config['purge_control.settings']['disable_purge'] = TRUE;
$config['purge_control.settings']['purge_auto_control'] = FALSE;
}📝 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.
| if ($settings['environment'] === ENVIRONMENT_LOCAL || $settings['environment'] === ENVIRONMENT_CI) { | |
| $config['purge_control.settings']['disable_purge'] = TRUE; | |
| $config['purge_control.settings']['purge_auto_control'] = FALSE; | |
| } | |
| if ((($settings['environment'] ?? null) === ENVIRONMENT_LOCAL) | |
| || (($settings['environment'] ?? null) === ENVIRONMENT_CI)) { | |
| $config['purge_control.settings']['disable_purge'] = TRUE; | |
| $config['purge_control.settings']['purge_auto_control'] = FALSE; | |
| } |
🤖 Prompt for AI Agents
In web/sites/default/includes/modules/settings.purge_control.php around lines 12
to 15, the condition accesses $settings['environment'] directly which can
trigger a notice if the index is undefined; replace direct access with a guarded
value (e.g. $env = $settings['environment'] ?? null) and then check membership
(e.g. in_array($env, [ENVIRONMENT_LOCAL, ENVIRONMENT_CI], TRUE)) or an
equivalent grouped comparison to keep the condition readable and null-safe; set
the purge_control config the same way when the guarded check passes.
Closes #95
Summary by CodeRabbit