diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2881faee8d5..297ae1a02fb 100644 --- a/config/locales/server.en.yml +++ b/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}" - 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}" + 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}" word_connector: # Connects words with a comma. Example: "foo, bar" diff --git a/lib/inline_oneboxer.rb b/lib/inline_oneboxer.rb index 48c01715198..9536b17061a 100644 --- a/lib/inline_oneboxer.rb +++ b/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 diff --git a/lib/topic_view.rb b/lib/topic_view.rb index e88a5057953..4a3549e0a23 100644 --- a/lib/topic_view.rb +++ b/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}" diff --git a/spec/components/inline_oneboxer_spec.rb b/spec/components/inline_oneboxer_spec.rb index d00c9822ccc..7043aa1146d 100644 --- a/spec/components/inline_oneboxer_spec.rb +++ b/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 diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb index e3a159ded3a..489bcdc8654 100644 --- a/spec/components/topic_view_spec.rb +++ b/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 }