Merge branch '212259-add-gitlab-employee-badge-to-assignees-be' into 'master'

Expose `gitlab_employee?` to issue/MR assignee component

Closes #216650

See merge request gitlab-org/gitlab!28375

app/serializers/issuable_sidebar_basic_entity.rb

@@ -21,7 +21,7 @@ class IssuableSidebarBasicEntity < Grape::Entity
   expose :labels, using: LabelEntity
 
   expose :current_user, if: lambda { |_issuable| current_user } do
-    expose :current_user, merge: true, using: API::Entities::UserBasic
+    expose :current_user, merge: true, using: ::API::Entities::UserBasic
 
     expose :todo, using: IssuableSidebarTodoEntity do |issuable|
       current_user.pending_todo_for(issuable)

app/serializers/issuable_sidebar_extras_entity.rb

@@ -21,5 +21,5 @@ class IssuableSidebarExtrasEntity < Grape::Entity
     issuable.subscribed?(request.current_user, issuable.project)
   end
 
-  expose :assignees, using: API::Entities::UserBasic
+  expose :assignees, using: ::API::Entities::UserBasic
 end

app/serializers/merge_request_assignee_entity.rb

@@ -5,3 +5,5 @@ class MergeRequestAssigneeEntity < ::API::Entities::UserBasic
     options[:merge_request]&.can_be_merged_by?(assignee)
   end
 end
+
+MergeRequestAssigneeEntity.prepend_if_ee('EE::MergeRequestAssigneeEntity')

ee/app/serializers/ee/merge_request_assignee_entity.rb

+# frozen_string_literal: true
+
+module EE
+  module MergeRequestAssigneeEntity
+    extend ActiveSupport::Concern
+
+    prepended do
+      expose :gitlab_employee?, as: :is_gitlab_employee, if: proc { ::Gitlab.com? && ::Feature.enabled?(:gitlab_employee_badge) }
+    end
+  end
+end

ee/lib/ee/api/entities/user_basic.rb

+# frozen_string_literal: true
+
+module EE
+  module API
+    module Entities
+      module UserBasic
+        extend ActiveSupport::Concern
+
+        prepended do
+          expose :gitlab_employee?, as: :is_gitlab_employee, if: proc { ::Gitlab.com? && ::Feature.enabled?(:gitlab_employee_badge) }
+        end
+      end
+    end
+  end
+end

ee/lib/ee/api/entities/user_path.rb

+# frozen_string_literal: true
+
+module EE
+  module API
+    module Entities
+      module UserPath
+        extend ActiveSupport::Concern
+
+        prepended do
+          expose :gitlab_employee?, as: :is_gitlab_employee, if: proc { ::Gitlab.com? && ::Feature.enabled?(:gitlab_employee_badge) }
+        end
+      end
+    end
+  end
+end

ee/spec/controllers/projects/issues_controller_spec.rb

@@ -236,6 +236,7 @@ describe Projects::IssuesController do
       subject { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } }
 
       before do
+        sign_in(user)
         allow(Gitlab).to receive(:com?).and_return(true)
         note_user = discussion.author
         note_user.update(email: email)
@@ -248,7 +249,7 @@ describe Projects::IssuesController do
 
           note_json = json_response.first['notes'].first
 
-          expect(note_json['author']['is_gitlab_employee']).to be nil
+          expect(note_json['author'].has_key?('is_gitlab_employee')).to be false
         end
       end
 
@@ -275,7 +276,13 @@ describe Projects::IssuesController do
       context 'when user is not a gitlab employee' do
         let(:email) { 'test@example.com' }
 
-        it_behaves_like 'non inclusion of gitlab employee badge'
+        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 feature flag is disabled' do
           before do
@@ -288,6 +295,67 @@ describe Projects::IssuesController do
     end
   end
 
