From 236eef1183a0dc8c2cf8fa6a322d83a448463a90 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Thu, 4 Dec 2025 21:55:13 +0000 Subject: [PATCH 1/2] Refactor user detail spec for improved validation clarity This refactor enhances the user detail specifications by consolidating validation tests for attributes such as discord, bluesky, mastodon, and orcid. The changes improve readability and maintainability of the tests, ensuring that error messages are clearly defined and validation rules are consistently applied. Technical changes: - Removed redundant context blocks for attribute validations - Introduced error messages for clarity in validation failures - Streamlined the use of subject and let_it_be for better test setup Changelog: changed --- spec/models/user_detail_spec.rb | 147 +++++++------------------------- 1 file changed, 29 insertions(+), 118 deletions(-) diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index e73355a353b07c..298f15c5825a64 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe UserDetail, feature_category: :system_access do + let_it_be(:user_detail) { build(:user_detail) } + it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:bot_namespace).inverse_of(:bot_user_details) } @@ -324,146 +326,55 @@ end describe '#discord' do - it { is_expected.to validate_length_of(:discord).is_at_most(500) } + let(:error_message) { 'must contain only a discord user ID.' } - context 'when discord is set' do - let_it_be(:user_detail) { create(:user).user_detail } + subject { user_detail } - it 'accepts a valid discord user id' do - user_detail.discord = '1234567890123456789' - - expect(user_detail).to be_valid - end - - it 'throws an error when other url format is wrong' do - user_detail.discord = '123456789' + it { is_expected.to validate_length_of(:discord).is_at_most(500) } - expect(user_detail).not_to be_valid - expect(user_detail.errors.full_messages).to match_array([_('Discord must contain only a discord user ID.')]) - end - end + it { is_expected.to allow_value('1234567890123456789').for(:discord) } + it { is_expected.not_to allow_value('123456789').for(:discord).with_message(error_message) } end describe '#bluesky' do - context 'when bluesky is set' do - let_it_be(:user_detail) { build(:user_detail) } - - let(:value) { 'did:plc:ewvi7nxzyoun6zhxrhs64oiz' } - - before do - user_detail.bluesky = value - end - - it 'accepts a valid bluesky did id' do - expect(user_detail).to be_valid - end - - shared_examples 'throws an error' do - it do - expect(user_detail).not_to be_valid - expect(user_detail.errors.full_messages) - .to match_array([_('Bluesky must contain only a bluesky did:plc identifier.')]) - end - end - - context 'when bluesky is set to a wrong format' do - context 'when bluesky did:plc is too long' do - let(:value) { 'a' * 33 } - - it_behaves_like 'throws an error' - end - - context 'when bluesky did:plc is wrong' do - let(:value) { 'did:plc:ewvi7nxzyoun6zhxrhs64OIZ' } + let(:error_message) { 'must contain only a bluesky did:plc identifier.' } - it_behaves_like 'throws an error' - end + subject { user_detail } - context 'when bluesky other bluesky did: formats are used' do - let(:value) { 'did:web:example.com' } + it { is_expected.to allow_value('did:plc:ewvi7nxzyoun6zhxrhs64oiz').for(:bluesky) } + it { is_expected.not_to allow_value('a' * 33).for(:bluesky).with_message(error_message) } - it_behaves_like 'throws an error' - end - end + it do + is_expected.not_to allow_value('did:plc:ewvi7nxzyoun6zhxrhs64OIZ').for(:bluesky).with_message(error_message) end + + it { is_expected.not_to allow_value('did:web:example.com').for(:bluesky).with_message(error_message) } end describe '#mastodon' do - it { is_expected.to validate_length_of(:mastodon).is_at_most(500) } - - context 'when mastodon is set' do - let_it_be(:user_detail) { create(:user).user_detail } + let(:error_message) { 'must contain only a mastodon handle.' } - it 'accepts a valid mastodon username' do - user_detail.mastodon = '@robin@example.com' + subject { user_detail } - expect(user_detail).to be_valid - end - - it 'throws an error when mastodon username format is wrong' do - user_detail.mastodon = '@robin' - - expect(user_detail).not_to be_valid - expect(user_detail.errors.full_messages) - .to match_array([_('Mastodon must contain only a mastodon handle.')]) - end - end + it { is_expected.to validate_length_of(:mastodon).is_at_most(500) } + it { is_expected.to allow_value('@robin@example.com').for(:mastodon) } + it { is_expected.not_to allow_value('@robin').for(:mastodon).with_message(error_message) } end describe '#orcid' do - context 'when orcid is set' do - let_it_be(:user_detail) { create(:user).user_detail } - - it 'accepts a valid orcid username' do - user_detail.orcid = '1234-1234-1234-1234' - - expect(user_detail).to be_valid - end + let_it_be(:user_detail) { create(:user).user_detail } - it 'accepts a valid orcid username' do - user_detail.orcid = '1234-1234-1234-123X' + let(:error_message) { 'must contain only a valid ORCID.' } - expect(user_detail).to be_valid - end + subject { user_detail } - context 'when orcid is wrong' do - shared_examples 'throws an error' do - before do - user_detail.orcid = orcid - end - - it 'throws an error' do - expect(user_detail).not_to be_valid - expect(user_detail.errors.full_messages) - .to match_array([_('Orcid must contain only a valid ORCID.')]) - end - end + it { is_expected.to allow_value('1234-1234-1234-1234').for(:orcid) } + it { is_expected.to allow_value('1234-1234-1234-123X').for(:orcid) } - context 'when the format is too long' do - let(:orcid) { '1234-1234-1234-1234-1234' } - - it_behaves_like 'throws an error' - end - - context 'when the format is too short' do - let(:orcid) { '1234-1234' } - - it_behaves_like 'throws an error' - end - - context 'when the format is letters' do - let(:orcid) { 'abcd-abcd-abcd-abcd' } - - it_behaves_like 'throws an error' - end - - context 'when the format end with another letter than X' do - let(:orcid) { '1234-1234-1234-123Y' } - - it_behaves_like 'throws an error' - end - end - end + it { is_expected.not_to allow_value('1234-1234-1234-1234-1234').for(:orcid).with_message(error_message) } + it { is_expected.not_to allow_value('1234-1234').for(:orcid).with_message(error_message) } + it { is_expected.not_to allow_value('abcd-abcd-abcd-abcd').for(:orcid).with_message(error_message) } + it { is_expected.not_to allow_value('1234-1234-1234-123Y').for(:orcid).with_message(error_message) } end describe '#location' do -- GitLab From 9efa0436d987190a0e049305d01e59799a23b1a7 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Wed, 17 Dec 2025 08:57:37 +0100 Subject: [PATCH 2/2] refactor: Apply suggestion from @tyleramos --- spec/models/user_detail_spec.rb | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index 298f15c5825a64..07450ba31c5f99 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -3,8 +3,6 @@ require 'spec_helper' RSpec.describe UserDetail, feature_category: :system_access do - let_it_be(:user_detail) { build(:user_detail) } - it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:bot_namespace).inverse_of(:bot_user_details) } @@ -326,9 +324,7 @@ end describe '#discord' do - let(:error_message) { 'must contain only a discord user ID.' } - - subject { user_detail } + let(:error_message) { _('must contain only a discord user ID.') } it { is_expected.to validate_length_of(:discord).is_at_most(500) } @@ -337,9 +333,7 @@ end describe '#bluesky' do - let(:error_message) { 'must contain only a bluesky did:plc identifier.' } - - subject { user_detail } + let(:error_message) { _('must contain only a bluesky did:plc identifier.') } it { is_expected.to allow_value('did:plc:ewvi7nxzyoun6zhxrhs64oiz').for(:bluesky) } it { is_expected.not_to allow_value('a' * 33).for(:bluesky).with_message(error_message) } @@ -352,9 +346,7 @@ end describe '#mastodon' do - let(:error_message) { 'must contain only a mastodon handle.' } - - subject { user_detail } + let(:error_message) { _('must contain only a mastodon handle.') } it { is_expected.to validate_length_of(:mastodon).is_at_most(500) } it { is_expected.to allow_value('@robin@example.com').for(:mastodon) } @@ -362,11 +354,7 @@ end describe '#orcid' do - let_it_be(:user_detail) { create(:user).user_detail } - - let(:error_message) { 'must contain only a valid ORCID.' } - - subject { user_detail } + let(:error_message) { _('must contain only a valid ORCID.') } it { is_expected.to allow_value('1234-1234-1234-1234').for(:orcid) } it { is_expected.to allow_value('1234-1234-1234-123X').for(:orcid) } -- GitLab