Skip to content

added option for account deletion requests #82

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

Merged
merged 7 commits into from
Jun 2, 2023

Conversation

sheldor1510
Copy link
Collaborator

  • Gave user option to request account deletion (only if user is not in any PI groups)
    Screenshot 2023-06-02 at 9 48 12 AM
  • Adds them to a new SQL table - account_deletion_requests
  • Sends email to admins
    Screenshot 2023-06-02 at 9 48 27 AM
  • Updates user management for admins (graying and italicizing the users who have requested account deletion)
    Screenshot 2023-06-02 at 9 48 47 AM
  • [Enhancement] Allow user requested account deletion #43

@sheldor1510 sheldor1510 changed the title added option for acc deletion requests added option for account deletion requests Jun 2, 2023
@sheldor1510 sheldor1510 requested a review from hakasapl June 2, 2023 15:01
Copy link
Collaborator

@hakasapl hakasapl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good, just 2 things.

Could you make the "Request Account Deletion" form not be visible on the account settings page if the user is in a PI group? Instead, just print some text there that says something along the lines of "You cannot request to delete your account while you are in a PI group". This way you don't have to include the javascript alert with the error later.

Also, once the user requests account deletion, they should not be allowed to join or request a PI group. Those checks should happen in UnityUser/UnityGroup, but also in the frontend:

  • The "Request PI Account" should be disabled on the account.php page
  • The "+" button on the "My PIs" page should be disabled
  • Both disabled buttons should have a note below them indicating that they are disabled because you have requested account deletion.

Let me know if I can clarify any of this, and thanks for this!

@sheldor1510 sheldor1510 requested a review from hakasapl June 2, 2023 16:59
Comment on lines +74 to +76
if (!$SQL->accDeletionRequestExists($USER->getUID())) {
$USER->requestAccountDeletion();
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should still check if the user is a member of any PI group before doing this. If they are, you can just die() the page, since it's not a state that should be reachable. Frontend verification is ideal, but the backend needs to verify as well since the user can change the frontend and submit the form in a way that you weren't expecting.

@sheldor1510 sheldor1510 requested a review from hakasapl June 2, 2023 17:18
Copy link
Collaborator

@hakasapl hakasapl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@hakasapl hakasapl merged commit cb6d232 into UnityHPC:main Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants