FIX: Prevent column name conflicts in reviewable code (#9753)

We were getting errors like this in Reviewables in some cases:

```
ActiveRecord::StatementInvalid (PG::AmbiguousColumn: ERROR:  column reference "category_id" is ambiguous
LINE 4: ...TRUE) OR (reviewable_by_group_id IN (NULL))) AND (category_i...
```

The problem that was making everything go boom is that plugins can add their own custom filters for Reviewables. If one is doing an INNER JOIN on topics, which has its own category_id column, we would get the above AmbiguousColumn error. The solution here is to just make all references to the reviewable columns in the list_for and viewable_by code prefixed by the table name e.g. reviewables.category_id.

app/models/reviewable.rb

@@ -406,7 +406,7 @@ class Reviewable < ActiveRecord::Base
   def self.viewable_by(user, order: nil, preload: true)
     return none unless user.present?
 
-    result = self.order(order || 'score desc, created_at desc')
+    result = self.order(order || 'reviewables.score desc, reviewables.created_at desc')
 
     if preload
       result = result.includes(
@@ -422,10 +422,10 @@ class Reviewable < ActiveRecord::Base
     group_ids = SiteSetting.enable_category_group_review? ? user.group_users.pluck(:group_id) : []
 
     result.where(
-      '(reviewable_by_moderator AND :staff) OR (reviewable_by_group_id IN (:group_ids))',
+      '(reviewables.reviewable_by_moderator AND :staff) OR (reviewables.reviewable_by_group_id IN (:group_ids))',
       staff: user.staff?,
       group_ids: group_ids
-    ).where("category_id IS NULL OR category_id IN (?)", Guardian.new(user).allowed_category_ids)
+    ).where("reviewables.category_id IS NULL OR reviewables.category_id IN (?)", Guardian.new(user).allowed_category_ids)
   end
 
   def self.pending_count(user)
@@ -451,13 +451,13 @@ class Reviewable < ActiveRecord::Base
 
     order = case sort_order
             when 'priority_asc'
-              'score ASC, created_at DESC'
+              'reviewables.score ASC, reviewables.created_at DESC'
             when 'created_at'
-              'created_at DESC, score DESC'
+              'reviewables.created_at DESC, reviewables.score DESC'
             when 'created_at_asc'
-              'created_at ASC, score DESC'
+              'reviewables.created_at ASC, reviewables.score DESC'
             else
-              'score DESC, created_at DESC'
+              'reviewables.score DESC, reviewables.created_at DESC'
     end
 
     if username.present?
@@ -469,19 +469,19 @@ class Reviewable < ActiveRecord::Base
     result = viewable_by(user, order: order)
 
     result = by_status(result, status)
-    result = result.where(type: type) if type
+    result = result.where('reviewables.type = ?', type) if type
-    result = result.where(category_id: category_id) if category_id
+    result = result.where('reviewables.category_id = ?', category_id) if category_id
-    result = result.where(topic_id: topic_id) if topic_id
+    result = result.where('reviewables.topic_id = ?', topic_id) if topic_id
-    result = result.where("created_at >= ?", from_date) if from_date
+    result = result.where("reviewables.created_at >= ?", from_date) if from_date
-    result = result.where("created_at <= ?", to_date) if to_date
+    result = result.where("reviewables.created_at <= ?", to_date) if to_date
 
     if min_score > 0 && status == :pending && type.nil?
       result = result.where(
-        "score >= ? OR type IN (?)",
+        "reviewables.score >= ? OR reviewables.type IN (?)",
         min_score, [ReviewableQueuedPost.name, ReviewableUser.name]
       )
     elsif min_score > 0
-      result = result.where("score >= ?", min_score)
+      result = result.where("reviewables.score >= ?", min_score)
     end
 
     if !custom_filters.empty?
@@ -497,7 +497,8 @@ class Reviewable < ActiveRecord::Base
     # If a reviewable doesn't have a target, allow us to filter on who created that reviewable.
     if user_id
       result = result.where(
-        "(target_created_by_id IS NULL AND created_by_id = :user_id) OR (target_created_by_id = :user_id)",
+        "(reviewables.target_created_by_id IS NULL AND reviewables.created_by_id = :user_id)
+        OR (reviewables.target_created_by_id = :user_id)",
         user_id: user_id
       )
     end

spec/models/reviewable_spec.rb

@@ -479,6 +479,33 @@ RSpec.describe Reviewable, type: :model do
       expect(results.size).to eq(1)
       expect(results.first).to eq first_reviewable
     end
+
+    context "when listing for a moderator with a custom filter that joins tables with same named columns" do
+      it "should not error" do
+        first_reviewable = Fabricate(:reviewable)
+        second_reviewable = Fabricate(:reviewable)
+        custom_filter = [
+          :troublemaker,
+          Proc.new do |results, value|
+            results.joins(<<~SQL
+          INNER JOIN posts p ON p.id = target_id
+          INNER JOIN topics t ON t.id = p.topic_id
+          INNER JOIN topic_custom_fields tcf ON tcf.topic_id = t.id
+          INNER JOIN users u ON u.id = tcf.value::integer
+                          SQL
+                         )
+              .where(target_type: Post.name)
+              .where('tcf.name = ?', 'troublemaker_user_id')
+              .where('u.username = ?', value)
+          end
+        ]
+
+        Reviewable.add_custom_filter(custom_filter)
+        mod = Fabricate(:moderator)
+        results = Reviewable.list_for(mod, additional_filters: { troublemaker: 'badguy' })
+        expect { results.first }.not_to raise_error
+      end
+    end
   end
 
   describe '.by_status' do