From 937653e519f5113b82cc27436c966ce8bf717663 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Mon, 17 Aug 2020 17:22:41 -0500
Subject: [PATCH] models: move html_data_attributes to policies.

Move html_data_attributes definitions from models to policies. Which
attributes are permitted as data-* attributes is a view level concern
and should be defined on the policy level, not the model level. Models
should be agnostic about how they're used in views.
---
 app/models/application_record.rb     | 10 ++++------
 app/models/ban.rb                    |  8 --------
 app/models/forum_post.rb             |  7 -------
 app/models/forum_topic.rb            |  4 ----
 app/models/ip_address.rb             |  4 ----
 app/models/post.rb                   |  4 ----
 app/models/user_feedback.rb          |  7 -------
 app/models/wiki_page.rb              |  7 -------
 app/policies/application_policy.rb   | 11 +++++++++++
 app/policies/ban_policy.rb           |  4 ++++
 app/policies/forum_post_policy.rb    |  4 ++++
 app/policies/forum_topic_policy.rb   |  4 ++++
 app/policies/ip_address_policy.rb    |  4 ++++
 app/policies/post_policy.rb          |  4 ++++
 app/policies/user_feedback_policy.rb |  4 ++++
 app/policies/wiki_page_policy.rb     |  4 ++++
 16 files changed, 43 insertions(+), 47 deletions(-)

diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index ec5af12fe..c65ae40d1 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -59,12 +59,10 @@ class ApplicationRecord < ActiveRecord::Base
       policy.api_attributes
     end
 
-    def html_data_attributes
-      data_attributes = self.class.columns.select do |column|
-        column.type.in?([:integer, :boolean]) && !column.array?
-      end.map(&:name).map(&:to_sym)
-
-      api_attributes & data_attributes
+    # XXX deprecated, shouldn't expose this as an instance method.
+    def html_data_attributes(user: CurrentUser.user)
+      policy = Pundit.policy([user, nil], self) || ApplicationPolicy.new([user, nil], self)
+      policy.html_data_attributes
     end
 
     def serializable_hash(options = {})
diff --git a/app/models/ban.rb b/app/models/ban.rb
index 4af738b30..408a33306 100644
--- a/app/models/ban.rb
+++ b/app/models/ban.rb
@@ -38,14 +38,6 @@ class Ban < ApplicationRecord
     q
   end
 
-  module ApiMethods
-    def html_data_attributes
-      super + [:expired?]
-    end
-  end
-
-  include ApiMethods
-
   def self.prune!
     expired.includes(:user).find_each do |ban|
       ban.user.unban! if ban.user.ban_expired?
diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb
index d82c7b15f..62815ff62 100644
--- a/app/models/forum_post.rb
+++ b/app/models/forum_post.rb
@@ -49,14 +49,7 @@ class ForumPost < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def html_data_attributes
-      super + [[:topic, :is_deleted?]]
-    end
-  end
-
   extend SearchMethods
-  include ApiMethods
 
   def self.new_reply(params)
     if params[:topic_id]
diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb
index 7daef04c4..f44a727d6 100644
--- a/app/models/forum_topic.rb
+++ b/app/models/forum_topic.rb
@@ -190,10 +190,6 @@ class ForumTopic < ApplicationRecord
     title.gsub(/\A\[APPROVED\]|\[REJECTED\]/, "")
   end
 
-  def html_data_attributes
-    super + [:is_read?]
-  end
-
   def self.searchable_includes
     [:creator, :updater, :forum_posts, :bulk_update_requests, :tag_aliases, :tag_implications]
   end
diff --git a/app/models/ip_address.rb b/app/models/ip_address.rb
index 00920ced7..322de5167 100644
--- a/app/models/ip_address.rb
+++ b/app/models/ip_address.rb
@@ -50,10 +50,6 @@ class IpAddress < ApplicationRecord
     true
   end
 
-  def html_data_attributes
-    super & attributes.keys.map(&:to_sym)
-  end
-
   def self.searchable_includes
     [:user, :model]
   end
diff --git a/app/models/post.rb b/app/models/post.rb
index b01b4979a..0c89dffbb 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -1504,10 +1504,6 @@ class Post < ApplicationRecord
     save
   end
 
-  def html_data_attributes
-    super + [:has_large?, :current_image_size]
-  end
-
   def self.model_restriction(table)
     super.where(table[:is_pending].eq(false)).where(table[:is_flagged].eq(false)).where(table[:is_deleted].eq(false))
   end
diff --git a/app/models/user_feedback.rb b/app/models/user_feedback.rb
index e7ffbcb45..eaf24fcf4 100644
--- a/app/models/user_feedback.rb
+++ b/app/models/user_feedback.rb
@@ -39,14 +39,7 @@ class UserFeedback < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def html_data_attributes
-      super + [:category]
-    end
-  end
-
   extend SearchMethods
