Add Status::Visibility concern to hold visibility logic (#33578)

app/models/concerns/status/visibility.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module Status::Visibility
+  extend ActiveSupport::Concern
+
+  included do
+    enum :visibility,
+         { public: 0, unlisted: 1, private: 2, direct: 3, limited: 4 },
+         suffix: :visibility,
+         validate: true
+
+    scope :distributable_visibility, -> { where(visibility: %i(public unlisted)) }
+    scope :list_eligible_visibility, -> { where(visibility: %i(public unlisted private)) }
+    scope :not_direct_visibility, -> { where.not(visibility: :direct) }
+
+    validates :visibility, exclusion: { in: %w(direct limited) }, if: :reblog?
+
+    before_validation :set_visibility, unless: :visibility?
+  end
+
+  class_methods do
+    def selectable_visibilities
+      visibilities.keys - %w(direct limited)
+    end
+  end
+
+  def hidden?
+    !distributable?
+  end
+
+  def distributable?
+    public_visibility? || unlisted_visibility?
+  end
+
+  alias sign? distributable?
+
+  private
+
+  def set_visibility
+    self.visibility ||= reblog.visibility if reblog?
+    self.visibility ||= visibility_from_account
+  end
+
+  def visibility_from_account
+    account.locked? ? :private : :public
+  end
+end

app/models/status.rb

@@ -38,6 +38,7 @@ class Status < ApplicationRecord
   include Status::SearchConcern
   include Status::SnapshotConcern
   include Status::ThreadingConcern
+  include Status::Visibility
 
   MEDIA_ATTACHMENTS_LIMIT = 4
 
@@ -52,8 +53,6 @@ class Status < ApplicationRecord
   update_index('statuses', :proper)
   update_index('public_statuses', :proper)
 
-  enum :visibility, { public: 0, unlisted: 1, private: 2, direct: 3, limited: 4 }, suffix: :visibility, validate: true
-
   belongs_to :application, class_name: 'Doorkeeper::Application', optional: true
 
   belongs_to :account, inverse_of: :statuses
@@ -98,7 +97,6 @@ class Status < ApplicationRecord
   validates_with StatusLengthValidator
   validates_with DisallowedHashtagsValidator
   validates :reblog, uniqueness: { scope: :account }, if: :reblog?
-  validates :visibility, exclusion: { in: %w(direct limited) }, if: :reblog?
 
   accepts_nested_attributes_for :poll
 
@@ -125,9 +123,6 @@ class Status < ApplicationRecord
   scope :tagged_with_none, lambda { |tag_ids|
     where('NOT EXISTS (SELECT * FROM statuses_tags forbidden WHERE forbidden.status_id = statuses.id AND forbidden.tag_id IN (?))', tag_ids)
   }
-  scope :distributable_visibility, -> { where(visibility: %i(public unlisted)) }
-  scope :list_eligible_visibility, -> { where(visibility: %i(public unlisted private)) }
-  scope :not_direct_visibility, -> { where.not(visibility: :direct) }
 
   after_create_commit :trigger_create_webhooks
   after_update_commit :trigger_update_webhooks
@@ -140,7 +135,6 @@ class Status < ApplicationRecord
 
   before_validation :prepare_contents, if: :local?
   before_validation :set_reblog
-  before_validation :set_visibility
   before_validation :set_conversation
   before_validation :set_local
 
@@ -242,16 +236,6 @@ class Status < ApplicationRecord
     PreviewCardsStatus.where(status_id: id).delete_all
   end
 
-  def hidden?
-    !distributable?
-  end
-
-  def distributable?
-    public_visibility? || unlisted_visibility?
-  end
-
-  alias sign? distributable?
-
   def with_media?
     ordered_media_attachments.any?
   end
@@ -351,10 +335,6 @@ class Status < ApplicationRecord
   end
 
   class << self
-    def selectable_visibilities
-      visibilities.keys - %w(direct limited)
-    end
-
     def favourites_map(status_ids, account_id)
       Favourite.select(:status_id).where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |f, h| h[f.status_id] = true }
     end
@@ -436,11 +416,6 @@ class Status < ApplicationRecord
     update_column(:poll_id, poll.id) if association(:poll).loaded? && poll.present?
   end
 
-  def set_visibility
-    self.visibility = reblog.visibility if reblog? && visibility.nil?
-    self.visibility = (account.locked? ? :private : :public) if visibility.nil?
-  end
-
   def set_conversation
     self.thread = thread.reblog if thread&.reblog?
 

spec/models/status_spec.rb

@@ -9,6 +9,8 @@ RSpec.describe Status do
   let(:bob)   { Fabricate(:account, username: 'bob') }
   let(:other) { Fabricate(:status, account: bob, text: 'Skulls for the skull god! The enemy\'s gates are sideways!') }
 
+  include_examples 'Status::Visibility'
+
   describe '#local?' do
     it 'returns true when no remote URI is set' do
       expect(subject.local?).to be true
@@ -84,36 +86,6 @@ RSpec.describe Status do
     end
   end
 
