summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@mcgivern.me.uk>2017-11-10 11:26:32 +0000
committerSean McGivern <sean@mcgivern.me.uk>2017-11-10 11:26:32 +0000
commit58dd9d73ab51c74bca354ae632b252db8def4e41 (patch)
treee619c52ea7989deb84ed6cb477ee8de49548a74e
parent09ae380698578205ece74ed594d48a3177042024 (diff)
parentd825c9cb5d89d18685a196789b477df83998fed2 (diff)
downloadgitlab-ce-58dd9d73ab51c74bca354ae632b252db8def4e41.tar.gz
Merge branch 'cleanup-issues-schema' into 'master'
Clean up the schema of the table "issues" Closes #31811 See merge request gitlab-org/gitlab-ce!15225
-rw-r--r--app/serializers/issue_entity.rb1
-rw-r--r--changelogs/unreleased/cleanup-issues-schema.yml5
-rw-r--r--db/migrate/20171106132212_issues_confidential_not_null.rb23
-rw-r--r--db/migrate/20171106135924_issues_milestone_id_foreign_key.rb38
-rw-r--r--db/migrate/20171106150657_issues_updated_by_id_foreign_key.rb45
-rw-r--r--db/migrate/20171106151218_issues_moved_to_id_foreign_key.rb44
-rw-r--r--db/migrate/20171106154015_remove_issues_branch_name.rb13
-rw-r--r--db/migrate/20171106155656_turn_issues_due_date_index_to_partial_index.rb37
-rw-r--r--db/migrate/20171106171453_add_timezone_to_issues_closed_at.rb19
-rw-r--r--db/post_migrate/20171106180641_cleanup_add_timezone_to_issues_closed_at.rb19
-rw-r--r--db/schema.rb14
-rw-r--r--lib/gitlab/hook_data/issue_builder.rb1
-rw-r--r--spec/features/issues/issue_detail_spec.rb7
-rw-r--r--spec/lib/gitlab/hook_data/issue_builder_spec.rb1
-rw-r--r--spec/services/milestones/destroy_service_spec.rb2
15 files changed, 257 insertions, 12 deletions
diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb
index 5f47592e4ad..9d52b8d9752 100644
--- a/app/serializers/issue_entity.rb
+++ b/app/serializers/issue_entity.rb
@@ -3,7 +3,6 @@ class IssueEntity < IssuableEntity
expose :state
expose :deleted_at
- expose :branch_name
expose :confidential
expose :discussion_locked
expose :assignees, using: API::Entities::UserBasic
diff --git a/changelogs/unreleased/cleanup-issues-schema.yml b/changelogs/unreleased/cleanup-issues-schema.yml
new file mode 100644
index 00000000000..9f5fb0bdf82
--- /dev/null
+++ b/changelogs/unreleased/cleanup-issues-schema.yml
@@ -0,0 +1,5 @@
+---
+title: Clean up schema of the "issues" table
+merge_request:
+author:
+type: other
diff --git a/db/migrate/20171106132212_issues_confidential_not_null.rb b/db/migrate/20171106132212_issues_confidential_not_null.rb
new file mode 100644
index 00000000000..c959d2dd938
--- /dev/null
+++ b/db/migrate/20171106132212_issues_confidential_not_null.rb
@@ -0,0 +1,23 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class IssuesConfidentialNotNull < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ class Issue < ActiveRecord::Base
+ self.table_name = 'issues'
+ end
+
+ def up
+ Issue.where('confidential IS NULL').update_all(confidential: false)
+
+ change_column_null :issues, :confidential, false
+ end
+
+ def down
+ # There's no way / point to revert this.
+ end
+end
diff --git a/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb b/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb
new file mode 100644
index 00000000000..e6a780d0964
--- /dev/null
+++ b/db/migrate/20171106135924_issues_milestone_id_foreign_key.rb
@@ -0,0 +1,38 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class IssuesMilestoneIdForeignKey < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ class Issue < ActiveRecord::Base
+ include EachBatch
+
+ self.table_name = 'issues'
+
+ def self.with_orphaned_milestones
+ where('NOT EXISTS (SELECT true FROM milestones WHERE milestones.id = issues.milestone_id)')
+ end
+ end
+
+ def up
+ Issue.with_orphaned_milestones.each_batch(of: 100) do |batch|
+ batch.update_all(milestone_id: nil)
+ end
+
+ add_concurrent_foreign_key(
+ :issues,
+ :milestones,
+ column: :milestone_id,
+ on_delete: :nullify
+ )
+ end
+
+ def down
+ remove_foreign_key_without_error(:issues, column: :milestone_id)
+ end
+end
diff --git a/db/migrate/20171106150657_issues_updated_by_id_foreign_key.rb b/db/migrate/20171106150657_issues_updated_by_id_foreign_key.rb
new file mode 100644
index 00000000000..3b8844d7d9f
--- /dev/null
+++ b/db/migrate/20171106150657_issues_updated_by_id_foreign_key.rb
@@ -0,0 +1,45 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class IssuesUpdatedByIdForeignKey < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ class Issue < ActiveRecord::Base
+ include EachBatch
+
+ self.table_name = 'issues'
+
+ def self.with_orphaned_updaters
+ where('NOT EXISTS (SELECT true FROM users WHERE users.id = issues.updated_by_id)')
+ .where('updated_by_id IS NOT NULL')
+ end
+ end
+
+ def up
+ Issue.with_orphaned_updaters.each_batch(of: 100) do |batch|
+ batch.update_all(updated_by_id: nil)
+ end
+
+ # This index is only used for foreign keys, and those in turn will always
+ # specify a value. As such we can add a WHERE condition to make the index
+ # smaller.
+ add_concurrent_index(:issues, :updated_by_id, where: 'updated_by_id IS NOT NULL')
+
+ add_concurrent_foreign_key(
+ :issues,
+ :users,
+ column: :updated_by_id,
+ on_delete: :nullify
+ )
+ end
+
+ def down
+ remove_foreign_key_without_error(:issues, column: :updated_by_id)
+ remove_concurrent_index(:issues, :updated_by_id)
+ end
+end
diff --git a/db/migrate/20171106151218_issues_moved_to_id_foreign_key.rb b/db/migrate/20171106151218_issues_moved_to_id_foreign_key.rb
new file mode 100644
index 00000000000..8d2ceb8cc18
--- /dev/null
+++ b/db/migrate/20171106151218_issues_moved_to_id_foreign_key.rb
@@ -0,0 +1,44 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class IssuesMovedToIdForeignKey < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ class Issue < ActiveRecord::Base
+ include EachBatch
+
+ self.table_name = 'issues'
+
+ def self.with_orphaned_moved_to_issues
+ where('NOT EXISTS (SELECT true FROM issues WHERE issues.id = issues.moved_to_id)')
+ .where('moved_to_id IS NOT NULL')
+ end
+ end
+
+ def up
+ Issue.with_orphaned_moved_to_issues.each_batch(of: 100) do |batch|
+ batch.update_all(moved_to_id: nil)
+ end
+
+ add_concurrent_foreign_key(
+ :issues,
+ :issues,
+ column: :moved_to_id,
+ on_delete: :nullify
+ )
+
+ # We're using a partial index here so we only index the data we actually
+ # care about.
+ add_concurrent_index(:issues, :moved_to_id, where: 'moved_to_id IS NOT NULL')
+ end
+
+ def down
+ remove_foreign_key_without_error(:issues, column: :moved_to_id)
+ remove_concurrent_index(:issues, :moved_to_id)
+ end
+end
diff --git a/db/migrate/20171106154015_remove_issues_branch_name.rb b/db/migrate/20171106154015_remove_issues_branch_name.rb
new file mode 100644
index 00000000000..3d08225c96d
--- /dev/null
+++ b/db/migrate/20171106154015_remove_issues_branch_name.rb
@@ -0,0 +1,13 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveIssuesBranchName < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ def change
+ remove_column :issues, :branch_name, :string
+ end
+end
diff --git a/db/migrate/20171106155656_turn_issues_due_date_index_to_partial_index.rb b/db/migrate/20171106155656_turn_issues_due_date_index_to_partial_index.rb
new file mode 100644
index 00000000000..e4bed778695
--- /dev/null
+++ b/db/migrate/20171106155656_turn_issues_due_date_index_to_partial_index.rb
@@ -0,0 +1,37 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class TurnIssuesDueDateIndexToPartialIndex < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ NEW_INDEX_NAME = 'idx_issues_on_project_id_and_due_date_and_id_and_state_partial'
+ OLD_INDEX_NAME = 'index_issues_on_project_id_and_due_date_and_id_and_state'
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index(
+ :issues,
+ [:project_id, :due_date, :id, :state],
+ where: 'due_date IS NOT NULL',
+ name: NEW_INDEX_NAME
+ )
+
+ # We set the column name to nil as otherwise Rails will ignore the custom
+ # index name and remove the wrong index.
+ remove_concurrent_index(:issues, nil, name: OLD_INDEX_NAME)
+ end
+
+ def down
+ add_concurrent_index(
+ :issues,
+ [:project_id, :due_date, :id, :state],
+ name: OLD_INDEX_NAME
+ )
+
+ remove_concurrent_index(:issues, nil, name: NEW_INDEX_NAME)
+ end
+end
diff --git a/db/migrate/20171106171453_add_timezone_to_issues_closed_at.rb b/db/migrate/20171106171453_add_timezone_to_issues_closed_at.rb
new file mode 100644
index 00000000000..ad540b1e509
--- /dev/null
+++ b/db/migrate/20171106171453_add_timezone_to_issues_closed_at.rb
@@ -0,0 +1,19 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddTimezoneToIssuesClosedAt < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ change_column_type_concurrently(:issues, :closed_at, :datetime_with_timezone)
+ end
+
+ def down
+ cleanup_concurrent_column_type_change(:issues, :closed_at)
+ end
+end
diff --git a/db/post_migrate/20171106180641_cleanup_add_timezone_to_issues_closed_at.rb b/db/post_migrate/20171106180641_cleanup_add_timezone_to_issues_closed_at.rb
new file mode 100644
index 00000000000..88dd8f89ba6
--- /dev/null
+++ b/db/post_migrate/20171106180641_cleanup_add_timezone_to_issues_closed_at.rb
@@ -0,0 +1,19 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class CleanupAddTimezoneToIssuesClosedAt < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ cleanup_concurrent_column_type_change(:issues, :closed_at)
+ end
+
+ # rubocop:disable Migration/Datetime
+ def down
+ change_column_type_concurrently(:issues, :closed_at, :datetime)
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index c60cb729b75..37e08d453c8 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20171106101200) do
+ActiveRecord::Schema.define(version: 20171106180641) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -817,13 +817,12 @@ ActiveRecord::Schema.define(version: 20171106101200) do
t.integer "project_id"
t.datetime "created_at"
t.datetime "updated_at"
- t.string "branch_name"
t.text "description"
t.integer "milestone_id"
t.string "state"
t.integer "iid"
t.integer "updated_by_id"
- t.boolean "confidential", default: false
+ t.boolean "confidential", default: false, null: false
t.datetime "deleted_at"
t.date "due_date"
t.integer "moved_to_id"
@@ -832,11 +831,11 @@ ActiveRecord::Schema.define(version: 20171106101200) do
t.text "description_html"
t.integer "time_estimate"
t.integer "relative_position"
- t.datetime "closed_at"
t.integer "cached_markdown_version"
t.datetime "last_edited_at"
t.integer "last_edited_by_id"
t.boolean "discussion_locked"
+ t.datetime_with_timezone "closed_at"
end
add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -845,13 +844,15 @@ ActiveRecord::Schema.define(version: 20171106101200) do
add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree
add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree
+ add_index "issues", ["moved_to_id"], name: "index_issues_on_moved_to_id", where: "(moved_to_id IS NOT NULL)", using: :btree
add_index "issues", ["project_id", "created_at", "id", "state"], name: "index_issues_on_project_id_and_created_at_and_id_and_state", using: :btree
- add_index "issues", ["project_id", "due_date", "id", "state"], name: "index_issues_on_project_id_and_due_date_and_id_and_state", using: :btree
+ add_index "issues", ["project_id", "due_date", "id", "state"], name: "idx_issues_on_project_id_and_due_date_and_id_and_state_partial", where: "(due_date IS NOT NULL)", using: :btree
add_index "issues", ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree
add_index "issues", ["project_id", "updated_at", "id", "state"], name: "index_issues_on_project_id_and_updated_at_and_id_and_state", using: :btree
add_index "issues", ["relative_position"], name: "index_issues_on_relative_position", using: :btree
add_index "issues", ["state"], name: "index_issues_on_state", using: :btree
add_index "issues", ["title"], name: "index_issues_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"}
+ add_index "issues", ["updated_by_id"], name: "index_issues_on_updated_by_id", where: "(updated_by_id IS NOT NULL)", using: :btree
create_table "keys", force: :cascade do |t|
t.integer "user_id"
@@ -1937,8 +1938,11 @@ ActiveRecord::Schema.define(version: 20171106101200) do
add_foreign_key "issue_assignees", "issues", name: "fk_b7d881734a", on_delete: :cascade
add_foreign_key "issue_assignees", "users", name: "fk_5e0c8d9154", on_delete: :cascade
add_foreign_key "issue_metrics", "issues", on_delete: :cascade
+ add_foreign_key "issues", "issues", column: "moved_to_id", name: "fk_a194299be1", on_delete: :nullify
+ add_foreign_key "issues", "milestones", name: "fk_96b1dd429c", on_delete: :nullify
add_foreign_key "issues", "projects", name: "fk_899c8f3231", on_delete: :cascade
add_foreign_key "issues", "users", column: "author_id", name: "fk_05f1e72feb", on_delete: :nullify
+ add_foreign_key "issues", "users", column: "updated_by_id", name: "fk_ffed080f01", on_delete: :nullify
add_foreign_key "label_priorities", "labels", on_delete: :cascade
add_foreign_key "label_priorities", "projects", on_delete: :cascade
add_foreign_key "labels", "namespaces", column: "group_id", on_delete: :cascade
diff --git a/lib/gitlab/hook_data/issue_builder.rb b/lib/gitlab/hook_data/issue_builder.rb
index de9cab80a02..196f2b6b34c 100644
--- a/lib/gitlab/hook_data/issue_builder.rb
+++ b/lib/gitlab/hook_data/issue_builder.rb
@@ -4,7 +4,6 @@ module Gitlab
SAFE_HOOK_ATTRIBUTES = %i[
assignee_id
author_id
- branch_name
closed_at
confidential
created_at
diff --git a/spec/features/issues/issue_detail_spec.rb b/spec/features/issues/issue_detail_spec.rb
index 6fbee0ebcb5..4224a8fe5d4 100644
--- a/spec/features/issues/issue_detail_spec.rb
+++ b/spec/features/issues/issue_detail_spec.rb
@@ -1,9 +1,9 @@
require 'rails_helper'
feature 'Issue Detail', :js do
- let(:user) { create(:user) }
- let(:project) { create(:project, :public) }
- let(:issue) { create(:issue, project: project, author: user) }
+ let(:user) { create(:user) }
+ let(:project) { create(:project, :public) }
+ let(:issue) { create(:issue, project: project, author: user) }
context 'when user displays the issue' do
before do
@@ -27,6 +27,7 @@ feature 'Issue Detail', :js do
click_link 'Edit'
fill_in 'issuable-title', with: 'issue title'
click_button 'Save'
+ wait_for_requests
Users::DestroyService.new(user).execute(user)
diff --git a/spec/lib/gitlab/hook_data/issue_builder_spec.rb b/spec/lib/gitlab/hook_data/issue_builder_spec.rb
index 6c529cdd051..aeacd577d18 100644
--- a/spec/lib/gitlab/hook_data/issue_builder_spec.rb
+++ b/spec/lib/gitlab/hook_data/issue_builder_spec.rb
@@ -11,7 +11,6 @@ describe Gitlab::HookData::IssueBuilder do
%w[
assignee_id
author_id
- branch_name
closed_at
confidential
created_at
diff --git a/spec/services/milestones/destroy_service_spec.rb b/spec/services/milestones/destroy_service_spec.rb
index 5739386dd0d..16e288b3148 100644
--- a/spec/services/milestones/destroy_service_spec.rb
+++ b/spec/services/milestones/destroy_service_spec.rb
@@ -4,7 +4,7 @@ describe Milestones::DestroyService do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:milestone) { create(:milestone, title: 'Milestone v1.0', project: project) }
- let(:issue) { create(:issue, project: project, milestone: milestone) }
+ let!(:issue) { create(:issue, project: project, milestone: milestone) }
let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) }
before do