Support Group Milestones to be associated with Project Releases in API

Part of https://gitlab.com/gitlab-org/gitlab/-/issues/235391

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/43385

app/models/milestone_release.rb

@@ -11,6 +11,10 @@ class MilestoneRelease < ApplicationRecord
   def same_project_between_milestone_and_release
     return if milestone&.project_id == release&.project_id
 
+    return if milestone&.group_id
+
     errors.add(:base, _('Release does not have the same project as the milestone'))
   end
 end
+
+MilestoneRelease.prepend_if_ee('EE::MilestoneRelease')

app/services/releases/base_service.rb

@@ -63,6 +63,7 @@ module Releases
           project: project,
           current_user: current_user,
           project_ids: Array(project.id),
+          group_ids: Array(project_group_id),
           state: 'all',
           title: params[:milestones]
         ).execute
@@ -79,5 +80,10 @@ module Releases
     def param_for_milestone_titles_provided?
       params.key?(:milestones)
     end
+
+    # overridden in EE
+    def project_group_id; end
   end
 end
+
+Releases::BaseService.prepend_if_ee('EE::Releases::BaseService')

app/services/releases/create_evidence_service.rb

@@ -12,7 +12,7 @@ module Releases
 
       summary = ::Evidences::EvidenceSerializer.new.represent(evidence, evidence_options) # rubocop: disable CodeReuse/Serializer
       evidence.summary = summary
-      # TODO: fix the sha generating https://gitlab.com/gitlab-org/gitlab/-/issues/209000
+      # TODO: fix the sha generation https://gitlab.com/groups/gitlab-org/-/epics/3683
       evidence.summary_sha = Gitlab::CryptoHelper.sha256(summary)
 
       evidence.save!

app/services/releases/update_service.rb

@@ -14,10 +14,18 @@ module Releases
         params[:milestones] = milestones
       end
 
-      if release.update(params)
-        success(tag: existing_tag, release: release, milestones_updated: milestones_updated?(previous_milestones))
-      else
-        error(release.errors.messages || '400 Bad request', 400)
+      # transaction needed as Rails applies `save!` to milestone_releases
+      # when it does assign_attributes instead of actual saving
+      # this leads to the validation error being raised
+      # see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/43385
+      ActiveRecord::Base.transaction do
+        if release.update(params)
+          success(tag: existing_tag, release: release, milestones_updated: milestones_updated?(previous_milestones))
+        else
+          error(release.errors.messages || '400 Bad request', 400)
+        end
+      rescue ActiveRecord::RecordInvalid => e
+        error(e.message || '400 Bad request', 400)
       end
     end
 

ee/app/models/ee/milestone_release.rb

+# frozen_string_literal: true
+
+module EE
+  module MilestoneRelease
+    extend ActiveSupport::Concern
+    extend ::Gitlab::Utils::Override
+
+    prepended do
+      validate :same_project_between_project_milestone_and_group
+
+      def same_project_between_project_milestone_and_group
+        return unless release_id_changed? || milestone_id_changed?
+
+        return unless milestone&.group_id && release&.project_id
+
+        return if release.project.feature_available?(:group_milestone_project_releases) && milestone.group.projects.where(id: release.project_id).exists?
+
+        errors.add(:base, _('None of the group milestones have the same project as the release'))
+      end
+    end
+  end
+end

ee/app/models/license.rb

@@ -83,6 +83,7 @@ class License < ApplicationRecord
     group_forking_protection
     group_ip_restriction
     group_merge_request_analytics
+    group_milestone_project_releases
     group_project_templates
     group_repository_analytics
     group_saml

ee/app/services/ee/releases/base_service.rb

+# frozen_string_literal: true
+
+module EE
+  module Releases
+    module BaseService
+      extend ActiveSupport::Concern
+      extend ::Gitlab::Utils::Override
+
+      prepended do
+        def project_group_id
+          project.group&.id
+        end
+      end
+    end
+  end
+end

ee/changelogs/unreleased/235391-support-group-milestones-to-be-associated-with-project-releases.yml

+---
+title: Support Group Milestones to be associated with Project Releases in API
+merge_request: 43385
+author:
+type: changed

