Fix admin notification e-mails from instance follow for Mastodon

instances

Show an appropriate name in the body of the mail

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

lib/mobilizon/actors/actor.ex

@@ -224,10 +224,23 @@ defmodule Mobilizon.Actors.Actor do
     preferred_username_and_domain(actor)
   end
 
-  def display_name_and_username(%__MODULE__{name: name} = actor) do
+  def display_name_and_username(%__MODULE__{
+        type: :Application,
+        name: name,
+        preferred_username: "relay",
+        domain: domain
+      })
+      when domain not in [nil, ""] and name not in [nil, ""] do
+    "#{name} (#{domain})"
+  end
+
+  def display_name_and_username(%__MODULE__{name: name, preferred_username: username} = actor)
+      when username not in [nil, ""] do
     "#{name} (@#{preferred_username_and_domain(actor)})"
   end
 
+  def display_name_and_username(_), do: nil
+
   @doc """
   Returns the preferred username with the eventual @domain suffix if it's
   a distant actor.
@@ -235,8 +248,18 @@ defmodule Mobilizon.Actors.Actor do
   @spec preferred_username_and_domain(t) :: String.t()
   def preferred_username_and_domain(%__MODULE__{
         preferred_username: preferred_username,
-        domain: nil
+        domain: domain
-      }) do
+      })
+      when domain in [nil, ""] do
+    preferred_username
+  end
+
+  def preferred_username_and_domain(%__MODULE__{
+        type: :Application,
+        preferred_username: preferred_username,
+        domain: domain
+      })
+      when not is_nil(domain) and preferred_username == domain do
     preferred_username
   end
 

lib/web/email/follow.ex

@@ -44,11 +44,19 @@ defmodule Mobilizon.Web.Email.Follow do
 
     subject =
       if follower_type == :Application do
-        gettext(
+        # Mastodon instance actor has no name and an username equal to the domain
-          "Instance %{name} (%{domain}) requests to follow your instance",
+        if is_nil(follower.name) and follower.preferred_username == follower.domain do
-          name: follower.name,
+          gettext(
-          domain: follower.domain
+            "Instance %{domain} requests to follow your instance",
-        )
+            domain: follower.domain
+          )
+        else
+          gettext(
+            "Instance %{name} (%{domain}) requests to follow your instance",
+            name: follower.name || follower.preferred_username,
+            domain: follower.domain
+          )
+        end
       else
         gettext(
           "%{name} requests to follow your instance",

lib/web/templates/email/instance_follow.html.heex

@@ -44,18 +44,10 @@
           style="padding: 20px 30px 0px 30px; color: #474467; font-family: 'Roboto', Helvetica, Arial, sans-serif; font-size: 18px; font-weight: 400; line-height: 25px;"
         >
           <p style="margin: 0;">
-            <%= if @follower.type == :Application do %>
+            <%= gettext("<b>%{name}</b> just requested to follow your instance.",
-              <%= gettext("<b>%{name} (%{domain})</b> just requested to follow your instance.",
+              name: Mobilizon.Actors.Actor.display_name_and_username(@follower)
-                name: @follower.name,
+            )
-                domain: @follower.domain
+            |> raw %>
-              )
-              |> raw %>
-            <% else %>
-              <%= gettext("<b>%{name}</b> just requested to follow your instance.",
-                name: Mobilizon.Actors.Actor.display_name_and_username(@follower)
-              )
-              |> raw %>
-            <% end %>
             <br />
             <%= if @follower.type == :Application do %>
               <%= gettext("If you accept, this instance will receive all of your public events.") %>
@@ -74,9 +66,8 @@
           >
             <p style="margin: 0;">
               <%= gettext(
-                "Note: %{name} (%{domain}) following you doesn't necessarily imply that you follow this instance, but you can ask to follow them too.",
+                "Note: %{name} following you doesn't necessarily imply that you follow this instance, but you can ask to follow them too.",
-                name: @follower.name,
+                name: Mobilizon.Actors.Actor.display_name_and_username(@follower)
-                domain: @follower.domain
               ) %>
             </p>
           </td>

lib/web/templates/email/instance_follow.text.eex

@@ -2,9 +2,9 @@
 
 ==
 
-<%= if @follower.type == :Application do %><%= gettext "%{name} (%{domain}) just requested to follow your instance.", name: @follower.name, domain: @follower.domain %><% else %><%= gettext "%{name} just requested to follow your instance.", name: Mobilizon.Actors.Actor.display_name_and_username(@follower) %><% end %>
+<%= if @follower.type == :Application do %><%= gettext "%{name} just requested to follow your instance.", name: Mobilizon.Actors.Actor.display_name_and_username(@follower) %><% end %>
 <%= if @follower.type == :Application do %><%= gettext "If you accept, this instance will receive all of your public events." %><% else %><%= gettext "If you accept, this profile will receive all of your public events." %><% end %>
-<%= if @follower.type == :Application do %><%= gettext "Note: %{name} (%{domain}) following you doesn't necessarily imply that you follow this instance, but you can ask to follow them too.", name: @follower.name, domain: @follower.domain %><% end %>
+<%= if @follower.type == :Application do %><%= gettext "Note: %{name} following you doesn't necessarily imply that you follow this instance, but you can ask to follow them too.", name: Mobilizon.Actors.Actor.display_name_and_username(@follower) %><% end %>
 
 <%= if @follower.type == :Application do %><%= gettext "To accept this invitation, head over to the instance's admin settings." %><% else %><%= gettext "To accept this invitation, head over to the profile's admin page." %><% end %>
 <%= if @follower.type == :Application do %><%= "#{Mobilizon.Web.Endpoint.url()}/settings/admin/relays/followers" %><% else %><%= "#{Mobilizon.Web.Endpoint.url()}/settings/admin/profiles/#{@follower.id}" %><% end %>

test/mobilizon/actors/actor_test.exs

@@ -0,0 +1,70 @@
+defmodule Mobilizon.ActorTest do
+  use Mobilizon.DataCase
+  alias Mobilizon.Actors.Actor
+
+  describe "display_name_and_username/1" do
+    test "returns correctly if everything is given" do
+      assert "hello (@someone@remote.tld)" ==
+               Actor.display_name_and_username(%Actor{
+                 name: "hello",
+                 domain: "remote.tld",
+                 preferred_username: "someone"
+               })
+    end
+
+    test "returns for a local actor" do
+      assert "hello (@someone)" ==
+               Actor.display_name_and_username(%Actor{
+                 name: "hello",
+                 domain: nil,
+                 preferred_username: "someone"
+               })
+
+      assert "hello (@someone)" ==
+               Actor.display_name_and_username(%Actor{
+                 name: "hello",
+                 domain: "",
+                 preferred_username: "someone"
+               })
+    end
+
+    test "returns nil if the name is all that's given" do
+      assert nil == Actor.display_name_and_username(%Actor{name: "hello"})
+    end
+
+    test "returns with just the username if that's all that's given" do
+      assert "someone" ==
+               Actor.display_name_and_username(%Actor{preferred_username: "someone"})
+    end
+
+    test "returns an appropriate name for a Mobilizon instance actor" do
+      assert "My Mobilizon Instance (remote.tld)" ==
+               Actor.display_name_and_username(%Actor{
+                 name: "My Mobilizon Instance",
+                 domain: "remote.tld",
+                 preferred_username: "relay",
+                 type: :Application
+               })
+    end
+
+    test "returns an appropriate name for a Mastodon instance actor" do
+      assert "remote.tld" ==
+               Actor.display_name_and_username(%Actor{
+                 name: nil,
+                 domain: "remote.tld",
+                 preferred_username: "remote.tld",
+                 type: :Application
+               })
+    end
+  end
+
+  describe "display_name/1" do
+    test "with name" do
+      assert "hello" == Actor.display_name(%Actor{preferred_username: "someone", name: "hello"})
+    end
+
+    test "without name" do
+      assert "someone" == Actor.display_name(%Actor{preferred_username: "someone"})
+    end
+  end
+end