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

Not able to delete the user from wagtail admin #3442

Closed
sandeepsajan0 opened this issue Jun 13, 2023 · 9 comments · Fixed by #3658
Closed

Not able to delete the user from wagtail admin #3442

sandeepsajan0 opened this issue Jun 13, 2023 · 9 comments · Fixed by #3658
Assignees
Labels
OTF Type: Bug Bugs! Things that are broken :-/
Milestone

Comments

@sandeepsajan0
Copy link
Member

Even after deleting all the related submissions and projects, the staff admin/staff is not able to delete the user.

It is showing the following error msg on deleting the user:
image

@sandeepsajan0 sandeepsajan0 added the Type: Bug Bugs! Things that are broken :-/ label Jun 13, 2023
@sandeepsajan0 sandeepsajan0 self-assigned this Jun 13, 2023
frjo pushed a commit that referenced this issue Aug 31, 2023
…on (#3445)

Possible Fix of #3442 

On submission deletion, all other activities, determination, and other
things are getting removed but as this `NEW SUBMISSION` event is not
directly connected to submission so not getting removed. It is creating
issues at the time of user deletion even after submission deletion.

I was also thinking to show the model name, and object name in error
itself but user deletion only seems to be dependent on `Activities` and
`Events`(user can also be deleted only by removing Activities and events
not submissions related to them), and even if we show staff the event or
activities ids I don't think staff can easily delete those anyhow via
UI. I think we need to decide a better way to delete a user, maybe issue
#3441 help us?
@fourthletter fourthletter modified the milestones: Backlog, Hotfix/Bugs Sep 22, 2023
@wes-otf
Copy link
Contributor

wes-otf commented Nov 1, 2023

@frjo @sandeepsajan0 Did #3445 fix this/can it be closed? Or is further action required?

@frjo
Copy link
Member

frjo commented Nov 1, 2023

If I remember correctly this issue is due to some left over items in "actions_*". So the error message are correct and we need to find a way to delete these last items.

@sandeepsajan0
Copy link
Member Author

@wes-otf It is kind of partially fixed. As Fredrik mentioned, the problem is the leftover actions for the submissions that already has been deleted.
So basically when we delete a user, we have to delete their submissions first and submissions take care of every other thing like reviews and determination but there is a submission creation action(indirectly connected event) that remains existed/undeleted so because of that action we were not able to delete the user.
In #3445, we took care of that action and deleted that as well with submission deletion. But this PR only works for new submission deletions while previously deleted submissions' actions are still there in the system. So, for now, we can delete the users who are not connected with those deleted submissions but the ones who are connected we can't delete them.

I think we can create a custom migration that can delete all the actions for non-existing submissions and then I think we will be ready to delete any user from the system.

@frjo
Copy link
Member

frjo commented Nov 2, 2023

@sandeepsajan0 Excellent summary of the issue. I think the custom migration fix is a good way forward.

@wes-otf
Copy link
Contributor

wes-otf commented Nov 2, 2023

@sandeepsajan0 yeah this was great, thanks for that background! I can take a stab at that custom migration.

@wes-otf
Copy link
Contributor

wes-otf commented Nov 8, 2023

@frjo @sandeepsajan0 Are we only worried about deleting Applicants accounts? Digging through the changes made in #3445 it looks like only NEW_SUBMISSION events were handled but when events like ARCHIVE_SUBMISSION (even on a deleted submission) or DELETE_SUBMISSION exist in the activity_event table with id of the intended user to delete, the deletion issue is seen.

For example, I created a submission as an applicant, then on a different admin account deleted the submission. I then attempted to delete said admin account, and ran into The object you are trying to delete is used somewhere. Please remove any usages and try again!.

Is this the intended functionality or should should these events also be deleted/have their by ids changed? Maybe this would even somewhat tie into #3441.

@frjo
Copy link
Member

frjo commented Nov 9, 2023

@wes-otf Good work on investigating this!

I think DELETE_SUBMISSION and ARCHIVE_SUBMISSION event should get the same fix as @sandeepsajan0 did for NEW_SUBMISSION.

I can not see any bad things come from that, what do you think?

@sandeepsajan0
Copy link
Member Author

sandeepsajan0 commented Nov 13, 2023

@wes-otf Great investigation. We should have a fix for all these events as well but I think the deletion of applicant accounts is the priority because:

  1. We have public forms and submitting the forms creates a user with an applicant role so spamming can be an issue, although we have other fixes like a rate limit to avoid spam still we can have some spam users(applicants).
  2. Staff and Admin are trusted accounts and are connected to most of the submissions so we might not want to remove all reviews or other activities of a staff for several submissions(data issues). I think deactivating instead of deleting is the better option for these accounts?
  3. It is easier and quicker to fix in comparison to staff and admin accounts. Staff deletion might be way more complex than applicant deletion and might require some sort of discussion as well(because of its destructive nature).
  4. On the other side, every applicant's data might not be that important. Ex: spams, and if a user submitted only one application a long time ago and staff/admin is comfortable with deleting that submission then they should able to delete the user as well because there is no data of that user.

Maybe this would even somewhat tie into #3441.

Right, We might want to anonymise the staff's data instead of deleting that as mentioned in #3441.

@wes-otf
Copy link
Contributor

wes-otf commented Nov 15, 2023

Great, that PR should address these issues and I'll add a note onto #3441 explaining that further action could be required here. @sandeepsajan0 Thanks again for the in-depth breakdowns! That was super helpful.

frjo pushed a commit that referenced this issue Nov 23, 2023
…3658)

Closes #3442. 

This small migration addition compliments #3445 as it will remove any
previous `NEW_SUBMISSION` events that belong to submissions that no
longer exist. This will allow for the removal of applicants that don't
have any active submission, but can't be deleted due to the `The object
you are trying to delete is used somewhere...` error.

Cheers!
wes-otf added a commit that referenced this issue May 7, 2024
…3658)

Closes #3442. 

This small migration addition compliments #3445 as it will remove any
previous `NEW_SUBMISSION` events that belong to submissions that no
longer exist. This will allow for the removal of applicants that don't
have any active submission, but can't be deleted due to the `The object
you are trying to delete is used somewhere...` error.

Cheers!
wes-otf added a commit that referenced this issue May 8, 2024
…3658)

Closes #3442. 

This small migration addition compliments #3445 as it will remove any
previous `NEW_SUBMISSION` events that belong to submissions that no
longer exist. This will allow for the removal of applicants that don't
have any active submission, but can't be deleted due to the `The object
you are trying to delete is used somewhere...` error.

Cheers!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OTF Type: Bug Bugs! Things that are broken :-/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants