Merge branch 'notify-mentions' into 'master'

Make mentions send notifications

See merge request framasoft/mobilizon!951

lib/service/activity/comment.ex

@@ -9,6 +9,8 @@ defmodule Mobilizon.Service.Activity.Comment do
   alias Mobilizon.Service.Activity
   alias Mobilizon.Service.Workers.{ActivityBuilder, LegacyNotifierBuilder}
 
+  import Mobilizon.Service.Activity.Utils, only: [maybe_inserted_at: 0]
+
   @behaviour Activity
 
   @impl Activity
@@ -64,15 +66,6 @@ defmodule Mobilizon.Service.Activity.Comment do
     )
   end
 
-  @spec maybe_inserted_at :: map()
-  defp maybe_inserted_at do
-    if Application.fetch_env!(:mobilizon, :env) == :test do
-      %{}
-    else
-      %{"inserted_at" => DateTime.utc_now()}
-    end
-  end
-
   @type notification_type :: atom()
 
   # An actor is mentionned

lib/service/activity/discussion.ex

@@ -3,9 +3,12 @@ defmodule Mobilizon.Service.Activity.Discussion do
   Insert a discussion activity
   """
   alias Mobilizon.{Actors, Discussions}
-  alias Mobilizon.Discussions.Discussion
+  alias Mobilizon.Actors.Actor
+  alias Mobilizon.Discussions.{Comment, Discussion}
   alias Mobilizon.Service.Activity
-  alias Mobilizon.Service.Workers.ActivityBuilder
+  alias Mobilizon.Service.Workers.{ActivityBuilder, LegacyNotifierBuilder}
+
+  import Mobilizon.Service.Activity.Utils, only: [maybe_inserted_at: 0]
 
   @behaviour Activity
 
@@ -24,16 +27,22 @@ defmodule Mobilizon.Service.Activity.Discussion do
     author_id = Keyword.get(options, :actor_id, author.id)
     old_discussion = Keyword.get(options, :old_discussion)
 
-    ActivityBuilder.enqueue(:build_activity, %{
+    send_mention_notifications(subject, discussion, discussion.last_comment, options)
-      "type" => "discussion",
+
-      "subject" => subject,
+    ActivityBuilder.enqueue(
-      "subject_params" => subject_params(discussion, subject, old_discussion),
+      :build_activity,
-      "group_id" => group.id,
+      %{
-      "author_id" => author_id,
+        "type" => "discussion",
-      "object_type" => "discussion",
+        "subject" => subject,
-      "object_id" => if(subject != "discussion_deleted", do: to_string(discussion.id), else: nil),
+        "subject_params" => subject_params(discussion, subject, old_discussion),
-      "inserted_at" => DateTime.utc_now()
+        "group_id" => group.id,
-    })
+        "author_id" => author_id,
+        "object_type" => "discussion",
+        "object_id" =>
+          if(subject != "discussion_deleted", do: to_string(discussion.id), else: nil)
+      }
+      |> Map.merge(maybe_inserted_at())
+    )
   end
 
   def insert_activity(_, _), do: {:ok, nil}
@@ -53,4 +62,41 @@ defmodule Mobilizon.Service.Activity.Discussion do
   defp subject_params(%Discussion{} = discussion, _, _) do
     %{discussion_slug: discussion.slug, discussion_title: discussion.title}
   end
+
+  # An actor is mentionned
+  @spec send_mention_notifications(String.t(), Discussion.t(), Comment.t(), Keyword.t()) ::
+          {:ok, Oban.Job.t()} | {:ok, :skipped}
+  defp send_mention_notifications(
+         subject,
+         %Discussion{
+           id: discussion_id,
+           title: title,
+           slug: slug,
+           actor: %Actor{id: group_id, type: :Group}
+         },
+         %Comment{actor_id: actor_id, mentions: mentions},
+         _options
+       )
+       when subject in ["discussion_created", "discussion_replied"] and length(mentions) > 0 do
+    LegacyNotifierBuilder.enqueue(
+      :legacy_notify,
+      %{
+        "subject" => :discussion_mention,
+        "subject_params" => %{
+          discussion_slug: slug,
+          discussion_title: title
+        },
+        "type" => :discussion,
+        "object_type" => :discussion,
+        "author_id" => actor_id,
+        "group_id" => group_id,
+        "object_id" => to_string(discussion_id),
+        "mentions" => Enum.map(mentions, & &1.actor_id)
+      }
+    )
+
+    {:ok, :enqueued}
+  end
+
+  defp send_mention_notifications(_, _, _, _), do: {:ok, :skipped}
 end

lib/service/activity/renderer/comment.ex

@@ -45,58 +45,9 @@ defmodule Mobilizon.Service.Activity.Renderer.Comment do
             ),
           url: event_url(activity)
         }
-
-      :discussion_mention ->
-        %{
-          body:
-            dgettext("activity", "%{profile} mentionned you in the discussion %{discussion}.", %{
-              profile: profile,
-              discussion: title(activity)
-            }),
-          url: discussion_url(activity)
-        }
-
-      :discussion_renamed ->
-        %{
-          body:
-            dgettext("activity", "%{profile} renamed the discussion %{discussion}.", %{
-              profile: profile,
-              discussion: title(activity)
-            }),
-          url: discussion_url(activity)
-        }
-
-      :discussion_archived ->
-        %{
-          body:
-            dgettext("activity", "%{profile} archived the discussion %{discussion}.", %{
-              profile: profile,
-              discussion: title(activity)
-            }),
-          url: discussion_url(activity)
-        }
-
-      :discussion_deleted ->
-        %{
-          body:
-            dgettext("activity", "%{profile} deleted the discussion %{discussion}.", %{
-              profile: profile,
-              discussion: title(activity)
-            }),
-          url: nil
-        }
     end
   end
 
-  defp discussion_url(activity) do
-    Routes.page_url(
-      Endpoint,
-      :discussion,
-      Actor.preferred_username_and_domain(activity.group),
-      activity.subject_params["discussion_slug"]
-    )
-  end
-
   defp event_url(activity) do
     Routes.page_url(
       Endpoint,
@@ -107,5 +58,4 @@ defmodule Mobilizon.Service.Activity.Renderer.Comment do
 
   defp profile(activity), do: Actor.display_name_and_username(activity.author)
   defp event_title(activity), do: activity.subject_params["event_title"]
-  defp title(activity), do: activity.subject_params["discussion_title"]
 end

lib/service/activity/renderer/discussion.ex

@@ -15,14 +15,16 @@ defmodule Mobilizon.Service.Activity.Renderer.Discussion do
   def render(%Activity{} = activity, options) do
     locale = Keyword.get(options, :locale, "en")
     Gettext.put_locale(locale)
+    profile = profile(activity)
+    title = title(activity)
 
     case activity.subject do
       :discussion_created ->
         %{
           body:
             dgettext("activity", "%{profile} created the discussion %{discussion}.", %{
-              profile: profile(activity),
+              profile: profile,
-              discussion: title(activity)
+              discussion: title
             }),
           url: discussion_url(activity)
         }
@@ -31,8 +33,18 @@ defmodule Mobilizon.Service.Activity.Renderer.Discussion do
         %{
           body:
             dgettext("activity", "%{profile} replied to the discussion %{discussion}.", %{
-              profile: profile(activity),
+              profile: profile,
-              discussion: title(activity)
+              discussion: title
+            }),
+          url: discussion_url(activity)
+        }
+
+      :discussion_mention ->
+        %{
+          body:
+            dgettext("activity", "%{profile} mentionned you in the discussion %{discussion}.", %{
+              profile: profile,
+              discussion: title
             }),
           url: discussion_url(activity)
         }
@@ -41,8 +53,8 @@ defmodule Mobilizon.Service.Activity.Renderer.Discussion do
         %{
           body:
             dgettext("activity", "%{profile} renamed the discussion %{discussion}.", %{
-              profile: profile(activity),
+              profile: profile,
-              discussion: title(activity)
+              discussion: title
             }),
           url: discussion_url(activity)
         }
@@ -51,8 +63,8 @@ defmodule Mobilizon.Service.Activity.Renderer.Discussion do
         %{
           body:
             dgettext("activity", "%{profile} archived the discussion %{discussion}.", %{
-              profile: profile(activity),
+              profile: profile,
-              discussion: title(activity)
+              discussion: title
             }),
           url: discussion_url(activity)
         }
@@ -61,8 +73,8 @@ defmodule Mobilizon.Service.Activity.Renderer.Discussion do
         %{
           body:
             dgettext("activity", "%{profile} deleted the discussion %{discussion}.", %{
-              profile: profile(activity),
+              profile: profile,
-              discussion: title(activity)
+              discussion: title
             }),
           url: nil
         }
@@ -79,6 +91,8 @@ defmodule Mobilizon.Service.Activity.Renderer.Discussion do
     |> URI.decode()
   end
 
-  defp profile(activity), do: Actor.display_name_and_username(activity.author)
+  defp profile(%Activity{author: author}), do: Actor.display_name_and_username(author)
-  defp title(activity), do: activity.subject_params["discussion_title"]
+
+  defp title(%Activity{subject_params: %{"discussion_title" => discussion_title}}),
+    do: discussion_title
 end

lib/service/activity/utils.ex

@@ -27,4 +27,13 @@ defmodule Mobilizon.Service.Activity.Utils do
   end
 
   defp transform_value(value), do: value
+
+  @spec maybe_inserted_at :: map()
+  def maybe_inserted_at do
+    if Application.fetch_env!(:mobilizon, :env) == :test do
+      %{}
+    else
+      %{"inserted_at" => DateTime.utc_now()}
+    end
+  end
 end

lib/service/workers/legacy_notifier_builder.ex

@@ -15,7 +15,7 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilder do
       activity = build_activity(args)
 
       args
-      |> users_to_notify(args["author_id"])
+      |> users_to_notify(author_id: args["author_id"], group_id: Map.get(args, "group_id"))
       |> Enum.each(&Notifier.notify(&1, activity, single_activity: true))
     end
   end
@@ -35,12 +35,21 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilder do
     }
   end
 
-  @spec users_to_notify(map(), integer() | String.t()) :: list(Users.t())
+  @spec users_to_notify(map(), Keyword.t()) :: list(Users.t())
   defp users_to_notify(
          %{"subject" => "event_comment_mention", "mentions" => mentionned_actor_ids},
-         author_id
+         options
        ) do
-    users_from_actor_ids(mentionned_actor_ids, author_id)
+    users_from_actor_ids(mentionned_actor_ids, Keyword.fetch!(options, :author_id))
+  end
+
+  defp users_to_notify(
+         %{"subject" => "discussion_mention", "mentions" => mentionned_actor_ids},
+         options
+       ) do
+    mentionned_actor_ids
+    |> Enum.filter(&Actors.is_member?(&1, Keyword.fetch!(options, :group_id)))
+    |> users_from_actor_ids(Keyword.fetch!(options, :author_id))
   end
 
   defp users_to_notify(
@@ -48,13 +57,13 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilder do
            "subject" => "participation_event_comment",
            "subject_params" => subject_params
          },
-         author_id
+         options
        ) do
     subject_params
     |> Map.get("event_id")
     |> Events.list_actors_participants_for_event()
     |> Enum.map(& &1.id)
-    |> users_from_actor_ids(author_id)
+    |> users_from_actor_ids(Keyword.fetch!(options, :author_id))
   end
 
   @spec users_from_actor_ids(list(), integer() | String.t()) :: list(Users.t())

test/service/activity/discussion_test.exs

@@ -0,0 +1,119 @@
+defmodule Mobilizon.Service.Activity.DiscussionTest do
+  @moduledoc """
+  Test the Discussion activity provider module
+  """
+
+  alias Mobilizon.Actors.Actor
+  alias Mobilizon.Discussions.{Comment, Discussion}
+  alias Mobilizon.Mention
+  alias Mobilizon.Service.Activity.Discussion, as: DiscussionActivity
+  alias Mobilizon.Service.Workers.{ActivityBuilder, LegacyNotifierBuilder}
+  alias Mobilizon.Users.User
+
+  use Mobilizon.DataCase, async: true
+  use Oban.Testing, repo: Mobilizon.Storage.Repo
+  import Mobilizon.Factory
+
+  describe "handle discussion with mentions" do
+    test "with no mentions" do
+      %Comment{} = comment = insert(:comment)
+
+      %Discussion{
+        id: discussion_id,
+        actor_id: group_id,
+        creator_id: author_id,
+        title: discussion_title,
+        slug: discussion_slug
+      } = discussion = insert(:discussion)
+
+      assert {:ok, _} =
+               DiscussionActivity.insert_activity(%Discussion{discussion | last_comment: comment},
+                 subject: "discussion_created"
+               )
+
+      refute_enqueued(
+        worker: LegacyNotifierBuilder,
+        args: %{op: :discussion_mention}
+      )
+
+      assert_enqueued(
+        worker: ActivityBuilder,
+        args: %{
+          "group_id" => group_id,
+          "author_id" => author_id,
+          "object_id" => to_string(discussion_id),
+          "object_type" => "discussion",
+          "op" => "build_activity",
+          "subject" => "discussion_created",
+          "subject_params" => %{
+            "discussion_slug" => discussion_slug,
+            "discussion_title" => discussion_title
+          },
+          "type" => "discussion"
+        }
+      )
+    end
+
+    test "with some mentions" do
+      %User{} = user = insert(:user)
+      %Actor{id: actor_id} = actor = insert(:actor, user: user)
+
+      %Comment{actor_id: author_id} = comment = insert(:comment, text: "Hey @you")
+
+      comment = %Comment{
+        comment
+        | mentions: [
+            %Mention{actor: actor, comment: comment, actor_id: actor_id}
+          ]
+      }
+
+      %Discussion{
+        id: discussion_id,
+        actor_id: group_id,
+        creator_id: discussion_author_id,
+        title: discussion_title,
+        slug: discussion_slug
+      } = discussion = insert(:discussion)
+
+      assert {:ok, _} =
+               DiscussionActivity.insert_activity(%Discussion{discussion | last_comment: comment},
+                 subject: "discussion_created"
+               )
+
+      assert_enqueued(
+        worker: LegacyNotifierBuilder,
+        args: %{
+          "author_id" => author_id,
+          "group_id" => group_id,
+          "mentions" => [actor_id],
+          "object_id" => to_string(discussion_id),
+          "object_type" => "discussion",
+          "op" => "legacy_notify",
+          "subject" => "discussion_mention",
+          "subject_params" => %{
+            "discussion_slug" => discussion_slug,
+            "discussion_title" => discussion_title
+          },
+          "type" => "discussion"
+        }
+      )
+
+      assert_enqueued(
+        worker: ActivityBuilder,
+        args: %{
+          "group_id" => group_id,
+          "author_id" => discussion_author_id,
+          "object_id" => to_string(discussion_id),
+          "object_type" => "discussion",
+          "op" => "build_activity",
+          "subject" => "discussion_created",
+          "subject_params" => %{
+            "discussion_slug" => discussion_slug,
+            "discussion_title" => discussion_title
+          },
+          "type" => "discussion"
+        }
+      )
+    end
+  end
+end

test/service/workers/legacy_notifier_builder_test.exs

@@ -5,7 +5,7 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilderTest do
 
   alias Mobilizon.Activities.Activity
   alias Mobilizon.Actors.Actor
-  alias Mobilizon.Discussions.Comment
+  alias Mobilizon.Discussions.{Comment, Discussion}
   alias Mobilizon.Events.Event
   alias Mobilizon.Service.Notifier.Mock, as: NotifierMock
   alias Mobilizon.Service.Workers.LegacyNotifierBuilder
@@ -26,7 +26,7 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilderTest do
     :ok
   end
 
-  @mentionned %{
+  @comment_mentionned %{
     "type" => "comment",
     "subject" => "event_comment_mention",
     "object_type" => "comment",
@@ -34,6 +34,14 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilderTest do
     "op" => "legacy_notify"
   }
 
+  @discussion_mentionned %{
+    "type" => "discussion",
+    "subject" => "discussion_mention",
+    "object_type" => "discussion",
+    "inserted_at" => DateTime.utc_now(),
+    "op" => "legacy_notify"
+  }
+
   @announcement %{
     "type" => "comment",
     "subject" => "participation_event_comment",
@@ -55,7 +63,7 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilderTest do
       %Comment{id: comment_id} = insert(:comment, event: event, actor: actor)
 
       args =
-        Map.merge(@mentionned, %{
+        Map.merge(@comment_mentionned, %{
           "subject_params" => %{
             "event_uuid" => uuid,
             "event_title" => title
@@ -93,7 +101,7 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilderTest do
       %Comment{id: comment_id} = insert(:comment, event: event, actor: actor)
 
       args =
-        Map.merge(@mentionned, %{
+        Map.merge(@comment_mentionned, %{
           "subject_params" => %{
             "event_uuid" => uuid,
             "event_title" => title
@@ -194,4 +202,135 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilderTest do
       assert :ok == LegacyNotifierBuilder.perform(%Oban.Job{args: args})
     end
   end
+
+  describe "Generates a discussion mention notification " do
+    test "not if the actor is remote" do
+      %User{} = user1 = insert(:user)
+
+      %Actor{id: actor_id} = actor = insert(:actor, user: user1)
+      %Actor{id: actor_id_2} = insert(:actor, domain: "remote.tld", user: nil)
+
+      %Comment{id: comment_id} = insert(:comment, actor: actor)
+
+      %Discussion{
+        actor_id: group_id,
+        title: discussion_title,
+        slug: discussion_slug
+      } = insert(:discussion)
+
+      args =
+        Map.merge(@discussion_mentionned, %{
+          "subject_params" => %{
+            "discussion_slug" => discussion_slug,
+            "discussion_title" => discussion_title
+          },
+          "author_id" => actor_id,
+          "group_id" => group_id,
+          "object_id" => to_string(comment_id),
+          "mentions" => [actor_id_2]
+        })
+
+      NotifierMock
+      |> expect(:ready?, 0, fn -> true end)
+      |> expect(:send, 0, fn %User{},
+                             %Activity{
+                               type: :discussion,
+                               subject: :discussion_mention,
+                               object_type: :discussion
+                             },
+                             [single_activity: true] ->
+        {:ok, :sent}
+      end)
+
+      assert :ok == LegacyNotifierBuilder.perform(%Oban.Job{args: args})
+    end
+
+    test "not if the actor is not a member" do
+      %User{} = user1 = insert(:user)
+
+      %Actor{id: actor_id} = actor = insert(:actor, user: user1)
+      %Actor{id: actor_id_2} = insert(:actor)
+
+      %Comment{id: comment_id} = insert(:comment, actor: actor)
+
+      %Discussion{
+        actor_id: group_id,
+        title: discussion_title,
+        slug: discussion_slug
+      } = insert(:discussion)
+
+      args =
+        Map.merge(@discussion_mentionned, %{
+          "subject_params" => %{
+            "discussion_slug" => discussion_slug,
+            "discussion_title" => discussion_title
+          },
+          "author_id" => actor_id,
+          "group_id" => group_id,
+          "object_id" => to_string(comment_id),
+          "mentions" => [actor_id_2]
+        })
+
+      NotifierMock
+      |> expect(:ready?, 0, fn -> true end)
+      |> expect(:send, 0, fn %User{},
+                             %Activity{
+                               type: :discussion,
+                               subject: :discussion_mention,
+                               object_type: :discussion
+                             },
+                             [single_activity: true] ->
+        {:ok, :sent}
+      end)
+
+      assert :ok == LegacyNotifierBuilder.perform(%Oban.Job{args: args})
+    end
+
+    test "if the actor mentionned is local and a member" do
+      %User{} = user1 = insert(:user)
+      %User{} = user2 = insert(:user)
+      %Setting{} = settings2 = insert(:settings, user: user2, user_id: user2.id)
+      user2 = %User{user2 | settings: settings2}
+
+      %Actor{id: actor_id} = actor = insert(:actor, user: user1)
+      %Actor{id: actor_id_2} = actor2 = insert(:actor, user: user2)
+      %Actor{} = group = insert(:group)
+
+      insert(:member, actor: actor2, parent: group, role: :member)
+
+      %Comment{id: comment_id} = insert(:comment, actor: actor)
+
+      %Discussion{
+        actor_id: group_id,
+        title: discussion_title,
+        slug: discussion_slug
+      } = insert(:discussion, actor: group)
+
+      args =
+        Map.merge(@discussion_mentionned, %{
+          "subject_params" => %{
+            "discussion_slug" => discussion_slug,
+            "discussion_title" => discussion_title
+          },
+          "author_id" => actor_id,
+          "group_id" => group_id,
+          "object_id" => to_string(comment_id),
+          "mentions" => [actor_id_2]
+        })
+
+      NotifierMock
+      |> expect(:ready?, fn -> true end)
+      |> expect(:send, fn %User{},
+                          %Activity{
+                            type: :discussion,
+                            subject: :discussion_mention,
+                            object_type: :discussion
+                          },
+                          [single_activity: true] ->
+        {:ok, :sent}
+      end)
+
+      assert :ok == LegacyNotifierBuilder.perform(%Oban.Job{args: args})
+    end
+  end
 end