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

feat: 🍰 Admin - Remove User Profile #3140

Merged
merged 33 commits into from
Aug 27, 2020
Merged

Conversation

ogerly
Copy link
Contributor

@ogerly ogerly commented Feb 23, 2020

🍰 Pullrequest

Delete a user as Admin.
A new modal is added.

The admin has the possibility to permanently delete a user account from the network.

  • At present, all contributions and comments of a user account are automatically and irrevocably deleted / overwritten.

There is a second security question where you have to confirm the deletion a second time.

ezgif com-video-to-gif (3)

Issues

@cypress
Copy link

cypress bot commented Feb 23, 2020



Test summary

66 0 0 0


Run details

Project Human-Connection
Status Passed
Commit fde69a3
Started Aug 27, 2020 8:10 AM
Ended Aug 27, 2020 8:33 AM
Duration 22:30 💡
OS Linux Ubuntu Linux - 16.04
Browser Firefox 68

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@cypress
Copy link

cypress bot commented Feb 23, 2020



Test summary

66 0 0 0


Run details

Project Human-Connection
Status Passed
Commit c0698ea ℹ️
Started Aug 27, 2020 8:10 AM
Ended Aug 27, 2020 8:34 AM
Duration 23:21 💡
OS Linux Ubuntu Linux - 16.04
Browser Firefox 68

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@ogerly
Copy link
Contributor Author

ogerly commented Mar 4, 2020

DeleteUser for Admin added in the profile dropdown menu, deleted from the Admin user overview

@ogerly
Copy link
Contributor Author

ogerly commented Mar 5, 2020

FireShot Capture 334 - Human Connection - Human Connection - localhost
i have changed the style, and do not bind the deleteData.vue from the user settings to the modal anymore.
i display the data of the user to be deleted.

Copy link
Member

@Tirokk Tirokk left a comment

Choose a reason for hiding this comment

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

I have done a "quick“ review.

  • I would like to have an additional Modal question, so that an admin can not delete a users account by accidentally clicking the button.
  • Graphically the modals information looks a bit big to me.
  • Should there a possibility do decide if all the posts and comments should be deleted or not?

Please let me know if I can help you.

backend/src/middleware/permissionsMiddleware.js Outdated Show resolved Hide resolved
backend/src/middleware/permissionsMiddleware.js Outdated Show resolved Hide resolved
backend/src/middleware/permissionsMiddleware.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/users.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/users.js Outdated Show resolved Hide resolved
webapp/components/DeleteUserModal/DeleteUserModal.vue Outdated Show resolved Hide resolved
webapp/components/DeleteUserModal/DeleteUserModal.vue Outdated Show resolved Hide resolved
webapp/components/Modal.vue Outdated Show resolved Hide resolved
webapp/locales/de.json Outdated Show resolved Hide resolved
"commentedCount": "Delete my {count} comments",
"contributionsCount": "Delete my {count} posts",
"infoAdmin": "All contributions and comments of the user are additionally deleted!",
Copy link
Member

Choose a reason for hiding this comment

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

… are deleted as well! ?

Copy link
Member

Choose a reason for hiding this comment

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

This is not done yet!

Copy link
Member

Choose a reason for hiding this comment

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

Still not done! 🤔

@ogerly
Copy link
Contributor Author

ogerly commented Mar 6, 2020

@Tirokk thx!!, I have made some changes. and an additional confirmModal where you have to confirm the deletion again.
ezgif com-video-to-gif (3)

@ogerly
Copy link
Contributor Author

ogerly commented Mar 6, 2020

FireShot Capture 336 - Human Connection - Human Connection - localhost

@ogerly ogerly changed the title [WIP] feature: add New Moadal - deleteUserModal feature: add New Moadal - deleteUserModal Mar 10, 2020
@Tirokk
Copy link
Member

Tirokk commented May 29, 2020

That being said, I don't think we should completely change it. If other people are happy with this solution, who am I to say change it. Just not how I imagined it.

I understand that you @mattwr18 and @roschaefer have imagined it more like on the delete own account on the settings page.

As I gave my first review I was more in the thinking frame like @ogerly has started this issue. I had not even thought about an admin can or should type the name of the user to delete.
One of the reasons of my thinking frame was probably that the way to trigger the delete goes over the content menu of the users profile page in this case.
But the place to trigger the delete like here I do prefer against the users list of an admin, because the admin has to look the user they delete right into the face, kind of. It’s not as anonym as deleting the user from a list I like to say …

To have an additional page sounds like an overkill to me. Just in case the typing of the users name should really replace the second modal I would go for having the typing of the user name in the first modal then.

What is @ogerly saying and what do other people say? @HC-Team

@Tirokk Tirokk changed the title feature: add New Modal - deleteUserModal feature: 🍰 Add New Modal - deleteUserModal Jun 11, 2020
@ogerly
Copy link
Contributor Author

ogerly commented Aug 4, 2020

I have adapted the style a little bit ... see last PR

FireShot Capture 198 - Human Connection - Human Connection - localhost

@ogerly ogerly requested review from Mogge and removed request for alina-beck and mattwr18 August 19, 2020 09:53
United States of America TO United States
Copy link
Contributor Author

@ogerly ogerly left a comment

Choose a reason for hiding this comment

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

United States of America to United States

@Tirokk Tirokk changed the title feature: 🍰 Add New Modal - deleteUserModal feat: 🍰 Admin - Remove User Profile Aug 26, 2020
Copy link
Member

@Tirokk Tirokk left a comment

Choose a reason for hiding this comment

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

Hey @ogerly ,

I have still found some little things, but that should be the last and we can do it together, if you like.

In the following modal I more would like to have the created, Posts, and Comments over the data.
And created should then with a capital Created. And then all three in bold and the data in normal types.

Bildschirmfoto 2020-08-26 um 17 10 57

webapp/locales/en.json Outdated Show resolved Hide resolved
webapp/components/ContentMenu/ContentMenu.vue Outdated Show resolved Hide resolved
webapp/components/ContentMenu/ContentMenu.vue Show resolved Hide resolved
webapp/components/DeleteData/DeleteData.vue Outdated Show resolved Hide resolved
`,
variables: { id: this.userdata.id, resource: ['Post', 'Comment'] },
})
.then(({ _data }) => {
Copy link
Member

Choose a reason for hiding this comment

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

Is not fixed yet.
Is this intensionally?

You like it this way better?

webapp/locales/de.json Outdated Show resolved Hide resolved
"commentedCount": "Delete my {count} comments",
"contributionsCount": "Delete my {count} posts",
"infoAdmin": "All contributions and comments of the user are additionally deleted!",
Copy link
Member

Choose a reason for hiding this comment

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

Still not done! 🤔

ogerly and others added 2 commits August 27, 2020 09:14
ok

Co-authored-by: Wolfgang Huß <wolle.huss@pjannto.com>
@ogerly ogerly requested a review from Tirokk August 27, 2020 07:53
Copy link
Member

@Tirokk Tirokk left a comment

Choose a reason for hiding this comment

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

Hey @ogerly ,
great contribution! 🚀🚀💫💫
Well done 🤗

It is so important that an admin can delete a users account.
Therefore I’m really happy that we can merge it. 😍

@Tirokk Tirokk merged commit 47e43f2 into master Aug 27, 2020
@Tirokk Tirokk deleted the 17-Admin-Remove-user-profile branch August 27, 2020 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 [Feature] Admin - Remove user profile
4 participants