Merge pull request #4989 from nextcloud/bugfix/dismiss

Fix the dismiss button: display it whenever possible.

src/gui/tray/ActivityItem.qml

@@ -46,7 +46,7 @@ ItemDelegate {
             Layout.fillWidth: true
             Layout.minimumHeight: Style.minActivityHeight
 
-            showDismissButton: model.links.length > 0 && model.linksForActionButtons.length === 0
+            showDismissButton: model.links.length > 0
 
             activityData: model
 
@@ -94,7 +94,7 @@ ItemDelegate {
 
             adjustedHeaderColor: root.adjustedHeaderColor
 
-            onTriggerAction: activityModel.slotTriggerAction(model.index, actionIndex)
+            onTriggerAction: activityModel.slotTriggerAction(model.activityIndex, actionIndex)
             onShowReplyField: root.isTalkReplyOptionVisible = true
         }
     }

src/gui/tray/ActivityItemContent.qml

@@ -180,37 +180,20 @@ RowLayout {
         }
     }
 
-    Button {
+    CustomButton {
         id: dismissActionButton
 
-        Layout.preferredWidth: Style.trayListItemIconSize * 0.6
-        Layout.preferredHeight: Style.trayListItemIconSize * 0.6
-
-        Layout.alignment: Qt.AlignCenter
-
-        Layout.margins: Style.roundButtonBackgroundVerticalMargins
-
-        NCToolTip {
-            visible: parent.hovered
-            text: qsTr("Dismiss")
-        }
-
-        Accessible.name: qsTr("Dismiss")
+        Layout.preferredWidth: Style.trayListItemIconSize
+        Layout.preferredHeight: Style.trayListItemIconSize
 
         visible: root.showDismissButton && !shareButton.visible
 
-        background: Rectangle {
-            color: "transparent"
-        }
+        imageSource: "image://svgimage-custom-color/clear.svg" + "/" + Style.ncTextColor
+        imageSourceHover: "image://svgimage-custom-color/clear.svg" + "/" + UserModel.currentUser.headerTextColor
 
-        contentItem: Image {
-            anchors.fill: parent
-            source: parent.hovered ? Theme.darkMode ?
-                "image://svgimage-custom-color/clear.svg/white" : "image://svgimage-custom-color/clear.svg/black" :
-                "image://svgimage-custom-color/clear.svg/grey"
-            sourceSize.width: 24
-            sourceSize.height: 24
-        }
+        toolTipText: qsTr("Dismiss")
+
+        bgColor: Style.menuBorder
 
         onClicked: root.dismissButtonClicked()
     }

src/gui/tray/activitylistmodel.cpp

@@ -740,27 +740,10 @@ void ActivityListModel::slotTriggerDismiss(const int activityIndex)
         return;
     }
 
-    const auto activityLinks = _finalList[activityIndex]._links;
-
-    const auto foundActivityLinkIt = std::find_if(std::cbegin(activityLinks), std::cend(activityLinks), [](const ActivityLink &link) {
-        return link._verb == QStringLiteral("DELETE");
-    });
-
-    if (foundActivityLinkIt == std::cend(activityLinks)) {
-        qCWarning(lcActivity) << "Couldn't find dismiss action in activity at index" << activityIndex
-                              << " links.size() " << activityLinks.size();
-        return;
-    }
-
-    const auto actionIndex = static_cast<int>(std::distance(activityLinks.begin(), foundActivityLinkIt));
-
-    if (actionIndex < 0 || actionIndex > activityLinks.size()) {
-        qCWarning(lcActivity) << "Couldn't find dismiss action in activity at index" << activityIndex
-                              << " actionIndex found " << actionIndex;
-        return;
-    }
+    constexpr auto deleteVerb = "DELETE";
+    const auto activity = _finalList[activityIndex];
 
-    slotTriggerAction(activityIndex, actionIndex);
+    emit sendNotificationRequest(activity._accName, Utility::concatUrlPath(accountState()->account()->url(), "ocs/v2.php/apps/notifications/api/v2/notifications/" + QString::number(activity._id)).toString(), deleteVerb, activityIndex);
 }
 
 AccountState *ActivityListModel::accountState() const

src/gui/tray/notificationhandler.cpp

@@ -154,22 +154,6 @@ void ServerNotificationHandler::slotNotificationsReceived(const QJsonDocument &j
         }
         a._link = link;
 
