From 0b49021f8b42f7c19fd03980a2564a8ab201824d Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Fri, 6 May 2022 16:36:04 +0200 Subject: [PATCH] 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 --- lib/mobilizon/actors/actor.ex | 29 +++++++- lib/web/email/follow.ex | 18 +++-- .../templates/email/instance_follow.html.heex | 21 ++---- .../templates/email/instance_follow.text.eex | 4 +- test/mobilizon/actors/actor_test.exs | 70 +++++++++++++++++++ 5 files changed, 117 insertions(+), 25 deletions(-) create mode 100644 test/mobilizon/actors/actor_test.exs diff --git a/lib/mobilizon/actors/actor.ex b/lib/mobilizon/actors/actor.ex index 1efa5aacc..4320d3562 100644 --- a/lib/mobilizon/actors/actor.ex +++ b/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 - }) do + domain: domain + }) + 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 diff --git a/lib/web/email/follow.ex b/lib/web/email/follow.ex index 5da1c80f7..fab391b9f 100644 --- a/lib/web/email/follow.ex +++ b/lib/web/email/follow.ex @@ -44,11 +44,19 @@ defmodule Mobilizon.Web.Email.Follow do subject = if follower_type == :Application do - gettext( - "Instance %{name} (%{domain}) requests to follow your instance", - name: follower.name, - domain: follower.domain - ) + # Mastodon instance actor has no name and an username equal to the domain + if is_nil(follower.name) and follower.preferred_username == follower.domain do + gettext( + "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", diff --git a/lib/web/templates/email/instance_follow.html.heex b/lib/web/templates/email/instance_follow.html.heex index 87f4ca93a..f2e551f71 100644 --- a/lib/web/templates/email/instance_follow.html.heex +++ b/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;" >

- <%= if @follower.type == :Application do %> - <%= gettext("%{name} (%{domain}) just requested to follow your instance.", - name: @follower.name, - domain: @follower.domain - ) - |> raw %> - <% else %> - <%= gettext("%{name} just requested to follow your instance.", - name: Mobilizon.Actors.Actor.display_name_and_username(@follower) - ) - |> raw %> - <% end %> + <%= gettext("%{name} just requested to follow your instance.", + name: Mobilizon.Actors.Actor.display_name_and_username(@follower) + ) + |> raw %>
<%= if @follower.type == :Application do %> <%= gettext("If you accept, this instance will receive all of your public events.") %> @@ -74,9 +66,8 @@ >

<%= 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 + "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) ) %>

diff --git a/lib/web/templates/email/instance_follow.text.eex b/lib/web/templates/email/instance_follow.text.eex index 642dd32bf..930a7ed2d 100644 --- a/lib/web/templates/email/instance_follow.text.eex +++ b/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 %> diff --git a/test/mobilizon/actors/actor_test.exs b/test/mobilizon/actors/actor_test.exs new file mode 100644 index 000000000..f48c5971e --- /dev/null +++ b/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