@alikon is this related to #18160?

I assume super users should not be allowed to have their account deleted?

If we require double confirmation to create an account (confirming the email) then we should do the same for deleting an account

@Quy can you please e-mail me at franz.wohlkoenig@community.joomla.org?

edit
Retested Full Success #19023 (comment)
I have tested this item ✅ successfully on 50ca4e4
edit

I have tested this item 🔴 unsuccessfully on 2d59495

Failed for the following reason:

1. If the user has created any Articles those Articles open with warning "JUser: :_load: Unable to load user with ID: "

why should the article roll back if that user made an edit - that makes no sense to me

Content published by the user is not the same thing as a user account. Especially as in many cases content submission guidelines will require some form of licensing/copyright agreement (usually agreed to when either creating an account or submitting the content).

So no, an account removal request should not arbitrarily delete anything that is basically "created by the account that is being deleted". That is an action that admins might be able to write automated tools to help in their jobs, but will still require the user making contact with the site owner to discuss the removal of content. This also gets more complicated as things like quoted comments aren't directly associated with the original post, but are inline text in a new post (i.e. comment threads or forum posts). So even if the original post is removed, a system has no way of knowing that parts of that post were quoted in another area.

@mbabker
I see your point. If that is a legal response then the PR only fails on on part of my point 1. Will edit my test report.

by the way I tested with two registered users and user A was not allowed to delete user B's account :D

@franz-wohlkoenig

is this related to #18160?

yes and no 😄

@C-Lodder

I assume super users should not be allowed to have their account deleted?

yes a SuperUser cannot delete himself from frontend
https://github.com/alikon/joomla-cms/blob/patch-99/components/com_users/models/delete.php#L154

@brianteeman

If we require double confirmation to create an account (confirming the email) then we should do the same for deleting an account

to be deleted you need to enter your email submit the form and then confirm
double for me 😄

@Webdongle

Failed for the following reason: If the user has created any Articles those Articles open with warning "JUser: :_load: Unable to load user with ID: "

it works like if you delete an user from backend see open discussion at #18160 for more

by the way I tested with two registered users and user A was not allowed to delete user B's account

this is wanted behaviuor

@alikon
> Failed for the following reason: If the user has created any Articles those Articles open with warning "JUser: :_load: Unable to load user with ID: "

it works like if you delete an user from backend see open discussion at #18160 for more

That does not mean it is expected behaviour that just shows the backend delete is an issue as well

  by the way I tested with two registered users and user A was not allowed to delete user B's account

this is wanted behaviuor

Of course it is that's why I put a :D (smiley face) but you missed that out when you quoted me.

@Webdongle got your point now

i've implement the onUserBeforeDelete event to check if there are articles createdby that user
and in this case prevent user deletion both side (admin/frontend)

"JUser: :_load: Unable to load user with ID: "

That message is actually the expected behavior if a user got deleted. JUser can't find the referenced user because, surprise, the user got deleted 😄
The article in question needs to be manually reassigned to a different user if you want to prevent that message.
To my knowledge, that message only appears if you want to edit a form, and then the message makes sense because you actually can reassign a new user in that form.

I have tested this item ✅ successfully on 50ca4e4

Perfect
Tested with user content Published, Unpublished and Trashed. User was not removed until the Article was emptied from Trash Expected behaviour.
Full Success

I have tested this item ✅ successfully on 50ca4e4

Hi,

Almost perfect. Maybe a message like "User successfully deleted" is missing.

Hi,

Almost perfect. Maybe a message like "User successfully deleted" is missing.

Almost perfect. Maybe a message like "User successfully deleted" is missing.

Please dont use the word "successfully". It is a redundant word that has been removed from similar strings already

Why is 'successfully ' a redundant word ?

#14607

Ready to Commit after two successful tests.

please remove the RTC status, as per #19023 (review) i'll complete the re-design work using specific plugins in the week-end

Ready to Commit removed as stated above, set on "Pending".

i've updated the test instructions to reflect the re-design changes
please re-test, review ect...

Is it ready for retesting ?

@Webdongle Yes per #19023 (comment)

I have tested this item 🔴 unsuccessfully on 50ca4e4

Allows deletion if user is set as contact

I have tested this item 🔴 unsuccessfully on 50ca4e4

I created a user and I accociated to this user one article and one contact.
Then I made the following tests:
I enabled both plugins --> I can not delete the user --> Tested successfully
I enabled only the "User Delete - Content" plugin --> I can not delete the user --> tested successfully
I enabled only the "User Delete - Contact" plugin --> I can delete the user --> I think it is not correct
I disabled both plugins --> I can delete the user --> Tested successfully

Step to reproduce the problem:

1. Create an user (assigned group: Registered)
2. Create a contact for the user
3. Add a menu item to the contact
4. Enable the plugin "User Delete - Contact"
5. Add a menu item type "user delete request"
6. Login as the new user and try to self delete

Expected result: the user will not be deleted

Actual result: the user is deleted. On the contact deltail I get: "JUser: :_load: Unable to load user with ID: 629"

oops I got the same result as Webdongle, but 2 minutes later.

Additional info:
On php_error_log I found the following lines:
PHP Notice: Undefined variable: i in C:\xampp\htdocs\joomla_test\joomla-cms-staging\plugins\user\joomla\joomla.php on line 424
PHP Warning: Creating default object from empty value in C:\xampp\htdocs\joomla_test\joomla-cms-staging\plugins\user\joomla\joomla.php on line 425

maybe this can help

@Webdongle , @robertolongo for contact i've forgot "linked user" fixed now

I have tested this item ✅ successfully on 50ca4e4

Tested successfully according the test instructions.

But, I would like to report 2 personal considerations:

1. I think it would be better to add a Message (with green box) if the user deletion succed.
   Now, when I try to delete a user I can get 3 different situation:

• An Error (red box) if I can not delete the user
• A Warning (yellow box) in case of "User not found"
• Success. In this case I have no box, just written "Deleted" (a Message with green box would be better - more homogeneous)

2. The 2 plugins seem correct. But what happen if a have a category associated to the user? ...or a custom field, etc...
   Why the plugins are only for articles and contacts?
   From my point of view the functionality seems not complete.

NOTE: Please note that I began to test joomla issues last week and I have very few experience. If I write nonsense please stop me.

Ready to Commit after two successful tests.

@alikon

Great job

@franz-wohlkoenig Please remove RTC as it needs one more test due to additional changes and comments.

removed "Ready to Commit".

Tested before the no need to query change
Set the user as Administrator
Logged in backend another browser and created menu and item
Logged in front end and deleted the new user
The menu and item stayed and can be accessed without error.
Same with category
But that is expected yes ?

@Webdongle yes expcted since there i'snt yet the plugins for check the esistence of a category, menu etc...
to answer @robertolongo question too i'll write them if maintainers like/approve this approach

So I take a closer look at the code today and from my point of view, there are still multiple issues with the implement of this PR which need to be addressed:

1. Can we re-use delete method of UsersModelUser class to delete a given user account? As I mentioned, this will require modify that method to remove the restriction to delete user own account

2. What should be the best method to give plugin a chance to check and prevent user account from being deleted (like in case account still have articles, contacts...). I think plugin can return false and add the warning message use $app->enqueueMessage method

3. What implement should be used to check and prevent delete user accounts in case use still have content associated with it? Right now, this PR triggers a new event (I think it is not needed, we can just use existing onUserBeforeDelete event) and multiple plugins (which would require admin to enable each of these plugins manually). Can we just add method onUserBeforeDelete into PlgUserJoomla and put off of necessary checks (articles, contacts...) there?

Could we get input from maintainers about these questions/concerns?

I don’t think 2/3 are really valid because of the new GDPR. If the user wants their account deleting it must happen regardless of content.

That is where JUser comes in and loads in a dummy User of User no longer exists or along them lines.

Agree with @tonypartridge

@tonypartridge @Webdongle I actually asked about technical questions about the best ways to implement these check. I guess you are saying that because of the new law, no check is needed? Just allow users to delete their account if they want anyway?

Yes exactly Tuan. I would suggest there is a plugin hook which is called on deleting users which allows 3rd party extensions to handle the content they when are deleted though.

On 22 Dec 2017, 01:13 +0000, Tuan Pham Ngoc notifications@github.com, wrote:

@tonypartridge @Webdongle I actually asked about technical questions about the best ways to implement these check. I guess you are saying that because of the new law, no check is needed? Just allow users to delete their account if they want anyway?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

That hooks is available out of the box, it is onUserBeforeDelete. The question is do we want to allow plugin to prevent deleting user? And what's the best way to allow plugin give the reason (message) to end user if it doesn't allow user account is deleted

Right now, we just trigger the event https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_users/models/user.php#L379 , and ignore the result returned by plugins https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_users/models/user.php#L379

Look like you are suggesting that user account should be deleted anyways, and the jobs of plugins are deleting the data associated to that user?

