Merge branch...

Merge branch '235391-support-group-milestones-to-be-associated-with-project-releases-in-api-2' into 'master'

Add missing milestone tests to Release API

See merge request gitlab-org/gitlab!44157

app/finders/README.md

 # Finders
 
-This type of classes responsible for collection items based on different conditions.
-To prevent lookup methods in models like this:
+These types of classes are responsible for retrieving collection items based on different conditions.
+They prevent lookup methods in models like this:
+
 
 ```ruby
-class Project
+class Project < ApplicationRecord
   def issues_for_user_filtered_by(user, filter)
     # A lot of logic not related to project model itself
   end
@@ -13,7 +14,7 @@ end
 issues = project.issues_for_user_filtered_by(user, params)
 ```
 
-Better use this:
+The GitLab approach is to use a Finder:
 
 ```ruby
 issues = IssuesFinder.new(project, user, filter).execute

lib/api/releases.rb

@@ -152,7 +152,7 @@ module API
       end
 
       def authorize_create_evidence!
-        # This is a separate method so that EE can extend its behaviour
+        # extended in EE
       end
 
       def release
@@ -160,15 +160,15 @@ module API
       end
 
       def log_release_created_audit_event(release)
-        # This is a separate method so that EE can extend its behaviour
+        # extended in EE
       end
 
       def log_release_updated_audit_event
-        # This is a separate method so that EE can extend its behaviour
+        # extended in EE
       end
 
       def log_release_milestones_updated_audit_event
-        # This is a separate method so that EE can extend its behaviour
+        # extended in EE
       end
     end
   end

spec/requests/api/releases_spec.rb

@@ -598,7 +598,7 @@ RSpec.describe API::Releases do
           end
         end
 
-        context 'when create two assets' do
+        context 'when creating two assets' do
           let(:params) do
             base_params.merge({
               assets: {
@@ -758,6 +758,65 @@ RSpec.describe API::Releases do
         expect(response).to have_gitlab_http_status(:conflict)
       end
     end
+
+    context 'with milestones' do
+      let(:subject) { post api("/projects/#{project.id}/releases", maintainer), params: params }
+      let(:milestone) { create(:milestone, project: project, title: 'v1.0') }
+      let(:returned_milestones) { json_response['milestones'].map {|m| m['title']} }
+
+      before do
+        params.merge!(milestone_params)
+
+        subject
+      end
+
+      context 'with a project milestone' do
+        let(:milestone_params) { { milestones: [milestone.title] } }
+
+        it 'adds the milestone' do
+          expect(response).to have_gitlab_http_status(:created)
+          expect(returned_milestones).to match_array(['v1.0'])
+        end
+      end
+
+      context 'with multiple milestones' do
+        let(:milestone2) { create(:milestone, project: project, title: 'm2') }
+        let(:milestone_params) { { milestones: [milestone.title, milestone2.title] } }
+
+        it 'adds all milestones' do
+          expect(response).to have_gitlab_http_status(:created)
+          expect(returned_milestones).to match_array(['v1.0', 'm2'])
+        end
+      end
+
+      context 'with an empty milestone' do
+        let(:milestone_params) { { milestones: [] } }
+
+        it 'removes all milestones' do
+          expect(response).to have_gitlab_http_status(:created)
+          expect(json_response['milestones']).to be_nil
+        end
+      end
+
+      context 'with a non-existant milestone' do
+        let(:milestone_params) { { milestones: ['xyz'] } }
+
+        it 'returns a 400 error as milestone not found' do
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response['message']).to eq("Milestone(s) not found: xyz")
+        end
+      end
+
+      context 'with a milestone from a different project' do
+        let(:milestone) { create(:milestone, title: 'v1.0') }
+        let(:milestone_params) { { milestones: [milestone.title] } }
+
+        it 'returns a 400 error as milestone not found' do
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response['message']).to eq("Milestone(s) not found: v1.0")
+        end
+      end
+    end
   end
 
   describe 'PUT /projects/:id/releases/:tag_name' do
@@ -863,6 +922,83 @@ RSpec.describe API::Releases do
         end
       end
     end
+
+    context 'with milestones' do
+      let(:returned_milestones) { json_response['milestones'].map {|m| m['title']} }
+
+      subject { put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params }
+
+      context 'when a milestone is passed in' do
+        let(:milestone) { create(:milestone, project: project, title: 'v1.0') }
+        let(:milestone_title) { milestone.title }
+        let(:params) { { milestones: [milestone_title] } }
+
+        before do
+          release.milestones << milestone
+        end
+
+        context 'a different milestone' do
+          let(:milestone_title) { 'v2.0' }
+          let!(:milestone2) { create(:milestone, project: project, title: milestone_title) }
+
+          it 'replaces the milestone' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(returned_milestones).to match_array(['v2.0'])
+          end
+        end
+
+        context 'an identical milestone' do
+          let(:milestone_title) { 'v1.0' }
+
+          it 'does not change the milestone' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(returned_milestones).to match_array(['v1.0'])
+          end
+        end
+
+        context 'an empty milestone' do
+          let(:milestone_title) { nil }
+
+          it 'removes the milestone' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['milestones']).to be_nil
+          end
+        end
+
+        context 'multiple milestones' do
+          context 'with one new' do
+            let!(:milestone2) { create(:milestone, project: project, title: 'milestone2') }
+            let(:params) { { milestones: [milestone.title, milestone2.title] } }
+
+            it 'adds the new milestone' do
+              subject
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(returned_milestones).to match_array(['v1.0', 'milestone2'])
+            end
+          end
+
+          context 'with all new' do
+            let!(:milestone2) { create(:milestone, project: project, title: 'milestone2') }
+            let!(:milestone3) { create(:milestone, project: project, title: 'milestone3') }
+            let(:params) { { milestones: [milestone2.title, milestone3.title] } }
+
+            it 'replaces the milestones' do
+              subject
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(returned_milestones).to match_array(%w(milestone2 milestone3))
+            end
+          end
+        end
+      end
+    end
   end
 
   describe 'DELETE /projects/:id/releases/:tag_name' do