DEV: Remove PostAction/UserAction bookmark refs (#16681)

We have not used anything related to bookmarks for PostAction
or UserAction records since 2020, bookmarks are their own thing
now. Deleting all this is just cleaning up old cruft.

app/jobs/regular/export_user_archive.rb

@@ -362,7 +362,7 @@ module Jobs
       # Most forums should not have post_action records other than flags and likes, but they are possible in historical oddities.
       PostAction
         .where(user_id: @current_user.id)
-        .where.not(post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like], PostActionType.types[:bookmark]])
+        .where.not(post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like]])
         .exists?
     end
 
@@ -371,7 +371,7 @@ module Jobs
       PostAction
         .with_deleted
         .where(user_id: @current_user.id)
-        .where.not(post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like], PostActionType.types[:bookmark]])
+        .where.not(post_action_type_id: PostActionType.flag_types.values + [PostActionType.types[:like]])
         .order(:created_at)
         .each do |pa|
         yield [

app/models/post_action.rb

@@ -150,10 +150,6 @@ class PostAction < ActiveRecord::Base
     save
   end
 
-  def is_bookmark?
-    post_action_type_id == PostActionType.types[:bookmark]
-  end
-
   def is_like?
     post_action_type_id == PostActionType.types[:like]
   end
@@ -169,11 +165,11 @@ class PostAction < ActiveRecord::Base
 
   # A custom rate limiter for this model
   def post_action_rate_limiter
-    return unless is_flag? || is_bookmark? || is_like?
+    return unless is_flag? || is_like?
 
     return @rate_limiter if @rate_limiter.present?
 
-    %w(like flag bookmark).each do |type|
+    %w(like flag).each do |type|
       if public_send("is_#{type}?")
         limit = SiteSetting.get("max_#{type}s_per_day")
 

app/models/post_action_type.rb

@@ -66,8 +66,9 @@ class PostActionType < ActiveRecord::Base
 
     def types
       unless @types
+        # NOTE: Previously bookmark was type 1 but that has been superseded
+        # by the separate Bookmark model and functionality
         @types = Enum.new(
-          bookmark: 1,
           like: 2
         )
         @types.merge!(flag_settings.flag_types)

app/models/topic_list.rb

@@ -104,19 +104,12 @@ class TopicList
     post_action_type =
       if @current_user
         if @opts[:filter].present?
-          if @opts[:filter] == "bookmarked"
+          if @opts[:filter] == "liked"
-            PostActionType.types[:bookmark]
-          elsif @opts[:filter] == "liked"
             PostActionType.types[:like]
           end
         end
       end
 
-    # Include bookmarks if you have bookmarked topics
-    if @current_user && !post_action_type
-      post_action_type = PostActionType.types[:bookmark] if @topic_lookup.any? { |_, tu| tu && tu.bookmarked }
-    end
-
     # Data for bookmarks or likes
     post_action_lookup = PostAction.lookup_for(@current_user, @topics, post_action_type) if post_action_type
 

app/models/user_action.rb

@@ -10,7 +10,6 @@ class UserAction < ActiveRecord::Base
 
   LIKE = 1
   WAS_LIKED = 2
-  BOOKMARK = 3
   NEW_TOPIC = 4
   REPLY = 5
   RESPONSE = 6
@@ -32,7 +31,6 @@ class UserAction < ActiveRecord::Base
     WAS_LIKED,
     MENTION,
     QUOTE,
-    BOOKMARK,
     EDIT,
     SOLVED,
     ASSIGNED,
@@ -42,7 +40,8 @@ class UserAction < ActiveRecord::Base
     @types ||= Enum.new(
       like: 1,
       was_liked: 2,
-      bookmark: 3,
+      # NOTE: Previously type 3 was bookmark but this was removed when we
+      # changed to using the Bookmark model.
       new_topic: 4,
       reply: 5,
       response: 6,
@@ -416,10 +415,6 @@ class UserAction < ActiveRecord::Base
       builder.where("t.visible")
     end
 
-    unless guardian.can_see_notifications?(User.where(id: user_id).first)
-      builder.where("a.action_type not in (#{BOOKMARK})")
-    end
-
     filter_private_messages(builder, user_id, guardian, ignore_private_messages)
     filter_categories(builder, guardian)
   end

app/serializers/post_serializer.rb

@@ -282,7 +282,7 @@ class PostSerializer < BasicPostSerializer
     result = []
     can_see_post = scope.can_see_post?(object)
 
-    PostActionType.types.except(:bookmark).each do |sym, id|
+    PostActionType.types.each do |sym, id|
       count_col = "#{sym}_count".to_sym
 
       count = object.public_send(count_col) if object.respond_to?(count_col)

app/serializers/topic_list_item_serializer.rb

@@ -11,7 +11,6 @@ class TopicListItemSerializer < ListableTopicSerializer
              :category_id,
              :op_like_count,
              :pinned_globally,
-             :bookmarked_post_numbers,
              :liked_post_numbers,
              :featured_link,
              :featured_link_root_domain,
@@ -46,10 +45,6 @@ class TopicListItemSerializer < ListableTopicSerializer
     object.participants_summary || []
   end
 
-  def include_bookmarked_post_numbers?
-    include_post_action? :bookmark
-  end
-
   def include_liked_post_numbers?
     include_post_action? :like
   end
@@ -64,10 +59,6 @@ class TopicListItemSerializer < ListableTopicSerializer
     object.user_data.post_action_data[PostActionType.types[:like]]
   end
 
-  def bookmarked_post_numbers
-    object.user_data.post_action_data[PostActionType.types[:bookmark]]
-  end
-
   def include_participants?
     object.private_message?
   end

app/services/user_action_manager.rb

@@ -110,7 +110,6 @@ private
   end
 
   def self.post_action_rows(post_action)
-    action = UserAction::BOOKMARK if post_action.is_bookmark?
     action = UserAction::LIKE if post_action.is_like?
     return [] unless action
 

db/fixtures/003_post_action_types.rb

@@ -1,12 +1,5 @@
 # frozen_string_literal: true
 
-PostActionType.seed do |s|
-  s.id = PostActionType.types[:bookmark]
-  s.name_key = 'bookmark'
-  s.is_flag = false
-  s.position = 1
-end
-
 PostActionType.seed do |s|
   s.id = PostActionType.types[:like]
   s.name_key = 'like'

lib/guardian/post_guardian.rb

@@ -232,9 +232,6 @@ module PostGuardian
   def can_delete_post_action?(post_action)
     return false unless is_my_own?(post_action) && !post_action.is_private_message?
 
-    # Bookmarks do not have a time constraint
-    return true if post_action.is_bookmark?
-
     post_action.created_at > SiteSetting.post_undo_action_window_mins.minutes.ago
   end
 

lib/post_action_creator.rb

@@ -19,7 +19,7 @@ class PostActionCreator
       ).perform
     end
 
-    [:like, :off_topic, :spam, :inappropriate, :bookmark].each do |action|
+    [:like, :off_topic, :spam, :inappropriate].each do |action|
       define_method(action) do |created_by, post, silent = false|
         create(created_by, post, action, silent: silent)
       end
@@ -285,7 +285,6 @@ private
     post_action
   rescue ActiveRecord::RecordNotUnique
     # can happen despite being .create
-    # since already bookmarked
     PostAction.where(where_attrs).first
   end
 

lib/topic_query.rb

@@ -793,9 +793,7 @@ class TopicQuery
 
     if (filter = (options[:filter] || options[:f])) && @user
       action =
-        if filter == "bookmarked"
+        if filter == "liked"
-          PostActionType.types[:bookmark]
-        elsif filter == "liked"
           PostActionType.types[:like]
         end
       if action

plugins/discourse-narrative-bot/plugin.rb

@@ -267,8 +267,6 @@ after_initialize do
           self.post_action_type_id == PostActionType.types[:inappropriate] ? "flag" : "reply"
         when PostActionType.types[:like]
           "like"
-        when PostActionType.types[:bookmark]
-          "bookmark"
         end
 
       if input

spec/fabricators/user_action_fabricator.rb

@@ -2,5 +2,5 @@
 
 Fabricator(:user_action) do
   user
-  action_type UserAction::BOOKMARK
+  action_type UserAction::EDIT
 end

spec/lib/guardian_spec.rb

@@ -431,7 +431,6 @@ describe Guardian do
       guardian = Guardian.new(user)
       expect(guardian.can_see_post_actors?(nil, PostActionType.types[:like])).to be_falsey
       expect(guardian.can_see_post_actors?(topic, PostActionType.types[:like])).to be_truthy
-      expect(guardian.can_see_post_actors?(topic, PostActionType.types[:bookmark])).to be_falsey
       expect(guardian.can_see_post_actors?(topic, PostActionType.types[:off_topic])).to be_falsey
       expect(guardian.can_see_post_actors?(topic, PostActionType.types[:spam])).to be_falsey
       expect(guardian.can_see_post_actors?(topic, PostActionType.types[:notify_user])).to be_falsey
@@ -2013,21 +2012,6 @@ describe Guardian do
 
   end
 
-  context "can_delete_post_action?" do
-    fab!(:post) { Fabricate(:post) }
-
-    it "allows us to remove a bookmark" do
-      pa = PostActionCreator.bookmark(user, post).post_action
-      expect(Guardian.new(user).can_delete_post_action?(pa)).to eq(true)
-    end
-
-    it "allows us to remove a very old bookmark" do
-      pa = PostActionCreator.bookmark(user, post).post_action
-      pa.update(created_at: 2.years.ago)
-      expect(Guardian.new(user).can_delete_post_action?(pa)).to eq(true)
-    end
-  end
-
   context 'can_delete?' do
 
     it 'returns false with a nil object' do

spec/lib/post_action_destroyer_spec.rb

@@ -62,25 +62,5 @@ describe PostActionDestroyer do
         expect(messages.last.data[:type]).to eq(:acted)
       end
     end
-
-    context 'not notifyable type' do
-      before do
-        PostActionCreator.new(user, post, PostActionType.types[:bookmark]).perform
-      end
-
-      it 'destroys the post action' do
-        expect {
-          PostActionDestroyer.destroy(user, post, :bookmark)
-        }.to change { PostAction.count }.by(-1)
-      end
-
-      it 'doesn’t notify subscribers' do
-        messages = MessageBus.track_publish do
-          PostActionDestroyer.destroy(user, post, :bookmark)
-        end
-
-        expect(messages).to be_blank
-      end
-    end
   end
 end

spec/lib/post_destroyer_spec.rb

@@ -876,7 +876,6 @@ describe PostDestroyer do
 
   describe "post actions" do
     let(:second_post) { Fabricate(:post, topic_id: post.topic_id) }
-    let!(:bookmark) { PostActionCreator.create(moderator, second_post, :bookmark).post_action }
     let(:flag_result) { PostActionCreator.off_topic(moderator, second_post) }
     let!(:flag) { flag_result.post_action }
 
@@ -888,14 +887,11 @@ describe PostDestroyer do
       url = second_post.url
       PostDestroyer.new(moderator, second_post).destroy
 
-      expect(PostAction.find_by(id: bookmark.id)).to eq(nil)
-
       off_topic = PostAction.find_by(id: flag.id)
       expect(off_topic).not_to eq(nil)
       expect(off_topic.agreed_at).not_to eq(nil)
 
       second_post.reload
-      expect(second_post.bookmark_count).to eq(0)
       expect(second_post.off_topic_count).to eq(1)
 
       jobs = Jobs::SendSystemMessage.jobs
@@ -948,11 +944,6 @@ describe PostDestroyer do
       expect(Jobs::SendSystemMessage.jobs.size).to eq(0)
       expect(ReviewableFlaggedPost.pending.count).to eq(0)
     end
-
-    it "should set the deleted_public_actions custom field" do
-      PostDestroyer.new(moderator, second_post).destroy
-      expect(second_post.custom_fields["deleted_public_actions"]).to eq("#{bookmark.id}")
-    end
   end
 
   describe "user actions" do
@@ -968,12 +959,10 @@ describe PostDestroyer do
     end
 
     it "should delete the user actions" do
-      bookmark = create_user_action(UserAction::BOOKMARK)
       like = create_user_action(UserAction::LIKE)
 
       PostDestroyer.new(moderator, second_post).destroy
 
-      expect(UserAction.find_by(id: bookmark.id)).to be_nil
       expect(UserAction.find_by(id: like.id)).to be_nil
     end
   end

spec/lib/topic_query_spec.rb

@@ -159,25 +159,6 @@ describe TopicQuery do
     end
   end
 
-  context 'bookmarks' do
-    it "filters and returns bookmarks correctly" do
-      post = Fabricate(:post)
-      reply = Fabricate(:post, topic: post.topic)
-
-      post2 = Fabricate(:post)
-
-      PostActionCreator.create(user, post, :bookmark)
-      PostActionCreator.create(user, reply, :bookmark)
-      TopicUser.change(user, post.topic, notification_level: 1)
-      TopicUser.change(user, post2.topic, notification_level: 1)
-
-      query = TopicQuery.new(user, filter: 'bookmarked').list_latest
-
-      expect(query.topics.length).to eq(1)
-      expect(query.topics.first.user_data.post_action_data).to eq(PostActionType.types[:bookmark] => [1, 2])
-    end
-  end
-
   context 'tracked' do
     it "filters tracked topics correctly" do
       SiteSetting.tagging_enabled = true

spec/models/post_action_spec.rb

@@ -9,7 +9,6 @@ describe PostAction do
   fab!(:admin) { Fabricate(:admin) }
   fab!(:post) { Fabricate(:post) }
   fab!(:second_post) { Fabricate(:post, topic: post.topic) }
-  let(:bookmark) { PostAction.new(user_id: post.user_id, post_action_type_id: PostActionType.types[:bookmark] , post_id: post.id) }
 
   def value_for(user_id, dt)
     GivenDailyLike.find_for(user_id, dt).pluck(:likes_given)[0] || 0
@@ -131,31 +130,6 @@ describe PostAction do
 
       tu = TopicUser.get(post.topic, codinghorror)
       expect(tu.liked).to be true
-      expect(tu.bookmarked).to be false
-    end
-
-  end
-
-  describe "when a user bookmarks something" do
-    it "increases the post's bookmark count when saved" do
-      expect { bookmark.save; post.reload }.to change(post, :bookmark_count).by(1)
-    end
-
-    describe 'when deleted' do
-
-      before do
-        bookmark.save
-        post.reload
-        @topic = post.topic
-        @topic.reload
-        bookmark.deleted_at = DateTime.now
-        bookmark.save
-      end
-
-      it 'reduces the bookmark count of the post' do
-        expect { post.reload }.to change(post, :bookmark_count).by(-1)
-      end
-
     end
   end
 
@@ -879,7 +853,7 @@ describe PostAction do
   describe ".lookup_for" do
     it "returns the correct map" do
       user = Fabricate(:user)
-      post_action = PostActionCreator.create(user, post, :bookmark).post_action
+      post_action = PostActionCreator.create(user, post, :like).post_action
       map = PostAction.lookup_for(user, [post.topic], post_action.post_action_type_id)
 
       expect(map).to eq(post.topic_id => [post.post_number])

spec/models/post_action_type_spec.rb

@@ -26,10 +26,6 @@ describe PostActionType do
         @types = PostActionType.types
       end
 
-      it "'bookmark' should be at 1st position" do
-        expect(@types[:bookmark]).to eq(1)
-      end
-
       it "'spam' should be at 8th position" do
         expect(@types[:spam]).to eq(8)
       end

spec/models/user_action_spec.rb

@@ -59,13 +59,12 @@ describe UserAction do
         log_test_action
         log_test_action(action_type: UserAction::GOT_PRIVATE_MESSAGE)
         log_test_action(action_type: UserAction::NEW_TOPIC, target_topic_id: public_topic.id, target_post_id: public_post.id)
-        log_test_action(action_type: UserAction::BOOKMARK)
 
         Jobs.run_immediately!
         PostActionNotifier.enable
 
         mystats = stats_for_user(user)
-        expecting = [UserAction::NEW_TOPIC, UserAction::NEW_PRIVATE_MESSAGE, UserAction::GOT_PRIVATE_MESSAGE, UserAction::BOOKMARK].sort
+        expecting = [UserAction::NEW_TOPIC, UserAction::NEW_PRIVATE_MESSAGE, UserAction::GOT_PRIVATE_MESSAGE].sort
         expect(mystats).to eq(expecting)
 
         expect(stream(user).map(&:action_type))
@@ -280,72 +279,6 @@ describe UserAction do
     end
   end
 
-  describe 'when user bookmarks' do
-    before do
-      @post = Fabricate(:post)
-      @user = @post.user
-      PostActionCreator.create(@user, @post, :bookmark)
-      @action = @user.user_actions.find_by(action_type: UserAction::BOOKMARK)
-    end
-
-    it 'creates the bookmark, and removes it properly' do
-      expect(@action.action_type).to eq(UserAction::BOOKMARK)
-      expect(@action.target_post_id).to eq(@post.id)
-      expect(@action.acting_user_id).to eq(@user.id)
-      expect(@action.user_id).to eq(@user.id)
-
-      PostActionDestroyer.destroy(@user, @post, :bookmark)
-      expect(@user.user_actions.find_by(action_type: UserAction::BOOKMARK)).to eq(nil)
-    end
-  end
-
-  describe 'secures private messages' do
-
-    fab!(:user) do
-      Fabricate(:user)
-    end
-
-    fab!(:user2) do
-      Fabricate(:user)
-    end
-
-    let(:private_message) do
-      PostCreator.create(user,
-                          raw: 'this is a private message',
-                          title: 'this is the pm title',
-                          target_usernames: user2.username,
-                          archetype: Archetype::private_message
-                        )
-    end
-
-    def count_bookmarks
-      UserAction.stream(
-        user_id: user.id,
-        action_types: [UserAction::BOOKMARK],
-        ignore_private_messages: false,
-        guardian: Guardian.new(user)
-      ).count
-    end
-
-    it 'correctly secures stream' do
-      PostActionCreator.create(user, private_message, :bookmark)
-
-      expect(count_bookmarks).to eq(1)
-
-      private_message.topic.topic_allowed_users.where(user_id: user.id).destroy_all
-
-      expect(count_bookmarks).to eq(0)
-
-      group = Fabricate(:group)
-      group.add(user)
-      private_message.topic.topic_allowed_groups.create(group_id: group.id)
-
-      expect(count_bookmarks).to eq(1)
-
-    end
-
-  end
-
   describe 'synchronize_target_topic_ids' do
     it 'correct target_topic_id' do
       post = Fabricate(:post)

spec/models/user_spec.rb

@@ -265,31 +265,6 @@ describe User do
     end
   end
 
-  describe 'bookmark' do
-    before_all do
-      @post = Fabricate(:post)
-    end
-
-    it "creates a bookmark with the true parameter" do
-      expect {
-        PostActionCreator.create(@post.user, @post, :bookmark)
-      }.to change(PostAction, :count).by(1)
-    end
-
-    describe 'when removing a bookmark' do
-      before do
-        PostActionCreator.create(@post.user, @post, :bookmark)
-      end
-
-      it 'reduces the bookmark count of the post' do
-        active = PostAction.where(deleted_at: nil)
-        expect {
-          PostActionDestroyer.destroy(@post.user, @post, :bookmark)
-        }.to change(active, :count).by(-1)
-      end
-    end
-  end
-
   describe 'delete posts in batches' do
     fab!(:post1) { Fabricate(:post) }
     fab!(:user) { post1.user }

spec/requests/post_actions_controller_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe PostActionsController do
       end
 
       it "returns 404 when the post action type doesn't exist for that user" do
-        delete "/post_actions/#{post.id}.json", params: { post_action_type_id: PostActionType.types[:bookmark] }
+        delete "/post_actions/#{post.id}.json", params: { post_action_type_id: PostActionType.types[:like] }
         expect(response.status).to eq(404)
       end
 
@@ -32,36 +32,31 @@ RSpec.describe PostActionsController do
           PostAction.create!(
             user_id: user.id,
             post_id: post.id,
-            post_action_type_id: PostActionType.types[:bookmark]
+            post_action_type_id: PostActionType.types[:like]
           )
         end
 
         it 'returns success' do
-          delete "/post_actions/#{post.id}.json", params: { post_action_type_id: PostActionType.types[:bookmark] }
+          delete "/post_actions/#{post.id}.json", params: { post_action_type_id: PostActionType.types[:like] }
           expect(response.status).to eq(200)
         end
 
         it 'deletes the action' do
           delete "/post_actions/#{post.id}.json", params: {
-            post_action_type_id: PostActionType.types[:bookmark]
+            post_action_type_id: PostActionType.types[:like]
           }
 
           expect(response.status).to eq(200)
           expect(PostAction.exists?(
             user_id: user.id,
             post_id: post.id,
-            post_action_type_id: PostActionType.types[:bookmark],
+            post_action_type_id: PostActionType.types[:like],
             deleted_at: nil
           )).to eq(false)
         end
 
         it "isn't deleted when the user doesn't have permission" do
-          pa = PostAction.create!(
+          post_action.update!(created_at: 1.day.ago)
-            post: post,
-            user: user,
-            post_action_type_id: PostActionType.types[:like],
-            created_at: 1.day.ago
-          )
 
           delete "/post_actions/#{post.id}.json", params: {
             post_action_type_id: PostActionType.types[:like]
@@ -85,7 +80,7 @@ RSpec.describe PostActionsController do
 
       post "/post_actions.json", params: {
         id: pm.id,
-        post_action_type_id: PostActionType.types[:bookmark]
+        post_action_type_id: PostActionType.types[:like]
       }
 
       expect(response.status).to eq(403)