Set release name when adding release notes to an existing tag

Also set the release sha and author

4 files changed, 80 insertions, 8 deletions

diff --git a/spec/controllers/projects/tags/releases_controller_spec.rb b/spec/controllers/projects/tags/releases_controller_spec.rb
index 29f206c574b..66eff4844c2 100644
--- a/spec/controllers/projects/tags/releases_controller_spec.rb
+++ b/spec/controllers/projects/tags/releases_controller_spec.rb
@@ -18,40 +18,85 @@ describe Projects::Tags::ReleasesController do
       tag_id = release.tag
       project.releases.destroy_all # rubocop: disable DestroyAll
 
-      get :edit, params: { namespace_id: project.namespace, project_id: project, tag_id: tag_id }
+      response = get :edit, params: { namespace_id: project.namespace, project_id: project, tag_id: tag_id }
 
       release = assigns(:release)
       expect(release).not_to be_nil
       expect(release).not_to be_persisted
+      expect(response).to have_http_status(:ok)
     end
 
     it 'retrieves an existing release' do
-      get :edit, params: { namespace_id: project.namespace, project_id: project, tag_id: release.tag }
+      response = get :edit, params: { namespace_id: project.namespace, project_id: project, tag_id: release.tag }
 
       release = assigns(:release)
       expect(release).not_to be_nil
       expect(release).to be_persisted
+      expect(response).to have_http_status(:ok)
     end
   end
 
   describe 'PUT #update' do
     it 'updates release note description' do
-      update_release('description updated')
+      response = update_release(release.tag, "description updated")
 
-      release = project.releases.find_by_tag(tag)
+      release = project.releases.find_by(tag: tag)
       expect(release.description).to eq("description updated")
+      expect(response).to have_http_status(:found)
     end
 
-    it 'deletes release note when description is null' do
-      expect { update_release('') }.to change(project.releases, :count).by(-1)
+    it 'creates a release if one does not exist' do
+      tag_without_release = create_new_tag
+
+      expect do
+        update_release(tag_without_release.name, "a new release")
+      end.to change { project.releases.count }.by(1)
+
+      expect(response).to have_http_status(:found)
+    end
+
+    it 'sets the release name, sha, and author for a new release' do
+      tag_without_release = create_new_tag
+
+      response = update_release(tag_without_release.name, "a new release")
+
+      release = project.releases.find_by(tag: tag_without_release.name)
+      expect(release.name).to eq(tag_without_release.name)
+      expect(release.sha).to eq(tag_without_release.target_commit.sha)
+      expect(release.author.id).to eq(user.id)
+      expect(response).to have_http_status(:found)
+    end
+
+    it 'deletes release when description is empty' do
+      initial_releases_count = project.releases.count
+
+      response = update_release(release.tag, "")
+
+      expect(initial_releases_count).to eq(1)
+      expect(project.releases.count).to eq(0)
+      expect(response).to have_http_status(:found)
+    end
+
+    it 'does nothing when description is empty and the tag does not have a release' do
+      tag_without_release = create_new_tag
+
+      expect do
+        update_release(tag_without_release.name, "")
+      end.not_to change { project.releases.count }
+
+      expect(response).to have_http_status(:found)
     end
   end
 
-  def update_release(description)
+  def create_new_tag
+    project.repository.add_tag(user, 'mytag', 'master')
+  end
+
+  def update_release(tag_id, description)
     put :update, params: {
       namespace_id: project.namespace.to_param,
       project_id: project,
-      tag_id: release.tag,
+      tag_id: tag_id,
       release: { description: description }
     }
   end
diff --git a/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb
index 082e3b36dd0..c57b96fb00d 100644
--- a/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb
+++ b/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb
@@ -25,6 +25,7 @@ describe Gitlab::LegacyGithubImport::ReleaseFormatter do
       expected = {
         project: project,
         tag: 'v1.0.0',
+        name: 'First release',
         description: 'Release v1.0.0',
         created_at: created_at,
         updated_at: created_at
diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb
index b4b32c95dee..0b19a4f8efc 100644
--- a/spec/models/release_spec.rb
+++ b/spec/models/release_spec.rb
@@ -18,6 +18,22 @@ RSpec.describe Release do
   describe 'validation' do
     it { is_expected.to validate_presence_of(:project) }
     it { is_expected.to validate_presence_of(:description) }
+    it { is_expected.to validate_presence_of(:name) }
+
+    context 'when a release exists in the database without a name' do
+      it 'does not require name' do
+        existing_release_without_name = build(:release, project: project, author: user, name: nil)
+        existing_release_without_name.save(validate: false)
+
+        existing_release_without_name.description = "change"
+        existing_release_without_name.save
+        existing_release_without_name.reload
+
+        expect(existing_release_without_name).to be_valid
+        expect(existing_release_without_name.description).to eq("change")
+        expect(existing_release_without_name.name).to be_nil
+      end
+    end
   end
 
   describe '#assets_count' do
diff --git a/spec/services/releases/create_service_spec.rb b/spec/services/releases/create_service_spec.rb
index 612e9f152e7..0efe37f1167 100644
--- a/spec/services/releases/create_service_spec.rb
+++ b/spec/services/releases/create_service_spec.rb
@@ -19,6 +19,8 @@ describe Releases::CreateService do
     shared_examples 'a successful release creation' do
       it 'creates a new release' do
         result = service.execute
+
+        expect(project.releases.count).to eq(1)
         expect(result[:status]).to eq(:success)
         expect(result[:tag]).not_to be_nil
         expect(result[:release]).not_to be_nil
@@ -69,4 +71,12 @@ describe Releases::CreateService do
       end
     end
   end
+
+  describe '#find_or_build_release' do
+    it 'does not save the built release' do
+      service.find_or_build_release
+
+      expect(project.releases.count).to eq(0)
+    end
+  end
 end