fed/{in,out}: federate voter count for polls
This will also fix non-sensical values being reported in our API for remote multi-selection polls, since we now actually know the authorative voter count. This was inspired by5aa3c8a06eand tests are mostly taken from727e9e7749but tweaked to be more accurate, remove redundancy and Akkoma API. However, these Pleroma commits were incomplete and buggy, e.g. miscalculating even local voter counts, missing a JSON-LD definition and more, thus this is a greatly reworked version. Conceptually we should also decrement the voter count when a vote is retracted, however we currently do not handle vote retractions at all and thus this is omitted here. Fixes: #485 Co-authored-by: nicole mikołajczyk <git@mkljczk.pl> Co-authored-by: Lain Soykaf <lain@lain.com> Co-authored-by: Oneric <oneric@oneric.stub>
This commit is contained in:
parent
c92456bf78
commit
e13f025e22
12 changed files with 251 additions and 1 deletions
|
|
@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
|
|||
|
||||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
|
||||
|
||||
## Unreleased
|
||||
|
||||
## Added
|
||||
- federated voter count of polls is now parsed and federated out too;
|
||||
this fixes vote percetanges for new and refreshed remote multi-selection polls
|
||||
|
||||
|
||||
## 2026.05 (3.19.0)
|
||||
|
||||
### General note
|
||||
|
|
|
|||
|
|
@ -336,12 +336,21 @@ defmodule Pleroma.Object do
|
|||
option
|
||||
end)
|
||||
|
||||
voters = [actor | object.data["voters"] || []] |> Enum.uniq()
|
||||
old_voters = object.data["voters"] || []
|
||||
old_voters_count = object.data["votersCount"] || length(old_voters)
|
||||
|
||||
{voters, voters_count} =
|
||||
if actor in old_voters do
|
||||
{old_voters, old_voters_count}
|
||||
else
|
||||
{[actor | old_voters], old_voters_count + 1}
|
||||
end
|
||||
|
||||
data =
|
||||
object.data
|
||||
|> Map.put(key, options)
|
||||
|> Map.put("voters", voters)
|
||||
|> Map.put("votersCount", voters_count)
|
||||
|
||||
object
|
||||
|> Object.change(%{data: data})
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
|
||||
defmodule Pleroma.Object.Updater do
|
||||
alias Pleroma.Maps
|
||||
require Pleroma.Constants
|
||||
|
||||
def update_content_fields(orig_object_data, updated_object) do
|
||||
|
|
@ -110,6 +111,7 @@ defmodule Pleroma.Object.Updater do
|
|||
# Choices are the same, but counts are different
|
||||
to_be_updated
|
||||
|> Map.put(key, updated_object[key])
|
||||
|> Maps.put_if_present("votersCount", updated_object["votersCount"])
|
||||
else
|
||||
# Choices (or vote type) have changed, do not allow this
|
||||
_ -> to_be_updated
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.QuestionValidator do
|
|||
end
|
||||
|
||||
field(:closed, ObjectValidators.DateTime)
|
||||
field(:votersCount, :integer)
|
||||
field(:voters, {:array, ObjectValidators.ObjectID}, default: [])
|
||||
field(:nonAnonymous, :boolean)
|
||||
embeds_many(:anyOf, QuestionOptionsValidator)
|
||||
|
|
|
|||
|
|
@ -811,6 +811,16 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
|
|||
end
|
||||
end
|
||||
|
||||
defp set_voters_count(%{"votersCount" => n} = obj) when is_integer(n) do
|
||||
obj
|
||||
end
|
||||
|
||||
defp set_voters_count(%{"voters" => voters} = obj) when is_list(voters) do
|
||||
Map.put_new(obj, "votersCount", length(voters))
|
||||
end
|
||||
|
||||
defp set_voters_count(obj), do: obj
|
||||
|
||||
# Prepares the object of an outgoing create activity.
|
||||
def prepare_object(object) do
|
||||
object
|
||||
|
|
@ -823,6 +833,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
|
|||
|> set_reply_to_uri
|
||||
|> set_quote_url()
|
||||
|> set_replies
|
||||
|> set_voters_count()
|
||||
|> strip_internal_fields
|
||||
|> strip_internal_tags
|
||||
|> set_type
|
||||
|
|
|
|||
|
|
@ -105,6 +105,10 @@ defmodule Pleroma.Web.ActivityPub.Utils do
|
|||
"https://purl.archive.org/socialweb/webfinger",
|
||||
%{
|
||||
"@language" => "und",
|
||||
# More Mastodon extensions not included in litepub
|
||||
# (The toot: prefix is already defined in the litepub schema)
|
||||
"votersCount" => "toot:votersCount",
|
||||
# Further verbose definitions
|
||||
"htmlMfm" => "https://w3id.org/fep/c16b#htmlMfm",
|
||||
"sm" => "http://smithereen.software/ns#",
|
||||
"nonAnonymous" => "sm:nonAnonymous"
|
||||
|
|
|
|||
|
|
@ -157,6 +157,7 @@ defmodule Pleroma.Web.CommonAPI.Utils do
|
|||
"type" => "Question",
|
||||
key => option_notes,
|
||||
"closed" => end_time,
|
||||
"votersCount" => 0,
|
||||
"nonAnonymous" => false
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -85,6 +85,8 @@ defmodule Pleroma.Web.MastodonAPI.PollView do
|
|||
nil
|
||||
end
|
||||
|
||||
defp voters_count(_multiple, %{data: %{"votersCount" => count}}), do: count
|
||||
|
||||
defp voters_count(_multiple, %{data: %{"voters" => voters}}) when is_list(voters) do
|
||||
length(voters)
|
||||
end
|
||||
|
|
|
|||
|
|
@ -0,0 +1,29 @@
|
|||
defmodule Pleroma.Repo.Migrations.SanitizePollsVotersCount do
|
||||
use Ecto.Migration
|
||||
|
||||
def up() do
|
||||
"""
|
||||
UPDATE objects
|
||||
SET data = jsonb_set(
|
||||
data,
|
||||
'{votersCount}'::text[],
|
||||
to_jsonb(
|
||||
CASE
|
||||
WHEN jsonb_typeof(data->'voters') = 'array' THEN jsonb_array_length(data->'voters')
|
||||
ELSE 0
|
||||
END
|
||||
),
|
||||
TRUE
|
||||
)
|
||||
WHERE data->>'type' = 'Question' AND
|
||||
COALESCE(jsonb_typeof(data->'votersCount'), 'NULL') <> 'number'
|
||||
;
|
||||
"""
|
||||
|> execute()
|
||||
end
|
||||
|
||||
def down() do
|
||||
# no need to revert
|
||||
:ok
|
||||
end
|
||||
end
|
||||
|
|
@ -170,4 +170,23 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.QuestionHandlingTest do
|
|||
|
||||
assert {:ok, %Activity{local: false}} = Transmogrifier.handle_incoming(data)
|
||||
end
|
||||
|
||||
test "it displays voters count for a poll" do
|
||||
user = insert(:user)
|
||||
other_user = insert(:user)
|
||||
|
||||
{:ok, activity} =
|
||||
CommonAPI.post(user, %{
|
||||
status: "???",
|
||||
poll: %{expires_in: 10, options: ["yes", "no"]}
|
||||
})
|
||||
|
||||
object = Object.normalize(activity, fetch: false)
|
||||
{:ok, _, _} = CommonAPI.vote(other_user, object, [1])
|
||||
|
||||
{:ok, modified} = Transmogrifier.prepare_outgoing(activity.data)
|
||||
|
||||
refute Map.has_key?(modified["object"], "voters")
|
||||
assert modified["object"]["votersCount"] == 1
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -146,6 +146,7 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do
|
|||
"https://purl.archive.org/socialweb/webfinger",
|
||||
%{
|
||||
"@language" => "und",
|
||||
"votersCount" => "toot:votersCount",
|
||||
"htmlMfm" => "https://w3id.org/fep/c16b#htmlMfm",
|
||||
"nonAnonymous" => "sm:nonAnonymous",
|
||||
"sm" => "http://smithereen.software/ns#"
|
||||
|
|
|
|||
|
|
@ -190,4 +190,168 @@ defmodule Pleroma.Web.MastodonAPI.PollViewTest do
|
|||
]
|
||||
} = PollView.render("show.json", %{object: object})
|
||||
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, multiple: true}
|
||||
})
|
||||
|
||||
object = Object.normalize(activity, fetch: false)
|
||||
|
||||
voter = insert(:user)
|
||||
{:ok, _, object} = CommonAPI.vote(voter, object, [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, multiple: true}
|
||||
})
|
||||
|
||||
object = Object.normalize(activity, fetch: false)
|
||||
|
||||
voter = insert(:user)
|
||||
{:ok, _, object} = CommonAPI.vote(voter, object, [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, multiple: true}
|
||||
})
|
||||
|
||||
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, multiple: true}
|
||||
})
|
||||
|
||||
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(voter, object, [0, 2])
|
||||
|
||||
assert object.data["votersCount"] == 1
|
||||
assert length(object.data["voters"]) == 1
|
||||
|
||||
result = PollView.render("show.json", %{object: object})
|
||||
option_result = result[:options]
|
||||
|
||||
assert result[:votes_count] == 2
|
||||
assert result[:voters_count] == 1
|
||||
assert Enum.find(option_result, fn %{title: t} -> t == "a" end)[:votes_count] == 1
|
||||
assert Enum.find(option_result, fn %{title: t} -> t == "b" end)[:votes_count] == 0
|
||||
assert Enum.find(option_result, fn %{title: t} -> t == "c" end)[:votes_count] == 1
|
||||
end
|
||||
|
||||
test "honours pre-existing votersCount" 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)
|
||||
|
||||
chg = Ecto.Changeset.change(object, data: Map.put(object.data, "votersCount", 13))
|
||||
{:ok, object} = Repo.update(chg)
|
||||
|
||||
voter = insert(:user)
|
||||
{:ok, _, object} = CommonAPI.vote(voter, object, [0, 1])
|
||||
|
||||
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, multiple: true}
|
||||
})
|
||||
|
||||
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
|
||||
end
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue