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.
This commit is contained in:
ThibG 2020-05-10 09:50:54 +02:00 committed by GitHub
parent 73f3842284
commit a4240fd027
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 106 additions and 64 deletions

38
app/lib/rss/serializer.rb Normal file
View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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