FEATURE: Include post number in inline Onebox titles (#11515)

config/locales/server.en.yml

@@ -59,6 +59,9 @@ en:
   redirect_warning: "We were unable to verify that the link you selected was actually posted to the forum. If you wish to proceed anyway, select the link below."
   on_another_topic: "On another topic"
 
+  inline_oneboxer:
+    topic_page_title_post_number: "#%{post_number}"
+    topic_page_title_post_number_by_user: "#%{post_number} by %{username}"
   themes:
     bad_color_scheme: "Can not update theme, invalid color palette"
     other_error: "Something went wrong updating theme"
@@ -210,8 +213,8 @@ en:
       invalid_address: "Sorry, we were unable to generate a preview for this web page, because the server '%{hostname}' could not be found. Instead of a preview, only a link will appear in your post. :cry:"
       error_response: "Sorry, we were unable to generate a preview for this web page, because the web server returned an error code of %{status_code}. Instead of a preview, only a link will appear in your post. :cry:"
       missing_data:
-        one:  "Sorry, we were unable to generate a preview for this web page, because the following oEmbed / OpenGraph tag could not be found: %{missing_attributes}"
+        one: "Sorry, we were unable to generate a preview for this web page, because the following oEmbed / OpenGraph tag could not be found: %{missing_attributes}"
-        other:  "Sorry, we were unable to generate a preview for this web page, because the following oEmbed / OpenGraph tags could not be found: %{missing_attributes}"
+        other: "Sorry, we were unable to generate a preview for this web page, because the following oEmbed / OpenGraph tags could not be found: %{missing_attributes}"
 
   word_connector:
     # Connects words with a comma. Example: "foo, bar"

lib/inline_oneboxer.rb

@@ -36,6 +36,11 @@ class InlineOneboxer
       if route[:controller] == "topics"
         if topic = Oneboxer.local_topic(url, route, opts)
           opts[:skip_cache] = true
+          post_number = [route[:post_number].to_i, topic.highest_post_number].min
+          if post_number > 1
+            opts[:post_number] = post_number
+            opts[:post_author] = post_author_for_title(topic, post_number)
+          end
           return onebox_for(url, topic.title, opts)
         end
       end
@@ -65,6 +70,22 @@ class InlineOneboxer
   private
 
   def self.onebox_for(url, title, opts)
+    title = title && Emoji.gsub_emoji_to_unicode(title)
+    if title && opts[:post_number]
+      title += " - "
+      if opts[:post_author]
+        title += I18n.t(
+          "inline_oneboxer.topic_page_title_post_number_by_user",
+          post_number: opts[:post_number],
+          username: opts[:post_author]
+        )
+      else
+        title += I18n.t(
+          "inline_oneboxer.topic_page_title_post_number",
+          post_number: opts[:post_number]
+        )
+      end
+    end
     onebox = { url: url, title: title && Emoji.gsub_emoji_to_unicode(title) }
     Discourse.cache.write(cache_key(url), onebox, expires_in: 1.day) if !opts[:skip_cache]
     onebox
@@ -74,4 +95,12 @@ class InlineOneboxer
     "inline_onebox:#{url}"
   end
 
+  def self.post_author_for_title(topic, post_number)
+    guardian = Guardian.new
+    post = topic.posts.find_by(post_number: post_number)
+    author = post&.user
+    if author && guardian.can_see_post?(post) && post.post_type == Post.types[:regular]
+      author.username
+    end
+  end
 end

lib/topic_view.rb

@@ -178,6 +178,23 @@ class TopicView
 
   def page_title
     title = @topic.title
+    if @post_number > 1
+      title += " - "
+      post = @topic.posts.find_by(post_number: @post_number)
+      author = post&.user
+      if author && @guardian.can_see_post?(post)
+        title += I18n.t(
+          "inline_oneboxer.topic_page_title_post_number_by_user",
+          post_number: @post_number,
+          username: author.username
+        )
+      else
+        title += I18n.t(
+          "inline_oneboxer.topic_page_title_post_number",
+          post_number: @post_number
+        )
+      end
+    end
     if SiteSetting.topic_page_title_includes_category
       if @topic.category_id != SiteSetting.uncategorized_category_id && @topic.category_id && @topic.category
         title += " - #{@topic.category.name}"

spec/components/inline_oneboxer_spec.rb

@@ -116,6 +116,37 @@ describe InlineOneboxer do
       expect(onebox[:title]).to eq("Hello 🍕 with an emoji")
     end
 
+    it "will append the post number post auther's username to the title" do
+      topic = Fabricate(:topic, title: "Inline oneboxer")
+      Fabricate(:post, topic: topic) # OP
+      Fabricate(:post, topic: topic)
+      lookup = -> (number) do
+        InlineOneboxer.lookup(
+          "#{topic.url}/#{number}",
+          skip_cache: true
+        )[:title]
+      end
+      posts = topic.reload.posts.order("post_number ASC")
+
+      expect(lookup.call(0)).to eq("Inline oneboxer")
+      expect(lookup.call(1)).to eq("Inline oneboxer")
+      expect(lookup.call(2)).to eq("Inline oneboxer - #2 by #{posts[1].user.username}")
+
+      Fabricate(:post, topic: topic, post_type: Post.types[:whisper])
+      posts = topic.reload.posts.order("post_number ASC")
+      # because the last post in the topic is a whisper, the onebox title
+      # will be the first regular post directly before our whisper
+      expect(lookup.call(3)).to eq("Inline oneboxer - #2 by #{posts[1].user.username}")
+      expect(lookup.call(99)).to eq("Inline oneboxer - #2 by #{posts[1].user.username}")
+
+      Fabricate(:post, topic: topic)
+      posts = topic.reload.posts.order("post_number ASC")
+      # username not appended to whisper posts
+      expect(lookup.call(3)).to eq("Inline oneboxer - #3")
+      expect(lookup.call(4)).to eq("Inline oneboxer - #4 by #{posts[3].user.username}")
+      expect(lookup.call(99)).to eq("Inline oneboxer - #4 by #{posts[3].user.username}")
+    end
+
     it "will not crawl domains that aren't allowlisted" do
       onebox = InlineOneboxer.lookup("https://eviltrout.com", skip_cache: true)
       expect(onebox).to be_blank

spec/components/topic_view_spec.rb

@@ -621,9 +621,51 @@ describe TopicView do
   context "page_title" do
     fab!(:tag1) { Fabricate(:tag) }
     fab!(:tag2) { Fabricate(:tag, topic_count: 2) }
+    fab!(:op_post) { Fabricate(:post, topic: topic) }
+    fab!(:post1) { Fabricate(:post, topic: topic) }
+    fab!(:whisper) { Fabricate(:post, topic: topic, post_type: Post.types[:whisper]) }
 
     subject { TopicView.new(topic.id, evil_trout).page_title }
 
+    context "when a post number is specified" do
+      context "admins" do
+        it "see post number and username for all posts" do
+          title = TopicView.new(topic.id, admin, post_number: 0).page_title
+          expect(title).to eq(topic.title)
+          title = TopicView.new(topic.id, admin, post_number: 1).page_title
+          expect(title).to eq(topic.title)
+
+          title = TopicView.new(topic.id, admin, post_number: 2).page_title
+          expect(title).to eq("#{topic.title} - #2 by #{post1.user.username}")
+          title = TopicView.new(topic.id, admin, post_number: 3).page_title
+          expect(title).to eq("#{topic.title} - #3 by #{whisper.user.username}")
+        end
+      end
+
+      context "regular users" do
+        it "see post number and username for regular posts" do
+          title = TopicView.new(topic.id, evil_trout, post_number: 0).page_title
+          expect(title).to eq(topic.title)
+          title = TopicView.new(topic.id, evil_trout, post_number: 1).page_title
+          expect(title).to eq(topic.title)
+
+          title = TopicView.new(topic.id, evil_trout, post_number: 2).page_title
+          expect(title).to eq("#{topic.title} - #2 by #{post1.user.username}")
+        end
+
+        it "see only post number for whisper posts" do
+          title = TopicView.new(topic.id, evil_trout, post_number: 3).page_title
+          expect(title).to eq("#{topic.title} - #3")
+          post2 = Fabricate(:post, topic: topic)
+          topic.reload
+          title = TopicView.new(topic.id, evil_trout, post_number: 3).page_title
+          expect(title).to eq("#{topic.title} - #3")
+          title = TopicView.new(topic.id, evil_trout, post_number: 4).page_title
+          expect(title).to eq("#{topic.title} - #4 by #{post2.user.username}")
+        end
+      end
+    end
+
     context "uncategorized topic" do
       context "topic_page_title_includes_category is false" do
         before { SiteSetting.topic_page_title_includes_category = false }