Add voters count support (mastodon#11917)

* 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”
hachyderm · Sep 29, 2019 · 3babf84 · 3babf84
1 parent cfe2d1c
commit 3babf84
Show file tree

Hide file tree

Showing 13 changed files with 113 additions and 20 deletions.
diff --git 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 = <FormattedMessage id='poll.total_people' defaultMessage='{count, plural, one {# person} other {# people}}' values={{ count: poll.get('voters_count') }} />;
+    } else {
+      votesCount = <FormattedMessage id='poll.total_votes' defaultMessage='{count, plural, one {# vote} other {# votes}}' values={{ count: poll.get('votes_count') }} />;
+    }
+
     return (
       <div className='poll'>
         <ul>
@@ -166,7 +175,7 @@ class Poll extends ImmutablePureComponent {
         <div className='poll__footer'>
           {!showResults && <button className='button button-secondary' disabled={disabled} onClick={this.handleVote}><FormattedMessage id='poll.vote' defaultMessage='Vote' /></button>}
           {showResults && !this.props.disabled && <span><button className='poll__link' onClick={this.handleRefresh}><FormattedMessage id='poll.refresh' defaultMessage='Refresh' /></button> · </span>}
-          <FormattedMessage id='poll.total_votes' defaultMessage='{count, plural, one {# vote} other {# votes}}' values={{ count: poll.get('votes_count') }} />
+          {votesCount}
           {poll.get('expires_at') && <span> · {timeRemaining}</span>}
         </div>
       </div>

diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb
@@ -232,25 +232,40 @@ def process_poll
       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 @@ def forward_for_reply
     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
@@ -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
@@ -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
@@ -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 @@ def closed
 
   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 @@ def poll_and_expired?
     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
@@ -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
@@ -28,6 +28,8 @@ def call(poll, json)
       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 @@ def call(poll, json)
         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
@@ -174,7 +174,7 @@ def scheduled_status_attributes
   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
@@ -12,12 +12,24 @@ def call(account, poll, choices)
     @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 @@ def deliver_votes!
   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
@@ -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
@@ -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
@@ -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
@@ -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 @@
     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