Commit 498fa739 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'issue-58418-release-notes' into 'master'

Set Release Name When Adding Release Notes to an Existing Tag

Closes #58418

See merge request gitlab-org/gitlab-ce!26807
parents 74ac04a6 5b700328
...@@ -12,16 +12,13 @@ class Projects::Tags::ReleasesController < Projects::ApplicationController ...@@ -12,16 +12,13 @@ class Projects::Tags::ReleasesController < Projects::ApplicationController
end end
def update 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? if release_params[:description].present?
release.update(release_params) release.update(release_params)
else else
release.destroy release.destroy
end end
redirect_to project_tag_path(@project, @tag.name) redirect_to project_tag_path(@project, tag.name)
end end
private private
...@@ -30,11 +27,10 @@ class Projects::Tags::ReleasesController < Projects::ApplicationController ...@@ -30,11 +27,10 @@ class Projects::Tags::ReleasesController < Projects::ApplicationController
@tag ||= @repository.find_tag(params[:tag_id]) @tag ||= @repository.find_tag(params[:tag_id])
end end
# rubocop: disable CodeReuse/ActiveRecord
def release def release
@release ||= @project.releases.find_or_initialize_by(tag: @tag.name) @release ||= Releases::CreateService.new(project, current_user, tag: @tag.name)
.find_or_build_release
end end
# rubocop: enable CodeReuse/ActiveRecord
def release_params def release_params
params.require(:release).permit(:description) params.require(:release).permit(:description)
......
...@@ -15,6 +15,7 @@ class Release < ApplicationRecord ...@@ -15,6 +15,7 @@ class Release < ApplicationRecord
accepts_nested_attributes_for :links, allow_destroy: true accepts_nested_attributes_for :links, allow_destroy: true
validates :description, :project, :tag, presence: true validates :description, :project, :tag, presence: true
validates :name, presence: true, on: :create
scope :sorted, -> { order(created_at: :desc) } scope :sorted, -> { order(created_at: :desc) }
......
...@@ -15,7 +15,7 @@ module Releases ...@@ -15,7 +15,7 @@ module Releases
end end
def name def name
params[:name] params[:name] || tag_name
end end
def description def description
......
...@@ -15,6 +15,10 @@ module Releases ...@@ -15,6 +15,10 @@ module Releases
create_release(tag) create_release(tag)
end end
def find_or_build_release
release || build_release(existing_tag)
end
private private
def ensure_tag def ensure_tag
...@@ -38,7 +42,17 @@ module Releases ...@@ -38,7 +42,17 @@ module Releases
end end
def create_release(tag) def create_release(tag)
release = project.releases.create!( release = build_release(tag)
release.save!
success(tag: tag, release: release)
rescue => e
error(e.message, 400)
end
def build_release(tag)
project.releases.build(
name: name, name: name,
description: description, description: description,
author: current_user, author: current_user,
...@@ -46,10 +60,6 @@ module Releases ...@@ -46,10 +60,6 @@ module Releases
sha: tag.dereferenced_target.sha, sha: tag.dereferenced_target.sha,
links_attributes: params.dig(:assets, 'links') || [] links_attributes: params.dig(:assets, 'links') || []
) )
success(tag: tag, release: release)
rescue => e
error(e.message, 400)
end end
end end
end end
---
title: Set release name when adding release notes to an existing tag
merge_request: 26807
author:
type: fixed
...@@ -7,6 +7,7 @@ module Gitlab ...@@ -7,6 +7,7 @@ module Gitlab
{ {
project: project, project: project,
tag: raw_data.tag_name, tag: raw_data.tag_name,
name: raw_data.name,
description: raw_data.body, description: raw_data.body,
created_at: raw_data.created_at, created_at: raw_data.created_at,
updated_at: raw_data.created_at updated_at: raw_data.created_at
......
...@@ -18,40 +18,85 @@ describe Projects::Tags::ReleasesController do ...@@ -18,40 +18,85 @@ describe Projects::Tags::ReleasesController do
tag_id = release.tag tag_id = release.tag
project.releases.destroy_all # rubocop: disable DestroyAll 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) release = assigns(:release)
expect(release).not_to be_nil expect(release).not_to be_nil
expect(release).not_to be_persisted expect(release).not_to be_persisted
expect(response).to have_http_status(:ok)
end end
it 'retrieves an existing release' do 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) release = assigns(:release)
expect(release).not_to be_nil expect(release).not_to be_nil
expect(release).to be_persisted expect(release).to be_persisted
expect(response).to have_http_status(:ok)
end end
end end
describe 'PUT #update' do describe 'PUT #update' do
it 'updates release note description' 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(release.description).to eq("description updated")
expect(response).to have_http_status(:found)
end end
it 'deletes release note when description is null' do it 'creates a release if one does not exist' do
expect { update_release('') }.to change(project.releases, :count).by(-1) 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
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: { put :update, params: {
namespace_id: project.namespace.to_param, namespace_id: project.namespace.to_param,
project_id: project, project_id: project,
tag_id: release.tag, tag_id: tag_id,
release: { description: description } release: { description: description }
} }
end end
......
...@@ -25,6 +25,7 @@ describe Gitlab::LegacyGithubImport::ReleaseFormatter do ...@@ -25,6 +25,7 @@ describe Gitlab::LegacyGithubImport::ReleaseFormatter do
expected = { expected = {
project: project, project: project,
tag: 'v1.0.0', tag: 'v1.0.0',
name: 'First release',
description: 'Release v1.0.0', description: 'Release v1.0.0',
created_at: created_at, created_at: created_at,
updated_at: created_at updated_at: created_at
......
...@@ -18,6 +18,22 @@ RSpec.describe Release do ...@@ -18,6 +18,22 @@ RSpec.describe Release do
describe 'validation' do describe 'validation' do
it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:description) } 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 end
describe '#assets_count' do describe '#assets_count' do
......
...@@ -19,6 +19,8 @@ describe Releases::CreateService do ...@@ -19,6 +19,8 @@ describe Releases::CreateService do
shared_examples 'a successful release creation' do shared_examples 'a successful release creation' do
it 'creates a new release' do it 'creates a new release' do
result = service.execute result = service.execute
expect(project.releases.count).to eq(1)
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
expect(result[:tag]).not_to be_nil expect(result[:tag]).not_to be_nil
expect(result[:release]).not_to be_nil expect(result[:release]).not_to be_nil
...@@ -69,4 +71,12 @@ describe Releases::CreateService do ...@@ -69,4 +71,12 @@ describe Releases::CreateService do
end end
end 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment