Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate description for shows with Apple feeds #1092

Merged
merged 14 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions app/helpers/podcasts_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,18 @@ module PodcastsHelper

RSS_LANGUAGE_CODES = %w[af sq eu be bg ca zh-cn zh-tw hr cs da nl nl-be nl-nl en en-au en-bz en-ca en-ie en-jm en-nz en-ph en-za en-tt en-gb en-us en-zw et fo fi fr fr-be fr-ca fr-fr fr-lu fr-mc fr-ch gl gd de de-at de-de de-li de-lu de-ch el haw hu is in ga it it-it it-ch ja ko mk no pl pt pt-br pt-pt ro ro-mo ro-ro ru ru-mo ru-ru sr sk sl es es-ar es-bo es-cl es-co es-cr es-do es-ec es-sv es-gt es-hn es-mx es-ni es-pa es-py es-pe es-pr es-es es-uy es-ve sv sv-fi sv-se tr uk]

def feed_description(feed, podcast)
[feed.description, podcast.description].detect { |d| d.present? } || ""
end

def episode_description(episode)
if episode.podcast.has_apple_feed?
episode.description_safe
else
episode.description_with_default
end
end
svevang marked this conversation as resolved.
Show resolved Hide resolved

def full_contact(type, item)
name = item.try("#{type}_name")
email = item.try("#{type}_email")
Expand Down
2 changes: 1 addition & 1 deletion app/models/apple/episode.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ def episode_create_parameters
guid: guid,
title: feeder_episode.title,
originalReleaseDate: feeder_episode.published_at.utc.iso8601,
description: feeder_episode.description_with_default,
description: feeder_episode.description_safe,
svevang marked this conversation as resolved.
Show resolved Hide resolved
websiteUrl: feeder_episode.url,
explicit: explicit,
episodeNumber: feeder_episode.episode_number,
Expand Down
10 changes: 8 additions & 2 deletions app/models/episode.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class Episode < ApplicationRecord
include ReleaseEpisodes

MAX_SEGMENT_COUNT = 10
MAX_DESCRIPTION_BYTES = 4000
svevang marked this conversation as resolved.
Show resolved Hide resolved
VALID_ITUNES_TYPES = %w[full trailer bonus]
DROP_DATE = "COALESCE(episodes.published_at, episodes.released_at)"

Expand All @@ -41,7 +42,7 @@ class Episode < ApplicationRecord

validates :podcast_id, :guid, presence: true
validates :title, presence: true
validates :description, bytesize: {maximum: 4000}, if: -> { strict_validations && description_changed? }
validates :description, bytesize: {maximum: MAX_DESCRIPTION_BYTES}, if: -> { strict_validations && description_changed? }
validates :url, http_url: true
validates :original_guid, presence: true, uniqueness: {scope: :podcast_id}, allow_nil: true
alias_error_messages :item_guid, :original_guid
Expand Down Expand Up @@ -309,8 +310,13 @@ def sanitize_text
self.original_guid = original_guid.strip if !original_guid.blank? && original_guid_changed?
end

# This value is safe to use in the RSS and integrations
def description_with_default
description || subtitle || title || ""
[description, subtitle, title].detect { |d| d.present? } || ""
end

def description_safe
description_with_default.truncate_bytes(MAX_DESCRIPTION_BYTES, omission: "")
end

def feeder_cdn_host
Expand Down
1 change: 1 addition & 0 deletions app/models/feed.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class Feed < ApplicationRecord
validates :enclosure_prefix, http_url: true
validates :display_episodes_count, numericality: {only_integer: true, greater_than: 0}, allow_nil: true
validates :display_full_episodes_count, numericality: {only_integer: true, greater_than: 0}, allow_nil: true
validates :description, bytesize: {maximum: Episode::MAX_DESCRIPTION_BYTES}
svevang marked this conversation as resolved.
Show resolved Hide resolved

after_initialize :set_defaults
before_validation :sanitize_text
Expand Down
6 changes: 6 additions & 0 deletions app/models/feeds/apple_subscription.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ class Feeds::AppleSubscription < Feed

after_initialize :set_defaults

after_create :publish_feeds

has_one :apple_config, class_name: "::Apple::Config", dependent: :destroy, autosave: true, validate: true, inverse_of: :feed

accepts_nested_attributes_for :apple_config, allow_destroy: true, reject_if: :all_blank
Expand All @@ -31,6 +33,10 @@ def self.model_name
Feed.model_name
end

def publish_feeds
podcast&.publish!
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per suggestion, consolidating the "getting ready to publish to Apple" sort of logic in this class.

I wish I could just trigger a publish of the feed without even trying to publish to apple when this first gets created, and before we want to try and sync with apple...maybe I will add a way to do that, but not sure how to decouple those in the current job?

end

def unchanged_defaults
return unless persisted?

Expand Down
4 changes: 4 additions & 0 deletions app/models/podcast.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ def apple_config
end
end

def has_apple_feed?
feeds.apple.exists?
svevang marked this conversation as resolved.
Show resolved Hide resolved
end

def reload(options = nil)
remove_instance_variable(:@apple_config) if defined?(@apple_config)
super
Expand Down
8 changes: 2 additions & 6 deletions app/views/podcasts/show.rss.builder
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ xml.rss "xmlns:atom" => "http://www.w3.org/2005/Atom",
xml.copyright @podcast.copyright unless @podcast.copyright.blank?
xml.webMaster @podcast.web_master unless @podcast.web_master.blank?

if @feed.description.present?
xml.description { xml.cdata!(@feed.description) }
elsif @podcast.description.present?
xml.description { xml.cdata!(@podcast.description) }
end
xml.description { xml.cdata!(feed_description(@feed, @podcast)) }

xml.managingEditor @podcast.managing_editor unless @podcast.managing_editor.blank?

Expand Down Expand Up @@ -112,7 +108,7 @@ xml.rss "xmlns:atom" => "http://www.w3.org/2005/Atom",
xml.title(ep.title)
xml.pubDate ep.published_at.utc.rfc2822
xml.link ep.url || ep.enclosure_url(@feed)
xml.description { xml.cdata!(ep.description_with_default) }
xml.description { xml.cdata!(episode_description(ep)) }
# TODO: may not reflect the content_type/file_size of replaced media
xml.enclosure(url: ep.enclosure_url(@feed), type: ep.media_content_type(@feed), length: ep.media_file_size) if ep.media?

Expand Down
19 changes: 19 additions & 0 deletions test/models/episode_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,25 @@
assert e.valid?
end

it "has a safe description for integrations" do
e = build_stubbed(:episode, segment_count: 2, published_at: nil, strict_validations: true)
e.description = "a" * 4001
assert e.description.bytesize == 4001
assert e.description_safe.bytesize == 4000
end

it "has a description with fallbacks" do
e = build_stubbed(:episode, segment_count: 2, published_at: nil, strict_validations: true)
e.title = "title"
e.subtitle = nil
e.description = ""
assert e.description_with_default == "title"
e.subtitle = "sub"
assert e.description_with_default == "sub"
e.description = "desc"
assert e.description_with_default == "desc"
end

it "validates unique original guids" do
e1 = create(:episode, original_guid: "original")
e2 = build(:episode, original_guid: "original", podcast: e1.podcast)
Expand Down
Loading