Improve RSS entries for statuses (#13592)

* Improve RSS entries for statuses

- Render polls in both accounts and tags serializers
- Refactor RSS serializers
- Change title preview to include ellipsis when truncated
- Change title preview to show CW instead of toot text
- Add tests

* Remove title from OEmbed serialization

Twitter doesn't serialize title either, and tihs allows us to move the
title formatting code to the RSS serializers.

diff --git a/app/lib/rss/serializer.rb b/app/lib/rss/serializer.rb
new file mode 100644 (file)
index 0000000..fd56c56
--- /dev/null
+++ b/app/lib/rss/serializer.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+class RSS::Serializer
+  private
+
+  def render_statuses(builder, statuses)
+    statuses.each do |status|
+      builder.item do |item|
+        item.title(status_title(status))
+            .link(ActivityPub::TagManager.instance.url_for(status))
+            .pub_date(status.created_at)
+            .description(status.spoiler_text.presence || Formatter.instance.format(status, inline_poll_options: true).to_str)
+
+        status.media_attachments.each do |media|
+          item.enclosure(full_asset_url(media.file.url(:original, false)), media.file.content_type, media.file.size)
+        end
+      end
+    end
+  end
+
+  def status_title(status)
+    return "#{status.account.acct} deleted status" if status.destroyed?
+
+    preview = status.proper.spoiler_text.presence || status.proper.text
+    if preview.length > 30 || preview[0, 30].include?("\n")
+      preview = preview[0, 30]
+      preview = preview[0, preview.index("\n").presence || 30] + '…'
+    end
+
+    preview = "#{status.proper.spoiler_text.present? ? 'CW ' : ''}“#{preview}”#{status.proper.sensitive? ? ' (sensitive)' : ''}"
+
+    if status.reblog?
+      "#{status.account.acct} boosted #{status.reblog.account.acct}: #{preview}"
+    else
+      "#{status.account.acct}: #{preview}"
+    end
+  end
+end

diff --git a/app/models/status.rb b/app/models/status.rb
index a938ff032563a285f96437aa2d144c43c0208074..8c7fe8dfa27942eed8ed0825153e5463375b58ac 100644 (file)
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -197,14 +197,6 @@ class Status < ApplicationRecord
     preview_cards.first
   end
 
-  def title
-    if destroyed?
-      "#{account.acct} deleted status"
-    else
-      reblog? ? "#{account.acct} shared a status by #{reblog.account.acct}" : "New status by #{account.acct}"
-    end
-  end
-
   def hidden?
     !distributable?
   end

diff --git a/app/serializers/oembed_serializer.rb b/app/serializers/oembed_serializer.rb
index 01689633b2d8ffa728639d4ea2cfe939862d5aab..d6261d72422b5dc94c0a3a8c371f80cd918eb180 100644 (file)
--- a/app/serializers/oembed_serializer.rb
+++ b/app/serializers/oembed_serializer.rb
@@ -4,7 +4,7 @@ class OEmbedSerializer < ActiveModel::Serializer
   include RoutingHelper
   include ActionView::Helpers::TagHelper
 
-  attributes :type, :version, :title, :author_name,
+  attributes :type, :version, :author_name,
              :author_url, :provider_name, :provider_url,
              :cache_age, :html, :width, :height
 

diff --git a/app/serializers/rss/account_serializer.rb b/app/serializers/rss/account_serializer.rb
index ee972ff9614ab7c967afd18e467f196d351d619c..81e24af0d0c5f893880383a52508fc294e64b053 100644 (file)
--- a/app/serializers/rss/account_serializer.rb
+++ b/app/serializers/rss/account_serializer.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class RSS::AccountSerializer
+class RSS::AccountSerializer < RSS::Serializer
   include ActionView::Helpers::NumberHelper
   include AccountsHelper
   include RoutingHelper
@@ -17,18 +17,7 @@ class RSS::AccountSerializer
     builder.image(full_asset_url(account.avatar.url(:original))) if account.avatar?
     builder.cover(full_asset_url(account.header.url(:original))) if account.header?
 
-    statuses.each do |status|
-      builder.item do |item|
-        item.title(status.title)
-            .link(ActivityPub::TagManager.instance.url_for(status))
-            .pub_date(status.created_at)
-            .description(status.spoiler_text.presence || Formatter.instance.format(status, inline_poll_options: true).to_str)
-
-        status.media_attachments.each do |media|
-          item.enclosure(full_asset_url(media.file.url(:original, false)), media.file.content_type, media.file.size)
-        end
-      end
-    end
+    render_statuses(builder, statuses)
 
     builder.to_xml
   end

diff --git a/app/serializers/rss/tag_serializer.rb b/app/serializers/rss/tag_serializer.rb
index ea26189a26a72a1fa32ff21813f6b2c7a1464131..e549ac3675ed0a03d40cd6f35f0b50dd79e18e4e 100644 (file)
--- a/app/serializers/rss/tag_serializer.rb
+++ b/app/serializers/rss/tag_serializer.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class RSS::TagSerializer
+class RSS::TagSerializer < RSS::Serializer
   include ActionView::Helpers::NumberHelper
   include ActionView::Helpers::SanitizeHelper
   include RoutingHelper
@@ -14,18 +14,7 @@ class RSS::TagSerializer
            .logo(full_pack_url('media/images/logo.svg'))
            .accent_color('2b90d9')
 
-    statuses.each do |status|
-      builder.item do |item|
-        item.title(status.title)
-            .link(ActivityPub::TagManager.instance.url_for(status))
-            .pub_date(status.created_at)
-            .description(status.spoiler_text.presence || Formatter.instance.format(status).to_str)
-
-        status.media_attachments.each do |media|
-          item.enclosure(full_asset_url(media.file.url(:original, false)), media.file.content_type, media.file.size)
-        end
-      end
-    end
+    render_statuses(builder, statuses)
 
     builder.to_xml
   end

diff --git a/spec/lib/rss/serializer_spec.rb b/spec/lib/rss/serializer_spec.rb
new file mode 100644 (file)
index 0000000..0364d13
--- /dev/null
+++ b/spec/lib/rss/serializer_spec.rb
@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe RSS::Serializer do
+  describe '#status_title' do
+    let(:text)      { 'This is a toot' }
+    let(:spoiler)   { '' }
+    let(:sensitive) { false }
+    let(:reblog)    { nil }
+    let(:account)   { Fabricate(:account) }
+    let(:status)    { Fabricate(:status, account: account, text: text, spoiler_text: spoiler, sensitive: sensitive, reblog: reblog) }
+
+    subject { RSS::Serializer.new.send(:status_title, status) }
+
+    context 'if destroyed?' do
+      it 'returns "#{account.acct} deleted status"' do
+        status.destroy!
+        expect(subject).to eq "#{account.acct} deleted status"
+      end
+    end
+
+    context 'on a toot with long text' do
+      let(:text) { "This toot's text is longer than the allowed number of characters" }
+
+      it 'truncates toot text appropriately' do
+        expect(subject).to eq "#{account.acct}: “This toot's text is longer tha…”"
+      end
+    end
+
+    context 'on a toot with long text with a newline' do
+      let(:text) { "This toot's text is longer\nthan the allowed number of characters" }
+
+      it 'truncates toot text appropriately' do
+        expect(subject).to eq "#{account.acct}: “This toot's text is longer…”"
+      end
+    end
+
+    context 'on a toot with a content warning' do
+      let(:spoiler) { 'long toot' }
+
+      it 'displays spoiler text instead of toot content' do
+        expect(subject).to eq "#{account.acct}: CW “long toot”"
+      end
+    end
+
+    context 'on a toot with sensitive media' do
+      let(:sensitive) { true }
+
+      it 'displays that the media is sensitive' do
+        expect(subject).to eq "#{account.acct}: “This is a toot” (sensitive)"
+      end
+    end
+
+    context 'on a reblog' do
+      let(:reblog) { Fabricate(:status, text: 'This is a toot') }
+
+      it 'display that the toot is a reblog' do
+        expect(subject).to eq "#{account.acct} boosted #{reblog.account.acct}: “This is a toot”"
+      end
+    end
+  end
+end

diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb
index 51a10cd1776f2acad4cf3387b49c8cc1b970f824..9c1eca1b5f3e18cae15877bf269096c08e196f53 100644 (file)
--- a/spec/models/status_spec.rb
+++ b/spec/models/status_spec.rb
@@ -82,35 +82,6 @@ RSpec.describe Status, type: :model do
     end
   end
 
-  describe '#title' do
-    # rubocop:disable Style/InterpolationCheck
-
-    let(:account) { subject.account }
-
-    context 'if destroyed?' do
-      it 'returns "#{account.acct} deleted status"' do
-        subject.destroy!
-        expect(subject.title).to eq "#{account.acct} deleted status"
-      end
-    end
-
-    context 'unless destroyed?' do
-      context 'if reblog?' do
-        it 'returns "#{account.acct} shared a status by #{reblog.account.acct}"' do
-          reblog = subject.reblog = other
-          expect(subject.title).to eq "#{account.acct} shared a status by #{reblog.account.acct}"
-        end
-      end
-
-      context 'unless reblog?' do
-        it 'returns "New status by #{account.acct}"' do
-          subject.reblog = nil
-          expect(subject.title).to eq "New status by #{account.acct}"
-        end
-      end
-    end
-  end
-
   describe '#hidden?' do
     context 'if private_visibility?' do
       it 'returns true' do