Bio node not deleted when user account delete

dldege - September 10, 2007 - 19:27
Project:Bio
Version:5.x-1.x-dev
Component:Code
Category:bug report
Priority:critical
Assigned:Unassigned
Status:active
Description

Currently the node is not deleted and in turn gets assigned to the anonymous user (uid = 0) by the node module handling of user delete. This is causing other problems for me later.

I tried adding support for the hook_user 'delete' op in bio module but my calls to node_delete() were failing and I could not figure out why. I made sure bio was being called before node module (system table weight) and so on. It almost seemed like the db was locked or something.

Anyway, I think this node cleanup should be added.

#1

Rob Loach - September 24, 2007 - 10:34

Could you post the code you had?

#2

dldege - September 24, 2007 - 15:19

I don't have the code any longer but it was pretty much

<?php
function bio_user($op, &$edit, &$user, $category = NULL) {
  if (!
variable_get('bio_profile', 0)) return;

  switch (
$op) {
    case
'view':
      if (!
$nid = bio_for_user($user->uid)) return;
      if (!
node_access('view', $node = node_load($nid))) return;
      return array(
node_get_types('name', variable_get('bio_nodetype', 'bio')) => array('bio' => array('value' => node_view($node, FALSE, TRUE, FALSE))));
    case
'delete' :
      if (!
$nid = bio_for_user($user->uid)) return;
     
node_delete($nid);
      break;
  }
}
?>

The node was not deleted and in turn assigned to the anonymous user which is the action performed by node.module's hook_user().

<?php
function node_user($op, &$edit, &$user) {
  if (
$op == 'delete') {
   
db_query('UPDATE {node} SET uid = 0 WHERE uid = %d', $user->uid);
   
db_query('UPDATE {node_revisions} SET uid = 0 WHERE uid = %d', $user->uid);
  }
}
?>

As I said I made sure bio was being called before node module and so on. Any insight would be greatly appreciated.

#3

jscheel - December 3, 2007 - 16:40

Hmm, I've noticed some problems with deleting nodes when a user is deleted too. Not sure what's up though.

#4

webchick - December 13, 2007 - 10:56
Priority:normal» critical

Bumping to critical. This ought to work.

#5

webchick - December 13, 2007 - 12:18

Hm.

I threw:

    if ($hook == 'user') {
      var_dump($function);
    }

into module_invoke_all() and then deleted a user from the admin panel.

It outputs:

string 'block_user' (length=10)

string 'comment_user' (length=12)

string 'node_user' (length=9)

string 'system_user' (length=11)

string 'user_user' (length=9)

string 'watchdog_user' (length=13)

string 'bio_user' (length=8)

string 'userreference_user' (length=18)

string 'userpoints_user' (length=15)

string 'user_titles_user' (length=16)

string 'project_user' (length=12)

All of these, with the exception of project which is 2, are weighted 0 in the system table.

What the heck kind of whack calling order is that? It's basically like it took the modules as they were enabled or something, and that's why block/user/watchdog/etc. are before bio. Hrm.

#6

prakasht - December 15, 2007 - 09:11

I have also same problem, when I delete from user from administration->user managerment-> users, and bio node type is exists for the selected user then it should deleted the bio node also, but it does not, thats why view still show all bio node but user is doesn't exists.

#7

Rol_ - December 30, 2007 - 00:42

This problem is because the user hook that is invoked after the user's account is deleted. Function bio_for_user, invoked in 'delete' case of bio_user, is unable to find biography node since its uid property has already been reset while deleting account (anonymus account).

I'm not sure if it's perfectly safe but you can modify user module in drupal core to invoke user hook before user's account is deleted

<?php
function user_delete($edit, $uid) { 
 
 
$account = user_load(array('uid' => $uid)); 
 
 
module_invoke_all('user', 'delete', $edit, $account);

 
sess_destroy_uid($uid);
 
db_query('DELETE FROM {users} WHERE uid = %d', $uid);
 
db_query('DELETE FROM {users_roles} WHERE uid = %d', $uid);
 
db_query('DELETE FROM {authmap} WHERE uid = %d', $uid);
 
$array = array('%name' => $account->name, '%email' => '<'. $account->mail .'>');
 
watchdog('user', t('Deleted user: %name %email.', $array), WATCHDOG_NOTICE);
 
drupal_set_message(t('%name has been deleted.', $array));

 
// oryginal
  //module_invoke_all('user', 'delete', $edit, $account);
}
?>

To have this working you should also make a modyfication in bio module (also posted before)

<?php
function bio_user($op, &$edit, &$user, $category = NULL) {
  if (!
variable_get('bio_profile', 0)) return;

  switch (
$op) {
    case
'view':
      if (!
$nid = bio_for_user($user->uid)) return;
      if (!
node_access('view', $node = node_load($nid))) return;
      return array(
node_get_types('name', variable_get('bio_nodetype', 'bio')) => array('bio' => array('value' => node_view($node, FALSE, TRUE, FALSE))));
    case
'delete' :
      if (!
$nid = bio_for_user($user->uid)) return;
     
node_delete($nid);
      break;
  }
}
?>

If you don't want to modify drupal core by moving this line of code, you can also define your own name of hook (for example 'userbefore') and invoke it before user's account is deleted.

#8

webchick - December 31, 2007 - 15:57

I'd prefer to find a solution to this that doesn't require hacking core. So far I've been unsuccessful, though I'm going to try stepping through in a debugger later.

The "easy" solution is simply for bio to create a table in the database that tracks the uid / bio nid pair and then delete the record that way. I was trying to avoid the requirement for a new database table, as I think one of the advantages of bio is that it is so light-weight, but then I also have to balance time invested debugging this vs. getting the problem actually fixed. ;)

#9

jscheel - January 2, 2008 - 15:30

Hmm, the relations table is a good idea, at least for the time being. Maybe it can be phased out in a 6.x branch, not sure if this problem is fixed in 6.x though?

#10

webchick - January 3, 2008 - 08:23
Status:active» patch (code needs review)

Ok, here's a patch that goes the db table route, lightly tested. Please back up your database before testing this. :P

I tried to include code in the update that removed the orphaned bio records, but for some reason it wasn't firing properly. Then I decided that it's probably just as well that a relatively benign operation such as updating a database doesn't try and destroy things on you.

AttachmentSize
bio-delete-user-174507-9.patch4.48 KB

#11

jscheel - January 4, 2008 - 20:28

I haven't had time to test it, but the latest patch needs a hook_uninstall() to drop the new table.

#12

webchick - January 5, 2008 - 22:26

Right you are, sir!

AttachmentSize
bio-delete-user-174507-12.patch5.14 KB

#13

JBrauer - January 12, 2008 - 01:02

I'll have to try it again later. I don't think I did the right sequence with the patch and then needing to update.php.... With that said it seems to work as advertised and it does delete the node when the user is deleted.

It doesn't seem to delete from the bio table when the user and node are deleted. Is there a reason to keep the entry in the bio table?

#14

webchick - January 12, 2008 - 01:04
Status:patch (code needs review)» patch (code needs work)

OMG. Duh. Thanks.

Yeah, I need a hook_nodeapi that acts on op delete.

#15

JBrauer - January 12, 2008 - 18:31
Status:patch (code needs work)» patch (code needs review)

Here's a patch that cleans up the entry in bio table on delete and updates the README.txt so that it no longer says no table is installed.

I haven't looked at it yet but I do notice that when I delete the user the "Deleted user XXXX" message appears twice leading me to wonder if something critical is being called twice.

AttachmentSize
bio-delete-user-080112.patch5.81 KB

#16

webchick - January 13, 2008 - 03:52

Hey, thanks!!

That might be because it's deleting both the user and the node associated with the user, both of which get a drupal_set_message, afaik. We could do some regex magic on $_SESSION['messages'] perhaps... or else copy/paste code from user_delete / node_delete sans drupal_set_message(). Meh.

No time to look at this just now, but I really appreciate it.

#17

JBrauer - January 13, 2008 - 18:53
Status:patch (code needs review)» patch (code needs work)

OK so now that I've thought about it and reread Angie's comment above and the patch I rolled deleted from the bio table on the hook for user delete. This version is probably better in that it deletes based on the bio_nodeapi delete call.

AttachmentSize
bio-delete-user-174507-17.patch5.93 KB

#18

JBrauer - January 13, 2008 - 18:54
Status:patch (code needs work)» patch (code needs review)

Oops... meant to leave it in code needs review.

#19

webchick - January 15, 2008 - 06:04
Status:patch (code needs review)» patch (code needs work)

Hm. This still isn't working, or at least not on my install. The bio table is never populated. "Use bio for user profiles" is checked. I'll try writing some tests tomorrow to see what's going on.

#20

dldege - January 18, 2008 - 21:05

This is re: #5

Its sorted by weight and filename not module name which causes the less then obvious order. That is, with weight 0 core modules (modules/name) will always come before contributed modules that are in sites/something. If you pass TRUE for $sort in module_list I believe it uses module name only for true alpha/weight ordering but module_invoke_all does not do this.

http://api.drupal.org/api/function/module_implements/5
http://api.drupal.org/api/function/module_list/5

#21

dldege - January 18, 2008 - 21:27

Its too bad to need something so complicated for such a small thing. The basic issue is that node modules user delete hook gets called before bios.

node.module

<?php
function node_user($op, &$edit, &$user) {
  if (
$op == 'delete') {
   
db_query('UPDATE {node} SET uid = 0 WHERE uid = %d', $user->uid);
   
db_query('UPDATE {node_revisions} SET uid = 0 WHERE uid = %d', $user->uid);
  }
}
?>

