Merge branch 'issue_3359' into 'master'

Remove duplicated notification settings and add unique index

See merge request !4472

5 files changed, 30 insertions, 6 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 6138573af27..bee1a824974 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -36,6 +36,7 @@ v 8.9.0 (unreleased)
   - Cache project build count in sidebar nav
   - Reduce number of queries needed to render issue labels in the sidebar
   - Improve error handling importing projects
+  - Remove duplicated notification settings
   - Put project Files and Commits tabs under Code tab
   - Replace Colorize with Rainbow for coloring console output in Rake tasks.
   - An indicator is now displayed at the top of the comment field for confidential issues.
diff --git a/db/migrate/20160603180330_remove_duplicated_notification_settings.rb b/db/migrate/20160603180330_remove_duplicated_notification_settings.rb
new file mode 100644
index 00000000000..c2fcac4c53d
--- /dev/null
+++ b/db/migrate/20160603180330_remove_duplicated_notification_settings.rb
@@ -0,0 +1,7 @@
+class RemoveDuplicatedNotificationSettings < ActiveRecord::Migration
+  def up
+    execute <<-SQL
+      DELETE FROM notification_settings WHERE id NOT IN ( SELECT min_id from (SELECT MIN(id) as min_id FROM notification_settings GROUP BY user_id, source_type, source_id) as dups )
+    SQL
+  end
+end
diff --git a/db/migrate/20160603182247_add_index_to_notification_settings.rb b/db/migrate/20160603182247_add_index_to_notification_settings.rb
new file mode 100644
index 00000000000..06462042b09
--- /dev/null
+++ b/db/migrate/20160603182247_add_index_to_notification_settings.rb
@@ -0,0 +1,9 @@
+class AddIndexToNotificationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def change
+    add_concurrent_index :notification_settings, [:user_id, :source_id, :source_type], { unique: true, name: "index_notifications_on_user_id_and_source_id_and_source_type" }
+  end
+end
diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb
index fd14234c558..978c3f7896d 100644
--- a/lib/gitlab/database/migration_helpers.rb
+++ b/lib/gitlab/database/migration_helpers.rb
@@ -11,7 +11,7 @@ module Gitlab
       #     add_concurrent_index :users, :some_column
       #
       # See Rails' `add_index` for more info on the available arguments.
-      def add_concurrent_index(*args)
+      def add_concurrent_index(table_name, column_name, options = {})
         if transaction_open?
           raise 'add_concurrent_index can not be run inside a transaction, ' \
             'you can disable transactions by calling disable_ddl_transaction! ' \
@@ -19,10 +19,10 @@ module Gitlab
         end
 
         if Database.postgresql?
-          args << { algorithm: :concurrently }
+          options = options.merge({ algorithm: :concurrently })
         end
 
-        add_index(*args)
+        add_index(table_name, column_name, options)
       end
 
       # Updates the value of a column in batches.
diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb
index 35ade7a2be0..83ddabe6b0b 100644
--- a/spec/lib/gitlab/database/migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migration_helpers_spec.rb
@@ -16,14 +16,21 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
       end
 
       context 'using PostgreSQL' do
-        it 'creates the index concurrently' do
-          expect(Gitlab::Database).to receive(:postgresql?).and_return(true)
+        before { expect(Gitlab::Database).to receive(:postgresql?).and_return(true) }
 
+        it 'creates the index concurrently' do
           expect(model).to receive(:add_index).
             with(:users, :foo, algorithm: :concurrently)
 
           model.add_concurrent_index(:users, :foo)
         end
+
+        it 'creates unique index concurrently' do
+          expect(model).to receive(:add_index).
+            with(:users, :foo, { algorithm: :concurrently, unique: true })
+
+          model.add_concurrent_index(:users, :foo, unique: true)
+        end
       end
 
       context 'using MySQL' do
@@ -31,7 +38,7 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
           expect(Gitlab::Database).to receive(:postgresql?).and_return(false)
 
           expect(model).to receive(:add_index).
-            with(:users, :foo)
+            with(:users, :foo, {})
 
           model.add_concurrent_index(:users, :foo)
         end