-        // Add another action to dismiss notification on server
-        // https://github.com/owncloud/notifications/blob/master/docs/ocs-endpoint-v1.md#deleting-a-notification-for-a-user
-        constexpr auto deleteVerb = "DELETE";
-        const auto itLink = std::find_if(std::cbegin(a._links), std::cend(a._links), [deleteVerb](const ActivityLink& link) {
-            Q_UNUSED(deleteVerb)
-            return link._verb == deleteVerb;
-        });
-        if (itLink == std::cend(a._links)) {
-            ActivityLink al;
-            al._label = tr("Dismiss");
-            al._link = Utility::concatUrlPath(ai->account()->url(), notificationsPath + "/" + QString::number(a._id)).toString();
-            al._verb = deleteVerb;
-            al._primary = false;
-            a._links.append(al);
-        }
-
         list.append(a);
     }
     emit newNotificationList(list);

test/testactivitylistmodel.cpp

@@ -109,17 +109,6 @@ public:
             activity.insert(QStringLiteral("datetime"), QDateTime::currentDateTime().toString(Qt::ISODate));
             activity.insert(QStringLiteral("icon"), QStringLiteral("http://example.de/core/img/places/calendar.svg"));
 
-            QJsonArray actionsArray;
-
-            QJsonObject secondaryAction;
-            secondaryAction.insert(QStringLiteral("label"), QStringLiteral("Dismiss"));
-            secondaryAction.insert(QStringLiteral("link"),
-                QString(QStringLiteral("http://cloud.example.de/remote.php/dav")
-                    + QStringLiteral("ocs/v2.php/apps/notifications/api/v2/notifications") + QString::number(i)));
-            secondaryAction.insert(QStringLiteral("type"), QStringLiteral("DELETE"));
-            secondaryAction.insert(QStringLiteral("primary"), false);
-            actionsArray.push_back(secondaryAction);
-
             _activityData.push_back(activity);
 
             _startingId++;
@@ -153,15 +142,6 @@ public:
             primaryAction.insert(QStringLiteral("primary"), false);
             actionsArray.push_back(primaryAction);
 
-            QJsonObject secondaryAction;
-            secondaryAction.insert(QStringLiteral("label"), QStringLiteral("Dismiss"));
-            secondaryAction.insert(QStringLiteral("link"),
-                QString(QStringLiteral("http://cloud.example.de/remote.php/dav")
-                    + QStringLiteral("ocs/v2.php/apps/notifications/api/v2/notifications") + QString::number(i)));
-            secondaryAction.insert(QStringLiteral("type"), QStringLiteral("DELETE"));
-            secondaryAction.insert(QStringLiteral("primary"), false);
-            actionsArray.push_back(secondaryAction);
-
             QJsonObject additionalAction;
             additionalAction.insert(QStringLiteral("label"), QStringLiteral("Additional 1"));
             additionalAction.insert(QStringLiteral("link"), QStringLiteral("http://cloud.example.de/call/9p4vjdzd"));
@@ -206,15 +186,6 @@ public:
             primaryAction.insert(QStringLiteral("primary"), false);
             actionsArray.push_back(primaryAction);
 
-            QJsonObject secondaryAction;
-            secondaryAction.insert(QStringLiteral("label"), QStringLiteral("Dismiss"));
-            secondaryAction.insert(QStringLiteral("link"),
-                QString(QStringLiteral("http://cloud.example.de/remote.php/dav")
-                    + QStringLiteral("ocs/v2.php/apps/notifications/api/v2/notifications") + QString::number(i)));
-            secondaryAction.insert(QStringLiteral("type"), QStringLiteral("DELETE"));
-            secondaryAction.insert(QStringLiteral("primary"), false);
-            actionsArray.push_back(secondaryAction);
-
             activity.insert(QStringLiteral("actions"), actionsArray);
 
             _activityData.push_back(activity);
@@ -250,11 +221,37 @@ public:
             replyAction.insert(QStringLiteral("primary"), false);
             actionsArray.push_back(replyAction);
 
