From ae2198bd955530c61dd1f4cd99f23c7a0c069b6e Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 25 Jan 2020 16:00:29 +0100 Subject: [PATCH] Fix validations of reactions limit (#12955) --- app/validators/reaction_validator.rb | 10 ++++-- spec/validators/reaction_validator_spec.rb | 42 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 spec/validators/reaction_validator_spec.rb diff --git a/app/validators/reaction_validator.rb b/app/validators/reaction_validator.rb index be899c89d..494b6041b 100644 --- a/app/validators/reaction_validator.rb +++ b/app/validators/reaction_validator.rb @@ -6,10 +6,10 @@ class ReactionValidator < ActiveModel::Validator LIMIT = 8 def validate(reaction) - return if reaction.name.blank? || reaction.custom_emoji_id.present? + return if reaction.name.blank? - reaction.errors.add(:name, I18n.t('reactions.errors.unrecognized_emoji')) unless unicode_emoji?(reaction.name) - reaction.errors.add(:base, I18n.t('reactions.errors.limit_reached')) if limit_reached?(reaction) + reaction.errors.add(:name, I18n.t('reactions.errors.unrecognized_emoji')) if reaction.custom_emoji_id.blank? && !unicode_emoji?(reaction.name) + reaction.errors.add(:base, I18n.t('reactions.errors.limit_reached')) if new_reaction?(reaction) && limit_reached?(reaction) end private @@ -18,6 +18,10 @@ class ReactionValidator < ActiveModel::Validator SUPPORTED_EMOJIS.include?(name) end + def new_reaction?(reaction) + !reaction.announcement.announcement_reactions.where(name: reaction.name).exists? + end + def limit_reached?(reaction) reaction.announcement.announcement_reactions.where.not(name: reaction.name).count('distinct name') >= LIMIT end diff --git a/spec/validators/reaction_validator_spec.rb b/spec/validators/reaction_validator_spec.rb new file mode 100644 index 000000000..d73104cb6 --- /dev/null +++ b/spec/validators/reaction_validator_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ReactionValidator do + let(:announcement) { Fabricate(:announcement) } + + describe '#validate' do + it 'adds error when not a valid unicode emoji' do + reaction = announcement.announcement_reactions.build(name: 'F') + subject.validate(reaction) + expect(reaction.errors).to_not be_empty + end + + it 'does not add error when non-unicode emoji is a custom emoji' do + custom_emoji = Fabricate(:custom_emoji) + reaction = announcement.announcement_reactions.build(name: custom_emoji.shortcode, custom_emoji_id: custom_emoji.id) + subject.validate(reaction) + expect(reaction.errors).to be_empty + end + + it 'adds error when 8 reactions already exist' do + %w(🐘 ❤️ 🙉 😍 😋 😂 😞 👍).each do |name| + announcement.announcement_reactions.create!(name: name, account: Fabricate(:account)) + end + + reaction = announcement.announcement_reactions.build(name: '😘') + subject.validate(reaction) + expect(reaction.errors).to_not be_empty + end + + it 'does not add error when new reaction is part of the existing ones' do + %w(🐘 ❤️ 🙉 😍 😋 😂 😞 👍).each do |name| + announcement.announcement_reactions.create!(name: name, account: Fabricate(:account)) + end + + reaction = announcement.announcement_reactions.build(name: '😋') + subject.validate(reaction) + expect(reaction.errors).to be_empty + end + end +end