federate votersCount #1116

Yonle · 2026-05-04T18:35:21Z

Yonle commented

2026-05-04 18:35:21 +00:00

note: some patches are retrieved from upstream pleroma, so there might be some logic errors.

todo:

fix votersCount getting bumped on every answers
cleanup patches [1/2]
fix test

note: some patches are retrieved from upstream pleroma, so there might be some logic errors. todo: - [x] fix `votersCount` getting bumped on every answers - [ ] cleanup patches [1/2] - [ ] fix test

Yonle added 2 commits

2026-05-04 18:35:23 +00:00

Federate votersCount correctly 26843656d7

Cherry-picked-from: 5aa3c8a06e9b62f5fa6a90ae319c7a75cbbac4cc (Pleroma-BE, nicole mikołajczyk <git@mkljczk.pl>)
Signed-off-by: Yonle <yonle@proton.me>

objectupdater: track remote votersCount when refreshing

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

44a450384c

Yonle changed title from ~~WIP: try to federate votersCount~~ to try to federate votersCount

2026-05-05 03:18:52 +00:00

Yonle added 2 commits

2026-05-05 06:18:06 +00:00

Poll View: Always return voters_count. e19e948eec

poll_view: update voters_count func

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

3af4bcf412

Yonle added 1 commit

2026-05-05 06:20:18 +00:00

poll_view: try to read votersCount first, and then manually count local voters.

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

b59d5e8b25

Yonle commented

2026-05-05 06:21:39 +00:00

alright should be good to address #485 . Only test script hasn't being polished, as poll_view now reads votersCount from the object first before counting local voters manually.

alright should be good to address #485 . Only test script hasn't being polished, as `poll_view` now reads `votersCount` from the object first before counting local voters manually.

Oneric reviewed

2026-05-05 10:05:54 +00:00

lib/pleroma/object.ex Outdated

					
				@ -341,0 +343,4 @@

				          object.data["votersCount"] + 1

				        else

				          length(voters)

				        end

this seems very wrong. If votersCount exists, it will just bump the count on each vote, including multiple votes from the same actor making it just a redundant copy of the sum of all option’s replies.totalItems

Without having checked, I’m guessing this function only runs on either local polls or local answers of remote polls *(else we wouldn’t know about voter identity), so you’ll want to merge this with the voters update and before both first check whether there’s already a vote from this actor.

Also add a test for this

this seems very wrong. If `votersCount` exists, it will just bump the count on _each vote_, including multiple votes _from the same actor_ making it just a redundant copy of the sum of all option’s `replies.totalItems` Without having checked, I’m guessing this function only runs on either local polls or local answers of remote polls *(else we wouldn’t know about voter identity), so you’ll want to merge this with the `voters` update and before both first check whether there’s already a vote from this actor. Also add a test for this

ok, damn. just noticed that once you pointed it out.

initially, i am thinking of doing

voters_count = object.data["votersCount"] || length(voters)

but then realized that local post also had votersCount, so i wonder how do we get this though. whenever we had to check that whenever this is a local object or not, and then use different logic for remote ones (given we voters here only have local voters)

ok, damn. just noticed that once you pointed it out. initially, i am thinking of doing ```exs voters_count = object.data["votersCount"] || length(voters) ``` but then realized that local post also had votersCount, so i wonder how do we get this though. whenever we had to check that whenever this is a local object or not, and then use different logic for remote ones (given we `voters` here only have local voters)

given we voters here only have local voters

For remote polls, voters only has local voters. For local polls, voters has actually all voters.

For remote polls though, we also only ever known about and process local vote(r)s anyway, so just

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

should probably work

> given we voters here only have local voters For _remote_ polls, `voters` only has local voters. For _local_ polls, `voters` has actually _all_ voters. For remote polls though, we also only ever known about and process local vote(r)s anyway, so just ```elixir 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 ``` should probably work

Yonle marked this conversation as resolved

