Use gitlab-com group to check if GitLab employee

Modifies the logic to determine GitLab employee.
Checks if the user belongs to gitlab-com group
to determine if the user is a Gitlab employee or
not. The method is used to display the GitLab
employee badge.

ee/app/models/ee/user.rb

@@ -348,7 +348,7 @@ module EE
     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"
+          human? && ::Gitlab::Com.gitlab_com_group_member_id?(id)
         else
           false
         end

ee/lib/gitlab/com.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Com
+    ALLOWED_USER_IDS_KEY = 'gitlab_com_group_allowed_user_ids'
+    EXPIRY_TIME_L1_CACHE = 1.minute
+    EXPIRY_TIME_L2_CACHE = 5.minutes
+    GITLAB_COM_GROUP = 'gitlab-com'
+
+    def self.gitlab_com_group_member_id?(user_id = nil)
+      return false unless Gitlab.com? && user_id && ::Feature.enabled?(:gitlab_employee_badge)
+
+      gitlab_com_user_ids.include?(user_id)
+    end
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def self.gitlab_com_user_ids
+      l1_cache_backend.fetch(ALLOWED_USER_IDS_KEY, expires_in: EXPIRY_TIME_L1_CACHE) do
+        l2_cache_backend.fetch(ALLOWED_USER_IDS_KEY, expires_in: EXPIRY_TIME_L2_CACHE) do
+          group = Group.find_by_name(GITLAB_COM_GROUP)
+
+          if group
+            GroupMembersFinder.new(group).execute.pluck(:user_id)
+          else
+            []
+          end
+        end
+      end
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+    private_class_method :gitlab_com_user_ids
+
+    def self.expire_allowed_user_ids_cache
+      l1_cache_backend.delete(ALLOWED_USER_IDS_KEY)
+      l2_cache_backend.delete(ALLOWED_USER_IDS_KEY)
+    end
+
+    def self.l1_cache_backend
+      Gitlab::ProcessMemoryCache.cache_backend
+    end
+
+    def self.l2_cache_backend
+      Rails.cache
+    end
+  end
+end

ee/spec/controllers/projects/issues_controller_spec.rb

@@ -238,23 +238,21 @@ describe Projects::IssuesController do
       before do
         sign_in(user)
         allow(Gitlab).to receive(:com?).and_return(true)
-        note_user = discussion.author
-        note_user.update(email: email)
-        note_user.confirm
+        discussion.update(author: user)
       end
 
-      shared_examples 'non inclusion of gitlab employee badge' do
+      shared_context 'non inclusion of gitlab team member badge' do |result|
         it 'does not render the is_gitlab_employee attribute' do
           subject
 
           note_json = json_response.first['notes'].first
 
-          expect(note_json['author'].has_key?('is_gitlab_employee')).to be false
+          expect(note_json['author']['is_gitlab_employee']).to be result
         end
       end
 
-      context 'when user is a gitlab employee' do
-        let(:email) { 'test@gitlab.com' }
+      context 'when user is a gitlab team member' do
+        include_context 'gitlab team member'
 
         it 'renders the is_gitlab_employee attribute' do
           subject
@@ -269,27 +267,19 @@ describe Projects::IssuesController do
             stub_feature_flags(gitlab_employee_badge: false)
           end
 
-          it_behaves_like 'non inclusion of gitlab employee badge'
+          it_behaves_like 'non inclusion of gitlab team member badge', nil
         end
       end
 
-      context 'when user is not a gitlab employee' do
-        let(:email) { 'test@example.com' }
-
-        it 'shows is_gitlab_employee attribute as false' do
-          subject
-
-          note_json = json_response.first['notes'].first
-
-          expect(note_json['author']['is_gitlab_employee']).to be false
-        end
+      context 'when user is not a gitlab team member' do
+        it_behaves_like 'non inclusion of gitlab team member badge', false
 
         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'
+          it_behaves_like 'non inclusion of gitlab team member badge', nil
         end
       end
     end

ee/spec/helpers/ee/issuables_helper_spec.rb

@@ -53,6 +53,7 @@ describe IssuablesHelper do
     end
 
     describe '#gitlab_team_member_badge' do
+      let(:user) { create(:user) }
       let(:issue) { build(:issue, author: user) }
 
       before do
@@ -60,7 +61,7 @@ describe IssuablesHelper do
       end
 
       context 'when `:gitlab_employee_badge` feature flag is disabled' do
-        let(:user) { build(:user, email: 'test@gitlab.com') }
+        include_context 'gitlab team member'
 
         before do
           stub_feature_flags(gitlab_employee_badge: false)
@@ -72,15 +73,13 @@ describe IssuablesHelper do
       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') }
+        include_context 'gitlab team member'
 
         it 'returns span with svg icon' do
           expect(helper.gitlab_team_member_badge(issue.author)).to have_selector('span > svg')