ee/spec/features/projects/releases/user_views_release_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User views Release', :js do
+  it 'renders the group milestone' do
+    stub_licensed_features(group_milestone_project_releases: true)
+
+    group = create(:group)
+    project = create(:project, :repository, group: group)
+    group_milestone = create(:milestone, group: group, title: 'group_milestone_1')
+    release = create(:release, project: project, milestones: [group_milestone])
+
+    user = create(:user, developer_projects: [project])
+    sign_in(user)
+    visit project_release_path(project, release)
+
+    expect(page).to have_content("group_milestone_1")
+  end
+end

ee/spec/models/milestone_release_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MilestoneRelease do
+  let(:project) { create(:project) }
+  let(:release) { create(:release, project: project) }
+
+  describe 'validations' do
+    let(:milestone_release) { build(:milestone_release, release: release, milestone: milestone) }
+
+    subject { milestone_release }
+
+    context 'when it is a project milestone' do
+      context 'when milestone and release have the same project' do
+        let(:milestone) { create(:milestone, project: project, group: nil) }
+
+        it { is_expected.to be_valid }
+      end
+
+      context 'when milestone and release do not have the same project' do
+        let(:milestone) { create(:milestone, project: create(:project), group: nil) }
+
+        it { is_expected.not_to be_valid }
+      end
+    end
+
+    context 'when it is a group milestone' do
+      let(:milestone) { create(:milestone, project: nil, group: group) }
+
+      context 'when group and release have the same project' do
+        let(:group) { create(:group) }
+        let(:project) { create(:project, group: group)}
+
+        context 'when it is licenced' do
+          before do
+            stub_licensed_features(group_milestone_project_releases: true)
+          end
+
+          it { is_expected.to be_valid }
+        end
+
+        context 'when it is not licensed' do
+          it { is_expected.not_to be_valid }
+        end
+      end
+
+      context 'when milestone and group do not have the same project' do
+        let(:group) { create(:group) }
+        let(:project2) { create(:project, group: group) }
+
+        context 'when it is licenced' do
+          before do
+            stub_licensed_features(group_milestone_project_releases: true)
+          end
+
+          it { is_expected.not_to be_valid }
+        end
+
+        it { is_expected.not_to be_valid }
+      end
+
+      context 'when it is a supergroup milestone' do
+        let(:supergroup) { create(:group) }
+        let(:group) { create(:group, parent: supergroup) }
+        let(:project) { create(:project, group: group) }
+        let(:milestone) { create(:milestone, project: nil, group: supergroup) }
+
+        context 'when it is licenced' do
+          before do
+            stub_licensed_features(group_milestone_project_releases: true)
+          end
+
+          it { is_expected.not_to be_valid }
+        end
+
+        it { is_expected.not_to be_valid }
+      end
+    end
+  end
+end

ee/spec/requests/api/releases_spec.rb

@@ -63,6 +63,70 @@ RSpec.describe API::Releases do
         end
       end
     end
