diff --git a/app/models/chat_channel.rb b/app/models/chat_channel.rb index 04ab2b0d6..07f572b69 100644 --- a/app/models/chat_channel.rb +++ b/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 diff --git a/app/models/chat_channel_membership.rb b/app/models/chat_channel_membership.rb index 5c7a6ae53..4dc45cc34 100644 --- a/app/models/chat_channel_membership.rb +++ b/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") diff --git a/app/models/organization.rb b/app/models/organization.rb index 0994d4fe7..93782f7df 100644 --- a/app/models/organization.rb +++ b/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 } diff --git a/app/models/response_template.rb b/app/models/response_template.rb index f4663a468..4c3b29f5f 100644 --- a/app/models/response_template.rb +++ b/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, diff --git a/db/migrate/20200526144234_add_unique_index_to_organizations_secret.rb b/db/migrate/20200526144234_add_unique_index_to_organizations_secret.rb new file mode 100644 index 000000000..3fcef1200 --- /dev/null +++ b/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 diff --git a/db/migrate/20200526145731_add_unique_index_to_chat_channel_memberships_chat_channel_id.rb b/db/migrate/20200526145731_add_unique_index_to_chat_channel_memberships_chat_channel_id.rb new file mode 100644 index 000000000..c78dd5983 --- /dev/null +++ b/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 diff --git a/db/migrate/20200526151431_add_unique_index_to_chat_channels_slug.rb b/db/migrate/20200526151431_add_unique_index_to_chat_channels_slug.rb new file mode 100644 index 000000000..a1439d0c3 --- /dev/null +++ b/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 diff --git a/db/migrate/20200526151807_add_unique_index_to_badge_achievements_badge_id.rb b/db/migrate/20200526151807_add_unique_index_to_badge_achievements_badge_id.rb new file mode 100644 index 000000000..8927c14ff --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 6cf6d9598..34ddbb435 100644 --- a/db/schema.rb +++ b/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 diff --git a/spec/models/badge_achievement_spec.rb b/spec/models/badge_achievement_spec.rb index 4bf325899..1b9733a4b 100644 --- a/spec/models/badge_achievement_spec.rb +++ b/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 diff --git a/spec/models/chat_channel_membership_spec.rb b/spec/models/chat_channel_membership_spec.rb index 9b29f3fa0..ab0ea51a9 100644 --- a/spec/models/chat_channel_membership_spec.rb +++ b/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 diff --git a/spec/models/chat_channel_spec.rb b/spec/models/chat_channel_spec.rb index 3a62d2aa0..010bdbf60 100644 --- a/spec/models/chat_channel_spec.rb +++ b/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) } diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index 9e0b175c5..475ef4860 100644 --- a/spec/models/organization_spec.rb +++ b/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