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:
@@ -15,7 +15,7 @@ class DmailsController < ApplicationController
|
|||||||
end
|
end
|
||||||
|
|
||||||
def index
|
def index
|
||||||
@dmails = Dmail.visible.paginated_search(params, count_pages: true)
|
@dmails = Dmail.visible(CurrentUser.user).paginated_search(params, count_pages: true)
|
||||||
@dmails = @dmails.includes(:owner, :to, :from) if request.format.html?
|
@dmails = @dmails.includes(:owner, :to, :from) if request.format.html?
|
||||||
|
|
||||||
respond_with(@dmails)
|
respond_with(@dmails)
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ class FavoriteGroupsController < ApplicationController
|
|||||||
|
|
||||||
def index
|
def index
|
||||||
params[:search][:creator_id] ||= params[:user_id]
|
params[:search][:creator_id] ||= params[:user_id]
|
||||||
@favorite_groups = FavoriteGroup.paginated_search(params)
|
@favorite_groups = FavoriteGroup.visible(CurrentUser.user).paginated_search(params)
|
||||||
@favorite_groups = @favorite_groups.includes(:creator) if request.format.html?
|
@favorite_groups = @favorite_groups.includes(:creator) if request.format.html?
|
||||||
|
|
||||||
respond_with(@favorite_groups)
|
respond_with(@favorite_groups)
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ class ForumPostVotesController < ApplicationController
|
|||||||
before_action :member_only, only: [:create, :destroy]
|
before_action :member_only, only: [:create, :destroy]
|
||||||
|
|
||||||
def index
|
def index
|
||||||
@forum_post_votes = ForumPostVote.visible.paginated_search(params, count_pages: true)
|
@forum_post_votes = ForumPostVote.visible(CurrentUser.user).paginated_search(params, count_pages: true)
|
||||||
@forum_post_votes = @forum_post_votes.includes(:creator, forum_post: [:creator, :topic]) if request.format.html?
|
@forum_post_votes = @forum_post_votes.includes(:creator, forum_post: [:creator, :topic]) if request.format.html?
|
||||||
|
|
||||||
respond_with(@forum_post_votes)
|
respond_with(@forum_post_votes)
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ class ForumPostsController < ApplicationController
|
|||||||
end
|
end
|
||||||
|
|
||||||
def index
|
def index
|
||||||
@forum_posts = ForumPost.paginated_search(params)
|
@forum_posts = ForumPost.visible(CurrentUser.user).paginated_search(params)
|
||||||
@forum_posts = @forum_posts.includes(:topic, :creator) if request.format.html?
|
@forum_posts = @forum_posts.includes(:topic, :creator) if request.format.html?
|
||||||
|
|
||||||
respond_with(@forum_posts)
|
respond_with(@forum_posts)
|
||||||
|
|||||||
@@ -23,7 +23,7 @@ class ForumTopicsController < ApplicationController
|
|||||||
params[:search][:order] ||= "sticky" if request.format.html?
|
params[:search][:order] ||= "sticky" if request.format.html?
|
||||||
params[:limit] ||= 40
|
params[:limit] ||= 40
|
||||||
|
|
||||||
@forum_topics = ForumTopic.paginated_search(params)
|
@forum_topics = ForumTopic.visible(CurrentUser.user).paginated_search(params)
|
||||||
|
|
||||||
if request.format.atom?
|
if request.format.atom?
|
||||||
@forum_topics = @forum_topics.includes(:creator, :original_post)
|
@forum_topics = @forum_topics.includes(:creator, :original_post)
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ class ModActionsController < ApplicationController
|
|||||||
respond_to :html, :xml, :json
|
respond_to :html, :xml, :json
|
||||||
|
|
||||||
def index
|
def index
|
||||||
@mod_actions = ModAction.paginated_search(params)
|
@mod_actions = ModAction.visible(CurrentUser.user).paginated_search(params)
|
||||||
@mod_actions = @mod_actions.includes(:creator) if request.format.html?
|
@mod_actions = @mod_actions.includes(:creator) if request.format.html?
|
||||||
|
|
||||||
respond_with(@mod_actions)
|
respond_with(@mod_actions)
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ class ModerationReportsController < ApplicationController
|
|||||||
end
|
end
|
||||||
|
|
||||||
def index
|
def index
|
||||||
@moderation_reports = ModerationReport.paginated_search(params, count_pages: true)
|
@moderation_reports = ModerationReport.visible(CurrentUser.user).paginated_search(params, count_pages: true)
|
||||||
@moderation_reports = @moderation_reports.includes(:creator, :model) if request.format.html?
|
@moderation_reports = @moderation_reports.includes(:creator, :model) if request.format.html?
|
||||||
|
|
||||||
respond_with(@moderation_reports)
|
respond_with(@moderation_reports)
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ class PostVotesController < ApplicationController
|
|||||||
rescue_with PostVote::Error, status: 422
|
rescue_with PostVote::Error, status: 422
|
||||||
|
|
||||||
def index
|
def index
|
||||||
@post_votes = PostVote.paginated_search(params, count_pages: true)
|
@post_votes = PostVote.visible(CurrentUser.user).paginated_search(params, count_pages: true)
|
||||||
@post_votes = @post_votes.includes(:user, post: :uploader) if request.format.html?
|
@post_votes = @post_votes.includes(:user, post: :uploader) if request.format.html?
|
||||||
|
|
||||||
respond_with(@post_votes)
|
respond_with(@post_votes)
|
||||||
|
|||||||
@@ -19,7 +19,7 @@ class UserFeedbacksController < ApplicationController
|
|||||||
end
|
end
|
||||||
|
|
||||||
def index
|
def index
|
||||||
@user_feedbacks = UserFeedback.paginated_search(params, count_pages: true)
|
@user_feedbacks = UserFeedback.visible(CurrentUser.user).paginated_search(params, count_pages: true)
|
||||||
@user_feedbacks = @user_feedbacks.includes(:user, :creator) if request.format.html?
|
@user_feedbacks = @user_feedbacks.includes(:user, :creator) if request.format.html?
|
||||||
|
|
||||||
respond_with(@user_feedbacks)
|
respond_with(@user_feedbacks)
|
||||||
|
|||||||
@@ -20,7 +20,7 @@ class UserNameChangeRequestsController < ApplicationController
|
|||||||
end
|
end
|
||||||
|
|
||||||
def index
|
def index
|
||||||
@change_requests = UserNameChangeRequest.visible.order("id desc").paginate(params[:page], :limit => params[:limit])
|
@change_requests = UserNameChangeRequest.visible(CurrentUser.user).order("id desc").paginate(params[:page], :limit => params[:limit])
|
||||||
respond_with(@change_requests)
|
respond_with(@change_requests)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,10 @@
|
|||||||
class ApplicationRecord < ActiveRecord::Base
|
class ApplicationRecord < ActiveRecord::Base
|
||||||
self.abstract_class = true
|
self.abstract_class = true
|
||||||
|
|
||||||
|
include Mentionable
|
||||||
|
extend HasBitFlags
|
||||||
|
extend Searchable
|
||||||
|
|
||||||
concerning :PaginationMethods do
|
concerning :PaginationMethods do
|
||||||
class_methods do
|
class_methods do
|
||||||
def paginate(*args, **options)
|
def paginate(*args, **options)
|
||||||
@@ -16,9 +20,15 @@ class ApplicationRecord < ActiveRecord::Base
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
module ApiMethods
|
concerning :PrivilegeMethods do
|
||||||
extend ActiveSupport::Concern
|
class_methods do
|
||||||
|
def visible(user)
|
||||||
|
all
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
concerning :ApiMethods do
|
||||||
class_methods do
|
class_methods do
|
||||||
def api_attributes(*attributes, including: [])
|
def api_attributes(*attributes, including: [])
|
||||||
return @api_attributes if @api_attributes
|
return @api_attributes if @api_attributes
|
||||||
@@ -175,9 +185,4 @@ class ApplicationRecord < ActiveRecord::Base
|
|||||||
def warnings
|
def warnings
|
||||||
@warnings ||= ActiveModel::Errors.new(self)
|
@warnings ||= ActiveModel::Errors.new(self)
|
||||||
end
|
end
|
||||||
|
|
||||||
include ApiMethods
|
|
||||||
include Mentionable
|
|
||||||
extend HasBitFlags
|
|
||||||
extend Searchable
|
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -142,6 +142,7 @@ class Comment < ApplicationRecord
|
|||||||
select { |comment| comment.visibility(user) == :hidden }
|
select { |comment| comment.visibility(user) == :hidden }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# XXX rename
|
||||||
def self.visible(user)
|
def self.visible(user)
|
||||||
select { |comment| comment.visibility(user) == :visible }
|
select { |comment| comment.visibility(user) == :visible }
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ class CommentVote < ApplicationRecord
|
|||||||
validate :validate_comment_can_be_down_voted
|
validate :validate_comment_can_be_down_voted
|
||||||
validates_inclusion_of :score, :in => [-1, 1], :message => "must be 1 or -1"
|
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?
|
if user.is_admin?
|
||||||
all
|
all
|
||||||
elsif user.is_member?
|
elsif user.is_member?
|
||||||
|
|||||||
@@ -21,7 +21,6 @@ class Dmail < ApplicationRecord
|
|||||||
scope :deleted, -> { where(is_deleted: true) }
|
scope :deleted, -> { where(is_deleted: true) }
|
||||||
scope :read, -> { where(is_read: true) }
|
scope :read, -> { where(is_read: true) }
|
||||||
scope :unread, -> { where(is_read: false) }
|
scope :unread, -> { where(is_read: false) }
|
||||||
scope :visible, -> { where(owner: CurrentUser.user) }
|
|
||||||
scope :sent, -> { where("dmails.owner_id = dmails.from_id") }
|
scope :sent, -> { where("dmails.owner_id = dmails.from_id") }
|
||||||
scope :received, -> { where("dmails.owner_id = dmails.to_id") }
|
scope :received, -> { where("dmails.owner_id = dmails.to_id") }
|
||||||
|
|
||||||
@@ -85,6 +84,10 @@ class Dmail < ApplicationRecord
|
|||||||
end
|
end
|
||||||
|
|
||||||
module SearchMethods
|
module SearchMethods
|
||||||
|
def visible(user)
|
||||||
|
where(owner: user)
|
||||||
|
end
|
||||||
|
|
||||||
def sent_by(user)
|
def sent_by(user)
|
||||||
where("dmails.from_id = ? AND dmails.owner_id != ?", user.id, user.id)
|
where("dmails.from_id = ? AND dmails.owner_id != ?", user.id, user.id)
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -27,7 +27,6 @@ class FavoriteGroup < ApplicationRecord
|
|||||||
|
|
||||||
def search(params)
|
def search(params)
|
||||||
q = super
|
q = super
|
||||||
q = q.visible(CurrentUser.user)
|
|
||||||
q = q.search_attributes(params, :name, :is_public, :post_ids, :creator)
|
q = q.search_attributes(params, :name, :is_public, :post_ids, :creator)
|
||||||
|
|
||||||
if params[:name_matches].present?
|
if params[:name_matches].present?
|
||||||
|
|||||||
@@ -40,13 +40,12 @@ class ForumPost < ApplicationRecord
|
|||||||
where(topic_id: ForumTopic.search(title_matches: title).select(:id))
|
where(topic_id: ForumTopic.search(title_matches: title).select(:id))
|
||||||
end
|
end
|
||||||
|
|
||||||
def permitted
|
def visible(user)
|
||||||
where(topic_id: ForumTopic.permitted)
|
where(topic_id: ForumTopic.visible(user))
|
||||||
end
|
end
|
||||||
|
|
||||||
def search(params)
|
def search(params)
|
||||||
q = super
|
q = super
|
||||||
q = q.permitted
|
|
||||||
q = q.search_attributes(params, :creator, :updater, :topic_id, :is_deleted, :body)
|
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)
|
q = q.text_attribute_matches(:body, params[:body_matches], index_column: :text_index)
|
||||||
|
|
||||||
|
|||||||
@@ -8,7 +8,10 @@ class ForumPostVote < ApplicationRecord
|
|||||||
scope :down, -> {where(score: -1)}
|
scope :down, -> {where(score: -1)}
|
||||||
scope :by, ->(user_id) {where(creator_id: user_id)}
|
scope :by, ->(user_id) {where(creator_id: user_id)}
|
||||||
scope :excluding_user, ->(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)
|
def self.forum_post_matches(params)
|
||||||
return all if params.blank?
|
return all if params.blank?
|
||||||
|
|||||||
@@ -52,8 +52,8 @@ class ForumTopic < ApplicationRecord
|
|||||||
end
|
end
|
||||||
|
|
||||||
module SearchMethods
|
module SearchMethods
|
||||||
def permitted
|
def visible(user)
|
||||||
where("min_level <= ?", CurrentUser.level)
|
where("min_level <= ?", user.level)
|
||||||
end
|
end
|
||||||
|
|
||||||
def read_by_user(user)
|
def read_by_user(user)
|
||||||
@@ -79,7 +79,6 @@ class ForumTopic < ApplicationRecord
|
|||||||
|
|
||||||
def search(params)
|
def search(params)
|
||||||
q = super
|
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.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)
|
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)
|
ForumTopicVisit.create(:user_id => user.id, :forum_topic_id => id, :last_read_at => updated_at)
|
||||||
end
|
end
|
||||||
|
|
||||||
unread_topics = ForumTopic.permitted.active.unread_by_user(user)
|
unread_topics = ForumTopic.visible(user).active.unread_by_user(user)
|
||||||
|
|
||||||
if !unread_topics.exists?
|
if !unread_topics.exists?
|
||||||
user.update!(last_forum_read_at: Time.zone.now)
|
user.update!(last_forum_read_at: Time.zone.now)
|
||||||
|
|||||||
@@ -54,7 +54,7 @@ class ModAction < ApplicationRecord
|
|||||||
other: 2000
|
other: 2000
|
||||||
}
|
}
|
||||||
|
|
||||||
def self.permitted(user)
|
def self.visible(user)
|
||||||
if user.is_moderator?
|
if user.is_moderator?
|
||||||
all
|
all
|
||||||
else
|
else
|
||||||
@@ -65,7 +65,6 @@ class ModAction < ApplicationRecord
|
|||||||
def self.search(params)
|
def self.search(params)
|
||||||
q = super
|
q = super
|
||||||
|
|
||||||
q = q.permitted(CurrentUser.user)
|
|
||||||
q = q.search_attributes(params, :creator, :category, :description)
|
q = q.search_attributes(params, :creator, :category, :description)
|
||||||
q = q.text_attribute_matches(:description, params[:description_matches])
|
q = q.text_attribute_matches(:description, params[:description_matches])
|
||||||
|
|
||||||
|
|||||||
@@ -26,14 +26,12 @@ class PostVote < ApplicationRecord
|
|||||||
positive.where(user_id: user_id).pluck(:post_id)
|
positive.where(user_id: user_id).pluck(:post_id)
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.visible(user = CurrentUser.user)
|
def self.visible(user)
|
||||||
return all if user.is_admin?
|
user.is_admin? ? all : where(user: user)
|
||||||
where(user: user)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.search(params)
|
def self.search(params)
|
||||||
q = super
|
q = super
|
||||||
q = q.visible
|
|
||||||
q = q.search_attributes(params, :post, :user, :score)
|
q = q.search_attributes(params, :post, :user, :score)
|
||||||
q.apply_default_order(params)
|
q.apply_default_order(params)
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -117,7 +117,7 @@ class User < ApplicationRecord
|
|||||||
has_many :dmails, -> {order("dmails.id desc")}, :foreign_key => "owner_id"
|
has_many :dmails, -> {order("dmails.id desc")}, :foreign_key => "owner_id"
|
||||||
has_many :saved_searches
|
has_many :saved_searches
|
||||||
has_many :forum_posts, -> {order("forum_posts.created_at, forum_posts.id")}, :foreign_key => "creator_id"
|
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 :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 :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
|
has_many :ip_bans, foreign_key: :creator_id
|
||||||
@@ -403,7 +403,7 @@ class User < ApplicationRecord
|
|||||||
module ForumMethods
|
module ForumMethods
|
||||||
def has_forum_been_updated?
|
def has_forum_been_updated?
|
||||||
return false unless is_gold?
|
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 false if max_updated_at.nil?
|
||||||
return true if last_forum_read_at.nil?
|
return true if last_forum_read_at.nil?
|
||||||
return max_updated_at > last_forum_read_at
|
return max_updated_at > last_forum_read_at
|
||||||
|
|||||||
@@ -21,7 +21,7 @@ class UserFeedback < ApplicationRecord
|
|||||||
scope :undeleted, -> { where(is_deleted: false) }
|
scope :undeleted, -> { where(is_deleted: false) }
|
||||||
|
|
||||||
module SearchMethods
|
module SearchMethods
|
||||||
def visible(viewer = CurrentUser.user)
|
def visible(viewer)
|
||||||
viewer.is_moderator? ? all : undeleted
|
viewer.is_moderator? ? all : undeleted
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -32,7 +32,6 @@ class UserFeedback < ApplicationRecord
|
|||||||
def search(params)
|
def search(params)
|
||||||
q = super
|
q = super
|
||||||
|
|
||||||
q = q.visible
|
|
||||||
q = q.search_attributes(params, :user, :creator, :category, :body, :is_deleted)
|
q = q.search_attributes(params, :user, :creator, :category, :body, :is_deleted)
|
||||||
q = q.text_attribute_matches(:body, params[:body_matches])
|
q = q.text_attribute_matches(:body, params[:body_matches])
|
||||||
|
|
||||||
|
|||||||
@@ -8,10 +8,10 @@ class UserNameChangeRequest < ApplicationRecord
|
|||||||
|
|
||||||
after_create :update_name!
|
after_create :update_name!
|
||||||
|
|
||||||
def self.visible(viewer = CurrentUser.user)
|
def self.visible(user)
|
||||||
if viewer.is_admin?
|
if user.is_admin?
|
||||||
all
|
all
|
||||||
elsif viewer.is_member?
|
elsif user.is_member?
|
||||||
where(user: User.undeleted)
|
where(user: User.undeleted)
|
||||||
else
|
else
|
||||||
none
|
none
|
||||||
|
|||||||
@@ -152,6 +152,8 @@ class UserPresenter
|
|||||||
end
|
end
|
||||||
|
|
||||||
def previous_names(template)
|
def previous_names(template)
|
||||||
user.user_name_change_requests.map { |req| template.link_to req.original_name, req }.join(", ").html_safe
|
user.user_name_change_requests.visible(CurrentUser.user).map do |req|
|
||||||
|
template.link_to req.original_name, req }.join(", ").html_safe
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user