Merge pull request #1114 from PRX/fix/summary_format

Deprecate summary and content attributes, and fix description, summary, content text formats in the RSS feeds

app/controllers/episodes_controller.rb

@@ -141,8 +141,6 @@ def episode_params
       :title,
       :clean_title,
       :subtitle,
-      :summary,
-      :content,
       :description,
       :production_notes,
       :explicit_option,

app/controllers/feeds_controller.rb

@@ -124,7 +124,6 @@ def nilified_feed_params
       :file_name,
       :title,
       :subtitle,
-      :summary,
       :description,
       :include_donation_url,
       :include_podcast_value,

app/controllers/podcasts_controller.rb

@@ -166,7 +166,6 @@ def podcast_params
       default_feed_attributes: [
         :id,
         :subtitle,
-        :summary,
         :description,
         itunes_category: [],
         itunes_subcategory: [],

app/helpers/podcasts_helper.rb

@@ -33,15 +33,11 @@ def first_nonblank(type, items)
   end
 
   def show_itunes_summary?(model)
-    model.summary.present? || model.description.present?
+    model.description.present?
   end
 
   def itunes_summary(model)
-    if model.summary.present?
-      model.summary
-    else
-      sanitize_links_only(model.description || "")
-    end
+    sanitize_links_only(model.description || "")
   end
 
   def podcast_settings_active?

app/models/concerns/import_utils.rb

@@ -2,8 +2,10 @@
 require "prx_access"
 require "net/http"
 require "uri"
+require "text_sanitizer"
 
 module ImportUtils
+  include TextSanitizer
   extend ActiveSupport::Concern
 
   class HttpError < StandardError
@@ -110,9 +112,7 @@ def remove_podcastchoices_link(str)
   end
 
   def sanitize_html(text)
-    return nil if text.blank?
-    sanitizer = Rails::Html::WhiteListSanitizer.new
-    sanitizer.sanitize(Loofah.fragment(text).scrub!(:prune).to_s).strip
+    sanitize_white_list(text)
   end
 
   def files_match?(file, url)

app/models/concerns/text_sanitizer.rb

@@ -7,20 +7,25 @@ module TextSanitizer
   def sanitize_white_list(text)
     return nil if text.blank?
     sanitizer = Rails::Html::WhiteListSanitizer.new
-    sanitizer.sanitize(Loofah.fragment(text).scrub!(:prune).to_s)
+    text = sanitizer.sanitize(Loofah.fragment(text).scrub!(:prune).to_s)
+    clean_whitespace(text)
   end
 
   def sanitize_links_only(text)
     return nil if text.blank?
     scrubber = Rails::Html::PermitScrubber.new
     scrubber.tags = %w[a]
     scrubber.attributes = %w[href target nofollow]
-    Loofah.fragment(text).scrub!(:prune).scrub!(scrubber).to_s
+    text = add_newlines_to_tags(text)
+    text = Loofah.fragment(text).scrub!(:prune).scrub!(scrubber).to_s
+    clean_whitespace(text)
   end
 
   def sanitize_text_only(text)
     return nil if text.blank?
-    Loofah.fragment(text).scrub!(:prune).text(encode_special_chars: false)
+    text = add_newlines_to_tags(text)
+    text = Loofah.fragment(text).scrub!(:prune).text(encode_special_chars: false)
+    clean_whitespace(text)
   end
 
   def sanitize_categories(kws, strict)
@@ -37,4 +42,21 @@ def sanitize_category(kw, max_length, strict)
       kw.strip.slice(0, max_length)
     end
   end
+
+  def add_newlines_to_tags(text)
+    text.gsub(/<p>/i, "\n<p>")
+      .gsub(/<\/p>/i, "</p>\n")
+      .gsub(/<div>/i, "\n<div>")
+      .gsub(/<\/div>/i, "</div>\n")
+      .gsub(/<br>/i, "\n<br>")
+      .gsub(/<br\s*\/>/i, "\n<br>")
+  end
+
+  def clean_whitespace(text)
+    text.gsub(/\R/, "\n") # if there is any kind of newline, make it \n
+      .gsub(/([ \t]*)\n([ \t]*)/, "\n") # get rid of whitespace on eother side of a newline
+      .gsub(/\n\n\n/, "\n\n") # no more than 2 newlines together
+      .squeeze(" ") # make all groups of spaces into a siingle space
+      .strip # get rid of leading or trailing spaces
+  end
 end

app/models/episode.rb

@@ -304,9 +304,7 @@ def set_external_keyword
 
   def sanitize_text
     self.description = sanitize_white_list(description) if description_changed?
-    self.content = sanitize_white_list(content) if content_changed?
     self.subtitle = sanitize_text_only(subtitle) if subtitle_changed?
-    self.summary = sanitize_links_only(summary) if summary_changed?
     self.title = sanitize_text_only(title) if title_changed?
     self.original_guid = original_guid.strip if !original_guid.blank? && original_guid_changed?
   end

app/models/feed.rb

@@ -78,7 +78,6 @@ def set_defaults
   def sanitize_text
     self.description = sanitize_white_list(description) if description_changed?
     self.subtitle = sanitize_text_only(subtitle) if subtitle_changed?
-    self.summary = sanitize_links_only(summary) if summary_changed?
     self.title = sanitize_text_only(title) if title_changed?
   end
 

app/models/imports/episode_rss_import.rb

@@ -83,9 +83,6 @@ def update_episode_with_entry!
     episode.subtitle = clean_string(episode_short_desc(entry))
     episode.title = clean_title(entry[:title])
 
-    if entry[:itunes_summary] && entry_description_attribute(entry) != :itunes_summary
-      episode.summary = clean_text(entry[:itunes_summary])
-    end
     episode.author = person(entry[:itunes_author] || entry[:author] || entry[:creator])
     episode.block = (clean_string(entry[:itunes_block]) == "yes")
     episode.explicit = explicit(entry[:itunes_explicit])
@@ -110,7 +107,7 @@ def entry_description(entry)
   end
 
   def entry_description_attribute(entry)
-    [:content, :itunes_summary, :description, :title].find { |d| !entry[d].blank? }
+    [:content, :description, :itunes_summary, :itunes_subtitle, :title].find { |d| !entry[d].blank? }
   end
 
   def title

app/models/imports/podcast_rss_import.rb

@@ -155,7 +155,7 @@ def create_or_update_episode_imports!
   end
 
   def feed_description
-    result = [channel[:itunes_summary], channel[:description]].find { |d| !d.blank? }
+    result = [channel[:description], channel[:itunes_summary]].find { |d| !d.blank? }
     clean_text(result)
   end
 
@@ -185,7 +185,6 @@ def build_podcast_attributes
       podcast_attributes[atr.to_sym] = clean_string(channel[atr])
     end
 
-    podcast_attributes[:summary] = clean_text(channel[:itunes_summary])
     podcast_attributes[:link] = clean_string(channel[:url])
     podcast_attributes[:explicit] = explicit(channel[:itunes_explicit], "false")
     podcast_attributes[:new_feed_url] = clean_string(channel[:itunes_new_feed_url])

app/models/podcast.rb

@@ -1,7 +1,7 @@
 require "text_sanitizer"
 
 class Podcast < ApplicationRecord
-  FEED_ATTRS = %i[subtitle description summary url new_feed_url display_episodes_count
+  FEED_ATTRS = %i[subtitle description url new_feed_url display_episodes_count
     display_full_episodes_count enclosure_prefix enclosure_template feed_image itunes_image
     ready_feed_image ready_itunes_image ready_image itunes_category itunes_subcategory itunes_categories]
   FEED_GETTERS = FEED_ATTRS.map { |s| [s, "#{s}_was".to_sym, "#{s}_changed?".to_sym] }.flatten

app/representers/api/auth/feed_representer.rb

@@ -9,7 +9,6 @@ class Api::Auth::FeedRepresenter < Api::BaseRepresenter
   property :title
   property :subtitle
   property :description
-  property :summary
   property :url
   property :new_feed_url
   property :enclosure_prefix

app/representers/api/episode_representer.rb

@@ -23,8 +23,6 @@ class Api::EpisodeRepresenter < Api::BaseRepresenter
   property :clean_title
   property :subtitle
   property :description
-  property :content
-  property :summary
   property :season_number
   property :episode_number
   property :itunes_type

app/representers/api/podcast_representer.rb

@@ -18,7 +18,6 @@ class Api::PodcastRepresenter < Api::BaseRepresenter
   property :title
   property :subtitle
   property :description
-  property :summary
   property :itunes_block
 
   property :explicit

app/views/episodes/_form_main.html.erb

@@ -30,8 +30,6 @@
     </div>
     <div class="card-body">
       <div class="form-floating">
-        <%= form.hidden_field :summary, value: nil %>
-        <%= form.hidden_field :content, value: nil %>
         <%= form.trix_editor :description %>
       </div>
     </div>

app/views/feeds/_form_overrides.html.erb

@@ -16,7 +16,6 @@
 
         <div class="col-12 mb-4">
           <div class="form-floating input-group">
-            <%= form.hidden_field :summary, value: nil %>
             <%= form.trix_editor :description %>
             <%= form.label :description %>
             <%= field_help_text t(".help.description") %>

app/views/podcasts/_form_main.html.erb

@@ -25,7 +25,6 @@
     <div class="card-body">
       <div class="form-floating">
         <%= form.fields_for :default_feed do |fields| %>
-          <%= fields.hidden_field :summary, value: nil %>
           <%= fields.trix_editor :description %>
         <% end %>
       </div>

app/views/podcasts/show.rss.builder

@@ -155,7 +155,7 @@ xml.rss "xmlns:atom" => "http://www.w3.org/2005/Atom",
               url: ep.enclosure_url(@feed))
           end
 
-          xml.content(:encoded) { xml.cdata!(ep.content) } unless ep.content.blank?
+          xml.content(:encoded) { xml.cdata!(episode_description(ep)) }
         end
       end
     end

test/factories/episode_factory.rb

@@ -15,14 +15,6 @@
       "<div><a href=\"/tina\">Tina</a> McElroy Ansa is a little girl when her father's business goes under.</div>"
     end
 
-    content do
-      "<div><a href=\"/tina\">Tina</a> McElroy Ansa is a little girl when her father's business goes under.</div>"
-    end
-
-    summary do
-      "<a href=\"/tina\">Tina</a> McElroy Ansa is a little girl when her father's business goes under"
-    end
-
     factory :episode_with_media do
       audio_version { "One segment audio" }
       segment_count { 1 }

test/factories/feed_factory.rb

@@ -8,7 +8,6 @@
     title { "new feed" }
     subtitle { "Goofy laughsters" }
     description { "A goofy fun-time laughcast with doofuses" }
-    summary { "Public radio host Jesse Thorn and @midnight writer Jordan Morris goof around" }
 
     url { "http://feeds.feedburner.com/thornmorris" }
     new_feed_url { "http://feeds.feedburner.com/newthornmorris" }

test/models/concerns/text_sanitizer_test.rb

@@ -7,25 +7,37 @@ class TestSanitizer
 describe TextSanitizer do
   let(:model) { TestSanitizer.new }
   let(:text) do
-    '<!-- Sorry! --><p>my</p> <b>dog & cat</b> <a href="/">ate</a> ' \
-      "<span><div>my</div></span> <script>homework</script>"
+    "<!-- Sorry! --><p>my</p>" \
+    "     <b>dog & cat</b> <a href=\"/\">ate</a><span>" \
+    "<div>my</div>" \
+    "</span><script>homework</script>"
   end
 
   it "scrubs all tags" do
-    assert_equal model.sanitize_text_only(text), "my dog & cat ate my "
+    assert_equal model.sanitize_text_only(text), "my\ndog & cat ate\nmy"
   end
 
   it "leaves ampersands alone" do
     assert_equal model.sanitize_text_only("Us & Them"), "Us & Them"
   end
 
   it "scrubs all but links" do
-    r = 'my dog &amp; cat <a href="/">ate</a> my '
+    r = "my\ndog &amp; cat <a href=\"/\">ate</a>\nmy"
     assert_equal model.sanitize_links_only(text), r
   end
 
+  it "adds space near some tags" do
+    t = "0<p>1</p>2<br>3<br />4<br/>5<div>6</div>7"
+    assert_equal model.add_newlines_to_tags(t), "0\n<p>1</p>\n2\n<br>3\n<br>4\n<br>5\n<div>6</div>\n7"
+  end
+
+  it "adds space for removed tags" do
+    t = " 0<p>1</p>2<br>3<br />4<br/>5<div> 6 </div>7 "
+    assert_equal model.sanitize_links_only(t), "0\n1\n2\n3\n4\n5\n6\n7"
+  end
+
   it "scrubs all but white listed" do
-    r = '<p>my</p> <b>dog &amp; cat</b> <a href="/">ate</a> <span><div>my</div></span> '
+    r = '<p>my</p> <b>dog &amp; cat</b> <a href="/">ate</a><span><div>my</div></span>'
     assert_equal model.sanitize_white_list(text), r
   end
 
@@ -34,4 +46,9 @@ class TestSanitizer
       "<caption></caption>\n</tbody>\n<tfoot></tfoot>\n</table>"
     assert_equal model.sanitize_white_list(text), text
   end
+
+  it "cleans up white space" do
+    text = " text, \r \n \r more\n     \ntext, and \r   words\n \n \r\r \n"
+    assert_equal model.clean_whitespace(text), "text,\n\nmore\n\ntext, and\nwords"
+  end
 end

test/models/imports/podcast_rss_import_test.rb

@@ -53,11 +53,9 @@
     _(importer.podcast.title).must_equal "Transistor"
     _(importer.podcast.subtitle).must_equal "A podcast of scientific questions and " \
       "stories featuring guest hosts and reporters."
-    _(importer.podcast.description).must_equal "Transistor is a podcast of scientific curiosities " \
-      "and current events, featuring guest hosts, " \
-      "scientists, and story-driven reporters. Presented " \
-      "by radio and podcast powerhouse PRX, with support " \
-      "from the Sloan Foundation."
+    _(importer.podcast.description).must_equal "A podcast of scientific questions and stories," \
+        " with many episodes hosted by key scientists" \
+        " at the forefront of discovery."
 
     _(importer.podcast.url).must_equal "http://feeds.prx.org/transistor_stem"
     _(importer.podcast.new_feed_url).must_equal "http://feeds.prx.org/transistor_stem"

test/representers/api/auth/feed_representer_test.rb

@@ -10,7 +10,6 @@
     _(json["slug"]).must_match(/myfeed(\d+)/)
     _(json["subtitle"]).must_equal feed.subtitle
     _(json["description"]).must_equal feed.description
-    _(json["summary"]).must_equal feed.summary
   end
 
   it "has links" do

test/representers/api/episode_representer_test.rb

@@ -7,7 +7,7 @@
 
   it "includes basic properties" do
     assert_match(%r{/api/v1/stories/}, json["prxUri"])
-    assert_match(%r{<a href="/tina">Tina</a>}, json["summary"])
+    assert_match(%r{<a href="/tina">Tina</a>}, json["description"])
   end
 
   it "includes clean title, season, episode, and ep type info" do
@@ -30,17 +30,8 @@
     assert_equal json["explicitContent"], true
   end
 
-  it "uses summary when not blank" do
-    episode.summary = 'summary has <a href="/">a link</a>'
+  it "uses sanitized description" do
     episode.description = '<b>tags</b> removed, <a href="/">links remain</a>'
-    assert_equal json["summary"], episode.summary
-    assert_equal json["description"], episode.description
-  end
-
-  it "uses sanitized description for nil summary" do
-    episode.summary = nil
-    episode.description = '<b>tags</b> removed, <a href="/">links remain</a>'
-    assert_nil json["summary"]
     assert_equal json["description"], episode.description
   end