Merge branch...

Merge branch '330369-unassign-all-assignees-from-merge-request-via-rest-api-not-possible' into 'master'

Resolve "Unassign all assignees from merge request via Rest API not possible"

See merge request gitlab-org/gitlab!61301

app/services/merge_requests/update_assignees_service.rb

@@ -45,7 +45,7 @@ module MergeRequests
     end
 
     def assignee_ids
-      params.fetch(:assignee_ids).first(1)
+      params.fetch(:assignee_ids).reject { _1 == 0 }.first(1)
     end
 
     def params

changelogs/unreleased/330369-unassign-all-assignees-from-merge-request-via-rest-api-not-possibl.yml

+---
+title: 'Merge Request API: Treat 0 as a non-assigning sentinel value'
+merge_request: 61301
+author:
+type: fixed

ee/app/services/ee/merge_requests/update_assignees_service.rb

@@ -5,7 +5,7 @@ module EE
     module UpdateAssigneesService
       def assignee_ids
         if project.licensed_feature_available?(:multiple_merge_request_assignees)
-          params.fetch(:assignee_ids)
+          params.fetch(:assignee_ids).reject { _1 == 0 }
         else
           super
         end

ee/spec/services/ee/merge_requests/update_assignees_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequests::UpdateAssigneesService do
+  let_it_be(:group) { create(:group, :public) }
+  let_it_be(:project) { create(:project, :private, :repository, group: group) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:user2) { create(:user) }
+  let_it_be(:user3) { create(:user) }
+
+  let_it_be_with_reload(:merge_request) do
+    create(:merge_request, :simple, :unique_branches,
+           assignee_ids: [user.id],
+           source_project: project,
+           author: user)
+  end
+
+  before do
+    project.add_maintainer(user)
+    project.add_developer(user2)
+    project.add_developer(user3)
+  end
+
+  let(:service) { described_class.new(project: project, current_user: user, params: opts) }
+
+  describe 'execute' do
+    def update_merge_request
+      service.execute(merge_request)
+      merge_request.reload
+    end
+
+    context 'when the parameters are valid' do
+      context 'when using sentinel values' do
+        let(:opts) { { assignee_ids: [0, 0, 0] } }
+
+        it 'removes all assignees' do
+          expect { update_merge_request }.to change(merge_request, :assignees).to([])
+        end
+      end
+
+      context 'the assignee_ids parameter is the empty list' do
+        let(:opts) { { assignee_ids: [] } }
+
+        it 'removes all assignees' do
+          expect { update_merge_request }.to change(merge_request, :assignees).to([])
+        end
+      end
+
+      context 'the assignee_ids parameter contains both zeros and valid IDs' do
+        let(:opts) { { assignee_ids: [0, user2.id, 0, user3.id, 0] } }
+
+        it 'ignores 0 IDs' do
+          expect { update_merge_request }.to change(merge_request, :assignees).to([user2, user3])
+        end
+      end
+    end
+  end
+end

spec/requests/api/merge_requests_spec.rb

@@ -2154,7 +2154,7 @@ RSpec.describe API::MergeRequests do
     end
   end
 
-  describe 'PUT /projects/:id/merge_reuests/:merge_request_iid' do
+  describe 'PUT /projects/:id/merge_requests/:merge_request_iid' do
     it_behaves_like 'issuable update endpoint' do
       let(:entity) { merge_request }
     end
@@ -2176,6 +2176,68 @@ RSpec.describe API::MergeRequests do
       end
     end
 
+    context 'when assignee_id=user2.id' do
+      let(:params) do
+        {
+          assignee_id: user2.id
+        }
+      end
+
+      it 'sets the assignees' do
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: params
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['assignees']).to contain_exactly(
+          a_hash_including('name' => user2.name)
+        )
+      end
+    end
+
+    context 'when only assignee_ids are provided, and the list is empty' do
+      let(:params) do
+        {
+          assignee_ids: []
+        }
+      end
+
+      it 'clears the assignees' do
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: params
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['assignees']).to be_empty
+      end
+    end
+
+    context 'when only assignee_ids are provided, and the list contains the sentinel value' do
+      let(:params) do
+        {
+          assignee_ids: [0]
+        }
+      end
+
+      it 'clears the assignees' do
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: params
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['assignees']).to be_empty
+      end
+    end
+
+    context 'when only assignee_id=0' do
+      let(:params) do
+        {
+          assignee_id: 0
+        }
+      end
+
+      it 'clears the assignees' do
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: params
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['assignees']).to be_empty
+      end
+    end
+
     context 'accepts reviewer_ids' do
       let(:params) do
         {

spec/services/merge_requests/update_assignees_service_spec.rb

@@ -36,6 +36,22 @@ RSpec.describe MergeRequests::UpdateAssigneesService do
     end
 
     context 'when the parameters are valid' do
+      context 'when using sentinel values' do
+        let(:opts) { { assignee_ids: [0] } }
+
+        it 'removes all assignees' do
+          expect { update_merge_request }.to change(merge_request, :assignees).to([])
+        end
+      end
+
+      context 'the assignee_ids parameter is the empty list' do
+        let(:opts) { { assignee_ids: [] } }
+
+        it 'removes all assignees' do
+          expect { update_merge_request }.to change(merge_request, :assignees).to([])
+        end
+      end
+
       it 'updates the MR, and queues the more expensive work for later' do
         expect_next(MergeRequests::HandleAssigneesChangeService, project: project, current_user: user) do |service|
           expect(service)