Merge branch 'rs-optimize-award_user_list-spec' into 'master'

Optimize the `award_user_list` helper spec

According to
https://gitlab.com/gitlab-org/gitlab-ce/issues/23034#note_16586657, each
test for this helper generated 1,833 queries.

Now we only generate stubbed records, and only as many as we need for
each test.

This also corrects a slight logic bug in the helper itself. When the
number of awards was greater than the limit (9 by default), _and_ the
current user was one of them, we actually included 10 names, including
"You", plus the remaining count. Now we return the correct number
regardless.

See merge request !6722
This commit is contained in:
Rémy Coutable 2016-10-07 15:18:22 +00:00
commit 212cf8f950
2 changed files with 25 additions and 16 deletions

View File

@ -113,14 +113,13 @@ module IssuesHelper
end end
end end
def award_user_list(awards, current_user) def award_user_list(awards, current_user, limit: 10)
names = awards.map do |award| names = awards.map do |award|
award.user == current_user ? 'You' : award.user.name award.user == current_user ? 'You' : award.user.name
end end
# Take first 9 OR current user + first 9
current_user_name = names.delete('You') current_user_name = names.delete('You')
names = names.first(9).insert(0, current_user_name).compact names = names.insert(0, current_user_name).compact.first(limit)
names << "#{awards.size - names.size} more." if awards.size > names.size names << "#{awards.size - names.size} more." if awards.size > names.size

View File

@ -63,28 +63,38 @@ describe IssuesHelper do
end end
describe '#award_user_list' do describe '#award_user_list' do
let!(:awards) { build_list(:award_emoji, 15) } it "returns a comma-separated list of the first X users" do
user = build_stubbed(:user, name: 'Joe')
awards = Array.new(3, build_stubbed(:award_emoji, user: user))
it "returns a comma seperated list of 1-9 users" do expect(award_user_list(awards, nil, limit: 3))
expect(award_user_list(awards.first(9), nil)).to eq(awards.first(9).map { |a| a.user.name }.to_sentence) .to eq('Joe, Joe, and Joe')
end end
it "displays the current user's name as 'You'" do it "displays the current user's name as 'You'" do
expect(award_user_list(awards.first(1), awards[0].user)).to eq('You') user = build_stubbed(:user, name: 'Joe')
award = build_stubbed(:award_emoji, user: user)
expect(award_user_list([award], user)).to eq('You')
expect(award_user_list([award], nil)).to eq 'Joe'
end end
it "truncates lists of larger than 9 users" do it "truncates lists" do
expect(award_user_list(awards, nil)).to eq(awards.first(9).map { |a| a.user.name }.join(', ') + ", and 6 more.") user = build_stubbed(:user, name: 'Jane')
awards = Array.new(5, build_stubbed(:award_emoji, user: user))
expect(award_user_list(awards, nil, limit: 3))
.to eq('Jane, Jane, Jane, and 2 more.')
end end
it "displays the current user in front of 0-9 other users" do it "displays the current user in front of other users" do
expect(award_user_list(awards, awards[0].user)). current_user = build_stubbed(:user)
to eq("You, " + awards[1..9].map { |a| a.user.name }.join(', ') + ", and 5 more.") my_award = build_stubbed(:award_emoji, user: current_user)
end award = build_stubbed(:award_emoji, user: build_stubbed(:user, name: 'Jane'))
awards = Array.new(5, award).push(my_award)
it "displays the current user in front regardless of position in the list" do expect(award_user_list(awards, current_user, limit: 2)).
expect(award_user_list(awards, awards[12].user)). to eq("You, Jane, and 4 more.")
to eq("You, " + awards[0..8].map { |a| a.user.name }.join(', ') + ", and 5 more.")
end end
end end