Skip to content
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.

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

is_installed routine fails #1543

Open
Sama34 opened this issue Oct 24, 2014 · 9 comments
Open

is_installed routine fails #1543

Sama34 opened this issue Oct 24, 2014 · 9 comments
Labels
b:1.8 Branch: 1.8.x p:low Priority: Low. Issue to resolve with low preference t:bug Type: Bug. An issue causing error / flaw / malfunction

Comments

@Sama34
Copy link
Contributor

Sama34 commented Oct 24, 2014

Well, not actually. The sorting of active/deactivated plugins only checks if the plugin is activated and shows it under the proper table. It ignores the returned value of its _is_installed() routine.

Original thread: is_installed routine fails

@Sama34 Sama34 added t:bug Type: Bug. An issue causing error / flaw / malfunction s:confirmed Status: Confirmed. Retested and found the issue exists b:1.8 Branch: 1.8.x labels Oct 24, 2014
@DiogoParrinha DiogoParrinha added this to the 1.8.2 milestone Nov 4, 2014
@DiogoParrinha DiogoParrinha modified the milestones: 1.8.4, 1.8.3 Nov 18, 2014
@Sama34 Sama34 added the p:low Priority: Low. Issue to resolve with low preference label Dec 25, 2014
@Sama34
Copy link
Contributor Author

Sama34 commented Dec 25, 2014

@mybb/developers @Destroy666x @JN-Jones

@JN-Jones JN-Jones modified the milestones: 1.8.5, 1.8.6 Apr 30, 2015
@JN-Jones JN-Jones modified the milestones: 1.8.6, 1.8.7 Aug 26, 2015
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.7, 1.8.8 Mar 2, 2016
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.8, 1.8.9 Aug 31, 2016
@JN-Jones
Copy link
Contributor

JN-Jones commented Nov 8, 2016

@mybb/developers As this shouldn't break plugins it's probably best to simply add another if for this. Or do we want to close this as it's been like this for years now?

@euantorano
Copy link
Member

I'm not sure if it's worth fixing given how long it's been like this - some people might be used to the current behaviour.

@JN-Jones
Copy link
Contributor

Anyone else with an opinion?

@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.9, 1.8.10 Dec 15, 2016
@Stefan-MyBB Stefan-MyBB removed this from the 1.8.9 milestone Dec 15, 2016
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.10, 1.8.11 Jan 8, 2017
@WildcardSearch
Copy link
Contributor

I think this is the correct behavior. Whether the plugin is completely uninstalled or just deactivated, it should be in the inactive column.

@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.11, 1.8.12 Apr 1, 2017
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.12, 1.8.13 May 22, 2017
@Stefan-MyBB Stefan-MyBB modified the milestones: 1.8.13, 1.8.14 Nov 6, 2017
@dvz dvz removed this from the 1.8.14 milestone Nov 22, 2017
@effone
Copy link
Member

effone commented May 14, 2018

As we have the control "Install & Activate" the plugin gets activated along with installation. Afterwards if its Deactivated it goes under "Inactive Plugins". is_installed() is required to be checked only if we have a separate section "Installed Plugins" which we don't have currently, so "Inactive Plugins" section itself indicates which one is installed (by showing 2 controls: "Activate" & "Uninstall") and which one is not (by showing single control "Install & Activate").

Now we can either make a separate section for installed but not activated plugins or close the issue as the behaviour is normal, it already indicating which one is installed and which one is not somehow.

@mybb/developers

@effone effone removed the s:confirmed Status: Confirmed. Retested and found the issue exists label May 14, 2018
@Sama34
Copy link
Contributor Author

Sama34 commented May 15, 2018

clipboard01 1

I barely recall exactly what the issue was back then but as per the picture it seems there is indeed a bug.

I think the checks were wrong imo

@effone
Copy link
Member

effone commented May 15, 2018

snap
If the above snap looks desired then there is something to do, otherwise .....
No idea//

@JordanMussi
Copy link
Contributor

So it looks like if a plugin has been activated and is recorded in the plugins cache as activated then it will appear in the active plugins table. However when deciding which controls to display if the plugin has an _is_installed function that returns false:

if(function_exists($installed_func) && $installed_func() != true)
{
$installed = false;
}

then the install and activate control is shown:
if($installed == false)
{
if($compatibility_warning)
{
$table->construct_cell("{$compatibility_warning}", array("class" => "align_center", "colspan" => 2));
}
else
{
$table->construct_cell("<a href=\"index.php?module=config-plugins&amp;action=activate&amp;plugin={$plugininfo['codename']}&amp;my_post_key={$mybb->post_code}\">{$lang->install_and_activate}</a>", array("class" => "align_center", "colspan" => 2));
}
}

Example plugin to recreate the behaviour
<?php

$plugins->add_hook('global_start', 'mytestplug_global');
function mytestplug_global()
{
    die('Plugin is active');
}

function mytestplug_info()
{
	return array(
		'name'			=> 'Test plugin',
		'compatibility'	=> '18*',
		'codename'		=> 'mytestplug'
	);
}
function mytestplug_activate(){}
function mytestplug_deactivate(){}
function mytestplug_install(){}
function mytestplug_is_installed(){}
function mytestplug_uninstall(){}

This is undesirable in my opinion since the plugin is actually active and plugin hooks execute but the administrator has no control to deactivate the plugin.

I think it would be right to ensure that after calling _install the _is_installed function returns true.

// If not installed and there is a custom installation function
if($installed == false && function_exists("{$codename}_install"))
{
call_user_func("{$codename}_install");
$message = $lang->success_plugin_installed;
$install_uninstall = true;
}

My suggestion would be something like:

diff --git a/admin/modules/config/plugins.php b/admin/modules/config/plugins.php
index e11875c23..1efeef889 100644
--- a/admin/modules/config/plugins.php
+++ b/admin/modules/config/plugins.php
@@ -424,6 +424,14 @@ if($mybb->input['action'] == "activate" || $mybb->input['action'] == "deactivate
                if($installed == false && function_exists("{$codename}_install"))
                {
                        call_user_func("{$codename}_install");
+
+                       if($installed_func() != true)
+                       {
+                               call_user_func("{$codename}_uninstall");
+                               flash_message($lang->error_plugin_not_installed_successfully, 'error');
+                               admin_redirect("index.php?module=config-plugins");
+                       }
+
                        $message = $lang->success_plugin_installed;
                        $install_uninstall = true;
                }

And also show a warning and change the controls for plugins that are active but say are not installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x p:low Priority: Low. Issue to resolve with low preference t:bug Type: Bug. An issue causing error / flaw / malfunction
Projects
None yet
Development

No branches or pull requests

9 participants