ee/spec/lib/gitlab/com_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Com do
+  it { expect(described_class.l1_cache_backend).to eq(Gitlab::ProcessMemoryCache.cache_backend) }
+  it { expect(described_class.l2_cache_backend).to eq(Rails.cache) }
+
+  describe '.gitlab_team_member?' do
+    subject { described_class.gitlab_com_group_member_id?(user&.id) }
+
+    let(:user) { create(:user) }
+
+    before do
+      allow(Gitlab).to receive(:com?).and_return(true)
+    end
+
+    context 'when user is a gitlab team member' do
+      include_context 'gitlab team member'
+
+      it { is_expected.to be true }
+
+      context 'caching of allowed user IDs' do
+        before do
+          described_class.gitlab_com_group_member_id?(user&.id)
+        end
+
+        it_behaves_like 'allowed user IDs are cached'
+      end
+    end
+
+    context 'when user is not a gitlab team member' do
+      it { is_expected.to be false }
+
+      context 'caching of allowed user IDs' do
+        before do
+          described_class.gitlab_com_group_member_id?(user&.id)
+        end
+
+        it_behaves_like 'allowed user IDs are cached'
+      end
+    end
+
+    context 'when user is nil' do
+      let(:user) { nil }
+
+      it { is_expected.to be false }
+    end
+
+    context 'when gitlab-com group does not exist' do
+      before do
+        allow(Group).to receive(:find_by_name).and_return(nil)
+      end
+
+      it { is_expected.to be false }
+    end
+
+    context 'when feature flag is turned off' do
+      before do
+        stub_feature_flags(gitlab_employee_badge: false)
+      end
+
+      it { is_expected.to be false }
+    end
+  end
+end

ee/spec/models/user_spec.rb

@@ -1101,46 +1101,73 @@ describe User do
 
     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) }
+    let_it_be(:gitlab_group) { create(:group, name: 'gitlab-com') }
+    let_it_be(:random_group) { create(:group, name: 'random-group') }
 
+    context 'based on group membership' do
       before do
         allow(Gitlab).to receive(:com?).and_return(is_com)
       end
 
-      it { is_expected.to be expected_result }
-    end
+      context 'when user belongs to gitlab-com group' do
+        where(:is_com, :expected_result) do
+          true  | true
+          false | false
+        end
 
-    context 'when email is of Gitlab and is not confirmed' do
-      let(:user) { build(:user, email: 'test@gitlab.com', confirmed_at: nil) }
+        with_them do
+          let(:user) { create(:user) }
 
-      it { is_expected.to be false }
-    end
+          before do
+            gitlab_group.add_user(user, Gitlab::Access::DEVELOPER)
+          end
+
+          it { is_expected.to be expected_result }
+        end
+      end
 
-    context 'when user is a bot' do
-      let(:user) { build(:user, email: 'test@gitlab.com', user_type: :alert_bot) }
+      context 'when user does not belongs to gitlab-com group' do
+        where(:is_com, :expected_result) do
+          true  | false
+          false | false
+        end
 
-      it { is_expected.to be false }
+        with_them do
+          let(:user) { create(:user) }
+
+          before do
+            random_group.add_user(user, Gitlab::Access::DEVELOPER)
+          end
+
+          it { is_expected.to be expected_result }
+        end
+      end
     end
 
-    context 'when user is ghost' do
-      let(:user) { build(:user, :ghost, email: 'test@gitlab.com') }
+    context 'based on user type' do
+      before do
+        gitlab_group.add_user(user, Gitlab::Access::DEVELOPER)
+      end
 
-      it { is_expected.to be false }
+      context 'when user is a bot' do
+        let(:user) { build(:user, user_type: :alert_bot) }
+
+        it { is_expected.to be false }
+      end
+
+      context 'when user is ghost' do
+        let(:user) { build(:user, :ghost) }
+
+        it { is_expected.to be false }
+      end
     end
 
     context 'when `:gitlab_employee_badge` feature flag is disabled' do
-      let(:user) { build(:user, email: 'test@gitlab.com') }
+      let(:user) { build(:user) }
 
       before do
         stub_feature_flags(gitlab_employee_badge: false)
+        gitlab_group.add_user(user, Gitlab::Access::DEVELOPER)
       end
 
       it { is_expected.to be false }

ee/spec/support/shared_contexts/gitlab_team_member_shared_context.rb

+# frozen_string_literal: true
+
+shared_context 'gitlab team member' do
+  let_it_be(:namespace) { create(:group, name: 'gitlab-com') }
+
+  before do
+    namespace.add_developer(user)
+  end
+end

ee/spec/views/profiles/show.html.haml_spec.rb

@@ -4,6 +4,8 @@ require 'spec_helper'
 
 describe 'profiles/show' do
   context 'gitlab.com organization field' do
+    let(:user) { create(:user) }
+
     before do
       assign(:user, user)
       allow(controller).to receive(:current_user).and_return(user)
@@ -12,8 +14,8 @@ describe 'profiles/show' do
     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') }
+      context 'and when user is a gitlab team member' do
+        include_context 'gitlab team member'
 
         it 'displays the organization field as `readonly` with a `value` of `GitLab`' do
           render
@@ -22,9 +24,7 @@ describe 'profiles/show' do
         end
       end
 
