From 3babf8464b0903b854ec16d355909444ef3ca0bc Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 29 Sep 2019 22:58:01 +0200 Subject: [PATCH] Add voters count support (#11917) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add voters count to polls * Add ActivityPub serialization and parsing of voters count * Add support for voters count in WebUI * Move incrementation of voters count out of redis lock * Reword “voters” to “people” --- app/javascript/mastodon/components/poll.js | 19 ++++++--- app/lib/activitypub/activity/create.rb | 40 ++++++++++++++++--- app/lib/activitypub/adapter.rb | 1 + app/models/poll.rb | 1 + .../activitypub/note_serializer.rb | 12 +++++- app/serializers/rest/poll_serializer.rb | 2 +- .../activitypub/process_poll_service.rb | 5 ++- app/services/post_status_service.rb | 2 +- app/services/vote_service.rb | 32 +++++++++++++-- app/views/statuses/_poll.html.haml | 8 +++- config/locales/en.yml | 3 ++ ...0190927232842_add_voters_count_to_polls.rb | 5 +++ db/schema.rb | 3 +- 13 files changed, 113 insertions(+), 20 deletions(-) create mode 100644 db/migrate/20190927232842_add_voters_count_to_polls.rb diff --git a/app/javascript/mastodon/components/poll.js b/app/javascript/mastodon/components/poll.js index f88d260f24..cdbcf8f709 100644 --- a/app/javascript/mastodon/components/poll.js +++ b/app/javascript/mastodon/components/poll.js @@ -102,10 +102,11 @@ class Poll extends ImmutablePureComponent { renderOption (option, optionIndex, showResults) { const { poll, disabled, intl } = this.props; - const percent = poll.get('votes_count') === 0 ? 0 : (option.get('votes_count') / poll.get('votes_count')) * 100; - const leading = poll.get('options').filterNot(other => other.get('title') === option.get('title')).every(other => option.get('votes_count') >= other.get('votes_count')); - const active = !!this.state.selected[`${optionIndex}`]; - const voted = option.get('voted') || (poll.get('own_votes') && poll.get('own_votes').includes(optionIndex)); + const pollVotesCount = poll.get('voters_count') || poll.get('votes_count'); + const percent = pollVotesCount === 0 ? 0 : (option.get('votes_count') / pollVotesCount) * 100; + const leading = poll.get('options').filterNot(other => other.get('title') === option.get('title')).every(other => option.get('votes_count') >= other.get('votes_count')); + const active = !!this.state.selected[`${optionIndex}`]; + const voted = option.get('voted') || (poll.get('own_votes') && poll.get('own_votes').includes(optionIndex)); let titleEmojified = option.get('title_emojified'); if (!titleEmojified) { @@ -157,6 +158,14 @@ class Poll extends ImmutablePureComponent { const showResults = poll.get('voted') || expired; const disabled = this.props.disabled || Object.entries(this.state.selected).every(item => !item); + let votesCount = null; + + if (poll.get('voters_count') !== null && poll.get('voters_count') !== undefined) { + votesCount = ; + } else { + votesCount = ; + } + return (
    @@ -166,7 +175,7 @@ class Poll extends ImmutablePureComponent {
    {!showResults && } {showResults && !this.props.disabled && · } - + {votesCount} {poll.get('expires_at') && · {timeRemaining}}
diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index e69193b710..76bf9b2e55 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -232,25 +232,40 @@ class ActivityPub::Activity::Create < ActivityPub::Activity items = @object['oneOf'] end + voters_count = @object['votersCount'] + @account.polls.new( multiple: multiple, expires_at: expires_at, options: items.map { |item| item['name'].presence || item['content'] }.compact, - cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 } + cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 }, + voters_count: voters_count ) end def poll_vote? return false if replied_to_status.nil? || replied_to_status.preloadable_poll.nil? || !replied_to_status.local? || !replied_to_status.preloadable_poll.options.include?(@object['name']) - unless replied_to_status.preloadable_poll.expired? - replied_to_status.preloadable_poll.votes.create!(account: @account, choice: replied_to_status.preloadable_poll.options.index(@object['name']), uri: @object['id']) - ActivityPub::DistributePollUpdateWorker.perform_in(3.minutes, replied_to_status.id) unless replied_to_status.preloadable_poll.hide_totals? - end + poll_vote! unless replied_to_status.preloadable_poll.expired? true end + def poll_vote! + poll = replied_to_status.preloadable_poll + already_voted = true + RedisLock.acquire(poll_lock_options) do |lock| + if lock.acquired? + already_voted = poll.votes.where(account: @account).exists? + poll.votes.create!(account: @account, choice: poll.options.index(@object['name']), uri: @object['id']) + else + raise Mastodon::RaceConditionError + end + end + increment_voters_count! unless already_voted + ActivityPub::DistributePollUpdateWorker.perform_in(3.minutes, replied_to_status.id) unless replied_to_status.preloadable_poll.hide_totals? + end + def resolve_thread(status) return unless status.reply? && status.thread.nil? && Request.valid_url?(in_reply_to_uri) ThreadResolveWorker.perform_async(status.id, in_reply_to_uri) @@ -416,7 +431,22 @@ class ActivityPub::Activity::Create < ActivityPub::Activity ActivityPub::RawDistributionWorker.perform_async(Oj.dump(@json), replied_to_status.account_id, [@account.preferred_inbox_url]) end + def increment_voters_count! + poll = replied_to_status.preloadable_poll + unless poll.voters_count.nil? + poll.voters_count = poll.voters_count + 1 + poll.save + end + rescue ActiveRecord::StaleObjectError + poll.reload + retry + end + def lock_options { redis: Redis.current, key: "create:#{@object['id']}" } end + + def poll_lock_options + { redis: Redis.current, key: "vote:#{replied_to_status.poll_id}:#{@account.id}" } + end end diff --git a/app/lib/activitypub/adapter.rb b/app/lib/activitypub/adapter.rb index cb2ac72d45..2a8f723334 100644 --- a/app/lib/activitypub/adapter.rb +++ b/app/lib/activitypub/adapter.rb @@ -21,6 +21,7 @@ class ActivityPub::Adapter < ActiveModelSerializers::Adapter::Base identity_proof: { 'toot' => 'http://joinmastodon.org/ns#', 'IdentityProof' => 'toot:IdentityProof' }, blurhash: { 'toot' => 'http://joinmastodon.org/ns#', 'blurhash' => 'toot:blurhash' }, discoverable: { 'toot' => 'http://joinmastodon.org/ns#', 'discoverable' => 'toot:discoverable' }, + voters_count: { 'toot' => 'http://joinmastodon.org/ns#', 'votersCount' => 'toot:votersCount' }, }.freeze def self.default_key_transform diff --git a/app/models/poll.rb b/app/models/poll.rb index 55a8f13a65..5427368fd0 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -16,6 +16,7 @@ # created_at :datetime not null # updated_at :datetime not null # lock_version :integer default(0), not null +# voters_count :bigint(8) # class Poll < ApplicationRecord diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 364d3eda59..110621a28b 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class ActivityPub::NoteSerializer < ActivityPub::Serializer - context_extensions :atom_uri, :conversation, :sensitive + context_extensions :atom_uri, :conversation, :sensitive, :voters_count attributes :id, :type, :summary, :in_reply_to, :published, :url, @@ -23,6 +23,8 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer attribute :end_time, if: :poll_and_expires? attribute :closed, if: :poll_and_expired? + attribute :voters_count, if: :poll_and_voters_count? + def id ActivityPub::TagManager.instance.uri_for(object) end @@ -141,6 +143,10 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer alias end_time closed + def voters_count + object.preloadable_poll.voters_count + end + def poll_and_expires? object.preloadable_poll&.expires_at&.present? end @@ -149,6 +155,10 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer object.preloadable_poll&.expired? end + def poll_and_voters_count? + object.preloadable_poll&.voters_count + end + class MediaAttachmentSerializer < ActivityPub::Serializer context_extensions :blurhash, :focal_point diff --git a/app/serializers/rest/poll_serializer.rb b/app/serializers/rest/poll_serializer.rb index eb98bb2d23..df6ebd0d44 100644 --- a/app/serializers/rest/poll_serializer.rb +++ b/app/serializers/rest/poll_serializer.rb @@ -2,7 +2,7 @@ class REST::PollSerializer < ActiveModel::Serializer attributes :id, :expires_at, :expired, - :multiple, :votes_count + :multiple, :votes_count, :voters_count has_many :loaded_options, key: :options has_many :emojis, serializer: REST::CustomEmojiSerializer diff --git a/app/services/activitypub/process_poll_service.rb b/app/services/activitypub/process_poll_service.rb index 2fbce65b9c..cb4a0d4602 100644 --- a/app/services/activitypub/process_poll_service.rb +++ b/app/services/activitypub/process_poll_service.rb @@ -28,6 +28,8 @@ class ActivityPub::ProcessPollService < BaseService end end + voters_count = @json['votersCount'] + latest_options = items.map { |item| item['name'].presence || item['content'] } # If for some reasons the options were changed, it invalidates all previous @@ -39,7 +41,8 @@ class ActivityPub::ProcessPollService < BaseService last_fetched_at: Time.now.utc, expires_at: expires_at, options: latest_options, - cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 } + cached_tallies: items.map { |item| item.dig('replies', 'totalItems') || 0 }, + voters_count: voters_count ) rescue ActiveRecord::StaleObjectError poll.reload diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 34ec6d5048..a0a650d624 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -174,7 +174,7 @@ class PostStatusService < BaseService def poll_attributes return if @options[:poll].blank? - @options[:poll].merge(account: @account) + @options[:poll].merge(account: @account, voters_count: 0) end def scheduled_options diff --git a/app/services/vote_service.rb b/app/services/vote_service.rb index 0eeb8fd56a..cb7dce6e8a 100644 --- a/app/services/vote_service.rb +++ b/app/services/vote_service.rb @@ -12,12 +12,24 @@ class VoteService < BaseService @choices = choices @votes = [] - ApplicationRecord.transaction do - @choices.each do |choice| - @votes << @poll.votes.create!(account: @account, choice: choice) + already_voted = true + + RedisLock.acquire(lock_options) do |lock| + if lock.acquired? + already_voted = @poll.votes.where(account: @account).exists? + + ApplicationRecord.transaction do + @choices.each do |choice| + @votes << @poll.votes.create!(account: @account, choice: choice) + end + end + else + raise Mastodon::RaceConditionError end end + increment_voters_count! unless already_voted + ActivityTracker.increment('activity:interactions') if @poll.account.local? @@ -53,4 +65,18 @@ class VoteService < BaseService def build_json(vote) Oj.dump(serialize_payload(vote, ActivityPub::VoteSerializer)) end + + def increment_voters_count! + unless @poll.voters_count.nil? + @poll.voters_count = @poll.voters_count + 1 + @poll.save + end + rescue ActiveRecord::StaleObjectError + @poll.reload + retry + end + + def lock_options + { redis: Redis.current, key: "vote:#{@poll.id}:#{@account.id}" } + end end diff --git a/app/views/statuses/_poll.html.haml b/app/views/statuses/_poll.html.haml index d6b36a5d12..d1aba6ef9c 100644 --- a/app/views/statuses/_poll.html.haml +++ b/app/views/statuses/_poll.html.haml @@ -1,12 +1,13 @@ - show_results = (user_signed_in? && poll.voted?(current_account)) || poll.expired? - own_votes = user_signed_in? ? poll.own_votes(current_account) : [] +- total_votes_count = poll.voters_count || poll.votes_count .poll %ul - poll.loaded_options.each_with_index do |option, index| %li - if show_results - - percent = poll.votes_count > 0 ? 100 * option.votes_count / poll.votes_count : 0 + - percent = total_votes_count > 0 ? 100 * option.votes_count / total_votes_count : 0 %span.poll__chart{ style: "width: #{percent}%" } %label.poll__text>< @@ -24,7 +25,10 @@ %button.button.button-secondary{ disabled: true } = t('statuses.poll.vote') - %span= t('statuses.poll.total_votes', count: poll.votes_count) + - if poll.voters_count.nil? + %span= t('statuses.poll.total_votes', count: poll.votes_count) + - else + %span= t('statuses.poll.total_people', count: poll.voters_count) - unless poll.expires_at.nil? · diff --git a/config/locales/en.yml b/config/locales/en.yml index dbdfe0ca03..82e20cb1fd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1030,6 +1030,9 @@ en: private: Non-public toot cannot be pinned reblog: A boost cannot be pinned poll: + total_people: + one: "%{count} person" + other: "%{count} people" total_votes: one: "%{count} vote" other: "%{count} votes" diff --git a/db/migrate/20190927232842_add_voters_count_to_polls.rb b/db/migrate/20190927232842_add_voters_count_to_polls.rb new file mode 100644 index 0000000000..8463857009 --- /dev/null +++ b/db/migrate/20190927232842_add_voters_count_to_polls.rb @@ -0,0 +1,5 @@ +class AddVotersCountToPolls < ActiveRecord::Migration[5.2] + def change + add_column :polls, :voters_count, :bigint + end +end diff --git a/db/schema.rb b/db/schema.rb index 8eeaf48a08..557b777e01 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_09_27_124642) do +ActiveRecord::Schema.define(version: 2019_09_27_232842) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -529,6 +529,7 @@ ActiveRecord::Schema.define(version: 2019_09_27_124642) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.integer "lock_version", default: 0, null: false + t.bigint "voters_count" t.index ["account_id"], name: "index_polls_on_account_id" t.index ["status_id"], name: "index_polls_on_status_id" end