DRY up reblog vs original status check

Checking reblog vs original status was happening in multiple places
across the app. For views, this logic was encapsulated in a helper
method named `proper_status` but in the other layers of the app, the
logic was duplicated.

Because the logic is used at all layers of the app, we extracted it into
a `Status#proper` method on the model and changed all uses of the logic
to use this method. There is now a single source of truth for this
condition.

We added test coverage to untested methods that got refactored.

7 files changed, 86 insertions, 11 deletions

diff --git a/app/helpers/stream_entries_helper.rb b/app/helpers/stream_entries_helper.rb
index a26e912a3..38e63ed8d 100644
--- a/app/helpers/stream_entries_helper.rb
+++ b/app/helpers/stream_entries_helper.rb
@@ -34,10 +34,6 @@ module StreamEntriesHelper
     user_signed_in? && @favourited.key?(status.id) ? 'favourited' : ''
   end
 
-  def proper_status(status)
-    status.reblog? ? status.reblog : status
-  end
-
   def rtl?(text)
     return false if text.empty?
 
diff --git a/app/lib/atom_serializer.rb b/app/lib/atom_serializer.rb
index be1cced8b..b9dcee6b3 100644
--- a/app/lib/atom_serializer.rb
+++ b/app/lib/atom_serializer.rb
@@ -328,7 +328,7 @@ class AtomSerializer
 
   def serialize_status_attributes(entry, status)
     append_element(entry, 'summary', status.spoiler_text) unless status.spoiler_text.blank?
-    append_element(entry, 'content', Formatter.instance.format(status.reblog? ? status.reblog : status).to_str, type: 'html')
+    append_element(entry, 'content', Formatter.instance.format(status.proper).to_str, type: 'html')
 
     status.mentions.each do |mentioned|
       append_element(entry, 'link', nil, rel: :mentioned, 'ostatus:object-type': TagManager::TYPES[:person], href: TagManager.instance.uri_for(mentioned.account))
diff --git a/app/models/account.rb b/app/models/account.rb
index 6968607a2..cbba8b5b6 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -125,11 +125,11 @@ class Account < ApplicationRecord
   end
 
   def favourited?(status)
-    (status.reblog? ? status.reblog : status).favourites.where(account: self).count.positive?
+    status.proper.favourites.where(account: self).count.positive?
   end
 
   def reblogged?(status)
-    (status.reblog? ? status.reblog : status).reblogs.where(account: self).count.positive?
+    status.proper.reblogs.where(account: self).count.positive?
   end
 
   def keypair
diff --git a/app/models/status.rb b/app/models/status.rb
index 6948ad77c..7e3dd3e28 100644
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -62,8 +62,12 @@ class Status < ApplicationRecord
     reply? ? :comment : :note
   end
 
+  def proper
+    reblog? ? reblog : self
+  end
+
   def content
-    reblog? ? reblog.text : text
+    proper.text
   end
 
   def target
diff --git a/app/views/stream_entries/_status.html.haml b/app/views/stream_entries/_status.html.haml
index cdd0dde3b..434c5c8da 100644
--- a/app/views/stream_entries/_status.html.haml
+++ b/app/views/stream_entries/_status.html.haml
@@ -16,7 +16,7 @@
           %strong= display_name(status.account)
         = t('stream_entries.reblogged')
 
-  = render partial: centered ? 'stream_entries/detailed_status' : 'stream_entries/simple_status', locals: { status: proper_status(status) }
+  = render partial: centered ? 'stream_entries/detailed_status' : 'stream_entries/simple_status', locals: { status: status.proper }
 
 - if include_threads
   = render partial: 'stream_entries/status', collection: @descendants, as: :status, locals: { is_successor: true }
diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb
index d7f59adb8..93a45459d 100644
--- a/spec/models/account_spec.rb
+++ b/spec/models/account_spec.rb
@@ -99,11 +99,75 @@ RSpec.describe Account, type: :model do
   end
 
   describe '#favourited?' do
-    pending
+    let(:original_status) do
+      author = Fabricate(:account, username: 'original')
+      Fabricate(:status, account: author)
+    end
+
+    context 'when the status is a reblog of another status' do
+      let(:original_reblog) do
+        author = Fabricate(:account, username: 'original_reblogger')
+        Fabricate(:status, reblog: original_status, account: author)
+      end
+
+      it 'is is true when this account has favourited it' do
+        Fabricate(:favourite, status: original_reblog, account: subject)
+
+        expect(subject.favourited?(original_status)).to eq true
+      end
+
+      it 'is false when this account has not favourited it' do
+        expect(subject.favourited?(original_status)).to eq false
+      end
+    end
+
+    context 'when the status is an original status' do
+      it 'is is true when this account has favourited it' do
+        Fabricate(:favourite, status: original_status, account: subject)
+
+        expect(subject.favourited?(original_status)).to eq true
+      end
+
+      it 'is false when this account has not favourited it' do
+        expect(subject.favourited?(original_status)).to eq false
+      end
+    end
   end
 
   describe '#reblogged?' do
-    pending
+    let(:original_status) do
+      author = Fabricate(:account, username: 'original')
+      Fabricate(:status, account: author)
+    end
+
+    context 'when the status is a reblog of another status'do
+      let(:original_reblog) do
+        author = Fabricate(:account, username: 'original_reblogger')
+        Fabricate(:status, reblog: original_status, account: author)
+      end
+
+      it 'is true when this account has reblogged it' do
+        Fabricate(:status, reblog: original_reblog, account: subject)
+
+        expect(subject.reblogged?(original_reblog)).to eq true
+      end
+
+      it 'is false when this account has not reblogged it' do
+        expect(subject.reblogged?(original_reblog)).to eq false
+      end
+    end
+
+    context 'when the status is an original status' do
+      it 'is true when this account has reblogged it' do
+        Fabricate(:status, reblog: original_status, account: subject)
+
+        expect(subject.reblogged?(original_status)).to eq true
+      end
+
+      it 'is false when this account has not reblogged it' do
+        expect(subject.reblogged?(original_status)).to eq false
+      end
+    end
   end
 
   describe '.find_local' do
diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb
index b9d079521..db244ebe7 100644
--- a/spec/models/status_spec.rb
+++ b/spec/models/status_spec.rb
@@ -97,4 +97,15 @@ RSpec.describe Status, type: :model do
   describe '#favourites_count' do
     pending
   end
+
+  describe '#proper' do
+    it 'is itself for original statuses' do
+      expect(subject.proper).to eq subject
+    end
+
+    it 'is the source status for reblogs' do
+      subject.reblog = other
+      expect(subject.proper).to eq other
+    end
+  end
 end