N°9571 - Do not allow DB change or install when upgrading iTop

[eespie]

N°9571 - Do not allow DB change or install when upgrading iTop

[greptile-apps]

Greptile Summary

This PR removes the WizStepInstallOrUpgrade wizard step and instead auto-detects install vs. upgrade mode in WizStepWelcome by reading the existing iTop config file. When upgrading, DB credentials are now locked to those found in the live config, preventing accidental DB changes during an upgrade run.

• WizStepInstallOrUpgrade deleted: routing and DB-parameter detection migrated to WizStepWelcome::Display, which calls SetupUtils::GetPreviousInstance() and persists the mode and credentials to session via SetParameter.
• WizStepDetectedInfo extended: a DB connection check (check_db async action + inline JS) is now embedded directly on the upgrade-info page so the user gets immediate feedback on the locked DB connection.
• Minor fixes: null-coalesce for db_tls.ca in backup.class.inc.php, and a CSS class addition (ibo-is-html-content) for setup message divs.

Confidence Score: 3/5

The wizard flow restructuring is sound, but DB credentials from the config file are interpolated into JavaScript strings without escaping, which will break the upgrade page for any installation whose DB password or other credential contains a single quote or backslash.

The routing change in WizStepWelcome is correct — SetParameter writes to the session, so install_mode persists reliably across the two HTTP requests. The deletion of WizStepInstallOrUpgrade is clean and the autoloader and test updates are consistent. The one concrete defect is in WizStepDetectedInfo: PHP DB values from the live config are spliced directly into single-quoted JS string literals with no addslashes/json_encode step, so any special character in the credentials produces a syntax error that silently prevents the DB check from running on the upgrade page.

setup/wizardsteps/WizStepDetectedInfo.php — the new JavaScript block at lines 248–256 needs escaping applied to all interpolated DB parameter variables.

Important Files Changed

Filename	Overview
setup/wizardsteps/WizStepDetectedInfo.php	New DB connection check JS block added; DB parameters are interpolated into JavaScript strings without escaping, which can break the page for any config with special characters in DB credentials.
setup/wizardsteps/WizStepWelcome.php	Now auto-detects install vs upgrade mode by reading the existing config in Display(); uses SetParameter (session-backed) so the decision persists correctly into UpdateWizardStateAndGetNextStep.
setup/wizardsteps/WizStepInstallOrUpgrade.php	Entire class deleted; its routing and DB-detection logic migrated to WizStepWelcome and WizStepDetectedInfo.
setup/wizardsteps/WizStepModulesChoice.php	WizStepInstallOrUpgrade removed from upgrade step list to match the new wizard flow.
setup/backup.class.inc.php	Added null-coalesce fallback for db_tls.ca config value to prevent passing null to CMDBSource.
tests/php-unit-tests/unitary-tests/setup/WizStepModulesChoiceTest.php	Test expectations updated to drop WizStepInstallOrUpgrade from the upgrade step list.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[WizStepWelcome::Display] --> B{GetPreviousInstance found?}
    B -- Yes --> C[SetParameter: install_mode=upgrade\nDB server/user/pwd/name/prefix/tls from config]
    B -- No --> D[SetParameter: install_mode=install\nsource_dir / datamodel_version]
    C --> E[User clicks Continue]
    D --> E
    E --> F[WizStepWelcome::UpdateWizardStateAndGetNextStep]
    F --> G{install_mode == install?}
    G -- Yes --> H[WizStepLicense]
    G -- No --> I[WizStepDetectedInfo]
    I --> J[Display upgrade info\nEmbed DB params in JS\nDoCheckDBConnection on load]
    J --> K{check_db async action}
    K --> L[SetupUtils::AsyncCheckDB\nshows result in #db_info]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[WizStepWelcome::Display] --> B{GetPreviousInstance found?}
    B -- Yes --> C[SetParameter: install_mode=upgrade\nDB server/user/pwd/name/prefix/tls from config]
    B -- No --> D[SetParameter: install_mode=install\nsource_dir / datamodel_version]
    C --> E[User clicks Continue]
    D --> E
    E --> F[WizStepWelcome::UpdateWizardStateAndGetNextStep]
    F --> G{install_mode == install?}
    G -- Yes --> H[WizStepLicense]
    G -- No --> I[WizStepDetectedInfo]
    I --> J[Display upgrade info\nEmbed DB params in JS\nDoCheckDBConnection on load]
    J --> K{check_db async action}
    K --> L[SetupUtils::AsyncCheckDB\nshows result in #db_info]

Loading

_{Reviews (1): Last reviewed commit: "N°9711 - Removed step Install or Upgrade" | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR reworks the iTop setup wizard so that the install-vs-upgrade decision is made automatically based on whether an existing instance is detected, rather than presenting the user with an editable "Install or Upgrade" screen. This directly supports the goal of preventing accidental DB changes or a fresh install while upgrading: the entire WizStepInstallOrUpgrade step (which exposed editable DB parameters and an install/upgrade radio choice) is removed. WizStepWelcome now detects the previous instance, pre-populates install mode and DB parameters, and routes to either the install path (WizStepLicense) or upgrade path (WizStepDetectedInfo), which now performs a read-only DB connection check.

Changes:

• Removed WizStepInstallOrUpgrade and all its references (autoload, flow doc, module-choice step list, tests).
• Moved previous-instance detection and install/upgrade + DB parameter initialization into WizStepWelcome::Display(), with branching in UpdateWizardStateAndGetNextStep().
• Added a non-editable DB connection validation (JS + AsyncAction) in WizStepDetectedInfo, and normalized null db_tls.ca to an empty string in the welcome, backup, and unattended-install paths.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
setup/wizardsteps/WizStepInstallOrUpgrade.php	Deleted; the install/upgrade choice screen is removed.
setup/wizardsteps/WizStepWelcome.php	Detects previous instance, initializes install/upgrade mode + DB params, routes to install/upgrade branch.
setup/wizardsteps/WizStepDetectedInfo.php	Adds read-only DB connection check JS and check_db async action for the upgrade path.
setup/wizardsteps/WizStepModulesChoice.php	Removes WizStepInstallOrUpgrade from the upgrade step sequence.
setup/wizardsteps/WizardStep.php	Updates the documented wizard flow diagram.
setup/wizardsteps_autoload.php	Removes the require_once for the deleted step.
setup/setuputils.class.inc.php	Adds ibo-is-html-content class to the DB-check error message.
setup/backup.class.inc.php	Normalizes null db_tls.ca to '' before DB connect.
setup/unattended-install/unattended-install.php	Cosmetic quote normalization of the db_tls.ca fallback.
tests/php-unit-tests/unitary-tests/setup/WizStepModulesChoiceTest.php	Updates expected step lists to drop WizStepInstallOrUpgrade.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.