Bug in module manager

[100rk]

Hi Wishy,

if I was register a simple dependency from one module to another (v. 0.8.2), module manager still allow deactivate BOTH modules INDEPENDENT on other - it seems like a bug (I hope it is not a feature ), try it please....

So, I was change file admin/plugins.php (lines 352 - 360) from

Code: Select all

echo "<td>".($dbm[$key]['Active']==="1"?"<a href='plugins.php?action=setfalse&module=".$key."'>".$image_true."</a>":"<a href='plugins.php?action=settrue&module=".$key."'>".$image_false."</a>")."</td>";
if (!$hasdeps)
{
	echo "<td><a href=\"plugins.php?action=uninstall&module=".$key."\" onclick=\"return confirm('".lang('uninstallconfirm')."');\">".lang('uninstall')."</a></td>";
}
else
{
	echo "<td>".lang('hasdependents')."</td>";
}

to

Code: Select all

if (!$hasdeps)
{
	echo "<td>".($dbm[$key]['Active']==="1"?"<a href='plugins.php?action=setfalse&module=".$key."'>".$image_true."</a>":"<a href='plugins.php?action=settrue&module=".$key."'>".$image_false."</a>")."</td>";
	echo "<td><a href=\"plugins.php?action=uninstall&module=".$key."\" onclick=\"return confirm('".lang('uninstallconfirm')."');\">".lang('uninstall')."</a></td>";
}
else
{
	echo "<td>".($dbm[$key]['Active']==="1"?$image_true:"<a href='plugins.php?action=settrue&module=".$key."'>".$image_false."</a>")."</td>";
	echo "<td>".lang('hasdependents')."</td>";
}

Now is impossible deactivate module with dependent modules.

But there is still another bug: plugin manager allows install module with dependency on deactivated module (see lines 303 - 336), because boolean $havedep shows state of last checked dependency only (by foreach loop), not for all dependencies (if there is more then one).

Therefore I was change file admin/plugins.php (lines 303 - 336) from

Code: Select all

$havedep = false;

if (isset($gCms->modules[$key]['dependency'])) #Check for any deps
{
	#Now check to see if we can satisfy any deps
	foreach ($gCms->modules[$key]['dependency'] as $onedepkey=>$onedepvalue)
	{
		if (isset($gCms->modules[$onedepkey]) && 
			$gCms->modules[$onedepkey]['Installed'] == true &&
			$gCms->modules[$onedepkey]['Active'] == true &&
			version_compare($gCms->modules[$onedepkey]['Version'], $onedepvalue) > -1)
		{
			$havedep = true;
		}
	}
}
else
{
	$havedep = true;
}

echo "<td>".$gCms->modules[$key]['Version']."</td>";
echo "<td>".lang('notinstalled')."</td>";

if ($havedep)
{
	echo "<td> </td>";
	echo "<td><a href=\"plugins.php?action=install&module=".$key."\">".lang('install')."</a></td>";
}
else
{
	echo "<td> </td>";
	echo '<td><a href="plugins.php?action=missingdeps&module='.$key.'">'.lang('missingdependency').'</a></td>';
}

to

Code: Select all

$brokendeps = 0;

if (isset($gCms->modules[$key]['dependency'])) #Check for any deps
{
	#Now check to see if we can satisfy any deps
	foreach ($gCms->modules[$key]['dependency'] as $onedepkey=>$onedepvalue)
	{
		if (isset($gCms->modules[$onedepkey]) && 
			($gCms->modules[$onedepkey]['Installed'] == false ||
			$gCms->modules[$onedepkey]['Active'] == false ||
			version_compare($gCms->modules[$onedepkey]['Version'], $onedepvalue) < 0))
		{
			$brokendeps++;
		}
	}
}

echo "<td>".$gCms->modules[$key]['Version']."</td>";
echo "<td>".lang('notinstalled')."</td>";

if ($brokendeps)
{
	echo "<td> </td>";
	echo '<td><a href="plugins.php?action=missingdeps&module='.$key.'">'.lang('missingdependency').'</a></td>';
}
else
{
	echo "<td> </td>";
	echo "<td><a href=\"plugins.php?action=install&module=".$key."\">".lang('install')."</a></td>";
}

and now it works perfect.

I really miss any complex API documentation (and I hope I am not alone), keep it in Your mind during rewriting module interface to OO, please... )))

Have a nice day!

[Ted]

Wow. You're right. I completely forgot about active and inactive... I've already rewritten this part for the new API, but I'll make sure I add that in before it's released.

As for documentation, the API should self document itself..... and I will have to reupdate the API introduction on the wiki for it.

[100rk]

Wow. You're right. I completely forgot about active and inactive... I've already rewritten this part for the new API, but I'll make sure I add that in before it's released.

As for documentation, the API should self document itself..... and I will have to reupdate the API introduction on the wiki for it.

Hi Wishy,
excuse me for interrupting, but I found the similar bug in admin/plugins.php... List of missing dependencies also didn't work as I expected - all dependencies looks as installed here, but I have not installed all of them...

current lines 210-231:

Code: Select all

		foreach ($gCms->modules[$module]['dependency'] as $key=>$value)
		{
			echo '<tr class="'.$curclass.'"><td>'.$key.'</td><td>'.$value.'</td><td>';

			$havedep = false;

			#Now check to see if we can satisfy any deps
			foreach ($gCms->modules[$module]['dependency'] as $onedepkey=>$onedepvalue)
			{
				if (isset($gCms->modules[$onedepkey]) && 
					$gCms->modules[$onedepkey]['Installed'] == true &&
					$gCms->modules[$onedepkey]['Active'] == true &&
					version_compare($gCms->modules[$onedepkey]['Version'], $onedepvalue) > -1)
				{
					$havedep = true;
				}
			}

			echo lang(($havedep?'true':'false'));
			echo '</td></tr>';
			($curclass=="row1"?$curclass="row2":$curclass="row1");
		}

should be replaced by

Code: Select all

		foreach ($gCms->modules[$module]['dependency'] as $key=>$value)
		{
			echo '<tr class="'.$curclass.'"><td>'.$key.'</td><td>'.$value.'</td><td>';

			$havedep = false;

			#Now check to see if we can satisfy this dep
			if (isset($gCms->modules[$key]) && 
				$gCms->modules[$key]['Installed'] == true &&
				$gCms->modules[$key]['Active'] == true &&
				version_compare($gCms->modules[$key]['Version'], $value) > -1)
			{
				$havedep = true;
			}

			echo lang(($havedep?'true':'false'));
			echo '</td></tr>';
			($curclass=="row1"?$curclass="row2":$curclass="row1");
		}

- second (inner) foreach is not necessary here.

By the way, my previous bugfix is also not correct, because in case 'root' module is not installed, it don't recognize it... Correct solution is (fragment nnly):

Code: Select all

                    #Now check to see if we can satisfy any deps
                    foreach ($gCms->modules[$key]['dependency'] as $onedepkey=>$onedepvalue)
                    {
                    	if (isset($gCms->modules[$onedepkey]) && 
                    		($gCms->modules[$onedepkey]['Installed'] == false ||
                    		$gCms->modules[$onedepkey]['Active'] == false ||
                    		version_compare($gCms->modules[$onedepkey]['Version'], $onedepvalue) < 0))
                    	{
                    		$brokendeps++;
                    	}
                    	elseif (!isset($gCms->modules[$onedepkey])) 
                    	{
                    		$brokendeps++;
                    	}
                    }

Have a nice day!
100rk