Merge branch 'ac-releases-name-sha-author' into 'master'

Add name, author and sha to releases

See merge request gitlab-org/gitlab-ce!23763

app/models/release.rb

@@ -6,6 +6,7 @@ class Release < ActiveRecord::Base
   cache_markdown_field :description
 
   belongs_to :project
+  belongs_to :author, class_name: 'User'
 
   validates :description, :project, :tag, presence: true
 end

app/models/user.rb

@@ -130,6 +130,7 @@ class User < ActiveRecord::Base
   has_many :issues,                   dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent
   has_many :merge_requests,           dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent
   has_many :events,                   dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent
+  has_many :releases,                 dependent: :nullify, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent
   has_many :subscriptions,            dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_one  :abuse_report,             dependent: :destroy, foreign_key: :user_id # rubocop:disable Cop/ActiveRecordDependent

app/services/create_release_service.rb

@@ -13,8 +13,13 @@ class CreateReleaseService < BaseService
       if release
         error('Release already exists', 409)
       else
-        release = project.releases.new({ tag: tag_name, description: release_description })
-        release.save
+        release = project.releases.create!(
+          tag: tag_name,
+          name: tag_name,
+          sha: existing_tag.dereferenced_target.sha,
+          author: current_user,
+          description: release_description
+        )
 
         success(release)
       end

app/services/tags/create_service.rb

@@ -20,7 +20,7 @@ module Tags
       end
 
       if new_tag
-        if release_description
+        if release_description.present?
           CreateReleaseService.new(@project, @current_user)
             .execute(tag_name, release_description)
         end

changelogs/unreleased/ac-releases-name-sha-author.yml

@@ -0,0 +1,5 @@
+---
+title: Add name, author_id, and sha to releases table
+merge_request: 23763
+author:
+type: added

db/migrate/20181211092510_add_name_author_id_and_sha_to_releases.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class AddNameAuthorIdAndShaToReleases < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :releases, :author_id, :integer
+    add_column :releases, :name, :string
+    add_column :releases, :sha, :string
+  end
+end

db/migrate/20181211092514_add_author_id_index_and_fk_to_releases.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class AddAuthorIdIndexAndFkToReleases < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :releases, :author_id
+
+    add_concurrent_foreign_key :releases, :users, column: :author_id, on_delete: :nullify
+  end
+
+  def down
+    remove_foreign_key :releases, column: :author_id
+
+    remove_concurrent_index :releases, column: :author_id
+  end
+end

db/migrate/20181212104941_backfill_releases_name_with_tag_name.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class BackfillReleasesNameWithTagName < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    update_column_in_batches(:releases, :name, Release.arel_table[:tag])
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20181204154019) do
+ActiveRecord::Schema.define(version: 20181212104941) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1803,6 +1803,10 @@ ActiveRecord::Schema.define(version: 20181204154019) do
     t.datetime "updated_at"
     t.text "description_html"
     t.integer "cached_markdown_version"
+    t.integer "author_id"
+    t.string "name"
+    t.string "sha"
+    t.index ["author_id"], name: "index_releases_on_author_id", using: :btree
     t.index ["project_id", "tag"], name: "index_releases_on_project_id_and_tag", using: :btree
     t.index ["project_id"], name: "index_releases_on_project_id", using: :btree
   end
@@ -2433,6 +2437,7 @@ ActiveRecord::Schema.define(version: 20181204154019) do
   add_foreign_key "protected_tags", "projects", name: "fk_8e4af87648", on_delete: :cascade
   add_foreign_key "push_event_payloads", "events", name: "fk_36c74129da", on_delete: :cascade
   add_foreign_key "releases", "projects", name: "fk_47fe2a0596", on_delete: :cascade
+  add_foreign_key "releases", "users", column: "author_id", name: "fk_8e4456f90f", on_delete: :nullify
   add_foreign_key "remote_mirrors", "projects", on_delete: :cascade
   add_foreign_key "repository_languages", "projects", on_delete: :cascade
   add_foreign_key "resource_label_events", "issues", on_delete: :cascade

lib/gitlab/import_export/import_export.yml

@@ -27,7 +27,8 @@ project_tree:
     - :award_emoji
     - notes:
         :author
-  - :releases
+  - releases:
+      :author
   - project_members:
     - :user
   - merge_requests:

spec/factories/releases.rb

@@ -1,6 +1,7 @@
 FactoryBot.define do
   factory :release do
     tag "v1.1.0"
+    name { tag }
     description "Awesome release"
     project
   end

spec/lib/gitlab/import_export/all_models.yml

@@ -64,6 +64,7 @@ snippets:
 - award_emoji
 - user_agent_detail
 releases:
+- author
 - project
 project_members:
 - created_by

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -112,8 +112,11 @@ ProjectSnippet:
 - visibility_level
 Release:
 - id
+- name
 - tag
+- sha
 - description
+- author_id
 - project_id
 - created_at
 - updated_at

spec/migrations/backfill_releases_name_with_tag_name_spec.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20181212104941_backfill_releases_name_with_tag_name.rb')
+
+describe BackfillReleasesNameWithTagName, :migration do
+  let(:releases)   { table(:releases) }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects)   { table(:projects) }
+
+  let(:namespace)  { namespaces.create(name: 'foo', path: 'foo') }
+  let(:project)    { projects.create!(namespace_id: namespace.id) }
+  let(:release)    { releases.create!(project_id: project.id, tag: 'v1.0.0') }
+
+  it 'defaults name to tag value' do
+    expect(release.tag).to be_present
+
+    migrate!
+
+    release.reload
+    expect(release.name).to eq(release.tag)
+  end
+end

spec/models/release_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe Release do
 
   describe 'associations' do
     it { is_expected.to belong_to(:project) }
+    it { is_expected.to belong_to(:author).class_name('User') }
   end
 
   describe 'validation' do

spec/models/user_spec.rb

@@ -45,6 +45,7 @@ describe User do
     it { is_expected.to have_many(:uploads) }
     it { is_expected.to have_many(:reported_abuse_reports).dependent(:destroy).class_name('AbuseReport') }
     it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') }
+    it { is_expected.to have_many(:releases).dependent(:nullify) }
 
     describe "#abuse_report" do
       let(:current_user) { create(:user) }

spec/services/create_release_service_spec.rb

@@ -6,6 +6,8 @@ describe CreateReleaseService do
   let(:tag_name) { project.repository.tag_names.first }
   let(:description) { 'Awesome release!' }
   let(:service) { described_class.new(project, user) }
+  let(:tag) { project.repository.find_tag(tag_name) }
+  let(:sha) { tag.dereferenced_target.sha }
 
   it 'creates a new release' do
     result = service.execute(tag_name, description)
@@ -13,6 +15,9 @@ describe CreateReleaseService do
     release = project.releases.find_by(tag: tag_name)
     expect(release).not_to be_nil
     expect(release.description).to eq(description)
+    expect(release.name).to eq(tag_name)
+    expect(release.sha).to eq(sha)
+    expect(release.author).to eq(user)
   end
 
   it 'raises an error if the tag does not exist' do