Add unique indexes - part 4 (#8059)

* Rubocop unique index cop needs a static list of columns

* Add unique index on organizations secret

* Add unique index on chat_channel_memberships chat_channel_id

* Add unique index to chat_channel slug

* Add unique index to badge_achievements badge_id

app/models/chat_channel.rb

@@ -1,6 +1,9 @@
 class ChatChannel < ApplicationRecord
   attr_accessor :current_user, :usernames_string
 
+  CHANNEL_TYPES = %w[open invite_only direct].freeze
+  STATUSES = %w[active inactive blocked].freeze
+
   has_many :messages, dependent: :destroy
   has_many :chat_channel_memberships, dependent: :destroy
   has_many :users, through: :chat_channel_memberships
@@ -15,8 +18,8 @@ class ChatChannel < ApplicationRecord
   has_many :rejected_users, through: :rejected_memberships, class_name: "User", source: :user
   has_many :mod_users, through: :mod_memberships, class_name: "User", source: :user
 
-  validates :channel_type, presence: true, inclusion: { in: %w[open invite_only direct] }
-  validates :status, presence: true, inclusion: { in: %w[active inactive blocked] }
+  validates :channel_type, presence: true, inclusion: { in: CHANNEL_TYPES }
+  validates :status, presence: true, inclusion: { in: STATUSES }
   validates :slug, uniqueness: true, presence: true
   validates :description, length: { maximum: 200 }, allow_blank: true
 

app/models/chat_channel_membership.rb

@@ -5,13 +5,17 @@ class ChatChannelMembership < ApplicationRecord
   SEARCH_SERIALIZER = Search::ChatChannelMembershipSerializer
   SEARCH_CLASS = Search::ChatChannelMembership
 
+  ROLES = %w[member mod].freeze
+  STATUSES = %w[active inactive pending rejected left_channel removed_from_channel joining_request].freeze
+
   belongs_to :chat_channel
   belongs_to :user
 
-  validates :user_id, presence: true, uniqueness: { scope: :chat_channel_id }
   validates :chat_channel_id, presence: true, uniqueness: { scope: :user_id }
-  validates :status, inclusion: { in: %w[active inactive pending rejected left_channel removed_from_channel joining_request] }
-  validates :role, inclusion: { in: %w[member mod] }
+  validates :role, inclusion: { in: ROLES }
+  validates :status, inclusion: { in: STATUSES }
+  validates :user_id, presence: true
+
   validate  :permission
 
   after_commit :index_to_elasticsearch, on: %i[create update]
@@ -89,7 +93,11 @@ class ChatChannelMembership < ApplicationRecord
   end
 
   def permission
-    errors.add(:user_id, "is not allowed in chat") if chat_channel.direct? && chat_channel.slug.split("/").exclude?(user.username)
+    return unless chat_channel
+    return unless chat_channel.direct? && chat_channel.slug.split("/").exclude?(user.username)
+
+    errors.add(:user_id, "is not allowed in chat")
+
     # To be possibly implemented in future
     # if chat_channel.users.size > 128
     #   errors.add(:base, "too many members in channel")

app/models/organization.rb

@@ -35,7 +35,8 @@ class Organization < ApplicationRecord
   validates :name, :summary, :url, :profile_image, presence: true
   validates :name, length: { maximum: 50 }
   validates :proof, length: { maximum: 1500 }
-  validates :secret, length: { is: 100 }, uniqueness: { allow_blank: true }, allow_blank: true
+  validates :secret, length: { is: 100 }, allow_nil: true
+  validates :secret, uniqueness: true
   validates :slug, exclusion: { in: ReservedWords.all, message: MESSAGES[:reserved_word] }
   validates :slug, format: { with: SLUG_REGEXP }, length: { in: 2..18 }
   validates :slug, presence: true, uniqueness: { case_sensitive: false }

app/models/response_template.rb

@@ -3,7 +3,6 @@ class ResponseTemplate < ApplicationRecord
 
   belongs_to :user, optional: true
 
-  UNIQUENESS_SCOPE = %i[user_id type_of content_type].freeze
   TYPE_OF_TYPES = %w[personal_comment mod_comment abuse_report_email_reply email_reply tag_adjustment].freeze
   USER_NIL_TYPE_OF_TYPES = %w[mod_comment abuse_report_email_reply email_reply tag_adjustment].freeze
   CONTENT_TYPES = %w[plain_text html body_markdown].freeze
@@ -14,7 +13,7 @@ class ResponseTemplate < ApplicationRecord
   USER_NIL_TYPE_OF_MSG = "cannot have a user ID associated.".freeze
 
   validates :type_of, :content_type, :content, :title, presence: true
-  validates :content, uniqueness: { scope: UNIQUENESS_SCOPE }
+  validates :content, uniqueness: { scope: %i[user_id type_of content_type] }
   validates :type_of, inclusion: { in: TYPE_OF_TYPES }
   validates :content_type, inclusion: { in: CONTENT_TYPES }
   validates :content_type,

db/migrate/20200526144234_add_unique_index_to_organizations_secret.rb

@@ -0,0 +1,7 @@
+class AddUniqueIndexToOrganizationsSecret < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    add_index :organizations, :secret, unique: true, algorithm: :concurrently
+  end
+end

db/migrate/20200526145731_add_unique_index_to_chat_channel_memberships_chat_channel_id.rb

@@ -0,0 +1,7 @@
+class AddUniqueIndexToChatChannelMembershipsChatChannelId < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    add_index :chat_channel_memberships, %i[chat_channel_id user_id], unique: true, algorithm: :concurrently
+  end
+end

db/migrate/20200526151431_add_unique_index_to_chat_channels_slug.rb

@@ -0,0 +1,7 @@
+class AddUniqueIndexToChatChannelsSlug < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    add_index :chat_channels, :slug, unique: true, algorithm: :concurrently
+  end
+end

db/migrate/20200526151807_add_unique_index_to_badge_achievements_badge_id.rb

@@ -0,0 +1,7 @@
+class AddUniqueIndexToBadgeAchievementsBadgeId < ActiveRecord::Migration[5.2]
+  disable_ddl_transaction!
+
+  def change
+    add_index :badge_achievements, %i[badge_id user_id], unique: true, algorithm: :concurrently
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_05_25_125611) do
+ActiveRecord::Schema.define(version: 2020_05_26_151807) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -163,6 +163,7 @@ ActiveRecord::Schema.define(version: 2020_05_25_125611) do
     t.text "rewarding_context_message_markdown"
     t.datetime "updated_at", null: false
     t.bigint "user_id", null: false
