6 files changed, 105 insertions, 91 deletions

diff --git a/app/services/releases/base_service.rb b/app/services/releases/base_service.rb
new file mode 100644
index 00000000000..15d040287a3
--- /dev/null
+++ b/app/services/releases/base_service.rb
@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+module Releases
+  class BaseService
+    include BaseServiceUtility
+    include Gitlab::Utils::StrongMemoize
+
+    attr_accessor :project, :current_user, :params
+
+    def initialize(project, user = nil, params = {})
+      @project, @current_user, @params = project, user, params.dup
+    end
+
+    delegate :repository, to: :project
+
+    def tag_name
+      params[:tag]
+    end
+
+    def ref
+      params[:ref]
+    end
+
+    def name
+      params[:name] || tag_name
+    end
+
+    def description
+      params[:description]
+    end
+
+    def released_at
+      params[:released_at]
+    end
+
+    def release
+      strong_memoize(:release) do
+        project.releases.find_by_tag(tag_name)
+      end
+    end
+
+    def existing_tag
+      strong_memoize(:existing_tag) do
+        repository.find_tag(tag_name)
+      end
+    end
+
+    def tag_exist?
+      existing_tag.present?
+    end
+
+    def repository
+      strong_memoize(:repository) do
+        project.repository
+      end
+    end
+
+    def milestones
+      return [] unless param_for_milestone_titles_provided?
+
+      strong_memoize(:milestones) do
+        MilestonesFinder.new(
+          project: project,
+          current_user: current_user,
+          project_ids: Array(project.id),
+          group_ids: Array(project_group_id),
+          state: 'all',
+          title: params[:milestones]
+        ).execute
+      end
+    end
+
+    def inexistent_milestones
+      return [] unless param_for_milestone_titles_provided?
+
+      existing_milestone_titles = milestones.map(&:title)
+      Array(params[:milestones]) - existing_milestone_titles
+    end
+
+    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')
diff --git a/app/services/releases/concerns.rb b/app/services/releases/concerns.rb
deleted file mode 100644
index a0ebaea77c8..00000000000
--- a/app/services/releases/concerns.rb
+++ /dev/null
@@ -1,77 +0,0 @@
-# frozen_string_literal: true
-
-module Releases
-  module Concerns
-    extend ActiveSupport::Concern
-    include Gitlab::Utils::StrongMemoize
-
-    included do
-      def tag_name
-        params[:tag]
-      end
-
-      def ref
-        params[:ref]
-      end
-
-      def name
-        params[:name] || tag_name
-      end
-
-      def description
-        params[:description]
-      end
-
-      def released_at
-        params[:released_at]
-      end
-
-      def release
-        strong_memoize(:release) do
-          project.releases.find_by_tag(tag_name)
-        end
-      end
-
-      def existing_tag
-        strong_memoize(:existing_tag) do
-          repository.find_tag(tag_name)
-        end
-      end
-
-      def tag_exist?
-        existing_tag.present?
-      end
-
-      def repository
-        strong_memoize(:repository) do
-          project.repository
-        end
-      end
-
-      def milestones
-        return [] unless param_for_milestone_titles_provided?
-
-        strong_memoize(:milestones) do
-          MilestonesFinder.new(
-            project: project,
-            current_user: current_user,
-            project_ids: Array(project.id),
-            state: 'all',
-            title: params[:milestones]
-          ).execute
-        end
-      end
-
-      def inexistent_milestones
-        return [] unless param_for_milestone_titles_provided?
-
-        existing_milestone_titles = milestones.map(&:title)
-        Array(params[:milestones]) - existing_milestone_titles
-      end
-
-      def param_for_milestone_titles_provided?
-        params.key?(:milestones)
-      end
-    end
-  end
-end
diff --git a/app/services/releases/create_evidence_service.rb b/app/services/releases/create_evidence_service.rb
index 9c370722d2c..78b6d77c2cb 100644
--- a/app/services/releases/create_evidence_service.rb
+++ b/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!
diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb
index 92a0b875dd4..887c2d355ee 100644
--- a/app/services/releases/create_service.rb
+++ b/app/services/releases/create_service.rb
@@ -1,9 +1,7 @@
 # frozen_string_literal: true
 
 module Releases
-  class CreateService < BaseService
-    include Releases::Concerns
-
+  class CreateService < Releases::BaseService
     def execute
       return error('Access Denied', 403) unless allowed?
       return error('Release already exists', 409) if release
diff --git a/app/services/releases/destroy_service.rb b/app/services/releases/destroy_service.rb
index f9f6101abdd..8abf9308689 100644
--- a/app/services/releases/destroy_service.rb
+++ b/app/services/releases/destroy_service.rb
@@ -1,9 +1,7 @@
 # frozen_string_literal: true
 
 module Releases
-  class DestroyService < BaseService
-    include Releases::Concerns
-
+  class DestroyService < Releases::BaseService
     def execute
       return error('Release does not exist', 404) unless release
       return error('Access Denied', 403) unless allowed?
diff --git a/app/services/releases/update_service.rb b/app/services/releases/update_service.rb
index a452f7aa17a..4786d35f31e 100644
--- a/app/services/releases/update_service.rb
+++ b/app/services/releases/update_service.rb
@@ -1,9 +1,7 @@
 # frozen_string_literal: true
 
 module Releases
-  class UpdateService < BaseService
-    include Releases::Concerns
-
+  class UpdateService < Releases::BaseService
     def execute
       return error('Tag does not exist', 404) unless existing_tag
       return error('Release does not exist', 404) unless release
@@ -16,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