models: refactor search visibility methods.

Refactor how model visibility works in index actions:

* Call `visible` in the controller instead of in model `search`
  methods. This decouples model visibility from model searching.

* Explicitly pass CurrentUser when calling `visible`. This reduces
  hidden dependencies on the current user inside models.

* Standardize on calling the method `visible`. In some places it was
  called `permitted` instead.

* Add a `visible` base method to ApplicationModel.
This commit is contained in:
evazion
2020-02-19 16:28:10 -06:00
parent bd6d896ee0
commit 0ad42d23c9
24 changed files with 49 additions and 42 deletions

View File

@@ -1,6 +1,10 @@
class ApplicationRecord < ActiveRecord::Base
self.abstract_class = true
include Mentionable
extend HasBitFlags
extend Searchable
concerning :PaginationMethods do
class_methods do
def paginate(*args, **options)
@@ -16,9 +20,15 @@ class ApplicationRecord < ActiveRecord::Base
end
end
module ApiMethods
extend ActiveSupport::Concern
concerning :PrivilegeMethods do
class_methods do
def visible(user)
all
end
end
end
concerning :ApiMethods do
class_methods do
def api_attributes(*attributes, including: [])
return @api_attributes if @api_attributes
@@ -175,9 +185,4 @@ class ApplicationRecord < ActiveRecord::Base
def warnings
@warnings ||= ActiveModel::Errors.new(self)
end
include ApiMethods
include Mentionable
extend HasBitFlags
extend Searchable
end

View File

@@ -142,6 +142,7 @@ class Comment < ApplicationRecord
select { |comment| comment.visibility(user) == :hidden }
end
# XXX rename
def self.visible(user)
select { |comment| comment.visibility(user) == :visible }
end

View File

@@ -10,7 +10,7 @@ class CommentVote < ApplicationRecord
validate :validate_comment_can_be_down_voted
validates_inclusion_of :score, :in => [-1, 1], :message => "must be 1 or -1"
def self.visible(user = CurrentUser.user)
def self.visible(user)
if user.is_admin?
all
elsif user.is_member?

View File

@@ -21,7 +21,6 @@ class Dmail < ApplicationRecord
scope :deleted, -> { where(is_deleted: true) }
scope :read, -> { where(is_read: true) }
scope :unread, -> { where(is_read: false) }
scope :visible, -> { where(owner: CurrentUser.user) }
scope :sent, -> { where("dmails.owner_id = dmails.from_id") }
scope :received, -> { where("dmails.owner_id = dmails.to_id") }
@@ -85,6 +84,10 @@ class Dmail < ApplicationRecord
end
module SearchMethods
def visible(user)
where(owner: user)
end
def sent_by(user)
where("dmails.from_id = ? AND dmails.owner_id != ?", user.id, user.id)
end

View File

@@ -27,7 +27,6 @@ class FavoriteGroup < ApplicationRecord
def search(params)
q = super
q = q.visible(CurrentUser.user)
q = q.search_attributes(params, :name, :is_public, :post_ids, :creator)
if params[:name_matches].present?

View File

@@ -40,13 +40,12 @@ class ForumPost < ApplicationRecord
where(topic_id: ForumTopic.search(title_matches: title).select(:id))
end
def permitted
where(topic_id: ForumTopic.permitted)
def visible(user)
where(topic_id: ForumTopic.visible(user))
end
def search(params)
q = super
q = q.permitted
q = q.search_attributes(params, :creator, :updater, :topic_id, :is_deleted, :body)
q = q.text_attribute_matches(:body, params[:body_matches], index_column: :text_index)

View File

@@ -8,7 +8,10 @@ class ForumPostVote < ApplicationRecord
scope :down, -> {where(score: -1)}
scope :by, ->(user_id) {where(creator_id: user_id)}
scope :excluding_user, ->(user_id) {where("creator_id <> ?", user_id)}
scope :visible, -> { where(forum_post: ForumPost.permitted) }
def self.visible(user)
where(forum_post: ForumPost.visible(user))
end
def self.forum_post_matches(params)
return all if params.blank?

View File