+    t.index ["badge_id", "user_id"], name: "index_badge_achievements_on_badge_id_and_user_id", unique: true
     t.index ["badge_id"], name: "index_badge_achievements_on_badge_id"
     t.index ["user_id", "badge_id"], name: "index_badge_achievements_on_user_id_and_badge_id"
     t.index ["user_id"], name: "index_badge_achievements_on_user_id"
@@ -297,6 +298,7 @@ ActiveRecord::Schema.define(version: 2020_05_25_125611) do
     t.string "status", default: "active"
     t.datetime "updated_at", null: false
     t.bigint "user_id", null: false
+    t.index ["chat_channel_id", "user_id"], name: "index_chat_channel_memberships_on_chat_channel_id_and_user_id", unique: true
     t.index ["chat_channel_id"], name: "index_chat_channel_memberships_on_chat_channel_id"
     t.index ["user_id", "chat_channel_id"], name: "index_chat_channel_memberships_on_user_id_and_chat_channel_id"
     t.index ["user_id"], name: "index_chat_channel_memberships_on_user_id"
@@ -312,6 +314,7 @@ ActiveRecord::Schema.define(version: 2020_05_25_125611) do
     t.string "slug"
     t.string "status", default: "active"
     t.datetime "updated_at", null: false
+    t.index ["slug"], name: "index_chat_channels_on_slug", unique: true
   end
 
   create_table "classified_listing_categories", force: :cascade do |t|
@@ -765,6 +768,7 @@ ActiveRecord::Schema.define(version: 2020_05_25_125611) do
     t.integer "unspent_credits_count", default: 0, null: false
     t.datetime "updated_at", null: false
     t.string "url"
+    t.index ["secret"], name: "index_organizations_on_secret", unique: true
     t.index ["slug"], name: "index_organizations_on_slug", unique: true
   end
 

spec/models/badge_achievement_spec.rb

@@ -4,10 +4,14 @@ RSpec.describe BadgeAchievement, type: :model do
   let_it_be(:achievement) { create(:badge_achievement) }
 
   describe "validations" do
-    it { is_expected.to belong_to(:user) }
-    it { is_expected.to belong_to(:badge) }
-    it { is_expected.to belong_to(:rewarder).class_name("User").optional }
-    it { is_expected.to validate_uniqueness_of(:badge_id).scoped_to(:user_id) }
+    describe "builtin validations" do
+      subject { achievement }
+
+      it { is_expected.to belong_to(:user) }
+      it { is_expected.to belong_to(:badge) }
+      it { is_expected.to belong_to(:rewarder).class_name("User").optional }
+      it { is_expected.to validate_uniqueness_of(:badge_id).scoped_to(:user_id) }
+    end
   end
 
   it "turns rewarding_context_message_markdown into rewarding_context_message HTML" do

