Merge branch '263365-add-upvotes-column-to-issues' into 'master'

Add issues.upvotes_count column

See merge request gitlab-org/gitlab!65250

app/models/award_emoji.rb

@@ -27,6 +27,9 @@ class AwardEmoji < ApplicationRecord
   after_save :expire_cache
   after_destroy :expire_cache
 
+  after_save :update_awardable_upvotes_count
+  after_destroy :update_awardable_upvotes_count
+
   class << self
     def votes_for_collection(ids, type)
       select('name', 'awardable_id', 'COUNT(*) as count')
@@ -64,6 +67,14 @@ class AwardEmoji < ApplicationRecord
     awardable.try(:bump_updated_at)
     awardable.try(:expire_etag_cache)
   end
+
+  private
+
+  def update_awardable_upvotes_count
+    return unless upvote? && awardable.has_attribute?(:upvotes_count)
+
+    awardable.update_column(:upvotes_count, awardable.upvotes)
+  end
 end
 
 AwardEmoji.prepend_mod_with('AwardEmoji')

db/migrate/20210701111627_add_upvotes_count_to_issues.rb

+# frozen_string_literal: true
+
+class AddUpvotesCountToIssues < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  def up
+    with_lock_retries do
+      add_column :issues, :upvotes_count, :integer, default: 0, null: false
+    end
+  end
+
+  def down
+    remove_column :issues, :upvotes_count
+  end
+end

db/post_migrate/20210701111909_backfill_issues_upvotes_count.rb

+# frozen_string_literal: true
+
+class BackfillIssuesUpvotesCount < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  MIGRATION = 'BackfillUpvotesCountOnIssues'
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 5_000
+
+  def up
+    scope = Issue.joins("INNER JOIN award_emoji e ON e.awardable_id = issues.id AND e.awardable_type = 'Issue' AND e.name = 'thumbsup'")
+
+    queue_background_migration_jobs_by_range_at_intervals(
+      scope,
+      MIGRATION,
+      DELAY_INTERVAL,
+      batch_size: BATCH_SIZE
+    )
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20210701111627

+c2efdad12c3d0ec5371259baa91466137b827f513250e901842ab28e56c3de0a
\ No newline at end of file

db/schema_migrations/20210701111909

+fdd7509fc88e563b65b487706cae1a64066a7ba7d4bd13d0414b8431c3ddfb68
\ No newline at end of file

db/structure.sql

@@ -14267,6 +14267,7 @@ CREATE TABLE issues (
     sprint_id bigint,
     issue_type smallint DEFAULT 0 NOT NULL,
     blocking_issues_count integer DEFAULT 0 NOT NULL,
+    upvotes_count integer DEFAULT 0 NOT NULL,
     CONSTRAINT check_fba63f706d CHECK ((lock_version IS NOT NULL))
 );
 

lib/gitlab/background_migration/backfill_upvotes_count_on_issues.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Class that will populate the upvotes_count field
+    # for each issue
+    class BackfillUpvotesCountOnIssues
+      BATCH_SIZE = 1_000
+
+      def perform(start_id, stop_id)
+        (start_id..stop_id).step(BATCH_SIZE).each do |offset|
+          update_issue_upvotes_count(offset, offset + BATCH_SIZE)
+        end
+      end
+
+      private
+
+      def execute(sql)
+        @connection ||= ::ActiveRecord::Base.connection
+        @connection.execute(sql)
+      end
+
+      def update_issue_upvotes_count(batch_start, batch_stop)
+        execute(<<~SQL)
+          UPDATE issues
+          SET upvotes_count = sub_q.count_all
+          FROM (
+            SELECT COUNT(*) AS count_all, e.awardable_id AS issue_id
+            FROM award_emoji AS e
+            WHERE e.name = 'thumbsup' AND
+            e.awardable_type = 'Issue' AND
+            e.awardable_id BETWEEN #{batch_start} AND #{batch_stop}
+            GROUP BY issue_id
+          ) AS sub_q
+          WHERE sub_q.issue_id = issues.id;
+        SQL
+      end
+    end
+  end
+end

lib/gitlab/import_export/project/import_export.yml

@@ -229,6 +229,7 @@ excluded_attributes:
     - :promoted_to_epic_id
     - :blocking_issues_count
     - :service_desk_reply_to
+    - :upvotes_count
   merge_request:
     - :milestone_id
     - :sprint_id