+            activity.insert(QStringLiteral("actions"), actionsArray);
+
+            _activityData.push_back(activity);
+
+            _startingId++;
+        }
+
+        // Insert notification data
+        for (quint32 i = 0; i < _numItemsToInsert; i++) {
+            QJsonObject activity;
+            activity.insert(QStringLiteral("activity_id"), _startingId);
+            activity.insert(QStringLiteral("object_type"), "2fa_id");
+            activity.insert(QStringLiteral("subject"), QStringLiteral("Login attempt from 127.0.0.1"));
+            activity.insert(QStringLiteral("message"), QStringLiteral("Please apporve or deny the login attempt."));
+            activity.insert(QStringLiteral("object_name"), QStringLiteral(""));
+            activity.insert(QStringLiteral("datetime"), QDateTime::currentDateTime().toString(Qt::ISODate));
+            activity.insert(QStringLiteral("icon"), QStringLiteral("http://example.de/core/img/places/password.svg"));
+
+            QJsonArray actionsArray;
+
+            QJsonObject primaryAction;
+            primaryAction.insert(QStringLiteral("label"), QStringLiteral("Approve"));
+            primaryAction.insert(QStringLiteral("link"), QStringLiteral("/ocs/v2.php/apps/twofactor_nextcloud_notification/api/v1/attempt/39"));
+            primaryAction.insert(QStringLiteral("type"), QStringLiteral("POST"));
+            primaryAction.insert(QStringLiteral("primary"), true);
+            actionsArray.push_back(primaryAction);
+
             QJsonObject secondaryAction;
-            secondaryAction.insert(QStringLiteral("label"), QStringLiteral("Dismiss"));
+            secondaryAction.insert(QStringLiteral("label"), QStringLiteral("Cancel"));
             secondaryAction.insert(QStringLiteral("link"),
-                QString(QStringLiteral("http://cloud.example.de/remote.php/dav")
-                    + QStringLiteral("ocs/v2.php/apps/notifications/api/v2/notifications") + QString::number(i)));
+                QString(QStringLiteral("/ocs/v2.php/apps/twofactor_nextcloud_notification/api/v1/attempt/39")));
             secondaryAction.insert(QStringLiteral("type"), QStringLiteral("DELETE"));
             secondaryAction.insert(QStringLiteral("primary"), false);
             actionsArray.push_back(secondaryAction);
@@ -669,10 +666,19 @@ private slots:
 
                     const auto objectType = index.data(OCC::ActivityListModel::ObjectTypeRole).toString();
 
+                    const auto actionButtonsLinks =
+                        index.data(OCC::ActivityListModel::ActionsLinksForActionButtonsRole).toList();
+
+                    // Login attempt notification
+                    if (objectType == QStringLiteral("2fa_id")) {
+                        QVERIFY(actionsLinks.size() == 2);
+                        QVERIFY(actionsLinks[0].value<OCC::ActivityLink>()._primary);
+                        QVERIFY(!actionsLinks[1].value<OCC::ActivityLink>()._primary);
+                        QVERIFY(actionsLinksContextMenu.isEmpty());
+                    }
+
                     if ((objectType == QStringLiteral("chat") || objectType == QStringLiteral("call")
                             || objectType == QStringLiteral("room"))) {
-                        const auto actionButtonsLinks =
-                            index.data(OCC::ActivityListModel::ActionsLinksForActionButtonsRole).toList();
 
                         auto replyActionPos = 0;
                         if (objectType == QStringLiteral("call")) {
@@ -700,25 +706,17 @@ private slots:
                                 const auto actionButtonsAndContextMenuEntries = actionButtonsLinks + actionsLinksContextMenu;
                                 // in case total actions is longer than ActivityListModel::maxActionButtons, then a sum of action buttons and action menu entries must be equal to a total of action links
                                 QVERIFY(actionButtonsLinks.size() + actionsLinksContextMenu.size() == actionsLinks.size());
-                            } else {
-                                // in case a total of actions is less or equal to than ActivityListModel::maxActionButtons, then the length of action buttons must be greater than 1 and should contain "Dismiss" button at the end
-                                QVERIFY(actionButtonsLinks.size() > 1);
-                                QVERIFY(actionButtonsLinks[1].value<OCC::ActivityLink>()._label
-                                    == QObject::tr("Dismiss"));
                             }
                         } else if ((objectType == QStringLiteral("call"))) {
                             QVERIFY(
                                 actionButtonsLinks[0].value<OCC::ActivityLink>()._label == QStringLiteral("Call back"));
                         }
-                    } else {
-                        QVERIFY(actionsLinks[0].value<OCC::ActivityLink>()._label == QStringLiteral("Dismiss"));
                     }
                 }
             }
 
         } while (prevModelRowCount < model->rowCount());
     };
-
 };
 
 QTEST_MAIN(TestActivityListModel)