Commit 8be484f4 authored by Eugenia Grieff's avatar Eugenia Grieff

Apply review suggestions

parent d355faea
...@@ -91,10 +91,10 @@ module Issuable ...@@ -91,10 +91,10 @@ module Issuable
end end
def schedule_group_issues_count_refresh(updated_issuables) def schedule_group_issues_count_refresh(updated_issuables)
group_ids = updated_issuables.map(&:project).map { |project| project.namespace_id } group_ids = updated_issuables.map(&:project).map(&:namespace_id)
return if group_ids.empty? return if group_ids.empty?
Issuables::RefreshGroupsIssueCounterWorker.perform_async(current_user.id, group_ids) Issuables::RefreshGroupsIssueCounterWorker.perform_async(group_ids)
end end
end end
end end
......
...@@ -8,25 +8,21 @@ module Issuables ...@@ -8,25 +8,21 @@ module Issuables
urgency :low urgency :low
feature_category :issue_tracking feature_category :issue_tracking
def perform(current_user_id, group_ids = []) def perform(group_ids = [])
return if group_ids.empty? return if group_ids.empty?
current_user = User.find(current_user_id)
groups_with_ancestors = Gitlab::ObjectHierarchy groups_with_ancestors = Gitlab::ObjectHierarchy
.new(Group.by_id(group_ids)) .new(Group.by_id(group_ids))
.base_and_ancestors .base_and_ancestors
.with_route
refresh_cached_count(current_user, groups_with_ancestors) refresh_cached_count(groups_with_ancestors)
rescue ActiveRecord::RecordNotFound => e
Gitlab::ErrorTracking.log_exception(e, user_id: current_user_id)
end end
private private
def refresh_cached_count(user, groups) def refresh_cached_count(groups)
groups.each do |group| groups.each do |group|
Groups::OpenIssuesCountService.new(group, user).refresh_cache_over_threshold Groups::OpenIssuesCountService.new(group).refresh_cache_over_threshold
end end
end end
end end
......
...@@ -12,14 +12,6 @@ RSpec.describe Issuables::RefreshGroupsIssueCounterWorker do ...@@ -12,14 +12,6 @@ RSpec.describe Issuables::RefreshGroupsIssueCounterWorker do
let(:count_service) { Groups::OpenIssuesCountService } let(:count_service) { Groups::OpenIssuesCountService }
let(:group_ids) { [root_group.id] } let(:group_ids) { [root_group.id] }
it 'anticipates the inability to find the issue' do
expect(Gitlab::ErrorTracking).to receive(:log_exception)
.with(ActiveRecord::RecordNotFound, include(user_id: -1))
expect(count_service).not_to receive(:new)
described_class.new.perform(-1, group_ids)
end
context 'when group_ids is empty' do context 'when group_ids is empty' do
let(:group_ids) { [] } let(:group_ids) { [] }
...@@ -27,7 +19,7 @@ RSpec.describe Issuables::RefreshGroupsIssueCounterWorker do ...@@ -27,7 +19,7 @@ RSpec.describe Issuables::RefreshGroupsIssueCounterWorker do
expect(count_service).not_to receive(:new) expect(count_service).not_to receive(:new)
expect(Gitlab::ErrorTracking).not_to receive(:log_exception) expect(Gitlab::ErrorTracking).not_to receive(:log_exception)
described_class.new.perform(user.id, group_ids) described_class.new.perform(group_ids)
end end
end end
...@@ -36,15 +28,15 @@ RSpec.describe Issuables::RefreshGroupsIssueCounterWorker do ...@@ -36,15 +28,15 @@ RSpec.describe Issuables::RefreshGroupsIssueCounterWorker do
let(:instance2) { instance_double(count_service) } let(:instance2) { instance_double(count_service) }
it_behaves_like 'an idempotent worker' do it_behaves_like 'an idempotent worker' do
let(:job_args) { [user.id, group_ids] } let(:job_args) { [group_ids] }
let(:exec_times) { IdempotentWorkerHelper::WORKER_EXEC_TIMES } let(:exec_times) { IdempotentWorkerHelper::WORKER_EXEC_TIMES }
it 'refreshes the issue count in given groups and ancestors' do it 'refreshes the issue count in given groups and ancestors' do
expect(count_service).to receive(:new) expect(count_service).to receive(:new)
.exactly(exec_times).times.with(root_group, user).and_return(instance1) .exactly(exec_times).times.with(root_group).and_return(instance1)
expect(count_service).to receive(:new) expect(count_service).to receive(:new)
.exactly(exec_times).times.with(parent_group, user).and_return(instance2) .exactly(exec_times).times.with(parent_group).and_return(instance2)
expect(count_service).not_to receive(:new).with(subgroup, user) expect(count_service).not_to receive(:new).with(subgroup)
[instance1, instance2].all? do |instance| [instance1, instance2].all? do |instance|
expect(instance).to receive(:refresh_cache_over_threshold).exactly(exec_times).times expect(instance).to receive(:refresh_cache_over_threshold).exactly(exec_times).times
......
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