Refactor RSS view renderer

In issue #1815, it was reported that Gollum RSS feeds attempt to put
entire commit messages into a feed item's `<title>` element.

Given that commit messages can be many paragraphs long, this is not an
acceptable way to render a feed item title.

If a commit has many lines or paragraphs, we now put those inside the
`<description>` element as of this commit.

While I was editing this view, I decided it would be a good time to
increase test coverage, as this view renderer was not under test at
all. I also fixed a typo ("Commited" should say "Committed"). There are
implicit dependencies on other gems that provide the
`Gollum::Git::Commit`, `Gollum::Git::Actor`, and `Rugged::Commit`
classes here, so mocking out simple versions of their interfaces seemed
like the path of least resistance to setting up controllable tests.

I also removed some now-duplicated assertions from `test/test_app.rb`.

lib/gollum/views/rss.rb

@@ -1,43 +1,84 @@
 require 'rss'
 
 class RSSView
-  
   include Precious::Views::AppHelpers
   include Precious::Views::RouteHelpers
-  
+
   attr_reader :base_url
-  
+
   def initialize(base_url, wiki_title, url, changes)
     @base_url = base_url
     @wiki_title = wiki_title
     @url = url
     @changes = changes
   end
-  
+
   def render
-    latest_changes = "#{@url}#{latest_changes_path}"
     RSS::Maker.make('2.0') do |maker|
       maker.channel.author = 'Gollum Wiki'
-      maker.channel.updated = @changes.first.authored_date
-      maker.channel.title = "#{@wiki_title} Latest Changes"
       maker.channel.description = "Latest Changes in #{@wiki_title}"
-      maker.channel.link = latest_changes
+      maker.channel.link = latest_changes_url
+      maker.channel.title = "#{@wiki_title} Latest Changes"
+      maker.channel.updated = @changes.first.authored_date
+
       @changes.each do |change|
         maker.items.new_item do |item|
-          item.link = latest_changes
+          item.description = feed_item_description(change)
-          item.title = change.message
+          item.link = latest_changes_url
+          item.title = feed_item_title(change)
           item.updated = change.authored_date
-          id = change.id
-          files = change.stats.files.map do |files|
-            [files[:old_file], files[:new_file]].compact.map do |file|
-              f = extract_page_dir(file)
-              "<li><a href=\"#{@url}#{page_route(f)}/#{id}\">#{f}</a></li>"
-            end
-          end
-          item.description = "Commited by: <a href=\"mailto:#{change.author.email}\">#{change.author.name}</a><br/>Commit ID: #{id[0..6]}<br/><br/>Affected files:<ul>#{files.join}</ul>"
         end
       end
     end.to_s
   end
 
-end
+  private
+
+  def feed_item_commit_body(change)
+    body = change.message.lines[1..-1].join
+    body = body.split(/\n{2}/).map { |paragraph| "<p>#{paragraph}</p>" }.join
+    body.gsub!(/\n/, ' ')
+    body
+  end
+
+  def feed_item_description(change)
+    ERB.new(<<~HTML_PARTIAL)
+      <%= feed_item_commit_body(change) %>
+      Committed by: <a href="mailto:<%= change.author.email %>">
+        <%= change.author.name %>
+      </a><br />
+      Commit ID: <%= change.id[0..6] %><br /><br />
+      <%= feed_item_files(change) %>
+    HTML_PARTIAL
+      .result(binding)
+  end
+
+  def feed_item_files(change)
+    file_list = change.stats.files.map { |change_files|
+      [
+        change_files[:old_file],
+        change_files[:new_file]
+      ].compact
+    }
+
+    ERB.new(<<~HTML_PARTIAL)
+      Affected files: <ul>
+        <% file_list.each do |change_files| %>
+          <% change_files.each do |file| %>
+            <% file_href = "%s%s/%s" % [@url, page_route(file), change.id] %>
+            <li><a href="<%= file_href %>"><%= file %></a></li>
+          <% end %>
+        <% end %>
+      </ul>
+    HTML_PARTIAL
+      .result(binding)
+  end
+
+  def feed_item_title(change)
+    change.message.lines.first.strip
+  end
+
+  def latest_changes_url
+    "%s%s" % [@url, latest_changes_path]
+  end
+end

test/test_app.rb

@@ -63,18 +63,10 @@ context "Frontend" do
   end
 
   test 'rss feed' do
-    channel_title = <<EOF
-<title>Gollum Wiki Latest Changes</title>
-EOF
-    item = <<EOF
-<description>Commited by: &lt;a href=&quot;mailto:dawa.ometto@phil.uu.nl&quot;&gt;Dawa Ometto&lt;/a&gt;&lt;br/&gt;Commit ID: 02796b1&lt;br/&gt;&lt;br/&gt;Affected files:&lt;ul&gt;&lt;li&gt;&lt;a href=&quot;http://example.org/custom.css/02796b1450691f90db5d6dc6a816a4980ce80d07&quot;&gt;custom.css&lt;/a&gt;&lt;/li&gt;&lt;li&gt;&lt;a href=&quot;http://example.org/custom.js/02796b1450691f90db5d6dc6a816a4980ce80d07&quot;&gt;custom.js&lt;/a&gt;&lt;/li&gt;&lt;/ul&gt;</description>
-EOF
     get '/gollum/feed/'
+
     assert last_response.ok?
     assert_equal 'application/rss+xml', last_response.headers['Content-Type']
-    assert last_response.body.start_with?('<?xml')
-    assert last_response.body.include?(item)
-    assert last_response.body.include?(channel_title)
   end
 
   test "show sidebar, header, footer when present" do
@@ -477,7 +469,7 @@ EOF
     assert_equal 'abc', file.raw_data
     Precious::App.set(:wiki_options, {allow_uploads: false})
   end
-  
+
   test "upload a file with mode page" do
     temp_upload_file = Tempfile.new(['upload', '.file']) << "abc\r"
     temp_upload_file.close
@@ -491,7 +483,7 @@ EOF
     assert_equal "abc\r", file.raw_data
     Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})
   end
-  
+
   test "upload a file with valid extension" do
     temp_upload_file = Tempfile.new(['upload', '.txt']) << "abc\r"
     temp_upload_file.close
@@ -505,7 +497,7 @@ EOF
     assert_equal "abc", file.raw_data
     Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})
   end
-  
+
   test 'upload a file with mode page from the edit page (drag and drop)' do
     temp_upload_file = Tempfile.new(['upload', '.file']) << "abc\r"
     temp_upload_file.close
@@ -517,9 +509,9 @@ EOF
     # Find the file in a page-specific subdir (here: foo/Bar), based on referer
     file = @wiki.file("uploads/foo/Bar/#{::File.basename(temp_upload_file.path)}")
     assert_equal "abc\r", file.raw_data
-    Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})    
+    Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})
   end
-  
+
   test "upload a file with https referer" do
     temp_upload_file = Tempfile.new(['https_upload', '.file']) << 'abc'
     temp_upload_file.close
@@ -1066,7 +1058,7 @@ context 'Frontend with base path' do
     assert last_response.ok?
     assert_equal '/wiki/gollum/history/Bilbo-Baggins.md', last_request.fullpath
   end
-  
+
   test 'upload a file with mode page from the edit page (drag and drop)' do
     temp_upload_file = Tempfile.new(['upload', '.file']) << "abc\r"
     temp_upload_file.close
@@ -1078,9 +1070,9 @@ context 'Frontend with base path' do
     # Find the file in a page-specific subdir (here: foo/Bar), based on referer
     file = @wiki.file("uploads/foo/Bar/#{::File.basename(temp_upload_file.path)}")
     assert_equal "abc\r", file.raw_data
-    Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})    
+    Precious::App.set(:wiki_options, {allow_uploads: false, per_page_uploads: false})
   end
-  
+
   def app
     Precious::MapGollum.new(@base_path)
   end
@@ -1148,4 +1140,4 @@ context "Default keybindings" do
   def app
     Precious::App
   end
-end
+end

test/test_rss_view.rb

@@ -0,0 +1,101 @@
+require_relative "helper"
+
+context "Precious::Views::RSS" do
+  # Simplisticially mimics a `Gollum::Git::Actor` object.
+  #
+  MockAuthor = Struct.new(:name, :email)
+
+  # Simplistically mimics a `Gollum::Git::Commit` object.
+  #
+  MockChange = Class.new do
+    def author
+      MockAuthor.new("committer name", "email@example.com")
+    end
+
+    def authored_date
+      Time.new(1999, 01, 01, 0, 0)
+    end
+
+    def files
+      ["file 1", "file 2"]
+    end
+
+    def id
+      "f0f0f0f0"
+    end
+
+    def message
+      <<~COMMIT_MESSAGE
+        Multi-line commit message
+
+        This commit is multiple lines long so we can test how this is
+        rendered in the feed.
+
+        Git's documentation says that the first line of a commit should
+        be 50 characters or fewer, and the rest of the commit body's
+        lines should not exceed 72 characters in length.
+      COMMIT_MESSAGE
+    end
+
+    def stats
+      OpenStruct.new(files: [{old_file: "old", new_file: "new"}])
+    end
+  end
+
+  test "renders a valid RSS feed" do
+    feed = RSSView.new(
+      "/",
+      "Wiki Name",
+      "https://example.com",
+      [MockChange.new]
+    ).render
+
+    # Assert that we have required RSS feed elements.
+    #
+    assert_match "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",              feed
+    assert_match /<rss version=\"2.0\"(.*)<\/rss>/m,                        feed
+    assert_match /<channel>(.*)<\/channel>/m,                               feed
+
+    # Assert that we have feed metadata.
+    #
+    assert_match "<title>Wiki Name Latest Changes</title>",                 feed
+    assert_match "<link>https://example.com/gollum/latest_changes</link>",  feed
+    assert_match "<description>Latest Changes in Wiki Name</description>",  feed
+    assert_match /<pubDate>(.*)<\/pubDate>/,                                feed
+
+    # Assert that we have an item in our feed.
+    #
+    assert_match /<item>(.*)<\/item>/m,                                     feed
+
+    # And it has a title.
+    #
+    assert_match "<title>Multi-line commit message</title>",                feed
+
+    # Assert that the description contains expected content.
+    #
+    assert_match /<description>(.*)<\/description>/m,                       feed
+    assert_match /&lt;p&gt; This commit(.*)&lt;\/p&gt;/,                    feed
+    assert_match /&lt;p&gt;Git's documentation(.*)&lt;\/p&gt;/,             feed
+
+    # Assert that the description contains information about the commit.
+    # i.e.:
+    #
+    #  <a href="mailto:email@example.com">committer name</a>
+    #
+    #  Commit ID: f0f0f0f0
+    #
+    assert_match /Committed by: /,                                          feed
+    assert_match /\&lt;a href=\&quot;mailto:email@example.com\&quot;\&gt;/, feed
+    assert_match /\&gt;\n  committer name\n\&lt;\/a&gt;/,                   feed
+    assert_match "Commit ID: f0f0f0f",                                      feed
+
+    # Assert that affected files include links to commits, i.e.:
+    #
+    #  <a href="https://example.com/old/f0f0f0f0">new</a>
+    #
+    assert_match /Affected files: /,                                        feed
+    assert_match /\&lt;a href=\&quot;https:\/\/example.com\/old\/f0f0f0f0\&quot;/,
+                                                                            feed
+    assert_match /f0f0f0f0&quot;&gt;new&lt;\/a&gt;/,                        feed
+  end
+end