+
+    context 'with a group milestone' do
+      let(:project) { create(:project, :repository, group: group) }
+      let(:group) { create(:group) }
+      let(:group_milestone) { create(:milestone, group: group, title: 'g1') }
+
+      before do
+        stub_licensed_features(group_milestone_project_releases: true)
+        params.merge!(milestone_params)
+      end
+
+      context 'succesfully adds a group milestone' do
+        let(:milestone_params) { { milestones: [group_milestone.title] } }
+
+        it 'adds the milestone', :aggregate_failures do
+          post api("/projects/#{project.id}/releases", maintainer), params: params
+
+          expect(response).to have_gitlab_http_status(:created)
+          expect(json_response['milestones'].map {|m| m['title']}).to match_array(['g1'])
+        end
+      end
+
+      context 'fails to add a group milestone if project does not belong to this group' do
+        let(:milestone_params) { { milestones: ['abc1'] } }
+
+        it 'returns a 400 error as milestone not found', :aggregate_failures do
+          post api("/projects/#{project.id}/releases", maintainer), params: params
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response['message']).to eq("Milestone(s) not found: abc1")
+        end
+      end
+
+      context 'when valid group and project milestones are passed' do
+        let(:project_milestone) { create(:milestone, project: project, title: 'v1.0') }
+        let(:milestone_params) { { milestones: [group_milestone.title, project_milestone.title] } }
+
+        it 'adds the milestone', :aggregate_failures do
+          post api("/projects/#{project.id}/releases", maintainer), params: params
+
+          expect(response).to have_gitlab_http_status(:created)
+          expect(json_response['milestones'].map {|m| m['title']}).to match_array(['g1', 'v1.0'])
+        end
+      end
+
+      context 'with a supergroup milestone' do
+        let(:group) { create(:group, parent: supergroup) }
+        let(:supergroup) { create(:group) }
+        let(:supergroup_milestone) { create(:milestone, group: supergroup, title: 'sg1') }
+        let(:milestone_params) { params.merge({ milestones: [supergroup_milestone.title] }) }
+
+        before do
+          stub_licensed_features(group_milestone_project_releases: true)
+          params.merge!(milestone_params)
+        end
+
+        it 'returns a 400 error as milestone not found', :aggregate_failures do
+          post api("/projects/#{project.id}/releases", maintainer), params: params
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response['message']).to eq("Milestone(s) not found: sg1")
+        end
+      end
+    end
   end
 
   describe 'PUT /projects/:id/releases/:tag_name' do
@@ -152,6 +216,42 @@ RSpec.describe API::Releases do
         end
       end
     end
+
+    context 'with group milestones' do
+      let(:project) { create(:project, :repository, group: group) }
+      let(:group) { create(:group) }
+
+      before do
+        stub_licensed_features(group_milestone_project_releases: true)
+
+        put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params
+      end
+
+      context 'when a group milestone is passed' do
+        let(:group_milestone) { create(:milestone, group: group, title: 'g1') }
+        let(:params) { { milestones: [group_milestone.title] } }
+
+        context 'when there is no project milestone' do
+          it 'adds the group milestone', :aggregate_failures do
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['milestones'].map {|m| m['title']}).to match_array([group_milestone.title])
+          end
+        end
+
+        context 'when there is an existing project milestone' do
+          let(:project_milestone) { create(:milestone, project: project, title: 'p1') }
+
+          before do
+            release.milestones << project_milestone
+          end
+
+          it 'replaces the project milestone with the group milestone', :aggregate_failures do
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['milestones'].map {|m| m['title']}).to match_array([group_milestone.title])
+          end
+        end
+      end
+    end
   end
 
   describe 'POST /projects/:id/releases/:tag_name/evidence' do

ee/spec/services/releases/create_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Releases::CreateService do
+  let(:group) { create :group }
+  let(:project) { create(:project, :repository, group: group) }
+  let(:user) { create(:user, maintainer_projects: [project]) }
+  let(:tag_name) { 'v1.1.0' }
+  let(:name) { 'Bionic Beaver' }
+  let(:description) { 'Awesome release!' }
+  let(:params) { { tag: tag_name, name: name, description: description } }
+  let(:release) { Release.last }
+  let(:service) { described_class.new(project, user, params_with_milestones) }
+
+  describe 'group milestones' do
+    context 'when a group milestone is passed' do
+      let(:group_milestone) { create(:milestone, group: group, title: 'g1') }
+      let(:params_with_milestones) { params.merge({ milestones: [group_milestone.title] }) }
+
+      context 'when licenced' do
+        before do
+          stub_licensed_features(group_milestone_project_releases: true)
+        end
+
+        it 'adds the group milestone', :aggregate_failures do
+          result = service.execute
+
+          expect(result[:status]).to eq(:success)
+          expect(release.milestones).to match_array([group_milestone])
+        end
+      end
+
+      context 'when unlicensed' do
+        it 'returns an error', :aggregate_failures do
+          result = service.execute
+
+          expect(result[:status]).to eq(:error)
+          expect(result[:message]).to match(/None of the group milestones have the same project as the release/)
+        end
+      end
+    end
+
+    context 'when a supergroup milestone is passed' do
+      let(:group) { create(:group, parent: supergroup) }
+      let(:supergroup) { create(:group) }
+      let(:supergroup_milestone) { create(:milestone, group: supergroup, title: 'sg1') }
+      let(:params_with_milestones) { params.merge({ milestones: [supergroup_milestone.title] }) }
+
+      it 'raises an error', :aggregate_failures do
+        result = service.execute
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:message]).to eq("Milestone(s) not found: sg1")
+        expect(release).to be_nil
+      end
+    end
+  end
+end