spec/lib/gitlab/background_migration/backfill_upvotes_count_on_issues_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillUpvotesCountOnIssues, schema: 20210701111909 do
+  let(:award_emoji) { table(:award_emoji) }
+
+  let!(:namespace) { table(:namespaces).create!(name: 'namespace', path: 'namespace') }
+  let!(:project1) { table(:projects).create!(namespace_id: namespace.id) }
+  let!(:project2) { table(:projects).create!(namespace_id: namespace.id) }
+  let!(:issue1) { table(:issues).create!(project_id: project1.id) }
+  let!(:issue2) { table(:issues).create!(project_id: project2.id) }
+  let!(:issue3) { table(:issues).create!(project_id: project2.id) }
+  let!(:issue4) { table(:issues).create!(project_id: project2.id) }
+
+  describe '#perform' do
+    before do
+      add_upvotes(issue1, :thumbsdown, 1)
+      add_upvotes(issue2, :thumbsup, 2)
+      add_upvotes(issue2, :thumbsdown, 1)
+      add_upvotes(issue3, :thumbsup, 3)
+      add_upvotes(issue4, :thumbsup, 4)
+    end
+
+    it 'updates upvotes_count' do
+      subject.perform(issue1.id, issue4.id)
+
+      expect(issue1.reload.upvotes_count).to eq(0)
+      expect(issue2.reload.upvotes_count).to eq(2)
+      expect(issue3.reload.upvotes_count).to eq(3)
+      expect(issue4.reload.upvotes_count).to eq(4)
+    end
+  end
+
+  private
+
+  def add_upvotes(issue, name, count)
+    count.times do
+      award_emoji.create!(
+        name: name.to_s,
+        awardable_type: 'Issue',
+        awardable_id: issue.id
+      )
+    end
+  end
+end

spec/migrations/backfill_issues_upvotes_count_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe BackfillIssuesUpvotesCount do
+  let(:migration) { described_class.new }
+  let(:issues) { table(:issues) }
+  let(:award_emoji) { table(:award_emoji) }
+
+  let!(:issue1) { issues.create! }
+  let!(:issue2) { issues.create! }
+  let!(:issue3) { issues.create! }
+  let!(:issue4) { issues.create! }
+  let!(:issue4_without_thumbsup) { issues.create! }
+
+  let!(:award_emoji1) { award_emoji.create!( name: 'thumbsup', awardable_type: 'Issue', awardable_id: issue1.id) }
+  let!(:award_emoji2) { award_emoji.create!( name: 'thumbsup', awardable_type: 'Issue', awardable_id: issue2.id) }
+  let!(:award_emoji3) { award_emoji.create!( name: 'thumbsup', awardable_type: 'Issue', awardable_id: issue3.id) }
+  let!(:award_emoji4) { award_emoji.create!( name: 'thumbsup', awardable_type: 'Issue', awardable_id: issue4.id) }
+
+  it 'correctly schedules background migrations' do
+    stub_const("#{described_class.name}::BATCH_SIZE", 2)
+
+    Sidekiq::Testing.fake! do
+      freeze_time do
+        migrate!
+
+        expect(described_class::MIGRATION).to be_scheduled_migration(issue1.id, issue2.id)
+        expect(described_class::MIGRATION).to be_scheduled_migration(issue3.id, issue4.id)
+        expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+      end
+    end
+  end
+end

spec/models/award_emoji_spec.rb

@@ -171,4 +171,43 @@ RSpec.describe AwardEmoji do
       expect(awards).to eq('thumbsup' => 2)
     end
   end
+
+  describe 'updating upvotes_count' do
+    context 'on an issue' do
+      let(:issue) { create(:issue) }
+      let(:upvote) { build(:award_emoji, :upvote, user: build(:user), awardable: issue) }
+      let(:downvote) { build(:award_emoji, :downvote, user: build(:user), awardable: issue) }
+
+      it 'updates upvotes_count on the issue when saved' do
+        expect(issue).to receive(:update_column).with(:upvotes_count, 1).once
+
+        upvote.save!
+        downvote.save!
+      end
+
+      it 'updates upvotes_count on the issue when destroyed' do
+        expect(issue).to receive(:update_column).with(:upvotes_count, 0).once
+
+        upvote.destroy!
+        downvote.destroy!
+      end
+    end
+
+    context 'on another awardable' do
+      let(:merge_request) { create(:merge_request) }
+      let(:award_emoji) { build(:award_emoji, user: build(:user), awardable: merge_request) }
+
+      it 'does not update upvotes_count on the merge_request when saved' do
+        expect(merge_request).not_to receive(:update_column)
+
+        award_emoji.save!
+      end
+
+      it 'does not update upvotes_count on the merge_request when destroyed' do
+        expect(merge_request).not_to receive(:update_column)
+
+        award_emoji.destroy!
+      end
+    end
+  end
 end