+  describe 'PUT #update' do
+    let(:issue) { create(:issue, project: project) }
+
+    def update_issue(issue_params: {}, additional_params: {}, id: nil)
+      id ||= issue.iid
+      params = {
+        namespace_id: project.namespace.to_param,
+        project_id: project,
+        id: id,
+        issue: { title: 'New title' }.merge(issue_params),
+        format: :json
+      }.merge(additional_params)
+
+      put :update, params: params
+    end
+
+    context 'changing the assignee' do
+      let(:assignee) { create(:user) }
+
+      before do
+        project.add_developer(assignee)
+        sign_in(assignee)
+      end
+
+      context 'when the gitlab_employee_badge flag is off' do
+        it 'does not expose the is_gitlab_employee attribute on the assignee' do
+          stub_feature_flags(gitlab_employee_badge: false)
+
+          update_issue(issue_params: { assignee_ids: [assignee.id] })
+
+          expect(json_response['assignees'].first.keys)
+            .to match_array(%w(id name username avatar_url state web_url))
+        end
+      end
+
+      context 'when the gitlab_employee_badge flag is on but we are not on gitlab.com' do
+        it 'does not expose the is_gitlab_employee attribute on the assignee' do
+          stub_feature_flags(gitlab_employee_badge: true)
+          allow(Gitlab).to receive(:com?).and_return(false)
+
+          update_issue(issue_params: { assignee_ids: [assignee.id] })
+
+          expect(json_response['assignees'].first.keys)
+            .to match_array(%w(id name username avatar_url state web_url))
+        end
+      end
+
+      context 'when the gitlab_employee_badge flag is on and we are on gitlab.com' do
+        it 'exposes the is_gitlab_employee attribute on the assignee' do
+          stub_feature_flags(gitlab_employee_badge: true)
+          allow(Gitlab).to receive(:com?).and_return(true)
+
+          update_issue(issue_params: { assignee_ids: [assignee.id] })
+
+          expect(json_response['assignees'].first.keys)
+            .to match_array(%w(id name username avatar_url state web_url is_gitlab_employee))
+        end
+      end
+    end
+  end
+
   it_behaves_like DescriptionDiffActions do
     let_it_be(:project)  { create(:project_empty_repo, :public) }
     let_it_be(:issuable) { create(:issue, project: project) }

ee/spec/fixtures/api/schemas/entities/discussion.json

+{
+  "type": "object",
+  "allOf": [
+    { "$ref": "../../../../../../spec/fixtures/api/schemas/entities/discussion.json" },
+    {
+      "properties" : {
+        "notes": {
+          "items": {
+            "properties" : {
+              "author": {
+                "properties": {
+                  "is_gitlab_employee": { "type": "boolean"}
+                },
+                "additionalProperties": false
+              }
+            }
+          }
+        }
+      }
+    }
+  ]
+}

ee/spec/fixtures/api/schemas/entities/note_user_entity.json

+{
+  "type": "object",
+  "allOf": [
+    { "$ref": "../../../../../../spec/fixtures/api/schemas/entities/note_user_entity.json" },
+    {
+      "properties" : {
+        "id": { "type": "integer" },
+        "state": { "type": "string" },
+        "avatar_url": { "type": "string" },
+        "path": { "type": "string" },
+        "name": { "type": "string" },
+        "username": { "type": "string" },
+        "status_tooltip_html": { "$ref": "../../../../../../spec/fixtures/api/schemas/types/nullable_string.json" },
+        "is_gitlab_employee": { "type": "boolean" }
+      },
+      "additionalProperties": false,
+      "required": [
+        "id",
+        "state",
+        "avatar_url",
+        "path",
+        "name",
+        "username"
+      ]
+    }
+  ]
+}

ee/spec/fixtures/api/schemas/entities/user.json

+{
+  "type": "object",
+  "allOf": [
+    { "$ref": "../../../../../../spec/fixtures/api/schemas/entities/user.json" },
+    {
+      "required" : [
+        "id",
+        "state",
+        "avatar_url",
+        "web_url",
+        "path",
+        "name",
+        "username"
+      ],
+      "properties" : {
+        "id": { "type": "integer" },
+        "state": { "type": "string" },
+        "avatar_url": { "type": "string" },
+        "web_url": { "type": "string" },
+        "path": { "type": "string" },
+        "name": { "type": "string" },
+        "username": { "type": "string" },
+        "status_tooltip_html": { "$ref": "../../../../../../spec/fixtures/api/schemas/types/nullable_string.json" },
+        "is_gitlab_employee": { "type": "boolean" }
+      },
+      "additionalProperties": false
+    }
+  ]
+}

