diff --git a/Changelog.md b/Changelog.md index dde393566..90acddad7 100644 --- a/Changelog.md +++ b/Changelog.md @@ -78,6 +78,7 @@ With the port to Bootstrap 3, app/views/terms/default.haml has a new structure. * Backend information panel & health checks for known pods [#6290](https://github.com/diaspora/diaspora/pull/6290) * Allow users to view a posts locations on an OpenStreetMap [#6256](https://github.com/diaspora/diaspora/pull/6256) * Redesign and unify error pages [#6428](https://github.com/diaspora/diaspora/pull/6428) +* Redesign and refactor report admin interface [#6378](https://github.com/diaspora/diaspora/pull/6378) # 0.5.4.0 diff --git a/app/assets/stylesheets/report.scss b/app/assets/stylesheets/report.scss index d964b55cd..4282ad637 100644 --- a/app/assets/stylesheets/report.scss +++ b/app/assets/stylesheets/report.scss @@ -1,21 +1,8 @@ #reports { - padding-top: 2em; - .content { - float: left; - span { - display: block; - } - span.text { - padding-bottom: 1em; - } + .reason { + padding-bottom: 20px; } - .options { - float: right; - } - .clear { - clear: both; - border-bottom: 1px solid #808080; - padding-bottom: 1em; - margin-bottom: 1em; + form input { + margin-right: 5px; } } diff --git a/app/assets/templates/comment_tpl.jst.hbs b/app/assets/templates/comment_tpl.jst.hbs index 5dc240b49..2f8a9ae14 100644 --- a/app/assets/templates/comment_tpl.jst.hbs +++ b/app/assets/templates/comment_tpl.jst.hbs @@ -13,7 +13,7 @@ {{else}} - + {{/if}} diff --git a/app/assets/templates/photo_tpl.jst.hbs b/app/assets/templates/photo_tpl.jst.hbs index a36e05879..9cf51d51b 100644 --- a/app/assets/templates/photo_tpl.jst.hbs +++ b/app/assets/templates/photo_tpl.jst.hbs @@ -7,7 +7,7 @@ {{else}} - + diff --git a/app/assets/templates/single-post-viewer/single-post-moderation_tpl.jst.hbs b/app/assets/templates/single-post-viewer/single-post-moderation_tpl.jst.hbs index 10fc6f0d2..d81f6a14b 100644 --- a/app/assets/templates/single-post-viewer/single-post-moderation_tpl.jst.hbs +++ b/app/assets/templates/single-post-viewer/single-post-moderation_tpl.jst.hbs @@ -5,7 +5,7 @@ {{else}} - + diff --git a/app/assets/templates/stream-element_tpl.jst.hbs b/app/assets/templates/stream-element_tpl.jst.hbs index 92ea4e5ee..bb13c0cfa 100644 --- a/app/assets/templates/stream-element_tpl.jst.hbs +++ b/app/assets/templates/stream-element_tpl.jst.hbs @@ -13,7 +13,7 @@ {{else}} - + diff --git a/app/helpers/report_helper.rb b/app/helpers/report_helper.rb index 2170e3665..9eb017843 100644 --- a/app/helpers/report_helper.rb +++ b/app/helpers/report_helper.rb @@ -3,15 +3,17 @@ # the COPYRIGHT file. module ReportHelper - def report_content(id, type) - if type == 'post' && !(post = Post.find_by_id(id)).nil? - raw t('report.post_label', title: link_to(post_page_title(post), post_path(id))) - elsif type == 'comment' && !(comment = Comment.find_by_id(id)).nil? - # comment_message is not html_safe. To prevent - # cross-site-scripting we have to escape html - raw t('report.comment_label', data: link_to(h(comment_message(comment)), post_path(comment.post.id, anchor: comment.guid))) + def report_content(report) + case (item = report.item) + when Post + raw t("report.post_label", title: link_to(post_page_title(item), post_path(item.id))) + when Comment + raw t("report.comment_label", data: link_to( + h(comment_message(item)), + post_path(item.post.id, anchor: item.author.guid) + )) else - raw t('report.not_found') + raw t("report.not_found") end end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 9e0eca2c7..b4194e7c8 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -30,6 +30,8 @@ class Comment < ActiveRecord::Base validates :text, :presence => true, :length => {:maximum => 65535} validates :parent, :presence => true #should be in relayable (pending on fixing Message) + has_many :reports, as: :item + scope :including_author, -> { includes(:author => :profile) } scope :for_a_stream, -> { including_author.merge(order('created_at ASC')) } diff --git a/app/models/post.rb b/app/models/post.rb index e59576085..787924f8e 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -17,6 +17,8 @@ class Post < ActiveRecord::Base xml_attr :provider_display_name + has_many :reports, as: :item + has_many :mentions, :dependent => :destroy has_many :reshares, :class_name => "Reshare", :foreign_key => :root_guid, :primary_key => :guid diff --git a/app/models/report.rb b/app/models/report.rb index 36356c48f..0ecaea927 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -1,8 +1,8 @@ class Report < ActiveRecord::Base validates :user_id, presence: true validates :item_id, presence: true - validates :item_type, presence: true, :inclusion => { :in => %w(post comment), - :message => 'Type should match `post` or `comment`!'} + validates :item_type, presence: true, inclusion: { + in: %w(Post Comment), message: "Type should match `Post` or `Comment`!"} validates :text, presence: true validate :entry_does_not_exist, :on => :create @@ -11,9 +11,14 @@ class Report < ActiveRecord::Base belongs_to :user belongs_to :post belongs_to :comment + belongs_to :item, polymorphic: true after_commit :send_report_notification, :on => :create + def reported_author + item.author if item + end + def entry_does_not_exist if Report.where(item_id: item_id, item_type: item_type).exists?(user_id: user_id) errors[:base] << 'You cannot report the same post twice.' @@ -27,35 +32,24 @@ class Report < ActiveRecord::Base end def destroy_reported_item - if item_type == 'post' - delete_post - elsif item_type == 'comment' - delete_comment + case item + when Post + if item.author.local? + item.author.owner.retract(item) + else + item.destroy + end + when Comment + if item.author.local? + item.author.owner.retract(item) + elsif item.parent.author.local? + item.parent.author.owner.retract(item) + else + item.destroy + end end mark_as_reviewed end - - def delete_post - if post = Post.where(id: item_id).first - if post.author.local? - post.author.owner.retract(post) - else - post.destroy - end - end - end - - def delete_comment - if comment = Comment.where(id: item_id).first - if comment.author.local? - comment.author.owner.retract(comment) - elsif comment.parent.author.local? - comment.parent.author.owner.retract(comment) - else - comment.destroy - end - end - end def mark_as_reviewed Report.where(item_id: item_id, item_type: item_type).update_all(reviewed: true) diff --git a/app/views/report/index.html.haml b/app/views/report/index.html.haml index b13031cd7..2df36adf8 100644 --- a/app/views/report/index.html.haml +++ b/app/views/report/index.html.haml @@ -7,26 +7,38 @@ - if current_user.admin? = render partial: "admins/admin_bar" .col-md-9 - %h1 - = t('report.title') #reports - - @reports.each do |r| - - username = User.find_by_id(r.user_id).username - .content - %span.text - = report_content(r.item_id, r.item_type) - %span - = raw t('report.reported_label', person: link_to(username, user_profile_path(username))) - %span - = t('report.reason_label', text: r.text) - .options.text-right - %span - = button_to t('report.review_link'), report_path(r.id, :type => r.item_type), - :class => "btn btn-info btn-small", - method: :put - %span - = button_to t('report.delete_link'), report_path(r.id, :type => r.item_type), - :data => { :confirm => t('report.confirm_deletion') }, - :class => "btn btn-danger btn-small", - method: :delete - .clear + %h1 + = t("report.title") + - @reports.each do |report| + - if report.item + .panel.panel-default + - username = report.user.username + .panel-heading + .reporter.pull-right + = raw t("report.reported_label", person: link_to(username, user_profile_path(username))) + .title + = report_content(report) + .panel-body + .reason + = t("report.reason_label", text: report.text) + + = button_to t("report.reported_user_details"), + user_search_path(admins_controller_user_search: {guid: report.reported_author.guid}), + class: "btn pull-left btn-info btn-small", method: :post + = button_to t("report.review_link"), report_path(report.id, type: report.item_type), + class: "btn pull-left btn-info btn-small", method: :put + = button_to t("report.delete_link"), report_path(report.id, type: report.item_type), + data: {confirm: t("report.confirm_deletion")}, + class: "btn pull-right btn-danger btn-small", method: :delete + - else + .panel.panel-default + - username = report.user.username + .panel-heading + .reporter.pull-right + = raw t("report.reported_label", person: link_to(username, user_profile_path(username))) + .title + = report_content(report) + .panel-body + = button_to t("report.review_link"), report_path(report.id, type: report.item_type), + class: "btn pull-left btn-info btn-small", method: :put diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 30446b96f..7b30fee8f 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -1006,6 +1006,7 @@ en: reason_label: "Reason: %{text}" review_link: "Mark as reviewed" delete_link: "Delete item" + reported_user_details: "Details on reported user" confirm_deletion: "Are you sure to delete the item?" not_found: "The post/comment was not found. It seems that it was deleted by the user!" status: diff --git a/db/migrate/20151003142048_update_report_item_types.rb b/db/migrate/20151003142048_update_report_item_types.rb new file mode 100644 index 000000000..9a318edff --- /dev/null +++ b/db/migrate/20151003142048_update_report_item_types.rb @@ -0,0 +1,7 @@ +class UpdateReportItemTypes < ActiveRecord::Migration + def change + Report.all.each do |report| + report.update_attribute :item_type, report[:item_type].capitalize + end + end +end diff --git a/db/schema.rb b/db/schema.rb index f2afae365..6a526fe46 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150828132451) do +ActiveRecord::Schema.define(version: 20151003142048) do create_table "account_deletions", force: :cascade do |t| t.string "diaspora_handle", limit: 255 diff --git a/spec/controllers/report_controller_spec.rb b/spec/controllers/report_controller_spec.rb index e5d5a49eb..fc64d36dd 100644 --- a/spec/controllers/report_controller_spec.rb +++ b/spec/controllers/report_controller_spec.rb @@ -49,16 +49,16 @@ describe ReportController, type: :controller do context "report offensive post" do it "succeeds" do - put :create, report: {item_id: @message.id, item_type: "post", text: "offensive content"} + put :create, report: {item_id: @message.id, item_type: "Post", text: "offensive content"} expect(response.status).to eq(200) - expect(Report.exists?(item_id: @message.id, item_type: "post")).to be true + expect(Report.exists?(item_id: @message.id, item_type: "Post")).to be true end end context "report offensive comment" do it "succeeds" do - put :create, report: {item_id: @comment.id, item_type: "comment", text: "offensive content"} + put :create, report: {item_id: @comment.id, item_type: "Comment", text: "offensive content"} expect(response.status).to eq(200) - expect(Report.exists?(item_id: @comment.id, item_type: "comment")).to be true + expect(Report.exists?(item_id: @comment.id, item_type: "Comment")).to be true end end end @@ -68,14 +68,14 @@ describe ReportController, type: :controller do it "is behind redirect_unless_admin_or_moderator" do put :update, id: @message.id, type: "post" expect(response).to redirect_to stream_path - expect(Report.where(reviewed: false, item_id: @message.id, item_type: "post")).to be_truthy + expect(Report.where(reviewed: false, item_id: @message.id, item_type: "Post")).to be_truthy end end context "mark comment report as user" do it "is behind redirect_unless_admin_or_moderator" do put :update, id: @comment.id, type: "comment" expect(response).to redirect_to stream_path - expect(Report.where(reviewed: false, item_id: @comment.id, item_type: "comment")).to be_truthy + expect(Report.where(reviewed: false, item_id: @comment.id, item_type: "Comment")).to be_truthy end end @@ -86,7 +86,7 @@ describe ReportController, type: :controller do it "succeeds" do put :update, id: @message.id, type: "post" expect(response.status).to eq(302) - expect(Report.where(reviewed: true, item_id: @message.id, item_type: "post")).to be_truthy + expect(Report.where(reviewed: true, item_id: @message.id, item_type: "Post")).to be_truthy end end context "mark comment report as admin" do @@ -96,7 +96,7 @@ describe ReportController, type: :controller do it "succeeds" do put :update, id: @comment.id, type: "comment" expect(response.status).to eq(302) - expect(Report.where(reviewed: true, item_id: @comment.id, item_type: "comment")).to be_truthy + expect(Report.where(reviewed: true, item_id: @comment.id, item_type: "Comment")).to be_truthy end end @@ -108,7 +108,7 @@ describe ReportController, type: :controller do it "succeeds" do put :update, id: @message.id, type: "post" expect(response.status).to eq(302) - expect(Report.where(reviewed: true, item_id: @message.id, item_type: "post")).to be_truthy + expect(Report.where(reviewed: true, item_id: @message.id, item_type: "Post")).to be_truthy end end @@ -119,7 +119,7 @@ describe ReportController, type: :controller do it "succeeds" do put :update, id: @comment.id, type: "comment" expect(response.status).to eq(302) - expect(Report.where(reviewed: true, item_id: @comment.id, item_type: "comment")).to be_truthy + expect(Report.where(reviewed: true, item_id: @comment.id, item_type: "Comment")).to be_truthy end end end @@ -129,14 +129,14 @@ describe ReportController, type: :controller do it "is behind redirect_unless_admin_or_moderator" do delete :destroy, id: @message.id, type: "post" expect(response).to redirect_to stream_path - expect(Report.where(reviewed: false, item_id: @message.id, item_type: "post")).to be_truthy + expect(Report.where(reviewed: false, item_id: @message.id, item_type: "Post")).to be_truthy end end context "destroy comment as user" do it "is behind redirect_unless_admin_or_moderator" do delete :destroy, id: @comment.id, type: "comment" expect(response).to redirect_to stream_path - expect(Report.where(reviewed: false, item_id: @comment.id, item_type: "comment")).to be_truthy + expect(Report.where(reviewed: false, item_id: @comment.id, item_type: "Comment")).to be_truthy end end @@ -147,7 +147,7 @@ describe ReportController, type: :controller do it "succeeds" do delete :destroy, id: @message.id, type: "post" expect(response.status).to eq(302) - expect(Report.where(reviewed: true, item_id: @message.id, item_type: "post")).to be_truthy + expect(Report.where(reviewed: true, item_id: @message.id, item_type: "Post")).to be_truthy end end context "destroy comment as admin" do @@ -157,7 +157,7 @@ describe ReportController, type: :controller do it "succeeds" do delete :destroy, id: @comment.id, type: "comment" expect(response.status).to eq(302) - expect(Report.where(reviewed: true, item_id: @comment.id, item_type: "comment")).to be_truthy + expect(Report.where(reviewed: true, item_id: @comment.id, item_type: "Comment")).to be_truthy end end @@ -168,7 +168,7 @@ describe ReportController, type: :controller do it "succeeds" do delete :destroy, id: @message.id, type: "post" expect(response.status).to eq(302) - expect(Report.where(reviewed: true, item_id: @message.id, item_type: "post")).to be_truthy + expect(Report.where(reviewed: true, item_id: @message.id, item_type: "Post")).to be_truthy end end context "destroy comment as moderator" do @@ -178,7 +178,7 @@ describe ReportController, type: :controller do it "succeeds" do delete :destroy, id: @comment.id, type: "comment" expect(response.status).to eq(302) - expect(Report.where(reviewed: true, item_id: @comment.id, item_type: "comment")).to be_truthy + expect(Report.where(reviewed: true, item_id: @comment.id, item_type: "Comment")).to be_truthy end end end diff --git a/spec/helpers/report_helper_spec.rb b/spec/helpers/report_helper_spec.rb index 8cb003c42..f7d6b2510 100644 --- a/spec/helpers/report_helper_spec.rb +++ b/spec/helpers/report_helper_spec.rb @@ -1,17 +1,29 @@ -require 'spec_helper' +require "spec_helper" -describe ReportHelper, :type => :helper do +describe ReportHelper, type: :helper do before do - @comment = FactoryGirl.create(:comment) - @post = @comment.post + @user = bob + @post = @user.post(:status_message, text: "hello", to: @user.aspects.first.id) + @comment = @user.comment!(@post, "welcome") + + @post_report = @user.reports.create( + item_id: @post.id, item_type: "Post", + text: "offensive content" + ) + @comment_report = @user.reports.create( + item_id: @comment.id, item_type: "Comment", + text: "offensive content" + ) end describe "#report_content" do it "contains a link to the post" do - expect(helper.report_content(@post, 'post')).to include %Q(href="#{post_path(@post)}") + expect(helper.report_content(@post_report)) + .to include %(href="#{post_path(@post)}") end - it "contains an anchor to the comment" do - expect(helper.report_content(@comment, 'comment')).to include %Q(href="#{post_path(@post, anchor: @comment.guid)}") + it "contains an anchor to the comment" do + expect(helper.report_content(@comment_report)) + .to include %(href="#{post_path(@post, anchor: @comment.author.guid)}") end end end diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index bdfc17d38..04e4bc85a 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -12,14 +12,12 @@ describe Report, :type => :model do @bob_comment = @user.comment!(@bob_post, "welcome") @valid_post_report = { - :item_id => @bob_post.id, - :item_type => 'post', - :text => 'offensive content' + item_id: @bob_post.id, item_type: "Post", + text: "offensive content" } @valid_comment_report = { - :item_id => @bob_comment.id, - :item_type => 'comment', - :text => 'offensive content' + item_id: @bob_comment.id, item_type: "Comment", + text: "offensive content" } end