feat(install): ask about usage reports in the install scripts - #324
Merged
Merged
Conversation
Reporting became opt-in in 0.24.3 and the web setup wizard asks about it on its
admin step. A Docker install never reaches that wizard: docker-entrypoint.sh
creates the admin account and writes storage/installed, which is explicitly
there to skip it. So nobody who installed with install.sh or install.ps1 was
ever asked — the question existed, on a screen that population never sees.
The prompt defaults to no, and an unattended run counts as no rather than as
consent, which is the same rule the application applies to a setting nobody
ever set.
Writing OPENMES_TELEMETRY to .env is not on its own enough, and that is the
part worth reading carefully. TelemetrySettings::enabled() treats the env var
as a veto — set and falsy forces reporting off — but what turns it ON is the
stored setting, and an absent row means off. So three pieces had to line up:
- install.sh / install.ps1 ask and write OPENMES_TELEMETRY to .env;
- docker-compose.yml passes it into the backend container, which is where the
scheduler that sends reports actually runs (entrypoint, not a sidecar) — it
was not in that environment list, so the variable would have gone nowhere;
- the entrypoint records the choice in system_settings, guarded on the
application not being marked installed yet, so it cannot overwrite a choice
an administrator later makes in Settings → System.
Verified each link rather than assuming it: the prompt answers no to silence,
"", and "n" and yes only to y/yes; `docker compose config` shows
OPENMES_TELEMETRY reaching the backend service; and the entrypoint's snippet
writes true and false into system_settings.telemetry_enabled against a real
database.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: Mes-Open/OpenMes/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
The gap
Reporting became opt-in in 0.24.3, and the web setup wizard asks about it on its admin step.
A Docker install never reaches that wizard.
docker-entrypoint.shcreates the admin account and writesstorage/installed— a block whose own comment says "skip web installer". So nobody who installed withinstall.shorinstall.ps1has ever been asked. The question existed, on a screen that population does not see.The part worth reading carefully
Writing
OPENMES_TELEMETRYto.envis not on its own enough, and assuming it was would have produced a change that looked right and did nothing.TelemetrySettings::enabled()treats the environment variable as a veto: set and falsy forces reporting off. What turns it on is the stored setting, and an absent row means off — "silence is not consent", as the code puts it.So three pieces had to line up:
install.sh/install.ps1ask, and writeOPENMES_TELEMETRYto.env.docker-compose.ymlpasses it into thebackendcontainer — which is where the scheduler that sends reports actually runs (started by the entrypoint on the primary, not in a sidecar). It was not in thatenvironment:list, so without this the variable would have gone nowhere.docker-entrypoint.shrecords the choice insystem_settings, guarded on the application not being marked installed yet, so it can never overwrite a choice an administrator later makes in Settings → System.Default is no
The prompt is
[y/N], and an unattended run counts as a refusal rather than as consent — the same rule the application applies to a setting nobody ever set. Only an explicity/yesopts in.The prompt also states plainly what is and is not sent, so the answer is an informed one:
Verified link by link
"",n, and a non-interactive runfalsey,yes,Ytruedocker compose configOPENMES_TELEMETRY: "true"on the backend servicetrue/falseintosystem_settings.telemetry_enabledFull suite: 2951 PHP tests, 173 JavaScript tests.
Scope
Additions only — 103 lines across the two install scripts, the compose file, the entrypoint and the changelog. No application code, so an existing installation is unaffected until it is reinstalled.