Fix polls not being validated on edition (#33755)

app/models/poll.rb

@@ -32,7 +32,8 @@ class Poll < ApplicationRecord
 
   validates :options, presence: true
   validates :expires_at, presence: true, if: :local?
-  validates_with PollValidator, on: :create, if: :local?
+  validates_with PollOptionsValidator, if: :local?
+  validates_with PollExpirationValidator, if: -> { local? && expires_at_changed? }
 
   scope :attached, -> { where.not(status_id: nil) }
   scope :unattached, -> { where(status_id: nil) }

app/serializers/rest/instance_serializer.rb

@@ -68,10 +68,10 @@ class REST::InstanceSerializer < ActiveModel::Serializer
       },
 
       polls: {
-        max_options: PollValidator::MAX_OPTIONS,
-        max_characters_per_option: PollValidator::MAX_OPTION_CHARS,
-        min_expiration: PollValidator::MIN_EXPIRATION,
-        max_expiration: PollValidator::MAX_EXPIRATION,
+        max_options: PollOptionsValidator::MAX_OPTIONS,
+        max_characters_per_option: PollOptionsValidator::MAX_OPTION_CHARS,
+        min_expiration: PollExpirationValidator::MIN_EXPIRATION,
+        max_expiration: PollExpirationValidator::MAX_EXPIRATION,
       },
 
       translation: {

app/serializers/rest/v1/instance_serializer.rb

@@ -78,10 +78,10 @@ class REST::V1::InstanceSerializer < ActiveModel::Serializer
       },
 
       polls: {
-        max_options: PollValidator::MAX_OPTIONS,
-        max_characters_per_option: PollValidator::MAX_OPTION_CHARS,
-        min_expiration: PollValidator::MIN_EXPIRATION,
-        max_expiration: PollValidator::MAX_EXPIRATION,
+        max_options: PollOptionsValidator::MAX_OPTIONS,
+        max_characters_per_option: PollOptionsValidator::MAX_OPTION_CHARS,
+        min_expiration: PollExpirationValidator::MIN_EXPIRATION,
+        max_expiration: PollExpirationValidator::MAX_EXPIRATION,
       },
     }
   end

app/validators/poll_expiration_validator.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class PollExpirationValidator < ActiveModel::Validator
+  MAX_EXPIRATION = 1.month.freeze
+  MIN_EXPIRATION = 5.minutes.freeze
+
+  def validate(poll)
+    current_time = Time.now.utc
+
+    poll.errors.add(:expires_at, I18n.t('polls.errors.duration_too_long')) if poll.expires_at.nil? || poll.expires_at - current_time > MAX_EXPIRATION
+    poll.errors.add(:expires_at, I18n.t('polls.errors.duration_too_short')) if poll.expires_at.present? && (poll.expires_at - current_time).ceil < MIN_EXPIRATION
+  end
+end

app/validators/poll_options_validator.rb

@@ -1,19 +1,13 @@
 # frozen_string_literal: true
 
-class PollValidator < ActiveModel::Validator
+class PollOptionsValidator < ActiveModel::Validator
   MAX_OPTIONS      = 4
   MAX_OPTION_CHARS = 50
-  MAX_EXPIRATION   = 1.month.freeze
-  MIN_EXPIRATION   = 5.minutes.freeze
 
   def validate(poll)
-    current_time = Time.now.utc
-
     poll.errors.add(:options, I18n.t('polls.errors.too_few_options')) unless poll.options.size > 1
     poll.errors.add(:options, I18n.t('polls.errors.too_many_options', max: MAX_OPTIONS)) if poll.options.size > MAX_OPTIONS
     poll.errors.add(:options, I18n.t('polls.errors.over_character_limit', max: MAX_OPTION_CHARS)) if poll.options.any? { |option| option.mb_chars.grapheme_length > MAX_OPTION_CHARS }
     poll.errors.add(:options, I18n.t('polls.errors.duplicate_options')) unless poll.options.uniq.size == poll.options.size
-    poll.errors.add(:expires_at, I18n.t('polls.errors.duration_too_long')) if poll.expires_at.nil? || poll.expires_at - current_time > MAX_EXPIRATION
-    poll.errors.add(:expires_at, I18n.t('polls.errors.duration_too_short')) if poll.expires_at.present? && (poll.expires_at - current_time).ceil < MIN_EXPIRATION
   end
 end

spec/validators/poll_expiration_validator_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe PollValidator, type: :validator do
+RSpec.describe PollExpirationValidator, type: :validator do
   describe '#validate' do
     before do
       validator.validate(poll)
@@ -14,14 +14,23 @@ RSpec.describe PollValidator, type: :validator do
     let(:options) { %w(foo bar) }
     let(:expires_at) { 1.day.from_now }
 
-    it 'have no errors' do
-      expect(errors).not_to have_received(:add)
+    it 'has no errors' do
+      expect(errors).to_not have_received(:add)
     end
 
-    context 'expires just 5 min ago' do
+    context 'when the poll expires in 5 min from now' do
       let(:expires_at) { 5.minutes.from_now }
-      it 'not calls errors add' do
-        expect(errors).not_to have_received(:add)
+
+      it 'has no errors' do
+        expect(errors).to_not have_received(:add)
+      end
+    end
+
+    context 'when the poll expires in the past' do
+      let(:expires_at) { 5.minutes.ago }
+
+      it 'has errors' do
+        expect(errors).to have_received(:add)
       end
     end
   end

spec/validators/poll_options_validator_spec.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe PollOptionsValidator do
+  describe '#validate' do
+    before do
+      validator.validate(poll)
+    end
+
+    let(:validator) { described_class.new }
+    let(:poll) { instance_double(Poll, options: options, expires_at: expires_at, errors: errors) }
+    let(:errors) { instance_double(ActiveModel::Errors, add: nil) }
+    let(:options) { %w(foo bar) }
+    let(:expires_at) { 1.day.from_now }
+
+    it 'has no errors' do
+      expect(errors).to_not have_received(:add)
+    end
+
+    context 'when the poll has duplicate options' do
+      let(:options) { %w(foo foo) }
+
+      it 'adds errors' do
+        expect(errors).to have_received(:add)
+      end
+    end
+
+    context 'when the poll has no options' do
+      let(:options) { [] }
+
+      it 'adds errors' do
+        expect(errors).to have_received(:add)
+      end
+    end
+
+    context 'when the poll has too many options' do
+      let(:options) { Array.new(described_class::MAX_OPTIONS + 1) { |i| "option #{i}" } }
+
+      it 'adds errors' do
+        expect(errors).to have_received(:add)
+      end
+    end
+  end
+end