diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 46ff0a334e9..27810f9e718 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -483,7 +483,7 @@ class ApplicationController < ActionController::Base def post_ids_including_replies post_ids = params[:post_ids].map(&:to_i) - post_ids |= PostReply.where(post_id: params[:reply_post_ids]).pluck(:reply_id) if params[:reply_post_ids] + post_ids |= PostReply.where(post_id: params[:reply_post_ids]).pluck(:reply_post_id) if params[:reply_post_ids] post_ids end diff --git a/app/models/post.rb b/app/models/post.rb index 5abe6dbd431..db05eb470e4 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -817,10 +817,10 @@ class Post < ActiveRecord::Base WITH RECURSIVE breadcrumb(id, level) AS ( SELECT :post_id, 0 UNION - SELECT reply_id, level + 1 + SELECT reply_post_id, level + 1 FROM post_replies AS r JOIN breadcrumb AS b ON (r.post_id = b.id) - WHERE r.post_id <> r.reply_id + WHERE r.post_id <> r.reply_post_id AND b.level < :max_reply_level ), breadcrumb_with_count AS ( SELECT @@ -828,8 +828,8 @@ class Post < ActiveRecord::Base level, COUNT(*) AS count FROM post_replies AS r - JOIN breadcrumb AS b ON (r.reply_id = b.id) - WHERE r.reply_id <> r.post_id + JOIN breadcrumb AS b ON (r.reply_post_id = b.id) + WHERE r.reply_post_id <> r.post_id GROUP BY id, level ) SELECT id, level @@ -1063,7 +1063,7 @@ class Post < ActiveRecord::Base def create_reply_relationship_with(post) return if post.nil? || self.deleted_at.present? - post_reply = post.post_replies.new(reply_id: id) + post_reply = post.post_replies.new(reply_post_id: id) if post_reply.save if Topic.visible_post_types.include?(self.post_type) Post.where(id: post.id).update_all ['reply_count = reply_count + 1'] diff --git a/app/models/post_mover.rb b/app/models/post_mover.rb index facccc8ebd7..4a8148f2971 100644 --- a/app/models/post_mover.rb +++ b/app/models/post_mover.rb @@ -250,7 +250,7 @@ class PostMover FROM ( SELECT r.post_id, mp.new_topic_id, COUNT(1) AS moved_reply_count FROM moved_posts mp - JOIN post_replies r ON (mp.old_post_id = r.reply_id) + JOIN post_replies r ON (mp.old_post_id = r.reply_post_id) GROUP BY r.post_id, mp.new_topic_id ) x WHERE x.post_id = p.id AND x.new_topic_id <> p.topic_id @@ -275,7 +275,7 @@ class PostMover SET post_id = mp.new_post_id FROM moved_posts mp WHERE mp.old_post_id <> mp.new_post_id AND pr.post_id = mp.old_post_id AND - EXISTS (SELECT 1 FROM moved_posts mr WHERE mr.new_post_id = pr.reply_id) + EXISTS (SELECT 1 FROM moved_posts mr WHERE mr.new_post_id = pr.reply_post_id) SQL end @@ -283,8 +283,8 @@ class PostMover DB.exec <<~SQL DELETE FROM post_replies pr USING moved_posts mp, posts p, posts r - WHERE (pr.reply_id = mp.old_post_id OR pr.post_id = mp.old_post_id) AND - p.id = pr.post_id AND r.id = pr.reply_id AND p.topic_id <> r.topic_id + WHERE (pr.reply_post_id = mp.old_post_id OR pr.post_id = mp.old_post_id) AND + p.id = pr.post_id AND r.id = pr.reply_post_id AND p.topic_id <> r.topic_id SQL end diff --git a/app/models/post_reply.rb b/app/models/post_reply.rb index 37654e67537..9cc9be26922 100644 --- a/app/models/post_reply.rb +++ b/app/models/post_reply.rb @@ -1,10 +1,14 @@ # frozen_string_literal: true class PostReply < ActiveRecord::Base - belongs_to :post - belongs_to :reply, class_name: 'Post' + self.ignored_columns = %w{ + reply_id + } - validates_uniqueness_of :reply_id, scope: :post_id + belongs_to :post + belongs_to :reply, foreign_key: :reply_post_id, class_name: 'Post' + + validates_uniqueness_of :reply_post_id, scope: :post_id validate :ensure_same_topic private @@ -23,13 +27,13 @@ end # # Table name: post_replies # -# post_id :integer -# reply_id :integer -# created_at :datetime not null -# updated_at :datetime not null +# post_id :integer +# created_at :datetime not null +# updated_at :datetime not null +# reply_post_id :integer # # Indexes # -# index_post_replies_on_post_id_and_reply_id (post_id,reply_id) UNIQUE -# index_post_replies_on_reply_id (reply_id) +# index_post_replies_on_post_id_and_reply_post_id (post_id,reply_post_id) UNIQUE +# index_post_replies_on_reply_post_id (reply_post_id) # diff --git a/db/migrate/20200116140132_rename_reply_id_column.rb b/db/migrate/20200116140132_rename_reply_id_column.rb new file mode 100644 index 00000000000..c90623aa458 --- /dev/null +++ b/db/migrate/20200116140132_rename_reply_id_column.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class RenameReplyIdColumn < ActiveRecord::Migration[6.0] + def up + Migration::ColumnDropper.mark_readonly(:post_replies, :reply_id) + + add_column :post_replies, :reply_post_id, :integer + + execute <<~SQL + UPDATE post_replies + SET reply_post_id = reply_id + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20200117141138_update_post_reply_indexes.rb b/db/migrate/20200117141138_update_post_reply_indexes.rb new file mode 100644 index 00000000000..0ba728edcc7 --- /dev/null +++ b/db/migrate/20200117141138_update_post_reply_indexes.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class UpdatePostReplyIndexes < ActiveRecord::Migration[6.0] + # Adding and removing indexes concurrently doesn't work within transaction. + # It also needs existence checks for indexes in case the migration fails and needs to run again. + disable_ddl_transaction! + + def up + if !index_exists?(:post_replies, [:post_id, :reply_post_id]) + add_index :post_replies, [:post_id, :reply_post_id], unique: true, algorithm: :concurrently + end + + if !index_exists?(:post_replies, [:reply_post_id]) + add_index :post_replies, [:reply_post_id], algorithm: :concurrently + end + + if index_exists?(:post_replies, [:post_id, :reply_id]) + remove_index :post_replies, column: [:post_id, :reply_id], algorithm: :concurrently + end + + if index_exists?(:post_replies, [:reply_id]) + remove_index :post_replies, column: [:reply_id], algorithm: :concurrently + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 2ae0654eb94..9fab38906b3 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -114,7 +114,7 @@ module Email referenced_posts = Post.includes(:incoming_email) .joins("INNER JOIN post_replies ON post_replies.post_id = posts.id ") - .where("post_replies.reply_id = ?", post_id) + .where("post_replies.reply_post_id = ?", post_id) .order(id: :desc) referenced_post_message_ids = referenced_posts.map do |referenced_post| diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 214cb2bbaa1..d53e33915ed 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -307,10 +307,10 @@ class PostDestroyer end def remove_associated_replies - post_ids = PostReply.where(reply_id: @post.id).pluck(:post_id) + post_ids = PostReply.where(reply_post_id: @post.id).pluck(:post_id) if post_ids.present? - PostReply.where(reply_id: @post.id).delete_all + PostReply.where(reply_post_id: @post.id).delete_all Post.where(id: post_ids).each { |p| p.update_column :reply_count, p.replies.count } end end diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake index d10eea7f031..14212bfb956 100644 --- a/lib/tasks/import.rake +++ b/lib/tasks/import.rake @@ -45,7 +45,7 @@ def insert_post_replies log "Inserting post replies..." DB.exec <<-SQL - INSERT INTO post_replies (post_id, reply_id, created_at, updated_at) + INSERT INTO post_replies (post_id, reply_post_id, created_at, updated_at) SELECT p2.id, p.id, p.created_at, p.created_at FROM posts p INNER JOIN posts p2 ON p2.post_number = p.reply_to_post_number AND p2.topic_id = p.topic_id @@ -300,7 +300,7 @@ def update_posts # WITH X AS ( # SELECT pr.post_id, p.user_id # FROM post_replies pr - # JOIN posts p ON p.id = pr.reply_id + # JOIN posts p ON p.id = pr.reply_post_id # ) # UPDATE posts # SET reply_to_user_id = X.user_id diff --git a/script/bulk_import/discourse_merger.rb b/script/bulk_import/discourse_merger.rb index 0bd4f37dfd4..abe45f492d4 100644 --- a/script/bulk_import/discourse_merger.rb +++ b/script/bulk_import/discourse_merger.rb @@ -511,7 +511,7 @@ class BulkImport::DiscourseMerger < BulkImport::Base end def process_post_reply(post_reply) - post_reply['reply_id'] = post_id_from_imported_id(post_reply['reply_id']) if post_reply['reply_id'] + post_reply['reply_post_id'] = post_id_from_imported_id(post_reply['reply_post_id']) if post_reply['reply_post_id'] post_reply end diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 6de14c80d48..3b370e4dc30 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -639,7 +639,7 @@ describe PostDestroyer do describe 'with a reply' do fab!(:reply) { Fabricate(:basic_reply, user: coding_horror, topic: post.topic) } - let!(:post_reply) { PostReply.create(post_id: post.id, reply_id: reply.id) } + let!(:post_reply) { PostReply.create(post_id: post.id, reply_post_id: reply.id) } it 'changes the post count of the topic' do post.reload diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index 99ea605ba10..7ae87eab6a4 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -955,10 +955,10 @@ describe PostMover do expect(topic.highest_post_number).to eq(p4.post_number) # updates replies for posts moved to same topic - expect(PostReply.where(reply_id: p2.id).pluck(:post_id)).to contain_exactly(new_first.id) + expect(PostReply.where(reply_post_id: p2.id).pluck(:post_id)).to contain_exactly(new_first.id) # leaves replies to the first post of the original topic unchanged - expect(PostReply.where(reply_id: p3.id).pluck(:post_id)).to contain_exactly(p1.id) + expect(PostReply.where(reply_post_id: p3.id).pluck(:post_id)).to contain_exactly(p1.id) end it "preserves post actions in the new post" do diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 24bc394b549..82990338847 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -236,7 +236,7 @@ describe PostsController do describe "can delete replies" do before do - PostReply.create(post_id: post1.id, reply_id: post2.id) + PostReply.create(post_id: post1.id, reply_post_id: post2.id) end it "deletes the post and the reply to it" do diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 4900b0555d2..9ac9edea019 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -162,7 +162,7 @@ RSpec.describe TopicsController do user = sign_in(moderator) p1 = Fabricate(:post, topic: topic, user: user) p2 = Fabricate(:post, topic: topic, user: user, reply_to_post_number: p1.post_number) - PostReply.create(post_id: p1.id, reply_id: p2.id) + PostReply.create(post_id: p1.id, reply_post_id: p2.id) post "/t/#{topic.id}/move-posts.json", params: { title: 'new topic title',