So we do! Shows how many user plugins I have created.

No, we shouldn’t ignore the result, that would be. BC break and there are valid cases where you may want to stop the delete, I.e. for an extension dev to implement a you have x content in the forums would you like this to be deleted?

But point being the core shouldn’t prevent a delete naturally, if a 3rd party does that’s for the end user to solve with the developer based on their needs.

Thinking out loud... we could look at another function within User delete which stores things requiring user action, I.e. com_content to request confirmation of deleting content and same for say com_mycomponent. Then the User has to tick a checkbox to auto delete the content for that component on processes again?

On 22 Dec 2017, 07:15 +0000, Tuan Pham Ngoc notifications@github.com, wrote:

That hooks is available out of the box, it is onUserBeforeDelete. The question is do we want to allow plugin to prevent deleting user? And what's the best way to allow plugin give the reason (message) to end user if it doesn't allow user account is deleted
Right now, we just trigger the event https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_users/models/user.php#L379 , and ignore the result returned by plugins https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_users/models/user.php#L379
Look like you are suggesting that user account should be deleted anyways, and the jobs of plugins are deleting the data associated to that user?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@joomdonation

I actually asked about technical questions about the best ways to implement these check. I guess you are saying that because of the new law, no check is needed?

No ... I think because of @mbabker saying about copywrite. It could be (due to T&C) that the user does not have the legal right to delete the content. #19023 (comment)

What about if the user deletion happens like this

• name, email, login name etc data in the users DB table are cleared,

... but the record in users table is kept as a "ghosted" record, maintaining the user id

• any profile data deleted

• usergroup mappings deleted

• user is blocked but also without any assignments to usergroups, user cannot login, (but you may take some extra step here)

• all content is kept

@ggppdk

but the record in users table is kept as a "ghosted" record, maintaining the user id

But that would prevent the user deleting all reference to themselves.

But the USERID isn’t them. It’s the system generated Id.

On 2 Apr 2018, 10:01 +0100, Kevin Griffiths notifications@github.com, wrote:

@ggppdk

but the record in users table is kept as a "ghosted" record, maintaining the user id
But that would prevent the user deleting all reference to themselves.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19023.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

But the user ID is used to display the author of the content. So Either the users name will be displayed in the article or it will show an error of not found.

You have the UserID already as foreign key in the content table. There is no point keeping an "empty" row for it in the user table. At least as long as we don't use referential integrity in our CMS.

It should return, sorry user not found. Or user unavailable or similar.

On 2 Apr 2018, 10:41 +0100, Kevin Griffiths notifications@github.com, wrote:

But the user ID is used to display the author of the content. So Either the users name will be displayed in the article or it will show an error of not found.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

It should return, sorry user not found. Or user unavailable or similar.

You can do that regardless of a row present or not.

But the user ID is used to display the author of the content. So Either the users name will be displayed in the article or it will show an error of not found.

The user ID is needed to query the users table to look up information, what is shown on the frontend is the result of the query. So, the database table's primary key (the user ID) is NOT personally identifiable information and should NOT be treated as such. Hence the reason the account should be pseudo anonymized, not deleted in full (basically run an UPDATE query on the users table row versus a DELETE query). You end up with something like @ghost here on GitHub in that state in that there is still a valid cross-table reference in the stored database information to a user record, but that user record is not identifiable to whatever individual for whom the account was originally created.

We already have enough problems with administrators deleting users in the backend then having all sorts of "user not found" errors show up on the site because, as core does not use proper relations and keys for cross-table references, records in other tables still have the deleted user ID in their fields (commonly created by and modified by). Let's not create more.

@mbabker

Hence the reason the account should be pseudo anonymized, not deleted in full (basically run an UPDATE query on the users table row versus a DELETE query). You end up with something like @ghost here on GitHub in that state in that there is still a valid cross-table reference in the stored database information to a user record, but that user record is not identifiable to whatever individual for whom the account was originally created.

We already have enough problems with administrators deleting users in the backend ...

So you support the idea of "anonymizing" the user record, and saying that deleting it fully is problematic, right ?

Giving a non-administrative user a button which can trigger a DELETE FROM users query should not happen, it carries too much risk (imagine if Joomla did have proper relational schema and there were CASCADE DELETE actions set on those relations); that delete query should ever only be triggered by an authorized administrator who fully understands the platform and the risk of said action.

So yes, the account should be anonymized.

KISS just have it delete user details regardless of content ... change the user name to 'Unknown user. No point in deleting the user record because that ID will not be used for new users anyway.

