From 689dd16be026ab5c83b6930b66aa1f5219eee88f Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Wed, 8 Mar 2017 11:49:39 +0800
Subject: [PATCH] FIX: Allow user to remove bookmark from posts as long as
 bookmark is present.

https://meta.discourse.org/t/bookmark-issue-when-access-to-topic-is-lost-pms/51993
---
 app/controllers/posts_controller.rb       | 12 +++++---
 spec/controllers/posts_controller_spec.rb | 37 ++++++++++++++++++++---
 2 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index 52618c5cafd..7baedf40895 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -377,17 +377,19 @@ class PostsController < ApplicationController
   end
 
   def bookmark
-    post = find_post_from_params
-
     if params[:bookmarked] == "true"
+      post = find_post_from_params
       PostAction.act(current_user, post, PostActionType.types[:bookmark])
     else
+      post_action = PostAction.find_by(post_id: params[:post_id], user_id: current_user.id)
+      post = post_action.post
+      raise Discourse::InvalidParameters unless post_action
+
       PostAction.remove_act(current_user, post, PostActionType.types[:bookmark])
     end
 
-    tu = TopicUser.get(post.topic, current_user)
-
-    render_json_dump(topic_bookmarked: tu.try(:bookmarked))
+    topic_user = TopicUser.get(post.topic, current_user)
+    render_json_dump(topic_bookmarked: topic_user.try(:bookmarked))
   end
 
   def wiki
diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb
index 8c8ba76a89e..98b198550a7 100644
--- a/spec/controllers/posts_controller_spec.rb
+++ b/spec/controllers/posts_controller_spec.rb
@@ -426,7 +426,8 @@ describe PostsController do
     include_examples 'action requires login', :put, :bookmark, post_id: 2
 
     describe 'when logged in' do
-      let(:post) { Fabricate(:post, user: log_in) }
+      let(:user) { log_in }
+      let(:post) { Fabricate(:post, user: user) }
       let(:private_message) { Fabricate(:private_message_post) }
 
       it "raises an error if the user doesn't have permission to see the post" do
@@ -436,13 +437,39 @@ describe PostsController do
       end
 
       it 'creates a bookmark' do
-        PostAction.expects(:act).with(post.user, post, PostActionType.types[:bookmark])
         xhr :put, :bookmark, post_id: post.id, bookmarked: 'true'
+
+        post_action = PostAction.find_by(user:user, post: post)
+
+        expect(post_action.post_action_type_id).to eq(PostActionType.types[:bookmark])
       end
 
-      it 'removes a bookmark' do
-        PostAction.expects(:remove_act).with(post.user, post, PostActionType.types[:bookmark])
-        xhr :put, :bookmark, post_id: post.id
+      context "removing a bookmark" do
+        let(:post_action) { PostAction.act(user, post, PostActionType.types[:bookmark]) }
+        let(:admin) { Fabricate(:admin) }
+
+        it 'should be able to remove a bookmark' do
+          post_action
+          xhr :put, :bookmark, post_id: post.id
+
+          expect(PostAction.find_by(id: post_action.id)).to eq(nil)
+        end
+
+        describe "when user doesn't have permission to see bookmarked post" do
+          it "should still be able to remove a bookmark" do
+            post_action
+            post = post_action.post
+            topic = post.topic
+            topic.convert_to_private_message(admin)
+            topic.remove_allowed_user(admin, user.username)
+
+            expect(Guardian.new(user).can_see_post?(post.reload)).to eq(false)
+
+            xhr :put, :bookmark, post_id: post.id
+
+            expect(PostAction.find_by(id: post_action.id)).to eq(nil)
+          end
+        end
       end
 
     end