diff --git a/app/finders/README.md b/app/finders/README.md
index a5a6fd8fe74070f1767f08a9ebd6f79159fb83d7..52f7378c4844a5e6979f4b7932333aa68435de45 100644
--- a/app/finders/README.md
+++ b/app/finders/README.md
@@ -1,10 +1,11 @@
# 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
diff --git a/lib/api/releases.rb b/lib/api/releases.rb
index 3c38721129f125e3485c931bfd20c32d2a8d7e8e..d648046eb4eb1e0f410847ecec758f918420d9f5 100644
--- a/lib/api/releases.rb
+++ b/lib/api/releases.rb
@@ -152,7 +152,7 @@ def authorize_download_code!
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 @@ def release
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
diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb
index 01ead9eef54fcde8b52c29fec85e71e15cc1bd3d..8ae4b7e918544c4656e4b2cf4761300a01793306 100644
--- a/spec/requests/api/releases_spec.rb
+++ b/spec/requests/api/releases_spec.rb
@@ -598,7 +598,7 @@
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 @@
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 @@
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