i'm strongly against ghost userid
if a user wich sumbit content or whatever ask for deletion he is asking for deleting all his account related stuff, i'm not a GDPR expert but.....

You have to ghost the account (completely anonymize it). Joomla lacks proper relational structure, and we have no idea how extensions are referencing the users table. A blind DELETE FROM users WHERE id = 42 query is going to cause problems for someone somewhere in ways that we cannot predict or fix. Whereas UPDATE users SET name = 'adlsfhklasdf', username = 'iuwherofasdnf', email = 'asdfjkn@whoue.com'... keeps the data record in place but has anonymized out the user information.

Sure, we can do the DELETE query. It's going to have consequences. Core's not in a state to deal with those consequences, extensions are probably even less so.

at some point in time we should really care about referential integrity
if extension developers are serious they can even write their onUserBeforeDelete code
the same as can do and should do the joomla core but maybe i'm too much optimistic, so let me fall down to the planet earth, i'll reconsider the ghost...

I know we need to get there (having referential integrity), but using what we have now I don't think it's practical to try and start down that path with this feature (especially as this particular item is more against a calendar deadline than core getting its schema together).

you're damn right as usual 😄
what should be the last available window/branch if i'll find the time to switch to the ghost "thing" ?

Whereas UPDATE users SET name = 'adlsfhklasdf', username = 'iuwherofasdnf', email = 'asdfjkn@whoue.com'... keeps the data record in place but has anonymized out the user information.

Sounds good but would it cause a problem when 'name'/'username'/'email' get duplicated when a second user deletes their accounts ?

Other than the primary key (that is unique by its nature)

• the other columns do not need to be unique

https://github.com/joomla/joomla-cms/blob/staging/installation/sql/mysql/joomla.sql#L1984-L2010

So you can set them to empty string

• i understand that some extensions will expect non-empty data and have some issues like email sending failures, just let these extensions deal with this

since the above is much less common problem than having almost all extensions assuming that a record in users table exist, causing a lot of problems if the row would get deleted

Also other topics

1. How to detect that a rows users table has been "ghosted"

• i say be empty username ?

2. Allow new user accounts having usernames and/or emails of ghosted account to be created ?

• just yes, no need to check or keep the old account name / emails, because of keep such information (even if we would keep a hash of them ...) then the promise that account data were deleted would be a lie

3. About extensions duplicating data from users table it will be their issue to deal with this
   but at least 1 (or 2 ?) new event(s) should be triggered to indicate the "ghosting" of a user record

How to detect that a rows users table has been "ghosted"

Why does there need to be detection of this? Why does a ghosted account need to be treated any differently than other accounts once it has been pseudo-anonymized?

Allow new user accounts having usernames and/or emails of ghosted account to be created ?

What remains in the database should still be valid data, even if pseudo-anonymized random stuff, and any validation rules should still apply. If today it is not allowed to duplicate usernames, a condition for "no duplicate usernames unless account is 'ghosted'" should not be introduced. These types of conditional rules are very complex to actually implement and much of the system would not be suited to account for it.

new event(s) should be triggered to indicate the "ghosting" of a user record

What is the need for new events that cannot be accounted for in the existing before/after save events?

Ok things are getting clearer on how this should be implemented

What remains in the database should still be valid data, even if pseudo-anonymized random stuff, and any validation rules should still apply. If today it is not allowed to duplicate usernames, a condition for "no duplicate usernames unless account is 'ghosted'" should not be introduced. These types of conditional rules are very complex to actually implement and much of the system would not be suited to account for it.

1. so we agree that since such conditionals should be avoided and anyway there will be no memory of ghosted usernames and emails,
   both of them will allowed for new acounts

2. about what data should be added in the ghosted user record (DB), it is not clear to me
   do we really want a random name ? and a random email ?

a. the displayed name should be "Ghost" or "Deleted user" or similar and not a random one because we do not want people to be able to distiguish between ghosted user content, it should not be possible to a web-sit's visitors to see that ghostAAA wrote these 3 comments, and ghostBBB wrote those 4 comments

b. the email should not be usable so that to no attempts should be made to send emails to it

3. about having a way to identify ghosted records, i think it is needed

e.g. easily exclude them from actions like promotional (newsletters) emails, etc)
and with a later PR be able to filter them in users manager too
and be possible in any extensions to create (if they want / need) a "users" like filter that can filter their views based on ghosted accounts)
a way to distiguish them can be ?

4. about events yes it can be done with existing events, no need for new events

please test/comment #20173