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: [WD-15753] Delete OIDC User #972

Merged
merged 1 commit into from
Nov 5, 2024
Merged

Conversation

Kxiru
Copy link
Contributor

@Kxiru Kxiru commented Oct 25, 2024

Done

  • Created API(s) to delete OIDC identities.
  • Created DeleteIdentitiesBtn
  • Added an inline delete identity button.

Fixes [list issues/bugs if needed]

QA

  1. Run the LXD-UI:
  2. Perform the following QA steps:
    • Navigate to Permissions -> Identities and attempt to delete one or many identities through the inline delete button or the bulk delete button.

Screenshots

image

@webteam-app
Copy link

@Kxiru Kxiru force-pushed the delete-oidc-user branch 2 times, most recently from 5859b20 to 1878c9c Compare October 25, 2024 17:35
@Kxiru Kxiru marked this pull request as ready for review October 25, 2024 17:36
@Kxiru
Copy link
Contributor Author

Kxiru commented Oct 25, 2024

Requires version 6.1/latest edge to run locally.
image

Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

The happy path works well, a bit of refinement is needed, see comments below.

@Kxiru Kxiru requested a review from edlerd October 29, 2024 09:53
@mas-who
Copy link
Collaborator

mas-who commented Oct 29, 2024

The styling of the action buttons is not consistent with the Permission Groups page, please reference the styles applied there to align the styling.

Screenshot from 2024-10-29 11-44-53

Copy link
Collaborator

@mas-who mas-who left a comment

Choose a reason for hiding this comment

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

some minor suggestions :)

src/context/useSupportedFeatures.tsx Show resolved Hide resolved
src/pages/permissions/PermissionIdentities.tsx Outdated Show resolved Hide resolved
src/pages/permissions/actions/DeleteIdentityBtn.tsx Outdated Show resolved Hide resolved
src/pages/permissions/actions/DeleteIdentityBtn.tsx Outdated Show resolved Hide resolved
@Kxiru Kxiru requested a review from mas-who October 31, 2024 11:34
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

QA LGTM, only some very small nitpicks.

src/pages/permissions/PermissionIdentities.tsx Outdated Show resolved Hide resolved
src/pages/permissions/actions/BulkDeleteIdentitiesBtn.tsx Outdated Show resolved Hide resolved
src/pages/permissions/actions/BulkDeleteIdentitiesBtn.tsx Outdated Show resolved Hide resolved
Signed-off-by: Nkeiruka <nkeiruka.whenu@canonical.com>
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

LGTM

@Kxiru Kxiru merged commit b33fd83 into canonical:main Nov 5, 2024
12 checks passed
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.

4 participants