accountmodel: improve handling for account list reordering

This update fixes an issue where a stale account list might be used
in AccountListModel by synchronizing list invalidation with a newly
introduced signal (accountsReordered). This change prevents a data race
that could occur due to the asynchronous nature of setting the account
order over D-Bus, or when queuing the signal handling for
accountsChanged, which is emitted only once the reordering is saved.

As a result, QAbstractItemModel mutations are now performed within AccountListModel instead of in the UI.

Gitlab: #1638 (Account list in popup is incorrect after selection)
Change-Id: I7ed6eeb45eb319f21e40554f3d023ad24e139a6f

src/app/accountlistmodel.cpp

@@ -29,6 +29,35 @@ AccountListModel::AccountListModel(LRCInstance* instance, QObject* parent)
     : AbstractListModelBase(parent)
 {
     lrcInstance_ = instance;
+
+    // Avoid resetting/redrawing the model when the account status changes.
+    QObject::connect(&lrcInstance_->accountModel(),
+                     &AccountModel::accountStatusChanged,
+                     this,
+                     [&](const QString& accountId) {
+                         auto accountList = lrcInstance_->accountModel().getAccountList();
+                         auto index = accountList.indexOf(accountId);
+                         if (index != -1) {
+                             QModelIndex modelIndex = QAbstractListModel::index(index, 0);
+                             Q_EMIT dataChanged(modelIndex, modelIndex /*, ALL ROLES */);
+                         }
+                     });
+    // If there's a reorder, it's reasonable to reset the model for simplicity, instead
+    // of computing the difference. The same goes for accounts being added and removed.
+    // These operations will only occur when the list is hidden, unless dbus is used while
+    // the list is visible.
+    QObject::connect(&lrcInstance_->accountModel(),
+                     &AccountModel::accountsReordered,
+                     this,
+                     &AccountListModel::reset);
+    QObject::connect(&lrcInstance_->accountModel(),
+                     &AccountModel::accountAdded,
+                     this,
+                     &AccountListModel::reset);
+    QObject::connect(&lrcInstance_->accountModel(),
+                     &AccountModel::accountRemoved,
+                     this,
+                     &AccountListModel::reset);
 }
 
 int
@@ -91,6 +120,7 @@ AccountListModel::roleNames() const
 void
 AccountListModel::reset()
 {
+    // Used to invalidate proxy models.
     beginResetModel();
     endResetModel();
 }

src/app/lrcinstance.cpp

@@ -43,9 +43,8 @@ LRCInstance::LRCInstance(const QString& updateUrl,
     muteDaemon_ = muteDaemon;
     threadPool_->setMaxThreadCount(1);
 
-    connect(this, &LRCInstance::currentAccountIdChanged, [this] {
-        // save to config, editing the accountlistmodel's underlying data
-        accountModel().setTopAccount(currentAccountId_);
+    // Update the current account when the account list changes.
+    connect(&accountModel(), &AccountModel::accountsReordered, this, [this] {
         Q_EMIT accountListChanged();
 
         profile::Info profileInfo;
@@ -62,6 +61,11 @@ LRCInstance::LRCInstance(const QString& updateUrl,
         set_currentAccountAvatarSet(!profileInfo.avatar.isEmpty());
     });
 
+    connect(this, &LRCInstance::currentAccountIdChanged, [this] {
+        // This will trigger `AccountModel::accountsReordered`.
+        accountModel().setTopAccount(currentAccountId_);
+    });
+
     connect(&accountModel(), &AccountModel::profileUpdated, this, [this](const QString& id) {
         if (id != currentAccountId_)
             return;

src/app/mainview/components/AccountComboBox.qml

@@ -34,24 +34,6 @@ Label {
 
     property bool inSettings: viewCoordinator.currentViewName === "SettingsView"
 
-    // TODO: remove these refresh hacks use QAbstractItemModels correctly
-    Connections {
-        target: AccountAdapter
-
-        function onAccountStatusChanged(accountId) {
-            AccountListModel.reset();
-        }
-    }
-
-    Connections {
-        target: LRCInstance
-
-        function onAccountListChanged() {
-            root.update();
-            AccountListModel.reset();
-        }
-    }
-
     function togglePopup() {
         if (root.popup.opened) {
             root.popup.close();

src/app/mainview/components/AccountComboBoxPopup.qml

@@ -60,23 +60,6 @@ Popup {
 
             property bool inSettings: viewCoordinator.currentViewName === "SettingsView"
 
-            // TODO: remove these refresh hacks use QAbstractItemModels correctly
-            Connections {
-                target: AccountAdapter
-
-                function onAccountStatusChanged(accountId) {
-                    AccountListModel.reset();
-                }
-            }
-
-            Connections {
-                target: LRCInstance
-
-                function onAccountListChanged() {
-                    AccountListModel.reset();
-                }
-            }
-
             RowLayout {
                 id: mainLayout
                 anchors.fill: parent
@@ -257,11 +240,6 @@ Popup {
             color: JamiTheme.smartListHoveredColor
         }
 
-        // fake footer item as workaround for Qt 5.15 bug
-        // https://bugreports.qt.io/browse/QTBUG-85302
-        // don't use the clip trick and footer item overlay
-        // explained here https://stackoverflow.com/a/64625149
-        // as it causes other complexities in handling the drop shadow
         ItemDelegate {
             id: addAccountItem
 

src/libclient/accountmodel.cpp

@@ -428,8 +428,9 @@ AccountModelPimpl::updateAccounts()
     const auto previousAccountIdListSize = accountIdList.size();
     accountIdList = configurationManager.getAccountList();
 
-    // If this is just a reordering of the accounts, don't do anything
+    // If this is just a reordering of the accounts, just notify the view.
     if (accountIdList.size() == previousAccountIdListSize) {
+        Q_EMIT linked.accountsReordered();
         return;
     }
 

src/libclient/api/accountmodel.h

@@ -288,6 +288,10 @@ Q_SIGNALS:
      * @param accountID
      */
     void accountRemoved(const QString& accountID);
+    /**
+     * Emitted when the account list order has changed.
+     */
+    void accountsReordered();
     /**
      * Connect this signal to know when an account was updated.
      * @param accountID