Fix releases will be deleted when description is nullac-releases-api-fix-release-will-be-deleted-when-description-is-null

Add comment

2 files changed, 26 insertions, 14 deletions

diff --git a/app/controllers/projects/tags/releases_controller.rb b/app/controllers/projects/tags/releases_controller.rb
index 334e1847cc8..bc96f59dcaa 100644
--- a/app/controllers/projects/tags/releases_controller.rb
+++ b/app/controllers/projects/tags/releases_controller.rb
@@ -4,7 +4,7 @@ class Projects::Tags::ReleasesController < Projects::ApplicationController
   # Authorize
   before_action :require_non_empty_project
   before_action :authorize_download_code!
-  before_action :authorize_push_code!
+  before_action :authorize_update_release!
   before_action :tag
   before_action :release
 
@@ -12,16 +12,15 @@ class Projects::Tags::ReleasesController < Projects::ApplicationController
   end
 
   def update
-    # Release belongs to Tag which is not active record object,
-    # it exists only to save a description to each Tag.
-    # If description is empty we should destroy the existing record.
-    if release_params[:description].present?
-      release.update(release_params)
+    ##
+    # Previously, we destroyed release object when description is empty,
+    # however, this is not the case anymore because releases persist variaous
+    # information, such as assets, today.
+    if release.update(release_params)
+      redirect_to project_tag_path(@project, @tag.name)
     else
-      release.destroy
+      render :edit
     end
-
-    redirect_to project_tag_path(@project, @tag.name)
   end
 
   private
@@ -30,13 +29,26 @@ class Projects::Tags::ReleasesController < Projects::ApplicationController
     @tag ||= @repository.find_tag(params[:tag_id])
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
+  def sha
+    @sha ||= tag.dereferenced_target.id
+  end
+
   def release
-    @release ||= @project.releases.find_or_initialize_by(tag: @tag.name)
+    @release ||= @project.releases.find_by_tag(tag.name) || build_release
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   def release_params
     params.require(:release).permit(:description)
   end
+
+  ##
+  # Legacy release creation form does not have `name` input.
+  # We should fill it with tag_name because `name` is requied parameter today.
+  def build_release
+    @project.releases.build(tag: tag.name, name: tag.name, sha: sha)
+  end
+
+  def authorize_update_release!
+    return access_denied! unless can?(current_user, :update_release, release)
+  end
 end
diff --git a/spec/controllers/projects/tags/releases_controller_spec.rb b/spec/controllers/projects/tags/releases_controller_spec.rb
index 29f206c574b..3a696b6ae4e 100644
--- a/spec/controllers/projects/tags/releases_controller_spec.rb
+++ b/spec/controllers/projects/tags/releases_controller_spec.rb
@@ -42,8 +42,8 @@ describe Projects::Tags::ReleasesController do
       expect(release.description).to eq("description updated")
     end
 
-    it 'deletes release note when description is null' do
-      expect { update_release('') }.to change(project.releases, :count).by(-1)
+    it 'does not delete release object when description is null' do
+      expect { update_release('') }.not_to change(project.releases, :count)
     end
   end