ee/spec/fixtures/api/schemas/pipeline_schedule.json

+{
+  "type": "object",
+  "allOf": [
+    { "$ref": "../../../../../spec/fixtures/api/schemas/pipeline_schedule.json" },
+    {
+      "properties": {
+        "owner": {
+          "properties": {
+            "name": { "type": "string" },
+            "username": { "type": "string" },
+            "id": { "type": "integer" },
+            "state": { "type": "string" },
+            "avatar_url": { "type": "uri" },
+            "web_url": { "type": "uri" },
+            "is_gitlab_employee": { "type": "boolean" }
+          },
+          "required" : [
+            "id", "name", "username", "state", "avatar_url", "web_url"
+          ],
+          "additionalProperties": false
+        }
+      }
+    }
+  ]
+}

ee/spec/fixtures/api/schemas/public_api/v4/issue.json

+{
+  "type": "object",
+  "allOf": [
+    { "$ref": "../../../../../../../spec/fixtures/api/schemas/public_api/v4/issue.json" },
+    {
+      "properties" : {
+        "author": {
+          "properties": {
+            "is_gitlab_employee": { "type": "boolean" }
+          },
+          "additionalProperties": false
+        }
+      }
+    }
+  ]
+}
\ No newline at end of file

ee/spec/fixtures/api/schemas/public_api/v4/members.json

+{
+  "type": "array",
+  "items": {
+    "type": "object",
+    "properties" : {
+      "id": { "type": "integer" },
+      "name": { "type": "string" },
+      "username": { "type": "string" },
+      "state": { "type": "string" },
+      "avatar_url": { "type": ["string", "null"] },
+      "web_url": { "type": ["string", "null"] },
+      "access_level": { "type": "integer" },
+      "expires_at": { "type": ["date", "null"] },
+      "is_using_seat": { "type": "boolean" },
+      "is_gitlab_employee": { "type": "boolean" }
+    },
+    "required": [
+      "id", "name", "username", "state",
+      "web_url", "access_level", "expires_at", "avatar_url"
+    ]
+  }
+}

ee/spec/fixtures/api/schemas/public_api/v4/notes.json

+{
+  "type": "array",
+  "allOf": [
+    { "$ref": "../../../../../../../spec/fixtures/api/schemas/public_api/v4/notes.json" },
+    {
+      "items": {
+        "properties" : {
+          "author": {
+            "properties": {
+              "name": { "type": "string" },
+              "username": { "type": "string" },
+              "id": { "type": "integer" },
+              "state": { "type": "string" },
+              "avatar_url": { "type": "uri" },
+              "web_url": { "type": "uri" },
+              "is_gitlab_employee": { "type": "boolean" }
+            },
+            "required" : [
+              "id", "name", "username", "state", "avatar_url", "web_url"
+            ],
+            "additionalProperties": false
+          }
+        }
+      }
+    }
+  ]
+}

ee/spec/fixtures/api/schemas/public_api/v4/snippets.json

+{
+  "type": "array",
+  "allOf": [
+    { "$ref": "../../../../../../../spec/fixtures/api/schemas/public_api/v4/snippets.json" },
+    {
+      "items": {
+        "properties" : {
+          "author": {
+            "properties": {
+              "name": { "type": "string" },
+              "username": { "type": "string" },
+              "id": { "type": "integer" },
+              "state": { "type": "string" },
+              "avatar_url": { "type": "uri" },
+              "web_url": { "type": "uri" },
+              "is_gitlab_employee": { "type": "boolean" }
+            },
+            "required" : [
+              "id", "name", "username", "state", "avatar_url", "web_url"
+            ],
+            "additionalProperties": false
+          }
+        }
+      }
+    }
+  ]
+}

ee/spec/requests/api/members_spec.rb

