-
-
Notifications
You must be signed in to change notification settings - Fork 28
[#1642] Added support for migration DB. #2269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds optional second (migration) database: install prompts/handlers, env and Docker wiring for Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Installer
participant PromptManager
participant CLI as Ahoy/CLI
participant Docker
participant DownloadScript
participant Drush
participant DB2 as database2
User->>Installer: run installer
Installer->>PromptManager: ask Migration & Source
PromptManager-->>Installer: write .env / tokens
User->>CLI: ahoy build / up
CLI->>Docker: start services
Docker->>DB2: start & become ready
User->>CLI: ahoy download-db2
CLI->>DownloadScript: run download-db.sh (VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2)
DownloadScript-->>CLI: place .data/db2.sql
User->>CLI: ahoy provision
CLI->>Drush: run provision-20-migration.sh
Drush->>DB2: import/verify dump, run migrations
Drush-->>CLI: report status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.vortex/installer/src/Prompts/PromptManager.php (1)
7-18:⚠️ Potential issue | 🟠 MajorFix use-statement ordering (CI failure).
The pipeline reports Slevomat ordering violations; move the new
Migration*imports into alphabetical position.✅ Suggested fix
-use DrevOps\VortexInstaller\Prompts\Handlers\DeployTypes; -use DrevOps\VortexInstaller\Prompts\Handlers\Migration; -use DrevOps\VortexInstaller\Prompts\Handlers\MigrationDownloadSource; -use DrevOps\VortexInstaller\Prompts\Handlers\Domain; -use DrevOps\VortexInstaller\Prompts\Handlers\Dotenv; +use DrevOps\VortexInstaller\Prompts\Handlers\DeployTypes; +use DrevOps\VortexInstaller\Prompts\Handlers\Domain; +use DrevOps\VortexInstaller\Prompts\Handlers\Dotenv; +use DrevOps\VortexInstaller\Prompts\Handlers\Migration; +use DrevOps\VortexInstaller\Prompts\Handlers\MigrationDownloadSource;
🤖 Fix all issues with AI agents
In @.ahoy.yml:
- Around line 142-149: The download-db command uses the wrong env var name
(VORTEX_DOWNLOAD_DB_FRESH) so the --fresh flag is ignored; change the variable
exported in the download-db command to VORTEX_DOWNLOAD_DB_FORCE to match the
check in download-db.sh (and align with download-db2), ensuring the script sees
the force flag when run.
In @.circleci/config.yml:
- Around line 366-370: The shell variable expansion
${VORTEX_DOWNLOAD_DB2_FILE:-db2.sql} is unquoted in the if test and docker
compose cp command which can cause word-splitting; update both occurrences (the
if [ -f ... ] test and the docker compose cp -L ... command) to use the quoted
form "${VORTEX_DOWNLOAD_DB2_FILE:-db2.sql}" so the filename is treated as a
single token and matches the quoting style used elsewhere in CI.
In @.env:
- Around line 243-247: The env file defines duplicate keys named
VORTEX_DOWNLOAD_DB2_URL (in both the MIGRATION and DEMO sections) which triggers
dotenv-linter warnings; pick a single canonical key or rename the demo-specific
one (e.g., VORTEX_DOWNLOAD_DB2_URL_DEMO), or comment/gate the unused entry so
only one active VORTEX_DOWNLOAD_DB2_URL remains; update any code or docs that
reference VORTEX_DOWNLOAD_DB2_URL to use the new demo-specific name if you
rename it.
In @.github/workflows/build-test-deploy.yml:
- Around line 348-352: The two shell lines that test and copy the DB file use
unquoted parameter expansion `${VORTEX_DOWNLOAD_DB2_FILE:-db2.sql}` which risks
word-splitting (ShellCheck SC2086); update both occurrences in the if-test and
the docker compose cp invocation to use quoted expansions (e.g., wrap the
parameter expansion in double quotes) so the test [ -f ... ] and docker compose
cp -L target path handle filenames with spaces correctly; ensure you quote both
the source and destination expansions consistently in the if condition and the
docker compose cp call.
In @.lagoon.yml:
- Around line 64-72: The migration download step runs unconditionally and lacks
the pre-download cleanup; either (A) restrict it to non-production like the
primary DB download by adding the same environment guard used there and run rm
-Rf $VORTEX_DB_DIR || true before invoking VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2
./scripts/vortex/download-db.sh (preserving the
VORTEX_DOWNLOAD_DB_SSH_FILE/VORTEX_DB_DIR vars), or (B) if it is intended to run
in all environments, add an explicit comment above this run block stating that
running migrations/downloads in production is intentional and explain why;
update the block around VORTEX_DOWNLOAD_DB_SSH_FILE, VORTEX_DB_DIR and the
download script accordingly.
In @.vortex/docs/.utils/create-migrate-source-db.sh:
- Around line 8-12: The usage comment is misleading: it mentions DB_URL and
implies the script executes against a DB, but the script only emits SQL for
piping/import; update the header of create-migrate-source-db.sh to accurately
describe behavior (e.g., "prints SQL to stdout for piping/import; to execute,
pipe into mysql using DB_URL or use --sql with drush"), or implement DB_URL
handling by adding execution logic that reads DB_URL and runs the generated SQL
against that connection; reference the script name create-migrate-source-db.sh
and the DB_URL and --sql usages when making the change.
In @.vortex/installer/src/Prompts/Handlers/MigrationDownloadSource.php:
- Around line 93-115: Add a guard at the start of process() to return early when
the handler should not run: call $this->shouldRun() and if it returns false skip
the rest so you never call getResponseAsString() when Migration is disabled;
keep the rest of the logic (writing VORTEX_DOWNLOAD_DB2_SOURCE via
Env::writeValueDotenv using $this->tmpDir, iterating MigrationDownloadSource
constants and calling File::removeTokenAsync) unchanged but only execute it
after the shouldRun check.
In `@composer.json`:
- Around line 18-19: The composer.json references an invalid release for
drupal/migrate_tools (^6.0.8); update the version constraint for
"drupal/migrate_tools" to "^6.0.5" (or to a known valid constraint) and
optionally bump "drupal/migrate_plus" to "^6.0.10" if you want explicit Drupal
11 support; after changing the "drupal/migrate_tools" and/or
"drupal/migrate_plus" entries run composer update drupal/migrate_tools
drupal/migrate_plus to verify the resolved versions and run your test/site build
to ensure compatibility.
In `@docker-compose.yml`:
- Around line 201-207: The database2 service currently hardcodes
MYSQL_DATABASE/MYSQL_USER/MYSQL_PASSWORD to "drupal", which conflicts with the
external override variables; update the environment values in the database2
service so they read from the corresponding override variables (use
DATABASE2_NAME, DATABASE2_USERNAME, DATABASE2_PASSWORD with sensible defaults)
instead of the fixed "drupal" strings; edit the database2 block and replace the
MYSQL_DATABASE, MYSQL_USER and MYSQL_PASSWORD entries to reference those
override variables so container bootstrapping matches runtime overrides.
In `@scripts/custom/provision-20-migration.sh`:
- Around line 111-137: The script interpolates MIGRATION_SOURCE_DB_PROBE_TABLE
directly into SQL which allows injection; add a strict validation step before
the first probe (and reuse it before the final verification) that ensures
MIGRATION_SOURCE_DB_PROBE_TABLE matches an allowlist or a tight regex (e.g.,
only [a-zA-Z0-9_]+ or compare against an explicit list of known table names) and
abort with a clear fail message if validation fails; update the checks around
the drush sql:query calls (the probe and the final verification) to rely on the
validated value so no untrusted input is ever concatenated into the SQL.
- Around line 125-129: The current import uses indirect command substitution
with drush sql:connect, which triggers ShellCheck SC2091; replace that use with
the supported drush sql:cli invocation to run the SQL file into the migrate
database. Specifically, keep the drush sql:drop --database=migrate call, remove
the $(drush sql:connect --database=migrate) <... pattern, and invoke drush
sql:cli --database=migrate with the SQL file redirected into its stdin (using
the same ${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE} variable) so the import
uses drush's CLI entrypoint instead of command substitution.
In `@web/modules/custom/ys_migrate/src/Plugin/migrate/source/Categories.php`:
- Around line 54-56: The prepareRow() method in the Categories source plugin is
an unnecessary override because it only calls parent::prepareRow($row);; remove
the redundant prepareRow() method from the Categories class to let the inherited
implementation be used and reduce noise (ensure no other code in this class
depends on this method existing before deleting it).
| #;< MIGRATION | ||
| download-db2: | ||
| usage: Download second database (migration). Run with "--fresh" to force. | ||
| aliases: [fetch-db2] | ||
| cmd: | | ||
| case " $* " in *" --fresh "*) export VORTEX_DOWNLOAD_DB_FORCE=1;; esac | ||
| VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 ./scripts/vortex/download-db.sh | ||
| #;> MIGRATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Check what variables download-db.sh actually uses
rg -n "VORTEX_DOWNLOAD_DB_FORCE|VORTEX_DOWNLOAD_DB_FRESH" scripts/vortex/download-db.shRepository: drevops/vortex
Length of output: 278
🏁 Script executed:
# Check the download-db command to verify the variable it uses
sed -n '135,145p' .ahoy.ymlRepository: drevops/vortex
Length of output: 446
🏁 Script executed:
# Get more context around both commands
sed -n '130,155p' .ahoy.ymlRepository: drevops/vortex
Length of output: 998
Fix variable name mismatch in download-db command — the --fresh flag is broken.
The download-db command sets VORTEX_DOWNLOAD_DB_FRESH=1, but download-db.sh checks for VORTEX_DOWNLOAD_DB_FORCE (line 58). This means the --fresh flag has no effect. The download-db2 command correctly uses VORTEX_DOWNLOAD_DB_FORCE, so align download-db to use the same variable for consistency.
🤖 Prompt for AI Agents
In @.ahoy.yml around lines 142 - 149, The download-db command uses the wrong env
var name (VORTEX_DOWNLOAD_DB_FRESH) so the --fresh flag is ignored; change the
variable exported in the download-db command to VORTEX_DOWNLOAD_DB_FORCE to
match the check in download-db.sh (and align with download-db2), ensuring the
script sees the force flag when run.
| #;< MIGRATION_DB_DOWNLOAD_SOURCE_URL | ||
| # Second database dump file sourced from a URL. | ||
| # | ||
| # HTTP Basic Authentication credentials should be embedded into the value. | ||
| VORTEX_DOWNLOAD_DB2_URL= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid duplicate VORTEX_DOWNLOAD_DB2_URL keys.
The key appears in both the MIGRATION and DEMO blocks, which triggers the dotenv-linter duplication warning and makes precedence unclear. Consider commenting out one, using a distinct demo-only key, or gating so only one active value remains.
Also applies to: 361-365
🤖 Prompt for AI Agents
In @.env around lines 243 - 247, The env file defines duplicate keys named
VORTEX_DOWNLOAD_DB2_URL (in both the MIGRATION and DEMO sections) which triggers
dotenv-linter warnings; pick a single canonical key or rename the demo-specific
one (e.g., VORTEX_DOWNLOAD_DB2_URL_DEMO), or comment/gate the unused entry so
only one active VORTEX_DOWNLOAD_DB2_URL remains; update any code or docs that
reference VORTEX_DOWNLOAD_DB2_URL to use the new demo-specific name if you
rename it.
| #;< MIGRATION | ||
| - run: | ||
| name: Download migration database | ||
| command: | | ||
| export VORTEX_DOWNLOAD_DB_SSH_FILE=false | ||
| export VORTEX_DB_DIR=/tmp/data | ||
| VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 ./scripts/vortex/download-db.sh | ||
| service: cli | ||
| #;> MIGRATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify intended behavior for migration database download.
Two potential concerns:
-
No environment check: Unlike the primary DB download (lines 54-60) which skips production, the migration download runs unconditionally. If migrations should only run in non-production environments, add an environment check.
-
Missing directory cleanup: The primary DB download runs
rm -Rf $VORTEX_DB_DIR || truebefore downloading, but the migration download doesn't. Since both use/tmp/data, stale files could cause issues.
If migrations should run in all environments, consider adding a comment to clarify this is intentional.
Suggested fix (if non-production only)
#;< MIGRATION
- run:
name: Download migration database
command: |
+ if [ "$LAGOON_ENVIRONMENT_TYPE" != "production" ] && [ "$LAGOON_GIT_BRANCH" != "${VORTEX_LAGOON_PRODUCTION_BRANCH:-main}" ]; then
export VORTEX_DOWNLOAD_DB_SSH_FILE=false
export VORTEX_DB_DIR=/tmp/data
VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 ./scripts/vortex/download-db.sh
+ fi
service: cli
#;> MIGRATION📝 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.
| #;< MIGRATION | |
| - run: | |
| name: Download migration database | |
| command: | | |
| export VORTEX_DOWNLOAD_DB_SSH_FILE=false | |
| export VORTEX_DB_DIR=/tmp/data | |
| VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 ./scripts/vortex/download-db.sh | |
| service: cli | |
| #;> MIGRATION | |
| #;< MIGRATION | |
| - run: | |
| name: Download migration database | |
| command: | | |
| if [ "$LAGOON_ENVIRONMENT_TYPE" != "production" ] && [ "$LAGOON_GIT_BRANCH" != "${VORTEX_LAGOON_PRODUCTION_BRANCH:-main}" ]; then | |
| export VORTEX_DOWNLOAD_DB_SSH_FILE=false | |
| export VORTEX_DB_DIR=/tmp/data | |
| VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 ./scripts/vortex/download-db.sh | |
| fi | |
| service: cli | |
| #;> MIGRATION |
🤖 Prompt for AI Agents
In @.lagoon.yml around lines 64 - 72, The migration download step runs
unconditionally and lacks the pre-download cleanup; either (A) restrict it to
non-production like the primary DB download by adding the same environment guard
used there and run rm -Rf $VORTEX_DB_DIR || true before invoking
VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 ./scripts/vortex/download-db.sh
(preserving the VORTEX_DOWNLOAD_DB_SSH_FILE/VORTEX_DB_DIR vars), or (B) if it is
intended to run in all environments, add an explicit comment above this run
block stating that running migrations/downloads in production is intentional and
explain why; update the block around VORTEX_DOWNLOAD_DB_SSH_FILE, VORTEX_DB_DIR
and the download script accordingly.
| "drupal/migrate_plus": "^6.0.7", | ||
| "drupal/migrate_tools": "^6.0.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
drupal/migrate_plus 6.0.7 drupal 11 compatibility
💡 Result:
Yes—drupal/migrate_plus 6.0.7 is compatible with Drupal 11: its release page lists “Works with Drupal: >= 9.1”, which includes Drupal 11. [1]
That said, for Drupal 11 you should generally prefer a newer 6.x release (e.g. 6.0.10), which explicitly declares Drupal core compatibility as ^10.5 || ^11 on the project page. [2]
Sources: [1] migrate_plus 6.0.7 release page, [2] Migrate Plus project page (latest release compatibility).
🌐 Web query:
drupal/migrate_tools 6.0.8 drupal 11 compatibility
💡 Result:
There is no drupal/migrate_tools release 6.0.8 on Drupal.org (the official upstream for the Composer package); the latest stable in the 6.0.x line is 6.0.5. [1], [2]
For Drupal 11 compatibility, migrate_tools 6.0.5 explicitly declares: “Works with Drupal: ^9.1 || ^10 || ^11”, so 6.0.5 is Drupal 11–compatible (per its release metadata). [2]
If you specifically have 6.0.8 in your lockfile, it’s likely coming from a non-Drupal.org source (or a mistaken version reference); in that case, compare your composer.lock “source/dist” to the official release list. [1], [3]
Sources:
[1] Migrate Tools project page (release list / supported cores)
[2] migrate_tools 6.0.5 release page (“Works with Drupal: ^9.1 || ^10 || ^11”)
[3] Drupal.org releases index (shows available tags; no 6.0.8)
Fix drupal/migrate_tools version: 6.0.8 does not exist on Drupal.org.
drupal/migrate_plus: ^6.0.7 is compatible with Drupal 11. However, drupal/migrate_tools: ^6.0.8 does not have a release on Drupal.org; the latest stable in the 6.0.x series is 6.0.5, which is Drupal 11-compatible (^9.1 || ^10 || ^11). Change ^6.0.8 to ^6.0.5. Alternatively, consider upgrading migrate_plus to ^6.0.10 for explicit Drupal 11 support (^10.5 || ^11).
🤖 Prompt for AI Agents
In `@composer.json` around lines 18 - 19, The composer.json references an invalid
release for drupal/migrate_tools (^6.0.8); update the version constraint for
"drupal/migrate_tools" to "^6.0.5" (or to a known valid constraint) and
optionally bump "drupal/migrate_plus" to "^6.0.10" if you want explicit Drupal
11 support; after changing the "drupal/migrate_tools" and/or
"drupal/migrate_plus" entries run composer update drupal/migrate_tools
drupal/migrate_plus to verify the resolved versions and run your test/site build
to ensure compatibility.
| database2: | ||
| image: uselagoon/mysql-8.4:26.1.0 | ||
| environment: | ||
| <<: *default-environment | ||
| MYSQL_DATABASE: drupal | ||
| MYSQL_USER: drupal | ||
| MYSQL_PASSWORD: drupal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align database2 init credentials with DATABASE2_* to avoid mismatches.
If users override DATABASE2_NAME/USERNAME/PASSWORD, the container still bootstraps drupal/drupal, which can break connections.
🔧 Proposed fix
environment:
<<: *default-environment
- MYSQL_DATABASE: drupal
- MYSQL_USER: drupal
- MYSQL_PASSWORD: drupal
+ MYSQL_DATABASE: ${DATABASE2_NAME:-drupal}
+ MYSQL_USER: ${DATABASE2_USERNAME:-drupal}
+ MYSQL_PASSWORD: ${DATABASE2_PASSWORD:-drupal}📝 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.
| database2: | |
| image: uselagoon/mysql-8.4:26.1.0 | |
| environment: | |
| <<: *default-environment | |
| MYSQL_DATABASE: drupal | |
| MYSQL_USER: drupal | |
| MYSQL_PASSWORD: drupal | |
| database2: | |
| image: uselagoon/mysql-8.4:26.1.0 | |
| environment: | |
| <<: *default-environment | |
| MYSQL_DATABASE: ${DATABASE2_NAME:-drupal} | |
| MYSQL_USER: ${DATABASE2_USERNAME:-drupal} | |
| MYSQL_PASSWORD: ${DATABASE2_PASSWORD:-drupal} |
🤖 Prompt for AI Agents
In `@docker-compose.yml` around lines 201 - 207, The database2 service currently
hardcodes MYSQL_DATABASE/MYSQL_USER/MYSQL_PASSWORD to "drupal", which conflicts
with the external override variables; update the environment values in the
database2 service so they read from the corresponding override variables (use
DATABASE2_NAME, DATABASE2_USERNAME, DATABASE2_PASSWORD with sensible defaults)
instead of the fixed "drupal" strings; edit the database2 block and replace the
MYSQL_DATABASE, MYSQL_USER and MYSQL_PASSWORD entries to reference those
override variables so container bootstrapping matches runtime overrides.
| # Detect if existing migration source database is corrupted. | ||
| if [ "${MIGRATION_SOURCE_DB_IMPORT}" != "1" ]; then | ||
| note "Source database import is set to be skipped. Checking existing database." | ||
| task "Probing for '${MIGRATION_SOURCE_DB_PROBE_TABLE}' table in the source database." | ||
| if ! drush sql:query --database=migrate "SELECT COUNT(*) FROM ${MIGRATION_SOURCE_DB_PROBE_TABLE}" >/dev/null 2>&1; then | ||
| note "Migration source database is corrupted or empty. Re-importing." | ||
| MIGRATION_SOURCE_DB_IMPORT=1 | ||
| fi | ||
| fi | ||
|
|
||
| # Import the migration source database from the dump file. | ||
| if [ "${MIGRATION_SOURCE_DB_IMPORT}" = "1" ]; then | ||
| task "Importing migration source database." | ||
|
|
||
| [ ! -f "${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}" ] && fail "Migration source database file not found. Please run 'ahoy download-db2'." && exit 1 | ||
|
|
||
| drush sql:drop --database=migrate | ||
| $(drush sql:connect --database=migrate) <"${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}" | ||
|
|
||
| pass "Imported migration source database." | ||
| else | ||
| note "Using existing migration source database." | ||
| fi | ||
|
|
||
| task "Verifying migration source database." | ||
| if ! drush sql:query --database=migrate "SELECT COUNT(*) FROM ${MIGRATION_SOURCE_DB_PROBE_TABLE}" >/dev/null 2>&1; then | ||
| fail "Migration source database is corrupted." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate MIGRATION_SOURCE_DB_PROBE_TABLE before interpolating into SQL.
It’s env-driven and currently allows SQL injection (e.g., categories; DROP TABLE ...). Add a strict whitelist check before the first query.
🛡️ Proposed guard
# Table name to probe in the source database to verify it is not corrupted.
MIGRATION_SOURCE_DB_PROBE_TABLE="${MIGRATION_SOURCE_DB_PROBE_TABLE:-categories}"
+
+# Allow only simple table identifiers to avoid SQL injection.
+if ! [[ "${MIGRATION_SOURCE_DB_PROBE_TABLE}" =~ ^[A-Za-z0-9_]+$ ]]; then
+ fail "Invalid MIGRATION_SOURCE_DB_PROBE_TABLE: ${MIGRATION_SOURCE_DB_PROBE_TABLE}"
+ exit 1
+fi🧰 Tools
🪛 GitHub Actions: Vortex - Test
[error] 128-128: ShellCheck SC2091 (warning): Remove surrounding $() to avoid executing output in provision-20-migration.sh.
🤖 Prompt for AI Agents
In `@scripts/custom/provision-20-migration.sh` around lines 111 - 137, The script
interpolates MIGRATION_SOURCE_DB_PROBE_TABLE directly into SQL which allows
injection; add a strict validation step before the first probe (and reuse it
before the final verification) that ensures MIGRATION_SOURCE_DB_PROBE_TABLE
matches an allowlist or a tight regex (e.g., only [a-zA-Z0-9_]+ or compare
against an explicit list of known table names) and abort with a clear fail
message if validation fails; update the checks around the drush sql:query calls
(the probe and the final verification) to rely on the validated value so no
untrusted input is ever concatenated into the SQL.
| [ ! -f "${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}" ] && fail "Migration source database file not found. Please run 'ahoy download-db2'." && exit 1 | ||
|
|
||
| drush sql:drop --database=migrate | ||
| $(drush sql:connect --database=migrate) <"${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and view the specific lines
if [ -f "scripts/custom/provision-20-migration.sh" ]; then
echo "=== File exists. Showing lines 120-135 ==="
sed -n '120,135p' scripts/custom/provision-20-migration.sh
else
echo "File not found at scripts/custom/provision-20-migration.sh"
# Try to find similar files
find . -name "*provision*migration*" -type f 2>/dev/null | head -20
fiRepository: drevops/vortex
Length of output: 687
🏁 Script executed:
# Search for other drush sql commands used in the codebase
echo "=== Drush sql command usage in codebase ==="
rg "drush sql:" --type sh --type bash -A 1 -B 1Repository: drevops/vortex
Length of output: 147
🌐 Web query:
Drush sql:cli command documentation
💡 Result:
Drush sql:cli documentation (Drush 12/13):
- Command:
drush sql:cli— opens a SQL command-line interface using the current Drupal site’s DB credentials. (drush.org) - Aliases:
sqlc,sql-cli. (drush.org) - Common examples:
drush sql:clidrush sql:cli --extra=--progress-reportsdrush sql:cli < example.sql(drush.org)
- Key options:
--extra,--database,--db-url,--target,--show-passwords. (drush.org)
Official docs page (URL in code form):
https://www.drush.org/12.4.3/commands/sql_cli/
Citations:
- 1: https://www.drush.org/12.4.3/commands/sql_cli/?utm_source=openai
- 2: https://www.drush.org/12.4.3/commands/sql_cli/?utm_source=openai
- 3: https://www.drush.org/12.4.3/commands/sql_cli/?utm_source=openai
- 4: https://www.drush.org/12.4.3/commands/sql_cli/?utm_source=openai
Fix import command to use proper Drush SQL CLI instead of command substitution.
The current line triggers the ShellCheck SC2091 failure in CI due to the indirect $(drush sql:connect) pattern. Use drush sql:cli instead—a purpose-built Drush command that is officially supported with the --database option.
Fix
- $(drush sql:connect --database=migrate) <"${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}"
+ drush sql:cli --database=migrate < "${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}"📝 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.
| [ ! -f "${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}" ] && fail "Migration source database file not found. Please run 'ahoy download-db2'." && exit 1 | |
| drush sql:drop --database=migrate | |
| $(drush sql:connect --database=migrate) <"${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}" | |
| [ ! -f "${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}" ] && fail "Migration source database file not found. Please run 'ahoy download-db2'." && exit 1 | |
| drush sql:drop --database=migrate | |
| drush sql:cli --database=migrate < "${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE}" | |
🧰 Tools
🪛 GitHub Actions: Vortex - Test
[error] 128-128: ShellCheck SC2091 (warning): Remove surrounding $() to avoid executing output in provision-20-migration.sh.
🤖 Prompt for AI Agents
In `@scripts/custom/provision-20-migration.sh` around lines 125 - 129, The current
import uses indirect command substitution with drush sql:connect, which triggers
ShellCheck SC2091; replace that use with the supported drush sql:cli invocation
to run the SQL file into the migrate database. Specifically, keep the drush
sql:drop --database=migrate call, remove the $(drush sql:connect
--database=migrate) <... pattern, and invoke drush sql:cli --database=migrate
with the SQL file redirected into its stdin (using the same
${VORTEX_DB_DIR}/${VORTEX_DOWNLOAD_DB2_FILE} variable) so the import uses
drush's CLI entrypoint instead of command substitution.
| public function prepareRow(Row $row) { | ||
| return parent::prepareRow($row); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Drop redundant prepareRow() override. It only calls the parent and adds noise.
♻️ Proposed cleanup
- public function prepareRow(Row $row) {
- return parent::prepareRow($row);
- }
-🤖 Prompt for AI Agents
In `@web/modules/custom/ys_migrate/src/Plugin/migrate/source/Categories.php`
around lines 54 - 56, The prepareRow() method in the Categories source plugin is
an unnecessary override because it only calls parent::prepareRow($row);; remove
the redundant prepareRow() method from the Categories class to let the inherited
implementation be used and reduce noise (ensure no other code in this class
depends on this method existing before deleting it).
a6861ff to
6e25d5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @.ahoy.yml:
- Around line 54-61: The db2 command in .ahoy.yml hardcodes the database path as
/drupal; update the command that builds the Sequel Ace URL (the db2 cmd block)
to use the DATABASE2_NAME environment variable with a default fallback (e.g.,
${DATABASE2_NAME:-drupal}) instead of the literal "/drupal" so custom DB names
are respected and the open URL is constructed dynamically.
In @.vortex/docs/.utils/create-migrate-source-db.sh:
- Around line 42-47: The conditional checking "${1:-}" = "--sql" is redundant
because both branches print the same sql variable; change behavior so "--sql"
remains a dry-run/output-only mode and the default path actually executes the
SQL against the configured DB_URL: keep the if block to print and exit when
"${1:-}" = "--sql", and in the else branch replace the echo "${sql}" with logic
that uses DB_URL to run the SQL (e.g., invoke psql/mysql/appropriate client with
DB_URL, handle errors and non-zero exit codes), ensuring you reference the sql
variable and DB_URL and return meaningful exit codes on failure.
In @.vortex/docs/content/drupal/migrations.mdx:
- Around line 146-153: The documentation wrongly lists the source plugin as
`ys_migrate_categories` — that is the migration ID, not the plugin; update the
table for the `ys_migrate_categories` migration to show Source plugin = `table`
(from `migrate_plus`) and, if helpful, add a short note clarifying that
`ys_migrate_categories` is the migration ID while `plugin: table` is the actual
source plugin used in the migration YAML.
In @.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php:
- Around line 265-269: Change the value for VORTEX_INSTALLER_PROMPT_MIGRATION in
the static::$sutInstallerEnv array from the PHP boolean TRUE to the string '1'
to match the other VORTEX_INSTALLER_PROMPT_* and VORTEX_INSTALLER_IS_DEMO
entries; update the array entry where static::$sutInstallerEnv is defined so the
key 'VORTEX_INSTALLER_PROMPT_MIGRATION' has the string '1' instead of TRUE.
In @.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php:
- Around line 793-801: The test subtestAhoyMigrationDownloadDb modifies .env by
adding VORTEX_DOWNLOAD_DB2_URL but never restores it; to prevent test bleed,
save the original .env contents (or the original value of
VORTEX_DOWNLOAD_DB2_URL) before calling fileAddVar, then ensure .env is restored
in a finally/tearDown block (or call a complementary fileRemoveVar/reset method)
after the download/assertion completes; update subtestAhoyMigrationDownloadDb to
capture the prior state, perform the existing actions, and always restore the
original .env value so subsequent tests run with the original environment.
In `@docker-compose.yml`:
- Around line 69-79: The DATABASE2_* entries (DATABASE2_HOST, DATABASE2_NAME,
DATABASE2_USERNAME, DATABASE2_PASSWORD, DATABASE2_PORT) are hardcoded so .env
overrides won't work; update each value to use environment variable substitution
with sensible defaults (e.g. change DATABASE2_HOST: database2 to DATABASE2_HOST:
"${DATABASE2_HOST:-database2}" and do the same pattern for DATABASE2_NAME,
DATABASE2_USERNAME, DATABASE2_PASSWORD, DATABASE2_PORT) so the compose file
reads from .env while preserving existing defaults; also ensure the
corresponding keys exist in the project .env for users to override.
In `@scripts/custom/provision-20-migration.sh`:
- Around line 58-59: The Settings class reference is incorrectly cased in the
drush php:eval call—change "Drupal\core\Site\Settings" to
"Drupal\Core\Site\Settings" in the environment assignment (the line setting
environment="$(drush php:eval "print
\Drupal\core\Site\Settings::get('environment');")") so the autoloader can find
the class on case-sensitive filesystems; update that occurrence to use the
correct namespace casing.
| #;< MIGRATION | ||
| db2: | ||
| usage: Open DB2 in Sequel Ace. | ||
| cmd: | | ||
| uname -a | grep -i -q darwin && test -d "${VORTEX_HOST_SEQUELACE_PATH:-/Applications/Sequel Ace.app}" && \ | ||
| VORTEX_HOST_DB2_PORT=$(docker compose port database2 3306 2>/dev/null | cut -d : -f 2) && \ | ||
| open "mysql://${DATABASE2_USERNAME:-drupal}:${DATABASE2_PASSWORD:-drupal}@127.0.0.1:${VORTEX_HOST_DB2_PORT}/drupal" -a "Sequel Ace" \ | ||
| || echo "Not a supported OS or Sequel Ace is not installed." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use DATABASE2_NAME in db2 command to respect custom DB names.
Hardcoding /drupal breaks when DB2 name differs from the default.
✏️ Suggested fix
- open "mysql://${DATABASE2_USERNAME:-drupal}:${DATABASE2_PASSWORD:-drupal}@127.0.0.1:${VORTEX_HOST_DB2_PORT}/drupal" -a "Sequel Ace" \
+ open "mysql://${DATABASE2_USERNAME:-drupal}:${DATABASE2_PASSWORD:-drupal}@127.0.0.1:${VORTEX_HOST_DB2_PORT}/${DATABASE2_NAME:-drupal}" -a "Sequel Ace" \📝 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.
| #;< MIGRATION | |
| db2: | |
| usage: Open DB2 in Sequel Ace. | |
| cmd: | | |
| uname -a | grep -i -q darwin && test -d "${VORTEX_HOST_SEQUELACE_PATH:-/Applications/Sequel Ace.app}" && \ | |
| VORTEX_HOST_DB2_PORT=$(docker compose port database2 3306 2>/dev/null | cut -d : -f 2) && \ | |
| open "mysql://${DATABASE2_USERNAME:-drupal}:${DATABASE2_PASSWORD:-drupal}@127.0.0.1:${VORTEX_HOST_DB2_PORT}/drupal" -a "Sequel Ace" \ | |
| || echo "Not a supported OS or Sequel Ace is not installed." | |
| #;< MIGRATION | |
| db2: | |
| usage: Open DB2 in Sequel Ace. | |
| cmd: | | |
| uname -a | grep -i -q darwin && test -d "${VORTEX_HOST_SEQUELACE_PATH:-/Applications/Sequel Ace.app}" && \ | |
| VORTEX_HOST_DB2_PORT=$(docker compose port database2 3306 2>/dev/null | cut -d : -f 2) && \ | |
| open "mysql://${DATABASE2_USERNAME:-drupal}:${DATABASE2_PASSWORD:-drupal}@127.0.0.1:${VORTEX_HOST_DB2_PORT}/${DATABASE2_NAME:-drupal}" -a "Sequel Ace" \ | |
| || echo "Not a supported OS or Sequel Ace is not installed." |
🤖 Prompt for AI Agents
In @.ahoy.yml around lines 54 - 61, The db2 command in .ahoy.yml hardcodes the
database path as /drupal; update the command that builds the Sequel Ace URL (the
db2 cmd block) to use the DATABASE2_NAME environment variable with a default
fallback (e.g., ${DATABASE2_NAME:-drupal}) instead of the literal "/drupal" so
custom DB names are respected and the open URL is constructed dynamically.
| if [ "${1:-}" = "--sql" ]; then | ||
| printf '%s\n' "${sql}" | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "${sql}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant conditional: both branches output identical content.
The --sql flag has no meaningful effect—both paths emit the same SQL to stdout. Consider either:
- Removing the
--sqlflag and always outputting SQL, or - Implementing actual database execution (using
DB_URL) in the default path, reserving--sqlfor dry-run/output-only mode.
💡 Option 1: Simplify by removing the flag
-if [ "${1:-}" = "--sql" ]; then
- printf '%s\n' "${sql}"
- exit 0
-fi
-
-echo "${sql}"
+printf '%s\n' "${sql}"🤖 Prompt for AI Agents
In @.vortex/docs/.utils/create-migrate-source-db.sh around lines 42 - 47, The
conditional checking "${1:-}" = "--sql" is redundant because both branches print
the same sql variable; change behavior so "--sql" remains a dry-run/output-only
mode and the default path actually executes the SQL against the configured
DB_URL: keep the if block to print and exit when "${1:-}" = "--sql", and in the
else branch replace the echo "${sql}" with logic that uses DB_URL to run the SQL
(e.g., invoke psql/mysql/appropriate client with DB_URL, handle errors and
non-zero exit codes), ensuring you reference the sql variable and DB_URL and
return meaningful exit codes on failure.
| ### Migration: `ys_migrate_categories` | ||
|
|
||
| | Property | Value | | ||
| |----------|-------| | ||
| | Source plugin | `ys_migrate_categories` (reads from `categories` table) | | ||
| | Source key | `migrate` (the `$databases['migrate']` connection) | | ||
| | Destination | `entity:taxonomy_term` | | ||
| | Vocabulary | `tags` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation error: incorrect source plugin name.
The table states the source plugin is ys_migrate_categories, but the actual migration YAML uses plugin: table (from migrate_plus). The ID is ys_migrate_categories, not the plugin.
📝 Suggested fix
| Property | Value |
|----------|-------|
-| Source plugin | `ys_migrate_categories` (reads from `categories` table) |
+| Source plugin | `table` (reads from `categories` table) |
| Source key | `migrate` (the `$databases['migrate']` connection) |📝 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.
| ### Migration: `ys_migrate_categories` | |
| | Property | Value | | |
| |----------|-------| | |
| | Source plugin | `ys_migrate_categories` (reads from `categories` table) | | |
| | Source key | `migrate` (the `$databases['migrate']` connection) | | |
| | Destination | `entity:taxonomy_term` | | |
| | Vocabulary | `tags` | | |
| ### Migration: `ys_migrate_categories` | |
| | Property | Value | | |
| |----------|-------| | |
| | Source plugin | `table` (reads from `categories` table) | | |
| | Source key | `migrate` (the `$databases['migrate']` connection) | | |
| | Destination | `entity:taxonomy_term` | | |
| | Vocabulary | `tags` | |
🤖 Prompt for AI Agents
In @.vortex/docs/content/drupal/migrations.mdx around lines 146 - 153, The
documentation wrongly lists the source plugin as `ys_migrate_categories` — that
is the migration ID, not the plugin; update the table for the
`ys_migrate_categories` migration to show Source plugin = `table` (from
`migrate_plus`) and, if helpful, add a short note clarifying that
`ys_migrate_categories` is the migration ID while `plugin: table` is the actual
source plugin used in the migration YAML.
| static::$sutInstallerEnv = [ | ||
| 'VORTEX_INSTALLER_IS_DEMO' => '1', | ||
| 'VORTEX_INSTALLER_PROMPT_MIGRATION' => TRUE, | ||
| 'VORTEX_INSTALLER_PROMPT_MIGRATION_DOWNLOAD_SOURCE' => 'url', | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all VORTEX_INSTALLER_PROMPT_* variables to see patterns
rg -n "VORTEX_INSTALLER_PROMPT_" --type=php -C1Repository: drevops/vortex
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Also check how VORTEX_INSTALLER_IS_DEMO is used across the test file to establish pattern
rg -n "VORTEX_INSTALLER_IS_DEMO" --type=php .vortex/tests/phpunit/Functional/AhoyWorkflowTest.php -C1Repository: drevops/vortex
Length of output: 1218
🏁 Script executed:
#!/bin/bash
# Check the actual test file around line 267 to see context
head -n 290 .vortex/tests/phpunit/Functional/AhoyWorkflowTest.php | tail -n 50Repository: drevops/vortex
Length of output: 1960
🏁 Script executed:
#!/bin/bash
# Find the parent class and see how $sutInstallerEnv is used
rg -n "sutInstallerEnv" --type=php -B2 -A2Repository: drevops/vortex
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for how VORTEX_INSTALLER_PROMPT_MIGRATION is read/used in installer code
rg -r "VORTEX_INSTALLER_PROMPT_MIGRATION" --type=phpRepository: drevops/vortex
Length of output: 454
🏁 Script executed:
#!/bin/bash
# Search for how sutInstallerEnv is used
rg "sutInstallerEnv" --type=phpRepository: drevops/vortex
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for the test base class
find . -name "*TestBase*" -o -name "*FunctionalTestBase*" | grep -E "\.php$"Repository: drevops/vortex
Length of output: 12952
🏁 Script executed:
#!/bin/bash
# Search for VORTEX_INSTALLER_PROMPT_MIGRATION in the codebase
rg "VORTEX_INSTALLER_PROMPT_MIGRATION" --type=phpRepository: drevops/vortex
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check the parent class and beginning of AhoyWorkflowTest to understand the inheritance
head -n 50 .vortex/tests/phpunit/Functional/AhoyWorkflowTest.phpRepository: drevops/vortex
Length of output: 1046
🏁 Script executed:
#!/bin/bash
# Find where the parent test base class is for Ahoy tests
find . -name "AhoyWorkflowTestBase.php" -o -name "*AhoyTest*Base*" 2>/dev/nullRepository: drevops/vortex
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Find FunctionalTestCase class
find . -name "FunctionalTestCase.php" | head -5Repository: drevops/vortex
Length of output: 175
🏁 Script executed:
#!/bin/bash
# Search for $sutInstallerEnv usage in the codebase more broadly
rg '\$sutInstallerEnv' --type=php -B2 -A2 | head -100Repository: drevops/vortex
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Read the FunctionalTestCase to see how sutInstallerEnv is used
cat .vortex/tests/phpunit/Functional/FunctionalTestCase.php | head -150Repository: drevops/vortex
Length of output: 4476
🏁 Script executed:
#!/bin/bash
# Find the SutTrait to see how $sutInstallerEnv is handled
find . -name "SutTrait.php" -type fRepository: drevops/vortex
Length of output: 101
🏁 Script executed:
#!/bin/bash
# Search for how environment variables are actually set/used in the installer
rg "prepareSut" --type=php -A10 | head -80Repository: drevops/vortex
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Read SutTrait to understand how $sutInstallerEnv is used
cat ./.vortex/tests/phpunit/Traits/SutTrait.phpRepository: drevops/vortex
Length of output: 29789
🏁 Script executed:
#!/bin/bash
# Check the ProcessTrait to understand how env parameter is handled
find . -name "ProcessTrait.php" -type f | grep -v FixturesRepository: drevops/vortex
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Search for how the installer reads VORTEX_INSTALLER_PROMPT_MIGRATION
find . -path "*/.vortex/installer*" -name "*.php" -type f | head -20Repository: drevops/vortex
Length of output: 1068
🏁 Script executed:
#!/bin/bash
# Search the installer code for how it reads and uses VORTEX_INSTALLER_PROMPT_MIGRATION
rg "VORTEX_INSTALLER_PROMPT_MIGRATION" ./.vortex/installer --type=php -B2 -A2Repository: drevops/vortex
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Look for getenv or environment variable handling in the installer
rg "getenv|env\[" ./.vortex/installer/src --type=php | grep -i prompt | head -20Repository: drevops/vortex
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for how environment variables are typically cast or used in InstallCommand
head -n 250 ./.vortex/installer/src/Command/InstallCommand.php | tail -n 100Repository: drevops/vortex
Length of output: 3633
🏁 Script executed:
#!/bin/bash
# Find the Env class to understand how getenv works
find ./.vortex/installer -name "*.php" -type f | xargs grep -l "class Env"Repository: drevops/vortex
Length of output: 744
🏁 Script executed:
#!/bin/bash
# Search for other VORTEX_INSTALLER_PROMPT usage in installer code
rg "Env::get.*VORTEX_INSTALLER_PROMPT" ./.vortex/installer --type=php -B2 -A2Repository: drevops/vortex
Length of output: 579
🏁 Script executed:
#!/bin/bash
# Read the Env class to understand how it handles environment variables
cat ./.vortex/installer/src/Utils/Env.phpRepository: drevops/vortex
Length of output: 6873
🏁 Script executed:
#!/bin/bash
# Check the full context around line 267 including the other PROMPT variables
sed -n '260,275p' .vortex/tests/phpunit/Functional/AhoyWorkflowTest.phpRepository: drevops/vortex
Length of output: 572
🏁 Script executed:
#!/bin/bash
# Look for any other VORTEX_INSTALLER_PROMPT settings in the test file
rg "VORTEX_INSTALLER_PROMPT" .vortex/tests/phpunit/Functional/AhoyWorkflowTest.php -nRepository: drevops/vortex
Length of output: 629
Change VORTEX_INSTALLER_PROMPT_MIGRATION from TRUE to '1' for consistency.
Line 267 uses PHP boolean TRUE, but all other VORTEX_INSTALLER_PROMPT_* variables in this test file use string values (lines 136, 137, 210, 211, 236, 237). Additionally, VORTEX_INSTALLER_IS_DEMO consistently uses string '1' throughout (lines 28, 75, 114). While PHP will convert TRUE to "1" when passed as an environment variable, maintaining consistent types improves code readability and prevents potential type-checking issues.
🤖 Prompt for AI Agents
In @.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php around lines 265 -
269, Change the value for VORTEX_INSTALLER_PROMPT_MIGRATION in the
static::$sutInstallerEnv array from the PHP boolean TRUE to the string '1' to
match the other VORTEX_INSTALLER_PROMPT_* and VORTEX_INSTALLER_IS_DEMO entries;
update the array entry where static::$sutInstallerEnv is defined so the key
'VORTEX_INSTALLER_PROMPT_MIGRATION' has the string '1' instead of TRUE.
| protected function subtestAhoyMigrationDownloadDb(): void { | ||
| $this->logStepStart(); | ||
|
|
||
| $this->fileAddVar('.env', 'VORTEX_DOWNLOAD_DB2_URL', static::VORTEX_INSTALLER_DEMO_DB_TEST); | ||
| $this->cmd('ahoy download-db2', txt: 'Download migration database'); | ||
| $this->assertFileExists('.data/db2.sql', 'Migration database file should exist after download'); | ||
|
|
||
| $this->logStepFinish(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore .env after subtestAhoyMigrationDownloadDb to avoid test bleed.
The test appends VORTEX_DOWNLOAD_DB2_URL but never restores .env, which can leak state into later tests.
🔧 Proposed fix
$this->fileAddVar('.env', 'VORTEX_DOWNLOAD_DB2_URL', static::VORTEX_INSTALLER_DEMO_DB_TEST);
$this->cmd('ahoy download-db2', txt: 'Download migration database');
$this->assertFileExists('.data/db2.sql', 'Migration database file should exist after download');
+
+ $this->fileRestore('.env');🤖 Prompt for AI Agents
In @.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php around lines 793
- 801, The test subtestAhoyMigrationDownloadDb modifies .env by adding
VORTEX_DOWNLOAD_DB2_URL but never restores it; to prevent test bleed, save the
original .env contents (or the original value of VORTEX_DOWNLOAD_DB2_URL) before
calling fileAddVar, then ensure .env is restored in a finally/tearDown block (or
call a complementary fileRemoveVar/reset method) after the download/assertion
completes; update subtestAhoyMigrationDownloadDb to capture the prior state,
perform the existing actions, and always restore the original .env value so
subsequent tests run with the original environment.
| #;< MIGRATION | ||
| # Local migration database host (not used in production). | ||
| DATABASE2_HOST: database2 | ||
| # Local migration database name (not used in production). | ||
| DATABASE2_NAME: drupal | ||
| # Local migration database user (not used in production). | ||
| DATABASE2_USERNAME: drupal | ||
| # Local migration database password (not used in production). | ||
| DATABASE2_PASSWORD: drupal | ||
| # Local migration database port (not used in production). | ||
| DATABASE2_PORT: 3306 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wire DATABASE2_* to env substitutions.
These values are hardcoded, so .env overrides won’t apply and the new configuration options won’t work as intended.
🔧 Proposed fix
- DATABASE2_HOST: database2
- DATABASE2_NAME: drupal
- DATABASE2_USERNAME: drupal
- DATABASE2_PASSWORD: drupal
- DATABASE2_PORT: 3306
+ DATABASE2_HOST: ${DATABASE2_HOST:-database2}
+ DATABASE2_NAME: ${DATABASE2_NAME:-drupal}
+ DATABASE2_USERNAME: ${DATABASE2_USERNAME:-drupal}
+ DATABASE2_PASSWORD: ${DATABASE2_PASSWORD:-drupal}
+ DATABASE2_PORT: ${DATABASE2_PORT:-3306}🤖 Prompt for AI Agents
In `@docker-compose.yml` around lines 69 - 79, The DATABASE2_* entries
(DATABASE2_HOST, DATABASE2_NAME, DATABASE2_USERNAME, DATABASE2_PASSWORD,
DATABASE2_PORT) are hardcoded so .env overrides won't work; update each value to
use environment variable substitution with sensible defaults (e.g. change
DATABASE2_HOST: database2 to DATABASE2_HOST: "${DATABASE2_HOST:-database2}" and
do the same pattern for DATABASE2_NAME, DATABASE2_USERNAME, DATABASE2_PASSWORD,
DATABASE2_PORT) so the compose file reads from .env while preserving existing
defaults; also ensure the corresponding keys exist in the project .env for users
to override.
| environment="$(drush php:eval "print \Drupal\core\Site\Settings::get('environment');")" | ||
| note "Environment: ${environment}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
head -70 scripts/custom/provision-20-migration.sh | tail -20Repository: drevops/vortex
Length of output: 771
🏁 Script executed:
# Search for namespace usage patterns in Drupal codebase or docs
rg "Drupal\\\\(core|Core)" --max-count=5 -t sh -t phpRepository: drevops/vortex
Length of output: 1425
🌐 Web query:
Drupal PSR-4 autoloading namespace casing requirements Core Site Settings
💡 Result:
Drupal (core + contrib/custom) follows PSR-4, which has strict casing requirements:
- Class names must be referenced case-sensitively. (So your
use/ FQCN casing must match.) [1] - Each sub-namespace maps to a subdirectory, and the directory name MUST match the sub-namespace’s case. [1]
- The
.phpfilename MUST match the class name’s case. [1]
What this means in Drupal
- A module class like
Drupal\my_module\Foo\Barmust live atmy_module/src/Foo/Bar.php(note:srcis not part of the namespace), andFoo/Bar.phpcasing must match exactly. Drupal’s OO conventions document this PSR-4 mapping. [2]
“Core Site Settings” example (Drupal core)
Drupal core’s Settings class is a canonical example of correct casing:
- Namespace:
Drupal\Core\Site - Class:
Settings - File:
core/lib/Drupal/Core/Site/Settings.php[3]
If any segment were cased differently (e.g., core/lib/Drupal/core/Site/settings.php), a PSR-4 autoloader can fail—especially on case-sensitive filesystems (common in Linux production).
Sources: PSR-4 spec [1], Drupal OO/PSR-4 conventions [2], Drupal core Settings API page showing namespace + file path [3].
[1] https://www.php-fig.org/psr/psr-4/
[2] https://docs.w3cub.com/drupal~8/core-core.api.php/group/oo_conventions/8.1.x
[3] https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Site%21Settings.php/11.x
Fix Drupal Settings class casing to avoid autoload failure.
Drupal\core\Site\Settings should be Drupal\Core\Site\Settings; on case-sensitive filesystems this will fail to autoload and break environment detection.
🔧 Proposed fix
-environment="$(drush php:eval "print \Drupal\core\Site\Settings::get('environment');")"
+environment="$(drush php:eval "print \Drupal\Core\Site\Settings::get('environment');")"🤖 Prompt for AI Agents
In `@scripts/custom/provision-20-migration.sh` around lines 58 - 59, The Settings
class reference is incorrectly cased in the drush php:eval call—change
"Drupal\core\Site\Settings" to "Drupal\Core\Site\Settings" in the environment
assignment (the line setting environment="$(drush php:eval "print
\Drupal\core\Site\Settings::get('environment');")") so the autoloader can find
the class on case-sensitive filesystems; update that occurrence to use the
correct namespace casing.
3b75ac7 to
ca0fefa
Compare
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In @.vortex/docs/content/drupal/migrations.mdx:
- Line 43: The docs show DATABASE2_PORT defaulting to 3306 but the
settings.migration.php example sets DATABASE2_PORT to an empty string; make them
consistent by updating one side: either change the table row in migrations.mdx
for `DATABASE2_PORT` to reflect an empty string default, or update the
`settings.migration.php` example to set DATABASE2_PORT to `3306` (adjust the
example variable assignment and any surrounding comments). Ensure the symbol
DATABASE2_PORT is updated in both the table and the settings example so both
sources match.
| | `DATABASE2_NAME` | `drupal` | Migration database name | | ||
| | `DATABASE2_USERNAME` | `drupal` | Migration database user | | ||
| | `DATABASE2_PASSWORD` | `drupal` | Migration database password | | ||
| | `DATABASE2_PORT` | `3306` | Migration database port | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent default for DATABASE2_PORT.
The documentation table shows DATABASE2_PORT defaults to 3306, but the settings.migration.php example on line 77 defaults to an empty string (''). These should be consistent to avoid confusion.
📝 Proposed fix
Either update the table to match the code:
-| `DATABASE2_PORT` | `3306` | Migration database port |
+| `DATABASE2_PORT` | _(empty)_ | Migration database port |Or update the settings.migration.php example to use 3306:
- 'port' => getenv('DATABASE2_PORT') ?: '',
+ 'port' => getenv('DATABASE2_PORT') ?: '3306',📝 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.
| | `DATABASE2_PORT` | `3306` | Migration database port | | |
| | `DATABASE2_PORT` | _(empty)_ | Migration database port | |
🤖 Prompt for AI Agents
In @.vortex/docs/content/drupal/migrations.mdx at line 43, The docs show
DATABASE2_PORT defaulting to 3306 but the settings.migration.php example sets
DATABASE2_PORT to an empty string; make them consistent by updating one side:
either change the table row in migrations.mdx for `DATABASE2_PORT` to reflect an
empty string default, or update the `settings.migration.php` example to set
DATABASE2_PORT to `3306` (adjust the example variable assignment and any
surrounding comments). Ensure the symbol DATABASE2_PORT is updated in both the
table and the settings example so both sources match.
ca0fefa to
cea8f46
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build-test-deploy.yml:
- Around line 142-145: Set the VORTEX_DOWNLOAD_DB2_FILE env var in the workflow
step that runs download-db.sh so the migration download writes to db2.sql
instead of the default db.sql; specifically, modify the step that sets
VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 to also export
VORTEX_DOWNLOAD_DB2_FILE=db2.sql so the remapping logic in download-db.sh (which
maps VORTEX_DOWNLOAD_DB2_* → VORTEX_DOWNLOAD_DB_*) doesn't cause the migration
dump to overwrite the primary dump and so the subsequent copy check for
.data/${VORTEX_DOWNLOAD_DB2_FILE:-db2.sql} succeeds.
In `@tests/phpunit/Drupal/DatabaseSettingsTest.php`:
- Around line 186-222: Add a new test case to the data provider in
DatabaseSettingsTest that verifies the DATABASE2_DATABASE fallback when
DATABASE2_NAME is absent: duplicate the existing dataset that uses
DATABASE2_NAME (the array block shown) but replace 'DATABASE2_NAME' with
'DATABASE2_DATABASE' in the input env array and set the expected migrate =>
default => database value to 'migrate_db_via_database_key'; ensure the new entry
follows the same structure and phpcs ignore markers as the other cases so the
test method in DatabaseSettingsTest exercises the fallback.
| #;< MIGRATION | ||
| - name: Download migration DB | ||
| run: VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 ./scripts/vortex/download-db.sh | ||
| #;> MIGRATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Run the command from the review comment to find references
rg -nP "VORTEX_DOWNLOAD_DB2_(FILE|DIR)|VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2"Repository: drevops/vortex
Length of output: 497
🏁 Script executed:
#!/bin/bash
# First, let's look at the actual workflow file around lines 142-145 and 348-351
cat -n .github/workflows/build-test-deploy.yml | sed -n '135,155p'
echo "---"
cat -n .github/workflows/build-test-deploy.yml | sed -n '340,360p'Repository: drevops/vortex
Length of output: 2275
🏁 Script executed:
#!/bin/bash
# Check if the download-db.sh script exists and examine it
find . -name "download-db.sh" -type fRepository: drevops/vortex
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Examine the download-db.sh script
wc -l ./scripts/vortex/download-db.shRepository: drevops/vortex
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Read the download-db.sh script to understand its behavior
cat -n ./scripts/vortex/download-db.shRepository: drevops/vortex
Length of output: 4519
🏁 Script executed:
#!/bin/bash
# Look for how VORTEX_VAR_PREFIX is used in the codebase
rg -nA5 "VORTEX_VAR_PREFIX" ./scriptsRepository: drevops/vortex
Length of output: 4298
Set VORTEX_DOWNLOAD_DB2_FILE=db2.sql to prevent migration download from overwriting the primary database dump.
When VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 is used, the script remaps any VORTEX_DOWNLOAD_DB2_* environment variables to VORTEX_DOWNLOAD_DB_* (line 15 of download-db.sh). Without an explicit VORTEX_DOWNLOAD_DB2_FILE env var, the script defaults to db.sql (line 32), writing to the same location as the primary dump. The later copy step at lines 348-351 checks for .data/${VORTEX_DOWNLOAD_DB2_FILE:-db2.sql}, which defaults to db2.sql when the variable is unset, causing the file check to fail and the migration DB to not be copied into the container.
🔧 Suggested fix
- name: Download migration DB
run: VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 ./scripts/vortex/download-db.sh
+ env:
+ VORTEX_DOWNLOAD_DB2_FILE: db2.sql
+ VORTEX_DOWNLOAD_DB2_DIR: .dataApplies to lines 142-145 and 348-351.
🤖 Prompt for AI Agents
In @.github/workflows/build-test-deploy.yml around lines 142 - 145, Set the
VORTEX_DOWNLOAD_DB2_FILE env var in the workflow step that runs download-db.sh
so the migration download writes to db2.sql instead of the default db.sql;
specifically, modify the step that sets VORTEX_VAR_PREFIX=VORTEX_DOWNLOAD_DB2 to
also export VORTEX_DOWNLOAD_DB2_FILE=db2.sql so the remapping logic in
download-db.sh (which maps VORTEX_DOWNLOAD_DB2_* → VORTEX_DOWNLOAD_DB_*) doesn't
cause the migration dump to overwrite the primary dump and so the subsequent
copy check for .data/${VORTEX_DOWNLOAD_DB2_FILE:-db2.sql} succeeds.
| // phpcs:ignore #;< MIGRATION | ||
| [ | ||
| [ | ||
| 'DATABASE2_NAME' => 'migrate_db_name', | ||
| 'DATABASE2_USERNAME' => 'migrate_db_user', | ||
| 'DATABASE2_PASSWORD' => 'migrate_db_pass', | ||
| 'DATABASE2_HOST' => 'migrate_db_host', | ||
| 'DATABASE2_PORT' => '3307', | ||
| ], | ||
| [ | ||
| 'default' => [ | ||
| 'default' => [ | ||
| 'database' => 'drupal', | ||
| 'username' => 'drupal', | ||
| 'password' => 'drupal', | ||
| 'host' => 'localhost', | ||
| 'port' => '3306', | ||
| 'charset' => 'utf8mb4', | ||
| 'collation' => 'utf8mb4_general_ci', | ||
| 'driver' => 'mysql', | ||
| 'prefix' => '', | ||
| ], | ||
| ], | ||
| 'migrate' => [ | ||
| 'default' => [ | ||
| 'database' => 'migrate_db_name', | ||
| 'username' => 'migrate_db_user', | ||
| 'password' => 'migrate_db_pass', | ||
| 'host' => 'migrate_db_host', | ||
| 'port' => '3307', | ||
| 'prefix' => '', | ||
| 'driver' => 'mysql', | ||
| ], | ||
| ], | ||
| ], | ||
| ], | ||
| // phpcs:ignore #;> MIGRATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider adding test coverage for the DATABASE2_DATABASE fallback.
This test case correctly validates that DATABASE2_* environment variables populate the migrate connection. However, per the PR objectives, DATABASE2_NAME should fall back to DATABASE2_DATABASE when not set. Consider adding a test case to verify this fallback behavior:
[
[
'DATABASE2_DATABASE' => 'migrate_db_via_database_key',
'DATABASE2_USERNAME' => 'migrate_db_user',
'DATABASE2_PASSWORD' => 'migrate_db_pass',
'DATABASE2_HOST' => 'migrate_db_host',
'DATABASE2_PORT' => '3307',
],
[
'default' => [
'default' => [
// ... default values
],
],
'migrate' => [
'default' => [
'database' => 'migrate_db_via_database_key',
// ... rest of migrate config
],
],
],
],🤖 Prompt for AI Agents
In `@tests/phpunit/Drupal/DatabaseSettingsTest.php` around lines 186 - 222, Add a
new test case to the data provider in DatabaseSettingsTest that verifies the
DATABASE2_DATABASE fallback when DATABASE2_NAME is absent: duplicate the
existing dataset that uses DATABASE2_NAME (the array block shown) but replace
'DATABASE2_NAME' with 'DATABASE2_DATABASE' in the input env array and set the
expected migrate => default => database value to 'migrate_db_via_database_key';
ensure the new entry follows the same structure and phpcs ignore markers as the
other cases so the test method in DatabaseSettingsTest exercises the fallback.
cea8f46 to
f914b78
Compare
This comment has been minimized.
This comment has been minimized.
|
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/vortex/download-db-s3.sh (1)
13-39:⚠️ Potential issue | 🟠 MajorRelocate these changes to
scripts/custom/and keepscripts/vortex/untouched.Repo guidelines forbid modifying scripts under
scripts/vortex/. Please move the prefix/default logic into a custom script (or wrapper) underscripts/custom/, update callers to use it, and restore this file to its original content.🛠️ Suggested fix (restore core script; move customization to scripts/custom)
-_vortex_var_prefix_default="VORTEX_DOWNLOAD_DB" -VORTEX_VAR_PREFIX="${VORTEX_VAR_PREFIX:-${_vortex_var_prefix_default}}" -for v in $(env | grep "^${VORTEX_VAR_PREFIX}_" | cut -d= -f1); do export "${_vortex_var_prefix_default}_${v#"${VORTEX_VAR_PREFIX}"_}=${!v}"; done - # Directory with database dump file. -VORTEX_DOWNLOAD_DB_S3_DB_DIR="${VORTEX_DOWNLOAD_DB_S3_DB_DIR:-${VORTEX_DOWNLOAD_DB_DIR:-${VORTEX_DB_DIR:-./.data}}}" +VORTEX_DOWNLOAD_DB_S3_DB_DIR="${VORTEX_DOWNLOAD_DB_S3_DB_DIR:-./.data}" # Database dump file name. -VORTEX_DOWNLOAD_DB_S3_DB_FILE="${VORTEX_DOWNLOAD_DB_S3_DB_FILE:-${VORTEX_DOWNLOAD_DB_FILE:-${VORTEX_DB_FILE:-db.sql}}}" +VORTEX_DOWNLOAD_DB_S3_DB_FILE="${VORTEX_DOWNLOAD_DB_S3_DB_FILE:-db.sql}"As per coding guidelines, "Never modify
scripts/vortex/- usescripts/custom/for custom scripts".
🤖 Fix all issues with AI agents
In @.vortex/tests/bats/unit/provision-migration.bats:
- Around line 234-264: The test "Provision migration: missing dump file" can be
flaky if prior tests left ./.data/db2.sql behind; at the start of the Bats test
(inside the test block for "Provision migration: missing dump file") explicitly
remove or truncate the dump file (e.g., rm -f ./.data/db2.sql) to guarantee
isolation, or add that cleanup to the shared setup/teardown used by the suite;
update the test block around the existing pushd "${LOCAL_REPO_DIR}" sequence
(the bats test function) to perform this explicit cleanup before running the
rest of the test steps.
| @test "Provision migration: missing dump file" { | ||
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | ||
|
|
||
| rm ./.env && touch ./.env | ||
|
|
||
| # Do NOT create .data/db2.sql. | ||
|
|
||
| create_global_command_wrapper "vendor/bin/drush" | ||
|
|
||
| export MIGRATION_SOURCE_DB_IMPORT=1 | ||
|
|
||
| declare -a STEPS=( | ||
| "@drush -y php:eval print \Drupal\core\Site\Settings::get('environment'); # local" | ||
|
|
||
| "Importing migration source database." | ||
| "Migration source database file not found." | ||
|
|
||
| "- Imported migration source database." | ||
| "- Starting migrations." | ||
| "- Finished migration operations." | ||
| ) | ||
|
|
||
| mocks="$(run_steps "setup")" | ||
|
|
||
| run ./scripts/custom/provision-20-migration.sh | ||
| assert_failure | ||
|
|
||
| run_steps "assert" "${mocks[@]}" | ||
|
|
||
| popd >/dev/null || exit 1 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure test isolation by clearing any prior db2.sql.
Earlier tests create ./.data/db2.sql. If that file persists, this “missing dump file” test can pass/fail incorrectly depending on execution order. Add an explicit cleanup at the start (or in shared setup/teardown).
🧹 Proposed fix
rm ./.env && touch ./.env
# Do NOT create .data/db2.sql.
+ rm -f "./.data/db2.sql"📝 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.
| @test "Provision migration: missing dump file" { | |
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | |
| rm ./.env && touch ./.env | |
| # Do NOT create .data/db2.sql. | |
| create_global_command_wrapper "vendor/bin/drush" | |
| export MIGRATION_SOURCE_DB_IMPORT=1 | |
| declare -a STEPS=( | |
| "@drush -y php:eval print \Drupal\core\Site\Settings::get('environment'); # local" | |
| "Importing migration source database." | |
| "Migration source database file not found." | |
| "- Imported migration source database." | |
| "- Starting migrations." | |
| "- Finished migration operations." | |
| ) | |
| mocks="$(run_steps "setup")" | |
| run ./scripts/custom/provision-20-migration.sh | |
| assert_failure | |
| run_steps "assert" "${mocks[@]}" | |
| popd >/dev/null || exit 1 | |
| } | |
| `@test` "Provision migration: missing dump file" { | |
| pushd "${LOCAL_REPO_DIR}" >/dev/null || exit 1 | |
| rm ./.env && touch ./.env | |
| # Do NOT create .data/db2.sql. | |
| rm -f "./.data/db2.sql" | |
| create_global_command_wrapper "vendor/bin/drush" | |
| export MIGRATION_SOURCE_DB_IMPORT=1 | |
| declare -a STEPS=( | |
| "@drush -y php:eval print \Drupal\core\Site\Settings::get('environment'); # local" | |
| "Importing migration source database." | |
| "Migration source database file not found." | |
| "- Imported migration source database." | |
| "- Starting migrations." | |
| "- Finished migration operations." | |
| ) | |
| mocks="$(run_steps "setup")" | |
| run ./scripts/custom/provision-20-migration.sh | |
| assert_failure | |
| run_steps "assert" "${mocks[@]}" | |
| popd >/dev/null || exit 1 | |
| } |
🤖 Prompt for AI Agents
In @.vortex/tests/bats/unit/provision-migration.bats around lines 234 - 264, The
test "Provision migration: missing dump file" can be flaky if prior tests left
./.data/db2.sql behind; at the start of the Bats test (inside the test block for
"Provision migration: missing dump file") explicitly remove or truncate the dump
file (e.g., rm -f ./.data/db2.sql) to guarantee isolation, or add that cleanup
to the shared setup/teardown used by the suite; update the test block around the
existing pushd "${LOCAL_REPO_DIR}" sequence (the bats test function) to perform
this explicit cleanup before running the rest of the test steps.
|
f914b78 to
360a319
Compare
|
This comment has been minimized.
This comment has been minimized.
|
360a319 to
7fae34e
Compare
This comment has been minimized.
This comment has been minimized.
|
|
7fae34e to
4c8f193
Compare
|
This comment has been minimized.
This comment has been minimized.
|
4c8f193 to
d9498a4
Compare
This comment has been minimized.
This comment has been minimized.
|
|
d9498a4 to
5f975f0
Compare
This comment has been minimized.
This comment has been minimized.
|
|
5f975f0 to
b3255c2
Compare
This comment has been minimized.
This comment has been minimized.
|
|
b3255c2 to
e20dc7a
Compare
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.
55fa555 to
844122c
Compare
|
|
|
Closes #1642
Summary by CodeRabbit
New Features
Documentation
Tests
Chores