Delete messages and settings when a user is deleted.
sja1 - August 25, 2009 - 17:38
| Project: | Privatemsg |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed |
Description
This is a minor point, but looking through the code it appears that the pm_email_notify table only ever gets updated or inserted. When users are deleted there is no code to delete their corresponding entry in the pm_email_notify table. One solution might be to add a hook user (pm_email_notify_user) function to perform cleanup on $op = 'delete'.
#1
That's not a bug. its a feature that was not considered oritinally. Good idea. Your approach is good (hook_user $op='delete'). Uplod a patch of work in progress so others can help out.
#2
Here's a swipe at it...
#3
Looks good, simple change.
#4
Patch looks good but i am setting this back to active for the sakes of the following discussion (Deppends on what we decide i will change this back to RTBC or needs work):
Similar issue to how user deletions are now handled in 7 core, we should consider having similar integration in privatemsg. What exactly do we clean up when user is deleted/disabled/etc.. Do we delete all of the user settings. Do we delete all of the user messages. Do we archive them somehow. Do we add some kind of marker that user is deleted even though he appears in the "from/to" fields. that leads into next problem of if user is deleted then we cant do join on the user table to fetch his user name to appear in the 'from/to' field.
Much to discuss, what do you guys think?
#5
Well, Drupal 7 contains a proper facility to cancel accounts, and it also allows to configure what to do with the contents of that user so that we can properly hook into that.
Not sure what to do for Drupal 6...
#6
My gut feeling want me to say: Lets have a new admin UI element to allow site admin to chose behavior for user deletions 1) keep the messages/settings/etc.. 2) delete the messages/settings/etc.
However on real usecase side of things it leads me to believe that it makes absolutely no sense to keep messages/settings/etc... in database after user was deleted because we will never be able to fetch necessary information such as user name or make joints on the users table. So it makes me think that we should also perform complete cleanup when user is deleted without giving site administrator an option to cancel this behavior.
#7
I think it makes more sense to perform cleanup, and leave configurable deletion handling options for a D7 version of the module. Certainly there is no value in retaining user settings anyway for deleted users, the settings provide no useful historical information.
Thinking about this, I am now wondering about a different problem that I'll open in a different issue.
#8
Seems we've reached a consensus. Lets make a patch that encompases all aspects of pmsg user content/settings cleanup (not just email notification settings).
#9
I'm not very familiar with the rest of the module. What else needs to get cleaned up?
#10
Have a quick scan through the module and it's feature-set...
I think we also need to clean up the actual messages as well. There could be more.
#11
I'm in patch writing mood today :)
#12
Updates:
- Added the test from #615834: Error if adressed to someone who got deleted since the patch changed and doesn't use them anymore.
- It does now also delete recipient entries that belong to messages written by that user.
#13
This has been commited to 6.x-1.x-dev.
#14
Updated the patch to D7. This does now use the user_cancel stuff.
It also contains a test when the user is actually deleted.
#15
#16
Integrated the bug fix including a test case from #626692: Deleting user causes mysql error.
#17
Commited against 7.x-1.x-dev.
#18
Automatically closed -- issue fixed for 2 weeks with no activity.