Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.0] New Feature - Change user before delete #40751

Closed
wants to merge 39 commits into from

Conversation

degobbis
Copy link
Contributor

Summary of Changes

As a feature often requested in the German community and solved by me with a plugin, I was asked by @tecpromotion, @coolcat-creations and some others to bring the feature into the core. So here is my tribute.

It opens the possibility to define a user to be used as a replacement when another user is deleted. As long as this user is entered as a replacement, it cannot be deleted itself.

This affects extensions that use users, such as com_content. Here it may also be necessary for copyright reasons that the original creator is still named. Therefore there is an option to enter the name of the user to be deleted as an alias.

All core tables in the database are searched for the user to be deleted and the user ID as well as the alias are adjusted according to the settings,

To make the whole thing also usable for other developers, a plugin group beforedeleteuser was added.
This makes it flexibly extendable, so that only an installable plugin with the necessary information is needed to implement the own extension.

Testing Instructions

At this point I can only say, test all possible scenarios to delete a user. There are countless, so I will not list them all here.

Actual result BEFORE applying this Pull Request

After deleting a user, the user ID remains in all core extensions, such as "com_content" and you get nasty warnings when editing an entry.

Expected result AFTER applying this Pull Request

  • No user can be deleted until a replacement user has been set.
  • After configuration, the user ID of the deleted user is replaced by the ID of the replacement user.
  • Depending on how it is set, the user name of the deleted user is entered as an alias.
    Of course, only where this is also offered by the extension.

Known Issue:

For large websites it may timeout, but the user is not deleted then.
So far I have not been able to intercept this server message in such a way that a message is displayed which entries have already been processed and that the deletion has to be triggered again to process the rest.

Maybe someone has an idea how this can be solved.

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-5.0-dev labels Jun 11, 2023
@degobbis
Copy link
Contributor Author

I still have one question:
What must be done so that it is also taken into account when updating from Joomla 4 to Joomla 5?
Or rather who takes care of it?

@brianteeman
Copy link
Contributor

please review the submitted code to ensure that the copy pasted copyright and doc blocks have been updated for the submitted code

@brianteeman
Copy link
Contributor

please review the xml additions to ensure that they match the codestyle of the rest of the xml file

@HLeithner
Copy link
Member

please review the submitted code to ensure that the copy pasted copyright and doc blocks have been updated for the submitted code

before you do this, we have to talk about the concept and the usecases and if they fit the needs of us and 3rd party extensions

@HLeithner HLeithner marked this pull request as draft June 11, 2023 08:35
@brianteeman
Copy link
Contributor

please review the submitted code to ensure that the copy pasted copyright and doc blocks have been updated for the submitted code

before you do this, we have to talk about the concept and the usecases and if they fit the needs of us and 3rd party extensions

in that case this is my comment to say that in its current form it should be an extension and not something for the core as in the real world this will be more of a pia than a problem solver.

it doesnt take into account if the user has authored any content or even if the user was an approved user and not a fake spam registration

@brianteeman
Copy link
Contributor

it also doesn't work as it doesnt change the modified field

image

@coolcat-creations
Copy link
Contributor

I would love to have such a feature because currently I struggle with lots of websites where I get the error that the user id was not found.
This can't be a core behaviour. When deleting an user the article should at least get the information that no user is assigned, but not an error "User Id Not found". It's possible now in core to assign "no user" so why is still the old user id assigned even the user is deleted?

@sandewt
Copy link
Contributor

sandewt commented Jun 11, 2023

In additioning to the previous comments:
Take a closer look at the date , version , since data if they match what is common within Joomla. Use __DEPLOY_VERSION__ if needed.

For some inspiration see #40553, there you will find good examples of the coding to be used.

@Fedik
Copy link
Member

Fedik commented Jun 26, 2023

I made alternative fix, please test #41048

@VillaesterModerneMedien

Tested this feature and I like it very much. I am very happy to have it now. Hope it finds the way in Core. Had many sites where I had to fix this manually in the database.
Specially when migrating older sites with many contents I had this problems. Now for the future this could be solved and a great time saver.
Thank you!!!

@HLeithner HLeithner changed the base branch from 5.0-dev to 5.1-dev September 30, 2023 22:49
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.1-dev.

@jjpcoca
Copy link

jjpcoca commented Oct 5, 2023

I have tested it but it does not work correctly. The deleted user is still assigned to the article with the ID but does not replace it with the chosen one.

screen shot 2023-10-05 at 12 50 47


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40751.

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.1-dev labels Oct 27, 2023
@joomla-cms-bot joomla-cms-bot removed the NPM Resource Changed This Pull Request can't be tested by Patchtester label Oct 27, 2023
@Hackwar
Copy link
Member

Hackwar commented Feb 21, 2024

Can you finish this, so that we can test this during PBF?

@Hackwar Hackwar added the PBF Pizza, Bugs and Fun label Feb 21, 2024
@coolcat-creations
Copy link
Contributor

@degobbis I would love the Feature in 5.1 :-)

@rdeutz
Copy link
Contributor

rdeutz commented Feb 21, 2024

I am closing this, we discussed this today and came to the conclusion that we want to go a different way. We think it is better to anonymise the user data instead of hard delete a user account. With this we not only fix it for com_content, also other extensions don't run into the problem of not existing user account. Essentially we do what we do in other areas and trash user accounts instead of delete. There will be an option for remove a user from the database, this will come with a warning that there can be site effects.

Thank you Guido for proposing this feature and working on this.

@rdeutz rdeutz closed this Feb 21, 2024
@degobbis
Copy link
Contributor Author

Too bad, I just finished the fix, including the action log.

@ot2sen
Copy link
Contributor

ot2sen commented Feb 21, 2024

@rdeutz Will this different way handle the Author rights (copyrights as said in PR intro)?
This PR using an Alias to preserve these rights did seem to provide a solution to that, although it didn´t seem necessary to go as far as always have a blanco user (spam regs and such.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Language Change This is for Translators PBF Pizza, Bugs and Fun PR-5.0-dev PR-5.1-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet