Commit aeaf3eb4 authored by Dmitry Gruzd's avatar Dmitry Gruzd

Add upvotes_count to issues

Changelog: changed
parent 44a778a9
......@@ -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')
# 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
# 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
c2efdad12c3d0ec5371259baa91466137b827f513250e901842ab28e56c3de0a
\ No newline at end of file
fdd7509fc88e563b65b487706cae1a64066a7ba7d4bd13d0414b8431c3ddfb68
\ No newline at end of file
......@@ -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))
);
# 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
......@@ -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
......
# 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
# 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
......@@ -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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment