From 4030837d91b7ff8525513589d5d810e9a1a6b959 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 25 May 2019 05:19:47 +0000 Subject: [PATCH 1/8] notification: add non_follows/non_followers notification control settings --- lib/pleroma/notification.ex | 32 +++++++++++++++++++++++++++++++- lib/pleroma/user/info.ex | 11 +++++++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 844264307..4095e0474 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -166,7 +166,17 @@ def get_notified_from_activity( def get_notified_from_activity(_, _local_only), do: [] def skip?(activity, user) do - [:self, :blocked, :local, :muted, :followers, :follows, :recently_followed] + [ + :self, + :blocked, + :local, + :muted, + :followers, + :follows, + :non_followers, + :non_follows, + :recently_followed + ] |> Enum.any?(&skip?(&1, activity, user)) end @@ -201,12 +211,32 @@ def skip?( User.following?(follower, user) end + def skip?( + :non_followers, + activity, + %{info: %{notification_settings: %{"non_followers" => false}}} = user + ) do + actor = activity.data["actor"] + follower = User.get_cached_by_ap_id(actor) + !User.following?(follower, user) + end + def skip?(:follows, activity, %{info: %{notification_settings: %{"follows" => false}}} = user) do actor = activity.data["actor"] followed = User.get_cached_by_ap_id(actor) User.following?(user, followed) end + def skip?( + :non_follows, + activity, + %{info: %{notification_settings: %{"non_follows" => false}}} = user + ) do + actor = activity.data["actor"] + followed = User.get_cached_by_ap_id(actor) + !User.following?(user, followed) + end + def skip?(:recently_followed, %{data: %{"type" => "Follow"}} = activity, user) do actor = activity.data["actor"] diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index 6397e2737..fb4cf3cc3 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -47,7 +47,14 @@ defmodule Pleroma.User.Info do field(:emoji, {:array, :map}, default: []) field(:notification_settings, :map, - default: %{"remote" => true, "local" => true, "followers" => true, "follows" => true} + default: %{ + "remote" => true, + "local" => true, + "followers" => true, + "follows" => true, + "non_follows" => true, + "non_followers" => true + } ) # Found in the wild @@ -71,7 +78,7 @@ def update_notification_settings(info, settings) do notification_settings = info.notification_settings |> Map.merge(settings) - |> Map.take(["remote", "local", "followers", "follows"]) + |> Map.take(["remote", "local", "followers", "follows", "non_follows", "non_followers"]) params = %{notification_settings: notification_settings} From 1542cccbbcc2935add83d1428b4cd9e4b146f1ec Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 25 May 2019 05:22:13 +0000 Subject: [PATCH 2/8] tests: chase notification setting changes --- test/web/mastodon_api/account_view_test.exs | 4 +++- test/web/twitter_api/util_controller_test.exs | 10 ++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/test/web/mastodon_api/account_view_test.exs b/test/web/mastodon_api/account_view_test.exs index aaf2261bb..6f8480ee2 100644 --- a/test/web/mastodon_api/account_view_test.exs +++ b/test/web/mastodon_api/account_view_test.exs @@ -81,7 +81,9 @@ test "Represent the user account for the account owner" do "remote" => true, "local" => true, "followers" => true, - "follows" => true + "follows" => true, + "non_follows" => true, + "non_followers" => true } privacy = user.info.default_scope diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index 2cd82b3e7..ca0b8cc26 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -110,8 +110,14 @@ test "it updates notification settings", %{conn: conn} do user = Repo.get(User, user.id) - assert %{"remote" => false, "local" => true, "followers" => false, "follows" => true} == - user.info.notification_settings + assert %{ + "remote" => false, + "local" => true, + "followers" => false, + "follows" => true, + "non_follows" => true, + "non_followers" => true + } == user.info.notification_settings end end From 0f7eeb0943472da80405639f17b9bc99e4fcd417 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 25 May 2019 05:25:40 +0000 Subject: [PATCH 3/8] tests: add tests for non-follows/non-followers settings --- test/notification_test.exs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/notification_test.exs b/test/notification_test.exs index 581db58a8..b54414dcd 100644 --- a/test/notification_test.exs +++ b/test/notification_test.exs @@ -113,6 +113,13 @@ test "it disables notifications from followers" do assert nil == Notification.create_notification(activity, followed) end + test "it disables notifications from non-followers" do + follower = insert(:user) + followed = insert(:user, info: %{notification_settings: %{"non_followers" => false}}) + {:ok, activity} = CommonAPI.post(follower, %{"status" => "hey @#{followed.nickname}"}) + assert nil == Notification.create_notification(activity, followed) + end + test "it disables notifications from people the user follows" do follower = insert(:user, info: %{notification_settings: %{"follows" => false}}) followed = insert(:user) @@ -122,6 +129,13 @@ test "it disables notifications from people the user follows" do assert nil == Notification.create_notification(activity, follower) end + test "it disables notifications from people the user does not follow" do + follower = insert(:user, info: %{notification_settings: %{"non_follows" => false}}) + followed = insert(:user) + {:ok, activity} = CommonAPI.post(followed, %{"status" => "hey @#{follower.nickname}"}) + assert nil == Notification.create_notification(activity, follower) + end + test "it doesn't create a notification for user if he is the activity author" do activity = insert(:note_activity) author = User.get_cached_by_ap_id(activity.data["actor"]) From 59a703fcbe6436c92d0e276caaf55f599c3165f4 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 25 May 2019 05:31:13 +0000 Subject: [PATCH 4/8] twitter api: user view: expose user notification settings under pleroma object --- lib/pleroma/web/twitter_api/views/user_view.ex | 7 +++++++ test/web/twitter_api/views/user_view_test.exs | 2 ++ 2 files changed, 9 insertions(+) diff --git a/lib/pleroma/web/twitter_api/views/user_view.ex b/lib/pleroma/web/twitter_api/views/user_view.ex index f0a4ddbd3..550f35f5f 100644 --- a/lib/pleroma/web/twitter_api/views/user_view.ex +++ b/lib/pleroma/web/twitter_api/views/user_view.ex @@ -121,6 +121,7 @@ defp do_render("user.json", %{user: user = %User{}} = assigns) do "tags" => user.tags } |> maybe_with_activation_status(user, for_user) + |> with_notification_settings(user, for_user) } |> maybe_with_user_settings(user, for_user) |> maybe_with_role(user, for_user) @@ -132,6 +133,12 @@ defp do_render("user.json", %{user: user = %User{}} = assigns) do end end + defp with_notification_settings(data, %User{id: user_id} = user, %User{id: user_id}) do + Map.put(data, "notification_settings", user.info.notification_settings) + end + + defp with_notification_settings(data, _, _), do: data + defp maybe_with_activation_status(data, user, %User{info: %{is_admin: true}}) do Map.put(data, "deactivated", user.info.deactivated) end diff --git a/test/web/twitter_api/views/user_view_test.exs b/test/web/twitter_api/views/user_view_test.exs index 74526673c..a48fc9b78 100644 --- a/test/web/twitter_api/views/user_view_test.exs +++ b/test/web/twitter_api/views/user_view_test.exs @@ -112,9 +112,11 @@ test "User exposes settings for themselves and only for themselves", %{user: use as_user = UserView.render("show.json", %{user: user, for: user}) assert as_user["default_scope"] == user.info.default_scope assert as_user["no_rich_text"] == user.info.no_rich_text + assert as_user["pleroma"]["notification_settings"] == user.info.notification_settings as_stranger = UserView.render("show.json", %{user: user}) refute as_stranger["default_scope"] refute as_stranger["no_rich_text"] + refute as_stranger["pleroma"]["notification_settings"] end test "A user for a given other follower", %{user: user} do From e7e2e7a1a633e2f7e493d040e290c931320d8cc8 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 25 May 2019 05:54:02 +0000 Subject: [PATCH 5/8] user info: allow formdata for notification settings like every other API --- lib/pleroma/user/info.ex | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index fb4cf3cc3..b0bfdf4f4 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -75,6 +75,11 @@ def set_activation_status(info, deactivated) do end def update_notification_settings(info, settings) do + settings = + settings + |> Enum.map(fn {k, v} -> {k, v in [true, "true", "True", "1"]} end) + |> Map.new() + notification_settings = info.notification_settings |> Map.merge(settings) From 5fbbc57c1b497f8d194f3bbdefd889f0a943525a Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sat, 25 May 2019 07:25:13 +0000 Subject: [PATCH 6/8] add migration to add notification settings to user accounts --- ...d_non_followers_fields_to_notification_settings.exs | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 priv/repo/migrations/20190525071417_add_non_follows_and_non_followers_fields_to_notification_settings.exs diff --git a/priv/repo/migrations/20190525071417_add_non_follows_and_non_followers_fields_to_notification_settings.exs b/priv/repo/migrations/20190525071417_add_non_follows_and_non_followers_fields_to_notification_settings.exs new file mode 100644 index 000000000..a88b0ea61 --- /dev/null +++ b/priv/repo/migrations/20190525071417_add_non_follows_and_non_followers_fields_to_notification_settings.exs @@ -0,0 +1,10 @@ +defmodule Pleroma.Repo.Migrations.AddNonFollowsAndNonFollowersFieldsToNotificationSettings do + use Ecto.Migration + + def change do + execute(""" + update users set info = jsonb_set(info, '{notification_settings}', '{"local": true, "remote": true, "follows": true, "followers": true, "non_follows": true, "non_followers": true}') + where local=true + """) + end +end From 750ede5764d30063587182696f7ebc50c05f8278 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sun, 26 May 2019 00:05:47 +0000 Subject: [PATCH 7/8] notification: remove local/remote match rules (too complicated) --- lib/pleroma/notification.ex | 7 ------- lib/pleroma/user/info.ex | 4 +--- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex index 4095e0474..35beffb87 100644 --- a/lib/pleroma/notification.ex +++ b/lib/pleroma/notification.ex @@ -169,7 +169,6 @@ def skip?(activity, user) do [ :self, :blocked, - :local, :muted, :followers, :follows, @@ -189,12 +188,6 @@ def skip?(:blocked, activity, user) do User.blocks?(user, %{ap_id: actor}) end - def skip?(:local, %{local: true}, %{info: %{notification_settings: %{"local" => false}}}), - do: true - - def skip?(:local, %{local: false}, %{info: %{notification_settings: %{"remote" => false}}}), - do: true - def skip?(:muted, activity, user) do actor = activity.data["actor"] diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index b0bfdf4f4..74573ba83 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -48,8 +48,6 @@ defmodule Pleroma.User.Info do field(:notification_settings, :map, default: %{ - "remote" => true, - "local" => true, "followers" => true, "follows" => true, "non_follows" => true, @@ -83,7 +81,7 @@ def update_notification_settings(info, settings) do notification_settings = info.notification_settings |> Map.merge(settings) - |> Map.take(["remote", "local", "followers", "follows", "non_follows", "non_followers"]) + |> Map.take(["followers", "follows", "non_follows", "non_followers"]) params = %{notification_settings: notification_settings} From 45e4642a58f5299d2cd3f142aea110a474eb477f Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Sun, 26 May 2019 00:20:54 +0000 Subject: [PATCH 8/8] tests: chase remote/local removal --- test/notification_test.exs | 27 ------------------- test/web/mastodon_api/account_view_test.exs | 2 -- test/web/twitter_api/util_controller_test.exs | 3 --- 3 files changed, 32 deletions(-) diff --git a/test/notification_test.exs b/test/notification_test.exs index b54414dcd..be292abd9 100644 --- a/test/notification_test.exs +++ b/test/notification_test.exs @@ -78,33 +78,6 @@ test "it doesn't create a notification for an activity from a muted thread" do assert nil == Notification.create_notification(activity, muter) end - test "it disables notifications from people on remote instances" do - user = insert(:user, info: %{notification_settings: %{"remote" => false}}) - other_user = insert(:user) - - create_activity = %{ - "@context" => "https://www.w3.org/ns/activitystreams", - "type" => "Create", - "to" => ["https://www.w3.org/ns/activitystreams#Public"], - "actor" => other_user.ap_id, - "object" => %{ - "type" => "Note", - "content" => "Hi @#{user.nickname}", - "attributedTo" => other_user.ap_id - } - } - - {:ok, %{local: false} = activity} = Transmogrifier.handle_incoming(create_activity) - assert nil == Notification.create_notification(activity, user) - end - - test "it disables notifications from people on the local instance" do - user = insert(:user, info: %{notification_settings: %{"local" => false}}) - other_user = insert(:user) - {:ok, activity} = CommonAPI.post(other_user, %{"status" => "hey @#{user.nickname}"}) - assert nil == Notification.create_notification(activity, user) - end - test "it disables notifications from followers" do follower = insert(:user) followed = insert(:user, info: %{notification_settings: %{"followers" => false}}) diff --git a/test/web/mastodon_api/account_view_test.exs b/test/web/mastodon_api/account_view_test.exs index 6f8480ee2..23f250990 100644 --- a/test/web/mastodon_api/account_view_test.exs +++ b/test/web/mastodon_api/account_view_test.exs @@ -78,8 +78,6 @@ test "Represent the user account for the account owner" do user = insert(:user) notification_settings = %{ - "remote" => true, - "local" => true, "followers" => true, "follows" => true, "non_follows" => true, diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index ca0b8cc26..cab9e5d90 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -102,7 +102,6 @@ test "it updates notification settings", %{conn: conn} do conn |> assign(:user, user) |> put("/api/pleroma/notification_settings", %{ - "remote" => false, "followers" => false, "bar" => 1 }) @@ -111,8 +110,6 @@ test "it updates notification settings", %{conn: conn} do user = Repo.get(User, user.id) assert %{ - "remote" => false, - "local" => true, "followers" => false, "follows" => true, "non_follows" => true,