FIX: Extraction of quoted posts failed in some cases

* It stored only oneboxed "quotes" when [quote] and links to topics or posts were mixed.
* Revising a post didn't add or remove records from the quoted_posts table.

app/models/quoted_post.rb

@@ -10,7 +10,8 @@ class QuotedPost < ActiveRecord::Base
     doc = Nokogiri::HTML.fragment(post.cooked)
 
     uniq = {}
-    ids = []
+
+    exec_sql("DELETE FROM quoted_posts WHERE post_id = :post_id", post_id: post.id)
 
     doc.css("aside.quote[data-topic]").each do |a|
       topic_id = a['data-topic'].to_i
@@ -22,34 +23,26 @@ class QuotedPost < ActiveRecord::Base
 
       begin
         # It would be so much nicer if we used post_id in quotes
-        results = exec_sql "INSERT INTO quoted_posts(post_id, quoted_post_id, created_at, updated_at)
+        exec_sql(<<~SQL, post_id: post.id, post_number: post_number, topic_id: topic_id)
-                 SELECT :post_id, p.id, current_timestamp, current_timestamp
+          INSERT INTO quoted_posts (post_id, quoted_post_id, created_at, updated_at)
-                 FROM posts p
+            SELECT :post_id, p.id, current_timestamp, current_timestamp
-                 LEFT JOIN quoted_posts q on q.post_id = :post_id AND q.quoted_post_id = p.id
+            FROM posts p
-                 WHERE post_number = :post_number AND
+              LEFT JOIN quoted_posts q on q.post_id = :post_id AND q.quoted_post_id = p.id
-                       topic_id = :topic_id AND
+            WHERE post_number = :post_number AND
-                       q.id IS NULL
+                  topic_id = :topic_id AND
-                 RETURNING quoted_post_id
+                  q.id IS NULL
-        ", post_id: post.id, post_number: post_number, topic_id: topic_id
+        SQL
-
-        results = results.to_a
-        ids << results[0]["quoted_post_id"].to_i if results.length > 0
       rescue ActiveRecord::RecordNotUnique, PG::UniqueViolation
         # it's fine
       end
     end
 
-    if ids.length > 0
-      exec_sql "DELETE FROM quoted_posts WHERE post_id = :post_id AND quoted_post_id NOT IN (:ids)",
-          post_id: post.id, ids: ids
-    end
-
     # simplest place to add this code
     reply_quoted = false
 
     if post.reply_to_post_number
       reply_post_id = Post.where(topic_id: post.topic_id, post_number: post.reply_to_post_number).pluck(:id).first
-      reply_quoted = !!(reply_post_id && ids.include?(reply_post_id))
+      reply_quoted = reply_post_id.present? && QuotedPost.where(post_id: post.id, quoted_post_id: reply_post_id).count > 0
     end
 
     if reply_quoted != post.reply_quoted

lib/post_revisor.rb

@@ -190,6 +190,7 @@ class PostRevisor
     # WARNING: do not pull this into the transaction
     # it can fire events in sidekiq before the post is done saving
     # leading to corrupt state
+    QuotedPost.extract_from(@post)
     post_process_post
 
     update_topic_word_counts
@@ -198,7 +199,6 @@ class PostRevisor
     grant_badge
 
     TopicLink.extract_from(@post)
-    QuotedPost.extract_from(@post)
 
     successfully_saved_post_and_topic
   end

spec/models/quoted_post_spec.rb

@@ -1,14 +1,34 @@
 require 'rails_helper'
 
 describe QuotedPost do
-  it 'correctly extracts quotes in integration test' do
+  it 'correctly extracts quotes' do
-    post1 = create_post
+    topic = Fabricate(:topic)
-    post2 = create_post(topic_id: post1.topic_id,
+    post1 = create_post(topic: topic, post_number: 1, raw: "foo bar")
-                        raw: "[quote=\"#{post1.user.username}, post: 1, topic:#{post1.topic_id}\"]\ntest\n[/quote]\nthis is a test post",
+    post2 = create_post(topic: topic, post_number: 2, raw: "lorem ipsum")
-                        reply_to_post_number: 1)
+    post3 = create_post(topic: topic, post_number: 3, raw: "test post")
 
-    expect(QuotedPost.find_by(post_id: post2.id, quoted_post_id: post1.id)).not_to eq(nil)
+    raw = <<~RAW
-    expect(post2.reply_quoted).to eq(true)
+      #{post1.full_url}
+
+      [quote="#{post2.user.username}, post:#{post2.post_number}, topic:#{post2.topic.id}"]
+      lorem
+      [/quote]
+
+      this is a test post
+
+      #{post3.full_url}
+    RAW
+
+    post4 = create_post(topic: topic, raw: raw, post_number: 4, reply_to_post_number: post3.post_number)
+
+    expect(QuotedPost.where(post_id: post4.id).pluck(:quoted_post_id)).to contain_exactly(post1.id, post2.id, post3.id)
+    expect(post4.reload.reply_quoted).to eq(true)
+
+    SiteSetting.editing_grace_period = 1.minute.to_i
+    post5 = create_post(topic: topic, post_number: 5, raw: "post 5")
+    raw.sub!(post3.full_url, post5.full_url)
+    post4.revise(post4.user, { raw: raw }, revised_at: post4.updated_at + 2.minutes)
+    expect(QuotedPost.where(post_id: post4.id).pluck(:quoted_post_id)).to contain_exactly(post1.id, post2.id, post5.id)
   end
 
   it 'correctly handles deltas' do

spec/services/username_changer_spec.rb

@@ -387,19 +387,6 @@ describe UsernameChanger do
             expect(post.raw).to eq(expected_raw)
             expect(post.cooked).to match_html(expected_cooked)
           end
-
-          it 'replaces the username in quote tags when the post is marked as deleted' do
-            post = create_post_and_change_username(raw: raw) do |p|
-              PostDestroyer.new(p.user, p).destroy
-            end
-
-            expect(post.raw).to_not include("foo")
-            expect(post.cooked).to_not include("foo")
-            expect(post.revisions.count).to eq(1)
-
-            expect(post.revisions[0].modifications["raw"][0]).to eq(expected_raw)
-            expect(post.revisions[0].modifications["cooked"][0]).to match_html(expected_cooked)
-          end
         end
       end