spec/models/chat_channel_membership_spec.rb

@@ -1,7 +1,30 @@
 require "rails_helper"
 
 RSpec.describe ChatChannelMembership, type: :model do
-  let(:chat_channel_membership) { FactoryBot.create(:chat_channel_membership) }
+  let(:chat_channel) { create(:chat_channel) }
+  let(:chat_channel_membership) { create(:chat_channel_membership, chat_channel: chat_channel) }
+
+  describe "validations" do
+    describe "builtin validations" do
+      subject { chat_channel_membership }
+
+      it { is_expected.to belong_to(:chat_channel) }
+      it { is_expected.to belong_to(:user) }
+
+      it { is_expected.to validate_inclusion_of(:role).in_array(%w[member mod]) }
+
+      # rubocop:disable RSpec/NamedSubject
+      it {
+        expect(subject).to validate_inclusion_of(:status).
+          in_array(%w[active inactive pending rejected left_channel removed_from_channel joining_request])
+      }
+      # rubocop:enable RSpec/NamedSubject
+
+      it { is_expected.to validate_presence_of(:chat_channel_id) }
+      it { is_expected.to validate_presence_of(:user_id) }
+      it { is_expected.to validate_uniqueness_of(:chat_channel_id).scoped_to(:user_id) }
+    end
+  end
 
   describe "#channel_text" do
     it "sets channel text using name, slug, and human names" do

spec/models/chat_channel_spec.rb

@@ -5,10 +5,22 @@ RSpec.describe ChatChannel, type: :model do
 
   let_it_be(:users) { create_list(:user, 2) }
 
-  it { is_expected.to have_many(:messages).dependent(:destroy) }
-  it { is_expected.to have_many(:chat_channel_memberships).dependent(:destroy) }
-  it { is_expected.to have_many(:users) }
-  it { is_expected.to validate_presence_of(:channel_type) }
+  describe "validations" do
+    describe "builtin validations" do
+      subject { chat_channel }
+
+      it { is_expected.to have_many(:messages).dependent(:destroy) }
+      it { is_expected.to have_many(:chat_channel_memberships).dependent(:destroy) }
+      it { is_expected.to have_many(:users).through(:chat_channel_memberships) }
+
+      it { is_expected.to validate_inclusion_of(:channel_type).in_array(%w[open invite_only direct]) }
+      it { is_expected.to validate_inclusion_of(:status).in_array(%w[active inactive blocked]) }
+      it { is_expected.to validate_length_of(:description).is_at_most(200) }
+      it { is_expected.to validate_presence_of(:channel_type) }
+      it { is_expected.to validate_presence_of(:status) }
+      it { is_expected.to validate_uniqueness_of(:slug) }
+    end
+  end
 
   describe "#clear_channel" do
     before { allow(Pusher).to receive(:trigger) }

spec/models/organization_spec.rb

@@ -40,6 +40,7 @@ RSpec.describe Organization, type: :model do
       it { is_expected.to validate_length_of(:company_size).is_at_most(7) }
       it { is_expected.to validate_length_of(:story).is_at_most(640) }
       it { is_expected.to validate_length_of(:tech_stack).is_at_most(640) }
+      it { is_expected.to validate_uniqueness_of(:secret).allow_nil }
       it { is_expected.to validate_uniqueness_of(:slug).case_insensitive }
 
       it { is_expected.not_to allow_value("#xyz").for(:bg_color_hex) }
@@ -53,7 +54,21 @@ RSpec.describe Organization, type: :model do
     end
   end
 
-  describe "#after_commit" do
+  context "when callbacks are triggered before save" do
+    it "generates a secret if set to empty string" do
+      organization.secret = ""
+      organization.save
+      expect(organization.reload.secret).not_to eq("")
+    end
+
+    it "generates a secret if set to nil" do
+      organization.secret = nil
+      organization.save
+      expect(organization.reload.secret).not_to be(nil)
+    end
+  end
+
+  context "when callbacks are triggered after commit" do
     it "on update syncs elasticsearch data" do
       article = create(:article, organization: organization)
       sidekiq_perform_enqueued_jobs
@@ -269,10 +284,4 @@ RSpec.describe Organization, type: :model do
       expect(organization.enough_credits?(1)).to be(true)
     end
   end
-
-  describe "#decoarated" do
-    it "returns not fully banished" do
-      expect(organization.decorate.fully_banished?).to eq(false)
-    end
-  end
 end