From 9fbe1436b6fe5aa3c1dfb3b711cf3af1801fab20 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Wed, 29 Nov 2017 21:52:41 +0800
Subject: [PATCH] UX: Replace heuristic solution root domain extraction for
topic featured link.
---
.../lib/render-topic-featured-link.js.es6 | 21 ++++------
app/models/topic.rb | 4 ++
app/serializers/topic_list_item_serializer.rb | 7 ++--
app/serializers/topic_view_serializer.rb | 5 +++
spec/models/topic_spec.rb | 15 +++++++
.../topic_list_item_serializer_spec.rb | 41 +++++++++++++++----
.../serializers/topic_view_serializer_spec.rb | 29 +++++++++++++
7 files changed, 100 insertions(+), 22 deletions(-)
diff --git a/app/assets/javascripts/discourse/lib/render-topic-featured-link.js.es6 b/app/assets/javascripts/discourse/lib/render-topic-featured-link.js.es6
index 3c34553fb5d..86b30b60bfe 100644
--- a/app/assets/javascripts/discourse/lib/render-topic-featured-link.js.es6
+++ b/app/assets/javascripts/discourse/lib/render-topic-featured-link.js.es6
@@ -1,4 +1,3 @@
-import { extractDomainFromUrl } from 'discourse/lib/utilities';
import { h } from 'virtual-dom';
const _decorators = [];
@@ -8,23 +7,22 @@ export function addFeaturedLinkMetaDecorator(decorator) {
}
export function extractLinkMeta(topic) {
- const href = topic.featured_link,
- target = Discourse.User.currentProp('external_links_in_new_tab') ? '_blank' : '';
+ const href = topic.get('featured_link');
+ const target = Discourse.User.currentProp('external_links_in_new_tab') ? '_blank' : '';
if (!href) { return; }
- let domain = extractDomainFromUrl(href);
- if (!domain) { return; }
+ const meta = {
+ target: target,
+ href,
+ domain: topic.get('featured_link_root_domain'),
+ rel: 'nofollow'
+ };
- // www appears frequently, so we truncate it
- if (domain && domain.substr(0, 4) === 'www.') {
- domain = domain.substring(4);
- }
-
- const meta = { target, href, domain, rel: 'nofollow' };
if (_decorators.length) {
_decorators.forEach(cb => cb(meta));
}
+
return meta;
}
@@ -45,4 +43,3 @@ export function topicFeaturedLinkNode(topic) {
}, meta.domain);
}
}
-
diff --git a/app/models/topic.rb b/app/models/topic.rb
index 070f9890fa3..e3f6aa4c116 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -1266,6 +1266,10 @@ SQL
result.ntuples != 0
end
+ def featured_link_root_domain
+ MiniSuffix.domain(URI.parse(self.featured_link).hostname)
+ end
+
private
def update_category_topic_count_by(num)
diff --git a/app/serializers/topic_list_item_serializer.rb b/app/serializers/topic_list_item_serializer.rb
index 9e1e642a159..0ce4f4cd500 100644
--- a/app/serializers/topic_list_item_serializer.rb
+++ b/app/serializers/topic_list_item_serializer.rb
@@ -11,7 +11,8 @@ class TopicListItemSerializer < ListableTopicSerializer
:bookmarked_post_numbers,
:liked_post_numbers,
:tags,
- :featured_link
+ :featured_link,
+ :featured_link_root_domain
has_many :posters, serializer: TopicPosterSerializer, embed: :objects
has_many :participants, serializer: TopicPosterSerializer, embed: :objects
@@ -77,8 +78,8 @@ class TopicListItemSerializer < ListableTopicSerializer
SiteSetting.topic_featured_link_enabled
end
- def featured_link
- object.featured_link
+ def include_featured_link_root_domain?
+ SiteSetting.topic_featured_link_enabled && object.featured_link.present?
end
end
diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb
index 552aeeb25f7..4a381bea7ef 100644
--- a/app/serializers/topic_view_serializer.rb
+++ b/app/serializers/topic_view_serializer.rb
@@ -37,6 +37,7 @@ class TopicViewSerializer < ApplicationSerializer
:user_id,
:pm_with_non_human_user?,
:featured_link,
+ :featured_link_root_domain,
:pinned_globally,
:pinned_at,
:pinned_until
@@ -261,6 +262,10 @@ class TopicViewSerializer < ApplicationSerializer
SiteSetting.topic_featured_link_enabled
end
+ def include_featured_link_root_domain?
+ SiteSetting.topic_featured_link_enabled && object.topic.featured_link
+ end
+
def include_unicode_title?
!!(object.topic.title =~ /:([\w\-+]*):/)
end
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index 0d85b51daa6..11686a14ee5 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -2095,4 +2095,19 @@ describe Topic do
end
end
end
+
+ describe '#featured_link_root_domain' do
+ let(:topic) { Fabricate.build(:topic) }
+
+ it 'should extract the root domain correctly' do
+ [
+ "https://meta.discourse.org",
+ "https://meta.discourse.org/",
+ "https://meta.discourse.org/?filter=test"
+ ].each do |featured_link|
+ topic.featured_link = featured_link
+ expect(topic.featured_link_root_domain).to eq("discourse.org")
+ end
+ end
+ end
end
diff --git a/spec/serializers/topic_list_item_serializer_spec.rb b/spec/serializers/topic_list_item_serializer_spec.rb
index 823682e3b25..86f599e30d3 100644
--- a/spec/serializers/topic_list_item_serializer_spec.rb
+++ b/spec/serializers/topic_list_item_serializer_spec.rb
@@ -2,18 +2,45 @@ require 'rails_helper'
require_dependency 'post_action'
describe TopicListItemSerializer do
-
- it "correctly serializes topic" do
+ let(:topic) do
date = Time.zone.now
- topic = Topic.new
- topic.title = "test"
- topic.created_at = date - 2.minutes
- topic.bumped_at = date
- topic.posters = []
+ Fabricate.build(:topic,
+ title: 'test',
+ created_at: date - 2.minutes,
+ bumped_at: date,
+ posters: [],
+ )
+ end
+
+ it "correctly serializes topic" do
+ SiteSetting.topic_featured_link_enabled = true
serialized = TopicListItemSerializer.new(topic, scope: Guardian.new, root: false).as_json
expect(serialized[:title]).to eq("test")
expect(serialized[:bumped]).to eq(true)
+ expect(serialized[:featured_link]).to eq(nil)
+ expect(serialized[:featured_link_root_domain]).to eq(nil)
+
+ featured_link = 'http://meta.discourse.org'
+ topic.featured_link = featured_link
+ serialized = TopicListItemSerializer.new(topic, scope: Guardian.new, root: false).as_json
+
+ expect(serialized[:featured_link]).to eq(featured_link)
+ expect(serialized[:featured_link_root_domain]).to eq('discourse.org')
+ end
+
+ describe 'when topic featured link is disable' do
+ before do
+ SiteSetting.topic_featured_link_enabled = false
+ end
+
+ it "should not include the topic's featured link" do
+ topic.featured_link = 'http://meta.discourse.org'
+ serialized = TopicListItemSerializer.new(topic, scope: Guardian.new, root: false).as_json
+
+ expect(serialized[:featured_link]).to eq(nil)
+ expect(serialized[:featured_link_root_domain]).to eq(nil)
+ end
end
end
diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb
index d4449c39ceb..c2970db194f 100644
--- a/spec/serializers/topic_view_serializer_spec.rb
+++ b/spec/serializers/topic_view_serializer_spec.rb
@@ -4,6 +4,35 @@ describe TopicViewSerializer do
let(:topic) { Fabricate(:topic) }
let(:user) { Fabricate(:user) }
+ describe '#featured_link and #featured_link_root_domain' do
+ let(:featured_link) { 'http://meta.discourse.org' }
+
+ describe 'when topic featured link is disable' do
+ it 'should return the right attributes' do
+ topic.update!(featured_link: featured_link)
+ SiteSetting.topic_featured_link_enabled = false
+
+ topic_view = TopicView.new(topic.id, user)
+ json = described_class.new(topic_view, scope: Guardian.new(user), root: false).as_json
+
+ expect(json[:featured_link]).to eq(nil)
+ expect(json[:featured_link_root_domain]).to eq(nil)
+ end
+ end
+
+ describe 'when topic featured link is enabled' do
+ it 'should return the right attributes' do
+ topic.update!(featured_link: featured_link)
+
+ topic_view = TopicView.new(topic.id, user)
+ json = described_class.new(topic_view, scope: Guardian.new(user), root: false).as_json
+
+ expect(json[:featured_link]).to eq(featured_link)
+ expect(json[:featured_link_root_domain]).to eq('discourse.org')
+ end
+ end
+ end
+
describe '#suggested_topics' do
let(:topic2) { Fabricate(:topic) }