-      context 'and when a user does not have an `@gitlab.com` email' do
-        let(:user) { create(:user, email: 'test@example.com') }
-
+      context 'and when a user is not a gitlab team member' do
         it 'displays an editable organization field' do
           render
 
@@ -38,8 +38,8 @@ describe 'profiles/show' 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') }
+      context 'and when a user is a gitlab team member' do
+        include_context 'gitlab team member'
 
         it 'displays an editable organization field' do
           render

ee/spec/views/projects/issues/show.html.haml_spec.rb

@@ -6,7 +6,9 @@ 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') }
+    let(:user) { create(:user) }
+
+    include_context 'gitlab team member'
 
     before do
       allow(Gitlab).to receive(:com?).and_return(true)

ee/spec/views/projects/merge_requests/show.html.haml_spec.rb

@@ -6,7 +6,9 @@ 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') }
+    let(:user) { create(:user) }
+
+    include_context 'gitlab team member'
 
     before do
       allow(Gitlab).to receive(:com?).and_return(true)

spec/lib/gitlab/performance_bar_spec.rb

@@ -3,41 +3,6 @@
 require 'spec_helper'
 
 describe Gitlab::PerformanceBar do
-  shared_examples 'allowed user IDs are cached' do
-    before do
-      # Warm the caches
-      described_class.enabled_for_user?(user)
-    end
-
-    it 'caches the allowed user IDs in cache', :use_clean_rails_memory_store_caching do
-      expect do
-        expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original
-        expect(described_class.l2_cache_backend).not_to receive(:fetch)
-        expect(described_class.enabled_for_user?(user)).to be_truthy
-      end.not_to exceed_query_limit(0)
-    end
-
-    it 'caches the allowed user IDs in L1 cache for 1 minute', :use_clean_rails_memory_store_caching do
-      Timecop.travel 2.minutes do
-        expect do
-          expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original
-          expect(described_class.l2_cache_backend).to receive(:fetch).and_call_original
-          expect(described_class.enabled_for_user?(user)).to be_truthy
-        end.not_to exceed_query_limit(0)
-      end
-    end
-
-    it 'caches the allowed user IDs in L2 cache for 5 minutes', :use_clean_rails_memory_store_caching do
-      Timecop.travel 6.minutes do
-        expect do
-          expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original
-          expect(described_class.l2_cache_backend).to receive(:fetch).and_call_original
-          expect(described_class.enabled_for_user?(user)).to be_truthy
-        end.not_to exceed_query_limit(2)
-      end
-    end
-  end
-
   it { expect(described_class.l1_cache_backend).to eq(Gitlab::ProcessMemoryCache.cache_backend) }
   it { expect(described_class.l2_cache_backend).to eq(Rails.cache) }
 
@@ -82,7 +47,16 @@ describe Gitlab::PerformanceBar do
           expect(described_class.enabled_for_user?(user)).to be_falsy
         end
 
-        it_behaves_like 'allowed user IDs are cached'
+        context 'caching of allowed user IDs' do
+          subject { described_class.enabled_for_user?(user) }
+
+          before do
+            # Warm the caches
+            described_class.enabled_for_user?(user)
+          end
+
+          it_behaves_like 'allowed user IDs are cached'
+        end
       end
 
       context 'when user is a member of the allowed group' do
@@ -94,7 +68,16 @@ describe Gitlab::PerformanceBar do
           expect(described_class.enabled_for_user?(user)).to be_truthy
         end
 
-        it_behaves_like 'allowed user IDs are cached'
+        context 'caching of allowed user IDs' do
+          subject { described_class.enabled_for_user?(user) }
+
+          before do
+            # Warm the caches
+            described_class.enabled_for_user?(user)
+          end
+
+          it_behaves_like 'allowed user IDs are cached'
+        end
       end
     end
 

spec/support/shared_contexts/cache_allowed_users_in_namespace_shared_context.rb

+# frozen_string_literal: true
+
+shared_examples 'allowed user IDs are cached' do
+  it 'caches the allowed user IDs in cache', :use_clean_rails_memory_store_caching do
+    expect do
+      expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original
+      expect(described_class.l2_cache_backend).not_to receive(:fetch)
+      expect(subject).to be_truthy
+    end.not_to exceed_query_limit(0)
+  end
+
+  it 'caches the allowed user IDs in L1 cache for 1 minute', :use_clean_rails_memory_store_caching do
+    Timecop.travel 2.minutes do
+      expect do
+        expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original
+        expect(described_class.l2_cache_backend).to receive(:fetch).and_call_original
+        expect(subject).to be_truthy
+      end.not_to exceed_query_limit(0)
+    end
+  end
+
+  it 'caches the allowed user IDs in L2 cache for 5 minutes', :use_clean_rails_memory_store_caching do
+    Timecop.travel 6.minutes do
+      expect do
+        expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original
+        expect(described_class.l2_cache_backend).to receive(:fetch).and_call_original
+        expect(subject).to be_truthy
+      end.not_to exceed_query_limit(2)
+    end
+  end
+end