Refactor AutocompleteSources#label:

1. The return value was inconsistent, sometimes returning a hash and sometimes returning an ActiveModel.
2. It was inconsistent with respect to other methods in the class since they all return ActiveModels.
This commit is contained in:
Mark Chao 2018-07-06 11:37:58 +08:00
parent 83f79ced3f
commit 389a22e7b7
3 changed files with 72 additions and 14 deletions

View File

@ -14,7 +14,7 @@ class Projects::AutocompleteSourcesController < Projects::ApplicationController
end
def labels
render json: @autocomplete_service.labels(target)
render json: @autocomplete_service.labels_as_hash(target)
end
def milestones

View File

@ -20,24 +20,28 @@ module Projects
MergeRequestsFinder.new(current_user, project_id: project.id, state: 'opened').execute.select([:iid, :title])
end
def labels(target = nil)
labels = LabelsFinder.new(current_user, project_id: project.id, include_ancestor_groups: true)
.execute.select([:color, :title])
def labels_as_hash(target = nil)
available_labels = LabelsFinder.new(
current_user,
project_id: project.id,
include_ancestor_groups: true
).execute
return labels unless target&.respond_to?(:labels)
label_hashes = available_labels.as_json(only: [:title, :color])
issuable_label_titles = target.labels.pluck(:title)
if issuable_label_titles
labels = labels.as_json(only: [:title, :color])
issuable_label_titles.each do |issuable_label_title|
found_label = labels.find { |label| label['title'] == issuable_label_title }
found_label[:set] = true if found_label
if target&.respond_to?(:labels)
already_set_labels = available_labels & target.labels
if already_set_labels.present?
titles = already_set_labels.map(&:title)
label_hashes.each do |hash|
if titles.include?(hash['title'])
hash[:set] = true
end
end
end
end
labels
label_hashes
end
def commands(noteable, type)

View File

@ -131,4 +131,58 @@ describe Projects::AutocompleteService do
end
end
end
describe '#labels_as_hash' do
def expect_labels_to_equal(labels, expected_labels)
expect(labels.size).to eq(expected_labels.size)
extract_title = lambda { |label| label['title'] }
expect(labels.map(&extract_title)).to eq(expected_labels.map(&extract_title))
end
let(:user) { create(:user) }
let(:group) { create(:group, :nested) }
let!(:sub_group) { create(:group, parent: group) }
let(:project) { create(:project, :public, group: group) }
let(:issue) { create(:issue, project: project) }
let!(:label1) { create(:label, project: project) }
let!(:label2) { create(:label, project: project) }
let!(:sub_group_label) { create(:group_label, group: sub_group) }
let!(:parent_group_label) { create(:group_label, group: group.parent) }
before do
create(:group_member, group: group, user: user)
end
it 'returns labels from project and ancestor groups' do
service = described_class.new(project, user)
results = service.labels_as_hash
expected_labels = [label1, label2, parent_group_label]
expect_labels_to_equal(results, expected_labels)
end
context 'some labels are already assigned' do
before do
issue.labels << label1
end
it 'marks already assigned as set' do
service = described_class.new(project, user)
results = service.labels_as_hash(issue)
expected_labels = [label1, label2, parent_group_label]
expect_labels_to_equal(results, expected_labels)
assigned_label_titles = issue.labels.map(&:title)
results.each do |hash|
if assigned_label_titles.include?(hash['title'])
expect(hash[:set]).to eq(true)
else
expect(hash.key?(:set)).to eq(false)
end
end
end
end
end
end