Oneric reviewed

2026-05-05 10:07:56 +00:00

lib/pleroma/web/mastodon_api/views/poll_view.ex Outdated

					
				@ -84,3 +80,1 @@

				    # only count local voters here; see https://akkoma.dev/AkkomaGang/akkoma/issues/190

				    nil

				  end

				  defp voters_count(%{data: %{"votersCount" => voters}}), do: voters

The comment clearly explains why removing the nil return for single-selection polls here is plain wrong and will (re)introduce bugs

The comment clearly explains why removing the `nil` return for single-selection polls here is plain wrong and will (re)introduce bugs

You restored the nil which is good, but
(a) you forgot to fix the test again
(b) the explaining comment is now gone and
(c) the whole change of the voters_count function is wholly unnecessary in the first place and just extra noise

The git log is not a diary, the state of the repo should be correct after each single commit and each commit be on logically cohesive, atomic change. Rebasing to drop misguided bits and sqaushing the actually sensible and logically interlocked bits down is much preferable

You restored the `nil` which is good, but (a) you forgot to fix the test again (b) the explaining comment is now gone and (c) the whole change of the `voters_count` function is wholly unnecessary in the first place and just extra noise The git log is not a diary, the state of the repo should be correct after each single commit and each commit be on logically cohesive, atomic change. Rebasing to drop misguided bits and sqaushing the actually sensible and logically interlocked bits down is much preferable

👀 1

Yonle marked this conversation as resolved

Yonle added 1 commit

2026-05-05 10:31:10 +00:00

poll_view: return nil if it's a multi-answer.

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

0b324eb9e4

Yonle force-pushed fed_votersCount from 0b324eb9e4

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

to 28eb65f0b3

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

2026-05-05 10:44:21 +00:00

Compare

Yonle commented

2026-05-05 10:44:52 +00:00

will need to replace test on poll_view later on.

Yonle added 1 commit

2026-05-05 11:07:28 +00:00

object: fix voters & voters-count calculation

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

04c7b251ff

previous solution incremented the voters_count by 1 on every votes in local post.
this has no problem when it's external but not on local post. where voters has
both external and local users being included (the real source)

Signed-off-by: Yonle <yonle@proton.me>

Oneric reviewed

2026-05-05 11:11:18 +00:00

lib/pleroma/web/activity_pub/transmogrifier.ex Outdated

					
				@ -813,1 +813,4 @@

				  defp set_voters_count(%{"voters" => [_ | _] = voters} = obj) do

				    Map.merge(obj, %{"votersCount" => length(voters)})

				  end

shouldn’t this keep the existing votersCount value, if one exists?

shouldn’t this keep the existing `votersCount` value, if one exists?

huh, yea

well, then

defp set_voters_count(%{"votersCount" => n} = obj) when is_integer(n), do: obj

defp set_voters_count(%{"voters" => voters} = obj) when is_list(voters) do
  Map.put(obj, "votersCount", length(voters))
end

as we're going to only set it if votersCount didn't exists in the original object (remote ones always exists, or didn't (eg. misskey))

huh, yea well, then ```exs defp set_voters_count(%{"votersCount" => n} = obj) when is_integer(n), do: obj defp set_voters_count(%{"voters" => voters} = obj) when is_list(voters) do Map.put(obj, "votersCount", length(voters)) end ``` as we're going to only set it if `votersCount` didn't exists in the original object (remote ones always exists, or didn't (eg. misskey))

Yonle marked this conversation as resolved

Yonle commented

2026-05-05 11:15:36 +00:00

that seems doing well now...

but then will do test files next

that seems doing well now... ![image](/attachments/d50af34b-e079-4907-8604-274b197cf191) ![image](/attachments/8100e02f-263d-4e92-a465-0fda7fc9d6db) but then will do test files next

image.png

26 KiB

image.png

26 KiB

Yonle added 1 commit

2026-05-05 11:30:59 +00:00

transmogrifier: only set votersCount manually if it doesn't exists in the object

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

c45494aba7

Yonle commented

2026-05-05 17:57:55 +00:00

time to fix tests....

Yonle commented

2026-05-05 18:26:06 +00:00

somehow i am stuck on writing the test

Yonle commented

2026-05-06 10:16:12 +00:00

putting some safety first

Yonle added 1 commit

2026-05-06 10:20:57 +00:00

Fix votersCount inflation in multiple-choice polls

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

79e755d790

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.

Oneric reviewed

2026-05-06 21:37:11 +00:00

Oneric left a comment

Checks for is_integer(votersCount) in processing ought to be unnecessary I believe. Either the object predates field(:votersCOunt, :integer) having been added tp the validator in which case the field got dropped (and thus is now nil), or it was added afterwards and thus is already guaranteed to be an integer. It just makes everything more complicated than it needs to be an harder to read.

If it was an actual concern, I think it would be better to run a migration to ensure all old object conform to the expect type or omit it entirely.

The new Pleroma commit you cherry-picked seems thus of little value except for tests (which it seems you haven’t run though, given there’s a misplaced, sure-to-fail test in there). Moreover, you kept the commit message from Pleroma, describing the state in Pleroma prior to any changes. But this description is wrong and actively misleading/confusing for the state of Akkoma prior to the cherry-pick.

Unrelated to this: I think it would be good to add a "votersCout" => 0 initialisation immediately to newly created local polls. This needs to be done in lib/pleroma/web/common_api/utils.ex::make_poll_data/1.

Furthermore, the new votersCount property must be defined in the JSON-LD context too; else JSON-LD processing implementation will get confused.

Conceptually this also needs a counterpart ot decrease voter count if a voter retracts all their votes. However, it seems we currently fully ignore vote retractions (Delete of the answer), so... out of scope for this PR ig

Checks for `is_integer(votersCount)` in processing ought to be unnecessary I believe. Either the object predates `field(:votersCOunt, :integer)` having been added tp the validator in which case the field got dropped (and thus is now `nil`), or it was added afterwards and thus is already guaranteed to be an integer. It just makes everything more complicated than it needs to be an harder to read. If it was an actual concern, I think it would be better to run a migration to ensure all old object conform to the expect type or omit it entirely. The new Pleroma commit you cherry-picked seems thus of little value except for tests *(which it seems you haven’t run though, given there’s a misplaced, sure-to-fail test in there)*. Moreover, you kept the commit message from Pleroma, describing the state _in Pleroma_ prior to any changes. But this description is wrong and actively misleading/confusing for the state of Akkoma prior to the cherry-pick. Unrelated to this: I think it would be good to add a `"votersCout" => 0` initialisation immediately to newly created local polls. This needs to be done in `lib/pleroma/web/common_api/utils.ex::make_poll_data/1`. Furthermore, the new `votersCount` property must be defined in the JSON-LD context too; else JSON-LD processing implementation will get confused. Conceptually this also needs a counterpart ot _decrease_ voter count if a voter retracts all their votes. However, it seems we currently fully ignore vote retractions (`Delete` of the answer), so... out of scope for this PR ig

test/pleroma/web/mastodon_api/views/poll_view_test.exs Outdated

					
				@ -193,0 +203,4 @@

				    result = PollView.render("show.json", %{object: object})

				    assert result[:pleroma][:non_anonymous] == true

				  end

A test with this purpose already exists. It also has nothing to do with the changes of this patch series. This does not belong here and this test case will fail both because it’s relying on a non-existing mock and because its testing the wrong API

damn, that slipped thru.

Yonle commented

2026-05-07 01:18:29 +00:00

I guess i will probably revert and then take some gists of the commits later, There's stuffs going on here.

@Oneric wrote in #1116 (comment):

If it was an actual concern, I think it would be better to run a migration to ensure all old object conform to the expect type or omit it entirely.

better be prepared than nothing i guess. Will write about it later

I think it would be good to add a "votersCout" => 0 initialisation immediately to newly created local polls. This needs to be done in lib/pleroma/web/common_api/utils.ex::make_poll_data/1.

will add it.

Furthermore, the new votersCount property must be defined in the JSON-LD context too; else JSON-LD processing implementation will get confused.

right, i forgot we haven't dealt with that one, so will do it.

it seems we currently fully ignore vote retractions (Delete of the answer), so... out of scope for this PR ig

as of now we haven't seen a server that did exactly that, so it's safe for now. but i think it's a good idea to add that later on, but however not on this PR of course. probably opening issues to track about it later

I guess i will probably revert and then take some gists of the commits later, There's stuffs going on here. @Oneric wrote in https://akkoma.dev/AkkomaGang/akkoma/pulls/1116#issuecomment-17084: > If it was an actual concern, I think it would be better to run a migration to ensure all old object conform to the expect type or omit it entirely. better be prepared than nothing i guess. Will write about it later > I think it would be good to add a "votersCout" => 0 initialisation immediately to newly created local polls. This needs to be done in lib/pleroma/web/common_api/utils.ex::make_poll_data/1. will add it. > Furthermore, the new votersCount property must be defined in the JSON-LD context too; else JSON-LD processing implementation will get confused. right, i forgot we haven't dealt with that one, so will do it. > it seems we currently fully ignore vote retractions (Delete of the answer), so... out of scope for this PR ig as of now we haven't seen a server that did exactly that, so it's safe for now. but i think it's a good idea to add that later on, but however not on this PR of course. probably opening issues to track about it later

Oneric force-pushed fed_votersCount from 79e755d790

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

to dbb06b4115

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

2026-05-09 19:33:41 +00:00

Compare

Oneric force-pushed fed_votersCount from dbb06b4115

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

to 6d5b1b936b

ci/woodpecker/pr/test/2 Pipeline was successful

Details

ci/woodpecker/pr/test/1 Pipeline was successful

Details

2026-05-09 20:45:20 +00:00

Compare

Oneric force-pushed fed_votersCount from 6d5b1b936b

ci/woodpecker/pr/test/2 Pipeline was successful

Details

ci/woodpecker/pr/test/1 Pipeline was successful

Details

to 3ea4b60c00

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

2026-05-09 20:52:06 +00:00

Compare

Oneric force-pushed fed_votersCount from 3ea4b60c00

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

to f694c7e081

ci/woodpecker/pr/test/2 Pipeline was successful

Details

ci/woodpecker/pr/test/1 Pipeline was successful

Details

2026-05-09 21:28:02 +00:00

Compare

Oneric force-pushed fed_votersCount from f694c7e081

ci/woodpecker/pr/test/2 Pipeline was successful

Details

ci/woodpecker/pr/test/1 Pipeline was successful

Details

to 7704e4a32b

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

2026-05-09 21:47:14 +00:00

Compare

Oneric force-pushed fed_votersCount from 7704e4a32b

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

to e13f025e22

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

2026-05-09 21:51:37 +00:00

Compare

Oneric changed title from ~~try to federate votersCount~~ to federate votersCount

2026-05-09 21:54:27 +00:00

Oneric merged commit 22e3792ddb into develop

2026-05-09 21:56:11 +00:00

Oneric referenced this pull request from a commit

2026-05-09 21:56:12 +00:00

Merge pull request 'federate votersCount' (#1116) from Yonle/akkoma:fed_votersCount into develop

Oneric referenced this pull request from AkkomaGang/akkoma-fe

2026-05-09 22:22:32 +00:00

poll(multianswer): fix totalFractionBase for multi answer poll #516

Sign in to join this conversation.

No reviewers

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

AkkomaGang/akkoma!1116

Rows
Columns