FIX: Unlike own posts on ownership transfer (#10446)
* FIX: Unlike own posts on ownership transfer If a user has liked a post that has passed the `post_undo_action_window_mins` system setting window and you transfer ownership of that post to that user you will be the owner of a post that you have liked, but cannot unlike resulting in a weird UI behavior. This commit fixes this issue. The existing tests didn't check for the timeout window for unliking posts so I added that in. I couldn't find a good way to do this logic inside of the guardian class so rather than duplicating behavior of the `PostActionDestroyer` class inside of the `PostOwnerChanger` I decided to pass in a "bypass" variable that could be used to check if the calling class is the 'post_owner_changer' and bypass the guardian instead. I went this route because the guardian `can_delete_post_action` method has no way of distinguishing how to allow a user to be able to unlike their own posts after the timeout window but only on a post owner change. * use an options hash instead
This commit is contained in:
parent
ea2e58e622
commit
367de2594d
|
@ -23,7 +23,7 @@ class PostOwnerChanger
|
||||||
|
|
||||||
post.topic = @topic
|
post.topic = @topic
|
||||||
post.set_owner(@new_owner, @acting_user, @skip_revision)
|
post.set_owner(@new_owner, @acting_user, @skip_revision)
|
||||||
PostActionDestroyer.destroy(@new_owner, post, :like)
|
PostActionDestroyer.destroy(@new_owner, post, :like, skip_delete_check: true)
|
||||||
|
|
||||||
level = post.is_first_post? ? :watching : :tracking
|
level = post.is_first_post? ? :watching : :tracking
|
||||||
TopicUser.change(@new_owner.id, @topic.id, notification_level: NotificationLevels.topic_levels[level])
|
TopicUser.change(@new_owner.id, @topic.id, notification_level: NotificationLevels.topic_levels[level])
|
||||||
|
|
|
@ -5,12 +5,12 @@ class PostActionDestroyer
|
||||||
attr_accessor :post
|
attr_accessor :post
|
||||||
end
|
end
|
||||||
|
|
||||||
def initialize(destroyed_by, post, post_action_type_id)
|
def initialize(destroyed_by, post, post_action_type_id, opts = {})
|
||||||
@destroyed_by, @post, @post_action_type_id = destroyed_by, post, post_action_type_id
|
@destroyed_by, @post, @post_action_type_id, @opts = destroyed_by, post, post_action_type_id, opts
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.destroy(destroyed_by, post, action_key)
|
def self.destroy(destroyed_by, post, action_key, opts = {})
|
||||||
new(destroyed_by, post, PostActionType.types[action_key]).perform
|
new(destroyed_by, post, PostActionType.types[action_key], opts).perform
|
||||||
end
|
end
|
||||||
|
|
||||||
def perform
|
def perform
|
||||||
|
@ -34,7 +34,7 @@ class PostActionDestroyer
|
||||||
return result
|
return result
|
||||||
end
|
end
|
||||||
|
|
||||||
unless guardian.can_delete?(post_action)
|
unless @opts[:skip_delete_check] == true || guardian.can_delete?(post_action)
|
||||||
result.forbidden = true
|
result.forbidden = true
|
||||||
result.add_error(I18n.t("invalid_access"))
|
result.add_error(I18n.t("invalid_access"))
|
||||||
return result
|
return result
|
||||||
|
|
|
@ -715,6 +715,23 @@ RSpec.describe TopicsController do
|
||||||
expect(t2.deleted_at).to be_nil
|
expect(t2.deleted_at).to be_nil
|
||||||
expect(p3.user).to eq(user_a)
|
expect(p3.user).to eq(user_a)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "removes likes by new owner" do
|
||||||
|
now = Time.zone.now
|
||||||
|
freeze_time(now - 1.day)
|
||||||
|
PostActionCreator.like(user_a, p1)
|
||||||
|
p1.reload
|
||||||
|
freeze_time(now)
|
||||||
|
post "/t/#{topic.id}/change-owner.json", params: {
|
||||||
|
username: user_a.username_lower, post_ids: [p1.id]
|
||||||
|
}
|
||||||
|
topic.reload
|
||||||
|
p1.reload
|
||||||
|
expect(response.status).to eq(200)
|
||||||
|
expect(topic.user.username).to eq(user_a.username)
|
||||||
|
expect(p1.user.username).to eq(user_a.username)
|
||||||
|
expect(p1.like_count).to eq(0)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -24,12 +24,15 @@ describe PostOwnerChanger do
|
||||||
|
|
||||||
it "changes the user" do
|
it "changes the user" do
|
||||||
bumped_at = freeze_time topic.bumped_at
|
bumped_at = freeze_time topic.bumped_at
|
||||||
|
now = Time.zone.now
|
||||||
|
freeze_time(now - 1.day)
|
||||||
|
|
||||||
old_user = p1.user
|
old_user = p1.user
|
||||||
PostActionCreator.like(user_a, p1)
|
PostActionCreator.like(user_a, p1)
|
||||||
p1.reload
|
p1.reload
|
||||||
expect(p1.topic.like_count).to eq(1)
|
expect(p1.topic.like_count).to eq(1)
|
||||||
|
|
||||||
|
freeze_time(now)
|
||||||
PostOwnerChanger.new(post_ids: [p1.id], topic_id: topic.id, new_owner: user_a, acting_user: editor).change_owner!
|
PostOwnerChanger.new(post_ids: [p1.id], topic_id: topic.id, new_owner: user_a, acting_user: editor).change_owner!
|
||||||
p1.reload
|
p1.reload
|
||||||
expect(p1.topic.like_count).to eq(0)
|
expect(p1.topic.like_count).to eq(0)
|
||||||
|
|
Loading…
Reference in New Issue