From f5898ab0fb25cf1f7282fc7849303e0ec9696174 Mon Sep 17 00:00:00 2001 From: Oneric Date: Fri, 6 Mar 2026 00:00:00 +0000 Subject: [PATCH] Fix malformed is_active qualifiers in User.Query usages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While "is_active" is a property of users, it is not a recognised keyword for the User.Query helper; instead "deactivated" with negated value must be used. This arose because originally the user property was also called "deactivated" until it was renamed adn negated five years ago in 860b5c78048ede3597a02b6029634d74fd520204. This included renaiming the parameter in most but not all User.Query usages. Notably the parameter in User.get_followers_query was renamed but not in User.get_friends_query (friends == following). The accepted query parameter in User.Query however was not changed. This lead to the former mistakenly including deleted users causing too large values to be reported in the ActivityPub follower, but not following collection as reported in #1078. In Masto API responses filtering by `User.is_visible` weeded out the extra accounts before they got displayed to API users. On the surface it might seem logical to align the name of the User.Query parameter with the actual property name. However, User.Query already accepts an "active" parameter which is an alias for limiting to accounts which are neither deleted nor deactivated by moderators (both indicated by is_active) and also are not newly created account requests still pending an admin approval (is_approved) or necessary email confirmation (is_confirmed); in short as the alias suggests whether the account is active. Two highly similar parameter names like this would be much too confusing. The renamed "is_active" on the other hand does not actually suffice to say whether an account is actually active, only whether it has (not yet) ceased to be active (by its own volition or moderator action). Meaning its "new" name is actively misleading. Arguably the rename made things worse for no reason whatsoever and should not ever have happened. For now, we’ll just revert the incorrect query helper parameter renames. Fixes: https://akkoma.dev/AkkomaGang/akkoma/ issues/ 1078 (cherry picked from commit a899663ddc0e3a2084a239eabd79608a94ac5493) --- lib/mix/tasks/pleroma/email.ex | 2 +- lib/mix/tasks/pleroma/user.ex | 4 +-- lib/pleroma/user.ex | 10 ++++---- test/pleroma/user_test.exs | 45 ++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/lib/mix/tasks/pleroma/email.ex b/lib/mix/tasks/pleroma/email.ex index 37272c124..76725f65f 100644 --- a/lib/mix/tasks/pleroma/email.ex +++ b/lib/mix/tasks/pleroma/email.ex @@ -33,7 +33,7 @@ defmodule Mix.Tasks.Pleroma.Email do Pleroma.User.Query.build(%{ local: true, - is_active: true, + deactivated: false, is_confirmed: false, invisible: false }) diff --git a/lib/mix/tasks/pleroma/user.ex b/lib/mix/tasks/pleroma/user.ex index 929fa1717..1035f2fce 100644 --- a/lib/mix/tasks/pleroma/user.ex +++ b/lib/mix/tasks/pleroma/user.ex @@ -362,7 +362,7 @@ defmodule Mix.Tasks.Pleroma.User do Pleroma.User.Query.build(%{ local: true, - is_active: true, + deactivated: false, is_moderator: false, is_admin: false, invisible: false @@ -380,7 +380,7 @@ defmodule Mix.Tasks.Pleroma.User do Pleroma.User.Query.build(%{ local: true, - is_active: true, + deactivated: false, is_moderator: false, is_admin: false, invisible: false diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 3adc90c67..a8e08e335 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -969,7 +969,7 @@ defmodule Pleroma.User do candidates = Config.get([:instance, :autofollowed_nicknames]) autofollowed_users = - User.Query.build(%{nickname: candidates, local: true, is_active: true}) + User.Query.build(%{nickname: candidates, local: true, deactivated: false}) |> Repo.all() follow_all(user, autofollowed_users) @@ -1401,7 +1401,7 @@ defmodule Pleroma.User do @spec get_followers_query(User.t(), pos_integer() | nil) :: Ecto.Query.t() def get_followers_query(%User{} = user, nil) do - User.Query.build(%{followers: user, is_active: true}) + User.Query.build(%{followers: user, deactivated: false}) end def get_followers_query(%User{} = user, page) do @@ -1611,7 +1611,7 @@ defmodule Pleroma.User do @spec get_users_from_set([String.t()], keyword()) :: [User.t()] def get_users_from_set(ap_ids, opts \\ []) do local_only = Keyword.get(opts, :local_only, true) - criteria = %{ap_id: ap_ids, is_active: true} + criteria = %{ap_id: ap_ids, deactivated: false} criteria = if local_only, do: Map.put(criteria, :local, true), else: criteria User.Query.build(criteria) @@ -1622,7 +1622,7 @@ defmodule Pleroma.User do def get_recipients_from_activity(%Activity{recipients: to, actor: actor}) do to = [actor | to] - query = User.Query.build(%{recipients_from_activity: to, local: true, is_active: true}) + query = User.Query.build(%{recipients_from_activity: to, local: true, deactivated: false}) query |> Repo.all() @@ -2473,7 +2473,7 @@ defmodule Pleroma.User do @spec all_superusers() :: [User.t()] def all_superusers do - User.Query.build(%{super_users: true, local: true, is_active: true}) + User.Query.build(%{super_users: true, local: true, deactivated: false}) |> Repo.all() end diff --git a/test/pleroma/user_test.exs b/test/pleroma/user_test.exs index ec5a2a12a..86e873037 100644 --- a/test/pleroma/user_test.exs +++ b/test/pleroma/user_test.exs @@ -1197,11 +1197,32 @@ defmodule Pleroma.UserTest do res = User.get_followers(user) + assert length(res) == 2 assert Enum.member?(res, follower_one) assert Enum.member?(res, follower_two) refute Enum.member?(res, not_follower) end + test "does not return deactivated followers" do + user = insert(:user) + follower_one = insert(:user) + follower_two = insert(:user) + not_follower = insert(:user) + + {:ok, follower_one, user} = User.follow(follower_one, user) + {:ok, follower_two, user} = User.follow(follower_two, user) + + User.delete(follower_two) + follower_two = User.get_cached_by_id(follower_two.id) + + res = User.get_followers(user) + + assert length(res) == 1 + assert Enum.member?(res, follower_one) + refute Enum.member?(res, follower_two) + refute Enum.member?(res, not_follower) + end + test "gets all friends (followed users) for a given user" do user = insert(:user) followed_one = insert(:user) @@ -1215,10 +1236,34 @@ defmodule Pleroma.UserTest do followed_one = User.get_cached_by_ap_id(followed_one.ap_id) followed_two = User.get_cached_by_ap_id(followed_two.ap_id) + + assert length(res) == 2 assert Enum.member?(res, followed_one) assert Enum.member?(res, followed_two) refute Enum.member?(res, not_followed) end + + test "does not return deactivated followed users (friends)" do + user = insert(:user) + followed_one = insert(:user) + followed_two = insert(:user) + not_followed = insert(:user) + + {:ok, user, followed_one} = User.follow(user, followed_one) + {:ok, user, followed_two} = User.follow(user, followed_two) + + User.delete(followed_two) + + res = User.get_friends(user) + + followed_one = User.get_cached_by_id(followed_one.id) + followed_two = User.get_cached_by_id(followed_two.id) + + assert length(res) == 1 + assert Enum.member?(res, followed_one) + refute Enum.member?(res, followed_two) + refute Enum.member?(res, not_followed) + end end describe "updating note and follower count" do