Added validator for extra profile field values with empty name (#33421)
This commit is contained in:
		
							parent
							
								
									9b82bedc6f
								
							
						
					
					
						commit
						3bdfa3eb4c
					
				| 
						 | 
					@ -118,6 +118,7 @@ class Account < ApplicationRecord
 | 
				
			||||||
  validates :display_name, length: { maximum: DISPLAY_NAME_LENGTH_LIMIT }, if: -> { local? && will_save_change_to_display_name? }
 | 
					  validates :display_name, length: { maximum: DISPLAY_NAME_LENGTH_LIMIT }, if: -> { local? && will_save_change_to_display_name? }
 | 
				
			||||||
  validates :note, note_length: { maximum: NOTE_LENGTH_LIMIT }, if: -> { local? && will_save_change_to_note? }
 | 
					  validates :note, note_length: { maximum: NOTE_LENGTH_LIMIT }, if: -> { local? && will_save_change_to_note? }
 | 
				
			||||||
  validates :fields, length: { maximum: DEFAULT_FIELDS_SIZE }, if: -> { local? && will_save_change_to_fields? }
 | 
					  validates :fields, length: { maximum: DEFAULT_FIELDS_SIZE }, if: -> { local? && will_save_change_to_fields? }
 | 
				
			||||||
 | 
					  validates_with EmptyProfileFieldNamesValidator, if: -> { local? && will_save_change_to_fields? }
 | 
				
			||||||
  with_options on: :create do
 | 
					  with_options on: :create do
 | 
				
			||||||
    validates :uri, absence: true, if: :local?
 | 
					    validates :uri, absence: true, if: :local?
 | 
				
			||||||
    validates :inbox_url, absence: true, if: :local?
 | 
					    validates :inbox_url, absence: true, if: :local?
 | 
				
			||||||
| 
						 | 
					@ -300,7 +301,7 @@ class Account < ApplicationRecord
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    if attributes.is_a?(Hash)
 | 
					    if attributes.is_a?(Hash)
 | 
				
			||||||
      attributes.each_value do |attr|
 | 
					      attributes.each_value do |attr|
 | 
				
			||||||
        next if attr[:name].blank?
 | 
					        next if attr[:name].blank? && attr[:value].blank?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        previous = old_fields.find { |item| item['value'] == attr[:value] }
 | 
					        previous = old_fields.find { |item| item['value'] == attr[:value] }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -0,0 +1,15 @@
 | 
				
			||||||
 | 
					# frozen_string_literal: true
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					class EmptyProfileFieldNamesValidator < ActiveModel::Validator
 | 
				
			||||||
 | 
					  def validate(account)
 | 
				
			||||||
 | 
					    return if account.fields.empty?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    account.errors.add(:fields, :fields_with_values_missing_labels) if fields_with_values_missing_names?(account)
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  private
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  def fields_with_values_missing_names?(account)
 | 
				
			||||||
 | 
					    account.fields.any? { |field| field.name.blank? && field.value.present? }
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
 | 
					end
 | 
				
			||||||
| 
						 | 
					@ -24,6 +24,8 @@ en:
 | 
				
			||||||
      models:
 | 
					      models:
 | 
				
			||||||
        account:
 | 
					        account:
 | 
				
			||||||
          attributes:
 | 
					          attributes:
 | 
				
			||||||
 | 
					            fields:
 | 
				
			||||||
 | 
					              fields_with_values_missing_labels: contains values with missing labels
 | 
				
			||||||
            username:
 | 
					            username:
 | 
				
			||||||
              invalid: must contain only letters, numbers and underscores
 | 
					              invalid: must contain only letters, numbers and underscores
 | 
				
			||||||
              reserved: is reserved
 | 
					              reserved: is reserved
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -822,6 +822,10 @@ RSpec.describe Account do
 | 
				
			||||||
      it { is_expected.to validate_length_of(:display_name).is_at_most(described_class::DISPLAY_NAME_LENGTH_LIMIT) }
 | 
					      it { is_expected.to validate_length_of(:display_name).is_at_most(described_class::DISPLAY_NAME_LENGTH_LIMIT) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      it { is_expected.to_not allow_values(account_note_over_limit).for(:note) }
 | 
					      it { is_expected.to_not allow_values(account_note_over_limit).for(:note) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      it { is_expected.to allow_value(fields_empty_name_value).for(:fields) }
 | 
				
			||||||
 | 
					      it { is_expected.to_not allow_value(fields_over_limit).for(:fields) }
 | 
				
			||||||
 | 
					      it { is_expected.to_not allow_value(fields_empty_name).for(:fields) }
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    context 'when account is remote' do
 | 
					    context 'when account is remote' do
 | 
				
			||||||
| 
						 | 
					@ -854,6 +858,18 @@ RSpec.describe Account do
 | 
				
			||||||
    def account_note_over_limit
 | 
					    def account_note_over_limit
 | 
				
			||||||
      'a' * described_class::NOTE_LENGTH_LIMIT * 2
 | 
					      'a' * described_class::NOTE_LENGTH_LIMIT * 2
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def fields_empty_name_value
 | 
				
			||||||
 | 
					      Array.new(4) { { 'name' => '', 'value' => '' } }
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def fields_over_limit
 | 
				
			||||||
 | 
					      Array.new(5) { { 'name' => 'Name', 'value' => 'Value', 'verified_at' => '01/01/1970' } }
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def fields_empty_name
 | 
				
			||||||
 | 
					      [{ 'name' => '', 'value' => 'Value', 'verified_at' => '01/01/1970' }]
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  describe 'scopes' do
 | 
					  describe 'scopes' do
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in New Issue