From c9070930d221d14a437bd22b58b1c7891ec880d5 Mon Sep 17 00:00:00 2001
From: fdb-hiroshima <35889323+fdb-hiroshima@users.noreply.github.com>
Date: Sat, 4 May 2019 17:15:41 +0200
Subject: [PATCH] Improve notification management (#561)

* Don't notify for comment when mentioned

fix #505

* Don't save notification for remote users

fix #472
---
 plume-models/src/comments.rs | 24 +++++++++++++++---------
 plume-models/src/follows.rs  | 21 ++++++++++++---------
 plume-models/src/likes.rs    | 20 +++++++++++---------
 plume-models/src/mentions.rs | 23 ++++++++++++++---------
 plume-models/src/reshares.rs | 20 +++++++++++---------
 src/routes/comments.rs       |  3 ++-
 6 files changed, 65 insertions(+), 46 deletions(-)

diff --git a/plume-models/src/comments.rs b/plume-models/src/comments.rs
index 50ad955..63da5a7 100644
--- a/plume-models/src/comments.rs
+++ b/plume-models/src/comments.rs
@@ -15,7 +15,7 @@ use medias::Media;
 use mentions::Mention;
 use notifications::*;
 use plume_common::activity_pub::{
-    inbox::{AsObject, FromId},
+    inbox::{AsActor, AsObject, FromId},
     Id, IntoId, PUBLIC_VISIBILITY,
 };
 use plume_common::utils;
@@ -157,14 +157,20 @@ impl Comment {
 
     pub fn notify(&self, conn: &Connection) -> Result<()> {
         for author in self.get_post(conn)?.get_authors(conn)? {
-            Notification::insert(
-                conn,
-                NewNotification {
-                    kind: notification_kind::COMMENT.to_string(),
-                    object_id: self.id,
-                    user_id: author.id,
-                },
-            )?;
+            if Mention::list_for_comment(conn, self.id)?
+                .iter()
+                .all(|m| m.get_mentioned(conn).map(|u| u != author).unwrap_or(true))
+                && author.is_local()
+            {
+                Notification::insert(
+                    conn,
+                    NewNotification {
+                        kind: notification_kind::COMMENT.to_string(),
+                        object_id: self.id,
+                        user_id: author.id,
+                    },
+                )?;
+            }
         }
         Ok(())
     }
diff --git a/plume-models/src/follows.rs b/plume-models/src/follows.rs
index 47d1d1e..20ab186 100644
--- a/plume-models/src/follows.rs
+++ b/plume-models/src/follows.rs
@@ -67,15 +67,18 @@ impl Follow {
         Ok(act)
     }
 
-    pub fn notify(&self, conn: &Connection) -> Result<Notification> {
-        Notification::insert(
-            conn,
-            NewNotification {
-                kind: notification_kind::FOLLOW.to_string(),
-                object_id: self.id,
-                user_id: self.following_id,
-            },
-        )
+    pub fn notify(&self, conn: &Connection) -> Result<()> {
+        if User::get(conn, self.following_id)?.is_local() {
+            Notification::insert(
+                conn,
+                NewNotification {
+                    kind: notification_kind::FOLLOW.to_string(),
+                    object_id: self.id,
+                    user_id: self.following_id,
+                },
+            )?;
+        }
+        Ok(())
     }
 
     /// from -> The one sending the follow request
diff --git a/plume-models/src/likes.rs b/plume-models/src/likes.rs
index 18678cc..6f57301 100644
--- a/plume-models/src/likes.rs
+++ b/plume-models/src/likes.rs
@@ -4,7 +4,7 @@ use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl};
 
 use notifications::*;
 use plume_common::activity_pub::{
-    inbox::{AsObject, FromId},
+    inbox::{AsActor, AsObject, FromId},
     Id, IntoId, PUBLIC_VISIBILITY,
 };
 use posts::Post;
@@ -54,14 +54,16 @@ impl Like {
     pub fn notify(&self, conn: &Connection) -> Result<()> {
         let post = Post::get(conn, self.post_id)?;
         for author in post.get_authors(conn)? {
-            Notification::insert(
-                conn,
-                NewNotification {
-                    kind: notification_kind::LIKE.to_string(),
-                    object_id: self.id,
-                    user_id: author.id,
-                },
-            )?;
+            if author.is_local() {
+                Notification::insert(
+                    conn,
+                    NewNotification {
+                        kind: notification_kind::LIKE.to_string(),
+                        object_id: self.id,
+                        user_id: author.id,
+                    },
+                )?;
+            }
         }
         Ok(())
     }
diff --git a/plume-models/src/mentions.rs b/plume-models/src/mentions.rs
index 520d822..0984b72 100644
--- a/plume-models/src/mentions.rs
+++ b/plume-models/src/mentions.rs
@@ -3,6 +3,7 @@ use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl};
 
 use comments::Comment;
 use notifications::*;
+use plume_common::activity_pub::inbox::AsActor;
 use posts::Post;
 use schema::mentions;
 use users::User;
@@ -129,14 +130,18 @@ impl Mention {
 
     fn notify(&self, conn: &Connection) -> Result<()> {
         let m = self.get_mentioned(conn)?;
-        Notification::insert(
-            conn,
-            NewNotification {
-                kind: notification_kind::MENTION.to_string(),
-                object_id: self.id,
-                user_id: m.id,
-            },
-        )
-        .map(|_| ())
+        if m.is_local() {
+            Notification::insert(
+                conn,
+                NewNotification {
+                    kind: notification_kind::MENTION.to_string(),
+                    object_id: self.id,
+                    user_id: m.id,
+                },
+            )
+            .map(|_| ())
+        } else {
+            Ok(())
+        }
     }
 }
diff --git a/plume-models/src/reshares.rs b/plume-models/src/reshares.rs
index bf8e515..8b11993 100644
--- a/plume-models/src/reshares.rs
+++ b/plume-models/src/reshares.rs
@@ -4,7 +4,7 @@ use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl};
 
 use notifications::*;
 use plume_common::activity_pub::{
-    inbox::{AsObject, FromId},
+    inbox::{AsActor, AsObject, FromId},
     Id, IntoId, PUBLIC_VISIBILITY,
 };
 use posts::Post;
@@ -79,14 +79,16 @@ impl Reshare {
     pub fn notify(&self, conn: &Connection) -> Result<()> {
         let post = self.get_post(conn)?;
         for author in post.get_authors(conn)? {
-            Notification::insert(
-                conn,
-                NewNotification {
-                    kind: notification_kind::RESHARE.to_string(),
-                    object_id: self.id,
-                    user_id: author.id,
-                },
-            )?;
+            if author.is_local() {
+                Notification::insert(
+                    conn,
+                    NewNotification {
+                        kind: notification_kind::RESHARE.to_string(),
+                        object_id: self.id,
+                        user_id: author.id,
+                    },
+                )?;
+            }
         }
         Ok(())
     }
diff --git a/src/routes/comments.rs b/src/routes/comments.rs
index a99ac18..991b830 100644
--- a/src/routes/comments.rs
+++ b/src/routes/comments.rs
@@ -62,7 +62,6 @@ pub fn create(
                 },
             )
             .expect("comments::create: insert error");
-            comm.notify(&*conn).expect("comments::create: notify error");
             let new_comment = comm
                 .create_activity(&rockets)
                 .expect("comments::create: activity error");
@@ -80,6 +79,8 @@ pub fn create(
                 .expect("comments::create: mention save error");
             }
 
+            comm.notify(&*conn).expect("comments::create: notify error");
+
             // federate
             let dest = User::one_by_instance(&*conn).expect("comments::create: dest error");
             let user_clone = user.clone();