Simplify notification handling

Summary:
Use QPointer for KNotification
Use ready signal for signalling updates

BUG: 400010

Test Plan: Spawned some notifications

Reviewers: #kde_connect, broulik, albertvaka

Reviewed By: #kde_connect, albertvaka

Subscribers: albertvaka, kdeconnect

Tags: #kde_connect

Differential Revision: https://phabricator.kde.org/D18354

interfaces/notificationsmodel.cpp

@@ -92,8 +92,6 @@ void NotificationsModel::setDeviceId(const QString& deviceId)
             this, &NotificationsModel::notificationRemoved);
     connect(m_dbusInterface, &OrgKdeKdeconnectDeviceNotificationsInterface::allNotificationsRemoved,
             this, &NotificationsModel::clearNotifications);
-    connect(m_dbusInterface, &OrgKdeKdeconnectDeviceNotificationsInterface::notificationUpdated,
-            this, &NotificationsModel::notificationUpdated);
 
     refreshNotificationList();
 
@@ -104,6 +102,7 @@ void NotificationsModel::notificationAdded(const QString& id)
 {
     beginInsertRows(QModelIndex(),  0, 0);
     NotificationDbusInterface* dbusInterface = new NotificationDbusInterface(m_deviceId, id, this);
+    connect(dbusInterface, &NotificationDbusInterface::ready, this, &NotificationsModel::notificationUpdated);
     m_notificationList.prepend(dbusInterface);
     endInsertRows();
 }
@@ -263,9 +262,7 @@ void NotificationsModel::clearNotifications()
     }
 }
 
-void NotificationsModel::notificationUpdated(const QString& id)
+void NotificationsModel::notificationUpdated()
 {
-    //TODO only emit the affected indices
-    Q_UNUSED(id);
     Q_EMIT dataChanged(index(0,0), index(m_notificationList.size() - 1, 0));
 }

interfaces/notificationsmodel.h

@@ -71,7 +71,7 @@ public Q_SLOTS:
 private Q_SLOTS:
     void notificationAdded(const QString& id);
     void notificationRemoved(const QString& id);
-    void notificationUpdated(const QString& id);
+    void notificationUpdated();
     void refreshNotificationList();
     void receivedNotifications(QDBusPendingCallWatcher* watcher);
     void clearNotifications();

plugins/notifications/notification.cpp

@@ -47,11 +47,10 @@ Notification::Notification(const NetworkPacket& np, QObject* parent)
     m_imagesDir = QDir::temp().absoluteFilePath(QStringLiteral("kdeconnect_") + username);
     m_imagesDir.mkpath(m_imagesDir.absolutePath());
     QFile(m_imagesDir.absolutePath()).setPermissions(QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner);
-    m_closed = false;
     m_ready = false;
 
     parseNetworkPacket(np);
-    createKNotification(false, np);
+    createKNotification(np);
 }
 
 Notification::~Notification()
@@ -70,7 +69,6 @@ void Notification::show()
     m_ready = true;
     Q_EMIT ready();
     if (!m_silent) {
-        m_closed = false;
         m_notification->sendEvent();
     }
 }
@@ -78,12 +76,12 @@ void Notification::show()
 void Notification::update(const NetworkPacket& np)
 {
     parseNetworkPacket(np);
-    createKNotification(!m_closed, np);
+    createKNotification(np);
 }
 
-KNotification* Notification::createKNotification(bool update, const NetworkPacket& np)
+KNotification* Notification::createKNotification(const NetworkPacket& np)
 {
-    if (!update) {
+    if (!m_notification) {
         m_notification = new KNotification(QStringLiteral("notification"), KNotification::CloseOnTimeout, this);
         m_notification->setComponentName(QStringLiteral("kdeconnect"));
     }
@@ -121,8 +119,6 @@ KNotification* Notification::createKNotification(bool update, const NetworkPacke
         connect(m_notification, &KNotification::action1Activated, this, &Notification::reply, Qt::UniqueConnection);
     }
 
-    connect(m_notification, &KNotification::closed, this, &Notification::closed);
-
     return m_notification;
 }
 
@@ -171,11 +167,6 @@ void Notification::reply()
     Q_EMIT replyRequested();
 }
 
-void Notification::closed()
-{
-    m_closed = true;
-}
-
 void Notification::parseNetworkPacket(const NetworkPacket& np)
 {
     m_internalId = np.get<QString>(QStringLiteral("id"));

plugins/notifications/notification.h

@@ -62,17 +62,16 @@ public:
     bool silent() const { return m_silent; }
     void update(const NetworkPacket& np);
     bool isReady() const { return m_ready; }
-    KNotification* createKNotification(bool update, const NetworkPacket& np);
+    KNotification* createKNotification(const NetworkPacket& np);
 
 public Q_SLOTS:
     Q_SCRIPTABLE void dismiss();
     Q_SCRIPTABLE void reply();
-    void closed();
 
     Q_SIGNALS:
     void dismissRequested(const QString& m_internalId);
     void replyRequested();
-    void ready();
+    Q_SCRIPTABLE void ready();
 
 private:
     QString m_internalId;
@@ -84,10 +83,9 @@ private:
     QString m_requestReplyId;
     bool m_dismissable;
     bool m_hasIcon;
-    KNotification* m_notification;
+    QPointer<KNotification> m_notification;
     QDir m_imagesDir;
     bool m_silent;
-    bool m_closed;
     QString m_payloadHash;
     bool m_ready;
 

plugins/notifications/notificationsdbusinterface.cpp

@@ -71,11 +71,15 @@ void NotificationsDbusInterface::processPacket(const NetworkPacket& np)
         if (id.startsWith(QLatin1String("org.kde.kdeconnect_tp::")))
             id = id.mid(id.indexOf(QLatin1String("::")) + 2);
         removeNotification(id);
-    } else {
+        return;
+    }
+
     QString id = np.get<QString>(QStringLiteral("id"));
 
+    Notification* noti = nullptr;
+
     if (!m_internalIdToPublicId.contains(id)) {
-            Notification* noti = new Notification(np, this);
+        noti = new Notification(np, this);
 
         if (noti->isReady()) {
             addNotification(noti);
@@ -84,22 +88,9 @@ void NotificationsDbusInterface::processPacket(const NetworkPacket& np)
         }
     } else {
         QString pubId = m_internalIdToPublicId.value(id);
-            Notification* noti = m_notifications.value(pubId);
+        noti = m_notifications.value(pubId);
-            if (!noti)
+    }
-                return;
-
     noti->update(np);
-
-            if (noti->isReady()) {
-                Q_EMIT notificationUpdated(pubId);
-            } else {
-                connect(noti, &Notification::ready, this, [this, pubId]{
-                    Q_EMIT notificationUpdated(pubId);
-                });
-            }
-        }
-
-    }
 }
 
 void NotificationsDbusInterface::addNotification(Notification* noti)