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

Deleting a user should show name #4943

Conversation

Chris-Codes-It
Copy link
Contributor

@Chris-Codes-It Chris-Codes-It commented Nov 5, 2023

Changes

Amended Delete User dialog title to include username of user to be deleted

Issues

https://features.jellyfin.org/posts/2392/deleting-a-user-should-show-name

@Chris-Codes-It Chris-Codes-It requested a review from a team as a code owner November 5, 2023 23:15
@Chris-Codes-It
Copy link
Contributor Author

I've made the changes to the required UI files, and the English strings files. I've left the other language files alone as I would not be able to accurately translate

@Chris-Codes-It
Copy link
Contributor Author

image

@Chris-Codes-It Chris-Codes-It changed the title Initial commit. Dialog title shows user to be deleted Dialog title shows user to be deleted Nov 6, 2023
CONTRIBUTORS.md Outdated Show resolved Hide resolved
@dmitrylyzo dmitrylyzo added enhancement Improve existing functionality or small fixes ui & ux This PR or issue mainly concerns UI & UX labels Nov 6, 2023
@Chris-Codes-It Chris-Codes-It changed the title Dialog title shows user to be deleted Deleting a user should show name Nov 8, 2023
@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Nov 29, 2023
@jellyfin-bot

This comment was marked as outdated.

@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Nov 30, 2023
@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Jan 5, 2024
@jellyfin-bot

This comment was marked as outdated.

@thornbill thornbill added this to the v10.9.0 milestone Jan 12, 2024
@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Feb 1, 2024
Copy link

sonarqubecloud bot commented Feb 2, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@ptalmeida ptalmeida left a comment

Choose a reason for hiding this comment

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

I'd recommend squashing all those commits into one


if (!userId) {
console.error('Unexpected null user id');
return;
}

if (!username) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to return if no username exists? if we only need the userId I think this check isn't necessary and could prevent the deletion of some buggy user with a missing username.

@jellyfin-bot

This comment was marked as outdated.

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Feb 7, 2024
@jellyfin-bot jellyfin-bot removed the merge conflict Conflicts prevent merging label Mar 7, 2024
@thornbill thornbill force-pushed the feature/deleting_a_user_should_show_name branch from 87ef7cb to e62bad4 Compare March 17, 2024 08:19
CONTRIBUTORS.md Outdated Show resolved Hide resolved
@thornbill thornbill force-pushed the feature/deleting_a_user_should_show_name branch from c7548d3 to 150964a Compare March 17, 2024 08:38
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit 150964a
Status ✅ Deployed!
Preview URL https://ec6097b4.jellyfin-web.pages.dev
Type 🔀 Preview

View build logs
View bot logs

@thornbill thornbill merged commit b026148 into jellyfin:master Mar 17, 2024
12 checks passed
@Chris-Codes-It Chris-Codes-It deleted the feature/deleting_a_user_should_show_name branch March 17, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality or small fixes ui & ux This PR or issue mainly concerns UI & UX
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants