fix(flex): use sql_upgrade.php CLI mode in upgradeOpenEMR#666
Merged
kojiromike merged 1 commit intoopenemr:masterfrom May 1, 2026
Merged
fix(flex): use sql_upgrade.php CLI mode in upgradeOpenEMR#666kojiromike merged 1 commit intoopenemr:masterfrom
kojiromike merged 1 commit intoopenemr:masterfrom
Conversation
Member
Author
|
@Firehed you broke my spacebar |
There was a problem hiding this comment.
Pull request overview
This PR updates the Flex container devtools database-upgrade helper to use OpenEMR’s supported sql_upgrade.php CLI mode instead of mutating the script via sed, addressing a silent failure that could leave dev-reset-install-demodata databases partially upgraded.
Changes:
- Replace
sed-based non-interactive execution ofsql_upgrade.phpwith a direct CLI invocation using--from=VERSION. - Remove temporary file creation/modification/cleanup logic now that native CLI mode is used.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sed -i "1s@^@<?php \$_GET['site'] = 'default'; ?>@" /var/www/localhost/htdocs/openemr/sql_upgrade_temp.php | ||
| php -f /var/www/localhost/htdocs/openemr/sql_upgrade_temp.php | ||
| rm -f /var/www/localhost/htdocs/openemr/sql_upgrade_temp.php | ||
| php -f /var/www/localhost/htdocs/openemr/sql_upgrade.php -- --from="${1}" |
There was a problem hiding this comment.
The previous implementation always forced the upgrade to run against the default site by injecting $_GET['site'] = 'default';. This new CLI invocation no longer sets the site context at all, which can change behavior (or fail) in multisite-enabled containers. Consider passing the site explicitly in CLI mode (or otherwise setting the site to preserve the prior default-site behavior).
The previous implementation rewrote sql_upgrade.php with sed to inject the source version. After openemr/openemr#11906 added native CLI support, the target sed pattern (`$form_old_version = $_POST['form_old_version'];`) no longer exists in sql_upgrade.php — the assignment is now `$form_old_version = $cliFromVersion ?? ($_POST['form_old_version'] ?? '')`. The sed becomes a silent no-op, leaving `$form_old_version = ''`. The loop guard `strcmp($version, '') < 0` is then false for every upgrade file, so sql_upgrade.php starts processing from `2_6_0-to-2_6_1_upgrade.sql` instead of the requested 5.0.0. That file contains an unguarded `INSERT INTO categories_seq VALUES (0)` which adds a second row alongside the demo dump's `(26)`. Several files later, `5_0_0-to-5_0_1_upgrade.sql:407` runs `UPDATE categories_seq SET id = (SELECT MAX(id) FROM categories)` without a WHERE clause; with two rows being set to the same id, the second hits a PRIMARY KEY violation and the upgrade aborts. The database is left at v5.0.0 with only the earliest upgrade scripts partially applied — visible to users as broken login CSS, missing columns (`lists_medication.reporting_source_record_id`), and erroring background services after `openemr-cmd drid`. Switch to the supported `--from=VERSION` CLI argument, which is honored by the loop guard at `sql_upgrade.php:379` (`!empty($_POST['form_submit']) || $cliFromVersion !== null`) and removes the need to mutate the script source. Validated end-to-end: `openemr-cmd drid` now leaves the database at v8.1.1-dev / v_database=538, with `categories_seq` containing the expected single row and `lists_medication.reporting_source_record_id` present. Assisted-by: Claude Code
kojiromike
force-pushed
the
fix-drid-upgrade-cli
branch
from
b936b69 to
151d08a
Compare
Contributor
I regret nothing. 🙃 |
This was referenced May 4, 2026
kojiromike
added a commit
that referenced
this pull request
May 4, 2026
## Summary Adds a `schedule` trigger to `.github/workflows/build-323.yml` so the default `openemr/openemr:flex` tag refreshes nightly at 2 AM UTC, matching the existing pattern in `build-edge.yml`, `build-810.yml`, and `build-811.yml`. Without this, fixes that land on `master` (devops or upstream openemr) can sit in the unpublished `openemr/openemr:flex` image for weeks until someone manually dispatches the workflow. The motivating example: #666 merged on 2026-05-01, but the published `flex` digest is still the 2026-04-07 build, so `openemr-cmd dev-reset-install-demodata` continues to fail in fresh containers. When a future Alpine version takes over `is_default_flex: true`, this trigger should move with it. Closes #686 ## Test plan - [ ] Verify actionlint passes - [ ] After merge, confirm a scheduled run kicks off the next 2 AM UTC and publishes a fresh `openemr/openemr:flex` digest
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After openemr/openemr#11906 added native CLI support to
sql_upgrade.php, the sed-based version-injection inupgradeOpenEMR()silently became a no-op, causingdev-reset-install-demodata(a.k.a.openemr-cmd drid) to leave the database at v5.0.0 with only the earliest upgrade scripts partially applied.Mechanism
demoData()callsupgradeOpenEMR 5.0.0.sed, looking for$form_old_version = $_POST['form_old_version'];.sql_upgrade.php. After fix(sql): Allow sql_upgrade to work on the cli openemr#11906, the assignment is$form_old_version = $cliFromVersion ?? ($_POST['form_old_version'] ?? '').$form_old_versionresolves to''.strcmp(\$version, '') < 0is false for every upgrade file, sosql_upgrade.phpstarts processing from2_6_0-to-2_6_1_upgrade.sqlinstead of from 5.0.0.2_6_0-to-2_6_1_upgrade.sql:40has an unguardedINSERT INTO categories_seq VALUES (0)— adding a second row alongside the demo dump's(26).5_0_0-to-5_0_1_upgrade.sql:407runsUPDATE categories_seq SET id = (SELECT MAX(id) FROM categories)without aWHERE. With two rows being set to the same id, the second hits aDuplicate entryPRIMARY KEY violation and the upgrade aborts.User-visible symptoms after
openemr-cmd dridSQL Statement failed on preparation: ... lists_medication ... reporting_source_record_id ....UUID_ServiceandEmail_Servicebackground services error.Fix
Use the supported
--from=VERSIONCLI argument added in openemr/openemr#11906. The loop guard atsql_upgrade.php:379is!empty(\$_POST['form_submit']) || \$cliFromVersion !== null, so passing--from=also satisfies the entry condition — no need to mutate the script source.Scope
Only
docker/openemr/flex/utilities/devtoolsLibrary.sourceis touched. The 8.0.0 / 8.1.0 / 8.1.1 image variants ship paired with their corresponding openemr release, where the older sed pattern still matches; they are out of scope here.Test plan
bash -non the patched file passes/root/devtoolsLibrary.sourceviadocker cpand re-ranopenemr-cmd dridversion= 8.1.1-dev / v_database=538 / v_acl=13,lists_medication.reporting_source_record_idpresent,categories_seqcontains the expected single row