[#1886] Added Lagoon logs package when Lagoon is used as a hosting provider. #1940
Conversation
WalkthroughAdds logic in HostingProvider::process() for Lagoon: after removing target htaccess, it loads composer.json from the temp directory, uses JsonManipulator to add a require entry for drupal/lagoon_logs:^3, and writes the updated composer.json back. Other providers unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant I as Installer CLI
participant HP as HostingProvider::process()
participant FS as File System
participant JM as JsonManipulator
U->>I: Select hosting provider (Lagoon)
I->>HP: process(selection="LAGOON", tmpDir)
HP->>FS: Remove target .htaccess
note over HP,FS: Existing step retained
HP->>FS: Read tmpDir/composer.json
FS-->>HP: composer.json contents
HP->>JM: addLink(require, "drupal/lagoon_logs", "^3", true)
JM-->>HP: Updated JSON
HP->>FS: Write tmpDir/composer.json
FS-->>HP: OK
HP-->>I: Done
I-->>U: Installer continues
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 phpcs (3.7.2).vortex/installer/src/Prompts/Handlers/HostingProvider.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information ✨ Finishing Touches
🧪 Generate unit tests
🪧 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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
.vortex/installer/tests/Fixtures/install/hosting_lagoon/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (1)
.vortex/installer/src/Prompts/Handlers/HostingProvider.php(2 hunks)
⏰ 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). (13)
- 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-workflow (3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-docs
🔇 Additional comments (1)
.vortex/installer/src/Prompts/Handlers/HostingProvider.php (1)
9-9: Import looks correct and scoped appropriately.Brings
JsonManipulatorinto scope for the new Lagoon branch logic. No concerns.
| $cj = JsonManipulator::fromFile($this->tmpDir . '/composer.json'); | ||
| $cj->addLink('require', 'drupal/lagoon_logs', '^3', TRUE); | ||
| file_put_contents($this->tmpDir . '/composer.json', $cj->getContents()); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Harden composer.json manipulation: guard rails + consistency.
Current code assumes composer.json exists and is readable, and writes without a lock. Also mixes $this->tmpDir with the local $t var. Recommend guarding existence/readability, using the local $t for consistency, checking addLink’s return value, and writing with LOCK_EX.
Apply this diff:
- $cj = JsonManipulator::fromFile($this->tmpDir . '/composer.json');
- $cj->addLink('require', 'drupal/lagoon_logs', '^3', TRUE);
- file_put_contents($this->tmpDir . '/composer.json', $cj->getContents());
+ $composerPath = sprintf('%s/composer.json', $t);
+ if (is_file($composerPath) && is_readable($composerPath)) {
+ $cj = JsonManipulator::fromFile($composerPath);
+ if ($cj->addLink('require', 'drupal/lagoon_logs', '^3', TRUE)) {
+ file_put_contents($composerPath, $cj->getContents(), LOCK_EX);
+ }
+ }If JsonManipulator::addLink() throws on invalid JSON or returns a non-boolean, consider wrapping in a try/catch and surfacing a friendly message to avoid installer crashes.
📝 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.
| $cj = JsonManipulator::fromFile($this->tmpDir . '/composer.json'); | |
| $cj->addLink('require', 'drupal/lagoon_logs', '^3', TRUE); | |
| file_put_contents($this->tmpDir . '/composer.json', $cj->getContents()); | |
| $composerPath = sprintf('%s/composer.json', $t); | |
| if (is_file($composerPath) && is_readable($composerPath)) { | |
| $cj = JsonManipulator::fromFile($composerPath); | |
| if ($cj->addLink('require', 'drupal/lagoon_logs', '^3', TRUE)) { | |
| file_put_contents($composerPath, $cj->getContents(), LOCK_EX); | |
| } | |
| } |
🤖 Prompt for AI Agents
.vortex/installer/src/Prompts/Handlers/HostingProvider.php lines 94-96: the code
assumes composer.json exists and is readable, uses $this->tmpDir instead of the
local $t var, doesn't check the result of addLink and writes without LOCK_EX;
update to first check that $t . '/composer.json' exists and is readable and
throw or return a clear installer-friendly error if not, use the local $t
variable everywhere for path consistency, call JsonManipulator::fromFile() and
wrap the addLink call in a try/catch to handle thrown exceptions or non-boolean
returns (and surface a friendly message), check the return value of addLink
before proceeding, and write the final contents with file_put_contents(...,
LOCK_EX) to avoid race conditions.
| $this->removeAcquia(); | ||
| @unlink(sprintf('%s/%s/.htaccess', $t, $w)); | ||
| $cj = JsonManipulator::fromFile($this->tmpDir . '/composer.json'); | ||
| $cj->addLink('require', 'drupal/lagoon_logs', '^3', TRUE); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double-check the version constraint '^3' against the template’s Drupal core.
If the project template targets a newer core that requires a later major of drupal/lagoon_logs, you may want to use a widened constraint (e.g., ^3 || ^4) or align to the matrix supported by your baseline. Please verify against the latest module releases.
🌐 Web query:
What is the latest stable major of the Composer package "drupal/lagoon_logs" and which Drupal core versions does it support?
💡 Result:
Searching for current package info... I’ll fetch and verify the latest stable major for drupal/lagoon_logs and its supported Drupal core versions, then cite sources.
🌐 Web query:
What is the latest stable major of the Composer package "drupal/lagoon_logs" and which Drupal core versions does it support?
💡 Result:
Search results indicate the Composer package "drupal/lagoon_logs" latest stable major is 2.x, which supports Drupal core 9 and 10. Source: project page on Drupal.org and the package release notes [1][2].
Sources:
[1] https://www.drupal.org/project/lagoon_logs
[2] https://www.drupal.org/project/lagoon_logs/releases
Update lagoon_logs version constraint to match latest stable major
The current constraint ^3 will fail because the latest stable major of drupal/lagoon_logs is 2.x (supporting Drupal core 9 and 10). Please adjust the constraint accordingly:
- File:
.vortex/installer/src/Prompts/Handlers/HostingProvider.php
Line 95: changeto either$cj->addLink('require', 'drupal/lagoon_logs', '^3', TRUE);
or, if you plan to adopt future 3.x releases once available, a widened range:$cj->addLink('require', 'drupal/lagoon_logs', '^2', TRUE);
$cj->addLink('require', 'drupal/lagoon_logs', '^2 || ^3', TRUE);
🤖 Prompt for AI Agents
.vortex/installer/src/Prompts/Handlers/HostingProvider.php around line 95: the
composer requirement currently pins drupal/lagoon_logs to ^3 which is
incompatible with the latest stable major (2.x); update the addLink call to
require a compatible constraint (e.g. ^2) or a widened range supporting both
current and future majors (e.g. ^2 || ^3) so the installer will not fail
resolving dependencies.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1940 +/- ##
===========================================
+ Coverage 76.23% 76.24% +0.01%
===========================================
Files 87 87
Lines 5382 5385 +3
Branches 35 35
===========================================
+ Hits 4103 4106 +3
Misses 1279 1279 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Closes #1886
Summary by CodeRabbit
New Features
Other Providers