Add unique index to subscriptions on subscribable and user and project

5 files changed, 34 insertions, 3 deletions

diff --git a/app/models/subscription.rb b/app/models/subscription.rb
index f77aec0cacf..f881d999384 100644
--- a/app/models/subscription.rb
+++ b/app/models/subscription.rb
@@ -5,7 +5,7 @@ class Subscription < ActiveRecord::Base
 
   validates :user, :project, :subscribable, presence: true
 
-  validates :user_id,
-            uniqueness: { scope: [:subscribable_id, :subscribable_type] },
+  validates :project_id,
+            uniqueness: { scope: [:subscribable_id, :subscribable_type, :user_id] },
             presence: true
 end
diff --git a/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb b/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb
new file mode 100644
index 00000000000..4b1b29e1265
--- /dev/null
+++ b/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb
@@ -0,0 +1,18 @@
+class AddUniqueIndexToSubscriptions < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = true
+  DOWNTIME_REASON = 'This migration requires downtime because it changes a column to not accept null values.'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :subscriptions, [:subscribable_id, :subscribable_type, :user_id, :project_id], { unique: true, name: 'index_subscriptions_on_subscribable_and_user_id_and_project_id' }
+    remove_index :subscriptions, name: 'subscriptions_user_id_and_ref_fields' if index_name_exists?(:subscriptions, 'subscriptions_user_id_and_ref_fields', false)
+  end
+
+  def down
+    add_concurrent_index :subscriptions, [:subscribable_id, :subscribable_type, :user_id], { unique: true, name: 'subscriptions_user_id_and_ref_fields' }
+    remove_index :subscriptions, name: 'index_subscriptions_on_subscribable_and_user_id_and_project_id' if index_name_exists?(:subscriptions, 'index_subscriptions_on_subscribable_and_user_id_and_project_id', false)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index fdd7f179788..fbe7c054e0f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1055,7 +1055,7 @@ ActiveRecord::Schema.define(version: 20161109150329) do
     t.integer "project_id"
   end
 
-  add_index "subscriptions", ["subscribable_id", "subscribable_type", "user_id"], name: "subscriptions_user_id_and_ref_fields", unique: true, using: :btree
+  add_index "subscriptions", ["subscribable_id", "subscribable_type", "user_id", "project_id"], name: "index_subscriptions_on_subscribable_and_user_id_and_project_id", unique: true, using: :btree
 
   create_table "taggings", force: :cascade do |t|
     t.integer "tag_id"
diff --git a/spec/factories/subscriptions.rb b/spec/factories/subscriptions.rb
new file mode 100644
index 00000000000..b11b0a0a17b
--- /dev/null
+++ b/spec/factories/subscriptions.rb
@@ -0,0 +1,7 @@
+FactoryGirl.define do
+  factory :subscription do
+    user
+    project factory: :empty_project
+    subscribable factory: :issue
+  end
+end
diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb
index 6cd6e01d0c7..ab674958387 100644
--- a/spec/models/subscription_spec.rb
+++ b/spec/models/subscription_spec.rb
@@ -11,5 +11,11 @@ describe Subscription, models: true do
     it { is_expected.to validate_presence_of(:project) }
     it { is_expected.to validate_presence_of(:subscribable) }
     it { is_expected.to validate_presence_of(:user) }
+
+    it 'validates uniqueness of project_id scoped to subscribable_id, subscribable_type, and user_id' do
+      create(:subscription)
+
+      expect(subject).to validate_uniqueness_of(:project_id).scoped_to([:subscribable_id, :subscribable_type, :user_id])
+    end
   end
 end