FIX: inbound link when the only slug available (#8457)

FIX: inbound link when the only slug available (#8457)

Problem mentioned in meta

When there is an internal link without ID, only slug, we should still try to create reflection link.

diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb
index 1544e9548b..ed0633ecd7 100644
--- a/app/models/topic_link.rb
+++ b/app/models/topic_link.rb
@@ -172,6 +172,7 @@ class TopicLink < ActiveRecord::Base
     internal = false
     topic_id = nil
     post_number = nil
+    topic = nil
 
     if upload = Upload.get_from_url(url)
       internal = Discourse.store.internal?
@@ -185,9 +186,11 @@ class TopicLink < ActiveRecord::Base
 
       topic_id = route[:topic_id].to_i
       post_number = route[:post_number] || 1
+      topic_slug = route[:id]
 
       # Store the canonical URL
       topic = Topic.find_by(id: topic_id)
+      topic ||= Topic.find_by(slug: topic_slug) if topic_slug
       topic_id = nil unless topic
 
       if topic.present?
@@ -197,11 +200,11 @@ class TopicLink < ActiveRecord::Base
     end
 
     # Skip linking to ourselves
-    return nil if topic_id == post.topic_id
+    return nil if topic&.id == post.topic_id
 
     reflected_post = nil
-    if post_number && topic_id
-      reflected_post = Post.find_by(topic_id: topic_id, post_number: post_number.to_i)
+    if post_number && topic
+      reflected_post = Post.find_by(topic_id: topic.id, post_number: post_number.to_i)
     end
 
     url = url[0...TopicLink.max_url_length]
@@ -216,7 +219,7 @@ class TopicLink < ActiveRecord::Base
                           url: url,
                           domain: parsed.host || Discourse.current_hostname,
                           internal: internal,
-                          link_topic_id: topic_id,
+                          link_topic_id: topic&.id,
                           link_post_id: reflected_post.try(:id),
                           quote: link.is_quote,
                           extension: file_extension)
@@ -228,31 +231,27 @@ class TopicLink < ActiveRecord::Base
     reflected_id = nil
 
     # Create the reflection if we can
-    if topic_id.present?
-      topic = Topic.find_by(id: topic_id)
-
-      if topic && post.topic && topic.archetype != 'private_message' && post.topic.archetype != 'private_message' && post.topic.visible?
-        prefix = Discourse.base_url_no_prefix
-        reflected_url = "#{prefix}#{post.topic.relative_url(post.post_number)}"
-        tl = TopicLink.find_by(topic_id: topic_id,
-                               post_id: reflected_post.try(:id),
-                               url: reflected_url)
-
-        unless tl
-          tl = TopicLink.create(user_id: post.user_id,
-                                topic_id: topic_id,
-                                post_id: reflected_post.try(:id),
-                                url: reflected_url,
-                                domain: Discourse.current_hostname,
-                                reflection: true,
-                                internal: true,
-                                link_topic_id: post.topic_id,
-                                link_post_id: post.id)
+    if topic && post.topic && topic.archetype != 'private_message' && post.topic.archetype != 'private_message' && post.topic.visible?
+      prefix = Discourse.base_url_no_prefix
+      reflected_url = "#{prefix}#{post.topic.relative_url(post.post_number)}"
+      tl = TopicLink.find_by(topic_id: topic&.id,
+                             post_id: reflected_post&.id,
+                             url: reflected_url)
+
+      unless tl
+        tl = TopicLink.create(user_id: post.user_id,
+                              topic_id: topic&.id,
+                              post_id: reflected_post&.id,
+                              url: reflected_url,
+                              domain: Discourse.current_hostname,
+                              reflection: true,
+                              internal: true,
+                              link_topic_id: post.topic_id,
+                              link_post_id: post.id)
 
-        end
-
-        reflected_id = tl.id if tl.persisted?
       end
+
+      reflected_id = tl.id if tl.persisted?
     end
 
     [url, reflected_id]
diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb
index 5ed450bbd1..2f8a0bec33 100644
--- a/spec/models/topic_link_spec.rb
+++ b/spec/models/topic_link_spec.rb
@@ -144,6 +144,27 @@ describe TopicLink do
         linked_post.revise(post.user, raw: "no more linkies https://eviltrout.com")
         expect(other_topic.reload.topic_links.where(link_post_id: linked_post.id)).to be_blank
       end
+
+      it 'works without id' do
+        post
+        url = "http://#{test_uri.host}/t/#{other_topic.slug}"
+        topic.posts.create(user: user, raw: 'initial post')
+        linked_post = topic.posts.create(user: user, raw: "Link to another topic: #{url}")
+
+        TopicLink.extract_from(linked_post)
+        link = topic.topic_links.first
+
+        reflection = other_topic.topic_links.first
+
+        expect(reflection).to be_present
+        expect(reflection).to be_reflection
+        expect(reflection.post_id).to be_present
+        expect(reflection.domain).to eq(test_uri.host)
+        expect(reflection.url).to eq("http://#{test_uri.host}/t/unique-topic-name/#{topic.id}/#{linked_post.post_number}")
+        expect(reflection.link_topic_id).to eq(topic.id)
+        expect(reflection.link_post_id).to eq(linked_post.id)
+        expect(reflection.user_id).to eq(link.user_id)
+      end
     end
 
     context "link to a user on discourse" do

GitHub sha: 46fc45de

1 Like

I think this is not used anymore?

sorry, I don’t follow, which part do you think is not used anymore?

Sorry, it’s not very clear. I meant this line

topic_id = nil unless topic

Doesn’t seem like we’re using the topic_id variable anymore.

1 Like

ahh, got you, you are right

2 Likes