diff --git a/app/models/user.rb b/app/models/user.rb index fdf3618b4dcf9a1baef9547991bbe856b3d00581..37f2e8b680e56937668da9bcc64f853c1edeb549 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -130,7 +130,7 @@ class User < ActiveRecord::Base 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 - has_many :notification_settings, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :notification_settings 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 diff --git a/changelogs/unreleased/44824-remove-ghost-notification-settings-for-group-and-project.yml b/changelogs/unreleased/44824-remove-ghost-notification-settings-for-group-and-project.yml new file mode 100644 index 0000000000000000000000000000000000000000..ddc878ee710c005fcc9459b57d83043ea04dc78e --- /dev/null +++ b/changelogs/unreleased/44824-remove-ghost-notification-settings-for-group-and-project.yml @@ -0,0 +1,5 @@ +--- +title: Adds foreign key to notification_settings.user_id +merge_request: 20567 +author: Jacopo Beschi @jacopo-beschi +type: added diff --git a/db/migrate/20180710162338_add_foreign_key_from_notification_settings_to_users.rb b/db/migrate/20180710162338_add_foreign_key_from_notification_settings_to_users.rb new file mode 100644 index 0000000000000000000000000000000000000000..91656f194e5f258e8a1cdf6a6c14564cb8a94df8 --- /dev/null +++ b/db/migrate/20180710162338_add_foreign_key_from_notification_settings_to_users.rb @@ -0,0 +1,30 @@ +class AddForeignKeyFromNotificationSettingsToUsers < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + class NotificationSetting < ActiveRecord::Base + self.table_name = 'notification_settings' + + include EachBatch + end + + class User < ActiveRecord::Base + self.table_name = 'users' + end + + DOWNTIME = false + + disable_ddl_transaction! + + def up + NotificationSetting.each_batch(of: 1000) do |batch| + batch.where('NOT EXISTS (?)', User.select(1).where('users.id = notification_settings.user_id')) + .delete_all + end + + add_concurrent_foreign_key(:notification_settings, :users, column: :user_id, on_delete: :cascade) + end + + def down + remove_foreign_key(:notification_settings, column: :user_id) + end +end diff --git a/db/schema.rb b/db/schema.rb index 769baa825a5f8dc66335513fce32a2b1f485943b..2bef2971f299469d7313c621d585bcf52e87a717 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2350,6 +2350,7 @@ ActiveRecord::Schema.define(version: 20180726172057) do add_foreign_key "milestones", "projects", name: "fk_9bd0a0c791", on_delete: :cascade add_foreign_key "note_diff_files", "notes", column: "diff_note_id", on_delete: :cascade add_foreign_key "notes", "projects", name: "fk_99e097b079", on_delete: :cascade + add_foreign_key "notification_settings", "users", name: "fk_0c95e91db7", on_delete: :cascade add_foreign_key "oauth_openid_requests", "oauth_access_grants", column: "access_grant_id", name: "fk_oauth_openid_requests_oauth_access_grants_access_grant_id" add_foreign_key "pages_domains", "projects", name: "fk_ea2f6dfc6f", on_delete: :cascade add_foreign_key "personal_access_tokens", "users" diff --git a/spec/migrations/add_foreign_key_from_notification_settings_to_users_spec.rb b/spec/migrations/add_foreign_key_from_notification_settings_to_users_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..656d4f75e3b480ba02d461317f45edcfb524aa01 --- /dev/null +++ b/spec/migrations/add_foreign_key_from_notification_settings_to_users_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20180710162338_add_foreign_key_from_notification_settings_to_users.rb') + +describe AddForeignKeyFromNotificationSettingsToUsers, :migration do + let(:notification_settings) { table(:notification_settings) } + let(:users) { table(:users) } + let(:projects) { table(:projects) } + + before do + users.create!(email: 'email@email.com', name: 'foo', username: 'foo', projects_limit: 0) + projects.create!(name: 'gitlab', path: 'gitlab-org/gitlab-ce', namespace_id: 1) + end + + describe 'removal of orphans without user' do + let!(:notification_setting_without_user) { create_notification_settings!(user_id: 123) } + let!(:notification_setting_with_user) { create_notification_settings!(user_id: users.last.id) } + + it 'removes orphaned notification_settings without user' do + expect { migrate! }.to change { notification_settings.count }.by(-1) + end + + it "doesn't remove notification_settings with valid user" do + expect { migrate! }.not_to change { notification_setting_with_user.reload } + end + end + + def create_notification_settings!(**opts) + notification_settings.create!( + source_id: projects.last.id, + source_type: 'Project', + user_id: users.last.id, + **opts) + end +end