FIX: Old notifications didn't link to correct post after moving post
This commit is contained in:
parent
5fc5a7f5ae
commit
1235105c03
|
@ -108,6 +108,8 @@ class PostMover
|
||||||
end
|
end
|
||||||
|
|
||||||
def create_first_post(post)
|
def create_first_post(post)
|
||||||
|
old_post_attributes = post_attributes(post)
|
||||||
|
|
||||||
new_post = PostCreator.create(
|
new_post = PostCreator.create(
|
||||||
post.user,
|
post.user,
|
||||||
raw: post.raw,
|
raw: post.raw,
|
||||||
|
@ -123,6 +125,7 @@ class PostMover
|
||||||
|
|
||||||
move_incoming_emails(post, new_post)
|
move_incoming_emails(post, new_post)
|
||||||
move_email_logs(post, new_post)
|
move_email_logs(post, new_post)
|
||||||
|
move_notifications(old_post_attributes, new_post)
|
||||||
|
|
||||||
PostAction.copy(post, new_post)
|
PostAction.copy(post, new_post)
|
||||||
new_post.update_column(:reply_count, @reply_count[1] || 0)
|
new_post.update_column(:reply_count, @reply_count[1] || 0)
|
||||||
|
@ -149,11 +152,12 @@ class PostMover
|
||||||
update[:reply_to_user_id] = nil
|
update[:reply_to_user_id] = nil
|
||||||
end
|
end
|
||||||
|
|
||||||
|
old_post_attributes = post_attributes(post)
|
||||||
post.attributes = update
|
post.attributes = update
|
||||||
post.save(validate: false)
|
post.save(validate: false)
|
||||||
|
|
||||||
move_incoming_emails(post, post)
|
move_incoming_emails(post, post)
|
||||||
move_email_logs(post, post)
|
move_notifications(old_post_attributes, post)
|
||||||
|
|
||||||
DiscourseEvent.trigger(:post_moved, post, original_topic.id)
|
DiscourseEvent.trigger(:post_moved, post, original_topic.id)
|
||||||
|
|
||||||
|
@ -175,6 +179,31 @@ class PostMover
|
||||||
.update_all(post_id: new_post.id)
|
.update_all(post_id: new_post.id)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def move_notifications(old_post_attributes, new_post)
|
||||||
|
params = {
|
||||||
|
old_topic_id: old_post_attributes[:topic_id],
|
||||||
|
old_post_number: old_post_attributes[:post_number],
|
||||||
|
new_topic_id: new_post.topic_id,
|
||||||
|
new_post_number: new_post.post_number,
|
||||||
|
new_topic_title: new_post.topic.title
|
||||||
|
}
|
||||||
|
|
||||||
|
DB.exec(<<~SQL, params)
|
||||||
|
UPDATE notifications
|
||||||
|
SET topic_id = :new_topic_id,
|
||||||
|
post_number = :new_post_number,
|
||||||
|
data = (data :: JSONB ||
|
||||||
|
jsonb_strip_nulls(
|
||||||
|
jsonb_build_object(
|
||||||
|
'topic_title', CASE WHEN data :: JSONB ->> 'topic_title' IS NULL
|
||||||
|
THEN NULL
|
||||||
|
ELSE :new_topic_title END
|
||||||
|
)
|
||||||
|
)) :: JSON
|
||||||
|
WHERE topic_id = :old_topic_id AND post_number = :old_post_number
|
||||||
|
SQL
|
||||||
|
end
|
||||||
|
|
||||||
def update_statistics
|
def update_statistics
|
||||||
destination_topic.update_statistics
|
destination_topic.update_statistics
|
||||||
original_topic.update_statistics
|
original_topic.update_statistics
|
||||||
|
@ -276,4 +305,11 @@ class PostMover
|
||||||
end
|
end
|
||||||
destination_topic.save!
|
destination_topic.save!
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def post_attributes(post)
|
||||||
|
{
|
||||||
|
topic_id: post.topic_id,
|
||||||
|
post_number: post.post_number
|
||||||
|
}
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -5,6 +5,7 @@ Fabricator(:notification) do
|
||||||
notification_type Notification.types[:mentioned]
|
notification_type Notification.types[:mentioned]
|
||||||
user
|
user
|
||||||
topic { |attrs| attrs[:post]&.topic || Fabricate(:topic, user: attrs[:user]) }
|
topic { |attrs| attrs[:post]&.topic || Fabricate(:topic, user: attrs[:user]) }
|
||||||
|
post_number { |attrs| attrs[:post]&.post_number }
|
||||||
data '{"poison":"ivy","killer":"croc"}'
|
data '{"poison":"ivy","killer":"croc"}'
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -57,3 +58,17 @@ Fabricator(:posted_notification, from: :notification) do
|
||||||
}.to_json
|
}.to_json
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
Fabricator(:mentioned_notification, from: :notification) do
|
||||||
|
notification_type Notification.types[:mentioned]
|
||||||
|
data do |attrs|
|
||||||
|
{
|
||||||
|
topic_title: attrs[:topic].title,
|
||||||
|
original_post_id: attrs[:post].id,
|
||||||
|
original_post_type: attrs[:post].post_type,
|
||||||
|
original_username: attrs[:post].user.username,
|
||||||
|
revision_number: nil,
|
||||||
|
display_username: attrs[:post].user.username
|
||||||
|
}.to_json
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
|
@ -292,6 +292,22 @@ describe PostMover do
|
||||||
notifications_reason_id: TopicUser.notification_reasons[:created_topic]
|
notifications_reason_id: TopicUser.notification_reasons[:created_topic]
|
||||||
)).to eq(true)
|
)).to eq(true)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "updates existing notifications" do
|
||||||
|
n3 = Fabricate(:mentioned_notification, post: p3, user: another_user)
|
||||||
|
n4 = Fabricate(:mentioned_notification, post: p4, user: another_user)
|
||||||
|
|
||||||
|
new_topic = topic.move_posts(user, [p3.id], title: "new testing topic name")
|
||||||
|
|
||||||
|
n3.reload
|
||||||
|
expect(n3.topic_id).to eq(new_topic.id)
|
||||||
|
expect(n3.post_number).to eq(1)
|
||||||
|
expect(n3.data_hash[:topic_title]).to eq(new_topic.title)
|
||||||
|
|
||||||
|
n4.reload
|
||||||
|
expect(n4.topic_id).to eq(topic.id)
|
||||||
|
expect(n4.post_number).to eq(4)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context "to an existing topic" do
|
context "to an existing topic" do
|
||||||
|
@ -369,6 +385,22 @@ describe PostMover do
|
||||||
moderator_post = topic.posts.find_by(post_number: 2)
|
moderator_post = topic.posts.find_by(post_number: 2)
|
||||||
expect(moderator_post.raw).to include("4 posts were merged")
|
expect(moderator_post.raw).to include("4 posts were merged")
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "updates existing notifications" do
|
||||||
|
n3 = Fabricate(:mentioned_notification, post: p3, user: another_user)
|
||||||
|
n4 = Fabricate(:mentioned_notification, post: p4, user: another_user)
|
||||||
|
|
||||||
|
moved_to = topic.move_posts(user, [p3.id], destination_topic_id: destination_topic.id)
|
||||||
|
|
||||||
|
n3.reload
|
||||||
|
expect(n3.topic_id).to eq(moved_to.id)
|
||||||
|
expect(n3.post_number).to eq(2)
|
||||||
|
expect(n3.data_hash[:topic_title]).to eq(moved_to.title)
|
||||||
|
|
||||||
|
n4.reload
|
||||||
|
expect(n4.topic_id).to eq(topic.id)
|
||||||
|
expect(n4.post_number).to eq(4)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context "to a message" do
|
context "to a message" do
|
||||||
|
|
Loading…
Reference in New Issue