[PATCH] kcm/users: refine "deleting logged-in user" UX
authorNate Graham <nate@kde.org>
Fri, 14 Mar 2025 22:23:58 +0000 (16:23 -0600)
committerAurélien COUDERC <coucouf@debian.org>
Tue, 20 May 2025 06:31:26 +0000 (08:31 +0200)
Right now there are two problems:
1. Deleting a logged-in user shows no warning about this.
2. Asking to delete the files of a logged-in user fails silently
   (AccountsService simply won't do it).

This commit solves both problems: now the user sees a warning dialog
when they try to delete another logged-in user, and it also prevents
them from trying to delete a logged-in user's files, instead redirecting
them to just delete the account but not the files.

To achieve this, I needed to change what the `loggedIn` property does,
because previously it was inaccurate, returning whether the queried user
is the currently logged-in user. Now it returns whether the quaried user
is logged in, and I added a new `isMe` property to hold whether the
queried user is the currently logged-in user.

BUG: 495494
FIXED-IN: 6.4.0

Gbp-Pq: Name upstream_9abfdd26_kcm-users-refine-deleting-logged-in-user-UX.patch

kcms/users/src/CMakeLists.txt
kcms/users/src/ui/UserDetailsPage.qml
kcms/users/src/ui/main.qml
kcms/users/src/user.cpp
kcms/users/src/user.h
kcms/users/src/usermodel.cpp
kcms/users/src/usermodel.h

index 45aed3c9702a2b9538b1ed15b503197a7b67c9b5..5848d376a72cb773bd117e177da7568cc25c35a7 100644 (file)
@@ -69,6 +69,7 @@ target_link_libraries(kcm_users PRIVATE
     KF6::I18n
     KF6::KCMUtilsQuick
     KF6::Wallet
+    PW::KWorkspace
     Qt::DBus
     crypt
 )
index d50e1bdcc69d06bc1add73501d8d29344e7f5c83..a4358d5eb7b6c914cdd2d8ca5b195cee2ee473be 100644 (file)
@@ -36,8 +36,8 @@ KCM.SimpleKCM {
     Connections {
         target: user
         function onPasswordSuccessfullyChanged() {
-            // Prompt to change the wallet password of the logged-in user
-            if (usersDetailPage.user.loggedIn && usersDetailPage.user.usesDefaultWallet()) {
+            // Prompt to change the wallet password of the current user
+            if (usersDetailPage.user.isCurrentUser && usersDetailPage.user.usesDefaultWallet()) {
                 changeWalletPassword.open()
             }
         }
@@ -81,6 +81,15 @@ KCM.SimpleKCM {
         return pending
     }
 
+    function deleteUser(uid: int, deleteUserFiles: bool) {
+        if (usersDetailPage.user.loggedIn) {
+            deleteLoggedInUserWarningDialog.askedToDeleteUserFilesWhenImpossible = deleteUserFiles
+            deleteLoggedInUserWarningDialog.open()
+        } else {
+            kcm.mainUi.deleteUser(usersDetailPage.user.uid, deleteUserFiles)
+        }
+    }
+
     Component.onCompleted: {
         kcm.needsSave = Qt.binding(resolvePending)
     }
@@ -93,6 +102,31 @@ KCM.SimpleKCM {
         position: Kirigami.InlineMessage.Position.Header
     }
 
+    Kirigami.PromptDialog {
+        id: deleteLoggedInUserWarningDialog
+
+        property bool askedToDeleteUserFilesWhenImpossible: false
+
+        parent: usersDetailPage.QQC2.Overlay.overlay
+        maximumWidth: Kirigami.Units.gridUnit * 30
+
+        title: askedToDeleteUserFilesWhenImpossible
+            ? i18nc("@title:window", "Delete Logged-In User Without Deleting Files?")
+            : i18nc("@title:window", "Delete Logged-In User?")
+        subtitle: askedToDeleteUserFilesWhenImpossible
+            ? xi18nc("@info:usagetip", "%1 is currently logged in, so their files cannot be deleted. Delete just the account instead?<nl/><nl/>This will make %1 unable to log in again, and they may experience strange behaviors until they log out.", usersDetailPage.user.displayPrimaryName)
+            : i18nc("@info:usagetip", "%1 is currently logged in. Deleting the account will make them unable to log in again, and they may experience strange behaviors until they log out.", usersDetailPage.user.displayPrimaryName)
+        dialogType: Kirigami.PromptDialog.Warning
+        standardButtons: Kirigami.Dialog.Ok | Kirigami.Dialog.Cancel
+
+        onAccepted: kcm.mainUi.deleteUser(usersDetailPage.user.uid, false)
+
+        Component.onCompleted: {
+            standardButton(Kirigami.Dialog.Ok).text = i18nc("@action: button", "Delete %1", usersDetailPage.user.realName)
+            standardButton(Kirigami.Dialog.Ok).icon.name = "edit-delete"
+        }
+    }
+
     ColumnLayout {
         KirigamiComponents.AvatarButton {
             readonly property int size: 6 * Kirigami.Units.gridUnit
@@ -193,7 +227,7 @@ KCM.SimpleKCM {
             QQC2.Button {
                 id: deleteUser
 
-                enabled: !usersDetailPage.user.loggedIn && (!kcm.userModel.rowCount() < 2)
+                enabled: !usersDetailPage.user.isCurrentUser
 
                 KeyNavigation.down: fingerprintButton
 
@@ -204,14 +238,14 @@ KCM.SimpleKCM {
                         text: i18n("Delete files")
                         icon.name: "edit-delete-shred"
                         onClicked: {
-                            kcm.mainUi.deleteUser(usersDetailPage.user.uid, true)
+                            usersDetailPage.deleteUser(usersDetailPage.user.uid, true);
                         }
                     }
                     QQC2.MenuItem {
                         text: i18n("Keep files")
                         icon.name: "document-multiple"
                         onClicked: {
-                            kcm.mainUi.deleteUser(usersDetailPage.user.uid, false)
+                            usersDetailPage.deleteUser(usersDetailPage.user.uid, false);
                         }
                     }
                 }
@@ -236,7 +270,7 @@ KCM.SimpleKCM {
                 if (kcm.fingerprintModel.currentlyEnrolling) {
                     kcm.fingerprintModel.stopEnrolling();
                 }
-                kcm.fingerprintModel.switchUser(user.name == kcm.userModel.getLoggedInUser().name ? "" : user.name);
+                kcm.fingerprintModel.switchUser(user.name == kcm.userModel.getCurrentUser().name ? "" : user.name);
 
                 if (fingerprintButton.dialog === null) {
                     const component = Qt.createComponent("FingerprintDialog.qml");
index 02215f76861b593bcbaedc2a370fbf8ada21f136..751fc8df773a409030f9ecf8ff9c1ea037464329 100644 (file)
@@ -71,7 +71,7 @@ KCM.ScrollViewKCM {
             kcm.columnWidth = Kirigami.Units.gridUnit * 15
 
             // Push users page on desktop for two pane layout
-            kcm.push("UserDetailsPage.qml", { user: kcm.userModel.getLoggedInUser() })
+            kcm.push("UserDetailsPage.qml", { user: kcm.userModel.getCurrentUser() })
         }
     }
 
index 41915a3bcb459d614f0d094dcb73d45f956681da..2cc823579b3b13dd41f1875c53a4c8529addafb8 100644 (file)
@@ -15,6 +15,7 @@
 #include <QImage>
 #include <QImageWriter>
 #include <config-workspace.h>
+#include <kdisplaymanager.h>
 #include <sys/types.h>
 #include <unistd.h>
 
@@ -211,12 +212,24 @@ void User::loadData()
         userDataChanged = true;
         Q_EMIT administratorChanged();
     }
-    const auto loggedIn = (mUid == getuid());
-    if (mLoggedIn != loggedIn) {
-        mLoggedIn = loggedIn;
-        mOriginalLoggedIn = mLoggedIn;
+
+    mIsCurrentUser = (mUid == getuid());
+
+    mOriginalLoggedIn = mLoggedIn;
+
+    SessList sessions;
+    KDisplayManager displayManager;
+    displayManager.localSessions(sessions);
+    for (auto s : sessions) {
+        if (s.user == mOriginalName)
+            mLoggedIn = true;
+    }
+
+    if (mOriginalLoggedIn != mLoggedIn) {
         userDataChanged = true;
+        Q_EMIT loggedInChanged();
     }
+
     if (userDataChanged) {
         Q_EMIT dataChanged();
     }
@@ -334,6 +347,11 @@ void User::changeWalletPassword()
     KWallet::Wallet::changePassword(QStringLiteral("kdewallet"), 1);
 }
 
+bool User::isCurrentUser() const
+{
+    return mIsCurrentUser;
+}
+
 bool User::loggedIn() const
 {
     return mLoggedIn;
index e58baaf880396c7c67e065e1ee7d77b6937240da..314e491667eabb15f4b40192eea71c0066b43510 100644 (file)
@@ -77,7 +77,9 @@ class User : public QObject
 
     Q_PROPERTY(bool faceValid READ faceValid NOTIFY faceValidChanged)
 
-    Q_PROPERTY(bool loggedIn READ loggedIn CONSTANT)
+    Q_PROPERTY(bool isCurrentUser READ isCurrentUser CONSTANT)
+
+    Q_PROPERTY(bool loggedIn READ loggedIn NOTIFY loggedInChanged)
 
     Q_PROPERTY(bool administrator READ administrator WRITE setAdministrator NOTIFY administratorChanged)
 
@@ -94,6 +96,7 @@ public:
     QString email() const;
     QUrl face() const;
     bool faceValid() const;
+    bool isCurrentUser() const;
     bool loggedIn() const;
     bool administrator() const;
     QDBusObjectPath path() const;
@@ -122,6 +125,7 @@ Q_SIGNALS:
     void realNameChanged();
     void displayNamesChanged();
     void emailChanged();
+    void loggedInChanged();
     void faceChanged();
     void faceValidChanged();
     void administratorChanged();
@@ -145,6 +149,7 @@ private:
     bool mOriginalAdministrator = false;
     bool mFaceValid = false;
     bool mOriginalFaceValid = false;
+    bool mIsCurrentUser = false;
     bool mLoggedIn = false;
     bool mOriginalLoggedIn = false;
     QDBusObjectPath mPath;
index 8be299659b2744666373004f4d0c9628b3b6b05b..868caf0d8236d4b674f75d282d44b63f5ba470f7 100644 (file)
@@ -83,7 +83,7 @@ UserModel::UserModel(QObject *parent)
     }
 
     std::ranges::stable_partition(m_userList, [](User *u) {
-        return u->loggedIn();
+        return u->isCurrentUser();
     });
 
     connect(this, &QAbstractItemModel::rowsInserted, this, &UserModel::moreThanOneAdminUserChanged);
@@ -103,6 +103,7 @@ QHash<int, QByteArray> UserModel::roleNames() const
     names.insert(AdministratorRole, QByteArrayLiteral("administrator"));
     names.insert(UserRole, QByteArrayLiteral("userObject"));
     names.insert(FaceValidRole, QByteArrayLiteral("faceValid"));
+    names.insert(IsCurrentUserRole, QByteArrayLiteral("isCurrentUser"));
     names.insert(LoggedInRole, QByteArrayLiteral("loggedIn"));
     names.insert(SectionHeaderRole, QByteArrayLiteral("sectionHeader"));
     return names;
@@ -110,10 +111,10 @@ QHash<int, QByteArray> UserModel::roleNames() const
 
 UserModel::~UserModel() = default;
 
-User *UserModel::getLoggedInUser() const
+User *UserModel::getCurrentUser() const
 {
     for (const auto user : std::as_const(m_userList)) {
-        if (user->loggedIn()) {
+        if (user->isCurrentUser()) {
             return user;
         }
     }
@@ -161,10 +162,12 @@ QVariant UserModel::data(const QModelIndex &index, int role) const
         return QFile::exists(user->face().path());
     case UserRole:
         return QVariant::fromValue(user);
+    case IsCurrentUserRole:
+        return user->isCurrentUser();
     case LoggedInRole:
         return user->loggedIn();
     case SectionHeaderRole:
-        return user->loggedIn() ? i18n("Your Account") : i18n("Other Accounts");
+        return user->isCurrentUser() ? i18n("Your Account") : i18n("Other Accounts");
     }
 
     return QVariant();
index a9c1efeba9d358b56e8461b8b1fe121eea0b2086..167107ae4300ca487982d0ed82449061a01a6c56 100644 (file)
@@ -29,6 +29,7 @@ public:
         FaceValidRole,
         AdministratorRole,
         UserRole,
+        IsCurrentUserRole,
         LoggedInRole,
         SectionHeaderRole,
     };
@@ -42,7 +43,7 @@ public:
     QVariant data(const QModelIndex &index, int role) const override;
     int rowCount(const QModelIndex &parent = QModelIndex()) const override;
 
-    Q_INVOKABLE User *getLoggedInUser() const;
+    Q_INVOKABLE User *getCurrentUser() const;
 
     QHash<int, QByteArray> roleNames() const override;