The problem goes away if bio is called first but we've seen that based on the funky way the module/weights are sorted from system table this can't and/or wont happen.

Here was my simple solution for my last project. This could be put into bio.module - problem solved.

<?php
function mysite_cron() {
 
//Clear up dead profiles
 
$result = db_query("SELECT nid, title from {node} WHERE type = '%s' AND uid = 0", variable_get('bio_nodetype','bio'));
  while (
$node = db_fetch_object($result)) {
   
node_delete($node->nid);
  }
}
?>

#22

JBrauer - January 19, 2008 - 03:59

Hmmm... I still can't seem to replicate the condition where the node and bio table entries aren't deleted at user delete.

For testing I set Bio to a weight of 200 and it is still successful in deleting the node.

I setup the module on a new site and noticed a couple of things.

The table wasn't installed because it reported an error in the bio table install SQL - an extra comma at the end of the table definition.

This situation creates a couple of differences. As expected the Bio is never really tied to the user and isn't deleted when the user is deleted.

The other effect of this is a difference in the duplicate delete statements. When properly installed there are two statements with the username saying the username has been deleted. In this case the second line is lacking the username so I get:

* lEZNWeGc has been deleted.
* has been deleted.

Here's the same patch from 17 with the correction to bio.install as well.

AttachmentSize
bio-delete-patch-174507-22.patch5.93 KB

#23

jscheel - January 22, 2008 - 20:09

dldege, that's a good idea to have a cleanup run. Maybe an option of what to do with bio nodes that are orphaned would help, like delete or unpublish? But that still skirts around the main problem :P

#24

webchick - January 28, 2008 - 07:08
Status:patch (code needs work)» patch (code needs review)

Here's a re-roll after the recent bio* clean-up.

AttachmentSize
bio-delete-user-174507-24.patch5.25 KB

#25

webchick - January 28, 2008 - 07:41
Status:patch (code needs review)» fixed

Ok, I committed the most recent patch, as it seems to work. No idea what was up with my test environment before.

Going to take one last spin through the queue and then roll a 1.2 release and cross my fingers. ;)

#26

webchick - January 28, 2008 - 08:01
Status:fixed» patch (code needs work)

Sigh. I spoke too soon. I had usernode in my test site because I was futzing with it, and its delete stuff kicked in. Bio still has the same problem reported above. :(

Steps to reproduce:
- Give authenticated users create bio content permissions.
- As admin, create a user.
- Login as that user, create a bio.
- Logout as the user, login as admin user.
- Go to admin/user/user and delete the user.
- Go to admin/content/node. Deleted user's bio is still there. :( bah.

#27

webchick - January 28, 2008 - 08:31
Status:patch (code needs work)» fixed

Grrr. Found the problem.

<?php
     
if (!variable_get('bio_profile', 0)) return;
?>

that line was at the top of hook_user, which prevented /any/ code (including the delete stuff) from firing unless the "Use bio for user profiles" option was enabled.

So I re-organized that whole mess of code dealing with the profiles and it seems to work now.

AttachmentSize
bio-delete-user-174507-27.patch2.54 KB

#28

Anonymous (not verified) - February 11, 2008 - 08:31
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

#29

The AL - March 4, 2008 - 14:02

So is the 174507-27 patch the latest 'n greatest solution?

p.s. Pardon my noobness, but how exactly do I implement this patch?

#30

The AL - March 4, 2008 - 21:18
Status:closed» active

#31

Rob Loach - March 5, 2008 - 00:29
Version:5.x-1.0» 5.x-1.x-dev
Status:active» fixed

She already committed it ;-)
http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/bio/bio.mod...

#32

Anonymous (not verified) - March 19, 2008 - 00:35
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

#33

ttrinidad - June 11, 2008 - 22:43
Status:closed» active

I hate to bring this back up after so long.

I'm currently having a lot of trouble with this problem, and interestingly enough I'm already using the most updated version.

I create both a user and its bio node programatically, and both are created and display fine. When I delete the user, it calls bio_user with $op = 'delete'.

I inserted a bunch of prints around the case: 'delete' block, and here's what I found.

node_delete is called with the proper NID. I even put a print statement into the node.module file for node_delete. Here's what it looked like:

<?php
function node_delete($nid, $show_message = TRUE) {
  print(
"node_delete: $nid");
 
$node = node_load($nid);
 
print_r($node);
...
?>

It prints "node_delete: 215" fine, which is the correct $nid. It does not, however, print out the $node.

I know it's node a problem with node.module because I can manually delete the bio without any problems. Anyone have any ideas?

#34

ttrinidad - September 19, 2008 - 19:20

anyone?

 
 

Drupal is a registered trademark of Dries Buytaert.