Remove attribution_domains_as_text

app/controllers/settings/verifications_controller.rb

@@ -18,7 +18,15 @@ class Settings::VerificationsController < Settings::BaseController
   private
 
   def account_params
-    params.require(:account).permit(:attribution_domains_as_text)
+    params.require(:account).permit(:attribution_domains).tap do |params|
+      params[:attribution_domains] = params[:attribution_domains]&.lines&.map do |line|
+        line
+          .strip
+          .delete_prefix('http://')
+          .delete_prefix('https://')
+          .delete_prefix('*.')
+      end
+    end
   end
 
   def set_account

app/models/concerns/account/attribution_domains.rb

@@ -4,21 +4,9 @@ module Account::AttributionDomains
   extend ActiveSupport::Concern
 
   included do
-    validates :attribution_domains_as_text, domain: { multiline: true }, lines: { maximum: 100 }, if: -> { local? && will_save_change_to_attribution_domains? }
-  end
+    normalizes :attribution_domains, with: ->(arr) { arr.uniq }
 
-  def attribution_domains_as_text
-    self[:attribution_domains].join("\n")
-  end
-
-  def attribution_domains_as_text=(str)
-    self[:attribution_domains] = str.split.filter_map do |line|
-      line
-        .strip
-        .delete_prefix('http://')
-        .delete_prefix('https://')
-        .delete_prefix('*.')
-    end
+    validates :attribution_domains, domain: true, length: { maximum: 100 }, if: -> { local? && will_save_change_to_attribution_domains? }
   end
 
   def can_be_attributed_from?(domain)

app/validators/domain_validator.rb

@@ -9,18 +9,18 @@ class DomainValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
     return if value.blank?
 
-    (options[:multiline] ? value.split : [value]).each do |domain|
+    Array.wrap(value).each do |domain|
       _, domain = domain.split('@') if options[:acct]
 
-      next if domain.blank?
-
-      record.errors.add(attribute, options[:multiline] ? :invalid_domain_on_line : :invalid, value: domain) unless compliant?(domain)
+      record.errors.add(attribute, value.is_a?(Enumerable) ? :invalid_domain_on_line : :invalid, value: domain) unless compliant?(domain)
     end
   end
 
   private
 
   def compliant?(value)
+    return false if value.blank?
+
     uri = Addressable::URI.new
     uri.host = value
     uri.normalized_host.size < MAX_DOMAIN_LENGTH && uri.normalized_host.split('.').all? { |label| label.size.between?(MIN_LABEL_LENGTH, MAX_LABEL_LENGTH) && label =~ ALLOWED_CHARACTERS_RE }

app/validators/lines_validator.rb

@@ -1,9 +0,0 @@
-# frozen_string_literal: true
-
-class LinesValidator < ActiveModel::EachValidator
-  def validate_each(record, attribute, value)
-    return if value.blank?
-
-    record.errors.add(attribute, :too_many_lines, limit: options[:maximum]) if options[:maximum].present? && value.split.size > options[:maximum]
-  end
-end

app/views/settings/verifications/show.html.haml

@@ -65,7 +65,7 @@
   %p.lead= t('author_attribution.then_instructions')
 
   .fields-group
-    = f.input :attribution_domains_as_text, as: :text, wrapper: :with_block_label, input_html: { placeholder: "example1.com\nexample2.com\nexample3.com", rows: 4 }
+    = f.input :attribution_domains, as: :text, wrapper: :with_block_label, input_html: { value: @account.attribution_domains.join("\n"), placeholder: "example1.com\nexample2.com\nexample3.com", rows: 4 }
 
   .actions
     = f.button :button, t('generic.save_changes'), type: :submit

spec/models/account_spec.rb

@@ -750,38 +750,14 @@ RSpec.describe Account do
     end
   end
 