@@ -34,7 +34,38 @@ describe API::Members do
         get api("/groups/#{group.to_param}/members", owner)
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to match_response_schema('public_api/v4/members')
+        expect(response).to match_response_schema('public_api/v4/members', dir: 'ee')
+      end
+
+      context 'when the flag gitlab_employee_badge is on and we are on gitlab.com' do
+        it 'includes is_gitlab_employee in the response' do
+          stub_feature_flags(gitlab_employee_badge: true)
+          allow(Gitlab).to receive(:com?).and_return(true)
+
+          get api("/groups/#{group.to_param}/members", owner)
+
+          expect(json_response.first.keys).to include('is_gitlab_employee')
+        end
+      end
+
+      context 'when the flag gitlab_employee_badge is off' do
+        it 'does not include is_gitlab_employee in the response' do
+          stub_feature_flags(gitlab_employee_badge: false)
+
+          get api("/groups/#{group.to_param}/members", owner)
+
+          expect(json_response.first.keys).not_to include('is_gitlab_employee')
+        end
+      end
+
+      context 'when we are not on gitlab.com' do
+        it 'does not include is_gitlab_employee in the response' do
+          allow(Gitlab).to receive(:com?).and_return(false)
+
+          get api("/groups/#{group.to_param}/members", owner)
+
+          expect(json_response.first.keys).not_to include('is_gitlab_employee')
+        end
       end
 
       context 'when a group has SAML provider configured' do

ee/spec/requests/api/projects_spec.rb

@@ -346,6 +346,72 @@ describe API::Projects do
     end
   end
 
+  describe 'GET /projects/:id/users' do
+    shared_examples_for 'project users response' do
+      it 'returns the project users' do
+        get api("/projects/#{project.id}/users", current_user)
+
+        user = project.namespace.owner
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.size).to eq(1)
+
+        first_user = json_response.first
+        expect(first_user['username']).to eq(user.username)
+        expect(first_user['name']).to eq(user.name)
+      end
+
+      context 'when the gitlab_employee_badge flag is off' do
+        it 'does not expose the is_gitlab_employee attribute on the user' do
+          stub_feature_flags(gitlab_employee_badge: false)
+
+          get api("/projects/#{project.id}/users", current_user)
+
+          expect(json_response.first.keys).to contain_exactly(*%w[name username id state avatar_url web_url])
+        end
+      end
+
+      context 'when the gitlab_employee_badge flag is on but we are not on gitlab.com' do
+        it 'does not expose the is_gitlab_employee attribute on the user' do
+          stub_feature_flags(gitlab_employee_badge: true)
+          allow(Gitlab).to receive(:com?).and_return(false)
+
+          get api("/projects/#{project.id}/users", current_user)
+
+          expect(json_response.first.keys).to contain_exactly(*%w[name username id state avatar_url web_url])
+        end
+      end
+
+      context 'when the gitlab_employee_badge flag is on and we are on gitlab.com' do
+        it 'exposes the is_gitlab_employee attribute on the user' do
+          stub_feature_flags(gitlab_employee_badge: true)
+          allow(Gitlab).to receive(:com?).and_return(true)
+
+          get api("/projects/#{project.id}/users", current_user)
+
+          expect(json_response.first.keys).to contain_exactly(*%w[name username id state avatar_url web_url is_gitlab_employee])
+        end
+      end
+    end
+
+    context 'when unauthenticated' do
+      it_behaves_like 'project users response' do
+        let(:project) { create(:project, :public) }
+        let(:current_user) { nil }
+      end
+    end
+
+    context 'when authenticated' do
+      context 'valid request' do
+        it_behaves_like 'project users response' do
+          let(:current_user) { user }
+        end
+      end
+    end
+  end
+
   describe 'POST /projects/user/:id' do
     let(:admin) { create(:admin) }
     let(:api_call) { post api("/projects/user/#{user.id}", admin), params: project_params }

