Fix votersCount inflation in multiple-choice polls
increase_vote_count/3 was incrementing votersCount on every vote activity, causing inflation when a single voter picks multiple options. Now only increments when the actor is a new unique voter, and preserves existing votersCount otherwise. Also adds is_integer guard to voters_count/1 to handle nil safely, and adds tests for the voters_count clause ordering and edge cases.
This commit is contained in:
parent
aec0deef8b
commit
727e9e7749
3 changed files with 189 additions and 6 deletions
|
|
@ -372,13 +372,21 @@ defmodule Pleroma.Object do
|
||||||
option
|
option
|
||||||
end)
|
end)
|
||||||
|
|
||||||
voters = [actor | object.data["voters"] || []] |> Enum.uniq()
|
existing_voters = object.data["voters"] || []
|
||||||
|
voters = [actor | existing_voters] |> Enum.uniq()
|
||||||
|
new_voter? = actor not in existing_voters
|
||||||
|
existing_voters_count = object.data["votersCount"]
|
||||||
|
|
||||||
voters_count =
|
voters_count =
|
||||||
if Map.has_key?(object.data, "votersCount") do
|
cond do
|
||||||
object.data["votersCount"] + 1
|
is_integer(existing_voters_count) and new_voter? ->
|
||||||
else
|
existing_voters_count + 1
|
||||||
length(voters)
|
|
||||||
|
is_integer(existing_voters_count) ->
|
||||||
|
existing_voters_count
|
||||||
|
|
||||||
|
true ->
|
||||||
|
length(voters)
|
||||||
end
|
end
|
||||||
|
|
||||||
data =
|
data =
|
||||||
|
|
|
||||||
|
|
@ -71,7 +71,7 @@ defmodule Pleroma.Web.MastodonAPI.PollView do
|
||||||
end)
|
end)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp voters_count(%{data: %{"votersCount" => voters}}), do: voters
|
defp voters_count(%{data: %{"votersCount" => voters}}) when is_integer(voters), do: voters
|
||||||
|
|
||||||
defp voters_count(%{data: %{"voters" => [_ | _] = voters}}) do
|
defp voters_count(%{data: %{"voters" => [_ | _] = voters}}) do
|
||||||
length(voters)
|
length(voters)
|
||||||
|
|
|
||||||
|
|
@ -180,4 +180,179 @@ defmodule Pleroma.Web.MastodonAPI.PollViewTest do
|
||||||
|
|
||||||
assert result[:pleroma][:non_anonymous] == true
|
assert result[:pleroma][:non_anonymous] == true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "prefers votersCount over voters list when both are present" do
|
||||||
|
user = insert(:user)
|
||||||
|
|
||||||
|
{:ok, activity} =
|
||||||
|
CommonAPI.post(user, %{
|
||||||
|
status: "Which flavor?",
|
||||||
|
poll: %{options: ["chocolate", "vanilla"], expires_in: 20}
|
||||||
|
})
|
||||||
|
|
||||||
|
object = Object.normalize(activity, fetch: false)
|
||||||
|
|
||||||
|
voter = insert(:user)
|
||||||
|
{:ok, _, object} = CommonAPI.vote(object, voter, [0])
|
||||||
|
|
||||||
|
assert object.data["votersCount"] == 1
|
||||||
|
assert length(object.data["voters"]) == 1
|
||||||
|
|
||||||
|
object = %{
|
||||||
|
object
|
||||||
|
| data: Map.put(object.data, "votersCount", 42)
|
||||||
|
}
|
||||||
|
|
||||||
|
result = PollView.render("show.json", %{object: object})
|
||||||
|
|
||||||
|
assert result[:voters_count] == 42
|
||||||
|
end
|
||||||
|
|
||||||
|
test "falls back to voters list when votersCount is absent" do
|
||||||
|
user = insert(:user)
|
||||||
|
|
||||||
|
{:ok, activity} =
|
||||||
|
CommonAPI.post(user, %{
|
||||||
|
status: "Which flavor?",
|
||||||
|
poll: %{options: ["chocolate", "vanilla"], expires_in: 20}
|
||||||
|
})
|
||||||
|
|
||||||
|
object = Object.normalize(activity, fetch: false)
|
||||||
|
|
||||||
|
voter = insert(:user)
|
||||||
|
{:ok, _, object} = CommonAPI.vote(object, voter, [0])
|
||||||
|
|
||||||
|
assert length(object.data["voters"]) == 1
|
||||||
|
|
||||||
|
data = Map.delete(object.data, "votersCount")
|
||||||
|
object = %{object | data: data}
|
||||||
|
|
||||||
|
result = PollView.render("show.json", %{object: object})
|
||||||
|
|
||||||
|
assert result[:voters_count] == 1
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns 0 when both votersCount and voters are absent" do
|
||||||
|
user = insert(:user)
|
||||||
|
|
||||||
|
{:ok, activity} =
|
||||||
|
CommonAPI.post(user, %{
|
||||||
|
status: "Which flavor?",
|
||||||
|
poll: %{options: ["chocolate", "vanilla"], expires_in: 20}
|
||||||
|
})
|
||||||
|
|
||||||
|
object = Object.normalize(activity, fetch: false)
|
||||||
|
|
||||||
|
data =
|
||||||
|
object.data
|
||||||
|
|> Map.delete("votersCount")
|
||||||
|
|> Map.delete("voters")
|
||||||
|
|
||||||
|
object = %{object | data: data}
|
||||||
|
|
||||||
|
result = PollView.render("show.json", %{object: object})
|
||||||
|
|
||||||
|
assert result[:voters_count] == 0
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns 0 when voters list is empty" do
|
||||||
|
user = insert(:user)
|
||||||
|
|
||||||
|
{:ok, activity} =
|
||||||
|
CommonAPI.post(user, %{
|
||||||
|
status: "Which flavor?",
|
||||||
|
poll: %{options: ["chocolate", "vanilla"], expires_in: 20}
|
||||||
|
})
|
||||||
|
|
||||||
|
object = Object.normalize(activity, fetch: false)
|
||||||
|
|
||||||
|
data =
|
||||||
|
object.data
|
||||||
|
|> Map.delete("votersCount")
|
||||||
|
|> Map.put("voters", [])
|
||||||
|
|
||||||
|
object = %{object | data: data}
|
||||||
|
|
||||||
|
result = PollView.render("show.json", %{object: object})
|
||||||
|
|
||||||
|
assert result[:voters_count] == 0
|
||||||
|
end
|
||||||
|
|
||||||
|
test "does not inflate votersCount when same voter picks multiple options" do
|
||||||
|
user = insert(:user)
|
||||||
|
|
||||||
|
{:ok, activity} =
|
||||||
|
CommonAPI.post(user, %{
|
||||||
|
status: "Pick several",
|
||||||
|
poll: %{options: ["a", "b", "c"], expires_in: 20, multiple: true}
|
||||||
|
})
|
||||||
|
|
||||||
|
object = Object.normalize(activity, fetch: false)
|
||||||
|
|
||||||
|
voter = insert(:user)
|
||||||
|
{:ok, _, object} = CommonAPI.vote(object, voter, [0, 2])
|
||||||
|
|
||||||
|
assert object.data["votersCount"] == 1
|
||||||
|
assert length(object.data["voters"]) == 1
|
||||||
|
end
|
||||||
|
|
||||||
|
test "preserves votersCount from remote source when existing voter picks another option" do
|
||||||
|
user = insert(:user)
|
||||||
|
|
||||||
|
{:ok, activity} =
|
||||||
|
CommonAPI.post(user, %{
|
||||||
|
status: "Pick several",
|
||||||
|
poll: %{options: ["a", "b"], expires_in: 20, multiple: true}
|
||||||
|
})
|
||||||
|
|
||||||
|
object = Object.normalize(activity, fetch: false)
|
||||||
|
|
||||||
|
voter = insert(:user)
|
||||||
|
{:ok, _, object} = CommonAPI.vote(object, voter, [0, 1])
|
||||||
|
|
||||||
|
object = %{object | data: Map.put(object.data, "votersCount", 14)}
|
||||||
|
|
||||||
|
result = PollView.render("show.json", %{object: object})
|
||||||
|
|
||||||
|
assert result[:voters_count] == 14
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns 0 when votersCount is explicitly 0" do
|
||||||
|
user = insert(:user)
|
||||||
|
|
||||||
|
{:ok, activity} =
|
||||||
|
CommonAPI.post(user, %{
|
||||||
|
status: "Pick one",
|
||||||
|
poll: %{options: ["a", "b"], expires_in: 20}
|
||||||
|
})
|
||||||
|
|
||||||
|
object = Object.normalize(activity, fetch: false)
|
||||||
|
|
||||||
|
object = %{object | data: Map.put(object.data, "votersCount", 0)}
|
||||||
|
|
||||||
|
result = PollView.render("show.json", %{object: object})
|
||||||
|
|
||||||
|
assert result[:voters_count] == 0
|
||||||
|
end
|
||||||
|
|
||||||
|
test "falls back to voters list when votersCount is nil" do
|
||||||
|
user = insert(:user)
|
||||||
|
|
||||||
|
{:ok, activity} =
|
||||||
|
CommonAPI.post(user, %{
|
||||||
|
status: "Pick one",
|
||||||
|
poll: %{options: ["a", "b"], expires_in: 20}
|
||||||
|
})
|
||||||
|
|
||||||
|
object = Object.normalize(activity, fetch: false)
|
||||||
|
|
||||||
|
voter = insert(:user)
|
||||||
|
{:ok, _, object} = CommonAPI.vote(object, voter, [0])
|
||||||
|
|
||||||
|
object = %{object | data: Map.put(object.data, "votersCount", nil)}
|
||||||
|
|
||||||
|
result = PollView.render("show.json", %{object: object})
|
||||||
|
|
||||||
|
assert result[:voters_count] == length(object.data["voters"])
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue