From 00c6ebd86f9a084394a7e1017c9e808e4c4b4d77 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 20 Nov 2023 04:07:25 -0500 Subject: [PATCH] Reduce `.times` usage in `StatusPin` and add `PIN_LIMIT` constant in validator (#27945) --- app/validators/status_pin_validator.rb | 4 ++- spec/models/status_pin_spec.rb | 49 +++++++++++++------------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/app/validators/status_pin_validator.rb b/app/validators/status_pin_validator.rb index 2fdd5b34fd..c9c1effba8 100644 --- a/app/validators/status_pin_validator.rb +++ b/app/validators/status_pin_validator.rb @@ -1,10 +1,12 @@ # frozen_string_literal: true class StatusPinValidator < ActiveModel::Validator + PIN_LIMIT = 5 + def validate(pin) pin.errors.add(:base, I18n.t('statuses.pin_errors.reblog')) if pin.status.reblog? pin.errors.add(:base, I18n.t('statuses.pin_errors.ownership')) if pin.account_id != pin.status.account_id pin.errors.add(:base, I18n.t('statuses.pin_errors.direct')) if pin.status.direct_visibility? - pin.errors.add(:base, I18n.t('statuses.pin_errors.limit')) if pin.account.status_pins.count > 4 && pin.account.local? + pin.errors.add(:base, I18n.t('statuses.pin_errors.limit')) if pin.account.status_pins.count >= PIN_LIMIT && pin.account.local? end end diff --git a/spec/models/status_pin_spec.rb b/spec/models/status_pin_spec.rb index 660b2e92ac..da375009ae 100644 --- a/spec/models/status_pin_spec.rb +++ b/spec/models/status_pin_spec.rb @@ -40,35 +40,34 @@ RSpec.describe StatusPin do expect(described_class.new(account: account, status: status).save).to be false end - max_pins = 5 - it 'does not allow pins above the max' do - account = Fabricate(:account) - status = [] + context 'with a pin limit' do + before { stub_const('StatusPinValidator::PIN_LIMIT', 2) } - (max_pins + 1).times do |i| - status[i] = Fabricate(:status, account: account) + it 'does not allow pins above the max' do + account = Fabricate(:account) + + Fabricate.times(StatusPinValidator::PIN_LIMIT, :status_pin, account: account) + + pin = described_class.new(account: account, status: Fabricate(:status, account: account)) + expect(pin.save) + .to be(false) + + expect(pin.errors[:base]) + .to contain_exactly(I18n.t('statuses.pin_errors.limit')) end - max_pins.times do |i| - expect(described_class.new(account: account, status: status[i]).save).to be true + it 'allows pins above the max for remote accounts' do + account = Fabricate(:account, domain: 'remote.test', username: 'bob', url: 'https://remote.test/') + + Fabricate.times(StatusPinValidator::PIN_LIMIT, :status_pin, account: account) + + pin = described_class.new(account: account, status: Fabricate(:status, account: account)) + expect(pin.save) + .to be(true) + + expect(pin.errors[:base]) + .to be_empty end - - expect(described_class.new(account: account, status: status[max_pins]).save).to be false - end - - it 'allows pins above the max for remote accounts' do - account = Fabricate(:account, domain: 'remote.test', username: 'bob', url: 'https://remote.test/') - status = [] - - (max_pins + 1).times do |i| - status[i] = Fabricate(:status, account: account) - end - - max_pins.times do |i| - expect(described_class.new(account: account, status: status[i]).save).to be true - end - - expect(described_class.new(account: account, status: status[max_pins]).save).to be true end end end