ee/spec/serializers/issuable_sidebar_extras_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe IssuableSidebarExtrasEntity do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :repository) }
+  let(:resource) { create(:issue, project: project, assignees: [user]) }
+  let(:request) { double('request', current_user: user) }
+
+  subject { described_class.new(resource, request: request).as_json }
+
+  context 'when the gitlab_employee_badge flag is off' do
+    it 'does not expose the is_gitlab_employee field for assignees' do
+      stub_feature_flags(gitlab_employee_badge: false)
+
+      expect(subject[:assignees].first).not_to include(:is_gitlab_employee)
+    end
+  end
+
+  context 'when the gitlab_employee_badge flag is on but we are not on gitlab.com' do
+    it 'does not expose the is_gitlab_employee field for assignees' do
+      stub_feature_flags(gitlab_employee_badge: true)
+      allow(Gitlab).to receive(:com?).and_return(false)
+
+      expect(subject[:assignees].first).not_to include(:is_gitlab_employee)
+    end
+  end
+
+  context 'when gitlab_employee_badge flag is on and we are on gitlab.com' do
+    it 'exposes is_gitlab_employee field for assignees' do
+      stub_feature_flags(gitlab_employee_badge: true)
+      allow(Gitlab).to receive(:com?).and_return(true)
+
+      expect(subject[:assignees].first).to include(:is_gitlab_employee)
+    end
+  end
+end

ee/spec/serializers/merge_request_sidebar_basic_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe MergeRequestSidebarBasicEntity do
+  let(:project) { create :project, :repository }
+  let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+  let(:user) { create(:user) }
+
+  let(:request) { double('request', current_user: user, project: project) }
+
+  let(:entity) { described_class.new(merge_request, request: request).as_json }
+
+  describe '#current_user' do
+    context 'when the gitlab_employee_badge flag is off' do
+      it 'does not expose the is_gitlab_employee field for the current user' do
+        stub_feature_flags(gitlab_employee_badge: false)
+
+        expect(entity[:current_user].keys).to contain_exactly(
+          :id, :name, :username, :state, :avatar_url, :web_url, :todo,
+          :can_edit, :can_move, :can_admin_label, :can_merge
+        )
+      end
+    end
+
+    context 'when the gitlab_employee_badge flag is on but we are not on gitlab.com' do
+      it 'does not expose the is_gitlab_employee field for the current user' do
+        stub_feature_flags(gitlab_employee_badge: true)
+        allow(Gitlab).to receive(:com?).and_return(false)
+
+        expect(entity[:current_user].keys).to contain_exactly(
+          :id, :name, :username, :state, :avatar_url, :web_url, :todo,
+          :can_edit, :can_move, :can_admin_label, :can_merge
+        )
+      end
+    end
+
+    context 'when the gitlab_employee_badge flag is on and we are on gitlab.com' do
+      it 'exposes the is_gitlab_employee field for the current user' do
+        stub_feature_flags(gitlab_employee_badge: true)
+        allow(Gitlab).to receive(:com?).and_return(true)
+
+        expect(entity[:current_user].keys).to contain_exactly(
+          :id, :name, :username, :state, :avatar_url, :web_url, :todo,
+          :can_edit, :can_move, :can_admin_label, :can_merge, :is_gitlab_employee
+        )
+      end
+    end
+  end
+end

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

@@ -10,6 +10,7 @@ describe 'projects/merge_requests/show.html.haml' do
 
     before do
       allow(Gitlab).to receive(:com?).and_return(true)
+      allow(user).to receive(:gitlab_employee?).and_return(true)
     end
 
     it 'renders an employee badge next to their name' do

lib/api/entities/user_basic.rb

@@ -18,3 +18,5 @@ module API
     end
   end
 end
+
+API::Entities::UserBasic.prepend_if_ee('EE::API::Entities::UserBasic')

lib/api/entities/user_path.rb

@@ -12,3 +12,5 @@ module API
     end
   end
 end
+
+API::Entities::UserPath.prepend_if_ee('EE::API::Entities::UserPath')

spec/controllers/projects/issues_controller_spec.rb

@@ -825,7 +825,7 @@ describe Projects::IssuesController do
           update_issue(issue_params: { assignee_ids: [assignee.id] })
 
           expect(json_response['assignees'].first.keys)
-            .to match_array(%w(id name username avatar_url state web_url))
+            .to include(*%w(id name username avatar_url state web_url))
         end
       end
 
@@ -1408,6 +1408,7 @@ describe Projects::IssuesController do
     it 'render merge request as json' do
       create_merge_request
 
