Skip to content

Make it possible to completely delete users not just disable #1243

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Jul 26, 2023
Merged

Make it possible to completely delete users not just disable #1243

merged 15 commits into from
Jul 26, 2023

Conversation

@kevinmessiaen
Copy link
Member

@kevinmessiaen kevinmessiaen commented Jul 13, 2023 •
edited
Loading

Description

This PR add the possibility to delete a disabled user from the administration panel. The delete icon is only showed for disabled users.
image

The following business rules are applied:

  • If the deleted is the owner of projects, those projects and associated entities will be deleted from database and local storage (models, datasets, test suites and python pickled methods).
  • The feedbacks generated by the user are kept and the owner will be shown ad "deleted user"

Related Issue

N/A

Type of Change

  • 📚 Examples / docs / tutorials / dependencies update
  • 🔧 Bug fix (non-breaking change which fixes an issue)
  • 🥂 Improvement (non-breaking change which improves an existing feature)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🔐 Security fix

Checklist

@linear
Copy link

linear bot commented Jul 13, 2023

GSK-1397 Make it possible to completely delete users, not just deactivate them

@kevinmessiaen kevinmessiaen marked this pull request as ready for review July 13, 2023 04:55
@henchaves
Copy link
Member

henchaves commented Jul 18, 2023 •
edited
Loading

I think we should display different icons for disable and delete actions. It was the first time I created / deleted a user, and it was a bit confusing. I didn't understand that after clicking on the bin icon this would make the user disabled instead of deleting it.

@kevinmessiaen
Copy link
Member Author

I think we should display different icons for disable and delete actions. It was the first time I created / deleted a user, and it was a bit confusing. I didn't understand that after clicking on the bin icon this would make the user disabled instead of deleting it.

You're right, now that we have the possibility to delete users we should update the disable icon. Let me update it

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

2.8% 2.8% Coverage
0.0% 0.0% Duplication

henchaves
henchaves approved these changes Jul 19, 2023
View reviewed changes
Copy link
Member

@henchaves henchaves left a comment

Choose a reason for hiding this comment

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

The "Restore User" doesn't work at the moment, but don't worry about that on this PR. There is a issue already created to deal with that https://linear.app/giskard/issue/GSK-1396/restore-user-doesnt-work

@kevinmessiaen
Merge branch 'main' into feature/gsk-1397-make-it-possible-to-complet…
a59601a 
…ely-delete-users-not-just

# Conflicts:
#	frontend/src/api.ts
@kevinmessiaen kevinmessiaen merged commit 717f6fa into main Jul 26, 2023
@Hartorn Hartorn deleted the feature/gsk-1397-make-it-possible-to-completely-delete-users-not-just branch September 13, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henchaves henchaves henchaves approved these changes

@Googleton Googleton Awaiting requested review from Googleton

@andreybavt andreybavt Awaiting requested review from andreybavt

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kevinmessiaen @henchaves