-  describe '#hidden?' do
-    context 'when private_visibility?' do
-      it 'returns true' do
-        subject.visibility = :private
-        expect(subject.hidden?).to be true
-      end
-    end
-
-    context 'when direct_visibility?' do
-      it 'returns true' do
-        subject.visibility = :direct
-        expect(subject.hidden?).to be true
-      end
-    end
-
-    context 'when public_visibility?' do
-      it 'returns false' do
-        subject.visibility = :public
-        expect(subject.hidden?).to be false
-      end
-    end
-
-    context 'when unlisted_visibility?' do
-      it 'returns false' do
-        subject.visibility = :unlisted
-        expect(subject.hidden?).to be false
-      end
-    end
-  end
-
   describe '#content' do
     it 'returns the text of the status if it is not a reblog' do
       expect(subject.content).to eql subject.text

spec/support/examples/models/concerns/status/visibility.rb

@@ -0,0 +1,184 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.shared_examples 'Status::Visibility' do
+  describe 'Validations' do
+    context 'when status is a reblog' do
+      subject { Fabricate.build :status, reblog: Fabricate(:status) }
+
+      it { is_expected.to allow_values('public', 'unlisted', 'private').for(:visibility) }
+      it { is_expected.to_not allow_values('direct', 'limited').for(:visibility) }
+    end
+
+    context 'when status is not reblog' do
+      subject { Fabricate.build :status, reblog_of_id: nil }
+
+      it { is_expected.to allow_values('public', 'unlisted', 'private', 'direct', 'limited').for(:visibility) }
+    end
+  end
+
+  describe 'Scopes' do
+    let!(:direct_status) { Fabricate :status, visibility: :direct }
+    let!(:limited_status) { Fabricate :status, visibility: :limited }
+    let!(:private_status) { Fabricate :status, visibility: :private }
+    let!(:public_status) { Fabricate :status, visibility: :public }
+    let!(:unlisted_status) { Fabricate :status, visibility: :unlisted }
+
+    describe '.list_eligible_visibility' do
+      it 'returns appropriate records' do
+        expect(Status.list_eligible_visibility)
+          .to include(
+            private_status,
+            public_status,
+            unlisted_status
+          )
+          .and not_include(direct_status)
+          .and not_include(limited_status)
+      end
+    end
+
+    describe '.distributable_visibility' do
+      it 'returns appropriate records' do
+        expect(Status.distributable_visibility)
+          .to include(
+            public_status,
+            unlisted_status
+          )
+          .and not_include(private_status)
+          .and not_include(direct_status)
+          .and not_include(limited_status)
+      end
+    end
+
+    describe '.not_direct_visibility' do
+      it 'returns appropriate records' do
+        expect(Status.not_direct_visibility)
+          .to include(
+            limited_status,
+            private_status,
+            public_status,
+            unlisted_status
+          )
+          .and not_include(direct_status)
+      end
+    end
+  end
+
+  describe 'Callbacks' do
+    describe 'Setting visibility in before validation' do
+      subject { Fabricate.build :status, visibility: nil }
+
+      context 'when explicit value is set' do
+        before { subject.visibility = :public }
+
+        it 'does not change' do
+          expect { subject.valid? }
+            .to_not change(subject, :visibility)
+        end
+      end
+
+      context 'when status is a reblog' do
+        before { subject.reblog = Fabricate(:status, visibility: :public) }
+
+        it 'changes to match the reblog' do
+          expect { subject.valid? }
+            .to change(subject, :visibility).to('public')
+        end
+      end
+
+      context 'when account is locked' do
+        before { subject.account = Fabricate.build(:account, locked: true) }
+
+        it 'changes to private' do
+          expect { subject.valid? }
+            .to change(subject, :visibility).to('private')
+        end
+      end
+
+      context 'when account is not locked' do
+        before { subject.account = Fabricate.build(:account, locked: false) }
+
+        it 'changes to public' do
+          expect { subject.valid? }
+            .to change(subject, :visibility).to('public')
+        end
+      end
+    end
+  end
+
+  describe '.selectable_visibilities' do
+    it 'returns options available for default privacy selection' do
+      expect(Status.selectable_visibilities)
+        .to match(%w(public unlisted private))
+    end
+  end
+
+  describe '#hidden?' do
+    subject { Status.new }
+
+    context 'when visibility is private' do
+      before { subject.visibility = :private }
+
+      it { is_expected.to be_hidden }
+    end
+
+    context 'when visibility is direct' do
+      before { subject.visibility = :direct }
+
+      it { is_expected.to be_hidden }
+    end
+
+    context 'when visibility is limited' do
+      before { subject.visibility = :limited }
+
+      it { is_expected.to be_hidden }
+    end
+
+    context 'when visibility is public' do
+      before { subject.visibility = :public }
+
+      it { is_expected.to_not be_hidden }
+    end
+
+    context 'when visibility is unlisted' do
+      before { subject.visibility = :unlisted }
+
+      it { is_expected.to_not be_hidden }
+    end
+  end
+
+  describe '#distributable?' do
+    subject { Status.new }
+
+    context 'when visibility is public' do
+      before { subject.visibility = :public }
+
+      it { is_expected.to be_distributable }
+    end
+
+    context 'when visibility is unlisted' do
+      before { subject.visibility = :unlisted }
+
+      it { is_expected.to be_distributable }
+    end
+
+    context 'when visibility is private' do
+      before { subject.visibility = :private }
+
+      it { is_expected.to_not be_distributable }
+    end
+
+    context 'when visibility is direct' do
+      before { subject.visibility = :direct }
+
+      it { is_expected.to_not be_distributable }
+    end
+
+    context 'when visibility is limited' do
+      before { subject.visibility = :limited }
+
+      it { is_expected.to_not be_distributable }
+    end
+  end
+end