Commit 0f2bb4ab authored by Robert Speicher's avatar Robert Speicher

Merge branch 'gitlab-employee-badge-bot-fix' into 'master'

Restrict GitLab Employee Badge to only human users

See merge request gitlab-org/gitlab!29196
parents d6e882ce 93510544
......@@ -196,7 +196,7 @@ module IssuablesHelper
author_output = link_to_member(project, issuable.author, size: 24, mobile_classes: "d-none d-sm-inline")
author_output << link_to_member(project, issuable.author, size: 24, by_username: true, avatar: false, mobile_classes: "d-inline d-sm-none")
author_output << gitlab_team_member_badge(issuable.author, css_class: 'ml-1')
author_output << issuable_meta_author_slot(issuable.author, css_class: 'ml-1')
if status = user_status(issuable.author)
author_output << "#{status}".html_safe
......@@ -213,6 +213,11 @@ module IssuablesHelper
output.join.html_safe
end
# This is a dummy method, and has an override defined in ee
def issuable_meta_author_slot(author, css_class: nil)
nil
end
def issuable_labels_tooltip(labels, limit: 5)
first, last = labels.partition.with_index { |_, i| i < limit }
......@@ -242,27 +247,6 @@ module IssuablesHelper
html.html_safe
end
def gitlab_team_member_badge(author, css_class: nil)
return unless author.gitlab_employee?
default_css_class = 'd-inline-block align-middle'
gitlab_team_member = _('GitLab Team Member')
content_tag(
:span,
class: css_class ? "#{default_css_class} #{css_class}" : default_css_class,
data: { toggle: 'tooltip', title: gitlab_team_member, container: 'body' },
role: 'img',
aria: { label: gitlab_team_member }
) do
sprite_icon(
'tanuki-verified',
size: 16,
css_class: 'gl-text-purple d-block'
)
end
end
def issuable_first_contribution_icon
content_tag(:span, class: 'fa-stack') do
concat(icon('certificate', class: "fa-stack-2x"))
......
......@@ -683,6 +683,10 @@ class User < ApplicationRecord
without_ghosts.with_project_bots
end
def human?
user_type.nil?
end
#
# Instance methods
#
......@@ -1700,16 +1704,6 @@ class User < ApplicationRecord
callouts.any?
end
def gitlab_employee?
strong_memoize(:gitlab_employee) do
if Feature.enabled?(:gitlab_employee_badge) && Gitlab.com?
Mail::Address.new(email).domain == "gitlab.com" && confirmed?
else
false
end
end
end
# Load the current highest access by looking directly at the user's memberships
def current_highest_access_level
members.non_request.maximum(:access_level)
......@@ -1719,10 +1713,6 @@ class User < ApplicationRecord
!confirmed? && !confirmation_period_valid?
end
def organization
gitlab_employee? ? 'GitLab' : super
end
protected
# override, from Devise::Validatable
......
# frozen_string_literal: true
class NoteUserEntity < UserEntity
expose :gitlab_employee?, as: :is_gitlab_employee, if: ->(user, options) { user.gitlab_employee? }
unexpose :web_url
end
NoteUserEntity.prepend_if_ee('EE::NoteUserEntity')
......@@ -101,7 +101,7 @@
- else
= f.text_field :location, label: s_('Profiles|Location'), class: 'input-lg', placeholder: s_("Profiles|City, country")
= f.text_field :job_title, class: 'input-md'
= f.text_field :organization, readonly: @user.gitlab_employee?, label: s_('Profiles|Organization'), class: 'input-md', help: s_("Profiles|Who you represent or work for")
= render 'shared/profiles/organization', { form: f, user: @user }
= f.text_area :bio, label: s_('Profiles|Bio'), rows: 4, maxlength: 250, help: s_("Profiles|Tell us about yourself in fewer than 250 characters")
%hr
%h5= s_("Private profile")
......
......@@ -24,7 +24,7 @@
&middot;
opened #{time_ago_with_tooltip(issue.created_at, placement: 'bottom')}
by #{link_to_member(@project, issue.author, avatar: false)}
= gitlab_team_member_badge(issue.author)
= render_if_exists 'shared/issuable/gitlab_team_member_badge', {author: issue.author}
- if issue.milestone
%span.issuable-milestone.d-none.d-sm-inline-block
&nbsp;
......
......@@ -20,7 +20,7 @@
&middot;
opened #{time_ago_with_tooltip(merge_request.created_at, placement: 'bottom')}
by #{link_to_member(@project, merge_request.author, avatar: false)}
= gitlab_team_member_badge(merge_request.author)
= render_if_exists 'shared/issuable/gitlab_team_member_badge', {author: merge_request.author}
- if merge_request.milestone
%span.issuable-milestone.d-none.d-sm-inline-block
&nbsp;
......
= form.text_field :organization, label: s_('Profiles|Organization'), class: 'input-md', help: s_("Profiles|Who you represent or work for")
......@@ -27,5 +27,31 @@ module EE
data
end
override :issuable_meta_author_slot
def issuable_meta_author_slot(author, css_class: nil)
gitlab_team_member_badge(author, css_class: css_class)
end
def gitlab_team_member_badge(author, css_class: nil)
return unless author.gitlab_employee?
default_css_class = 'd-inline-block align-middle'
gitlab_team_member = _('GitLab Team Member')
content_tag(
:span,
class: css_class ? "#{default_css_class} #{css_class}" : default_css_class,
data: { toggle: 'tooltip', title: gitlab_team_member, container: 'body' },
role: 'img',
aria: { label: gitlab_team_member }
) do
sprite_icon(
'tanuki-verified',
size: 16,
css_class: 'gl-text-purple d-block'
)
end
end
end
end
......@@ -335,6 +335,20 @@ module EE
filter == UserPreference::FEATURE_FILTER_EXPERIMENT
end
def gitlab_employee?
strong_memoize(:gitlab_employee) do
if ::Gitlab.com? && ::Feature.enabled?(:gitlab_employee_badge)
confirmed? && human? && Mail::Address.new(email).domain == "gitlab.com"
else
false
end
end
end
def organization
gitlab_employee? ? 'GitLab' : super
end
protected
override :password_required?
......
# frozen_string_literal: true
module EE
module NoteUserEntity
extend ActiveSupport::Concern
prepended do
expose :gitlab_employee?, as: :is_gitlab_employee, if: ->(user, options) { user.gitlab_employee? }
end
end
end
= form.text_field :organization, readonly: user.gitlab_employee?, label: s_('Profiles|Organization'), class: 'input-md', help: s_("Profiles|Who you represent or work for")
......@@ -258,6 +258,61 @@ describe Projects::IssuesController do
it_behaves_like 'user cannot see confidential issue', Gitlab::Access::NO_ACCESS
end
end
context 'is_gitlab_employee attribute' do
subject { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } }
before do
allow(Gitlab).to receive(:com?).and_return(true)
note_user = discussion.author
note_user.update(email: email)
note_user.confirm
end
shared_examples 'non inclusion of gitlab employee badge' do
it 'does not render the is_gitlab_employee attribute' do
subject
note_json = json_response.first['notes'].first
expect(note_json['author']['is_gitlab_employee']).to be nil
end
end
context 'when user is a gitlab employee' do
let(:email) { 'test@gitlab.com' }
it 'renders the is_gitlab_employee attribute' do
subject
note_json = json_response.first['notes'].first
expect(note_json['author']['is_gitlab_employee']).to be true
end
context 'when feature flag is disabled' do
before do
stub_feature_flags(gitlab_employee_badge: false)
end
it_behaves_like 'non inclusion of gitlab employee badge'
end
end
context 'when user is not a gitlab employee' do
let(:email) { 'test@example.com' }
it_behaves_like 'non inclusion of gitlab employee badge'
context 'when feature flag is disabled' do
before do
stub_feature_flags(gitlab_employee_badge: false)
end
it_behaves_like 'non inclusion of gitlab employee badge'
end
end
end
end
it_behaves_like DescriptionDiffActions do
......
......@@ -51,5 +51,57 @@ describe IssuablesHelper do
expect(helper.issuable_initial_data(issue)).to include(canAdmin: true)
end
end
describe '#gitlab_team_member_badge' do
let(:issue) { build(:issue, author: user) }
before do
allow(Gitlab).to receive(:com?).and_return(true)
end
context 'when `:gitlab_employee_badge` feature flag is disabled' do
let(:user) { build(:user, email: 'test@gitlab.com') }
before do
stub_feature_flags(gitlab_employee_badge: false)
end
it 'returns nil' do
expect(helper.gitlab_team_member_badge(issue.author)).to be_nil
end
end
context 'when issue author is not a GitLab team member' do
let(:user) { build(:user, email: 'test@example.com') }
it 'returns nil' do
expect(helper.gitlab_team_member_badge(issue.author)).to be_nil
end
end
context 'when issue author is a GitLab team member' do
let(:user) { build(:user, email: 'test@gitlab.com') }
it 'returns span with svg icon' do
expect(helper.gitlab_team_member_badge(issue.author)).to have_selector('span > svg')
end
context 'when `css_class` parameter is passed' do
it 'adds CSS classes' do
expect(helper.gitlab_team_member_badge(issue.author, css_class: 'foo bar baz')).to have_selector('span.foo.bar.baz')
end
end
end
end
describe '#issuable_meta_author_slot' do
it 'invoked gitlab_team_member_badge method' do
user = double
expect(helper).to receive(:gitlab_team_member_badge).with(user, css_class: nil)
helper.issuable_meta_author_slot(user)
end
end
end
end
......@@ -1104,4 +1104,76 @@ describe User do
end
end
end
describe '#gitlab_employee?' do
using RSpec::Parameterized::TableSyntax
subject { user.gitlab_employee? }
where(:email, :is_com, :expected_result) do
'test@gitlab.com' | true | true
'test@example.com' | true | false
'test@gitlab.com' | false | false
'test@example.com' | false | false
end
with_them do
let(:user) { build(:user, email: email) }
before do
allow(Gitlab).to receive(:com?).and_return(is_com)
end
it { is_expected.to be expected_result }
end
context 'when email is of Gitlab and is not confirmed' do
let(:user) { build(:user, email: 'test@gitlab.com', confirmed_at: nil) }
it { is_expected.to be false }
end
context 'when user is a bot' do
let(:user) { build(:user, email: 'test@gitlab.com', user_type: :alert_bot) }
it { is_expected.to be false }
end
context 'when user is ghost' do
let(:user) { build(:user, email: 'test@gitlab.com', ghost: true) }
it { is_expected.to be false }
end
context 'when `:gitlab_employee_badge` feature flag is disabled' do
let(:user) { build(:user, email: 'test@gitlab.com') }
before do
stub_feature_flags(gitlab_employee_badge: false)
end
it { is_expected.to be false }
end
end
describe '#organization' do
using RSpec::Parameterized::TableSyntax
let(:user) { build(:user, organization: 'ACME') }
subject { user.organization }
where(:gitlab_employee?, :expected_result) do
true | 'GitLab'
false | 'ACME'
end
with_them do
before do
allow(user).to receive(:gitlab_employee?).and_return(gitlab_employee?)
end
it { is_expected.to eql(expected_result) }
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'profiles/show' do
context 'gitlab.com organization field' do
before do
assign(:user, user)
allow(controller).to receive(:current_user).and_return(user)
allow(view).to receive(:experiment_enabled?)
allow(Gitlab).to receive(:com?).and_return(true)
end
context 'when `:gitlab_employee_badge` feature flag is enabled' do
context 'and when user has an `@gitlab.com` email address' do
let(:user) { create(:user, email: 'test@gitlab.com') }
it 'displays the organization field as `readonly` with a `value` of `GitLab`' do
render
expect(rendered).to have_selector('#user_organization[readonly][value="GitLab"]')
end
end
context 'and when a user does not have an `@gitlab.com` email' do
let(:user) { create(:user, email: 'test@example.com') }
it 'displays an editable organization field' do
render
expect(rendered).to have_selector('#user_organization:not([readonly]):not([value="GitLab"])')
end
end
end
context 'when `:gitlab_employee_badge` feature flag is disabled' do
before do
stub_feature_flags(gitlab_employee_badge: false)
end
context 'and when a user has an `@gitlab.com` email' do
let(:user) { create(:user, email: 'test@gitlab.com') }
it 'displays an editable organization field' do
render
expect(rendered).to have_selector('#user_organization:not([readonly]):not([value="GitLab"])')
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'projects/issues/show' do
include_context 'project show action'
context 'when issue is created by a GitLab team member' do
let(:user) { create(:user, email: 'test@gitlab.com') }
before do
allow(Gitlab).to receive(:com?).and_return(true)
end
it 'renders an employee badge next to their name' do
render
expect(rendered).to have_selector('[aria-label="GitLab Team Member"]')
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'projects/merge_requests/show.html.haml' do
include_context 'merge request show action'
context 'when merge request is created by a GitLab team member' do
let(:user) { create(:user, email: 'test@gitlab.com') }
before do
allow(Gitlab).to receive(:com?).and_return(true)
end
it 'renders an employee badge next to their name' do
render
expect(rendered).to have_selector('[aria-label="GitLab Team Member"]')
end
end
end
......@@ -1515,61 +1515,6 @@ describe Projects::IssuesController do
expect(note_json['author']['status_tooltip_html']).to be_present
end
context 'is_gitlab_employee attribute' do
subject { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } }
before do
allow(Gitlab).to receive(:com?).and_return(true)
note_user = discussion.author
note_user.update(email: email)
note_user.confirm
end
shared_examples 'non inclusion of gitlab employee badge' do
it 'does not render the is_gitlab_employee attribute' do
subject
note_json = json_response.first['notes'].first
expect(note_json['author']['is_gitlab_employee']).to be nil
end
end
context 'when user is a gitlab employee' do
let(:email) { 'test@gitlab.com' }
it 'renders the is_gitlab_employee attribute' do
subject
note_json = json_response.first['notes'].first
expect(note_json['author']['is_gitlab_employee']).to be true
end
context 'when feature flag is disabled' do
before do
stub_feature_flags(gitlab_employee_badge: false)
end
it_behaves_like 'non inclusion of gitlab employee badge'
end
end
context 'when user is not a gitlab employee' do
let(:email) { 'test@example.com' }
it_behaves_like 'non inclusion of gitlab employee badge'
context 'when feature flag is disabled' do
before do
stub_feature_flags(gitlab_employee_badge: false)
end
it_behaves_like 'non inclusion of gitlab employee badge'
end
end
end
it 'does not cause an extra query for the status' do
control = ActiveRecord::QueryRecorder.new do
get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
......
......@@ -303,46 +303,4 @@ describe IssuablesHelper do
end
end
end
describe '#gitlab_team_member_badge' do
let(:issue) { build(:issue, author: user) }
before do
allow(Gitlab).to receive(:com?).and_return(true)
end
context 'when `:gitlab_employee_badge` feature flag is disabled' do
let(:user) { build(:user, email: 'test@gitlab.com') }
before do
stub_feature_flags(gitlab_employee_badge: false)
end
it 'returns nil' do
expect(helper.gitlab_team_member_badge(issue.author)).to be_nil
end
end
context 'when issue author is not a GitLab team member' do
let(:user) { build(:user, email: 'test@example.com') }
it 'returns nil' do
expect(helper.gitlab_team_member_badge(issue.author)).to be_nil
end
end
context 'when issue author is a GitLab team member' do
let(:user) { build(:user, email: 'test@gitlab.com') }
it 'returns span with svg icon' do
expect(helper.gitlab_team_member_badge(issue.author)).to have_selector('span > svg')
end
context 'when `css_class` parameter is passed' do
it 'adds CSS classes' do
expect(helper.gitlab_team_member_badge(issue.author, css_class: 'foo bar baz')).to have_selector('span.foo.bar.baz')
end
end
end
end
end
......@@ -4458,45 +4458,6 @@ describe User, :do_not_mock_admin_mode do
end
end
describe '#gitlab_employee?' do
using RSpec::Parameterized::TableSyntax
subject { user.gitlab_employee? }
where(:email, :is_com, :expected_result) do
'test@gitlab.com' | true | true
'test@example.com' | true | false
'test@gitlab.com' | false | false
'test@example.com' | false | false
end
with_them do
let(:user) { build(:user, email: email) }
before do
allow(Gitlab).to receive(:com?).and_return(is_com)
end
it { is_expected.to be expected_result }
end
context 'when email is of Gitlab and is not confirmed' do
let(:user) { build(:user, email: 'test@gitlab.com', confirmed_at: nil) }
it { is_expected.to be false }
end
context 'when `:gitlab_employee_badge` feature flag is disabled' do
let(:user) { build(:user, email: 'test@gitlab.com') }
before do
stub_feature_flags(gitlab_employee_badge: false)
end
it { is_expected.to be false }
end
end
describe '#current_highest_access_level' do
let_it_be(:user) { create(:user) }
......@@ -4517,27 +4478,6 @@ describe User, :do_not_mock_admin_mode do
end
end
describe '#organization' do
using RSpec::Parameterized::TableSyntax
let(:user) { build(:user, organization: 'ACME') }
subject { user.organization }
where(:gitlab_employee?, :expected_result) do
true | 'GitLab'
false | 'ACME'
end
with_them do
before do
allow(user).to receive(:gitlab_employee?).and_return(gitlab_employee?)
end
it { is_expected.to eql(expected_result) }
end
end
context 'when after_commit :update_highest_role' do
describe 'create user' do
subject { create(:user) }
......@@ -4685,4 +4625,26 @@ describe User, :do_not_mock_admin_mode do
it_behaves_like 'does not require password to be present'
end
end
describe '#human?' do
subject { user.human? }
let_it_be(:user) { create(:user) }
context 'when user is a human' do
before do
user.update(user_type: nil)
end
it { is_expected.to be true }
end
context 'when user is not a human' do
before do
user.update(user_type: 'alert_bot')
end
it { is_expected.to be false }
end
end
end
# frozen_string_literal: true
shared_context 'merge request show action' do
include Devise::Test::ControllerHelpers
include ProjectForksHelper
let(:user) { create(:user) }
let(:project) { create(:project, :public, :repository) }
let(:forked_project) { fork_project(project, user, repository: true) }
let(:unlink_project) { Projects::UnlinkForkService.new(forked_project, user) }
let(:note) { create(:note_on_merge_request, project: project, noteable: closed_merge_request) }
let(:closed_merge_request) do
create(:closed_merge_request,
source_project: forked_project,
target_project: project,
author: user)
end
def preload_view_requirements
# This will load the status fields of the author of the note and merge request
# to avoid queries in when rendering the view being tested.
closed_merge_request.author.status
note.author.status
end
before do
assign(:project, project)
assign(:merge_request, closed_merge_request)
assign(:commits_count, 0)
assign(:note, note)
assign(:noteable, closed_merge_request)
assign(:notes, [])
assign(:pipelines, Ci::Pipeline.none)
assign(:issuable_sidebar, serialize_issuable_sidebar(user, project, closed_merge_request))
preload_view_requirements
allow(view).to receive_messages(current_user: user,
can?: true,
current_application_settings: Gitlab::CurrentSettings.current_application_settings)
end
def serialize_issuable_sidebar(user, project, merge_request)
MergeRequestSerializer
.new(current_user: user, project: project)
.represent(closed_merge_request, serializer: 'sidebar')
end
end
# frozen_string_literal: true
shared_context 'project show action' do
let(:project) { create(:project, :repository) }
let(:issue) { create(:issue, project: project, author: user) }
let(:user) { create(:user) }
before do
assign(:project, project)
assign(:issue, issue)
assign(:noteable, issue)
stub_template 'shared/issuable/_sidebar' => ''
stub_template 'projects/issues/_discussion' => ''
allow(view).to receive(:user_status).and_return('')
end
end
......@@ -19,48 +19,4 @@ describe 'profiles/show' do
expect(rendered).to have_field('user_id', with: user.id)
end
end
context 'gitlab.com organization field' do
before do
allow(Gitlab).to receive(:com?).and_return(true)
end
context 'when `:gitlab_employee_badge` feature flag is enabled' do
context 'and when user has an `@gitlab.com` email address' do
let(:user) { create(:user, email: 'test@gitlab.com') }
it 'displays the organization field as `readonly` with a `value` of `GitLab`' do
render
expect(rendered).to have_selector('#user_organization[readonly][value="GitLab"]')
end
end
context 'and when a user does not have an `@gitlab.com` email' do
let(:user) { create(:user, email: 'test@example.com') }
it 'displays an editable organization field' do
render
expect(rendered).to have_selector('#user_organization:not([readonly]):not([value="GitLab"])')
end
end
end
context 'when `:gitlab_employee_badge` feature flag is disabled' do
before do
stub_feature_flags(gitlab_employee_badge: false)
end
context 'and when a user has an `@gitlab.com` email' do
let(:user) { create(:user, email: 'test@gitlab.com') }
it 'displays an editable organization field' do
render
expect(rendered).to have_selector('#user_organization:not([readonly]):not([value="GitLab"])')
end
end
end
end
end
......@@ -3,18 +3,7 @@
require 'spec_helper'
describe 'projects/issues/show' do
let(:project) { create(:project, :repository) }
let(:issue) { create(:issue, project: project, author: user) }
let(:user) { create(:user) }
before do
assign(:project, project)
assign(:issue, issue)
assign(:noteable, issue)
stub_template 'shared/issuable/_sidebar' => ''
stub_template 'projects/issues/_discussion' => ''
allow(view).to receive(:user_status).and_return('')
end
include_context 'project show action'
context 'when the issue is closed' do
before do
......@@ -152,18 +141,4 @@ describe 'projects/issues/show' do
expect(rendered).not_to have_selector('#js-sentry-error-stack-trace')
end
end
context 'when issue is created by a GitLab team member' do
let(:user) { create(:user, email: 'test@gitlab.com') }
before do
allow(Gitlab).to receive(:com?).and_return(true)
end
it 'renders an employee badge next to their name' do
render
expect(rendered).to have_selector('[aria-label="GitLab Team Member"]')
end
end
end
......@@ -3,45 +3,7 @@
require 'spec_helper'
describe 'projects/merge_requests/show.html.haml' do
include Devise::Test::ControllerHelpers
include ProjectForksHelper
let(:user) { create(:user) }
let(:project) { create(:project, :public, :repository) }
let(:forked_project) { fork_project(project, user, repository: true) }
let(:unlink_project) { Projects::UnlinkForkService.new(forked_project, user) }
let(:note) { create(:note_on_merge_request, project: project, noteable: closed_merge_request) }
let(:closed_merge_request) do
create(:closed_merge_request,
source_project: forked_project,
target_project: project,
author: user)
end
def preload_view_requirements
# This will load the status fields of the author of the note and merge request
# to avoid queries in when rendering the view being tested.
closed_merge_request.author.status
note.author.status
end
before do
assign(:project, project)
assign(:merge_request, closed_merge_request)
assign(:commits_count, 0)
assign(:note, note)
assign(:noteable, closed_merge_request)
assign(:notes, [])
assign(:pipelines, Ci::Pipeline.none)
assign(:issuable_sidebar, serialize_issuable_sidebar(user, project, closed_merge_request))
preload_view_requirements
allow(view).to receive_messages(current_user: user,
can?: true,
current_application_settings: Gitlab::CurrentSettings.current_application_settings)
end
include_context 'merge request show action'
describe 'merge request assignee sidebar' do
context 'when assignee is allowed to merge' do
......@@ -92,24 +54,4 @@ describe 'projects/merge_requests/show.html.haml' do
expect(rendered).to have_css('a', visible: false, text: 'Close')
end
end
context 'when merge request is created by a GitLab team member' do
let(:user) { create(:user, email: 'test@gitlab.com') }
before do
allow(Gitlab).to receive(:com?).and_return(true)
end
it 'renders an employee badge next to their name' do
render
expect(rendered).to have_selector('[aria-label="GitLab Team Member"]')
end
end
def serialize_issuable_sidebar(user, project, merge_request)
MergeRequestSerializer
.new(current_user: user, project: project)
.represent(closed_merge_request, serializer: 'sidebar')
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