FIX: Unlike own posts on ownership transfer (#10446)

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
diff --git a/app/services/post_owner_changer.rb b/app/services/post_owner_changer.rb
index 9a2d128..785c2cf 100644
--- a/app/services/post_owner_changer.rb
+++ b/app/services/post_owner_changer.rb
@@ -23,7 +23,7 @@ class PostOwnerChanger
 
       post.topic = @topic
       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
       TopicUser.change(@new_owner.id, @topic.id, notification_level: NotificationLevels.topic_levels[level])
diff --git a/lib/post_action_destroyer.rb b/lib/post_action_destroyer.rb
index cf2280a..77d004b 100644
--- a/lib/post_action_destroyer.rb
+++ b/lib/post_action_destroyer.rb
@@ -5,12 +5,12 @@ class PostActionDestroyer
     attr_accessor :post
   end
 
-  def initialize(destroyed_by, post, post_action_type_id)
-    @destroyed_by, @post, @post_action_type_id = destroyed_by, post, post_action_type_id
+  def initialize(destroyed_by, post, post_action_type_id, opts = {})
+    @destroyed_by, @post, @post_action_type_id, @opts = destroyed_by, post, post_action_type_id, opts
   end
 
-  def self.destroy(destroyed_by, post, action_key)
-    new(destroyed_by, post, PostActionType.types[action_key]).perform
+  def self.destroy(destroyed_by, post, action_key, opts = {})
+    new(destroyed_by, post, PostActionType.types[action_key], opts).perform
   end
 
   def perform
@@ -34,7 +34,7 @@ class PostActionDestroyer
       return result
     end
 
-    unless guardian.can_delete?(post_action)
+    unless @opts[:skip_delete_check] == true || guardian.can_delete?(post_action)
       result.forbidden = true
       result.add_error(I18n.t("invalid_access"))
       return result
diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb
index 311a7bf..17a896b 100644
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@@ -715,6 +715,23 @@ RSpec.describe TopicsController do
         expect(t2.deleted_at).to be_nil
         expect(p3.user).to eq(user_a)
       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
 
diff --git a/spec/services/post_owner_changer_spec.rb b/spec/services/post_owner_changer_spec.rb
index d2cf53b..a6c4d3a 100644
--- a/spec/services/post_owner_changer_spec.rb
+++ b/spec/services/post_owner_changer_spec.rb
@@ -24,12 +24,15 @@ describe PostOwnerChanger do
 
     it "changes the user" do
       bumped_at = freeze_time topic.bumped_at
+      now = Time.zone.now
+      freeze_time(now - 1.day)
 
       old_user = p1.user
       PostActionCreator.like(user_a, p1)
       p1.reload
       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!
       p1.reload
       expect(p1.topic.like_count).to eq(0)

GitHub sha: 367de259

1 Like

This commit appears in #10446 which was approved by eviltrout. It was merged by blake.