-  describe '#attribution_domains_as_text=' do
-    subject { Fabricate(:account) }
-
-    it 'sets attribution_domains accordingly' do
-      subject.attribution_domains_as_text = "hoge.com\nexample.com"
-
-      expect(subject.attribution_domains).to contain_exactly('hoge.com', 'example.com')
-    end
-
-    it 'strips leading "*."' do
-      subject.attribution_domains_as_text = "hoge.com\n*.example.com"
-
-      expect(subject.attribution_domains).to contain_exactly('hoge.com', 'example.com')
-    end
-
-    it 'strips the protocol if present' do
-      subject.attribution_domains_as_text = "http://hoge.com\nhttps://example.com"
-
-      expect(subject.attribution_domains).to contain_exactly('hoge.com', 'example.com')
-    end
-
-    it 'strips a combination of leading "*." and protocol' do
-      subject.attribution_domains_as_text = "http://*.hoge.com\nhttps://*.example.com"
-
-      expect(subject.attribution_domains).to contain_exactly('hoge.com', 'example.com')
-    end
-  end
-
   describe 'Normalizations' do
     describe 'username' do
       it { is_expected.to normalize(:username).from(" \u3000bob \t \u00a0 \n ").to('bob') }
     end
+
+    describe 'attribution_domains' do
+      it { is_expected.to normalize(:attribution_domains).from(['example.com', 'example.com', 'example.net']).to(['example.com', 'example.net']) }
+    end
   end
 
   describe 'Validations' do
@@ -822,6 +798,9 @@ 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_not allow_values(account_note_over_limit).for(:note) }
+
+      it { is_expected.to allow_values([], ['example.com'], (1..100).to_a).for(:attribution_domains) }
+      it { is_expected.to_not allow_values([''], ['@'], (1..101).to_a).for(:attribution_domains) }
     end
 
     context 'when account is remote' do

spec/requests/settings/verification_spec.rb

@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Settings / Verification' do
+  let(:user) { Fabricate :user, account_attributes: { attribution_domains: ['example.com', 'example.net'] } }
+  let(:params) do
+    {
+      account: {
+        attribution_domains: attribution_domains,
+      },
+    }
+  end
+  let(:attribution_domains) { " example.com\n\n  https://example.org" }
+
+  describe 'GET /settings/verification' do
+    it 'shows attribution domains in textarea' do
+      sign_in user if user
+      get settings_verification_path
+
+      expect(response.body)
+        .to include(">\nexample.com\nexample.net</textarea>")
+    end
+
+    context 'when not signed in' do
+      it 'redirects to sign in page' do
+        get settings_verification_path
+
+        expect(response)
+          .to redirect_to new_user_session_path
+      end
+    end
+  end
+
+  describe 'PUT /settings/verification' do
+    before do
+      sign_in user if user
+      put settings_verification_path, params: params
+    end
+
+    it 'updates account and redirects' do
+      expect(user.account.reload.attribution_domains)
+        .to eq ['example.com', 'example.org']
+
+      expect(response)
+        .to redirect_to settings_verification_path
+    end
+
+    context 'when attribution_domains contains invalid domain' do
+      let(:attribution_domains) { "example.com\ninvalid_com" }
+
+      it 'mentions invalid domain' do
+        expect(response).to have_http_status(200)
+        expect(response.body)
+          .to include I18n.t('activerecord.errors.messages.invalid_domain_on_line', value: 'invalid_com')
+      end
+    end
+
+    context 'when not signed in' do
+      let(:user) { nil }
+
+      it 'redirects to sign in page' do
+        expect(response)
+          .to redirect_to new_user_session_path
+      end
+    end
+  end
+end

spec/validators/lines_validator_spec.rb

@@ -1,46 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-RSpec.describe LinesValidator do
-  let(:record_class) do
-    Class.new do
-      include ActiveModel::Validations
-
-      attr_accessor :text
-
-      validates :text, lines: { maximum: 5 }
-    end
-  end
-
-  let(:record) { record_class.new }
-
-  describe '#validate_each' do
-    context 'with a nil value' do
-      it 'does not add errors' do
-        record.text = nil
-
-        expect(record).to be_valid
-        expect(record.errors).to be_empty
-      end
-    end
-
-    context 'with lines below the limit' do
-      it 'does not add errors' do
-        record.text = "hoge\n" * 5
-
-        expect(record).to be_valid
-        expect(record.errors).to be_empty
-      end
-    end
-
-    context 'with more lines than limit' do
-      it 'adds an error' do
-        record.text = "hoge\n" * 6
-
-        expect(record).to_not be_valid
-        expect(record.errors.where(:text)).to_not be_empty
-      end
-    end
-  end
-end