@@ -52,8 +52,8 @@ class ForumTopic < ApplicationRecord
end
module SearchMethods
def permitted
where("min_level <= ?", CurrentUser.level)
def visible(user)
where("min_level <= ?", user.level)
end
def read_by_user(user)
@@ -79,7 +79,6 @@ class ForumTopic < ApplicationRecord
def search(params)
q = super
q = q.permitted
q = q.search_attributes(params, :creator, :updater, :is_sticky, :is_locked, :is_deleted, :category_id, :title, :response_count)
q = q.text_attribute_matches(:title, params[:title_matches], index_column: :text_index)
@@ -113,7 +112,7 @@ class ForumTopic < ApplicationRecord
ForumTopicVisit.create(:user_id => user.id, :forum_topic_id => id, :last_read_at => updated_at)
end
unread_topics = ForumTopic.permitted.active.unread_by_user(user)
unread_topics = ForumTopic.visible(user).active.unread_by_user(user)
if !unread_topics.exists?
user.update!(last_forum_read_at: Time.zone.now)

View File

@@ -54,7 +54,7 @@ class ModAction < ApplicationRecord
other: 2000
}
def self.permitted(user)
def self.visible(user)
if user.is_moderator?
all
else
@@ -65,7 +65,6 @@ class ModAction < ApplicationRecord
def self.search(params)
q = super
q = q.permitted(CurrentUser.user)
q = q.search_attributes(params, :creator, :category, :description)
q = q.text_attribute_matches(:description, params[:description_matches])

View File

@@ -26,14 +26,12 @@ class PostVote < ApplicationRecord
positive.where(user_id: user_id).pluck(:post_id)
end
def self.visible(user = CurrentUser.user)
return all if user.is_admin?
where(user: user)
def self.visible(user)
user.is_admin? ? all : where(user: user)
end
def self.search(params)
q = super
q = q.visible
q = q.search_attributes(params, :post, :user, :score)
q.apply_default_order(params)
end

View File

@@ -117,7 +117,7 @@ class User < ApplicationRecord
has_many :dmails, -> {order("dmails.id desc")}, :foreign_key => "owner_id"
has_many :saved_searches
has_many :forum_posts, -> {order("forum_posts.created_at, forum_posts.id")}, :foreign_key => "creator_id"
has_many :user_name_change_requests, -> {visible.order("user_name_change_requests.created_at desc")}
has_many :user_name_change_requests, -> {order("user_name_change_requests.created_at desc")}
has_many :favorite_groups, -> {order(name: :asc)}, foreign_key: :creator_id
has_many :favorites, ->(rec) {where("user_id % 100 = #{rec.id % 100} and user_id = #{rec.id}").order("id desc")}
has_many :ip_bans, foreign_key: :creator_id
@@ -403,7 +403,7 @@ class User < ApplicationRecord
module ForumMethods
def has_forum_been_updated?
return false unless is_gold?
max_updated_at = ForumTopic.permitted.active.maximum(:updated_at)
max_updated_at = ForumTopic.visible(self).active.maximum(:updated_at)
return false if max_updated_at.nil?
return true if last_forum_read_at.nil?
return max_updated_at > last_forum_read_at

View File

@@ -21,7 +21,7 @@ class UserFeedback < ApplicationRecord
scope :undeleted, -> { where(is_deleted: false) }
module SearchMethods
def visible(viewer = CurrentUser.user)
def visible(viewer)
viewer.is_moderator? ? all : undeleted
end
@@ -32,7 +32,6 @@ class UserFeedback < ApplicationRecord
def search(params)
q = super
q = q.visible
q = q.search_attributes(params, :user, :creator, :category, :body, :is_deleted)
q = q.text_attribute_matches(:body, params[:body_matches])

View File

@@ -8,10 +8,10 @@ class UserNameChangeRequest < ApplicationRecord
after_create :update_name!
def self.visible(viewer = CurrentUser.user)
if viewer.is_admin?
def self.visible(user)
if user.is_admin?
all
elsif viewer.is_member?
elsif user.is_member?
where(user: User.undeleted)
else
none