-  include ApiMethods
 
   def user_name=(name)
     self.user = User.find_by_name(name)
diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index f2651be56..f3c9b5611 100644
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -105,14 +105,7 @@ class WikiPage < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def html_data_attributes
-      super + [:category_name]
-    end
-  end
-
   extend SearchMethods
-  include ApiMethods
 
   def validate_rename
     return unless title_changed?
diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb
index c4c123547..f36487709 100644
--- a/app/policies/application_policy.rb
+++ b/app/policies/application_policy.rb
@@ -70,7 +70,18 @@ class ApplicationPolicy
     permitted_attributes_for_update
   end
 
+  # The list of attributes that are permitted to be returned by the API.
   def api_attributes
     record.class.attribute_types.reject { |name, attr| attr.type.in?([:inet, :tsvector]) }.keys.map(&:to_sym)
   end
+
+  # The list of attributes that are permitted to be used as data-* attributes
+  # in tables and in the <body> tag on show pages.
+  def html_data_attributes
+    data_attributes = record.class.columns.select do |column|
+      column.type.in?([:integer, :boolean]) && !column.array?
+    end.map(&:name).map(&:to_sym)
+
+    api_attributes & data_attributes
+  end
 end
diff --git a/app/policies/ban_policy.rb b/app/policies/ban_policy.rb
index 3a4c5d52e..16606eb07 100644
--- a/app/policies/ban_policy.rb
+++ b/app/policies/ban_policy.rb
@@ -15,4 +15,8 @@ class BanPolicy < ApplicationPolicy
   def permitted_attributes_for_update
     [:reason, :duration, :expires_at]
   end
+
+  def html_data_attributes
+    super + [:expired?]
+  end
 end
diff --git a/app/policies/forum_post_policy.rb b/app/policies/forum_post_policy.rb
index 546991f09..30952650e 100644
--- a/app/policies/forum_post_policy.rb
+++ b/app/policies/forum_post_policy.rb
@@ -42,4 +42,8 @@ class ForumPostPolicy < ApplicationPolicy
   def permitted_attributes_for_update
     [:body]
   end
+
+  def html_data_attributes
+    super + [[:topic, :is_deleted?]]
+  end
 end
diff --git a/app/policies/forum_topic_policy.rb b/app/policies/forum_topic_policy.rb
index 0dd88b71a..de6e2edd3 100644
--- a/app/policies/forum_topic_policy.rb
+++ b/app/policies/forum_topic_policy.rb
@@ -37,4 +37,8 @@ class ForumTopicPolicy < ApplicationPolicy
       ([:is_sticky, :is_locked, :min_level] if moderate?)
     ].compact.flatten
   end
+
+  def html_data_attributes
+    super + [:is_read?]
+  end
 end
diff --git a/app/policies/ip_address_policy.rb b/app/policies/ip_address_policy.rb
index 4e30e78e2..fecd6dd23 100644
--- a/app/policies/ip_address_policy.rb
+++ b/app/policies/ip_address_policy.rb
@@ -2,4 +2,8 @@ class IpAddressPolicy < ApplicationPolicy
   def index?
     user.is_moderator?
   end
+
+  def html_data_attributes
+    super & attributes.keys.map(&:to_sym)
+  end
 end
diff --git a/app/policies/post_policy.rb b/app/policies/post_policy.rb
index 25046263f..316e7034b 100644
--- a/app/policies/post_policy.rb
+++ b/app/policies/post_policy.rb
@@ -95,4 +95,8 @@ class PostPolicy < ApplicationPolicy
     attributes -= [:fav_string] if !user.is_moderator?
     attributes
   end
+
+  def html_data_attributes
+    super + [:has_large?, :current_image_size]
+  end
 end
diff --git a/app/policies/user_feedback_policy.rb b/app/policies/user_feedback_policy.rb
index f7512e1c5..64c1d34f8 100644
--- a/app/policies/user_feedback_policy.rb
+++ b/app/policies/user_feedback_policy.rb
@@ -22,4 +22,8 @@ class UserFeedbackPolicy < ApplicationPolicy
   def permitted_attributes_for_update
     [:body, :category, :is_deleted]
   end
+
+  def html_data_attributes
+    super + [:category]
+  end
 end
diff --git a/app/policies/wiki_page_policy.rb b/app/policies/wiki_page_policy.rb
index c6e34850d..ca1bf5324 100644
--- a/app/policies/wiki_page_policy.rb
+++ b/app/policies/wiki_page_policy.rb
@@ -18,4 +18,8 @@ class WikiPagePolicy < ApplicationPolicy
   def api_attributes
     super + [:category_name]
   end
+
+  def html_data_attributes
+    super + [:category_name]
+  end
 end