Fix malformed is_active qualifiers in User.Query usages
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 860b5c7804. 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)
This commit is contained in:
parent
c2bdfb6a34
commit
f5898ab0fb
4 changed files with 53 additions and 8 deletions
|
|
@ -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
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue