-
Notifications
You must be signed in to change notification settings - Fork 10
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
Conversation
There was a problem hiding this 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!
if (!$SQL->accDeletionRequestExists($USER->getUID())) { | ||
$USER->requestAccountDeletion(); | ||
} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
account_deletion_requests