Commit 186ae1b8 authored by Etienne Baqué's avatar Etienne Baqué

Switched to many-to-many relationship

Updated models to set up the new relationship.
Updated spec files and fixtures as well.
parent 9440cc2f
...@@ -48,25 +48,29 @@ module Releases ...@@ -48,25 +48,29 @@ module Releases
end end
end end
def milestone def milestones
return unless params[:milestone] return [] unless params[:milestones]
strong_memoize(:milestone) do strong_memoize(:milestones) do
MilestonesFinder.new( MilestonesFinder.new(
project: project, project: project,
current_user: current_user, current_user: current_user,
project_ids: Array(project.id), project_ids: Array(project.id),
title: params[:milestone] state: 'all',
).execute.first title: params[:milestones]
).execute
end end
end end
def inexistent_milestone? def inexistent_milestones
params[:milestone] && !params[:milestone].empty? && !milestone return unless params[:milestones] && !params[:milestones].empty?
existing_milestone_titles = milestones.map(&:title)
(params[:milestones] - existing_milestone_titles).join(', ')
end end
def param_for_milestone_title_provided? def param_for_milestone_titles_provided?
params[:milestone].present? || params[:milestone]&.empty? params[:milestones].present? || params[:milestones]&.empty?
end end
end end
end end
......
...@@ -7,7 +7,7 @@ module Releases ...@@ -7,7 +7,7 @@ module Releases
def execute def execute
return error('Access Denied', 403) unless allowed? return error('Access Denied', 403) unless allowed?
return error('Release already exists', 409) if release return error('Release already exists', 409) if release
return error('Milestone does not exist', 400) if inexistent_milestone? return error("Inexistent milestone(s): #{inexistent_milestones}", 400) if inexistent_milestones.present?
tag = ensure_tag tag = ensure_tag
...@@ -49,6 +49,7 @@ module Releases ...@@ -49,6 +49,7 @@ module Releases
success(tag: tag, release: release) success(tag: tag, release: release)
rescue => e rescue => e
binding.pry
error(e.message, 400) error(e.message, 400)
end end
...@@ -61,7 +62,7 @@ module Releases ...@@ -61,7 +62,7 @@ module Releases
sha: tag.dereferenced_target.sha, sha: tag.dereferenced_target.sha,
released_at: released_at, released_at: released_at,
links_attributes: params.dig(:assets, 'links') || [], links_attributes: params.dig(:assets, 'links') || [],
milestone: milestone milestones: milestones
) )
end end
end end
......
...@@ -9,9 +9,9 @@ module Releases ...@@ -9,9 +9,9 @@ module Releases
return error('Release does not exist', 404) unless release return error('Release does not exist', 404) unless release
return error('Access Denied', 403) unless allowed? return error('Access Denied', 403) unless allowed?
return error('params is empty', 400) if empty_params? return error('params is empty', 400) if empty_params?
return error('Milestone does not exist', 400) if inexistent_milestone? return error("Inexistent milestone(s): #{inexistent_milestones}", 400) if inexistent_milestones.present?
params[:milestone] = milestone if param_for_milestone_title_provided? params[:milestones] = milestones if param_for_milestone_titles_provided?
if release.update(params) if release.update(params)
success(tag: existing_tag, release: release) success(tag: existing_tag, release: release)
......
...@@ -1280,7 +1280,7 @@ module API ...@@ -1280,7 +1280,7 @@ module API
expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? } expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? }
expose :commit, using: Entities::Commit, if: lambda { |_, _| can_download_code? } expose :commit, using: Entities::Commit, if: lambda { |_, _| can_download_code? }
expose :upcoming_release?, as: :upcoming_release expose :upcoming_release?, as: :upcoming_release
expose :milestone, using: Entities::Milestone, if: -> (release, _) { release.milestone.present? } expose :milestones, using: Entities::Milestone, if: -> (release, _) { release.milestones.present? }
expose :assets do expose :assets do
expose :assets_count, as: :count do |release, _| expose :assets_count, as: :count do |release, _|
......
...@@ -54,7 +54,7 @@ module API ...@@ -54,7 +54,7 @@ module API
requires :url, type: String requires :url, type: String
end end
end end
optional :milestone, type: String, desc: 'The title of the related milestone' optional :milestones, type: Array, desc: 'The titles of the related milestones'
optional :released_at, type: DateTime, desc: 'The date when the release will be/was ready. Defaults to the current time.' optional :released_at, type: DateTime, desc: 'The date when the release will be/was ready. Defaults to the current time.'
end end
post ':id/releases' do post ':id/releases' do
...@@ -80,7 +80,7 @@ module API ...@@ -80,7 +80,7 @@ module API
optional :name, type: String, desc: 'The name of the release' optional :name, type: String, desc: 'The name of the release'
optional :description, type: String, desc: 'Release notes with markdown support' optional :description, type: String, desc: 'Release notes with markdown support'
optional :released_at, type: DateTime, desc: 'The date when the release will be/was ready.' optional :released_at, type: DateTime, desc: 'The date when the release will be/was ready.'
optional :milestone, type: String, desc: 'The title of the related milestone' optional :milestones, type: Array, desc: 'The titles of the related milestones'
end end
put ':id/releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMENTS do put ':id/releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMENTS do
authorize_update_release! authorize_update_release!
......
...@@ -15,7 +15,10 @@ ...@@ -15,7 +15,10 @@
"author": { "author": {
"oneOf": [{ "type": "null" }, { "$ref": "user/basic.json" }] "oneOf": [{ "type": "null" }, { "$ref": "user/basic.json" }]
}, },
"milestone": { "type": "string" }, "milestones": {
"type": "array",
"items": { "$ref": "milestone.json" }
},
"assets": { "assets": {
"required": ["count", "links", "sources"], "required": ["count", "links", "sources"],
"properties": { "properties": {
......
...@@ -65,8 +65,8 @@ milestone: ...@@ -65,8 +65,8 @@ milestone:
- participants - participants
- events - events
- boards - boards
- milestone_release - milestone_releases
- release - releases
snippets: snippets:
- author - author
- project - project
...@@ -77,8 +77,8 @@ releases: ...@@ -77,8 +77,8 @@ releases:
- author - author
- project - project
- links - links
- milestone_release - milestone_releases
- milestone - milestones
links: links:
- release - release
project_members: project_members:
......
...@@ -75,10 +75,11 @@ describe Releases::CreateService do ...@@ -75,10 +75,11 @@ describe Releases::CreateService do
context 'when a passed-in milestone does not exist for this project' do context 'when a passed-in milestone does not exist for this project' do
it 'raises an error saying the milestone is inexistent' do it 'raises an error saying the milestone is inexistent' do
service = described_class.new(project, user, params.merge!({ milestone: 'v111.0' })) inexistent_milestone_tag = 'v111.0'
service = described_class.new(project, user, params.merge!({ milestones: [inexistent_milestone_tag] }))
result = service.execute result = service.execute
expect(result[:status]).to eq(:error) expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('Milestone does not exist') expect(result[:message]).to eq("Inexistent milestone(s): #{inexistent_milestone_tag}")
end end
end end
end end
...@@ -93,7 +94,7 @@ describe Releases::CreateService do ...@@ -93,7 +94,7 @@ describe Releases::CreateService do
context 'when existing milestone is passed in' do context 'when existing milestone is passed in' do
let(:title) { 'v1.0' } let(:title) { 'v1.0' }
let(:milestone) { create(:milestone, :active, project: project, title: title) } let(:milestone) { create(:milestone, :active, project: project, title: title) }
let(:params_with_milestone) { params.merge!({ milestone: title }) } let(:params_with_milestone) { params.merge!({ milestones: [title] }) }
it 'creates a release and ties this milestone to it' do it 'creates a release and ties this milestone to it' do
service = described_class.new(milestone.project, user, params_with_milestone) service = described_class.new(milestone.project, user, params_with_milestone)
...@@ -104,18 +105,18 @@ describe Releases::CreateService do ...@@ -104,18 +105,18 @@ describe Releases::CreateService do
release = project.releases.last release = project.releases.last
expect(release.milestone).to eq(milestone) expect(release.milestones.first).to eq(milestone)
end end
context 'when another release was previously created with that same milestone linked' do context 'when another release was previously created with that same milestone linked' do
it 'also creates another release tied to that same milestone' do it 'also creates another release tied to that same milestone' do
other_release = create(:release, milestone: milestone, project: project, tag: 'v1.0') other_release = create(:release, milestones: [milestone], project: project, tag: 'v1.0')
service = described_class.new(milestone.project, user, params_with_milestone) service = described_class.new(milestone.project, user, params_with_milestone)
service.execute service.execute
release = project.releases.last release = project.releases.last
expect(release.milestone).to eq(milestone) expect(release.milestones.first).to eq(milestone)
expect(other_release.milestone).to eq(milestone) expect(other_release.milestones.first).to eq(milestone)
expect(release.id).not_to eq(other_release.id) expect(release.id).not_to eq(other_release.id)
end end
end end
...@@ -123,10 +124,10 @@ describe Releases::CreateService do ...@@ -123,10 +124,10 @@ describe Releases::CreateService do
context 'when no milestone is passed in' do context 'when no milestone is passed in' do
it 'creates a release without a milestone tied to it' do it 'creates a release without a milestone tied to it' do
expect(params.key? :milestone).to be_falsey expect(params.key? :milestones).to be_falsey
service.execute service.execute
release = project.releases.last release = project.releases.last
expect(release.milestone).to be_nil expect(release.milestones).not_to be_present
end end
it 'does not create any new MilestoneRelease object' do it 'does not create any new MilestoneRelease object' do
...@@ -136,10 +137,10 @@ describe Releases::CreateService do ...@@ -136,10 +137,10 @@ describe Releases::CreateService do
context 'when an empty value is passed as a milestone' do context 'when an empty value is passed as a milestone' do
it 'creates a release without a milestone tied to it' do it 'creates a release without a milestone tied to it' do
service = described_class.new(project, user, params.merge!({ milestone: '' })) service = described_class.new(project, user, params.merge!({ milestones: [] }))
service.execute service.execute
release = project.releases.last release = project.releases.last
expect(release.milestone).to be_nil expect(release.milestones).not_to be_present
end end
end end
end end
......
...@@ -54,35 +54,33 @@ describe Releases::UpdateService do ...@@ -54,35 +54,33 @@ describe Releases::UpdateService do
let(:new_title) { 'v2.0' } let(:new_title) { 'v2.0' }
let(:milestone) { create(:milestone, project: project, title: old_title) } let(:milestone) { create(:milestone, project: project, title: old_title) }
let(:new_milestone) { create(:milestone, project: project, title: new_title) } let(:new_milestone) { create(:milestone, project: project, title: new_title) }
let(:params_with_milestone) { params.merge!({ milestone: new_title }) } let(:params_with_milestone) { params.merge!({ milestones: [new_title] }) }
before do before do
release.milestone = milestone release.milestones << milestone
release.save!
described_class.new(new_milestone.project, user, params_with_milestone).execute described_class.new(new_milestone.project, user, params_with_milestone).execute
release.reload release.reload
end end
it 'updates the related milestone accordingly' do it 'updates the related milestone accordingly' do
expect(release.milestone.title).to eq(new_title) expect(release.milestones.first.title).to eq(new_title)
end end
end end
context "when an 'empty' milestone is passed in" do context "when an 'empty' milestone is passed in" do
let(:milestone) { create(:milestone, project: project, title: 'v1.0') } let(:milestone) { create(:milestone, project: project, title: 'v1.0') }
let(:params_with_empty_milestone) { params.merge!({ milestone: '' }) } let(:params_with_empty_milestone) { params.merge!({ milestones: [] }) }
before do before do
release.milestone = milestone release.milestones << milestone
release.save!
described_class.new(milestone.project, user, params_with_empty_milestone).execute described_class.new(milestone.project, user, params_with_empty_milestone).execute
release.reload release.reload
end end
it 'removes the old milestone and does not associate any new milestone' do it 'removes the old milestone and does not associate any new milestone' do
expect(release.milestone).to be_nil expect(release.milestones).not_to be_present
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment