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

Allow user to delete self #1787

Merged
merged 14 commits into from
Mar 14, 2023
Merged

Allow user to delete self #1787

merged 14 commits into from
Mar 14, 2023

Conversation

Steve-Mcl
Copy link
Contributor

Description

  • Provide the means for a user to delete own account under the following rules:
    • user is not the last platform administrator
    • user is not the sole owner of a team
  • Adds DELETE route to user
  • Adds deletion button to "User Settings"
  • Adds logger for user.deleted
  • Updates UI for logging
  • Updates failing tests (due to above rules changes)
  • Adds the following tests...
    • Can not delete own account if cookie not present
    • Admin can delete own account if another admin exists
    • Last admin cannot delete own account
    • Last owner of a team cannot delete own account
    • Non admin user who is a team member can delete own account
    • Team owner can delete own account if another team owner exists
  • Updates "User Management" doc

Screenshots

Deleting last admin

image

image

Deleting sole team owner

image

image

Related Issue(s)

#1767

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • [-] Upgrade instructions
    • [-] Configuration details
    • Concepts

Labels

  • [-] Backport needed? -> add the backport label
  • [-] Includes a DB migration? -> add the area:migration label

@Steve-Mcl Steve-Mcl linked an issue Mar 9, 2023 that may be closed by this pull request
3 tasks
forge/db/models/User.js Outdated Show resolved Hide resolved
@@ -20,4 +20,10 @@ Users can only be removed if they don't own any teams and projects. As such the
user should delete any projects and ensure their teams have alternative owners. They
can either do this themselves or an Admin user can do it for them.

Once all teams and projects are removed, the user can be removed using the option in the "Edit User" dialog.
### Deleting your own account
Copy link
Member

Choose a reason for hiding this comment

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

This section is for admin users - not where regular users will go looking for help.

We probably want to add a bit to docs/cloud/README.md as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in b503d52

forge/routes/api/user.js Outdated Show resolved Hide resolved
frontend/src/components/audit-log/AuditEntryVerbose.vue Outdated Show resolved Hide resolved
@@ -19,7 +19,10 @@
</div>
</template>
<template v-else>
<ff-button @click="startEdit">Edit</ff-button>
<div class="flex space-x-4">
Copy link
Member

Choose a reason for hiding this comment

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

The way this is presented, it feels like a 'cancel' button next to 'edit'.

I think it should appear more consistent with the 'Delete Project' button under Project/Instance settings:

image

The text can also say something about needing to delete/transfer team ownership as well. We shouldn't let the user try to delete their account if we know it's going to fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated in 7c26d2c

image

@Steve-Mcl
Copy link
Contributor Author

tests failing on untouched / unrelated issue:

/home/runner/work/flowforge/flowforge/frontend/src/pages/project/Overview.vue
Error:   107:9  error  The "PlusSmIcon" component has been registered but not used  vue/no-unused-components

@Pezmc
Copy link
Contributor

Pezmc commented Mar 14, 2023

@Steve-Mcl Fix in #1814

@Steve-Mcl
Copy link
Contributor Author

Thanks @Pezmc weird that even though #1814 is merged, it still fails.

Will merge or rebase shortly.

@Steve-Mcl
Copy link
Contributor Author

@knolleary @hardillb I believe I have addressed all of the issues.

Please re-review at your convenience.

Copy link
Contributor

@hardillb hardillb left a comment

Choose a reason for hiding this comment

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

Looks good to me

@knolleary knolleary merged commit b276e88 into main Mar 14, 2023
@knolleary knolleary deleted the allow-user-delete-self branch March 14, 2023 16:10
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.

Allow users to delete accounts
4 participants