+      expect(response).to have_gitlab_http_status(:ok)
       expect(response).to match_response_schema('merge_request')
     end
 

spec/fixtures/api/schemas/entities/discussion.json

@@ -30,7 +30,14 @@
               "status_tooltip_html": { "type": ["string", "null"] },
               "path": { "type": "string" }
             }, 
-            "additionalProperties": false
+            "required": [
+              "id",
+              "state",
+              "avatar_url",
+              "path",
+              "name",
+              "username"
+            ]
           },
           "created_at": { "type": "date" },
           "updated_at": { "type": "date" },

spec/fixtures/api/schemas/entities/note_user_entity.json

@@ -16,6 +16,5 @@
     "name": { "type": "string" },
     "username": { "type": "string" },
     "status_tooltip_html": { "$ref": "../types/nullable_string.json" }
-  },
-  "additionalProperties": false
+  }
 }

spec/fixtures/api/schemas/entities/user.json

@@ -18,6 +18,5 @@
     "name": { "type": "string" },
     "username": { "type": "string" },
     "status_tooltip_html": { "$ref": "../types/nullable_string.json" }
-  },
-  "additionalProperties": false
+  }
 }

spec/fixtures/api/schemas/pipeline_schedule.json

@@ -30,7 +30,9 @@
         "avatar_url": { "type": "uri" },
         "web_url": { "type": "uri" }
       },
-      "additionalProperties": false
+      "required": [
+        "id", "name", "username", "state", "avatar_url", "web_url"
+      ]
     },
     "variables": {
       "type": "array",

spec/fixtures/api/schemas/public_api/v4/issue.json

@@ -71,7 +71,14 @@
         "avatar_url": { "type": "uri" },
         "web_url": { "type": "uri" }
       },
-      "additionalProperties": false
+      "required": [
+        "id",
+        "state",
+        "avatar_url",
+        "name",
+        "username",
+        "web_url"
+      ]
     },
     "user_notes_count": { "type": "integer" },
     "upvotes": { "type": "integer" },

spec/fixtures/api/schemas/public_api/v4/members.json

@@ -15,8 +15,7 @@
     },
     "required": [
       "id", "name", "username", "state",
-      "web_url", "access_level", "expires_at"
-    ],
-    "additionalProperties": false
+      "web_url", "access_level", "expires_at", "avatar_url"
+    ]
   }
 }

spec/fixtures/api/schemas/public_api/v4/notes.json

@@ -17,7 +17,9 @@
           "avatar_url": { "type": "uri" },
           "web_url": { "type": "uri" }
         },
-        "additionalProperties": false
+        "required" : [
+          "id", "name", "username", "state", "avatar_url", "web_url"
+        ]
       },
       "commands_changes": { "type": "object", "additionalProperties": true },
       "created_at": { "type": "date" },

spec/fixtures/api/schemas/public_api/v4/snippets.json

@@ -23,7 +23,9 @@
           "avatar_url": { "type": "uri" },
           "web_url": { "type": "uri" }
         },
-        "additionalProperties": false
+        "required" : [
+          "id", "name", "username", "state", "avatar_url", "web_url"
+        ]
       }
     },
     "required": [

spec/requests/api/projects_spec.rb

@@ -1806,7 +1806,7 @@ describe API::Projects do
         first_user = json_response.first
         expect(first_user['username']).to eq(user.username)
         expect(first_user['name']).to eq(user.name)
-        expect(first_user.keys).to contain_exactly(*%w[name username id state avatar_url web_url])
+        expect(first_user.keys).to include(*%w[name username id state avatar_url web_url])
       end
     end
 

spec/serializers/merge_request_sidebar_basic_entity_spec.rb

@@ -13,7 +13,7 @@ describe MergeRequestSidebarBasicEntity do
 
   describe '#current_user' do
     it 'contains attributes related to the current user' do
-      expect(entity[:current_user].keys).to contain_exactly(
+      expect(entity[:current_user].keys).to include(
         :id, :name, :username, :state, :avatar_url, :web_url, :todo,
         :can_edit, :can_move, :can_admin_label, :can_merge
       )