Commit d07addbf authored by Andreas Brandl's avatar Andreas Brandl

Add foreign keys to todos table.

Fixes #32282.
parent 2fe09e6a
......@@ -60,7 +60,7 @@ class Note < ActiveRecord::Base
belongs_to :updated_by, class_name: "User"
belongs_to :last_edited_by, class_name: 'User'
has_many :todos, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :todos
has_many :events, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_one :system_note_metadata
......
......@@ -125,7 +125,7 @@ class User < ActiveRecord::Base
has_many :spam_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :builds, dependent: :nullify, class_name: 'Ci::Build' # rubocop:disable Cop/ActiveRecordDependent
has_many :pipelines, dependent: :nullify, class_name: 'Ci::Pipeline' # rubocop:disable Cop/ActiveRecordDependent
has_many :todos, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :todos
has_many :notification_settings, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :award_emoji, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id # rubocop:disable Cop/ActiveRecordDependent
......
---
title: Add foreign key constraints to todos table.
merge_request: 16849
author:
type: other
class AddForeignKeysToTodos < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
class Todo < ActiveRecord::Base
self.table_name = 'todos'
include EachBatch
end
BATCH_SIZE = 1000
DOWNTIME = false
disable_ddl_transaction!
def up
Todo.where('NOT EXISTS ( SELECT true FROM users WHERE id=todos.user_id )').each_batch(of: BATCH_SIZE) do |batch|
batch.delete_all
end
Todo.where('NOT EXISTS ( SELECT true FROM users WHERE id=todos.author_id )').each_batch(of: BATCH_SIZE) do |batch|
batch.delete_all
end
Todo.where('note_id IS NOT NULL AND NOT EXISTS ( SELECT true FROM notes WHERE id=todos.note_id )').each_batch(of: BATCH_SIZE) do |batch|
batch.delete_all
end
add_concurrent_foreign_key :todos, :users, column: :user_id, on_delete: :cascade
add_concurrent_foreign_key :todos, :users, column: :author_id, on_delete: :cascade
add_concurrent_foreign_key :todos, :notes, column: :note_id, on_delete: :cascade
end
def down
remove_foreign_key :todos, :users
remove_foreign_key :todos, column: :author_id
remove_foreign_key :todos, :notes
end
end
......@@ -2045,7 +2045,10 @@ ActiveRecord::Schema.define(version: 20180202111106) do
add_foreign_key "system_note_metadata", "notes", name: "fk_d83a918cb1", on_delete: :cascade
add_foreign_key "timelogs", "issues", name: "fk_timelogs_issues_issue_id", on_delete: :cascade
add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade
add_foreign_key "todos", "notes", name: "fk_91d1f47b13", on_delete: :cascade
add_foreign_key "todos", "projects", name: "fk_45054f9c45", on_delete: :cascade
add_foreign_key "todos", "users", column: "author_id", name: "fk_ccf0373936", on_delete: :cascade
add_foreign_key "todos", "users", name: "fk_d94154aa95", on_delete: :cascade
add_foreign_key "trending_projects", "projects", on_delete: :cascade
add_foreign_key "u2f_registrations", "users"
add_foreign_key "user_callouts", "users", on_delete: :cascade
......
require 'spec_helper'
require Rails.root.join('db', 'migrate', '20180201110056_add_foreign_keys_to_todos.rb')
describe AddForeignKeysToTodos, :migration do
let(:todos) { table(:todos) }
let(:project) { create(:project) }
let(:user) { create(:user) }
context 'add foreign key on user_id' do
let!(:todo_with_user) { create_todo(user_id: user.id) }
let!(:todo_without_user) { create_todo(user_id: 4711) }
it 'removes orphaned todos without corresponding user' do
expect { migrate! }.to change { Todo.count }.from(2).to(1)
end
it 'does not remove entries with valid user_id' do
expect { migrate! }.not_to change { todo_with_user.reload }
end
end
context 'add foreign key on author_id' do
let!(:todo_with_author) { create_todo(author_id: user.id) }
let!(:todo_with_invalid_author) { create_todo(author_id: 4711) }
it 'removes orphaned todos by author_id' do
expect { migrate! }.to change { Todo.count }.from(2).to(1)
end
it 'does not touch author_id for valid entries' do
expect { migrate! }.not_to change { todo_with_author.reload }
end
end
context 'add foreign key on note_id' do
let(:note) { create(:note) }
let!(:todo_with_note) { create_todo(note_id: note.id) }
let!(:todo_with_invalid_note) { create_todo(note_id: 4711) }
let!(:todo_without_note) { create_todo(note_id: nil) }
it 'deletes todo if note_id is set but does not exist in notes table' do
expect { migrate! }.to change { Todo.count }.from(3).to(2)
end
it 'does not touch entry if note_id is nil' do
expect { migrate! }.not_to change { todo_without_note.reload }
end
it 'does not touch note_id for valid entries' do
expect { migrate! }.not_to change { todo_with_note.reload }
end
end
def create_todo(**opts)
todos.create!(
project_id: project.id,
user_id: user.id,
author_id: user.id,
target_type: '',
action: 0,
state: '', **opts
)
end
end
......@@ -8,7 +8,7 @@ describe Note do
it { is_expected.to belong_to(:noteable).touch(false) }
it { is_expected.to belong_to(:author).class_name('User') }
it { is_expected.to have_many(:todos).dependent(:destroy) }
it { is_expected.to have_many(:todos) }
end
describe 'modules' do
......
......@@ -33,7 +33,7 @@ describe User do
it { is_expected.to have_many(:merge_requests).dependent(:destroy) }
it { is_expected.to have_many(:identities).dependent(:destroy) }
it { is_expected.to have_many(:spam_logs).dependent(:destroy) }
it { is_expected.to have_many(:todos).dependent(:destroy) }
it { is_expected.to have_many(:todos) }
it { is_expected.to have_many(:award_emoji).dependent(:destroy) }
it { is_expected.to have_many(:triggers).dependent(:destroy) }
it { is_expected.to have_many(:builds).dependent(:nullify) }
......
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