From b782ba1113970728989eebdf4c8fc44f8091c8d8 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Wed, 12 Dec 2018 10:56:43 +0100 Subject: [PATCH] Add name, author and sha to releases This commit adds a name to each release, defaulting it to tag name, keeps track of the SHA when a new release is created and tracks the current user as release author. --- app/models/release.rb | 1 + app/models/user.rb | 1 + app/services/create_release_service.rb | 9 ++++++-- app/services/tags/create_service.rb | 2 +- .../ac-releases-name-sha-author.yml | 5 ++++ ..._add_name_author_id_and_sha_to_releases.rb | 13 +++++++++++ ..._add_author_id_index_and_fk_to_releases.rb | 21 +++++++++++++++++ ...41_backfill_releases_name_with_tag_name.rb | 17 ++++++++++++++ db/schema.rb | 7 +++++- lib/gitlab/import_export/import_export.yml | 3 ++- spec/factories/releases.rb | 1 + spec/lib/gitlab/import_export/all_models.yml | 1 + .../import_export/safe_model_attributes.yml | 3 +++ ...ckfill_releases_name_with_tag_name_spec.rb | 23 +++++++++++++++++++ spec/models/release_spec.rb | 1 + spec/models/user_spec.rb | 1 + spec/services/create_release_service_spec.rb | 5 ++++ 17 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/ac-releases-name-sha-author.yml create mode 100644 db/migrate/20181211092510_add_name_author_id_and_sha_to_releases.rb create mode 100644 db/migrate/20181211092514_add_author_id_index_and_fk_to_releases.rb create mode 100644 db/migrate/20181212104941_backfill_releases_name_with_tag_name.rb create mode 100644 spec/migrations/backfill_releases_name_with_tag_name_spec.rb diff --git a/app/models/release.rb b/app/models/release.rb index cba80ad30ca..7a09ee459a6 100644 --- a/app/models/release.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index dbd754dd25a..f20756d1cc3 100644 --- a/app/models/user.rb +++ b/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 diff --git a/app/services/create_release_service.rb b/app/services/create_release_service.rb index 8d1fdbe11c3..ab2dc5337aa 100644 --- a/app/services/create_release_service.rb +++ b/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 diff --git a/app/services/tags/create_service.rb b/app/services/tags/create_service.rb index 35390f5082c..6bb9bb3988e 100644 --- a/app/services/tags/create_service.rb +++ b/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 diff --git a/changelogs/unreleased/ac-releases-name-sha-author.yml b/changelogs/unreleased/ac-releases-name-sha-author.yml new file mode 100644 index 00000000000..e84b82847eb --- /dev/null +++ b/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 diff --git a/db/migrate/20181211092510_add_name_author_id_and_sha_to_releases.rb b/db/migrate/20181211092510_add_name_author_id_and_sha_to_releases.rb new file mode 100644 index 00000000000..60815e0c31a --- /dev/null +++ b/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 diff --git a/db/migrate/20181211092514_add_author_id_index_and_fk_to_releases.rb b/db/migrate/20181211092514_add_author_id_index_and_fk_to_releases.rb new file mode 100644 index 00000000000..f6350a49944 --- /dev/null +++ b/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 diff --git a/db/migrate/20181212104941_backfill_releases_name_with_tag_name.rb b/db/migrate/20181212104941_backfill_releases_name_with_tag_name.rb new file mode 100644 index 00000000000..e152dc87bc1 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index e5e19eb7745..1a6a16decee 100644 --- a/db/schema.rb +++ b/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 @@ -2423,6 +2427,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 diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index d10d4f2f746..3cd8ede830c 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/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: diff --git a/spec/factories/releases.rb b/spec/factories/releases.rb index d80c65cf8bb..18047c74a5d 100644 --- a/spec/factories/releases.rb +++ b/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 diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index bae5b21c26f..b5f63630e8d 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -63,6 +63,7 @@ snippets: - award_emoji - user_agent_detail releases: +- author - project project_members: - created_by diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index d3bfde181bc..24b1f2d995b 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/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 diff --git a/spec/migrations/backfill_releases_name_with_tag_name_spec.rb b/spec/migrations/backfill_releases_name_with_tag_name_spec.rb new file mode 100644 index 00000000000..6f436de84b7 --- /dev/null +++ b/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 diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 3f86347c3ae..51725eeacac 100644 --- a/spec/models/release_spec.rb +++ b/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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ff075e65c76..8b3021113bc 100644 --- a/spec/models/user_spec.rb +++ b/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) } diff --git a/spec/services/create_release_service_spec.rb b/spec/services/create_release_service_spec.rb index ac0a0458f56..1a2dd0b39ee 100644 --- a/spec/services/create_release_service_spec.rb +++ b/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