From 7d5650c3f1006bf4c79a367e4c3dcbe890bd3c3d Mon Sep 17 00:00:00 2001 From: Anna Buianova Date: Fri, 22 Mar 2024 15:40:36 +0300 Subject: [PATCH] Admin access to spam comments by url (#20775) * Admin access to spam comments by url * Fixed spec * Fixed view spec --- app/controllers/comments_controller.rb | 19 +++++++++++++++++-- app/helpers/comments_helper.rb | 11 ++++++++--- app/views/comments/_comment.html.erb | 1 + app/views/comments/_comment_proper.html.erb | 2 +- .../comments/_comment_quality_marker.html.erb | 3 +++ app/views/comments/index.html.erb | 5 +++-- config/locales/views/comments/en.yml | 1 + config/locales/views/comments/fr.yml | 1 + spec/requests/comments_spec.rb | 18 ++++++++++++++++++ spec/requests/comments_with_cache_spec.rb | 18 +++++++++++------- spec/views/comments/_comment.html.erb_spec.rb | 1 + 11 files changed, 65 insertions(+), 15 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 545fc383d..ae3d64a0f 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -9,6 +9,8 @@ class CommentsController < ApplicationController # GET /comments # GET /comments.json + # rubocop:disable Metrics/CyclomaticComplexity + # rubocop:disable Metrics/PerceivedComplexity def index skip_authorization @comment = Comment.new @@ -19,8 +21,9 @@ class CommentsController < ApplicationController if @root_comment # 404 for all low-quality for not signed in not_found if @root_comment.score < Comment::LOW_QUALITY_THRESHOLD && !user_signed_in? - # 404 only for < -400 w/o children for signed in - not_found if @root_comment.score < Comment::HIDE_THRESHOLD && !@root_comment.has_children? + set_admin_access + # 404 only for < -400 w/o children for all except admins + not_found if !@is_admin && @root_comment.score < Comment::HIDE_THRESHOLD && !@root_comment.has_children? end if @podcast @@ -37,6 +40,9 @@ class CommentsController < ApplicationController set_surrogate_key_header "comments-for-#{@commentable.id}-#{@commentable_type}" if @commentable end + # rubocop:enable Metrics/CyclomaticComplexity + # rubocop:enable Metrics/PerceivedComplexity + # GET /comments/1 # GET /comments/1.json # GET /comments/1/edit @@ -269,6 +275,15 @@ class CommentsController < ApplicationController private + # for spam content we need to remove cache control headers to access current_user to check admin access + # so that admins could have access to spam articles, profiles and comments (by direct url) + def set_admin_access + return unless user_signed_in? + + unset_cache_control_headers + @is_admin = current_user&.any_admin? + end + def comment_should_be_visible? if @article @article.published? diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb index 6fee321ea..f81b48b7d 100644 --- a/app/helpers/comments_helper.rb +++ b/app/helpers/comments_helper.rb @@ -93,13 +93,18 @@ module CommentsHelper private - def nested_comments(tree:, commentable:, is_view_root: false) + def nested_comments(tree:, commentable:, is_view_root: false, is_admin: false) comments = tree.filter_map do |comment, sub_comments| is_childless = sub_comments.empty? - unless comment.decorate.super_low_quality && is_childless + # hide childless comments with score below hide threshold (but show for admins) + hide = comment.decorate.super_low_quality && is_childless + if is_admin || !hide render("comments/comment", comment: comment, commentable: commentable, is_view_root: is_view_root, is_childless: is_childless, - subtree_html: nested_comments(tree: sub_comments, commentable: commentable)) + is_admin: is_admin, + subtree_html: nested_comments(tree: sub_comments, + commentable: commentable, + is_admin: is_admin)) end end safe_join(comments) diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 1d4261c35..924ceceb4 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -30,6 +30,7 @@ commentable: commentable, is_view_root: is_view_root, is_childless: is_childless, + is_admin: is_admin, subtree_html: subtree_html) %> <% if comment.depth < 3 %> diff --git a/app/views/comments/_comment_proper.html.erb b/app/views/comments/_comment_proper.html.erb index d7e6a544c..c475b5303 100644 --- a/app/views/comments/_comment_proper.html.erb +++ b/app/views/comments/_comment_proper.html.erb @@ -5,7 +5,7 @@
- <% if comment.deleted || decorated_comment.super_low_quality %> + <% if comment.deleted || (decorated_comment.super_low_quality && !is_admin) %>
<%= t("views.comments.delete.comment_deleted") %> diff --git a/app/views/comments/_comment_quality_marker.html.erb b/app/views/comments/_comment_quality_marker.html.erb index 99f3e0183..d07c99ee8 100644 --- a/app/views/comments/_comment_quality_marker.html.erb +++ b/app/views/comments/_comment_quality_marker.html.erb @@ -1,6 +1,9 @@ <% if decorated_comment.low_quality %>
<%= t("views.comments.quality.low") %> <%= t("views.comments.quality.conduct") %> + <% if decorated_comment.super_low_quality %> +

(<%= t("views.comments.quality.superlow") %>)

+ <% end %>
<% end %> diff --git a/app/views/comments/index.html.erb b/app/views/comments/index.html.erb index 4daff88a6..2625eed83 100644 --- a/app/views/comments/index.html.erb +++ b/app/views/comments/index.html.erb @@ -138,8 +138,9 @@
<% if @root_comment.present? %> - <% cache ["comment_root-view-root_#{user_signed_in?}", @root_comment] do %> - <%= nested_comments(tree: Comments::Tree.for_root_comment(@root_comment, include_negative: user_signed_in?), commentable: @commentable, is_view_root: true) %> + <% cache ["comment_root-view-root_#{user_signed_in?}-#{@is_admin}", @root_comment] do %> + <%= nested_comments(tree: Comments::Tree.for_root_comment(@root_comment, include_negative: user_signed_in?), + commentable: @commentable, is_view_root: true, is_admin: @is_admin) %> <% end %> <% else %> <% Comments::Tree.for_commentable(@commentable, include_negative: user_signed_in?).each do |comment, sub_comments| %> diff --git a/config/locales/views/comments/en.yml b/config/locales/views/comments/en.yml index 9308fcee4..116449abb 100644 --- a/config/locales/views/comments/en.yml +++ b/config/locales/views/comments/en.yml @@ -73,6 +73,7 @@ en: unmute: Unmute notifications quality: low: Comment marked as low quality/non-constructive by the community. + superlow: Comment score below hiding threshold, displayed only for admins conduct: View Code of Conduct hidden: icon: Info diff --git a/config/locales/views/comments/fr.yml b/config/locales/views/comments/fr.yml index b81c259e3..f65070882 100644 --- a/config/locales/views/comments/fr.yml +++ b/config/locales/views/comments/fr.yml @@ -73,6 +73,7 @@ fr: unmute: Unmute notifications quality: low: Comment marked as low quality/non-constructive by the community. + superlow: Comment score below hiding threshold, displayed only for admins conduct: View Code of Conduct hidden: icon: Info diff --git a/spec/requests/comments_spec.rb b/spec/requests/comments_spec.rb index 33bc59e36..86dc2ad6f 100644 --- a/spec/requests/comments_spec.rb +++ b/spec/requests/comments_spec.rb @@ -3,6 +3,7 @@ require "requests/shared_examples/comment_hide_or_unhide_request" RSpec.describe "Comments" do let(:user) { create(:user) } + let(:admin) { create(:user, :admin) } let(:article) { create(:article, user: user) } let(:podcast) { create(:podcast) } let(:podcast_episode) { create(:podcast_episode, podcast_id: podcast.id) } @@ -231,6 +232,13 @@ RSpec.describe "Comments" do end.to raise_error(ActiveRecord::RecordNotFound) end + it "renders success when no children + admin signed in", :aggregate_failures do + sign_in admin + get low_comment.path + expect(response).to be_successful + expect(response.body).to include("low-comment") + end + it "raises 404 when has children and not signed in" do create(:comment, commentable: article, user: user, parent: low_comment, body_markdown: "child of a low-quality comment") @@ -256,6 +264,16 @@ RSpec.describe "Comments" do expect(response.body).to include("child of a low-quality comment") end + it "displays text when there are children + admin signed in", :aggregate_failures do + create(:comment, commentable: article, user: user, parent: low_comment, + body_markdown: "child of a low-quality comment") + sign_in admin + get low_comment.path + expect(response).to be_successful + expect(response.body).to include("low-comment") + expect(response.body).to include("child of a low-quality comment") + end + it "hides negative children for signed out" do create(:comment, commentable: article, user: user, score: -10, parent: comment, body_markdown: "low-child of a comment") diff --git a/spec/requests/comments_with_cache_spec.rb b/spec/requests/comments_with_cache_spec.rb index 40fcab504..e070b3b62 100644 --- a/spec/requests/comments_with_cache_spec.rb +++ b/spec/requests/comments_with_cache_spec.rb @@ -5,24 +5,23 @@ require "rails_helper" RSpec.describe "ArticleCommentsWithCache" do - let(:user) { create(:user, :admin) } - let(:article) { create(:article, user: user, published: true) } + let(:admin) { create(:user, :admin) } + let(:article) { create(:article, user: admin, published: true) } let(:user2) { create(:user) } - let(:parent) { create(:comment, commentable: article, user: user, body_markdown: "parent-comment") } + let(:parent) { create(:comment, commentable: article, user: admin, body_markdown: "parent-comment") } let(:cache_store) { ActiveSupport::Cache.lookup_store(:redis_cache_store) } let(:cache) { Rails.cache } before do - sign_in user allow(Rails).to receive(:cache).and_return(cache_store) end def assign_spam_role sidekiq_perform_enqueued_jobs do Moderator::ManageActivityAndRoles.handle_user_roles( - admin: user, + admin: admin, user: user2, user_params: { note_for_current_role: "note", @@ -34,6 +33,7 @@ RSpec.describe "ArticleCommentsWithCache" do describe "GET /:slug (articles)" do it "busts comments cache" do + sign_in admin create(:comment, commentable: article, user: user2, body_markdown: "potential-spam-comment") get article.path expect(response.body).to include("potential-spam-comment") @@ -43,9 +43,11 @@ RSpec.describe "ArticleCommentsWithCache" do end it "busts cache when spam comment is a child and a parent" do + sign_in admin + comment = create(:comment, commentable: article, user: user2, parent: parent, body_markdown: "potential-spam-comment") - create(:comment, commentable: article, user: user, body_markdown: "child-comment", parent: comment) + create(:comment, commentable: article, user: admin, body_markdown: "child-comment", parent: comment) get article.path expect(response.body).to include("potential-spam-comment") @@ -61,9 +63,11 @@ RSpec.describe "ArticleCommentsWithCache" do describe "GET /:username/comment/:id_code (root comment path)" do it "busts cache for root comment" do + sign_in user2 + comment = create(:comment, commentable: article, user: user2, parent: parent, body_markdown: "potential-spam-comment") - create(:comment, commentable: article, user: user, body_markdown: "child-comment", parent: comment) + create(:comment, commentable: article, user: admin, body_markdown: "child-comment", parent: comment) get parent.path expect(response.body).to include("potential-spam-comment") diff --git a/spec/views/comments/_comment.html.erb_spec.rb b/spec/views/comments/_comment.html.erb_spec.rb index 818a6d787..958ac5ac6 100644 --- a/spec/views/comments/_comment.html.erb_spec.rb +++ b/spec/views/comments/_comment.html.erb_spec.rb @@ -12,6 +12,7 @@ RSpec.describe "rendering locals in a partial" do commentable: article, is_view_root: true, is_childless: true, + is_admin: false, subtree_html: "" expect(rendered).to match(/crayons-notice crayons-notice--warning low-quality-comment-marker/)