e107inc / e107

e107 Bootstrap CMS (Content Management System) v2 with PHP, MySQL, HTML5, jQuery and Twitter Bootstrap. Issue Discussion Room: https://gitter.im/e107inc/e107
https://e107.org
GNU General Public License v3.0
320 stars 214 forks source link

Missing event(s)? #3969

Closed sbwMikael closed 4 years ago

sbwMikael commented 4 years ago

Hello,

I'm building a plugin around the admin "admin_user_delete" event mentioned on this page - towards the bottom.

But a grep of the e107 distribution isn't locating the "admin_user_delete" string, and I can't seem to find it in the repository.

Has it now been implemented yet? Thank you.

Moc commented 4 years ago

Seems like it has not been implemented yet. Will need to dive deeper into the code to find the location where to add it though... will get back to this.

sbwMikael commented 4 years ago

Thanks Moc, sorry for the headache.

My use-case is that I want to delete some files (uploaded by the user) from disk that have their paths stored in the database. If a user account is deleted, I want to delete all associated files too.

Moc commented 4 years ago

I pushed something, untested, that may do the trick. Please test and let me know :)

sbwMikael commented 4 years ago

Crikey, that was quick! Thanks Moc.

I tested it just now with a little trial plugin, and it didn't fire - which is generally my fault.

But re-reading the patch, I note the new event is named: "admin_user_deleted" while the event page says "admin_user_delete" (that final extra 'd' in case a quick parse tricks the eye!).

Separately (and this is my problem rather than yours) I actually need to get notification of the account deletion before the e107_user row is removed because my other table (containing the file paths) has an ON DELETE CASCADE on a foreign key to the e107_user table.

So I won't be able to do a SELECT statement on my file paths table if get notified after the user delete has already happened (since the paths will have been deleted by then too). I suppose either I need to remove the DELETE CASCADE, or... is there any chance of an "admin_before_user_delete" event? I know that would mess up the documentation too. But just throwing it out there.

I'll be away from my computer for most of the day now, so apologies if there are no follow-up replies for some time.

Moc commented 4 years ago

But re-reading the patch, I note the new event is named: "admin_user_deleted" while the event page says "admin_user_delete" (that final extra 'd' in case a quick parse tricks the eye!).

Ouch, that is me being a little too quick. I'll correct that. It should be admin_user_delete as the documentation states.

Separately (and this is my problem rather than yours) I actually need to get notification of the account deletion before the e107_user row is removed

That would indeed require a separate event such as admin_before_user_delete @CaMer0n What are your thoughts on this?

@sbwMikael Your short term solution indeed is to remove the DELETE CASCADE and put that code in with the file removal code.

sbwMikael commented 4 years ago

Thank you, can confirm that the test plugin receives the "admin_user_delete" event successfully now.

And will indeed remove the DELETE CASCADE and perform the related table update manually.

Moc commented 4 years ago

I think there is this method, which would allow to do what you want, but I'm not sure if we really want to add those 'before' events as well. Let's see what Cameron thinks first.

    public function beforeDelete($data, $id)
    {
        // 
    }
sbwMikael commented 4 years ago

Thanks Moc, but no need to add this for my sake - though I certainly appreciate the gesture! I'm 90% of the way through modifing my configuration to deal with a post-delete event, so it's not a big deal.

Moc commented 4 years ago

Closing this issue for now. You can use the beforeDelete() method in the Admin UI if needed.