deactivate_extension not dropping tables

@joe: any pointers on the "official" way to get FA to drop tables created by extensions when they get deactivated for a specific company?

Fixed it! Pull it in from my repo.

There was a missing function deactivate_hooks() which has now been added.

FA 2.3.x is designed for PHP 5 to 5.3 and a few have reported success with PHP 5.4 possibly with some mods.

In case you are testing with PHP 5.6+, please see what functions are deprecated / dropped in 5.4, 5.5 and then 5.6 and make sure you make appropriate changes / replacements.

There is no intention of the devs and myself in supporting any later versions of PHP other than 5.3 for FA v2.3.x. You are pretty much on your own here.

To assist others however, you may post your findings in a separate thread for such later PHP versions. Such findings posted in this thread will only serve to confuse end users - yes, it will make for big consultancy business...... but FA has been designed with simplicity in mind and for self service.

The code portion for activating extensions has not been touched in this fix and if it doesn't work for you now, then it never did in the first place prior to this fix as well.

I must get Janusz to look at this first.

I am out of office right now, but will address him when I am back.

Joe

I removed the indirect calling using hook_invoke method as the global $Hooks array is empty when it reaches inside the $hooks->deactivate_extension() function just as it does when it reaches inside the $hooks->activate_extension() function which was why the latter was used to overcome it. Unless we get to where and when the global $Hooks array gets populated (or re-initialised / overwritten), we will need this workaround.

First off, the current callback method of deactivating the extensions is simply not working as far as the dropping of the extension's tables are concerned - wonder when was the last time it worked, if ever.

Lines 213 to 225 of the current admin/inst_module.php file are:

    foreach($exts as $i => $ext) {
        if ($ext['package'] && ($ext['active'] ^ check_value('Active'.$i))) {
            if (!$ext['active'])
                $activated = activate_hooks($ext['package'], $comp);
            else
                $activated = hook_invoke($ext['package'], check_value('Active'.$i) ?
                 'activate_extension':'deactivate_extension', $comp, false);
            if ($activated !== null)
                $result &= $activated;
            if ($activated || ($activated === null))
                $exts[$i]['active'] = check_value('Active'.$i);
        }
    }

The second line (first if) above has an XOR (^) which means that the code fragment is executed only if either

1. the extension was already active or

2. the activate extension checkbox has just been ticked and submitted

but not both.

The second "if" activates the extension using the direct function activate _hooks() if it was inactive which because of the XOR condition above would mean that the activate extension checkbox had just been ticked. This works correctly and the module gets installed. However, this does not happen through the callback portion of the hook_invoke() function through which it is never called. The use of the direct function activate _hooks() was necessary since the value of the $Hooks global array variable is empty when it enters the activate_extension() method by callback if the hook_invoke() was used.

The "else" of the second "if" triggers the callback only for the deactivate_extension() method when the activate extension checkbox has just been unticked whilst the extension was active. Here, the value of the $Hooks global array variable is empty when it enters the said deactivate_extension() method which exits without executing the deactivation sql if any. It is for this reason that the new direct function deactivate_hooks() was used to do the job.

The esoteric callback method in hook_invoke() seems to make the code quite a bit of a challenge for the average user to comprehend and I was initially blinded to conclude that the said deactivate_extension() method was never called. We should try to use simple and easy to understand constructs for ease of debugging / troubleshooting by the average user for mass trusted adoption.

Steps to reproduce:
1.  Install and activate for one company any extension (currently, only dashboard extension has a drop sql separately) that creates tables and has a means to drop them as well. Alternatively create an extension that only creates one table and then on deactivation drops that table and then install and activate it for one company.
2. Now choose to deactivate it for the company for which it is currently active.
3. Check and find that the newly created table(s) have not been dropped despite deactivation.
4. Use my new function method and then repeat the steps above and you will find successful deactivation with the newly created table(s) dropped.

My tests have been on a plain vanilla FA 2.3.24+ install and the problem exists in it. It is imperative to fix it in the stable version as well based on the commit in my repo. The current activate_hooks is in itself a workaround on which the deactivate_hooks is based and the comment above the former would attest to this in includes/hooks.inc after the class definition:

/*
    Non active hooks are not included in $Hooks array, so we can use special function to 
    activate.
*/

Only, in this case, at this point of entry into the hooks_invoke even the active hooks are not in it! There are a host of functions that are outside the hooks class definition that create a new instance of the hooks class and then invoke the hooks methods!

Install the extension in the default company and activate it for a non default company alone and then try to de-activate it and see if the tables get dropped.

Only the official dashboard extension has the drop sql in it.

Try to only install the dashboard extension (and not the theme) and then activate it and then de-activate it and you will see that the tables remain. At first I thought that it was to save the tables for a re-installation so that user changes are persistent but then most extensions drop the table if it exists before re-creating them during the activation phase and hence it gives us an illusion of having been deleted during deactivation unless the tables are checked just after deactivation.

The usage of XOR is may be standard even if not very widely used, but the usage of the hook_invoke by callback isn't where a simple if...else could have sufficed.

The real solution however, would be to make sure that the global $Hooks array gets pupulated earlier than this invocation and does not get overwritten thereafter. In fact, we have @kvvaradha to thank as this was investigated during the vetting of his hrm extension based on the dashboard skeleton.

It is pertinent to note that solution may lie in the fact that the extension install and uninstall works perfectly in includes/packages.inc wherein the $Hooks array gets populated with the active extensions in:

            if (($ext['active'] == true) && file_exists($path_to_root.'/'.$ext['path'].'/hooks.php'))
            {
                // we need to include the new hooks file to activate extension
                include_once($path_to_root.'/'.$ext['path'].'/hooks.php');
                foreach($db_connections as $comp => $db)
                    activate_hooks($ext['package'], $comp);
            }

Perhaps something like this should precede the hooks_invoke in admin/inst_module.php.

Issue has been fixed in stable repo
Thanks for patient cooperation .

Janusz