The function ldap_authorization_og_uninstall() is designed to remove the traces of the module from user accounts.
/**
* Implements hook_uninstall().
*/
function ldap_authorization_og_uninstall() {
module_load_include('inc', 'ldap_servers', 'ldap_servers.user_data_remove');
batch_set(ldap_severs_user_data_setup_batch('og_group'));
$batch =& batch_get();
}
function ldap_servers_empty_user_data($start, $step, $consumer_type, &$context) {
[..]
unset($account->data['ldap_authorizations'][$consumer_type]);
However, after running drush pmu ldap_authorization_og, the entries on user accounts are still there.
E.g. on user/123/devel I see $user->data['ldap_authorizations']['og_group'] === array().
Yes it is an empty array, but the code in ldap_servers_empty_user_data() suggests that the ['og_group'] key should be completely gone after the uninstall.
Also the uninstall happened suspiciously fast. Looping through all user accounts cannot be that fast!
Maybe the idea is that cron will execute the batch process?
I currently don't know enough how Drupal batch works to tell if this would be the case or not.
If it is NOT the case then this uninstall hook is broken.
If it IS the case, then this would mean that a user cannot safely remove the module after uninstall, because the batch might not be finished yet.
Comments
Comment #2
donquixote commentedAnd either way a comment should be added to explain what this is meant to do.
Comment #3
donquixote commentedA look at the other code suggests that the leftover data does not really hurt.
If the "consumer type" no longer exists then this data will have no effect.
Only when re-enabling the module, the old data will still be there.
(Of course a leftover batch process _would_ be a problem, but currently I think this is not how batch works right?)
Comment #4
grahlThis functionality is likely broken but as you indicate it doesn't really hurt but ideally the module should clean up after itself.
This might be caused by how drush runs differently due to batch's behavior in the browser. Try replacing $batch =& batch_get(); with just batch_process(). This might trigger the required action in the background correctly.
Patches welcome!
Comment #5
grahlComment #6
grahlClosing issue as outdated due to no further development on 7.x, if you feel this issue is still relevant and you are willing to work on a patch and/or debug the problem, please reopen.