From 5acec087caed4a2fdf0fd8ed11f891222496f321 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Thu, 5 Sep 2024 11:36:05 -0400
Subject: [PATCH] Simplify basic presence validations (#29664)
---
spec/models/announcement_spec.rb | 9 +-----
spec/models/block_spec.rb | 13 ++-------
spec/models/custom_emoji_category_spec.rb | 7 +----
spec/models/custom_filter_spec.rb | 15 ++--------
spec/models/domain_allow_spec.rb | 6 +---
spec/models/domain_block_spec.rb | 6 +---
spec/models/follow_spec.rb | 13 ++-------
spec/models/import_spec.rb | 17 ++---------
spec/models/ip_block_spec.rb | 15 ++--------
spec/models/marker_spec.rb | 11 ++-----
spec/models/media_attachment_spec.rb | 7 +----
spec/models/mention_spec.rb | 13 ++-------
spec/models/poll_spec.rb | 7 ++---
spec/models/user_spec.rb | 6 +---
spec/models/webauthn_credential_spec.rb | 35 +++--------------------
spec/models/webhook_spec.rb | 7 +----
16 files changed, 28 insertions(+), 159 deletions(-)
diff --git a/spec/models/announcement_spec.rb b/spec/models/announcement_spec.rb
index e3865e6fc..8bd1e74b2 100644
--- a/spec/models/announcement_spec.rb
+++ b/spec/models/announcement_spec.rb
@@ -64,14 +64,7 @@ RSpec.describe Announcement do
end
describe 'Validations' do
- describe 'text' do
- it 'validates presence of attribute' do
- record = Fabricate.build(:announcement, text: nil)
-
- expect(record).to_not be_valid
- expect(record.errors[:text]).to be_present
- end
- end
+ it { is_expected.to validate_presence_of(:text) }
describe 'ends_at' do
it 'validates presence when starts_at is present' do
diff --git a/spec/models/block_spec.rb b/spec/models/block_spec.rb
index 8249503c5..84f0f318f 100644
--- a/spec/models/block_spec.rb
+++ b/spec/models/block_spec.rb
@@ -4,17 +4,8 @@ require 'rails_helper'
RSpec.describe Block do
describe 'validations' do
- it 'is invalid without an account' do
- block = Fabricate.build(:block, account: nil)
- block.valid?
- expect(block).to model_have_error_on_field(:account)
- end
-
- it 'is invalid without a target_account' do
- block = Fabricate.build(:block, target_account: nil)
- block.valid?
- expect(block).to model_have_error_on_field(:target_account)
- end
+ it { is_expected.to belong_to(:account).required }
+ it { is_expected.to belong_to(:target_account).required }
end
it 'removes blocking cache after creation' do
diff --git a/spec/models/custom_emoji_category_spec.rb b/spec/models/custom_emoji_category_spec.rb
index 3da77344e..2b414a66e 100644
--- a/spec/models/custom_emoji_category_spec.rb
+++ b/spec/models/custom_emoji_category_spec.rb
@@ -4,11 +4,6 @@ require 'rails_helper'
RSpec.describe CustomEmojiCategory do
describe 'validations' do
- it 'validates name presence' do
- record = described_class.new(name: nil)
-
- expect(record).to_not be_valid
- expect(record).to model_have_error_on_field(:name)
- end
+ it { is_expected.to validate_presence_of(:name) }
end
end
diff --git a/spec/models/custom_filter_spec.rb b/spec/models/custom_filter_spec.rb
index 5bb615bb3..afbc42024 100644
--- a/spec/models/custom_filter_spec.rb
+++ b/spec/models/custom_filter_spec.rb
@@ -4,19 +4,8 @@ require 'rails_helper'
RSpec.describe CustomFilter do
describe 'Validations' do
- it 'requires presence of title' do
- record = described_class.new(title: '')
- record.valid?
-
- expect(record).to model_have_error_on_field(:title)
- end
-
- it 'requires presence of context' do
- record = described_class.new(context: nil)
- record.valid?
-
- expect(record).to model_have_error_on_field(:context)
- end
+ it { is_expected.to validate_presence_of(:title) }
+ it { is_expected.to validate_presence_of(:context) }
it 'requires non-empty of context' do
record = described_class.new(context: [])
diff --git a/spec/models/domain_allow_spec.rb b/spec/models/domain_allow_spec.rb
index 92f1ef8cc..d8f438f07 100644
--- a/spec/models/domain_allow_spec.rb
+++ b/spec/models/domain_allow_spec.rb
@@ -4,11 +4,7 @@ require 'rails_helper'
RSpec.describe DomainAllow do
describe 'Validations' do
- it 'is invalid without a domain' do
- domain_allow = Fabricate.build(:domain_allow, domain: nil)
- domain_allow.valid?
- expect(domain_allow).to model_have_error_on_field(:domain)
- end
+ it { is_expected.to validate_presence_of(:domain) }
it 'is invalid if the same normalized domain already exists' do
_domain_allow = Fabricate(:domain_allow, domain: 'にゃん')
diff --git a/spec/models/domain_block_spec.rb b/spec/models/domain_block_spec.rb
index d595441fd..8278454cd 100644
--- a/spec/models/domain_block_spec.rb
+++ b/spec/models/domain_block_spec.rb
@@ -4,11 +4,7 @@ require 'rails_helper'
RSpec.describe DomainBlock do
describe 'validations' do
- it 'is invalid without a domain' do
- domain_block = Fabricate.build(:domain_block, domain: nil)
- domain_block.valid?
- expect(domain_block).to model_have_error_on_field(:domain)
- end
+ it { is_expected.to validate_presence_of(:domain) }
it 'is invalid if the same normalized domain already exists' do
_domain_block = Fabricate(:domain_block, domain: 'にゃん')
diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb
index 9aa172b2f..f22bd6ea8 100644
--- a/spec/models/follow_spec.rb
+++ b/spec/models/follow_spec.rb
@@ -9,17 +9,8 @@ RSpec.describe Follow do
describe 'validations' do
subject { described_class.new(account: alice, target_account: bob, rate_limit: true) }
- it 'is invalid without an account' do
- follow = Fabricate.build(:follow, account: nil)
- follow.valid?
- expect(follow).to model_have_error_on_field(:account)
- end
-
- it 'is invalid without a target_account' do
- follow = Fabricate.build(:follow, target_account: nil)
- follow.valid?
- expect(follow).to model_have_error_on_field(:target_account)
- end
+ it { is_expected.to belong_to(:account).required }
+ it { is_expected.to belong_to(:target_account).required }
it 'is invalid if account already follows too many people' do
alice.update(following_count: FollowLimitValidator::LIMIT)
diff --git a/spec/models/import_spec.rb b/spec/models/import_spec.rb
index 10df5f8c0..587e0a9d2 100644
--- a/spec/models/import_spec.rb
+++ b/spec/models/import_spec.rb
@@ -3,19 +3,8 @@
require 'rails_helper'
RSpec.describe Import do
- let(:account) { Fabricate(:account) }
- let(:type) { 'following' }
- let(:data) { attachment_fixture('imports.txt') }
-
- describe 'validations' do
- it 'is invalid without an type' do
- import = described_class.create(account: account, data: data)
- expect(import).to model_have_error_on_field(:type)
- end
-
- it 'is invalid without a data' do
- import = described_class.create(account: account, type: type)
- expect(import).to model_have_error_on_field(:data)
- end
+ describe 'Validations' do
+ it { is_expected.to validate_presence_of(:type) }
+ it { is_expected.to validate_presence_of(:data) }
end
end
diff --git a/spec/models/ip_block_spec.rb b/spec/models/ip_block_spec.rb
index 6f1eb3842..ac9f5db60 100644
--- a/spec/models/ip_block_spec.rb
+++ b/spec/models/ip_block_spec.rb
@@ -4,19 +4,8 @@ require 'rails_helper'
RSpec.describe IpBlock do
describe 'validations' do
- it 'validates ip presence', :aggregate_failures do
- ip_block = described_class.new(ip: nil, severity: :no_access)
-
- expect(ip_block).to_not be_valid
- expect(ip_block).to model_have_error_on_field(:ip)
- end
-
- it 'validates severity presence', :aggregate_failures do
- ip_block = described_class.new(ip: '127.0.0.1', severity: nil)
-
- expect(ip_block).to_not be_valid
- expect(ip_block).to model_have_error_on_field(:severity)
- end
+ it { is_expected.to validate_presence_of(:ip) }
+ it { is_expected.to validate_presence_of(:severity) }
it 'validates ip uniqueness', :aggregate_failures do
described_class.create!(ip: '127.0.0.1', severity: :no_access)
diff --git a/spec/models/marker_spec.rb b/spec/models/marker_spec.rb
index 8339f8e25..1da8eb6f4 100644
--- a/spec/models/marker_spec.rb
+++ b/spec/models/marker_spec.rb
@@ -3,14 +3,7 @@
require 'rails_helper'
RSpec.describe Marker do
- describe 'validations' do
- describe 'timeline' do
- it 'must be included in valid list' do
- record = described_class.new(timeline: 'not real timeline')
-
- expect(record).to_not be_valid
- expect(record).to model_have_error_on_field(:timeline)
- end
- end
+ describe 'Validations' do
+ it { is_expected.to validate_inclusion_of(:timeline).in_array(described_class::TIMELINES) }
end
end
diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb
index 50f8d00a5..5f91ae096 100644
--- a/spec/models/media_attachment_spec.rb
+++ b/spec/models/media_attachment_spec.rb
@@ -257,12 +257,7 @@ RSpec.describe MediaAttachment, :attachment_processing do
end
end
- it 'is invalid without file' do
- media = described_class.new
-
- expect(media.valid?).to be false
- expect(media).to model_have_error_on_field(:file)
- end
+ it { is_expected.to validate_presence_of(:file) }
describe 'size limit validation' do
it 'rejects video files that are too large' do
diff --git a/spec/models/mention_spec.rb b/spec/models/mention_spec.rb
index b241049a5..3a9b9fddf 100644
--- a/spec/models/mention_spec.rb
+++ b/spec/models/mention_spec.rb
@@ -4,16 +4,7 @@ require 'rails_helper'
RSpec.describe Mention do
describe 'validations' do
- it 'is invalid without an account' do
- mention = Fabricate.build(:mention, account: nil)
- mention.valid?
- expect(mention).to model_have_error_on_field(:account)
- end
-
- it 'is invalid without a status' do
- mention = Fabricate.build(:mention, status: nil)
- mention.valid?
- expect(mention).to model_have_error_on_field(:status)
- end
+ it { is_expected.to belong_to(:account).required }
+ it { is_expected.to belong_to(:status).required }
end
end
diff --git a/spec/models/poll_spec.rb b/spec/models/poll_spec.rb
index 740ef63d8..736f3615d 100644
--- a/spec/models/poll_spec.rb
+++ b/spec/models/poll_spec.rb
@@ -32,12 +32,9 @@ RSpec.describe Poll do
describe 'validations' do
context 'when not valid' do
- let(:poll) { Fabricate.build(:poll, expires_at: nil) }
+ subject { Fabricate.build(:poll) }
- it 'is invalid without an expire date' do
- poll.valid?
- expect(poll).to model_have_error_on_field(:expires_at)
- end
+ it { is_expected.to validate_presence_of(:expires_at) }
end
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 5c2af4dc3..fcff4c0d3 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -32,11 +32,7 @@ RSpec.describe User do
end
describe 'validations' do
- it 'is invalid without an account' do
- user = Fabricate.build(:user, account: nil)
- user.valid?
- expect(user).to model_have_error_on_field(:account)
- end
+ it { is_expected.to belong_to(:account).required }
it 'is invalid without a valid email' do
user = Fabricate.build(:user, email: 'john@')
diff --git a/spec/models/webauthn_credential_spec.rb b/spec/models/webauthn_credential_spec.rb
index 23f0229a6..c4105d915 100644
--- a/spec/models/webauthn_credential_spec.rb
+++ b/spec/models/webauthn_credential_spec.rb
@@ -4,37 +4,10 @@ require 'rails_helper'
RSpec.describe WebauthnCredential do
describe 'validations' do
- it 'is invalid without an external id' do
- webauthn_credential = Fabricate.build(:webauthn_credential, external_id: nil)
-
- webauthn_credential.valid?
-
- expect(webauthn_credential).to model_have_error_on_field(:external_id)
- end
-
- it 'is invalid without a public key' do
- webauthn_credential = Fabricate.build(:webauthn_credential, public_key: nil)
-
- webauthn_credential.valid?
-
- expect(webauthn_credential).to model_have_error_on_field(:public_key)
- end
-
- it 'is invalid without a nickname' do
- webauthn_credential = Fabricate.build(:webauthn_credential, nickname: nil)
-
- webauthn_credential.valid?
-
- expect(webauthn_credential).to model_have_error_on_field(:nickname)
- end
-
- it 'is invalid without a sign_count' do
- webauthn_credential = Fabricate.build(:webauthn_credential, sign_count: nil)
-
- webauthn_credential.valid?
-
- expect(webauthn_credential).to model_have_error_on_field(:sign_count)
- end
+ it { is_expected.to validate_presence_of(:external_id) }
+ it { is_expected.to validate_presence_of(:public_key) }
+ it { is_expected.to validate_presence_of(:nickname) }
+ it { is_expected.to validate_presence_of(:sign_count) }
it 'is invalid if already exist a webauthn credential with the same external id' do
Fabricate(:webauthn_credential, external_id: '_Typ0ygudDnk9YUVWLQayw')
diff --git a/spec/models/webhook_spec.rb b/spec/models/webhook_spec.rb
index 1b2d803bd..03ef1dcc6 100644
--- a/spec/models/webhook_spec.rb
+++ b/spec/models/webhook_spec.rb
@@ -6,12 +6,7 @@ RSpec.describe Webhook do
let(:webhook) { Fabricate(:webhook) }
describe 'Validations' do
- it 'requires presence of events' do
- record = described_class.new(events: nil)
- record.valid?
-
- expect(record).to model_have_error_on_field(:events)
- end
+ it { is_expected.to validate_presence_of(:events) }
it 'requires non-empty events value' do
record = described_class.new(events: [])