From 2b148d3e88e295c8cc5b4a04cc5fa4a689d7cada Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Tue, 28 Jan 2025 15:38:18 +0100
Subject: [PATCH] Fix polls not being validated on edition (#33755)
---
app/models/poll.rb | 3 +-
app/serializers/rest/instance_serializer.rb | 8 ++--
.../rest/v1/instance_serializer.rb | 8 ++--
app/validators/poll_expiration_validator.rb | 13 ++++++
...validator.rb => poll_options_validator.rb} | 8 +---
spec/requests/api/v2/instance_spec.rb | 2 +-
...c.rb => poll_expiration_validator_spec.rb} | 16 +++++--
.../validators/poll_options_validator_spec.rb | 45 +++++++++++++++++++
8 files changed, 82 insertions(+), 21 deletions(-)
create mode 100644 app/validators/poll_expiration_validator.rb
rename app/validators/{poll_validator.rb => poll_options_validator.rb} (57%)
rename spec/validators/{poll_validator_spec.rb => poll_expiration_validator_spec.rb} (64%)
create mode 100644 spec/validators/poll_options_validator_spec.rb
diff --git a/app/models/poll.rb b/app/models/poll.rb
index cc4184f80..baa0dbe53 100644
--- a/app/models/poll.rb
+++ b/app/models/poll.rb
@@ -37,7 +37,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) }
diff --git a/app/serializers/rest/instance_serializer.rb b/app/serializers/rest/instance_serializer.rb
index 19361277a..4f662e566 100644
--- a/app/serializers/rest/instance_serializer.rb
+++ b/app/serializers/rest/instance_serializer.rb
@@ -86,10 +86,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: {
diff --git a/app/serializers/rest/v1/instance_serializer.rb b/app/serializers/rest/v1/instance_serializer.rb
index 7f9f21c5a..6ddf7888b 100644
--- a/app/serializers/rest/v1/instance_serializer.rb
+++ b/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
diff --git a/app/validators/poll_expiration_validator.rb b/app/validators/poll_expiration_validator.rb
new file mode 100644
index 000000000..ea8b08e18
--- /dev/null
+++ b/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
diff --git a/app/validators/poll_validator.rb b/app/validators/poll_options_validator.rb
similarity index 57%
rename from app/validators/poll_validator.rb
rename to app/validators/poll_options_validator.rb
index a32727796..0ac84f93f 100644
--- a/app/validators/poll_validator.rb
+++ b/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
diff --git a/spec/requests/api/v2/instance_spec.rb b/spec/requests/api/v2/instance_spec.rb
index fae92b739..f6a670cc2 100644
--- a/spec/requests/api/v2/instance_spec.rb
+++ b/spec/requests/api/v2/instance_spec.rb
@@ -56,7 +56,7 @@ RSpec.describe 'Instances' do
max_media_attachments: Status::MEDIA_ATTACHMENTS_LIMIT
),
polls: include(
- max_options: PollValidator::MAX_OPTIONS
+ max_options: PollOptionsValidator::MAX_OPTIONS
)
)
)
diff --git a/spec/validators/poll_validator_spec.rb b/spec/validators/poll_expiration_validator_spec.rb
similarity index 64%
rename from spec/validators/poll_validator_spec.rb
rename to spec/validators/poll_expiration_validator_spec.rb
index f2a253489..41b8c9621 100644
--- a/spec/validators/poll_validator_spec.rb
+++ b/spec/validators/poll_expiration_validator_spec.rb
@@ -2,7 +2,7 @@
require 'rails_helper'
-RSpec.describe PollValidator do
+RSpec.describe PollExpirationValidator do
describe '#validate' do
before do
validator.validate(poll)
@@ -14,16 +14,24 @@ RSpec.describe PollValidator do
let(:options) { %w(foo bar) }
let(:expires_at) { 1.day.from_now }
- it 'have no errors' do
+ it 'has no errors' do
expect(errors).to_not have_received(:add)
end
- context 'when expires is 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
+ 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
end
diff --git a/spec/validators/poll_options_validator_spec.rb b/spec/validators/poll_options_validator_spec.rb
new file mode 100644
index 000000000..9e4ec744d
--- /dev/null
+++ b/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