ee/spec/services/releases/update_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Releases::UpdateService do
+  let(:group) { create(:group) }
+  let(:project) { create(:project, :repository, group: group) }
+  let(:user) { create(:user) }
+  let(:params) { { tag: tag_name } }
+  let!(:release) { create(:release, project: project) }
+  let(:tag_name) { 'v1.1.0' }
+  let(:service) { described_class.new(project, user, params_with_milestones) }
+
+  before do
+    project.add_developer(user)
+  end
+
+  describe 'group milestones' do
+    context 'when a group milestone is passed' do
+      let(:group_milestone) { create(:milestone, group: group, title: 'g1') }
+      let(:params_with_milestones) { params.merge({ milestones: [group_milestone.title] }) }
+
+      context 'when there is no project milestone' do
+        context 'when licenced' do
+          before do
+            stub_licensed_features(group_milestone_project_releases: true)
+          end
+
+          it 'adds the group milestone', :aggregate_failures do
+            result = service.execute
+            release.reload
+
+            expect(release.milestones).to match_array([group_milestone])
+            expect(result[:milestones_updated]).to be_truthy
+          end
+        end
+
+        context 'when unlicensed' do
+          it 'returns an error', :aggregate_failures do
+            result = service.execute
+
+            expect(result[:status]).to eq(:error)
+            expect(result[:milestones_updated]).to be_falsy
+            expect(result[:message]).to match(/None of the group milestones have the same project as the release/)
+          end
+        end
+      end
+
+      context 'when there is an existing project milestone' do
+        let(:project_milestone) { create(:milestone, project: project, title: 'p1') }
+
+        before do
+          release.milestones << project_milestone
+        end
+
+        context 'when licenced' do
+          before do
+            stub_licensed_features(group_milestone_project_releases: true)
+          end
+
+          it 'replaces the project milestone with the group milestone', :aggregate_failures do
+            result = service.execute
+            release.reload
+
+            expect(release.milestones).to match_array([group_milestone])
+            expect(result[:milestones_updated]).to be_truthy
+          end
+        end
+
+        context 'when unlicensed' do
+          it 'returns an error', :aggregate_failures do
+            result = service.execute
+
+            expect(result[:status]).to eq(:error)
+            expect(result[:message]).to match(/None of the group milestones have the same project as the release/)
+          end
+        end
+      end
+
+      context 'when an empty milestone array is passed' do
+        let(:project_milestone) { create(:milestone, project: project, title: 'p1') }
+        let(:params_with_milestones) { params.merge({ milestones: [] }) }
+
+        before do
+          release.milestones << project_milestone
+        end
+
+        it 'clears the milestone array', :aggregate_failures do
+          result = service.execute
+          release.reload
+
+          expect(release.milestones).to match_array([])
+          expect(result[:milestones_updated]).to be_truthy
+        end
+      end
+
+      context 'when a supergroup milestone is passed' do
+        let(:group) { create(:group, parent: supergroup) }
+        let(:supergroup) { create(:group) }
+        let(:supergroup_milestone) { create(:milestone, group: supergroup, title: 'sg1') }
+        let(:params_with_milestones) { params.merge({ milestones: [supergroup_milestone.title] }) }
+
+        it 'ignores the milestone' do
+          service.execute
+          release.reload
+
+          expect(release.milestones).to match_array([])
+        end
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -18035,6 +18035,9 @@ msgstr ""
 msgid "None"
 msgstr ""
 
+msgid "None of the group milestones have the same project as the release"
+msgstr ""
+
 msgid "Not Implemented"
 msgstr ""
 

spec/models/milestone_release_spec.rb

@@ -10,8 +10,8 @@ RSpec.describe MilestoneRelease do
   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) }
+    it { is_expected.to belong_to(:milestone) }
   end
 
   context 'when trying to create the same record in milestone_releases twice' do