FIX: Do not reset link counts when post is rebaked

FIX: Do not reset link counts when post is rebaked

This was an indentation mistake introduced in 44eba0b. Pretty understandable, considering we are indented 8 levels deep in this method. Will follow-up with a refactor to improve this.

From 37249c9a32b8e7f303d70fbd100d3e2778fb437a Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Wed, 5 Dec 2018 17:16:27 +0000
Subject: [PATCH] FIX: Do not reset link counts when post is rebaked

This was an indentation mistake introduced in 44eba0b. Pretty understandable, considering we are indented 8 levels deep in this method. Will follow-up with a refactor to improve this.

diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb
index 229e2eb..a8c9474 100644
--- a/app/models/topic_link.rb
+++ b/app/models/topic_link.rb
@@ -223,34 +223,35 @@ SQL
           # If we can't find the route, no big deal
         end
       end
+    end
+
+    # Remove links that aren't there anymore
+    if added_urls.present?
+      TopicLink.where(
+        "(url not in (:urls)) AND (post_id = :post_id AND NOT reflection)",
+        urls: added_urls, post_id: post.id
+      ).delete_all
 
-      # Remove links that aren't there anymore
-      if added_urls.present?
+      reflected_ids.compact!
+      if reflected_ids.present?
         TopicLink.where(
-          "(url not in (:urls)) AND (post_id = :post_id AND NOT reflection)",
-          urls: added_urls, post_id: post.id
+          "(id not in (:reflected_ids)) AND (link_post_id = :post_id AND reflection)",
+          reflected_ids: reflected_ids, post_id: post.id
         ).delete_all
-
-        reflected_ids.compact!
-        if reflected_ids.present?
-          TopicLink.where(
-            "(id not in (:reflected_ids)) AND (link_post_id = :post_id AND reflection)",
-            reflected_ids: reflected_ids, post_id: post.id
-          ).delete_all
-        else
-          TopicLink
-            .where("link_post_id = :post_id AND reflection", post_id: post.id)
-            .delete_all
-        end
       else
         TopicLink
-          .where(
-            "(post_id = :post_id AND NOT reflection) OR (link_post_id = :post_id AND reflection)",
-            post_id: post.id
-          )
+          .where("link_post_id = :post_id AND reflection", post_id: post.id)
           .delete_all
       end
+    else
+      TopicLink
+        .where(
+          "(post_id = :post_id AND NOT reflection) OR (link_post_id = :post_id AND reflection)",
+          post_id: post.id
+        )
+        .delete_all
     end
+
   end
 
   # Crawl a link's title after it's saved
diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb
index 20a81c6..8c7cea0 100644
--- a/spec/models/topic_link_spec.rb
+++ b/spec/models/topic_link_spec.rb
@@ -26,15 +26,17 @@ describe TopicLink do
   end
 
   describe 'external links' do
-    before do
-      post = Fabricate(:post, raw: <<~RAW, user: user, topic: topic)
+    let(:post2) do
+      Fabricate(:post, raw: <<~RAW, user: user, topic: topic)
         http://a.com/
         https://b.com/b
         http://#{'a' * 200}.com/invalid
         //b.com/#{'a' * 500}
       RAW
+    end
 
-      TopicLink.extract_from(post)
+    before do
+      TopicLink.extract_from(post2)
     end
 
     it 'works' do
@@ -45,6 +47,16 @@ describe TopicLink do
       )
     end
 
+    it "doesn't reset them when rebaking" do
+      old_ids = topic.topic_links.pluck(:id)
+
+      TopicLink.extract_from(post2)
+
+      new_ids = topic.topic_links.pluck(:id)
+
+      expect(new_ids).to contain_exactly(*old_ids)
+    end
+
   end
 
   describe 'internal links' do

GitHub

2 Likes