From a43ab8d6a430014e875deb3bff3fd8d8da256747 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Etienne=20Baqu=C3=A9?= Date: Tue, 3 Sep 2019 09:38:59 +0000 Subject: [PATCH] Added relationships between Release and Milestone Modified schema via migrations. Added one-to-one relationship between the two models. Added changelog file --- app/models/milestone.rb | 7 ++ app/models/milestone_release.rb | 17 ++++ app/models/release.rb | 7 ++ app/services/releases/concerns.rb | 21 +++++ app/services/releases/create_service.rb | 4 +- app/services/releases/update_service.rb | 3 + .../unreleased/62402-milestone-release-be.yml | 5 + ...2144316_create_milestone_releases_table.rb | 20 ++++ db/schema.rb | 9 ++ doc/api/releases/index.md | 94 +++++++++++++++---- lib/api/entities.rb | 1 + lib/api/releases.rb | 2 + spec/factories/milestone_releases.rb | 14 +++ .../api/schemas/public_api/v4/release.json | 1 + spec/lib/gitlab/import_export/all_models.yml | 7 ++ spec/models/milestone_release_spec.rb | 36 +++++++ spec/models/milestone_spec.rb | 20 ++++ spec/models/release_spec.rb | 15 +++ .../milestones/destroy_service_spec.rb | 14 +++ spec/services/releases/create_service_spec.rb | 62 ++++++++++++ .../services/releases/destroy_service_spec.rb | 10 ++ spec/services/releases/update_service_spec.rb | 37 ++++++++ 22 files changed, 385 insertions(+), 21 deletions(-) create mode 100644 app/models/milestone_release.rb create mode 100644 changelogs/unreleased/62402-milestone-release-be.yml create mode 100644 db/migrate/20190722144316_create_milestone_releases_table.rb create mode 100644 spec/factories/milestone_releases.rb create mode 100644 spec/models/milestone_release_spec.rb diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 2ad2838111e..101e963ea29 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -24,6 +24,12 @@ class Milestone < ApplicationRecord belongs_to :project belongs_to :group + # A one-to-one relationship is set up here as part of a MVC: https://gitlab.com/gitlab-org/gitlab-ce/issues/62402 + # However, on the long term, we will want a many-to-many relationship between Release and Milestone. + # The "has_one through" allows us today to set up this one-to-one relationship while setting up the architecture for the long-term (ie intermediate table). + has_one :milestone_release + has_one :release, through: :milestone_release + has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.milestones&.maximum(:iid) } has_internal_id :iid, scope: :group, init: ->(s) { s&.group&.milestones&.maximum(:iid) } @@ -59,6 +65,7 @@ class Milestone < ApplicationRecord validate :milestone_type_check validate :start_date_should_be_less_than_due_date, if: proc { |m| m.start_date.present? && m.due_date.present? } validate :dates_within_4_digits + validates_associated :milestone_release, message: -> (_, obj) { obj[:value].errors.full_messages.join(",") } strip_attributes :title diff --git a/app/models/milestone_release.rb b/app/models/milestone_release.rb new file mode 100644 index 00000000000..c8743a8cad8 --- /dev/null +++ b/app/models/milestone_release.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class MilestoneRelease < ApplicationRecord + belongs_to :milestone + belongs_to :release + + validates :milestone_id, uniqueness: { scope: [:release_id] } + validate :same_project_between_milestone_and_release + + private + + def same_project_between_milestone_and_release + return if milestone&.project_id == release&.project_id + + errors.add(:base, 'does not have the same project as the milestone') + end +end diff --git a/app/models/release.rb b/app/models/release.rb index 459a7c29ad0..b2e65974aa0 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -12,6 +12,12 @@ class Release < ApplicationRecord has_many :links, class_name: 'Releases::Link' + # A one-to-one relationship is set up here as part of a MVC: https://gitlab.com/gitlab-org/gitlab-ce/issues/62402 + # However, on the long term, we will want a many-to-many relationship between Release and Milestone. + # The "has_one through" allows us today to set up this one-to-one relationship while setting up the architecture for the long-term (ie intermediate table). + has_one :milestone_release + has_one :milestone, through: :milestone_release + default_value_for :released_at, allows_nil: false do Time.zone.now end @@ -20,6 +26,7 @@ class Release < ApplicationRecord validates :description, :project, :tag, presence: true validates :name, presence: true, on: :create + validates_associated :milestone_release, message: -> (_, obj) { obj[:value].errors.full_messages.join(",") } scope :sorted, -> { order(released_at: :desc) } diff --git a/app/services/releases/concerns.rb b/app/services/releases/concerns.rb index 618d96717b8..b5412e97284 100644 --- a/app/services/releases/concerns.rb +++ b/app/services/releases/concerns.rb @@ -47,6 +47,27 @@ module Releases project.repository end end + + def milestone + return unless params[:milestone] + + strong_memoize(:milestone) do + MilestonesFinder.new( + project: project, + current_user: current_user, + project_ids: Array(project.id), + title: params[:milestone] + ).execute.first + end + end + + def inexistent_milestone? + params[:milestone] && !params[:milestone].empty? && !milestone + end + + def param_for_milestone_title_provided? + params[:milestone].present? || params[:milestone]&.empty? + end end end end diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb index 5b13ac631ba..c91d43084d3 100644 --- a/app/services/releases/create_service.rb +++ b/app/services/releases/create_service.rb @@ -7,6 +7,7 @@ module Releases def execute return error('Access Denied', 403) unless allowed? return error('Release already exists', 409) if release + return error('Milestone does not exist', 400) if inexistent_milestone? tag = ensure_tag @@ -59,7 +60,8 @@ module Releases tag: tag.name, sha: tag.dereferenced_target.sha, released_at: released_at, - links_attributes: params.dig(:assets, 'links') || [] + links_attributes: params.dig(:assets, 'links') || [], + milestone: milestone ) end end diff --git a/app/services/releases/update_service.rb b/app/services/releases/update_service.rb index fabfa398c59..70acc68f747 100644 --- a/app/services/releases/update_service.rb +++ b/app/services/releases/update_service.rb @@ -9,6 +9,9 @@ module Releases return error('Release does not exist', 404) unless release return error('Access Denied', 403) unless allowed? return error('params is empty', 400) if empty_params? + return error('Milestone does not exist', 400) if inexistent_milestone? + + params[:milestone] = milestone if param_for_milestone_title_provided? if release.update(params) success(tag: existing_tag, release: release) diff --git a/changelogs/unreleased/62402-milestone-release-be.yml b/changelogs/unreleased/62402-milestone-release-be.yml new file mode 100644 index 00000000000..3b1f6edfe6b --- /dev/null +++ b/changelogs/unreleased/62402-milestone-release-be.yml @@ -0,0 +1,5 @@ +--- +title: Allow milestones to be associated with a release (backend) +merge_request: 30816 +author: +type: added diff --git a/db/migrate/20190722144316_create_milestone_releases_table.rb b/db/migrate/20190722144316_create_milestone_releases_table.rb new file mode 100644 index 00000000000..55878bcec41 --- /dev/null +++ b/db/migrate/20190722144316_create_milestone_releases_table.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class CreateMilestoneReleasesTable < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + create_table :milestone_releases do |t| + t.references :milestone, foreign_key: { on_delete: :cascade }, null: false, index: false + t.references :release, foreign_key: { on_delete: :cascade }, null: false + end + + add_index :milestone_releases, [:milestone_id, :release_id], unique: true, name: 'index_miletone_releases_on_milestone_and_release' + end + + def down + drop_table :milestone_releases + end +end diff --git a/db/schema.rb b/db/schema.rb index 3980627045e..b24558f459e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2158,6 +2158,13 @@ ActiveRecord::Schema.define(version: 2019_09_02_131045) do t.index ["user_id"], name: "index_merge_trains_on_user_id" end + create_table "milestone_releases", force: :cascade do |t| + t.bigint "milestone_id", null: false + t.bigint "release_id", null: false + t.index ["milestone_id", "release_id"], name: "index_miletone_releases_on_milestone_and_release", unique: true + t.index ["release_id"], name: "index_milestone_releases_on_release_id" + end + create_table "milestones", id: :serial, force: :cascade do |t| t.string "title", null: false t.integer "project_id" @@ -3932,6 +3939,8 @@ ActiveRecord::Schema.define(version: 2019_09_02_131045) do add_foreign_key "merge_trains", "merge_requests", on_delete: :cascade add_foreign_key "merge_trains", "projects", column: "target_project_id", on_delete: :cascade add_foreign_key "merge_trains", "users", on_delete: :cascade + add_foreign_key "milestone_releases", "milestones", on_delete: :cascade + add_foreign_key "milestone_releases", "releases", on_delete: :cascade add_foreign_key "milestones", "namespaces", column: "group_id", name: "fk_95650a40d4", on_delete: :cascade add_foreign_key "milestones", "projects", name: "fk_9bd0a0c791", on_delete: :cascade add_foreign_key "namespace_aggregation_schedules", "namespaces", on_delete: :cascade diff --git a/doc/api/releases/index.md b/doc/api/releases/index.md index 850cf57a06f..8d5b3a65789 100644 --- a/doc/api/releases/index.md +++ b/doc/api/releases/index.md @@ -57,6 +57,19 @@ Example response: "committer_email":"admin@example.com", "committed_date":"2019-01-03T01:55:38.000Z" }, + "milestone":{ + "id":51, + "iid":1, + "project_id":24, + "title":"v1.0-rc", + "description":"Voluptate fugiat possimus quis quod aliquam expedita.", + "state":"closed", + "created_at":"2019-07-12T19:45:44.256Z", + "updated_at":"2019-07-12T19:45:44.256Z", + "due_date":"2019-08-16T11:00:00.256Z", + "start_date":"2019-07-30T12:00:00.256Z", + "web_url":"http://localhost:3000/root/awesome-app/-/milestones/1" + }, "assets":{ "count":6, "sources":[ @@ -205,6 +218,19 @@ Example response: "committer_email":"admin@example.com", "committed_date":"2019-01-03T01:53:28.000Z" }, + "milestone":{ + "id":51, + "iid":1, + "project_id":24, + "title":"v1.0-rc", + "description":"Voluptate fugiat possimus quis quod aliquam expedita.", + "state":"closed", + "created_at":"2019-07-12T19:45:44.256Z", + "updated_at":"2019-07-12T19:45:44.256Z", + "due_date":"2019-08-16T11:00:00.256Z", + "start_date":"2019-07-30T12:00:00.256Z", + "web_url":"http://localhost:3000/root/awesome-app/-/milestones/1" + }, "assets":{ "count":4, "sources":[ @@ -240,23 +266,24 @@ Create a Release. You need push access to the repository to create a Release. POST /projects/:id/releases ``` -| Attribute | Type | Required | Description | -| -------------------| -------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). | -| `name` | string | yes | The release name. | -| `tag_name` | string | yes | The tag where the release will be created from. | -| `description` | string | yes | The description of the release. You can use [markdown](../../user/markdown.md). | -| `ref` | string | no | If `tag_name` doesn't exist, the release will be created from `ref`. It can be a commit SHA, another tag name, or a branch name. | -| `assets:links` | array of hash | no | An array of assets links. | -| `assets:links:name`| string | required by: `assets:links` | The name of the link. | -| `assets:links:url` | string | required by: `assets:links` | The url of the link. | -| `released_at` | datetime | no | The date when the release will be/was ready. Defaults to the current time. Expected in ISO 8601 format (`2019-03-15T08:00:00Z`). | +| Attribute | Type | Required | Description | +| -------------------| --------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). | +| `name` | string | yes | The release name. | +| `tag_name` | string | yes | The tag where the release will be created from. | +| `description` | string | yes | The description of the release. You can use [markdown](../../user/markdown.md). | +| `ref` | string | no | If `tag_name` doesn't exist, the release will be created from `ref`. It can be a commit SHA, another tag name, or a branch name. | +| `milestone` | string | no | The title of the milestone the release is associated with. | +| `assets:links` | array of hash | no | An array of assets links. | +| `assets:links:name`| string | required by: `assets:links` | The name of the link. | +| `assets:links:url` | string | required by: `assets:links` | The url of the link. | +| `released_at` | datetime | no | The date when the release will be/was ready. Defaults to the current time. Expected in ISO 8601 format (`2019-03-15T08:00:00Z`). | Example request: ```sh curl --header 'Content-Type: application/json' --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" \ - --data '{ "name": "New release", "tag_name": "v0.3", "description": "Super nice release", "assets": { "links": [{ "name": "hoge", "url": "https://google.com" }] } }' \ + --data '{ "name": "New release", "tag_name": "v0.3", "description": "Super nice release", "milestone": "v1.0-rc", "assets": { "links": [{ "name": "hoge", "url": "https://google.com" }] } }' \ --request POST https://gitlab.example.com/api/v4/projects/24/releases ``` @@ -294,6 +321,19 @@ Example response: "committer_email":"admin@example.com", "committed_date":"2019-01-03T01:55:38.000Z" }, + "milestone":{ + "id":51, + "iid":1, + "project_id":24, + "title":"v1.0-rc", + "description":"Voluptate fugiat possimus quis quod aliquam expedita.", + "state":"active", + "created_at":"2019-07-12T19:45:44.256Z", + "updated_at":"2019-07-12T19:45:44.256Z", + "due_date":"2019-08-16T11:00:00.256Z", + "start_date":"2019-07-30T12:00:00.256Z", + "web_url":"http://localhost:3000/root/awesome-app/-/milestones/1" + }, "assets":{ "count":5, "sources":[ @@ -334,18 +374,19 @@ Update a Release. PUT /projects/:id/releases/:tag_name ``` -| Attribute | Type | Required | Description | -| ------------- | -------------- | -------- | -------------------------------------------------------------------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). | -| `tag_name` | string | yes | The tag where the release will be created from. | -| `name` | string | no | The release name. | -| `description` | string | no | The description of the release. You can use [markdown](../../user/markdown.md). | -| `released_at` | datetime | no | The date when the release will be/was ready. Expected in ISO 8601 format (`2019-03-15T08:00:00Z`). | +| Attribute | Type | Required | Description | +| ------------- | -------------- | -------- | --------------------------------------------------------------------------------------------------------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](../README.md#namespaced-path-encoding). | +| `tag_name` | string | yes | The tag where the release will be created from. | +| `name` | string | no | The release name. | +| `description` | string | no | The description of the release. You can use [markdown](../../user/markdown.md). | +| `milestone` | string | no | The title of the milestone to associate with the release (`""` to remove the milestone from the release). | +| `released_at` | datetime | no | The date when the release will be/was ready. Expected in ISO 8601 format (`2019-03-15T08:00:00Z`). | Example request: ```sh -curl --request PUT --data name="new name" --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "https://gitlab.example.com/api/v4/projects/24/releases/v0.1" +curl --header 'Content-Type: application/json' --request PUT --data '{"name": "new name", "milestone": "v1.0"}' --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "https://gitlab.example.com/api/v4/projects/24/releases/v0.1" ``` Example response: @@ -382,6 +423,19 @@ Example response: "committer_email":"admin@example.com", "committed_date":"2019-01-03T01:53:28.000Z" }, + "milestone":{ + "id":53, + "iid":2, + "project_id":24, + "title":"v1.0", + "description":"Voluptate fugiat possimus quis quod aliquam expedita.", + "state":"active", + "created_at":"2019-09-01T13:00:00.256Z", + "updated_at":"2019-09-01T13:00:00.256Z", + "due_date":"2019-09-20T13:00:00.256Z", + "start_date":"2019-09-05T12:00:00.256Z", + "web_url":"http://localhost:3000/root/awesome-app/-/milestones/3" + }, "assets":{ "count":4, "sources":[ diff --git a/lib/api/entities.rb b/lib/api/entities.rb index cfcf6228225..ba58e125568 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1229,6 +1229,7 @@ module API expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? } expose :commit, using: Entities::Commit, if: lambda { |_, _| can_download_code? } expose :upcoming_release?, as: :upcoming_release + expose :milestone, using: Entities::Milestone, if: -> (release, _) { release.milestone.present? } expose :assets do expose :assets_count, as: :count do |release, _| diff --git a/lib/api/releases.rb b/lib/api/releases.rb index 7a3d804c30c..11a9a085068 100644 --- a/lib/api/releases.rb +++ b/lib/api/releases.rb @@ -54,6 +54,7 @@ module API requires :url, type: String end end + optional :milestone, type: String, desc: 'The title of the related milestone' optional :released_at, type: DateTime, desc: 'The date when the release will be/was ready. Defaults to the current time.' end post ':id/releases' do @@ -79,6 +80,7 @@ module API optional :name, type: String, desc: 'The name of the release' 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 :milestone, type: String, desc: 'The title of the related milestone' end put ':id/releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMETS do authorize_update_release! diff --git a/spec/factories/milestone_releases.rb b/spec/factories/milestone_releases.rb new file mode 100644 index 00000000000..08e109480ab --- /dev/null +++ b/spec/factories/milestone_releases.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :milestone_release do + milestone + release + + before(:create, :build) do |mr| + project = create(:project) + mr.milestone.project = project + mr.release.project = project + end + end +end diff --git a/spec/fixtures/api/schemas/public_api/v4/release.json b/spec/fixtures/api/schemas/public_api/v4/release.json index ec3fa59cdb1..078b1c0b982 100644 --- a/spec/fixtures/api/schemas/public_api/v4/release.json +++ b/spec/fixtures/api/schemas/public_api/v4/release.json @@ -15,6 +15,7 @@ "author": { "oneOf": [{ "type": "null" }, { "$ref": "user/basic.json" }] }, + "milestone": { "type": "string" }, "assets": { "required": ["count", "links", "sources"], "properties": { diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index ec4a6ef05b9..c6aa4a2482c 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -62,6 +62,8 @@ milestone: - participants - events - boards +- milestone_release +- release snippets: - author - project @@ -72,6 +74,8 @@ releases: - author - project - links +- milestone_release +- milestone links: - release project_members: @@ -484,3 +488,6 @@ lists: - board - label - list_user_preferences +milestone_releases: +- milestone +- release diff --git a/spec/models/milestone_release_spec.rb b/spec/models/milestone_release_spec.rb new file mode 100644 index 00000000000..d6f73275977 --- /dev/null +++ b/spec/models/milestone_release_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MilestoneRelease do + let(:project) { create(:project) } + let(:release) { create(:release, project: project) } + let(:milestone) { create(:milestone, project: project) } + + subject { build(:milestone_release, release: release, milestone: milestone) } + + describe 'associations' do + it { is_expected.to belong_to(:milestone) } + it { is_expected.to belong_to(:release) } + end + + describe 'validations' do + it { is_expected.to validate_uniqueness_of(:milestone_id).scoped_to(:release_id) } + + context 'when milestone and release do not have the same project' do + it 'is not valid' do + other_project = create(:project) + release = build(:release, project: other_project) + milestone_release = described_class.new(milestone: milestone, release: release) + expect(milestone_release).not_to be_valid + end + end + + context 'when milestone and release have the same project' do + it 'is valid' do + milestone_release = described_class.new(milestone: milestone, release: release) + expect(milestone_release).to be_valid + end + end + end +end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 3704a2d468d..64030f5b92a 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -54,11 +54,31 @@ describe Milestone do expect(milestone.errors[:due_date]).to include("date must not be after 9999-12-31") end end + + describe 'milestone_release' do + let(:milestone) { build(:milestone, project: project) } + + context 'when it is tied to a release for another project' do + it 'creates a validation error' do + other_project = create(:project) + milestone.release = build(:release, project: other_project) + expect(milestone).not_to be_valid + end + end + + context 'when it is tied to a release for the same project' do + it 'is valid' do + milestone.release = build(:release, project: project) + expect(milestone).to be_valid + end + end + end end describe "Associations" do it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:issues) } + it { is_expected.to have_one(:release) } end let(:project) { create(:project, :public) } diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index e9d846e7291..29ce9f762b0 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -13,6 +13,7 @@ RSpec.describe Release do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to have_many(:links).class_name('Releases::Link') } + it { is_expected.to have_one(:milestone) } end describe 'validation' do @@ -34,6 +35,20 @@ RSpec.describe Release do expect(existing_release_without_name.name).to be_nil end end + + context 'when a release is tied to a milestone for another project' do + it 'creates a validation error' do + release.milestone = build(:milestone, project: create(:project)) + expect(release).not_to be_valid + end + end + + context 'when a release is tied to a milestone linked to the same project' do + it 'is valid' do + release.milestone = build(:milestone, project: project) + expect(release).to be_valid + end + end end describe '#assets_count' do diff --git a/spec/services/milestones/destroy_service_spec.rb b/spec/services/milestones/destroy_service_spec.rb index 3a22e4d4f92..ff1e1256166 100644 --- a/spec/services/milestones/destroy_service_spec.rb +++ b/spec/services/milestones/destroy_service_spec.rb @@ -65,5 +65,19 @@ describe Milestones::DestroyService do expect { service.execute(group_milestone) }.not_to change { Event.count } end end + + context 'when a release is tied to a milestone' do + it 'destroys the milestone but not the associated release' do + release = create( + :release, + tag: 'v1.0', + project: project, + milestone: milestone + ) + + expect { service.execute(milestone) }.not_to change { Release.count } + expect(release.reload).to be_persisted + end + end end end diff --git a/spec/services/releases/create_service_spec.rb b/spec/services/releases/create_service_spec.rb index e26676cdd55..5c9d6537df1 100644 --- a/spec/services/releases/create_service_spec.rb +++ b/spec/services/releases/create_service_spec.rb @@ -72,6 +72,15 @@ describe Releases::CreateService do expect(project.releases.find_by(tag: tag_name).description).to eq(description) end end + + context 'when a passed-in milestone does not exist for this project' do + it 'raises an error saying the milestone is inexistent' do + service = described_class.new(project, user, params.merge!({ milestone: 'v111.0' })) + result = service.execute + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq('Milestone does not exist') + end + end end describe '#find_or_build_release' do @@ -80,5 +89,58 @@ describe Releases::CreateService do expect(project.releases.count).to eq(0) end + + context 'when existing milestone is passed in' do + let(:title) { 'v1.0' } + let(:milestone) { create(:milestone, :active, project: project, title: title) } + let(:params_with_milestone) { params.merge!({ milestone: title }) } + + it 'creates a release and ties this milestone to it' do + service = described_class.new(milestone.project, user, params_with_milestone) + result = service.execute + + expect(project.releases.count).to eq(1) + expect(result[:status]).to eq(:success) + + release = project.releases.last + + expect(release.milestone).to eq(milestone) + end + + context 'when another release was previously created with that same milestone linked' do + it 'also creates another release tied to that same milestone' do + other_release = create(:release, milestone: milestone, project: project, tag: 'v1.0') + service = described_class.new(milestone.project, user, params_with_milestone) + service.execute + release = project.releases.last + + expect(release.milestone).to eq(milestone) + expect(other_release.milestone).to eq(milestone) + expect(release.id).not_to eq(other_release.id) + end + end + end + + context 'when no milestone is passed in' do + it 'creates a release without a milestone tied to it' do + expect(params.key? :milestone).to be_falsey + service.execute + release = project.releases.last + expect(release.milestone).to be_nil + end + + it 'does not create any new MilestoneRelease object' do + expect { service.execute }.not_to change { MilestoneRelease.count } + end + end + + context 'when an empty value is passed as a milestone' do + it 'creates a release without a milestone tied to it' do + service = described_class.new(project, user, params.merge!({ milestone: '' })) + service.execute + release = project.releases.last + expect(release.milestone).to be_nil + end + end end end diff --git a/spec/services/releases/destroy_service_spec.rb b/spec/services/releases/destroy_service_spec.rb index f4c901e6585..c3172e5edbc 100644 --- a/spec/services/releases/destroy_service_spec.rb +++ b/spec/services/releases/destroy_service_spec.rb @@ -57,5 +57,15 @@ describe Releases::DestroyService do http_status: 403) end end + + context 'when a milestone is tied to the release' do + let!(:milestone) { create(:milestone, :active, project: project, title: 'v1.0') } + let!(:release) { create(:release, milestone: milestone, project: project, tag: tag) } + + it 'destroys the release but leave the milestone intact' do + expect { subject }.not_to change { Milestone.count } + expect(milestone.reload).to be_persisted + end + end end end diff --git a/spec/services/releases/update_service_spec.rb b/spec/services/releases/update_service_spec.rb index 14e6a5f13c8..944f3d8c9ad 100644 --- a/spec/services/releases/update_service_spec.rb +++ b/spec/services/releases/update_service_spec.rb @@ -48,5 +48,42 @@ describe Releases::UpdateService do it_behaves_like 'a failed update' end + + context 'when a milestone is passed in' do + let(:old_title) { 'v1.0' } + let(:new_title) { 'v2.0' } + let(:milestone) { create(:milestone, project: project, title: old_title) } + let(:new_milestone) { create(:milestone, project: project, title: new_title) } + let(:params_with_milestone) { params.merge!({ milestone: new_title }) } + + before do + release.milestone = milestone + release.save! + + described_class.new(new_milestone.project, user, params_with_milestone).execute + release.reload + end + + it 'updates the related milestone accordingly' do + expect(release.milestone.title).to eq(new_title) + end + end + + context "when an 'empty' milestone is passed in" do + let(:milestone) { create(:milestone, project: project, title: 'v1.0') } + let(:params_with_empty_milestone) { params.merge!({ milestone: '' }) } + + before do + release.milestone = milestone + release.save! + + described_class.new(milestone.project, user, params_with_empty_milestone).execute + release.reload + end + + it 'removes the old milestone and does not associate any new milestone' do + expect(release.milestone).to be_nil + end + end end end -- GitLab