Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion setup/setuputils.class.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -2150,7 +2150,7 @@ class SetupInfo
/**
* Called by the setup process to initializes the list of selected modules. Do not call this method
* from an 'auto_select' rule
* @param hash $aModules
* @param array $aModules
* @return void
*/
public static function SetSelectedModules($aModules)
Expand Down
7 changes: 5 additions & 2 deletions setup/wizardsteps/WizStepModulesChoice.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class WizStepModulesChoice extends WizardStep
*/
protected bool $bChoicesFromDatabase;

private array $aAnalyzeInstallationModules;
private array $aAnalyzeInstallationModules = [];
private ?MissingDependencyException $oMissingDependencyException = null;

public function __construct(WizardController $oWizard, $sCurrentState)
Expand Down Expand Up @@ -486,7 +486,7 @@ private function GetPhpExpressionEvaluator(): PhpExpressionEvaluator
*
* @return string A text representation of what will be installed
*/
protected function GetSelectedModules($aInfo, $aSelectedChoices, &$aModules, $sParentId = '', $sDisplayChoices = '', &$aSelectedExtensions = null)
public function GetSelectedModules($aInfo, $aSelectedChoices, &$aModules, $sParentId = '', $sDisplayChoices = '', &$aSelectedExtensions = null)
{
if ($sParentId == '') {
// Check once (before recursing) that the hidden modules are selected
Expand Down Expand Up @@ -514,6 +514,9 @@ protected function GetSelectedModules($aInfo, $aSelectedChoices, &$aModules, $sP
(isset($aSelectedChoices[$sChoiceId]) && ($aSelectedChoices[$sChoiceId] == $sChoiceId))) {
$sDisplayChoices .= '<li>'.$aChoice['title'].'</li>';
if (isset($aChoice['modules'])) {
if (count($aChoice['modules']) === 0) {
throw new Exception('Setup option does not have any module associated');
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The error message 'Setup option does not have any module associated' could be more helpful by including which extension or choice caused the error. Consider adding the extension code or choice title to the error message to help users identify the problematic configuration more easily.

Suggested change
throw new Exception('Setup option does not have any module associated');
$sChoiceTitle = isset($aChoice['title']) ? $aChoice['title'] : 'unknown title';
$sExtensionCode = isset($aChoice['extension_code']) ? $aChoice['extension_code'] : 'unknown extension';
throw new Exception(sprintf(
'Setup option "%s" (extension "%s") does not have any module associated',
$sChoiceTitle,
$sExtensionCode
));

Copilot uses AI. Check for mistakes.
}
foreach ($aChoice['modules'] as $sModuleId) {
$bSelected = true;
if (isset($aModuleInfo[$sModuleId])) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -559,4 +559,99 @@ private static function GetStep($index)

return $aSteps[$index] ?? null;
}

public function ProviderGetSelectedModules()
{
return [
'No extension selected' => [
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder : do we have other cases with different conditions and same result? How about having test cases with exclusive options ?

'aSelected' => [],
'aExpectedModules' => [],
'aExpectedExtensions' => [],
],
'One extension selected' => [
Copy link
Contributor

Choose a reason for hiding this comment

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

Independently from the implementation, we should have tests scenarios with multiple extensions selected

'aSelected' => ['_0' => '_0'],
'aExpectedModules' => ['combodo-sample-module' => true],
'aExpectedExtensions' => ['combodo-sample'],
],
];
}

/**
* @dataProvider ProviderGetSelectedModules
*/
public function testGetSelectedModules($aSelectedExtensions, $aExpectedModules, $aExpectedExtensions)
{
$aExtensionsMapData = [
'combodo-sample' => [
'installed' => false,
],
];
$this->oStep->setExtensionMap(iTopExtensionsMapFake::createFromArray($aExtensionsMapData, ));
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

There is a trailing comma after the array parameter. The createFromArray method signature only has one required parameter. This trailing comma appears to be a typo and should be removed.

Copilot uses AI. Check for mistakes.

//GetSelectedModules
$aStepInfo = [
'title' => 'Extensions',
'description' => '',
'banner' => '',
'options' => [
[
'extension_code' => 'combodo-sample',
'title' => 'Sample extension',
'description' => '',
'more_info' => '',
'default' => true,
'modules' => [
'combodo-sample-module',
],
'mandatory' => false,
'source_label' => '',
'uninstallable' => true,
'missing' => false,
],
],
];

$aModules = [];
$aExtensions = [];
$this->oStep->GetSelectedModules($aStepInfo, $aSelectedExtensions, $aModules, '', '', $aExtensions);
$this->assertEquals($aExpectedModules, $aModules);
$this->assertEquals($aExpectedExtensions, $aExtensions);
}

public function testGetSelectedModulesShouldThrowAnExceptionWhenAnySelectedExtensionDoesNotHaveAnyAssociatedModules()
{
$aExtensionsMapData = [
'combodo-sample' => [
'installed' => false,
],
];
$this->oStep->setExtensionMap(iTopExtensionsMapFake::createFromArray($aExtensionsMapData, ));
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

There is a trailing comma after the array parameter. The createFromArray method signature only has one required parameter. This trailing comma appears to be a typo and should be removed.

Copilot uses AI. Check for mistakes.

//GetSelectedModules
$aStepInfo = [
'title' => 'Extensions',
'description' => '',
'banner' => '',
'options' => [
[
'extension_code' => 'combodo-sample',
'title' => 'Sample extension',
'description' => '',
'more_info' => '',
'default' => true,
'modules' => [],
'mandatory' => false,
'source_label' => '',
'uninstallable' => true,
'missing' => false,
],
],
];

$aModules = [];
$aExtensions = [];
$this->expectException('Exception');
$this->oStep->GetSelectedModules($aStepInfo, ['_0' => '_0'], $aModules, '', '', $aExtensions);
}

}