Skip to content

Conversation

AlexSkrypnyk
Copy link
Member

@AlexSkrypnyk AlexSkrypnyk commented Oct 8, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Redis now enables only when the Redis extension is loaded or when an environment variable is explicitly set to '1', preventing unintended activation from ambiguous values and making configuration more predictable.
  • Tests
    • Removed an obsolete unit test that validated partial Redis settings when the Redis extension was disabled.

Copy link

coderabbitai bot commented Oct 8, 2025

Walkthrough

Removes a unit test for partial Redis settings, and simplifies Redis enablement logic in settings.redis.php to trigger when the Redis PHP extension is loaded or when the VORTEX_REDIS_EXTENSION_LOADED environment variable equals '1'.

Changes

Cohort / File(s) Summary
Tests: SwitchableSettings
tests/phpunit/Drupal/SwitchableSettingsTest.php
Removed method testRedisPartial() that validated behavior when Redis extension was disabled with partial settings. Other tests unchanged.
Settings: Redis enablement
web/sites/default/includes/modules/settings.redis.php
Simplified enablement condition: enable Redis if extension_loaded('redis') is true or VORTEX_REDIS_EXTENSION_LOADED == '1'. Removed previous mixed condition handling and updated inline comment.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Req as Request
  participant SL as Settings Loader
  participant PE as PHP Env & Extensions
  participant RB as Redis Bootstrap

  Req->>SL: Load settings
  SL->>PE: Check extension_loaded('redis')
  alt Redis extension loaded
    SL->>RB: Enable Redis backend
    RB-->>SL: Bootstrap container set
  else Not loaded
    SL->>PE: Read env VORTEX_REDIS_EXTENSION_LOADED
    alt Env == "1"
      SL->>RB: Enable Redis backend
      RB-->>SL: Bootstrap container set
    else Otherwise
      SL-->>Req: Proceed without Redis
    end
  end
  SL-->>Req: Continue initialization
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

A twitch of whiskers, toggle of a bit,
Redis awakes when envs permit.
One hop for tests—a partial gone,
Two hops for settings—clean and strong.
I stash my carrots, keys aligned 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The provided title clearly summarizes the main change by stating that the Redis extension loaded condition has been simplified, matching the core modification in settings.redis.php and giving a concise, specific description that allows reviewers to understand the primary update.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/fix-redis-condition

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between adc71b7 and e44b585.

⛔ Files ignored due to path filters (7)
  • .vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/SwitchableSettingsTest.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/includes/modules/settings.redis.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/modules/settings.redis.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/hosting_project_name___acquia/docroot/sites/default/includes/modules/settings.redis.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/non_interactive_config_file/tests/phpunit/Drupal/SwitchableSettingsTest.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/services_no_redis/tests/phpunit/Drupal/SwitchableSettingsTest.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/services_none/tests/phpunit/Drupal/SwitchableSettingsTest.php is excluded by !.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (2)
  • tests/phpunit/Drupal/SwitchableSettingsTest.php (0 hunks)
  • web/sites/default/includes/modules/settings.redis.php (1 hunks)
💤 Files with no reviewable changes (1)
  • tests/phpunit/Drupal/SwitchableSettingsTest.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: build (1)
  • GitHub Check: build (0)
  • GitHub Check: vortex-test-workflow (3)
  • GitHub Check: vortex-test-workflow (2)
  • GitHub Check: vortex-test-deployment (1)
  • GitHub Check: vortex-test-workflow (4)
  • GitHub Check: vortex-test-workflow (1)
  • GitHub Check: vortex-test-workflow (0)
  • GitHub Check: vortex-test-installer (8.3)
  • GitHub Check: vortex-test-installer (8.2)
  • GitHub Check: vortex-test-deployment (0)
  • GitHub Check: vortex-test-installer (8.4)
  • GitHub Check: vortex-test-common
  • GitHub Check: vortex-test-docs
🔇 Additional comments (1)
web/sites/default/includes/modules/settings.redis.php (1)

38-40: LGTM! Previous review feedback has been fully addressed.

Both suggestions from the previous review have been implemented:

  1. Comment updated to specify = '1' instead of "non-zero"
  2. Strict comparison (===) now used for type-safe comparison

The simplified logic (extension loaded OR env var equals '1') is clear and aligns well with the removal of the testRedisPartial() test mentioned in the PR summary.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot temporarily deployed to commit October 8, 2025 06:16 Inactive
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 65eeacc and adc71b7.

⛔ Files ignored due to path filters (7)
  • .vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/SwitchableSettingsTest.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/includes/modules/settings.redis.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/modules/settings.redis.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/hosting_project_name___acquia/docroot/sites/default/includes/modules/settings.redis.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/non_interactive_config_file/tests/phpunit/Drupal/SwitchableSettingsTest.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/services_no_redis/tests/phpunit/Drupal/SwitchableSettingsTest.php is excluded by !.vortex/installer/tests/Fixtures/**
  • .vortex/installer/tests/Fixtures/install/services_none/tests/phpunit/Drupal/SwitchableSettingsTest.php is excluded by !.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (2)
  • tests/phpunit/Drupal/SwitchableSettingsTest.php (0 hunks)
  • web/sites/default/includes/modules/settings.redis.php (1 hunks)
💤 Files with no reviewable changes (1)
  • tests/phpunit/Drupal/SwitchableSettingsTest.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: build (1)
  • GitHub Check: build (0)
  • GitHub Check: vortex-test-deployment (0)
  • GitHub Check: vortex-test-deployment (1)
  • GitHub Check: vortex-test-workflow (1)
  • GitHub Check: vortex-test-common
  • GitHub Check: vortex-test-workflow (3)
  • GitHub Check: vortex-test-installer (8.3)
  • GitHub Check: vortex-test-installer (8.4)
  • GitHub Check: vortex-test-workflow (4)
  • GitHub Check: vortex-test-workflow (0)
  • GitHub Check: vortex-test-workflow (2)
  • GitHub Check: vortex-test-installer (8.2)
  • GitHub Check: vortex-test-docs

Copy link

codecov bot commented Oct 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.05%. Comparing base (65eeacc) to head (e44b585).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2049   +/-   ##
========================================
  Coverage    64.05%   64.05%           
========================================
  Files           92       92           
  Lines         5692     5692           
  Branches        44       44           
========================================
  Hits          3646     3646           
  Misses        2046     2046           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/fix-redis-condition branch from adc71b7 to e44b585 Compare October 8, 2025 20:12
@github-actions github-actions bot temporarily deployed to commit October 8, 2025 20:14 Inactive
@AlexSkrypnyk AlexSkrypnyk merged commit 4c3f5dd into develop Oct 8, 2025
31 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/fix-redis-condition branch October 8, 2025 20:31
@github-project-automation github-project-automation bot moved this from BACKLOG to Release queue in Vortex Oct 8, 2025
@AlexSkrypnyk AlexSkrypnyk added this to the 25.9.0 milestone Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Release queue

Development

Successfully merging this pull request may close these issues.

1 participant