From 9242bf522b46e8a042dc0c4472e5f926dc5dc0f8 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 17 Mar 2020 23:09:32 -0500 Subject: [PATCH] pundit: convert moderation reports to pundit. --- .../moderation_reports_controller.rb | 29 +++---------------- app/models/comment.rb | 4 --- app/models/dmail.rb | 4 --- app/models/forum_post.rb | 8 ----- app/models/user.rb | 4 --- app/policies/comment_policy.rb | 4 +++ app/policies/dmail_policy.rb | 4 +++ app/policies/forum_post_policy.rb | 4 +++ app/policies/moderation_report_policy.rb | 13 +++++++++ app/policies/user_policy.rb | 4 +++ .../comments/partials/show/_comment.html.erb | 4 +-- app/views/dmails/index.html.erb | 2 +- app/views/dmails/show.html.erb | 2 +- app/views/forum_posts/_forum_post.html.erb | 4 +-- app/views/users/_secondary_links.html.erb | 2 +- .../moderation_reports_controller_test.rb | 26 +++++++++++++---- 16 files changed, 60 insertions(+), 58 deletions(-) create mode 100644 app/policies/moderation_report_policy.rb diff --git a/app/controllers/moderation_reports_controller.rb b/app/controllers/moderation_reports_controller.rb index 21a23b9d3..a36f3e13c 100644 --- a/app/controllers/moderation_reports_controller.rb +++ b/app/controllers/moderation_reports_controller.rb @@ -1,49 +1,28 @@ class ModerationReportsController < ApplicationController respond_to :html, :xml, :json, :js - before_action :member_only, only: [:new, :create] - before_action :moderator_only, only: [:index] def new - @moderation_report = ModerationReport.new(moderation_report_params) - check_privilege(@moderation_report) + @moderation_report = authorize ModerationReport.new(permitted_attributes(ModerationReport)) respond_with(@moderation_report) end def index - @moderation_reports = ModerationReport.visible(CurrentUser.user).paginated_search(params, count_pages: true) + @moderation_reports = authorize ModerationReport.visible(CurrentUser.user).paginated_search(params, count_pages: true) @moderation_reports = @moderation_reports.includes(:creator, :model) if request.format.html? respond_with(@moderation_reports) end def show + authorize ModerationReport redirect_to moderation_reports_path(search: { id: params[:id] }) end def create - @moderation_report = ModerationReport.new(moderation_report_params.merge(creator: CurrentUser.user)) - check_privilege(@moderation_report) + @moderation_report = authorize ModerationReport.new(creator: CurrentUser.user, **permitted_attributes(ModerationReport)) @moderation_report.save flash.now[:notice] = @moderation_report.valid? ? "Report submitted" : @moderation_report.errors.full_messages.join("; ") respond_with(@moderation_report) end - - private - - def model_type - params.fetch(:moderation_report, {}).fetch(:model_type) - end - - def model_id - params.fetch(:moderation_report, {}).fetch(:model_id) - end - - def check_privilege(moderation_report) - raise User::PrivilegeError unless moderation_report.model.reportable_by?(CurrentUser.user) - end - - def moderation_report_params - params.fetch(:moderation_report, {}).permit(%i[model_type model_id reason]) - end end diff --git a/app/models/comment.rb b/app/models/comment.rb index ea4b36e9e..ca4231caa 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -117,10 +117,6 @@ class Comment < ApplicationRecord end end - def reportable_by?(user) - creator_id != user.id && !creator.is_moderator? - end - def voted_by?(user) return false if user.is_anonymous? user.id.in?(votes.map(&:user_id)) diff --git a/app/models/dmail.rb b/app/models/dmail.rb index 86417d78f..a155582ff 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -169,10 +169,6 @@ class Dmail < ApplicationRecord end end - def reportable_by?(user) - owner == user && is_recipient? && !is_automated? && !from.is_moderator? - end - def dtext_shortlink(key: false, **options) key ? "dmail ##{id}/#{self.key}" : "dmail ##{id}" end diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index 0bcb3397a..ee15d1904 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -81,10 +81,6 @@ class ForumPost < ApplicationRecord end end - def reportable_by?(user) - visible?(user) && creator_id != user.id && !creator.is_moderator? - end - def votable? bulk_update_request.present? && bulk_update_request.is_pending? end @@ -99,10 +95,6 @@ class ForumPost < ApplicationRecord end end - def visible?(user, show_deleted_posts = false) - user.is_moderator? || (user.level >= topic.min_level && (show_deleted_posts || !is_deleted?)) - end - def update_topic_updated_at_on_create if topic # need to do this to bypass the topic's original post from getting touched diff --git a/app/models/user.rb b/app/models/user.rb index 31203ddfd..ccc63785c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -697,10 +697,6 @@ class User < ApplicationRecord CurrentUser.as(self, &block) end - def reportable_by?(user) - ModerationReport.enabled? && user.is_builder? && id != user.id && !is_moderator? - end - def hide_favorites? !CurrentUser.is_admin? && enable_private_favorites? && CurrentUser.user.id != id end diff --git a/app/policies/comment_policy.rb b/app/policies/comment_policy.rb index cfa1f55ed..2d0583275 100644 --- a/app/policies/comment_policy.rb +++ b/app/policies/comment_policy.rb @@ -3,6 +3,10 @@ class CommentPolicy < ApplicationPolicy unbanned? && (user.is_moderator? || record.updater_id == user.id) end + def reportable? + unbanned? && record.creator_id != user.id && !record.creator.is_moderator? + end + def can_sticky_comment? user.is_moderator? end diff --git a/app/policies/dmail_policy.rb b/app/policies/dmail_policy.rb index effb614d1..5dc03a0b5 100644 --- a/app/policies/dmail_policy.rb +++ b/app/policies/dmail_policy.rb @@ -19,6 +19,10 @@ class DmailPolicy < ApplicationPolicy user.is_member? && (record.owner_id == user.id || record.valid_key?(request.params[:key])) end + def reportable? + unbanned? && record.owner_id == user.id && record.is_recipient? && !record.is_automated? && !record.from.is_moderator? + end + def permitted_attributes_for_create [:title, :body, :to_name, :to_id] end diff --git a/app/policies/forum_post_policy.rb b/app/policies/forum_post_policy.rb index 34787cef3..a444b16a0 100644 --- a/app/policies/forum_post_policy.rb +++ b/app/policies/forum_post_policy.rb @@ -19,6 +19,10 @@ class ForumPostPolicy < ApplicationPolicy unbanned? && show? && user.is_moderator? end + def reportable? + unbanned? && show? && record.creator_id != user.id && !record.creator.is_moderator? + end + def show_deleted? !record.is_deleted? || user.is_moderator? end diff --git a/app/policies/moderation_report_policy.rb b/app/policies/moderation_report_policy.rb new file mode 100644 index 000000000..12eef8914 --- /dev/null +++ b/app/policies/moderation_report_policy.rb @@ -0,0 +1,13 @@ +class ModerationReportPolicy < ApplicationPolicy + def index? + user.is_moderator? + end + + def create? + unbanned? && policy(record.model).reportable? + end + + def permitted_attributes + [:model_type, :model_id, :reason] + end +end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 1ed65844f..7f600efb2 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -15,6 +15,10 @@ class UserPolicy < ApplicationPolicy user.is_member? end + def reportable? + false + end + def fix_counts? user.is_member? end diff --git a/app/views/comments/partials/show/_comment.html.erb b/app/views/comments/partials/show/_comment.html.erb index fcaec4e76..6a4365ba6 100644 --- a/app/views/comments/partials/show/_comment.html.erb +++ b/app/views/comments/partials/show/_comment.html.erb @@ -12,7 +12,7 @@ data-is-deleted="<%= comment.is_deleted? %>" data-is-sticky="<%= comment.is_sticky? %>" data-below-threshold="<%= comment.score < CurrentUser.user.comment_threshold %>" - <% if CurrentUser.is_moderator? %> + <% if policy(moderation_reports).show? %> data-is-reported="<%= moderation_reports.pluck(:model_id).include?(comment.id) %>" <% end %> data-is-voted="<%= comment.voted_by?(CurrentUser.user) %>"> @@ -56,7 +56,7 @@
  • <%= link_to "Unvote", comment_comment_votes_path(comment_id: comment.id), method: :delete, remote: true %>
    • - <% if comment.reportable_by?(CurrentUser.user) %> + <% if policy(comment).reportable? %>
  • <%= link_to "Report", new_moderation_report_path(moderation_report: { model_type: "Comment", model_id: comment.id }), remote: true %>
    • <% end %> diff --git a/app/views/dmails/index.html.erb b/app/views/dmails/index.html.erb index 02c582b2f..24e268372 100644 --- a/app/views/dmails/index.html.erb +++ b/app/views/dmails/index.html.erb @@ -37,7 +37,7 @@ <%= link_to "Delete", dmail_path(dmail, format: :js), remote: true, method: :put, "data-params": "dmail[is_deleted]=true", "data-confirm": "Are you sure you want to delete this dmail?" %> <% end %> - <% if dmail.reportable_by?(CurrentUser.user) %> + <% if policy(dmail).reportable? %> | <%= link_to "Report", new_moderation_report_path(moderation_report: { model_type: "Dmail", model_id: dmail.id }), remote: true, title: "Report this dmail to the moderators" %> <% end %> <% end %> diff --git a/app/views/dmails/show.html.erb b/app/views/dmails/show.html.erb index 065b6f2ac..bca9226e1 100644 --- a/app/views/dmails/show.html.erb +++ b/app/views/dmails/show.html.erb @@ -29,7 +29,7 @@ <%= link_to "Respond", new_dmail_path(:respond_to_id => @dmail) %> | <%= link_to "Forward", new_dmail_path(:respond_to_id => @dmail, :forward => true) %> | <%= link_to "Share", dmail_path(@dmail, key: @dmail.key), title: "Anyone with this link will be able to view this dmail." %> - <% if @dmail.reportable_by?(CurrentUser.user) %> + <% if policy(@dmail).reportable? %> | <%= link_to "Report", new_moderation_report_path(moderation_report: { model_type: "Dmail", model_id: @dmail.id }), remote: true, title: "Report this dmail to the moderators" %> <% end %>

    diff --git a/app/views/forum_posts/_forum_post.html.erb b/app/views/forum_posts/_forum_post.html.erb index 617ddd952..36e802f49 100644 --- a/app/views/forum_posts/_forum_post.html.erb +++ b/app/views/forum_posts/_forum_post.html.erb @@ -1,7 +1,7 @@ <% if policy(forum_post).show_deleted? %>
    + <% if policy(moderation_reports).show? %> data-is-reported="<%= moderation_reports.pluck(:model_id).include?(forum_post.id) %>" <% end %> data-creator="<%= forum_post.creator.name %>"> @@ -37,7 +37,7 @@
  • <%= link_to "Edit", edit_forum_post_path(forum_post.id), :id => "edit_forum_post_link_#{forum_post.id}", :class => "edit_forum_post_link" %>
    • <% end %> <% end %> - <% if forum_post.reportable_by?(CurrentUser.user) %> + <% if policy(forum_post).reportable? %>
  • <%= link_to "Report", new_moderation_report_path(moderation_report: { model_type: "ForumPost", model_id: forum_post.id }), remote: true, title: "Report this forum post to the moderators" %>
    • <% end %> <% if forum_post.bulk_update_request.present? %> diff --git a/app/views/users/_secondary_links.html.erb b/app/views/users/_secondary_links.html.erb index 0ea703204..739f0132d 100644 --- a/app/views/users/_secondary_links.html.erb +++ b/app/views/users/_secondary_links.html.erb @@ -22,7 +22,7 @@ <% if !@user.is_platinum? %> <%= subnav_link_to "Gift upgrade", new_user_upgrade_path(:user_id => @user.id) %> <% end %> - <% if @user.reportable_by?(CurrentUser.user) %> + <% if policy(@user).reportable? %> <%= subnav_link_to "Report user", new_moderation_report_path(moderation_report: { model_type: "User", model_id: @user.id }), remote: true %> <% end %> <% end %> diff --git a/test/functional/moderation_reports_controller_test.rb b/test/functional/moderation_reports_controller_test.rb index c10a6bdff..568e6e81b 100644 --- a/test/functional/moderation_reports_controller_test.rb +++ b/test/functional/moderation_reports_controller_test.rb @@ -8,6 +8,7 @@ class ModerationReportsControllerTest < ActionDispatch::IntegrationTest @mod = create(:moderator_user, created_at: 2.weeks.ago) as(@spammer) do + @dmail = create(:dmail, from: @spammer, owner: @user, to: @user) @comment = create(:comment, creator: @spammer) @forum_topic = create(:forum_topic, creator: @spammer) @forum_post = create(:forum_post, topic: @forum_topic, creator: @spammer) @@ -15,10 +16,9 @@ class ModerationReportsControllerTest < ActionDispatch::IntegrationTest end context "new action" do - should "render the access denied page" do - get_auth new_moderation_report_path, User.anonymous + should "render the access denied page for anonymous users" do + get new_moderation_report_path assert_response 403 - assert_select "h1", /Access Denied/ end should "render" do @@ -32,13 +32,12 @@ class ModerationReportsControllerTest < ActionDispatch::IntegrationTest create(:moderation_report, model: @comment, creator: @user) end - should "render the access denied page" do + should "render the access denied page for members" do get_auth moderation_reports_path, @user assert_response 403 - assert_select "h1", /Access Denied/ end - should "render" do + should "render for mods" do get_auth moderation_reports_path, @mod assert_response :success end @@ -51,6 +50,14 @@ class ModerationReportsControllerTest < ActionDispatch::IntegrationTest end end + context "show action" do + should "redirect" do + @report = create(:moderation_report, model: @comment, creator: @user) + get_auth moderation_report_path(@report), @mod + assert_redirected_to moderation_reports_path(search: { id: @report.id }) + end + end + context "create action" do should "create a new moderation report on a comment" do assert_difference("ModerationReport.count", 1) do @@ -65,6 +72,13 @@ class ModerationReportsControllerTest < ActionDispatch::IntegrationTest assert_response :success end end + + should "create a new moderation report on a dmail" do + assert_difference("ModerationReport.count", 1) do + post_auth moderation_reports_path, @user, params: { format: "js", moderation_report: { model_id: @dmail.id, model_type: "Dmail", reason: "xxx" }} + assert_response :success + end + end end end end