Remove legacy vote post action code. (#6009)

app/models/post.rb

@@ -10,6 +10,9 @@ require 'archetype'
 require 'digest/sha1'
 
 class Post < ActiveRecord::Base
+  # TODO: Remove this after 19th Dec 2018
+  self.ignored_columns = %w{vote_count}
+
   include RateLimiter::OnCreateRecord
   include Trashable
   include Searchable
@@ -834,7 +837,6 @@ end
 #  score                   :float
 #  reads                   :integer          default(0), not null
 #  post_type               :integer          default(1), not null
-#  vote_count              :integer          default(0), not null
 #  sort_order              :integer
 #  last_editor_id          :integer
 #  hidden                  :boolean          default(FALSE), not null

app/models/post_action.rb

@@ -470,9 +470,6 @@ class PostAction < ActiveRecord::Base
 
     # We probably want to refactor this method to something cleaner.
     case post_action_type_key
-    when :vote
-      # Voting also changes the sort_order
-      Post.where(id: post_id).update_all ["vote_count = :count, sort_order = :max - :count", count: count, max: Topic.max_sort_order]
     when :like
       # 'like_score' is weighted higher for staff accounts
       score = PostAction.joins(:user)

app/models/post_action_type.rb

@@ -70,8 +70,7 @@ class PostActionType < ActiveRecord::Base
       unless @types
         @types = Enum.new(
           bookmark: 1,
-          like: 2,
-          vote: 5
+          like: 2
         )
         @types.merge!(flag_settings.flag_types)
       end

app/models/topic.rb

@@ -13,6 +13,9 @@ require_dependency 'topic_posters_summary'
 require_dependency 'topic_featured_users'
 
 class Topic < ActiveRecord::Base
+  # TODO: Remove this after 19th Dec 2018
+  self.ignored_columns = %w{vote_count}
+
   class UserExists < StandardError; end
   include ActionView::Helpers::SanitizeHelper
   include RateLimiter::OnCreateRecord
@@ -44,10 +47,6 @@ class Topic < ActiveRecord::Base
     end
   end
 
-  def self.max_sort_order
-    @max_sort_order ||= (2**31) - 1
-  end
-
   def self.max_fancy_title_length
     400
   end
@@ -1429,7 +1428,6 @@ end
 #  archived                  :boolean          default(FALSE), not null
 #  bumped_at                 :datetime         not null
 #  has_summary               :boolean          default(FALSE), not null
-#  vote_count                :integer          default(0), not null
 #  archetype                 :string           default("regular"), not null
 #  featured_user4_id         :integer
 #  notify_moderators_count   :integer          default(0), not null

config/locales/client.en.yml

@@ -2110,7 +2110,6 @@ en:
           inappropriate: "Undo flag"
           bookmark: "Undo bookmark"
           like: "Undo like"
-          vote: "Undo vote"
         people:
           off_topic: "flagged this as off-topic"
           spam: "flagged this as spam"
@@ -2122,7 +2121,6 @@ en:
           like_capped:
             one: "and {{count}} other liked this"
             other: "and {{count}} others liked this"
-          vote: "voted for this"
         by_you:
           off_topic: "You flagged this as off-topic"
           spam: "You flagged this as spam"
@@ -2131,7 +2129,6 @@ en:
           notify_user: "You sent a message to this user"
           bookmark: "You bookmarked this post"
           like: "You liked this"
-          vote: "You voted for this post"
         by_you_and_others:
           off_topic:
             one: "You and 1 other flagged this as off-topic"
@@ -2154,9 +2151,6 @@ en:
           like:
             one: "You and 1 other liked this"
             other: "You and {{count}} other people liked this"
-          vote:
-            one: "You and 1 other voted for this post"
-            other: "You and {{count}} other people voted for this post"
         by_others:
           off_topic:
             one: "1 person flagged this as off-topic"
@@ -2179,9 +2173,6 @@ en:
           like:
             one: "1 person liked this"
             other: "{{count}} people liked this"
-          vote:
-            one: "1 person voted for this post"
-            other: "{{count}} people voted for this post"
 
       delete:
         confirm:

config/locales/server.en.yml

@@ -758,11 +758,6 @@ en:
       description: 'Like this post'
       short_description: 'Like this post'
       long_form: 'liked this'
-    vote:
-      title: 'Vote'
-      description: 'Vote for this post'
-      short_description: 'Vote for this post'
-      long_form: 'voted for this post'
 
   user_activity:
     no_default:

db/fixtures/000_delayed_drops.rb

@@ -54,34 +54,70 @@ Migration::ColumnDropper.drop(
 
 Migration::ColumnDropper.drop(
   table: 'topics',
-  after_migration: 'DropUnreadTrackingColumns',
-  columns: %w{
-    inappropriate_count
-    bookmark_count
-    off_topic_count
-    illegal_count
-    notify_user_count
-    last_unread_at
-  },
-  on_drop: ->() {
-    STDERR.puts "Removing superflous topic columns!"
-  }
-)
-
-Migration::ColumnDropper.drop(
-  table: 'topics',
-  after_migration: 'RemoveAutoCloseColumnsFromTopics',
+  after_migration: 'DropVoteCountFromTopicsAndPosts',
   columns: %w{
     auto_close_at
     auto_close_user_id
     auto_close_started_at
     auto_close_based_on_last_post
     auto_close_hours
+    inappropriate_count
+    bookmark_count
+    off_topic_count
+    illegal_count
+    notify_user_count
+    last_unread_at
+    vote_count
   },
   on_drop: ->() {
     STDERR.puts "Removing superflous topic columns!"
+  }
+)
+
+VIEW_NAME = "badge_posts".freeze
+
+def badge_posts_view_exists?
+  sql = <<~SQL
+  SELECT 1
+  FROM pg_catalog.pg_views
+  WHERE schemaname
+  IN ('public')
+  AND viewname = '#{VIEW_NAME}';
+  SQL
+
+  DB.exec(sql) == 1
+end
+
+Migration::ColumnDropper.drop(
+  table: 'posts',
+  after_migration: 'DropVoteCountFromTopicsAndPosts',
+  columns: %w{
+    vote_count
   },
-  delay: 3600
+  on_drop: ->() {
+    STDERR.puts "Removing superflous post columns!"
+
+    DB.exec("DROP VIEW #{VIEW_NAME}")
+    raise "Failed to drop '#{VIEW_NAME}' view" if badge_posts_view_exists?
+  },
+  after_drop: -> () {
+    sql = <<~SQL
+    CREATE VIEW #{VIEW_NAME} AS
+    SELECT p.*
+    FROM posts p
+    JOIN topics t ON t.id = p.topic_id
+    JOIN categories c ON c.id = t.category_id
+    WHERE c.allow_badges AND
+          p.deleted_at IS NULL AND
+          t.deleted_at IS NULL AND
+          NOT c.read_restricted AND
+          t.visible AND
+          p.post_type IN (1,2,3)
+    SQL
+
+    DB.exec(sql)
+    raise "Failed to create '#{VIEW_NAME}' view" unless badge_posts_view_exists?
+  }
 )
 
 Migration::ColumnDropper.drop(

db/fixtures/003_post_action_types.rb

@@ -27,13 +27,6 @@ PostActionType.seed do |s|
   s.position = 4
 end
 
-PostActionType.seed do |s|
-  s.id = PostActionType.types[:vote]
-  s.name_key = 'vote'
-  s.is_flag = false
-  s.position = 5
-end
-
 PostActionType.seed do |s|
   s.id = PostActionType.types[:spam]
   s.name_key = 'spam'

db/migrate/20120928170023_add_sort_order_to_posts.rb

@@ -3,11 +3,5 @@ class AddSortOrderToPosts < ActiveRecord::Migration[4.2]
     add_column :posts, :sort_order, :integer
     remove_index :posts, :user_id
     execute "UPDATE posts AS p SET sort_order = post_number FROM forum_threads AS ft WHERE ft.id = p.forum_thread_id AND ft.archetype_id = 1"
-    execute "UPDATE posts AS p SET sort_order =
-                CASE WHEN post_number = 1 THEN 1
-                     ELSE 2147483647 - p.vote_count
-                END
-             FROM forum_threads AS ft
-             WHERE ft.id = p.forum_thread_id AND ft.archetype_id = 2"
   end
 end

db/migrate/20180619091608_drop_vote_count_from_topics_and_posts.rb

@@ -0,0 +1,9 @@
+class DropVoteCountFromTopicsAndPosts < ActiveRecord::Migration[5.2]
+  def up
+    # Delayed drop
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

db/migrate/20180619092320_clean_up_vote_post_action.rb

@@ -0,0 +1,10 @@
+class CleanUpVotePostAction < ActiveRecord::Migration[5.2]
+  def up
+    execute "DELETE FROM post_actions WHERE post_action_type_id = 5"
+    execute "DELETE FROM post_action_types WHERE id = 5"
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/migration/base_dropper.rb

@@ -1,8 +1,9 @@
 module Migration
   class BaseDropper
-    def initialize(after_migration, delay, on_drop)
+    def initialize(after_migration, delay, on_drop, after_drop)
       @after_migration = after_migration
       @on_drop = on_drop
+      @after_drop = after_drop
 
       # in production we need some extra delay to allow for slow migrations
       @delay = delay || (Rails.env.production? ? 3600 : 0)
@@ -12,6 +13,7 @@ module Migration
       if droppable?
         @on_drop&.call
         execute_drop!
+        @after_drop&.call
 
         Discourse.reset_active_record_cache
       end

lib/migration/column_dropper.rb

@@ -2,11 +2,13 @@ require_dependency 'migration/base_dropper'
 
 module Migration
   class ColumnDropper < BaseDropper
-    def self.drop(table:, after_migration:, columns:, delay: nil, on_drop: nil)
+    def self.drop(table:, after_migration:, columns:, delay: nil, on_drop: nil, after_drop: nil)
       validate_table_name(table)
       columns.each { |column| validate_column_name(column) }
 
-      ColumnDropper.new(table, columns, after_migration, delay, on_drop).delayed_drop
+      ColumnDropper.new(
+        table, columns, after_migration, delay, on_drop, after_drop
+      ).delayed_drop
     end
 
     def self.mark_readonly(table_name, column_name)
@@ -24,8 +26,8 @@ module Migration
 
     private
 
-    def initialize(table, columns, after_migration, delay, on_drop)
-      super(after_migration, delay, on_drop)
+    def initialize(table, columns, after_migration, delay, on_drop, after_drop)
+      super(after_migration, delay, on_drop, after_drop)
 
       @table = table
       @columns = columns

lib/migration/table_dropper.rb

@@ -2,17 +2,21 @@ require_dependency 'migration/base_dropper'
 
 module Migration
   class Migration::TableDropper < BaseDropper
-    def self.delayed_drop(table_name:, after_migration:, delay: nil, on_drop: nil)
+    def self.delayed_drop(table_name:, after_migration:, delay: nil, on_drop: nil, after_drop: nil)
       validate_table_name(table_name)
 
-      TableDropper.new(table_name, nil, after_migration, delay, on_drop).delayed_drop
+      TableDropper.new(
+        table_name, nil, after_migration, delay, on_drop, after_drop
+      ).delayed_drop
     end
 
-    def self.delayed_rename(old_name:, new_name:, after_migration:, delay: nil, on_drop: nil)
+    def self.delayed_rename(old_name:, new_name:, after_migration:, delay: nil, on_drop: nil, after_drop: nil)
       validate_table_name(old_name)
       validate_table_name(new_name)
 
-      TableDropper.new(old_name, new_name, after_migration, delay, on_drop).delayed_drop
+      TableDropper.new(
+        old_name, new_name, after_migration, delay, on_drop, after_drop
+      ).delayed_drop
     end
 
     def self.read_only_table(table_name)
@@ -29,8 +33,8 @@ module Migration
 
     private
 
-    def initialize(old_name, new_name, after_migration, delay, on_drop)
-      super(after_migration, delay, on_drop)
+    def initialize(old_name, new_name, after_migration, delay, on_drop, after_drop)
+      super(after_migration, delay, on_drop, after_drop)
 
       @old_name = old_name
       @new_name = new_name

spec/components/guardian_spec.rb

@@ -407,7 +407,6 @@ describe Guardian do
       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[:vote])).to be_truthy
       expect(guardian.can_see_post_actors?(topic, PostActionType.types[:notify_user])).to be_falsey
 
       expect(Guardian.new(moderator).can_see_post_actors?(topic, PostActionType.types[:notify_user])).to be_truthy
@@ -1037,18 +1036,6 @@ describe Guardian do
         expect(guardian.post_can_act?(post, :vote)).to be_truthy
       end
 
-      it "doesn't allow voting if the user has an action from voting already" do
-        expect(guardian.post_can_act?(post, :vote, opts: {
-          taken_actions: { PostActionType.types[:vote] => 1 }
-        })).to be_falsey
-      end
-
-      it "allows voting if the user has performed a different action" do
-        expect(guardian.post_can_act?(post, :vote, opts: {
-          taken_actions: { PostActionType.types[:like] => 1 }
-        })).to be_truthy
-      end
-
       it "isn't allowed on archived topics" do
         topic.archived = true
         expect(Guardian.new(user).post_can_act?(post, :like)).to be_falsey

spec/components/migration/column_dropper_spec.rb

@@ -40,6 +40,7 @@ RSpec.describe Migration::ColumnDropper do
 
     it "can correctly drop columns after correct delay" do
       dropped_proc_called = false
+      after_dropped_proc_called = false
       update_first_migration_date(2.years.ago)
 
       Migration::ColumnDropper.drop(
@@ -47,35 +48,42 @@ RSpec.describe Migration::ColumnDropper do
         after_migration: migration_name,
         columns: ['junk'],
         delay: 20.minutes,
-        on_drop: ->() { dropped_proc_called = true }
+        on_drop: ->() { dropped_proc_called = true },
+        after_drop: ->() { after_dropped_proc_called = true }
       )
 
       expect(has_column?('topics', 'junk')).to eq(true)
       expect(dropped_proc_called).to eq(false)
+      expect(dropped_proc_called).to eq(false)
 
       Migration::ColumnDropper.drop(
         table: 'topics',
         after_migration: migration_name,
         columns: ['junk'],
         delay: 10.minutes,
-        on_drop: ->() { dropped_proc_called = true }
+        on_drop: ->() { dropped_proc_called = true },
+        after_drop: ->() { after_dropped_proc_called = true }
       )
 
       expect(has_column?('topics', 'junk')).to eq(false)
       expect(dropped_proc_called).to eq(true)
+      expect(after_dropped_proc_called).to eq(true)
 
       dropped_proc_called = false
+      after_dropped_proc_called = false
 
       Migration::ColumnDropper.drop(
         table: 'topics',
         after_migration: migration_name,
         columns: ['junk'],
         delay: 10.minutes,
-        on_drop: ->() { dropped_proc_called = true }
+        on_drop: ->() { dropped_proc_called = true },
+        after_drop: ->() { after_dropped_proc_called = true }
       )
 
       # it should call "on_drop" only when there are columns to drop
       expect(dropped_proc_called).to eq(false)
+      expect(after_dropped_proc_called).to eq(false)
     end
 
     it "drops the columns immediately if the first migration was less than 10 minutes ago" do

spec/serializers/post_serializer_spec.rb

@@ -29,10 +29,10 @@ describe PostSerializer do
     end
 
     it "displays the correct info" do
-      expect(visible_actions_for(actor).sort).to eq([:like, :notify_user, :spam, :vote])
-      expect(visible_actions_for(post.user).sort).to eq([:like, :vote])
-      expect(visible_actions_for(nil).sort).to eq([:like, :vote])
-      expect(visible_actions_for(admin).sort).to eq([:like, :notify_user, :spam, :vote])
+      expect(visible_actions_for(actor).sort).to eq([:like, :notify_user, :spam])
+      expect(visible_actions_for(post.user).sort).to eq([:like])
+      expect(visible_actions_for(nil).sort).to eq([:like])
+      expect(visible_actions_for(admin).sort).to eq([:like, :notify_user, :spam])
     end
 
     it "can't flag your own post to notify yourself" do