Skip to content

N°9141 Log when extension uninstallation is forced #821

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.

Sign up for GitHub

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

Merged
Timmy38 merged 3 commits into from
Mar 5, 2026
Merged

N°9141 Log when extension uninstallation is forced #821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99168bc
N°9141 Log when extension uninstallation is forced
Timmy38 Feb 27, 2026
c3fa3e4
N°9141 Add message when force option checked (whether extensions actu…
Timmy38 Feb 27, 2026
a295ce6
N°9141 Add remote extensions in forced uninstalled extensions
Timmy38 Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions setup/wizardsteps/WizStepInstall.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,34 @@ public function Display(WebPage $oPage)
$oPage->add('<input type="hidden" id="authent_token" value="'.$sAuthentToken.'"/>');
if (!$this->CheckDependencies()) {
$oPage->error($this->sDependencyIssue);
$oPage->add_ready_script(<<<JS
$("#wiz_form").data("installation_status", "error");
document.getElementById("setup_msg").innerText = "Unmet dependencies";
JS);
return;
}

//When the setup reach this step, it already checked whether extensions were uninstallable (during WizStepModulesChoice). We only need to log what has been done.
if ($this->oWizard->GetParameter('force-uninstall', false)) {
SetupLog::Warning("User disabled uninstallation checks");
}
$aExtensionsRemoved = json_decode($this->oWizard->GetParameter('removed_extensions'), true) ?? [];
$aExtensionsNotUninstallable = json_decode($this->oWizard->GetParameter('extensions_not_uninstallable'));
$aExtensionsForceUninstalled = [];
foreach ($aExtensionsRemoved as $sExtensionCode => $sLabel) {
if (in_array($sExtensionCode, $aExtensionsNotUninstallable)) {
$aExtensionsForceUninstalled[] = $sExtensionCode;
}
}
if (count($aExtensionsForceUninstalled)) {
SetupLog::Warning("Extensions uninstalled forcefully : ".implode(',', $aExtensionsForceUninstalled));
Comment on lines +97 to +106
Copy link
Contributor

@odain-cbd odain-cbd Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems this section is not dealing with "uninstallation-forced" flag. it logs as if extensions removal is forced. when it is not, i thought it should failf/stop setup, no?

Copy link
Contributor Author

@Timmy38 Timmy38 Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a non-uninstallable extension is removed, the setup will stop at the extension selection step, the user won't be able to progress further.
If he reaches this step (install), this checks has already been performed.

Copy link
Contributor

@odain-cbd odain-cbd Mar 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why checking option value then? 'if' is not necessary, right?
if ($this->oWizard->GetParameter('force-uninstall', false)) {
SetupLog::Warning("User disabled uninstallation checks");
}

maybe you can add a php comment. to document with below remark. today it is obvious to you that if we reach this piece of code we passed the check. later on some others may not understand.

Copy link
Contributor Author

@Timmy38 Timmy38 Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be useful to know when a user disable uninstallation checks, whether it was actually needed or not (to advise them not to unless needed for example)
Good idea for the php comment, I'm adding it.

}

$oPage->add_ready_script(
<<<JS
$oPage->add_ready_script(<<<JS
$("#wiz_form").data("installation_status", "not started");
ExecuteStep("");
JS
);
JS);

}

/**
Expand Down
2 changes: 1 addition & 1 deletion setup/wizardsteps/WizStepModulesChoice.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public function GetAddedAndRemovedExtensions($aSelectedExtensions)
$bSelected = in_array($oExtension->sCode, $aSelectedExtensions);
if ($oExtension->bInstalled && !$bSelected) {
$aExtensionsRemoved[$oExtension->sCode] = $oExtension->sLabel;
if (!$oExtension->CanBeUninstalled()) {
if (!$oExtension->CanBeUninstalled() || $oExtension->sSource === iTopExtension::SOURCE_REMOTE) {
$aExtensionsNotUninstallable[$oExtension->sCode] = true;
}
} elseif (!$oExtension->bInstalled && $bSelected) {
Expand Down