fix(graphQL): fix error when retry mark notifications read (#3562)

* fix(graphQL): fix error when retry mark notifications read Signed-off-by: Wei Zhang <[email protected]> * fix: duplicated queried read notifications Signed-off-by: Wei Zhang <[email protected]> * fix: fix test for retry mark all Signed-off-by: Wei Zhang <[email protected]> * chore: use upsert to handle conflict Signed-off-by: Wei Zhang <[email protected]> --------- Signed-off-by: Wei Zhang <[email protected]>
TabbyML · Dec 14, 2024 · 011f04b · 011f04b
1 parent 0314db6
commit 011f04b
Show file tree

Hide file tree

Showing 4 changed files with 124 additions and 48 deletions.
diff --git a/ee/tabby-db/schema/schema.sql b/ee/tabby-db/schema/schema.sql
@@ -232,7 +232,7 @@ CREATE TABLE notifications(
   -- content of notification, in markdown format.
   content TEXT NOT NULL
 );
-CREATE TABLE readed_notifications(
+CREATE TABLE read_notifications(
   id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT,
   user_id INTEGER NOT NULL,
   notification_id INTEGER NOT NULL,

diff --git a/ee/tabby-db/schema/schema.svg b/ee/tabby-db/schema/schema.svg
diff --git a/ee/tabby-db/src/notifications.rs b/ee/tabby-db/src/notifications.rs
@@ -33,9 +33,12 @@ impl DbConn {
 
     pub async fn mark_notification_read(&self, id: i64, user_id: i64) -> Result<()> {
         query!(
-            "INSERT INTO read_notifications (notification_id, user_id) VALUES (?, ?)",
+            "INSERT INTO read_notifications (notification_id, user_id)
+             VALUES (?, ?)
+             ON CONFLICT (notification_id, user_id)
+             DO NOTHING",
             id,
-            user_id
+            user_id,
         )
         .execute(&self.pool)
         .await?;
@@ -71,7 +74,7 @@ ON
     notifications.id = read_notifications.notification_id
     AND read_notifications.user_id = ?
 WHERE
-    {}
+    ({})
     AND read_notifications.notification_id IS NULL;
         "#,
             recipient_clause

diff --git a/ee/tabby-webserver/src/service/notification.rs b/ee/tabby-webserver/src/service/notification.rs
@@ -176,7 +176,7 @@ mod tests {
         assert!(service
             .mark_read(&user_id, Some(&notification_id))
             .await
-            .is_err())
+            .is_ok())
     }
 
     #[tokio::test]
@@ -419,4 +419,77 @@ mod tests {
         assert_eq!(notifications.len(), 1);
         assert!(notifications[0].read);
     }
+
+    #[tokio::test]
+    async fn test_admin_mark_single_then_all() {
+        let db = DbConn::new_in_memory().await.unwrap();
+        let service = create(db.clone());
+
+        let user = db
+            .create_user("test1".into(), None, true, None)
+            .await
+            .unwrap()
+            .as_id();
+        let notification = db
+            .create_notification("admin", "notification1")
+            .await
+            .unwrap()
+            .as_id();
+        db.create_notification("admin", "notification2")
+            .await
+            .unwrap();
+
+        // mark single notification
+        service.mark_read(&user, Some(&notification)).await.unwrap();
+        let notifications = service.list(&user).await.unwrap();
+        assert_eq!(notifications.len(), 2);
+        assert!(notifications[0].read);
+        assert!(!notifications[1].read);
+
+        // mark all notifications
+        service.mark_read(&user, None).await.unwrap();
+        let notifications = service.list(&user).await.unwrap();
+        assert_eq!(notifications.len(), 2);
+        assert!(notifications[0].read);
+        assert!(notifications[1].read);
+    }
+
+    #[tokio::test]
+    async fn test_admin_mark_single_twice() {
+        let db = DbConn::new_in_memory().await.unwrap();
+        let service = create(db.clone());
+
+        let user = db
+            .create_user("test1".into(), None, true, None)
+            .await
+            .unwrap()
+            .as_id();
+        let notification = db
+            .create_notification("admin", "notification1")
+            .await
+            .unwrap()
+            .as_id();
+
+        service.mark_read(&user, Some(&notification)).await.unwrap();
+        assert!(service.mark_read(&user, Some(&notification)).await.is_ok());
+    }
+
+    #[tokio::test]
+    async fn test_admin_mark_all_twice() {
+        let db = DbConn::new_in_memory().await.unwrap();
+        let service = create(db.clone());
+
+        let user = db
+            .create_user("test1".into(), None, true, None)
+            .await
+            .unwrap()
+            .as_id();
+        db.create_notification("admin", "notification1")
+            .await
+            .unwrap()
+            .as_id();
+
+        service.mark_read(&user, None).await.unwrap();
+        assert!(service.mark_read(&user, None).await.is_ok());
+    }
 }