From 1748d3b9402c6b04b4a1e1bc9b71c7f7d3a3c62c Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Tue, 21 Jan 2014 09:09:45 -0500 Subject: [PATCH 01/31] It is now possible to report comments * Renamed PostReport to Report * Added report button to SPV * Updated rspec refs diaspora/diaspora#4732 refs diaspora/diaspora#4710 refs diaspora/diaspora#4711 refs diaspora/diaspora#4517 --- .../icons/{postreport.png => report.png} | Bin .../javascripts/app/models/post_report.js | 3 - app/assets/javascripts/app/models/report.js | 8 ++ .../javascripts/app/views/comment_view.js | 17 ++- .../javascripts/app/views/feedback_view.js | 17 ++- app/assets/javascripts/app/views/post_view.js | 15 +++ .../app/views/stream_post_views.js | 17 +-- app/assets/stylesheets/application.css.sass | 6 +- app/assets/stylesheets/entypo.css.scss | 1 + .../{post_report.css.scss => report.css.scss} | 4 +- .../stylesheets/single-post-view.css.scss | 6 + app/assets/templates/comment_tpl.jst.hbs | 14 +- .../single-post-actions_tpl.jst.hbs | 4 + .../templates/stream-element_tpl.jst.hbs | 4 +- app/controllers/post_report_controller.rb | 61 --------- app/controllers/report_controller.rb | 67 ++++++++++ app/helpers/report_helper.rb | 13 ++ app/mailers/post_report_mailer.rb | 18 --- app/mailers/report_mailer.rb | 26 ++++ app/models/{post_report.rb => report.rb} | 7 +- app/views/admins/_admin_bar.haml | 2 +- app/views/post_report/index.html.haml | 21 --- .../post_report/post_report_email.markerb | 2 - app/views/report/index.html.haml | 21 +++ app/views/report/report_email.markerb | 2 + app/workers/mail/post_report_worker.rb | 12 -- app/workers/mail/report_worker.rb | 12 ++ config/locales/diaspora/en.yml | 27 +++- config/locales/javascript/javascript.en.yml | 6 +- config/routes.rb | 2 +- ...0121132816_add_post_type_to_post_report.rb | 5 + ...0214104217_rename_post_report_to_report.rb | 8 ++ db/schema.rb | 12 ++ .../post_report_controller_spec.rb | 79 ----------- spec/controllers/report_controller_spec.rb | 126 ++++++++++++++++++ 35 files changed, 406 insertions(+), 239 deletions(-) rename app/assets/images/icons/{postreport.png => report.png} (100%) delete mode 100644 app/assets/javascripts/app/models/post_report.js create mode 100644 app/assets/javascripts/app/models/report.js rename app/assets/stylesheets/{post_report.css.scss => report.css.scss} (71%) delete mode 100644 app/controllers/post_report_controller.rb create mode 100644 app/controllers/report_controller.rb create mode 100644 app/helpers/report_helper.rb delete mode 100644 app/mailers/post_report_mailer.rb create mode 100644 app/mailers/report_mailer.rb rename app/models/{post_report.rb => report.rb} (53%) delete mode 100644 app/views/post_report/index.html.haml delete mode 100644 app/views/post_report/post_report_email.markerb create mode 100644 app/views/report/index.html.haml create mode 100644 app/views/report/report_email.markerb delete mode 100644 app/workers/mail/post_report_worker.rb create mode 100644 app/workers/mail/report_worker.rb create mode 100644 db/migrate/20140121132816_add_post_type_to_post_report.rb create mode 100644 db/migrate/20140214104217_rename_post_report_to_report.rb delete mode 100644 spec/controllers/post_report_controller_spec.rb create mode 100644 spec/controllers/report_controller_spec.rb diff --git a/app/assets/images/icons/postreport.png b/app/assets/images/icons/report.png similarity index 100% rename from app/assets/images/icons/postreport.png rename to app/assets/images/icons/report.png diff --git a/app/assets/javascripts/app/models/post_report.js b/app/assets/javascripts/app/models/post_report.js deleted file mode 100644 index 7a9705bb5..000000000 --- a/app/assets/javascripts/app/models/post_report.js +++ /dev/null @@ -1,3 +0,0 @@ -app.models.PostReport = Backbone.Model.extend({ - urlRoot: '/post_report' -}); diff --git a/app/assets/javascripts/app/models/report.js b/app/assets/javascripts/app/models/report.js new file mode 100644 index 000000000..594c7565b --- /dev/null +++ b/app/assets/javascripts/app/models/report.js @@ -0,0 +1,8 @@ +app.models.Report = Backbone.Model.extend({ + urlRoot: '/report', + + getReason: function() { + return prompt(Diaspora.I18n.t('report_prompt'), Diaspora.I18n.t('report_prompt_default')); + } + +}); diff --git a/app/assets/javascripts/app/views/comment_view.js b/app/assets/javascripts/app/views/comment_view.js index 8ed7ff48a..a02676fff 100644 --- a/app/assets/javascripts/app/views/comment_view.js +++ b/app/assets/javascripts/app/views/comment_view.js @@ -5,7 +5,8 @@ app.views.Comment = app.views.Content.extend({ events : function() { return _.extend({}, app.views.Content.prototype.events, { - "click .comment_delete": "destroyModel" + "click .comment_delete": "destroyModel", + "click .comment_report": "report" }); }, @@ -31,6 +32,20 @@ app.views.Comment = app.views.Content.extend({ canRemove : function() { return app.currentUser.authenticated() && (this.ownComment() || this.postOwner()) + }, + + report: function(evt) { + if(evt) { evt.preventDefault(); } + var report = new app.models.Report(); + var msg = report.getReason(); + if (msg !== null) { + var id = this.model.id; + var type = $(evt.currentTarget).data("type"); + report.fetch({ + data: { id: id, type: type, text: msg }, + type: 'POST' + }); + } } }); diff --git a/app/assets/javascripts/app/views/feedback_view.js b/app/assets/javascripts/app/views/feedback_view.js index d7f84a653..62e898f04 100644 --- a/app/assets/javascripts/app/views/feedback_view.js +++ b/app/assets/javascripts/app/views/feedback_view.js @@ -6,7 +6,8 @@ app.views.Feedback = app.views.Base.extend({ events: { "click *[rel='auth-required']" : "requireAuth", "click .like" : "toggleLike", - "click .reshare" : "resharePost" + "click .reshare" : "resharePost", + "click .post_report" : "report" }, tooltipSelector : ".label", @@ -44,5 +45,19 @@ app.views.Feedback = app.views.Base.extend({ if( app.currentUser.authenticated() ) { return } alert("you must be logged in to do that!") return false; + }, + + report: function(evt) { + if(evt) { evt.preventDefault(); } + var report = new app.models.Report(); + var msg = report.getReason(); + if (msg !== null) { + var id = this.model.id; + var type = $(evt.currentTarget).data("type"); + report.fetch({ + data: { id: id, type: type, text: msg }, + type: 'POST' + }); + } } }); diff --git a/app/assets/javascripts/app/views/post_view.js b/app/assets/javascripts/app/views/post_view.js index 7d45e6bdf..9aef82728 100644 --- a/app/assets/javascripts/app/views/post_view.js +++ b/app/assets/javascripts/app/views/post_view.js @@ -13,5 +13,20 @@ app.views.Post = app.views.Base.extend({ showPost : function() { return (app.currentUser.get("showNsfw")) || !this.model.get("nsfw") + }, + + report: function(evt) { + if(evt) { evt.preventDefault(); } + var report = new app.models.Report(); + var msg = report.getReason(); + if (msg !== null) { + var id = this.model.id; + var type = $(evt.currentTarget).data("type"); + report.fetch({ + data: { id: id, type: type, text: msg }, + type: 'POST' + }); + } } + }); diff --git a/app/assets/javascripts/app/views/stream_post_views.js b/app/assets/javascripts/app/views/stream_post_views.js index fb1d846d4..ef557362d 100644 --- a/app/assets/javascripts/app/views/stream_post_views.js +++ b/app/assets/javascripts/app/views/stream_post_views.js @@ -20,7 +20,7 @@ app.views.StreamPost = app.views.Post.extend({ "click .remove_post": "destroyModel", "click .hide_post": "hidePost", - "click .post_report": "postReport", + "click .post_report": "report", "click .block_user": "blockUser" }, @@ -108,21 +108,6 @@ app.views.StreamPost = app.views.Post.extend({ this.remove(); }, - postReport : function(evt) { - if(evt) { evt.preventDefault(); } - var text = prompt(Diaspora.I18n.t('post_report_prompt'), - Diaspora.I18n.t('post_report_prompt_default')); - - var postReport = new app.models.PostReport(); - postReport.fetch({ - data: { - post_id: this.model.id, - text: text - }, - type: 'POST' - }); - }, - focusCommentTextarea: function(evt){ evt.preventDefault(); this.$(".new_comment_form_wrapper").removeClass("hidden"); diff --git a/app/assets/stylesheets/application.css.sass b/app/assets/stylesheets/application.css.sass index 96184c315..52199d047 100644 --- a/app/assets/stylesheets/application.css.sass +++ b/app/assets/stylesheets/application.css.sass @@ -17,8 +17,8 @@ @import 'facebox' @import 'aspects' @import 'popover' -@import 'post_report' @import 'stream_element' +@import 'report' /* ====== media ====== */ .media @@ -211,10 +211,10 @@ ul.as-selections :z-index 6 :float right - .post_report + .post_report, .comment_report :display inline-block - .icons-postreport + .icons-report :height 14px :width 14px diff --git a/app/assets/stylesheets/entypo.css.scss b/app/assets/stylesheets/entypo.css.scss index a2d2a3774..615ed4e28 100644 --- a/app/assets/stylesheets/entypo.css.scss +++ b/app/assets/stylesheets/entypo.css.scss @@ -141,6 +141,7 @@ &.globe:before { content: '\1f30e'; } /* 1f30e */ &.graduation-cap:before { content: '\1f393 '; } /* 1f393 */ &.heart-empty:before { content: '\2661'; } /* 2661 */ + &.report:before { content: '\21'; } /* 21 */ &.heart:before { content: '\2665'; } /* 2665 */ &.help:before { content: '\2753'; } /* 2753 */ &.home:before { content: '\2302'; } /* 2302 */ diff --git a/app/assets/stylesheets/post_report.css.scss b/app/assets/stylesheets/report.css.scss similarity index 71% rename from app/assets/stylesheets/post_report.css.scss rename to app/assets/stylesheets/report.css.scss index ba865f9da..6fa29ce07 100644 --- a/app/assets/stylesheets/post_report.css.scss +++ b/app/assets/stylesheets/report.css.scss @@ -1,4 +1,4 @@ -#post_report { +#report { padding-top: 2em; span { display: block; @@ -11,6 +11,8 @@ } .clear { clear: both; + border-bottom: 1px solid #808080; padding-bottom: 1em; + margin-bottom: 1em; } } diff --git a/app/assets/stylesheets/single-post-view.css.scss b/app/assets/stylesheets/single-post-view.css.scss index 2fd30c888..056863407 100644 --- a/app/assets/stylesheets/single-post-view.css.scss +++ b/app/assets/stylesheets/single-post-view.css.scss @@ -61,6 +61,12 @@ i.comment:hover { color: #424242; } + i.report.gray:hover { + color: $red; + } + i.report.red:hover { + color: #f55f5a; + } i.heart.gray:hover { color: $red; } diff --git a/app/assets/templates/comment_tpl.jst.hbs b/app/assets/templates/comment_tpl.jst.hbs index e87eb2eeb..71be0bd4e 100644 --- a/app/assets/templates/comment_tpl.jst.hbs +++ b/app/assets/templates/comment_tpl.jst.hbs @@ -6,13 +6,17 @@
+
{{#if canRemove}} - diff --git a/app/assets/templates/stream-element_tpl.jst.hbs b/app/assets/templates/stream-element_tpl.jst.hbs index 21ed75fd4..45a73f9a0 100644 --- a/app/assets/templates/stream-element_tpl.jst.hbs +++ b/app/assets/templates/stream-element_tpl.jst.hbs @@ -10,8 +10,8 @@ {{#if loggedIn}}
{{#unless authorIsCurrentUser}} - -
+ +
diff --git a/app/controllers/post_report_controller.rb b/app/controllers/post_report_controller.rb deleted file mode 100644 index cd8dba827..000000000 --- a/app/controllers/post_report_controller.rb +++ /dev/null @@ -1,61 +0,0 @@ -class PostReportController < ApplicationController - before_filter :authenticate_user! - before_filter :redirect_unless_admin, :except => [:create] - - def index - @post_report = PostReport.where(reviewed: false).all - end - - def update - if PostReport.exists?(post_id: params[:id]) - mark_as_reviewed - end - redirect_to :action => :index and return - end - - def destroy - if Post.exists?(params[:id]) - delete_post - mark_as_reviewed - end - redirect_to :action => :index and return - end - - def create - username = current_user.username - unless PostReport.where(post_id: params[:post_id]).exists?(user_id: username) - post = PostReport.new( - :post_id => params[:post_id], - :user_id => username, - :text => params[:text]) - result = post.save - status(( 200 if result ) || ( 422 if !result )) - else - status(409) - end - end - - private - def delete_post - post = Post.find(params[:id]) - current_user.retract(post) - flash[:notice] = I18n.t 'post_report.status.destroyed' - end - - def mark_as_reviewed id = params[:id] - posts = PostReport.where(post_id: id) - posts.each do |post| - post.update_attributes(reviewed: true) - end - flash[:notice] = I18n.t 'post_report.status.marked' - end - - def status(code) - if code == 200 - flash[:notice] = I18n.t 'post_report.status.created' - else - flash[:error] = I18n.t 'post_report.status.failed' - end - render :nothing => true, :status => code - end -end diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb new file mode 100644 index 000000000..a5dcef21d --- /dev/null +++ b/app/controllers/report_controller.rb @@ -0,0 +1,67 @@ +class ReportController < ApplicationController + before_filter :authenticate_user! + before_filter :redirect_unless_admin, :except => [:create] + + def index + @report = Report.where(reviewed: false).all + end + + def update + if Report.where(post_type: params[:type]).exists?(post_id: params[:id]) + mark_as_reviewed + end + redirect_to :action => :index and return + end + + def destroy + if (params[:type].eql? "post") + if Post.exists?(params[:id]) + delete_post + end + elsif (params[:type].eql? "comment") + if Comment.exists?(params[:id]) + delete_comment + end + end + redirect_to :action => :index and return + end + + def create + code = 400 + username = current_user.username + post = Report.new( + :post_id => params[:id], + :post_type => params[:type], + :user_id => username, + :text => params[:text]) + unless Report.where("post_id = ? AND post_type = ?", params[:id], params[:type]).exists?(user_id: username) + result = post.save + code = 200 if result + end + render :nothing => true, :status => code + end + + private + def delete_post + post = Post.find(params[:id]) + current_user.retract(post) + mark_as_reviewed + flash[:notice] = I18n.t 'report.status.destroyed' + end + + def delete_comment + comment = Comment.find(params[:id]) + #current_user.retract(comment) + comment.destroy + mark_as_reviewed + flash[:notice] = I18n.t 'report.status.destroyed' + end + + def mark_as_reviewed + posts = Report.where("post_id = ? AND post_type = ?", params[:id], params[:type]) + posts.each do |post| + post.update_attributes(reviewed: true) + end + flash[:notice] = I18n.t 'report.status.marked' + end +end diff --git a/app/helpers/report_helper.rb b/app/helpers/report_helper.rb new file mode 100644 index 000000000..91b94e49f --- /dev/null +++ b/app/helpers/report_helper.rb @@ -0,0 +1,13 @@ +# Copyright (c) 2012, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +module ReportHelper + def report_content(id, type) + if type.eql? "post" + raw t('report.post_label', title: link_to(post_page_title(Post.find_by_id(id)), post_path(id))) + elsif type.eql? "comment" + raw t('report.comment_label', data: comment_message(Comment.find_by_id(id))) + end + end +end diff --git a/app/mailers/post_report_mailer.rb b/app/mailers/post_report_mailer.rb deleted file mode 100644 index 46b6737cc..000000000 --- a/app/mailers/post_report_mailer.rb +++ /dev/null @@ -1,18 +0,0 @@ -class PostReportMailer < ActionMailer::Base - default :from => AppConfig.mail.sender_address - - def new_report - Role.admins.each do |role| - email = User.find_by_id(role.person_id).email - format(email).deliver - end - end - - private - def format(email) - mail(to: email, subject: I18n.t('notifier.post_report_email.subject')) do |format| - format.text { render 'post_report/post_report_email' } - format.html { render 'post_report/post_report_email' } - end - end -end diff --git a/app/mailers/report_mailer.rb b/app/mailers/report_mailer.rb new file mode 100644 index 000000000..e7dbb911c --- /dev/null +++ b/app/mailers/report_mailer.rb @@ -0,0 +1,26 @@ +class ReportMailer < ActionMailer::Base + default :from => AppConfig.mail.sender_address + + def new_report(type, id) + url = AppConfig.pod_uri.to_s + url << report_index_path + resource = Hash[ + :subject => I18n.t('notifier.report_email.subject', :type => type), + :url => url, + :type => type, + :id => id + ] + Role.admins.each do |role| + resource[:email] = User.find_by_id(role.person_id).email + format(resource).deliver + end + end + + private + def format(resource) + mail(to: resource[:email], subject: resource[:subject]) do |format| + format.html { render 'report/report_email', :locals => { :resource => resource } } + format.text { render 'report/report_email', :locals => { :resource => resource } } + end + end +end diff --git a/app/models/post_report.rb b/app/models/report.rb similarity index 53% rename from app/models/post_report.rb rename to app/models/report.rb index a521162ad..b47a006dc 100644 --- a/app/models/post_report.rb +++ b/app/models/report.rb @@ -1,15 +1,16 @@ -class PostReport < ActiveRecord::Base +class Report < ActiveRecord::Base validates :user_id, presence: true validates :post_id, presence: true + validates :post_type, presence: true belongs_to :user belongs_to :post - has_many :post_reports + has_many :reports after_create :send_report_notification def send_report_notification - Workers::Mail::PostReportWorker.perform_async + Workers::Mail::ReportWorker.perform_async(self.post_type, self.post_id) end end diff --git a/app/views/admins/_admin_bar.haml b/app/views/admins/_admin_bar.haml index dff8955fa..cb6ac9b34 100644 --- a/app/views/admins/_admin_bar.haml +++ b/app/views/admins/_admin_bar.haml @@ -5,7 +5,7 @@ %li= link_to t('.user_search'), user_search_path %li= link_to t('.weekly_user_stats'), weekly_user_stats_path %li= link_to t('.pod_stats'), pod_stats_path - %li= link_to t('.post_report'), post_report_index_path + %li= link_to t('.report'), report_index_path %li= link_to t('.correlations'), correlations_path %li= link_to t('.sidekiq_monitor'), sidekiq_path diff --git a/app/views/post_report/index.html.haml b/app/views/post_report/index.html.haml deleted file mode 100644 index 538ff39b0..000000000 --- a/app/views/post_report/index.html.haml +++ /dev/null @@ -1,21 +0,0 @@ -.span-24 - = render :partial => 'admins/admin_bar' - -.span-24.last - %h1 - = t('post_report.title') - %div#post_report - - @post_report.each do |report| - %div.content - %span - = raw t('post_report.post_label', title: link_to(post_page_title(Post.find_by_id(report.post_id)), post_path(report.post_id))) - %span - = raw t('post_report.reported_label', person: link_to(report.user_id, user_profile_path(report.user_id))) - %span - = t('post_report.reason_label', text: report.text) - %div.options - %span - = link_to t('post_report.review_link'), post_report_path(report.post_id), method: :put - %span - = link_to t('post_report.delete_link'), post_report_path(report.post_id), method: :delete - %div.clear diff --git a/app/views/post_report/post_report_email.markerb b/app/views/post_report/post_report_email.markerb deleted file mode 100644 index fb4b688f1..000000000 --- a/app/views/post_report/post_report_email.markerb +++ /dev/null @@ -1,2 +0,0 @@ -<%= t('notifier.post_report_email.body') %> - diff --git a/app/views/report/index.html.haml b/app/views/report/index.html.haml new file mode 100644 index 000000000..8da7c73bd --- /dev/null +++ b/app/views/report/index.html.haml @@ -0,0 +1,21 @@ +.span-24 + = render :partial => 'admins/admin_bar' + +.span-24.last + %h1 + = t('report.title') + %div#report + - @report.each do |r| + %div.content + %span + = report_content(r.post_id, r.post_type) + %span + = raw t('report.reported_label', person: link_to(r.user_id, user_profile_path(r.user_id))) + %span + = t('report.reason_label', text: r.text) + %div.options + %span + = link_to t('report.review_link'), report_path(r.post_id, :type => r.post_type), method: :put + %span + = link_to t('report.delete_link'), report_path(r.post_id, :type => r.post_type), method: :delete + %div.clear diff --git a/app/views/report/report_email.markerb b/app/views/report/report_email.markerb new file mode 100644 index 000000000..ba787d56a --- /dev/null +++ b/app/views/report/report_email.markerb @@ -0,0 +1,2 @@ +<%= t('notifier.report_email.body', url: resource[:url], type: resource[:type], id: resource[:id]) %> + diff --git a/app/workers/mail/post_report_worker.rb b/app/workers/mail/post_report_worker.rb deleted file mode 100644 index a3f841942..000000000 --- a/app/workers/mail/post_report_worker.rb +++ /dev/null @@ -1,12 +0,0 @@ -module Workers - module Mail - class PostReportWorker < Base - sidekiq_options queue: :mail - - def perform - PostReportMailer.new_report - end - end - end -end - diff --git a/app/workers/mail/report_worker.rb b/app/workers/mail/report_worker.rb new file mode 100644 index 000000000..92c201dcb --- /dev/null +++ b/app/workers/mail/report_worker.rb @@ -0,0 +1,12 @@ +module Workers + module Mail + class ReportWorker < Base + sidekiq_options queue: :mail + + def perform(type, id) + ReportMailer.new_report(type, id) + end + end + end +end + diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 60dc8c116..e5a440f76 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -99,7 +99,7 @@ en: user_search: "User Search" weekly_user_stats: "Weekly User Stats" pod_stats: "Pod Stats" - post_report: "Reported Posts" + report: "Reports" correlations: "Correlations" sidekiq_monitor: "Sidekiq monitor" correlations: @@ -736,9 +736,23 @@ en: confirm_email: subject: "Please activate your new email address %{unconfirmed_email}" click_link: "To activate your new email address %{unconfirmed_email}, please follow this link:" - post_report_email: - subject: "A new post was marked as offensive" - body: "Please review as soon as possible!" + report_email: + subject: "A new %{type} was marked as offensive" + body: |- + Hello, + + the %{type} with ID %{id} was marked as offensive. + + [%{url}][1] + + Please review as soon as possible! + + + Cheers, + + The diaspora* email robot! + + [1]: %{url} accept_invite: "Accept Your diaspora* invite!" invited_you: "%{name} invited you to diaspora*" invite: @@ -868,9 +882,10 @@ en: other: "%{count} photos by %{author}" reshare_by: "Reshare by %{author}" - post_report: - title: "Marked Reports Overview" + report: + title: "Reports Overview" post_label: "Post: %{title}" + comment_label: "Comment:
%{data}" reported_label: "Reported by %{person}" reason_label: "Reason: %{text}" review_link: "Mark as reviewed" diff --git a/config/locales/javascript/javascript.en.yml b/config/locales/javascript/javascript.en.yml index 5a619206e..e10959db6 100644 --- a/config/locales/javascript/javascript.en.yml +++ b/config/locales/javascript/javascript.en.yml @@ -6,11 +6,11 @@ en: javascripts: confirm_dialog: "Are you sure?" - post_report_prompt: "Please enter a reason:" - post_report_prompt_default: "offensive content" + report_prompt: "Please enter a reason:" + report_prompt_default: "offensive content" delete: "Delete" ignore: "Ignore" - post_report: "Report" + report: "Report" ignore_user: "Ignore this user?" and: "and" comma: "," diff --git a/config/routes.rb b/config/routes.rb index 39b66432e..dc659bbbb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -5,7 +5,7 @@ require 'sidekiq/web' Diaspora::Application.routes.draw do - resources :post_report, :except => [:edit] + resources :report, :except => [:edit, :new] if Rails.env.production? mount RailsAdmin::Engine => '/admin_panel', :as => 'rails_admin' diff --git a/db/migrate/20140121132816_add_post_type_to_post_report.rb b/db/migrate/20140121132816_add_post_type_to_post_report.rb new file mode 100644 index 000000000..be50eca0a --- /dev/null +++ b/db/migrate/20140121132816_add_post_type_to_post_report.rb @@ -0,0 +1,5 @@ +class AddPostTypeToPostReport < ActiveRecord::Migration + def change + add_column :post_reports, :post_type, :string, :null => false, :after => :post_id + end +end diff --git a/db/migrate/20140214104217_rename_post_report_to_report.rb b/db/migrate/20140214104217_rename_post_report_to_report.rb new file mode 100644 index 000000000..2d477491e --- /dev/null +++ b/db/migrate/20140214104217_rename_post_report_to_report.rb @@ -0,0 +1,8 @@ +class RenamePostReportToReport < ActiveRecord::Migration + def self.up + rename_table :post_reports, :reports + end + def self.down + rename_table :reports, :post_reports + end +end diff --git a/db/schema.rb b/db/schema.rb index 97f7cb6c2..b483d9b03 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -410,6 +410,18 @@ ActiveRecord::Schema.define(:version => 20140308154022) do add_index "rails_admin_histories", ["item", "table", "month", "year"], :name => "index_rails_admin_histories" + create_table "reports", :force => true do |t| + t.integer "post_id", :null => false + t.string "post_type", :null => false + t.string "user_id" + t.boolean "reviewed", :default => false + t.text "text" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false + end + + add_index "reports", ["post_id"], :name => "index_post_reports_on_post_id" + create_table "roles", :force => true do |t| t.integer "person_id" t.string "name" diff --git a/spec/controllers/post_report_controller_spec.rb b/spec/controllers/post_report_controller_spec.rb deleted file mode 100644 index 513e19164..000000000 --- a/spec/controllers/post_report_controller_spec.rb +++ /dev/null @@ -1,79 +0,0 @@ -require 'spec_helper' - -describe PostReportController do - before do - sign_in alice - @message = alice.post(:status_message, :text => "hey", :to => alice.aspects.first.id) - end - - describe '#index' do - context 'admin not signed in' do - it 'is behind redirect_unless_admin' do - get :index - response.should redirect_to stream_path - end - end - - context 'admin signed in' do - before do - Role.add_admin(alice.person) - end - it 'succeeds and renders index' do - get :index - response.should render_template('index') - end - end - end - - describe '#create' do - context 'report offensive content' do - it 'succeeds' do - put :create, :post_id => @message.id, :text => 'offensive content' - response.status.should == 200 - PostReport.exists?(:post_id => @message.id).should be_true - end - end - end - - describe '#update' do - context 'mark report as user' do - it 'is behind redirect_unless_admin' do - put :update, :id => @message.id - response.should redirect_to stream_path - PostReport.where(:reviewed => false, :post_id => @message.id).should be_true - end - end - - context 'mark report as admin' do - before do - Role.add_admin(alice.person) - end - it 'succeeds' do - put :update, :id => @message.id - response.status.should == 302 - PostReport.where(:reviewed => true, :post_id => @message.id).should be_true - end - end - end - - describe '#destroy' do - context 'destroy post as user' do - it 'is behind redirect_unless_admin' do - delete :destroy, :id => @message.id - response.should redirect_to stream_path - PostReport.where(:reviewed => false, :post_id => @message.id).should be_true - end - end - - context 'destroy post as admin' do - before do - Role.add_admin(alice.person) - end - it 'succeeds' do - delete :destroy, :id => @message.id - response.status.should == 302 - PostReport.where(:reviewed => true, :post_id => @message.id).should be_true - end - end - end -end diff --git a/spec/controllers/report_controller_spec.rb b/spec/controllers/report_controller_spec.rb new file mode 100644 index 000000000..614dc1b45 --- /dev/null +++ b/spec/controllers/report_controller_spec.rb @@ -0,0 +1,126 @@ +require 'spec_helper' + +describe ReportController do + before do + sign_in alice + @message = alice.post(:status_message, :text => "hey", :to => alice.aspects.first.id) + @comment = alice.comment!(@message, "flying pigs, everywhere") + end + + describe '#index' do + context 'admin not signed in' do + it 'is behind redirect_unless_admin' do + get :index + response.should redirect_to stream_path + end + end + + context 'admin signed in' do + before do + Role.add_admin(alice.person) + end + it 'succeeds and renders index' do + get :index + response.should render_template('index') + end + end + end + + describe '#create' do + let(:comment_hash) { + {:text =>"facebook, is that you?", + :post_id =>"#{@post.id}"} + } + + context 'report offensive post' do + it 'succeeds' do + put :create, :id => @message.id, :type => 'post', :text => 'offensive content' + response.status.should == 200 + Report.exists?(:post_id => @message.id, :post_type => 'post').should be_true + end + end + context 'report offensive comment' do + it 'succeeds' do + put :create, :id => @comment.id, :type => 'comment', :text => 'offensive content' + response.status.should == 200 + Report.exists?(:post_id => @comment.id, :post_type => 'comment').should be_true + end + end + end + + describe '#update' do + context 'mark post report as user' do + it 'is behind redirect_unless_admin' do + put :update, :id => @message.id, :type => 'post' + response.should redirect_to stream_path + Report.where(:reviewed => false, :post_id => @message.id, :post_type => 'post').should be_true + end + end + context 'mark comment report as user' do + it 'is behind redirect_unless_admin' do + put :update, :id => @comment.id, :type => 'comment' + response.should redirect_to stream_path + Report.where(:reviewed => false, :post_id => @comment.id, :post_type => 'comment').should be_true + end + end + + context 'mark post report as admin' do + before do + Role.add_admin(alice.person) + end + it 'succeeds' do + put :update, :id => @message.id, :type => 'post' + response.status.should == 302 + Report.where(:reviewed => true, :post_id => @message.id, :post_type => 'post').should be_true + end + end + context 'mark comment report as admin' do + before do + Role.add_admin(alice.person) + end + it 'succeeds' do + put :update, :id => @comment.id, :type => 'comment' + response.status.should == 302 + Report.where(:reviewed => true, :post_id => @comment.id, :post_type => 'comment').should be_true + end + end + end + + describe '#destroy' do + context 'destroy post as user' do + it 'is behind redirect_unless_admin' do + delete :destroy, :id => @message.id, :type => 'post' + response.should redirect_to stream_path + Report.where(:reviewed => false, :post_id => @message.id, :post_type => 'post').should be_true + end + end + context 'destroy comment as user' do + it 'is behind redirect_unless_admin' do + delete :destroy, :id => @comment.id, :type => 'comment' + response.should redirect_to stream_path + Report.where(:reviewed => false, :post_id => @comment.id, :post_type => 'comment').should be_true + end + end + + context 'destroy post as admin' do + before do + Role.add_admin(alice.person) + end + it 'succeeds' do + delete :destroy, :id => @message.id, :type => 'post' + response.status.should == 302 + Report.where(:reviewed => true, :post_id => @message.id, :post_type => 'post').should be_true + end + end + context 'destroy comment as admin' do + before do + Role.add_admin(alice.person) + end + it 'succeeds' do + delete :destroy, :id => @comment.id, :type => 'comment' + response.status.should == 302 + Report.where(:reviewed => true, :post_id => @comment.id, :post_type => 'comment').should be_true + end + end + end +end From d23f4a66da8783c9b4550f67261c8d9c16947264 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 24 Feb 2014 11:57:59 -0500 Subject: [PATCH 02/31] Cleaned javascript report view --- app/assets/javascripts/app/models/report.js | 5 ----- app/assets/javascripts/app/views.js | 14 ++++++++++++++ app/assets/javascripts/app/views/comment_view.js | 14 -------------- app/assets/javascripts/app/views/feedback_view.js | 14 -------------- app/assets/javascripts/app/views/post_view.js | 15 --------------- 5 files changed, 14 insertions(+), 48 deletions(-) diff --git a/app/assets/javascripts/app/models/report.js b/app/assets/javascripts/app/models/report.js index 594c7565b..394aa0ce4 100644 --- a/app/assets/javascripts/app/models/report.js +++ b/app/assets/javascripts/app/models/report.js @@ -1,8 +1,3 @@ app.models.Report = Backbone.Model.extend({ urlRoot: '/report', - - getReason: function() { - return prompt(Diaspora.I18n.t('report_prompt'), Diaspora.I18n.t('report_prompt_default')); - } - }); diff --git a/app/assets/javascripts/app/views.js b/app/assets/javascripts/app/views.js index f5c17c0f9..8ff682cc1 100644 --- a/app/assets/javascripts/app/views.js +++ b/app/assets/javascripts/app/views.js @@ -81,6 +81,20 @@ app.views.Base = Backbone.View.extend({ } }, + report: function(evt) { + if(evt) { evt.preventDefault(); } + var msg = prompt(Diaspora.I18n.t('report_prompt'), Diaspora.I18n.t('report_prompt_default')); + if (msg !== null) { + var report = new app.models.Report(); + var id = this.model.id; + var type = $(evt.currentTarget).data("type"); + report.fetch({ + data: { id: id, type: type, text: msg }, + type: 'POST' + }); + } + }, + destroyModel: function(evt) { evt && evt.preventDefault(); var self = this; diff --git a/app/assets/javascripts/app/views/comment_view.js b/app/assets/javascripts/app/views/comment_view.js index a02676fff..540c54b99 100644 --- a/app/assets/javascripts/app/views/comment_view.js +++ b/app/assets/javascripts/app/views/comment_view.js @@ -32,20 +32,6 @@ app.views.Comment = app.views.Content.extend({ canRemove : function() { return app.currentUser.authenticated() && (this.ownComment() || this.postOwner()) - }, - - report: function(evt) { - if(evt) { evt.preventDefault(); } - var report = new app.models.Report(); - var msg = report.getReason(); - if (msg !== null) { - var id = this.model.id; - var type = $(evt.currentTarget).data("type"); - report.fetch({ - data: { id: id, type: type, text: msg }, - type: 'POST' - }); - } } }); diff --git a/app/assets/javascripts/app/views/feedback_view.js b/app/assets/javascripts/app/views/feedback_view.js index 62e898f04..41e957c89 100644 --- a/app/assets/javascripts/app/views/feedback_view.js +++ b/app/assets/javascripts/app/views/feedback_view.js @@ -45,19 +45,5 @@ app.views.Feedback = app.views.Base.extend({ if( app.currentUser.authenticated() ) { return } alert("you must be logged in to do that!") return false; - }, - - report: function(evt) { - if(evt) { evt.preventDefault(); } - var report = new app.models.Report(); - var msg = report.getReason(); - if (msg !== null) { - var id = this.model.id; - var type = $(evt.currentTarget).data("type"); - report.fetch({ - data: { id: id, type: type, text: msg }, - type: 'POST' - }); - } } }); diff --git a/app/assets/javascripts/app/views/post_view.js b/app/assets/javascripts/app/views/post_view.js index 9aef82728..7d45e6bdf 100644 --- a/app/assets/javascripts/app/views/post_view.js +++ b/app/assets/javascripts/app/views/post_view.js @@ -13,20 +13,5 @@ app.views.Post = app.views.Base.extend({ showPost : function() { return (app.currentUser.get("showNsfw")) || !this.model.get("nsfw") - }, - - report: function(evt) { - if(evt) { evt.preventDefault(); } - var report = new app.models.Report(); - var msg = report.getReason(); - if (msg !== null) { - var id = this.model.id; - var type = $(evt.currentTarget).data("type"); - report.fetch({ - data: { id: id, type: type, text: msg }, - type: 'POST' - }); - } } - }); From ed96ddac989e1faa98066c3319540d59660bbc5c Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Tue, 25 Feb 2014 12:16:15 -0500 Subject: [PATCH 03/31] Display status when the user send a report --- app/assets/javascripts/app/views.js | 34 +++++++++++++------ app/assets/templates/comment_tpl.jst.hbs | 2 +- .../single-post-actions_tpl.jst.hbs | 2 +- .../templates/stream-element_tpl.jst.hbs | 2 +- config/locales/javascript/javascript.en.yml | 10 ++++-- 5 files changed, 34 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/app/views.js b/app/assets/javascripts/app/views.js index 8ff682cc1..f65d04379 100644 --- a/app/assets/javascripts/app/views.js +++ b/app/assets/javascripts/app/views.js @@ -83,16 +83,30 @@ app.views.Base = Backbone.View.extend({ report: function(evt) { if(evt) { evt.preventDefault(); } - var msg = prompt(Diaspora.I18n.t('report_prompt'), Diaspora.I18n.t('report_prompt_default')); - if (msg !== null) { - var report = new app.models.Report(); - var id = this.model.id; - var type = $(evt.currentTarget).data("type"); - report.fetch({ - data: { id: id, type: type, text: msg }, - type: 'POST' - }); - } + var msg = prompt(Diaspora.I18n.t('report.prompt'), Diaspora.I18n.t('report.prompt_default')); + if (msg == null) return; + var report = new app.models.Report(); + var id = this.model.id; + var type = $(evt.currentTarget).data("type"); + + report.fetch({ + data: { id: id, type: type, text: msg }, + type: 'POST', + statusCode: { + 200: function(xhr) { + Diaspora.page.flashMessages.render({ + success: true, + notice: Diaspora.I18n.t('report.status.created') + }); + }, + 400: function(xhr) { + Diaspora.page.flashMessages.render({ + success: false, + notice: Diaspora.I18n.t('report.status.exists') + }); + } + } + }); }, destroyModel: function(evt) { diff --git a/app/assets/templates/comment_tpl.jst.hbs b/app/assets/templates/comment_tpl.jst.hbs index 71be0bd4e..de65d6d64 100644 --- a/app/assets/templates/comment_tpl.jst.hbs +++ b/app/assets/templates/comment_tpl.jst.hbs @@ -12,7 +12,7 @@
{{else}} - +
{{/if}} diff --git a/app/assets/templates/single-post-viewer/single-post-actions_tpl.jst.hbs b/app/assets/templates/single-post-viewer/single-post-actions_tpl.jst.hbs index 26fcd0822..2d188f6ac 100644 --- a/app/assets/templates/single-post-viewer/single-post-actions_tpl.jst.hbs +++ b/app/assets/templates/single-post-viewer/single-post-actions_tpl.jst.hbs @@ -23,7 +23,7 @@ {{/if}} {{/if}} - +
diff --git a/app/assets/templates/stream-element_tpl.jst.hbs b/app/assets/templates/stream-element_tpl.jst.hbs index 45a73f9a0..9a6952808 100644 --- a/app/assets/templates/stream-element_tpl.jst.hbs +++ b/app/assets/templates/stream-element_tpl.jst.hbs @@ -10,7 +10,7 @@ {{#if loggedIn}}
{{#unless authorIsCurrentUser}} - +
diff --git a/config/locales/javascript/javascript.en.yml b/config/locales/javascript/javascript.en.yml index e10959db6..a9647a999 100644 --- a/config/locales/javascript/javascript.en.yml +++ b/config/locales/javascript/javascript.en.yml @@ -6,11 +6,15 @@ en: javascripts: confirm_dialog: "Are you sure?" - report_prompt: "Please enter a reason:" - report_prompt_default: "offensive content" delete: "Delete" ignore: "Ignore" - report: "Report" + report: + prompt: "Please enter a reason:" + prompt_default: "offensive content" + name: "Report" + status: + created: "The report was successfully created" + exists: "The report already exists" ignore_user: "Ignore this user?" and: "and" comma: "," From 2e36f8d375cbf36837d4c4162522b9f751b8150a Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Fri, 21 Mar 2014 09:21:12 -0400 Subject: [PATCH 04/31] Diaspora review part 1 * join the conditions of the inner ifs * add a uniqueness constraint to the model * differentiate between author is a local or a remote user * simplify controller/mailer functions --- app/assets/javascripts/app/views.js | 10 ++++- app/assets/stylesheets/report.css.scss | 2 +- app/controllers/report_controller.rb | 62 ++++++++------------------ app/mailers/report_mailer.rb | 8 ++-- app/models/report.rb | 49 +++++++++++++++++++- app/models/user.rb | 2 + app/views/report/index.html.haml | 8 ++-- 7 files changed, 83 insertions(+), 58 deletions(-) diff --git a/app/assets/javascripts/app/views.js b/app/assets/javascripts/app/views.js index f65d04379..8f2c718d4 100644 --- a/app/assets/javascripts/app/views.js +++ b/app/assets/javascripts/app/views.js @@ -90,7 +90,13 @@ app.views.Base = Backbone.View.extend({ var type = $(evt.currentTarget).data("type"); report.fetch({ - data: { id: id, type: type, text: msg }, + data: { + report: { + post_id: id, + post_type: type, + text: msg + } + }, type: 'POST', statusCode: { 200: function(xhr) { @@ -142,4 +148,4 @@ app.views.StaticContentView = app.views.Base.extend({ presenter : function(){ return this.data; }, -}); \ No newline at end of file +}); diff --git a/app/assets/stylesheets/report.css.scss b/app/assets/stylesheets/report.css.scss index 6fa29ce07..61816777a 100644 --- a/app/assets/stylesheets/report.css.scss +++ b/app/assets/stylesheets/report.css.scss @@ -1,4 +1,4 @@ -#report { +#reports { padding-top: 2em; span { display: block; diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb index a5dcef21d..7e379f343 100644 --- a/app/controllers/report_controller.rb +++ b/app/controllers/report_controller.rb @@ -3,65 +3,39 @@ class ReportController < ApplicationController before_filter :redirect_unless_admin, :except => [:create] def index - @report = Report.where(reviewed: false).all + @reports = Report.where(reviewed: false).all end def update - if Report.where(post_type: params[:type]).exists?(post_id: params[:id]) - mark_as_reviewed + if report = Report.where(id: params[:id]).first + report.mark_as_reviewed end - redirect_to :action => :index and return + redirect_to :action => :index end def destroy - if (params[:type].eql? "post") - if Post.exists?(params[:id]) - delete_post - end - elsif (params[:type].eql? "comment") - if Comment.exists?(params[:id]) - delete_comment + if report = Report.where(id: params[:id]).first + if report.destroy_reported_item + flash[:notice] = I18n.t 'report.status.destroyed' + else + flash[:error] = I18n.t 'report.status.failed' end + else + flash[:error] = I18n.t 'report.status.failed' end - redirect_to :action => :index and return + redirect_to :action => :index end def create - code = 400 - username = current_user.username - post = Report.new( - :post_id => params[:id], - :post_type => params[:type], - :user_id => username, - :text => params[:text]) - unless Report.where("post_id = ? AND post_type = ?", params[:id], params[:type]).exists?(user_id: username) - result = post.save - code = 200 if result + if current_user.reports.create! report_params + flash.now[:notice] = I18n.t 'report.status.created' + else + flash.now[:error] = I18n.t 'report.status.failed' end - render :nothing => true, :status => code end private - def delete_post - post = Post.find(params[:id]) - current_user.retract(post) - mark_as_reviewed - flash[:notice] = I18n.t 'report.status.destroyed' - end - - def delete_comment - comment = Comment.find(params[:id]) - #current_user.retract(comment) - comment.destroy - mark_as_reviewed - flash[:notice] = I18n.t 'report.status.destroyed' - end - - def mark_as_reviewed - posts = Report.where("post_id = ? AND post_type = ?", params[:id], params[:type]) - posts.each do |post| - post.update_attributes(reviewed: true) - end - flash[:notice] = I18n.t 'report.status.marked' + def report_params + params.require(:report).permit(:post_id, :post_type, :text) end end diff --git a/app/mailers/report_mailer.rb b/app/mailers/report_mailer.rb index e7dbb911c..70a4b6e53 100644 --- a/app/mailers/report_mailer.rb +++ b/app/mailers/report_mailer.rb @@ -2,14 +2,12 @@ class ReportMailer < ActionMailer::Base default :from => AppConfig.mail.sender_address def new_report(type, id) - url = AppConfig.pod_uri.to_s - url << report_index_path - resource = Hash[ + resource = { :subject => I18n.t('notifier.report_email.subject', :type => type), - :url => url, + :url => report_index_url, :type => type, :id => id - ] + } Role.admins.each do |role| resource[:email] = User.find_by_id(role.person_id).email format(resource).deliver diff --git a/app/models/report.rb b/app/models/report.rb index b47a006dc..149177b5b 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -2,14 +2,59 @@ class Report < ActiveRecord::Base validates :user_id, presence: true validates :post_id, presence: true validates :post_type, presence: true + validates :text, presence: true + + validate :entry_exists, :on => :create belongs_to :user belongs_to :post - - has_many :reports + belongs_to :comment after_create :send_report_notification + def entry_exists + if Report.where(post_id: post_id, post_type: post_type).exists?(user_id: user_id) + errors[:base] << 'You cannot report the same post twice.' + end + end + + def destroy_reported_item + if post_type == 'post' + delete_post + elsif post_type == 'comment' + delete_comment + end + mark_as_reviewed + end + + def delete_post + if post = Post.where(id: post_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: post_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 + if reports = Report.where(post_id: post_id, post_type: post_type) + reports.update_all(reviewed: true) + end + end + def send_report_notification Workers::Mail::ReportWorker.perform_async(self.post_type, self.post_id) end diff --git a/app/models/user.rb b/app/models/user.rb index 0261857f9..2e9168a80 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -69,6 +69,8 @@ class User < ActiveRecord::Base has_many :notifications, :foreign_key => :recipient_id + has_many :reports + before_save :guard_unconfirmed_email, :save_person! diff --git a/app/views/report/index.html.haml b/app/views/report/index.html.haml index 8da7c73bd..b27826a28 100644 --- a/app/views/report/index.html.haml +++ b/app/views/report/index.html.haml @@ -4,8 +4,8 @@ .span-24.last %h1 = t('report.title') - %div#report - - @report.each do |r| + %div#reports + - @reports.each do |r| %div.content %span = report_content(r.post_id, r.post_type) @@ -15,7 +15,7 @@ = t('report.reason_label', text: r.text) %div.options %span - = link_to t('report.review_link'), report_path(r.post_id, :type => r.post_type), method: :put + = link_to t('report.review_link'), report_path(r.id, :type => r.post_type), method: :put %span - = link_to t('report.delete_link'), report_path(r.post_id, :type => r.post_type), method: :delete + = link_to t('report.delete_link'), report_path(r.id, :type => r.post_type), method: :delete %div.clear From 719edcd1a7cb79827935fced10a922a037af3658 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Fri, 21 Mar 2014 09:37:21 -0400 Subject: [PATCH 05/31] Added missing action in report controller --- app/controllers/report_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb index 7e379f343..001ad7916 100644 --- a/app/controllers/report_controller.rb +++ b/app/controllers/report_controller.rb @@ -29,8 +29,10 @@ class ReportController < ApplicationController def create if current_user.reports.create! report_params flash.now[:notice] = I18n.t 'report.status.created' + render :nothing => true, :status => 200 else flash.now[:error] = I18n.t 'report.status.failed' + render :nothing => true, :status => 409 end end From 26d0c81dae88370b5efd1d10a813eb157eb580ab Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Fri, 21 Mar 2014 10:48:48 -0400 Subject: [PATCH 06/31] Added the ability to disable report-email-notification Podmin can see a extra checkbox in Settings > Account for disabling report-email-notification --- app/controllers/users_controller.rb | 1 + app/mailers/report_mailer.rb | 7 +++++-- app/models/user_preference.rb | 3 ++- app/views/users/edit.html.haml | 11 +++++++++++ app/views/users/edit.mobile.haml | 15 +++++++++++++-- config/locales/diaspora/en.yml | 1 + 6 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 886b905b1..b893621c5 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -179,6 +179,7 @@ class UsersController < ApplicationController :remember_me, :getting_started, email_preferences: [ + :someone_reported, :also_commented, :mentioned, :comment_on_post, diff --git a/app/mailers/report_mailer.rb b/app/mailers/report_mailer.rb index 70a4b6e53..a9223c752 100644 --- a/app/mailers/report_mailer.rb +++ b/app/mailers/report_mailer.rb @@ -9,8 +9,11 @@ class ReportMailer < ActionMailer::Base :id => id } Role.admins.each do |role| - resource[:email] = User.find_by_id(role.person_id).email - format(resource).deliver + user = User.find_by_id(role.person_id) + if !user.user_preferences.exists?(:email_type => :someone_reported) + resource[:email] = user.email + format(resource).deliver + end end end diff --git a/app/models/user_preference.rb b/app/models/user_preference.rb index cf73c7d58..354a48400 100644 --- a/app/models/user_preference.rb +++ b/app/models/user_preference.rb @@ -4,7 +4,8 @@ class UserPreference < ActiveRecord::Base validate :must_be_valid_email_type VALID_EMAIL_TYPES = - ["mentioned", + ["someone_reported", + "mentioned", "comment_on_post", "private_message", "started_sharing", diff --git a/app/views/users/edit.html.haml b/app/views/users/edit.html.haml index 88458c021..55af40b83 100644 --- a/app/views/users/edit.html.haml +++ b/app/views/users/edit.html.haml @@ -125,11 +125,22 @@ = f.fields_for :email_preferences do |type| #email_prefs + - if current_user.admin? + %p.checkbox_select + = type.label :someone_reported, t('.someone_reported') + = type.check_box :someone_reported, {:checked => @email_prefs['someone_reported']}, false, true + + %br %p.checkbox_select = type.label :started_sharing, t('.started_sharing') = type.check_box :started_sharing, {:checked => @email_prefs['started_sharing']}, false, true %br + %p.checkbox_select + = type.label :also_commented, t('.also_commented') + = type.check_box :also_commented, {:checked => @email_prefs['also_commented']}, false, true + %br + %p.checkbox_select = type.label :mentioned, t('.mentioned') = type.check_box :mentioned, {:checked => @email_prefs['mentioned']}, false, true diff --git a/app/views/users/edit.mobile.haml b/app/views/users/edit.mobile.haml index a08170ed5..a1bbc08d6 100644 --- a/app/views/users/edit.mobile.haml +++ b/app/views/users/edit.mobile.haml @@ -113,14 +113,25 @@ = f.fields_for :email_preferences do |type| #email_prefs + - if current_user.admin? + %p.checkbox_select + = type.label :someone_reported, t('.someone_reported') + = type.check_box :someone_reported, {:checked => @email_prefs['someone_reported']}, false, true + + %br %p.checkbox_select = type.label :started_sharing, t('.started_sharing') = type.check_box :started_sharing, {:checked => @email_prefs['started_sharing']}, false, true %br %p.checkbox_select - = type.label :mentioned, t('.mentioned') - = type.check_box :mentioned, {:checked => @email_prefs['mentioned']}, false, true + = type.label :also_commented, t('.also_commented') + = type.check_box :also_commented, {:checked => @email_prefs['also_commented']}, false, true + %br + + %p.checkbox_select + = type.label :mentioned, t('.mentioned') + = type.check_box :mentioned, {:checked => @email_prefs['mentioned']}, false, true %br %p.checkbox_select diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index e5a440f76..7b8ac5e08 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -1221,6 +1221,7 @@ en: edit_account: "Edit account" receive_email_notifications: "Receive email notifications when:" started_sharing: "someone starts sharing with you" + someone_reported: "someone sent a report" mentioned: "you are mentioned in a post" liked: "someone likes your post" reshared: "someone reshares your post" From 6f21ccda064f749a7feca01224a7d91445ccc91b Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Sat, 22 Mar 2014 21:01:34 +0100 Subject: [PATCH 07/31] Using case instead of equal --- app/helpers/report_helper.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/helpers/report_helper.rb b/app/helpers/report_helper.rb index 91b94e49f..3d8ee929b 100644 --- a/app/helpers/report_helper.rb +++ b/app/helpers/report_helper.rb @@ -4,10 +4,11 @@ module ReportHelper def report_content(id, type) - if type.eql? "post" - raw t('report.post_label', title: link_to(post_page_title(Post.find_by_id(id)), post_path(id))) - elsif type.eql? "comment" - raw t('report.comment_label', data: comment_message(Comment.find_by_id(id))) + raw case type + when 'post' + t('report.post_label', title: link_to(post_page_title(Post.find_by_id(id)), post_path(id))) + when 'comment' + t('report.comment_label', data: comment_message(Comment.find_by_id(id))) end end end From 1a0c9f5983b63eff1f17aa96e4e1f4dc502004c1 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Sat, 22 Mar 2014 21:03:27 +0100 Subject: [PATCH 08/31] Make report-type translatable --- app/mailers/report_mailer.rb | 5 +++-- config/locales/diaspora/en.yml | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/mailers/report_mailer.rb b/app/mailers/report_mailer.rb index a9223c752..4a9b639a9 100644 --- a/app/mailers/report_mailer.rb +++ b/app/mailers/report_mailer.rb @@ -2,10 +2,11 @@ class ReportMailer < ActionMailer::Base default :from => AppConfig.mail.sender_address def new_report(type, id) + report_type = I18n.t('notifier.report_email.type.' + type) resource = { - :subject => I18n.t('notifier.report_email.subject', :type => type), + :subject => I18n.t('notifier.report_email.subject', :type => report_type), :url => report_index_url, - :type => type, + :type => report_type, :id => id } Role.admins.each do |role| diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 7b8ac5e08..3401ec951 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -737,6 +737,9 @@ en: subject: "Please activate your new email address %{unconfirmed_email}" click_link: "To activate your new email address %{unconfirmed_email}, please follow this link:" report_email: + type: + post: "post" + comment: "comment" subject: "A new %{type} was marked as offensive" body: |- Hello, From 0fae1137fa35cd92bc55a4da22525865a8eb9e38 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Sat, 22 Mar 2014 21:04:09 +0100 Subject: [PATCH 09/31] Using unless instead of 'if !' --- app/mailers/report_mailer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/mailers/report_mailer.rb b/app/mailers/report_mailer.rb index 4a9b639a9..143f22d8b 100644 --- a/app/mailers/report_mailer.rb +++ b/app/mailers/report_mailer.rb @@ -11,7 +11,7 @@ class ReportMailer < ActionMailer::Base } Role.admins.each do |role| user = User.find_by_id(role.person_id) - if !user.user_preferences.exists?(:email_type => :someone_reported) + unless user.user_preferences.exists?(:email_type => :someone_reported) resource[:email] = user.email format(resource).deliver end From 6309e1a4eed6d64928b5ee9e860755985c23a8b6 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Sat, 22 Mar 2014 21:06:08 +0100 Subject: [PATCH 10/31] Cleaned and optimized report model --- app/models/report.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/report.rb b/app/models/report.rb index 149177b5b..6abac47d0 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -10,7 +10,7 @@ class Report < ActiveRecord::Base belongs_to :post belongs_to :comment - after_create :send_report_notification + after_commit :send_report_notification, :on => :create def entry_exists if Report.where(post_id: post_id, post_type: post_type).exists?(user_id: user_id) @@ -50,9 +50,7 @@ class Report < ActiveRecord::Base end def mark_as_reviewed - if reports = Report.where(post_id: post_id, post_type: post_type) - reports.update_all(reviewed: true) - end + Report.where(post_id: post_id, post_type: post_type).update_all(reviewed: true) end def send_report_notification From 512d96bda6715e49ab43ddf811f81bbf4edec17e Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 24 Mar 2014 11:16:14 -0400 Subject: [PATCH 11/31] Display validation errors to user --- app/controllers/report_controller.rb | 3 ++- app/models/report.rb | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb index 001ad7916..097c92989 100644 --- a/app/controllers/report_controller.rb +++ b/app/controllers/report_controller.rb @@ -27,7 +27,8 @@ class ReportController < ApplicationController end def create - if current_user.reports.create! report_params + report = current_user.reports.new(report_params) + if report.save flash.now[:notice] = I18n.t 'report.status.created' render :nothing => true, :status => 200 else diff --git a/app/models/report.rb b/app/models/report.rb index 6abac47d0..e934e216d 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -4,7 +4,7 @@ class Report < ActiveRecord::Base validates :post_type, presence: true validates :text, presence: true - validate :entry_exists, :on => :create + validate :entry_does_not_exist, :on => :create belongs_to :user belongs_to :post @@ -12,7 +12,7 @@ class Report < ActiveRecord::Base after_commit :send_report_notification, :on => :create - def entry_exists + def entry_does_not_exist if Report.where(post_id: post_id, post_type: post_type).exists?(user_id: user_id) errors[:base] << 'You cannot report the same post twice.' end From 045ced05185840cbe8e45206d7dc38e6aa32777b Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 24 Mar 2014 11:35:05 -0400 Subject: [PATCH 12/31] Joined if statements and removed duplicated code --- app/controllers/report_controller.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb index 097c92989..da44bea1a 100644 --- a/app/controllers/report_controller.rb +++ b/app/controllers/report_controller.rb @@ -14,12 +14,8 @@ class ReportController < ApplicationController end def destroy - if report = Report.where(id: params[:id]).first - if report.destroy_reported_item - flash[:notice] = I18n.t 'report.status.destroyed' - else - flash[:error] = I18n.t 'report.status.failed' - end + if (report = Report.where(id: params[:id]).first) && report.destroy_reported_item + flash[:notice] = I18n.t 'report.status.destroyed' else flash[:error] = I18n.t 'report.status.failed' end From 011db282b7f37ea50805fbadf0474728d4564c40 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 24 Mar 2014 11:36:52 -0400 Subject: [PATCH 13/31] Removed local variable in ReportMailer --- app/mailers/report_mailer.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/mailers/report_mailer.rb b/app/mailers/report_mailer.rb index 143f22d8b..5438bd9a7 100644 --- a/app/mailers/report_mailer.rb +++ b/app/mailers/report_mailer.rb @@ -2,11 +2,9 @@ class ReportMailer < ActionMailer::Base default :from => AppConfig.mail.sender_address def new_report(type, id) - report_type = I18n.t('notifier.report_email.type.' + type) resource = { - :subject => I18n.t('notifier.report_email.subject', :type => report_type), :url => report_index_url, - :type => report_type, + :type => I18n.t('notifier.report_email.type.' + type), :id => id } Role.admins.each do |role| @@ -20,7 +18,7 @@ class ReportMailer < ActionMailer::Base private def format(resource) - mail(to: resource[:email], subject: resource[:subject]) do |format| + mail(to: resource[:email], subject: I18n.t('notifier.report_email.subject', :type => resource[:type])) do |format| format.html { render 'report/report_email', :locals => { :resource => resource } } format.text { render 'report/report_email', :locals => { :resource => resource } } end From ed9cd815044c1ea3cafda5e212330ca6ad266be9 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 24 Mar 2014 12:08:01 -0400 Subject: [PATCH 14/31] Fixed put request for Report controller --- spec/controllers/report_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/report_controller_spec.rb b/spec/controllers/report_controller_spec.rb index 614dc1b45..bedb90385 100644 --- a/spec/controllers/report_controller_spec.rb +++ b/spec/controllers/report_controller_spec.rb @@ -34,14 +34,14 @@ describe ReportController do context 'report offensive post' do it 'succeeds' do - put :create, :id => @message.id, :type => 'post', :text => 'offensive content' + put :create, :report => { :post_id => @message.id, :post_type => 'post', :text => 'offensive content' } response.status.should == 200 Report.exists?(:post_id => @message.id, :post_type => 'post').should be_true end end context 'report offensive comment' do it 'succeeds' do - put :create, :id => @comment.id, :type => 'comment', :text => 'offensive content' + put :create, :report => { :post_id => @comment.id, :post_type => 'comment', :text => 'offensive content' } response.status.should == 200 Report.exists?(:post_id => @comment.id, :post_type => 'comment').should be_true end From 8b8a232b17a395c7f004a5d9df6c0e2fc83139d2 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 24 Mar 2014 13:52:10 -0400 Subject: [PATCH 15/31] Added diaspora copyright --- app/controllers/report_controller.rb | 4 ++++ spec/controllers/report_controller_spec.rb | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb index da44bea1a..0cb8c8b9b 100644 --- a/app/controllers/report_controller.rb +++ b/app/controllers/report_controller.rb @@ -1,3 +1,7 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + class ReportController < ApplicationController before_filter :authenticate_user! before_filter :redirect_unless_admin, :except => [:create] diff --git a/spec/controllers/report_controller_spec.rb b/spec/controllers/report_controller_spec.rb index bedb90385..719ee2bbb 100644 --- a/spec/controllers/report_controller_spec.rb +++ b/spec/controllers/report_controller_spec.rb @@ -1,3 +1,7 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + require 'spec_helper' describe ReportController do From 435f6594675a0f6b9e1dfc0342fcda4ade864482 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 24 Mar 2014 14:04:28 -0400 Subject: [PATCH 16/31] Added report model spec --- spec/models/report_spec.rb | 65 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 spec/models/report_spec.rb diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb new file mode 100644 index 000000000..d3ec0815f --- /dev/null +++ b/spec/models/report_spec.rb @@ -0,0 +1,65 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +require 'spec_helper' + +describe Report do + before do + #:report => { :post_id => @message.id, :post_type => 'post', :text => 'offensive content' } + @valid_post = { + :post_id => 666, + :post_type => 'post', + :text => 'offensive content' + } + @valid_comment = { + :post_id => 666, + :post_type => 'comment', + :text => 'offensive content' + } + @report_post = bob.reports.new(@valid_post) + @report_comment = bob.reports.new(@valid_comment) + end + + describe '#validation' do + it 'validates that post ID is required' do + report = bob.reports.new(:post_type => 'post', :text => 'blub') + report.save.should_not be_true + end + + it 'validates that post type is required' do + report = bob.reports.new(:post_id => 666, :text => 'blub') + report.save.should_not be_true + end + end + + describe '#insert' do + it 'post successfully' do + @report_post.save.should be_true + end + + it 'comment successfully' do + @report_comment.save.should be_true + end + end + + describe '#delete' do + it 'post' do + @report_post.destroy_reported_item.should be_true + end + + it 'comment' do + @report_comment.destroy_reported_item.should be_true + end + end + + describe '.check_database' do + it 'post' do + Report.where(:reviewed => true, :post_id => 666, :post_type => 'post').should be_true + end + + it 'comment' do + Report.where(:reviewed => true, :post_id => 666, :post_type => 'comment').should be_true + end + end +end From 1a4ab274a348f970bcb5d146b41ee08f208817e0 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 31 Mar 2014 09:27:51 -0400 Subject: [PATCH 17/31] Changed report spec * Removed ActiveRecord tests (that is handled in the controller spec) * Added Mailer tests * Added validation tests --- spec/mailers/report_spec.rb | 26 +++++++++++++++++++++++ spec/models/report_spec.rb | 41 ++++++++----------------------------- 2 files changed, 34 insertions(+), 33 deletions(-) create mode 100644 spec/mailers/report_spec.rb diff --git a/spec/mailers/report_spec.rb b/spec/mailers/report_spec.rb new file mode 100644 index 000000000..e5d1b1608 --- /dev/null +++ b/spec/mailers/report_spec.rb @@ -0,0 +1,26 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +require 'spec_helper' + +describe Report do + describe '#make_notification' do + before do + @user = bob + Role.add_admin(@user) + end + + it "should deliver successfully" do + expect { + ReportMailer.new_report('post', 666) + }.to_not raise_error + end + + it "should be added to the delivery queue" do + expect { + ReportMailer.new_report('post', 666) + }.to change(ActionMailer::Base.deliveries, :size).by(1) + end + end +end diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index d3ec0815f..061fab8e4 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -17,49 +17,24 @@ describe Report do :post_type => 'comment', :text => 'offensive content' } - @report_post = bob.reports.new(@valid_post) - @report_comment = bob.reports.new(@valid_comment) end describe '#validation' do it 'validates that post ID is required' do - report = bob.reports.new(:post_type => 'post', :text => 'blub') - report.save.should_not be_true + bob.reports.build(:post_type => 'post', :text => 'blub').should_not be_valid end it 'validates that post type is required' do - report = bob.reports.new(:post_id => 666, :text => 'blub') - report.save.should_not be_true - end - end - - describe '#insert' do - it 'post successfully' do - @report_post.save.should be_true + bob.reports.build(:post_id => 666, :text => 'blub').should_not be_valid end - it 'comment successfully' do - @report_comment.save.should be_true + it 'validates that entry does not exist' do + bob.reports.build(@valid_post).should be_valid end - end - - describe '#delete' do - it 'post' do - @report_post.destroy_reported_item.should be_true - end - - it 'comment' do - @report_comment.destroy_reported_item.should be_true - end - end - - describe '.check_database' do - it 'post' do - Report.where(:reviewed => true, :post_id => 666, :post_type => 'post').should be_true - end - - it 'comment' do - Report.where(:reviewed => true, :post_id => 666, :post_type => 'comment').should be_true + + it 'validates that entry does exist' do + bob.reports.create(@valid_post) + bob.reports.build(@valid_post).should_not be_valid end end end From 6f65ef8437e690d6414ce1f4c3b19bbd21f3c9fe Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Tue, 1 Apr 2014 06:24:48 -0400 Subject: [PATCH 18/31] Using save for report model Instead of checking the status code I am using success and error callbacks from model-save. In that case we have to return json in the controller for signaling that the request was sucessfully. --- app/assets/javascripts/app/models/report.js | 1 + app/assets/javascripts/app/views.js | 47 ++++++++++----------- app/controllers/report_controller.rb | 4 +- 3 files changed, 24 insertions(+), 28 deletions(-) diff --git a/app/assets/javascripts/app/models/report.js b/app/assets/javascripts/app/models/report.js index 394aa0ce4..d2d734221 100644 --- a/app/assets/javascripts/app/models/report.js +++ b/app/assets/javascripts/app/models/report.js @@ -1,3 +1,4 @@ app.models.Report = Backbone.Model.extend({ urlRoot: '/report', + type: 'POST' }); diff --git a/app/assets/javascripts/app/views.js b/app/assets/javascripts/app/views.js index 8f2c718d4..e0f41e553 100644 --- a/app/assets/javascripts/app/views.js +++ b/app/assets/javascripts/app/views.js @@ -84,33 +84,30 @@ app.views.Base = Backbone.View.extend({ report: function(evt) { if(evt) { evt.preventDefault(); } var msg = prompt(Diaspora.I18n.t('report.prompt'), Diaspora.I18n.t('report.prompt_default')); - if (msg == null) return; + if (msg == null) { + return; + } + var data = { + report: { + post_id: this.model.id, + post_type: $(evt.currentTarget).data("type"), + text: msg + } + }; + var report = new app.models.Report(); - var id = this.model.id; - var type = $(evt.currentTarget).data("type"); - - report.fetch({ - data: { - report: { - post_id: id, - post_type: type, - text: msg - } + report.save(data, { + success: function(model, response) { + Diaspora.page.flashMessages.render({ + success: true, + notice: Diaspora.I18n.t('report.status.created') + }); }, - type: 'POST', - statusCode: { - 200: function(xhr) { - Diaspora.page.flashMessages.render({ - success: true, - notice: Diaspora.I18n.t('report.status.created') - }); - }, - 400: function(xhr) { - Diaspora.page.flashMessages.render({ - success: false, - notice: Diaspora.I18n.t('report.status.exists') - }); - } + error: function(model, response) { + Diaspora.page.flashMessages.render({ + success: false, + notice: Diaspora.I18n.t('report.status.exists') + }); } }); }, diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb index 0cb8c8b9b..42312c88e 100644 --- a/app/controllers/report_controller.rb +++ b/app/controllers/report_controller.rb @@ -29,10 +29,8 @@ class ReportController < ApplicationController def create report = current_user.reports.new(report_params) if report.save - flash.now[:notice] = I18n.t 'report.status.created' - render :nothing => true, :status => 200 + render :json => true, :status => 200 else - flash.now[:error] = I18n.t 'report.status.failed' render :nothing => true, :status => 409 end end From f6eba7966d88cb105779e42c1fe1588448fd4d27 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Tue, 1 Apr 2014 12:38:42 -0400 Subject: [PATCH 19/31] Added destroy_reported_item to report model spec --- spec/models/report_spec.rb | 45 +++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index 061fab8e4..8000ab0ea 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -7,6 +7,7 @@ require 'spec_helper' describe Report do before do #:report => { :post_id => @message.id, :post_type => 'post', :text => 'offensive content' } + @user = bob @valid_post = { :post_id => 666, :post_type => 'post', @@ -21,20 +22,54 @@ describe Report do describe '#validation' do it 'validates that post ID is required' do - bob.reports.build(:post_type => 'post', :text => 'blub').should_not be_valid + @user.reports.build(:post_type => 'post', :text => 'blub').should_not be_valid end it 'validates that post type is required' do - bob.reports.build(:post_id => 666, :text => 'blub').should_not be_valid + @user.reports.build(:post_id => 666, :text => 'blub').should_not be_valid end it 'validates that entry does not exist' do - bob.reports.build(@valid_post).should be_valid + @user.reports.build(@valid_post).should be_valid end it 'validates that entry does exist' do - bob.reports.create(@valid_post) - bob.reports.build(@valid_post).should_not be_valid + @user.reports.create(@valid_post) + @user.reports.build(@valid_post).should_not be_valid + end + end + + describe '#destroy_reported_item' do + before do + @post = @user.reports.create(@valid_post) + @comment = @user.reports.create(@valid_comment) + end + + describe '.post' do + it 'should destroy it' do + @post.destroy_reported_item.should be_true + end + + it 'should be marked' do + expect { + @post.destroy_reported_item + }.to change{ Report.where(@valid_post).first.reviewed }.to(true).from(false) + end + end + + describe '.comment' do + it 'should destroy it' do + @comment.destroy_reported_item.should be_true + end + + xit 'nothing' do + end + + it 'should be marked' do + expect { + @comment.destroy_reported_item + }.to change{ Report.where(@valid_comment).first.reviewed }.to(true).from(false) + end end end end From e667a785edebfbe0fc354e071d32880e37957ff9 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 14 Apr 2014 04:37:18 -0400 Subject: [PATCH 20/31] Assert that valid post/comment are gone --- spec/models/report_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index 8000ab0ea..c218811b9 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -48,6 +48,7 @@ describe Report do describe '.post' do it 'should destroy it' do @post.destroy_reported_item.should be_true + expect { Post.find(@post) }.to raise_error(ActiveRecord::RecordNotFound) end it 'should be marked' do @@ -60,9 +61,7 @@ describe Report do describe '.comment' do it 'should destroy it' do @comment.destroy_reported_item.should be_true - end - - xit 'nothing' do + expect { Comment.find(@comment) }.to raise_error(ActiveRecord::RecordNotFound) end it 'should be marked' do From 9d3af93c7dcd6126f6fb6f7414323969379f1992 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Tue, 22 Apr 2014 09:02:10 -0400 Subject: [PATCH 21/31] Removed unicode from entypo css file --- app/assets/stylesheets/entypo.css.scss | 1 - app/assets/stylesheets/single-post-view.css.scss | 5 +---- .../single-post-viewer/single-post-actions_tpl.jst.hbs | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/entypo.css.scss b/app/assets/stylesheets/entypo.css.scss index 615ed4e28..a2d2a3774 100644 --- a/app/assets/stylesheets/entypo.css.scss +++ b/app/assets/stylesheets/entypo.css.scss @@ -141,7 +141,6 @@ &.globe:before { content: '\1f30e'; } /* 1f30e */ &.graduation-cap:before { content: '\1f393 '; } /* 1f393 */ &.heart-empty:before { content: '\2661'; } /* 2661 */ - &.report:before { content: '\21'; } /* 21 */ &.heart:before { content: '\2665'; } /* 2665 */ &.help:before { content: '\2753'; } /* 2753 */ &.home:before { content: '\2302'; } /* 2302 */ diff --git a/app/assets/stylesheets/single-post-view.css.scss b/app/assets/stylesheets/single-post-view.css.scss index 056863407..b47897b88 100644 --- a/app/assets/stylesheets/single-post-view.css.scss +++ b/app/assets/stylesheets/single-post-view.css.scss @@ -61,12 +61,9 @@ i.comment:hover { color: #424242; } - i.report.gray:hover { + .post_report i.gray:hover { color: $red; } - i.report.red:hover { - color: #f55f5a; - } i.heart.gray:hover { color: $red; } diff --git a/app/assets/templates/single-post-viewer/single-post-actions_tpl.jst.hbs b/app/assets/templates/single-post-viewer/single-post-actions_tpl.jst.hbs index 2d188f6ac..c70b326f6 100644 --- a/app/assets/templates/single-post-viewer/single-post-actions_tpl.jst.hbs +++ b/app/assets/templates/single-post-viewer/single-post-actions_tpl.jst.hbs @@ -24,7 +24,7 @@ {{/if}} - + !
From 8ae89a443b8599fdae13713088f54bc926199d61 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Tue, 22 Apr 2014 09:32:15 -0400 Subject: [PATCH 22/31] Replaced fake post/comment with existing one That fixed the correct validation whether a post/comment is gone after the report was marked as deleted --- spec/models/report_spec.rb | 41 +++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index c218811b9..387a77f89 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -8,13 +8,16 @@ describe Report do before do #:report => { :post_id => @message.id, :post_type => 'post', :text => 'offensive content' } @user = bob - @valid_post = { - :post_id => 666, + @bob_post = @user.post(:status_message, :text => "hello", :to => @user.aspects.first.id) + @bob_comment = @user.comment!(@bob_post, "welcome") + + @valid_post_report = { + :post_id => @bob_post.id, :post_type => 'post', :text => 'offensive content' } - @valid_comment = { - :post_id => 666, + @valid_comment_report = { + :post_id => @bob_comment.id, :post_type => 'comment', :text => 'offensive content' } @@ -30,44 +33,46 @@ describe Report do end it 'validates that entry does not exist' do - @user.reports.build(@valid_post).should be_valid + @user.reports.build(@valid_post_report).should be_valid end it 'validates that entry does exist' do - @user.reports.create(@valid_post) - @user.reports.build(@valid_post).should_not be_valid + @user.reports.create(@valid_post_report) + @user.reports.build(@valid_post_report).should_not be_valid end end describe '#destroy_reported_item' do - before do - @post = @user.reports.create(@valid_post) - @comment = @user.reports.create(@valid_comment) + before(:each) do + @post_report = @user.reports.create(@valid_post_report) + @comment_report = @user.reports.create(@valid_comment_report) end describe '.post' do it 'should destroy it' do - @post.destroy_reported_item.should be_true - expect { Post.find(@post) }.to raise_error(ActiveRecord::RecordNotFound) + expect { + @post_report.destroy_reported_item + }.to change { Post.count }.by(-1) end it 'should be marked' do expect { - @post.destroy_reported_item - }.to change{ Report.where(@valid_post).first.reviewed }.to(true).from(false) + @post_report.destroy_reported_item + }.to change { Report.where(@valid_post_report).first.reviewed }.to(true).from(false) end end describe '.comment' do it 'should destroy it' do - @comment.destroy_reported_item.should be_true - expect { Comment.find(@comment) }.to raise_error(ActiveRecord::RecordNotFound) + expect { + @comment_report.destroy_reported_item + }.to change { Comment.count }.by(-1) end it 'should be marked' do expect { - @comment.destroy_reported_item - }.to change{ Report.where(@valid_comment).first.reviewed }.to(true).from(false) + @comment_report.destroy_reported_item + }.to change { Report.where(@valid_comment_report).first.reviewed }.to(true).from(false) end end end From 218845d5b4e125ac64cc70c87e6b53f5ab09536f Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Tue, 22 Apr 2014 10:50:57 -0400 Subject: [PATCH 23/31] Changed and renamed database columns * changed user_id type to integer * renamed post_id to item_id * renamed post_type to item_type --- app/assets/javascripts/app/views.js | 4 +-- app/controllers/report_controller.rb | 2 +- app/models/report.rb | 18 ++++++------- app/views/report/index.html.haml | 9 ++++--- ...40422134050_rename_post_columns_to_item.rb | 11 ++++++++ ...22134627_change_user_id_type_to_integer.rb | 9 +++++++ .../20140422135040_drop_table_post_reports.rb | 9 +++++++ db/schema.rb | 21 ++++----------- spec/controllers/report_controller_spec.rb | 26 +++++++++---------- spec/models/report_spec.rb | 14 +++++----- 10 files changed, 71 insertions(+), 52 deletions(-) create mode 100644 db/migrate/20140422134050_rename_post_columns_to_item.rb create mode 100644 db/migrate/20140422134627_change_user_id_type_to_integer.rb create mode 100644 db/migrate/20140422135040_drop_table_post_reports.rb diff --git a/app/assets/javascripts/app/views.js b/app/assets/javascripts/app/views.js index e0f41e553..78baa19b7 100644 --- a/app/assets/javascripts/app/views.js +++ b/app/assets/javascripts/app/views.js @@ -89,8 +89,8 @@ app.views.Base = Backbone.View.extend({ } var data = { report: { - post_id: this.model.id, - post_type: $(evt.currentTarget).data("type"), + item_id: this.model.id, + item_type: $(evt.currentTarget).data("type"), text: msg } }; diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb index 42312c88e..64022d988 100644 --- a/app/controllers/report_controller.rb +++ b/app/controllers/report_controller.rb @@ -37,6 +37,6 @@ class ReportController < ApplicationController private def report_params - params.require(:report).permit(:post_id, :post_type, :text) + params.require(:report).permit(:item_id, :item_type, :text) end end diff --git a/app/models/report.rb b/app/models/report.rb index e934e216d..358704009 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -1,7 +1,7 @@ class Report < ActiveRecord::Base validates :user_id, presence: true - validates :post_id, presence: true - validates :post_type, presence: true + validates :item_id, presence: true + validates :item_type, presence: true validates :text, presence: true validate :entry_does_not_exist, :on => :create @@ -13,22 +13,22 @@ class Report < ActiveRecord::Base after_commit :send_report_notification, :on => :create def entry_does_not_exist - if Report.where(post_id: post_id, post_type: post_type).exists?(user_id: user_id) + 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.' end end def destroy_reported_item - if post_type == 'post' + if item_type == 'post' delete_post - elsif post_type == 'comment' + elsif item_type == 'comment' delete_comment end mark_as_reviewed end def delete_post - if post = Post.where(id: post_id).first + if post = Post.where(id: item_id).first if post.author.local? post.author.owner.retract(post) else @@ -38,7 +38,7 @@ class Report < ActiveRecord::Base end def delete_comment - if comment = Comment.where(id: post_id).first + if comment = Comment.where(id: item_id).first if comment.author.local? comment.author.owner.retract(comment) elsif comment.parent.author.local? @@ -50,10 +50,10 @@ class Report < ActiveRecord::Base end def mark_as_reviewed - Report.where(post_id: post_id, post_type: post_type).update_all(reviewed: true) + Report.where(item_id: item_id, item_type: item_type).update_all(reviewed: true) end def send_report_notification - Workers::Mail::ReportWorker.perform_async(self.post_type, self.post_id) + Workers::Mail::ReportWorker.perform_async(self.item_type, self.item_id) end end diff --git a/app/views/report/index.html.haml b/app/views/report/index.html.haml index b27826a28..a45b048ca 100644 --- a/app/views/report/index.html.haml +++ b/app/views/report/index.html.haml @@ -6,16 +6,17 @@ = t('report.title') %div#reports - @reports.each do |r| + - username = User.find_by_id(r.user_id).username %div.content %span - = report_content(r.post_id, r.post_type) + = report_content(r.item_id, r.item_type) %span - = raw t('report.reported_label', person: link_to(r.user_id, user_profile_path(r.user_id))) + = raw t('report.reported_label', person: link_to(username, user_profile_path(username))) %span = t('report.reason_label', text: r.text) %div.options %span - = link_to t('report.review_link'), report_path(r.id, :type => r.post_type), method: :put + = link_to t('report.review_link'), report_path(r.id, :type => r.item_type), method: :put %span - = link_to t('report.delete_link'), report_path(r.id, :type => r.post_type), method: :delete + = link_to t('report.delete_link'), report_path(r.id, :type => r.item_type), method: :delete %div.clear diff --git a/db/migrate/20140422134050_rename_post_columns_to_item.rb b/db/migrate/20140422134050_rename_post_columns_to_item.rb new file mode 100644 index 000000000..4550eacd7 --- /dev/null +++ b/db/migrate/20140422134050_rename_post_columns_to_item.rb @@ -0,0 +1,11 @@ +class RenamePostColumnsToItem < ActiveRecord::Migration + def up + rename_column :reports, :post_id, :item_id + rename_column :reports, :post_type, :item_type + end + + def down + rename_column :reports, :item_id, :post_id + rename_column :reports, :item_type, :post_type + end +end diff --git a/db/migrate/20140422134627_change_user_id_type_to_integer.rb b/db/migrate/20140422134627_change_user_id_type_to_integer.rb new file mode 100644 index 000000000..de21f2bd3 --- /dev/null +++ b/db/migrate/20140422134627_change_user_id_type_to_integer.rb @@ -0,0 +1,9 @@ +class ChangeUserIdTypeToInteger < ActiveRecord::Migration + def up + change_column :reports, :user_id, :integer + end + + def down + change_column :reports, :user_id, :string + end +end diff --git a/db/migrate/20140422135040_drop_table_post_reports.rb b/db/migrate/20140422135040_drop_table_post_reports.rb new file mode 100644 index 000000000..1d7ba7e15 --- /dev/null +++ b/db/migrate/20140422135040_drop_table_post_reports.rb @@ -0,0 +1,9 @@ +class DropTablePostReports < ActiveRecord::Migration + def up + drop_table :post_reports + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/schema.rb b/db/schema.rb index b483d9b03..2d633c2a6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20140308154022) do +ActiveRecord::Schema.define(:version => 20140422135040) do create_table "account_deletions", :force => true do |t| t.string "diaspora_handle" @@ -316,17 +316,6 @@ ActiveRecord::Schema.define(:version => 20140308154022) do add_index "polls", ["status_message_id"], :name => "index_polls_on_status_message_id" - create_table "post_reports", :force => true do |t| - t.integer "post_id", :null => false - t.string "user_id" - t.boolean "reviewed", :default => false - t.text "text" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false - end - - add_index "post_reports", ["post_id"], :name => "index_post_reports_on_post_id" - create_table "posts", :force => true do |t| t.integer "author_id", :null => false t.boolean "public", :default => false, :null => false @@ -411,16 +400,16 @@ ActiveRecord::Schema.define(:version => 20140308154022) do add_index "rails_admin_histories", ["item", "table", "month", "year"], :name => "index_rails_admin_histories" create_table "reports", :force => true do |t| - t.integer "post_id", :null => false - t.string "post_type", :null => false - t.string "user_id" + t.integer "item_id", :null => false + t.string "item_type", :null => false + t.integer "user_id" t.boolean "reviewed", :default => false t.text "text" t.datetime "created_at", :null => false t.datetime "updated_at", :null => false end - add_index "reports", ["post_id"], :name => "index_post_reports_on_post_id" + add_index "reports", ["item_id"], :name => "index_post_reports_on_post_id" create_table "roles", :force => true do |t| t.integer "person_id" diff --git a/spec/controllers/report_controller_spec.rb b/spec/controllers/report_controller_spec.rb index 719ee2bbb..9c48503c0 100644 --- a/spec/controllers/report_controller_spec.rb +++ b/spec/controllers/report_controller_spec.rb @@ -33,21 +33,21 @@ describe ReportController do describe '#create' do let(:comment_hash) { {:text =>"facebook, is that you?", - :post_id =>"#{@post.id}"} + :item_id =>"#{@post.id}"} } context 'report offensive post' do it 'succeeds' do - put :create, :report => { :post_id => @message.id, :post_type => 'post', :text => 'offensive content' } + put :create, :report => { :item_id => @message.id, :item_type => 'post', :text => 'offensive content' } response.status.should == 200 - Report.exists?(:post_id => @message.id, :post_type => 'post').should be_true + Report.exists?(:item_id => @message.id, :item_type => 'post').should be_true end end context 'report offensive comment' do it 'succeeds' do - put :create, :report => { :post_id => @comment.id, :post_type => 'comment', :text => 'offensive content' } + put :create, :report => { :item_id => @comment.id, :item_type => 'comment', :text => 'offensive content' } response.status.should == 200 - Report.exists?(:post_id => @comment.id, :post_type => 'comment').should be_true + Report.exists?(:item_id => @comment.id, :item_type => 'comment').should be_true end end end @@ -57,14 +57,14 @@ describe ReportController do it 'is behind redirect_unless_admin' do put :update, :id => @message.id, :type => 'post' response.should redirect_to stream_path - Report.where(:reviewed => false, :post_id => @message.id, :post_type => 'post').should be_true + Report.where(:reviewed => false, :item_id => @message.id, :item_type => 'post').should be_true end end context 'mark comment report as user' do it 'is behind redirect_unless_admin' do put :update, :id => @comment.id, :type => 'comment' response.should redirect_to stream_path - Report.where(:reviewed => false, :post_id => @comment.id, :post_type => 'comment').should be_true + Report.where(:reviewed => false, :item_id => @comment.id, :item_type => 'comment').should be_true end end @@ -75,7 +75,7 @@ describe ReportController do it 'succeeds' do put :update, :id => @message.id, :type => 'post' response.status.should == 302 - Report.where(:reviewed => true, :post_id => @message.id, :post_type => 'post').should be_true + Report.where(:reviewed => true, :item_id => @message.id, :item_type => 'post').should be_true end end context 'mark comment report as admin' do @@ -85,7 +85,7 @@ describe ReportController do it 'succeeds' do put :update, :id => @comment.id, :type => 'comment' response.status.should == 302 - Report.where(:reviewed => true, :post_id => @comment.id, :post_type => 'comment').should be_true + Report.where(:reviewed => true, :item_id => @comment.id, :item_type => 'comment').should be_true end end end @@ -95,14 +95,14 @@ describe ReportController do it 'is behind redirect_unless_admin' do delete :destroy, :id => @message.id, :type => 'post' response.should redirect_to stream_path - Report.where(:reviewed => false, :post_id => @message.id, :post_type => 'post').should be_true + Report.where(:reviewed => false, :item_id => @message.id, :item_type => 'post').should be_true end end context 'destroy comment as user' do it 'is behind redirect_unless_admin' do delete :destroy, :id => @comment.id, :type => 'comment' response.should redirect_to stream_path - Report.where(:reviewed => false, :post_id => @comment.id, :post_type => 'comment').should be_true + Report.where(:reviewed => false, :item_id => @comment.id, :item_type => 'comment').should be_true end end @@ -113,7 +113,7 @@ describe ReportController do it 'succeeds' do delete :destroy, :id => @message.id, :type => 'post' response.status.should == 302 - Report.where(:reviewed => true, :post_id => @message.id, :post_type => 'post').should be_true + Report.where(:reviewed => true, :item_id => @message.id, :item_type => 'post').should be_true end end context 'destroy comment as admin' do @@ -123,7 +123,7 @@ describe ReportController do it 'succeeds' do delete :destroy, :id => @comment.id, :type => 'comment' response.status.should == 302 - Report.where(:reviewed => true, :post_id => @comment.id, :post_type => 'comment').should be_true + Report.where(:reviewed => true, :item_id => @comment.id, :item_type => 'comment').should be_true end end end diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index 387a77f89..82d5d8603 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -6,30 +6,30 @@ require 'spec_helper' describe Report do before do - #:report => { :post_id => @message.id, :post_type => 'post', :text => 'offensive content' } + #:report => { :item_id => @message.id, :item_type => 'post', :text => 'offensive content' } @user = bob @bob_post = @user.post(:status_message, :text => "hello", :to => @user.aspects.first.id) @bob_comment = @user.comment!(@bob_post, "welcome") @valid_post_report = { - :post_id => @bob_post.id, - :post_type => 'post', + :item_id => @bob_post.id, + :item_type => 'post', :text => 'offensive content' } @valid_comment_report = { - :post_id => @bob_comment.id, - :post_type => 'comment', + :item_id => @bob_comment.id, + :item_type => 'comment', :text => 'offensive content' } end describe '#validation' do it 'validates that post ID is required' do - @user.reports.build(:post_type => 'post', :text => 'blub').should_not be_valid + @user.reports.build(:item_type => 'post', :text => 'blub').should_not be_valid end it 'validates that post type is required' do - @user.reports.build(:post_id => 666, :text => 'blub').should_not be_valid + @user.reports.build(:item_id => 666, :text => 'blub').should_not be_valid end it 'validates that entry does not exist' do From 23d0890bdc6c7ae456a196af402adab2e20856ac Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Tue, 22 Apr 2014 11:25:34 -0400 Subject: [PATCH 24/31] Fixed and cleaned comment template/stylesheet fixed: * comment-report-icon will not be displayed when post author is current user * if you hover a comment all report icons will be displayed --- app/assets/stylesheets/stream_element.css.scss | 6 +++--- app/assets/templates/comment_tpl.jst.hbs | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/stream_element.css.scss b/app/assets/stylesheets/stream_element.css.scss index ac7f35c66..b7cfe0708 100644 --- a/app/assets/stylesheets/stream_element.css.scss +++ b/app/assets/stylesheets/stream_element.css.scss @@ -133,17 +133,17 @@ padding-top: 10px; .controls { - .comment_delete { + .comment_delete, .comment_report { @include transition(opacity); @include opacity(0); } } &:hover { .controls { - .comment_delete { + .comment_delete, .comment_report { @include opacity(0.3); } - .comment_delete:hover { + .comment_delete:hover, .comment_report:hover { @include opacity(1); } } diff --git a/app/assets/templates/comment_tpl.jst.hbs b/app/assets/templates/comment_tpl.jst.hbs index de65d6d64..98dd06045 100644 --- a/app/assets/templates/comment_tpl.jst.hbs +++ b/app/assets/templates/comment_tpl.jst.hbs @@ -7,14 +7,15 @@
+ {{#unless authorIsCurrentUser}} + +
+ + {{/unless}} {{#if canRemove}}
- {{else}} - -
- {{/if}}
From 6ff214150304ad8e4b9a7d2cab63b176ab220c08 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Tue, 22 Apr 2014 13:36:38 -0400 Subject: [PATCH 25/31] If you're able to remove the comment you shouldn't be able to report it --- app/assets/templates/comment_tpl.jst.hbs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/assets/templates/comment_tpl.jst.hbs b/app/assets/templates/comment_tpl.jst.hbs index 98dd06045..5d5c7c1cf 100644 --- a/app/assets/templates/comment_tpl.jst.hbs +++ b/app/assets/templates/comment_tpl.jst.hbs @@ -7,15 +7,14 @@
- {{#unless authorIsCurrentUser}} - -
- - {{/unless}} {{#if canRemove}}
+ {{else}} + +
+ {{/if}}
From cfc95b01f79135d655f21d83c6cd8e2be28c5e1e Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Wed, 23 Apr 2014 06:13:17 -0400 Subject: [PATCH 26/31] Revoke drop of non-existing table --- db/migrate/20140422135040_drop_table_post_reports.rb | 9 --------- db/schema.rb | 2 +- 2 files changed, 1 insertion(+), 10 deletions(-) delete mode 100644 db/migrate/20140422135040_drop_table_post_reports.rb diff --git a/db/migrate/20140422135040_drop_table_post_reports.rb b/db/migrate/20140422135040_drop_table_post_reports.rb deleted file mode 100644 index 1d7ba7e15..000000000 --- a/db/migrate/20140422135040_drop_table_post_reports.rb +++ /dev/null @@ -1,9 +0,0 @@ -class DropTablePostReports < ActiveRecord::Migration - def up - drop_table :post_reports - end - - def down - raise ActiveRecord::IrreversibleMigration - end -end diff --git a/db/schema.rb b/db/schema.rb index 2d633c2a6..46d425636 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20140422135040) do +ActiveRecord::Schema.define(:version => 20140422134627) do create_table "account_deletions", :force => true do |t| t.string "diaspora_handle" From 693986bba033c7457f811733bf1238f89d0366fa Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Wed, 23 Apr 2014 08:44:08 -0400 Subject: [PATCH 27/31] Fixed report icon in single post view --- app/assets/stylesheets/single-post-view.css.scss | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/assets/stylesheets/single-post-view.css.scss b/app/assets/stylesheets/single-post-view.css.scss index b47897b88..cf6ffa3d6 100644 --- a/app/assets/stylesheets/single-post-view.css.scss +++ b/app/assets/stylesheets/single-post-view.css.scss @@ -207,6 +207,13 @@ &:hover { @include opacity(1); } + .comment_report { + display: inline-block; + .icons-report { + height: 14px; + width: 14px; + } + } .delete { display: inline-block; .icons-deletelabel { From e4adb7e11b61337dbb2bd7048a60f7364f0e6a48 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Wed, 23 Apr 2014 09:30:20 -0400 Subject: [PATCH 28/31] Ignore user report associations --- lib/account_deleter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/account_deleter.rb b/lib/account_deleter.rb index da2d7ba9e..314201f92 100644 --- a/lib/account_deleter.rb +++ b/lib/account_deleter.rb @@ -50,7 +50,7 @@ class AccountDeleter end def ignored_ar_user_associations - [:followed_tags, :invited_by, :contact_people, :aspect_memberships, :ignored_people, :conversation_visibilities, :conversations] + [:followed_tags, :invited_by, :contact_people, :aspect_memberships, :ignored_people, :conversation_visibilities, :conversations, :reports] end def delete_standard_user_associations From 7ef802127e4e05163315c6e4bc153a9437293898 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 28 Apr 2014 09:15:43 -0400 Subject: [PATCH 29/31] Added confirm-dialog to report-delete-button * changed button description * replaced links with buttons --- app/assets/stylesheets/report.css.scss | 6 +++--- app/views/report/index.html.haml | 9 +++++++-- config/locales/diaspora/en.yml | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/report.css.scss b/app/assets/stylesheets/report.css.scss index 61816777a..2ee1789e7 100644 --- a/app/assets/stylesheets/report.css.scss +++ b/app/assets/stylesheets/report.css.scss @@ -1,10 +1,10 @@ #reports { padding-top: 2em; - span { - display: block; - } .content { float: left; + span { + display: block; + } } .options { float: right; diff --git a/app/views/report/index.html.haml b/app/views/report/index.html.haml index a45b048ca..b5026c643 100644 --- a/app/views/report/index.html.haml +++ b/app/views/report/index.html.haml @@ -16,7 +16,12 @@ = t('report.reason_label', text: r.text) %div.options %span - = link_to t('report.review_link'), report_path(r.id, :type => r.item_type), method: :put + = button_to t('report.review_link'), report_path(r.id, :type => r.item_type), + :class => "button", + method: :put %span - = link_to t('report.delete_link'), report_path(r.id, :type => r.item_type), method: :delete + = button_to t('report.delete_link'), report_path(r.id, :type => r.item_type), + :data => { :confirm => t('report.confirm_deletion') }, + :class => "button delete", + method: :delete %div.clear diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 3401ec951..5fc467451 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -892,7 +892,8 @@ en: reported_label: "Reported by %{person}" reason_label: "Reason: %{text}" review_link: "Mark as reviewed" - delete_link: "Delete post" + delete_link: "Delete item" + confirm_deletion: "Are you sure to delete the item?" status: marked: "The report was marked as reviewed" destroyed: "The post was destroyed" From 462a7116de711777794fb41b73934593124c5ad6 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 28 Apr 2014 10:01:44 -0400 Subject: [PATCH 30/31] Fixed possible XSS; escape comment text in report helper --- app/helpers/report_helper.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/helpers/report_helper.rb b/app/helpers/report_helper.rb index 3d8ee929b..370981314 100644 --- a/app/helpers/report_helper.rb +++ b/app/helpers/report_helper.rb @@ -8,7 +8,9 @@ module ReportHelper when 'post' t('report.post_label', title: link_to(post_page_title(Post.find_by_id(id)), post_path(id))) when 'comment' - t('report.comment_label', data: comment_message(Comment.find_by_id(id))) + # comment_message is not html_safe. To prevent + # cross-site-scripting we have to escape html + t('report.comment_label', data: h(comment_message(Comment.find_by_id(id)))) end end end From 3d9fceb479028b58f5c7302d767bfa2070b6a598 Mon Sep 17 00:00:00 2001 From: Lukas Matt Date: Mon, 28 Apr 2014 10:15:25 -0400 Subject: [PATCH 31/31] DB fix to work with existing entries * added temp. default values for user_id and item_type * changed model validation for item_type --- app/models/report.rb | 3 ++- db/migrate/20140121132816_add_post_type_to_post_report.rb | 3 ++- .../20140422134627_change_user_id_type_to_integer.rb | 7 +++++-- db/schema.rb | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/app/models/report.rb b/app/models/report.rb index 358704009..6ee562293 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -1,7 +1,8 @@ class Report < ActiveRecord::Base validates :user_id, presence: true validates :item_id, presence: true - validates :item_type, presence: true + 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 diff --git a/db/migrate/20140121132816_add_post_type_to_post_report.rb b/db/migrate/20140121132816_add_post_type_to_post_report.rb index be50eca0a..4d6686eea 100644 --- a/db/migrate/20140121132816_add_post_type_to_post_report.rb +++ b/db/migrate/20140121132816_add_post_type_to_post_report.rb @@ -1,5 +1,6 @@ class AddPostTypeToPostReport < ActiveRecord::Migration def change - add_column :post_reports, :post_type, :string, :null => false, :after => :post_id + add_column :post_reports, :post_type, :string, :null => false, :after => :post_id, :default => 'post' + change_column_default :post_reports, :post_type, nil end end diff --git a/db/migrate/20140422134627_change_user_id_type_to_integer.rb b/db/migrate/20140422134627_change_user_id_type_to_integer.rb index de21f2bd3..1f4918e8b 100644 --- a/db/migrate/20140422134627_change_user_id_type_to_integer.rb +++ b/db/migrate/20140422134627_change_user_id_type_to_integer.rb @@ -1,9 +1,12 @@ class ChangeUserIdTypeToInteger < ActiveRecord::Migration def up - change_column :reports, :user_id, :integer + remove_column :reports, :user_id + add_column :reports, :user_id, :integer, :null => false, :default => 1 + change_column_default :reports, :user_id, nil end def down - change_column :reports, :user_id, :string + remove_column :reports, :user_id + add_column :reports, :user_id, :string end end diff --git a/db/schema.rb b/db/schema.rb index 46d425636..8ae07e086 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -402,11 +402,11 @@ ActiveRecord::Schema.define(:version => 20140422134627) do create_table "reports", :force => true do |t| t.integer "item_id", :null => false t.string "item_type", :null => false - t.integer "user_id" t.boolean "reviewed", :default => false t.text "text" t.datetime "created_at", :null => false t.datetime "updated_at", :null => false + t.integer "user_id", :null => false end add_index "reports", ["item_id"], :name => "index_post_reports_on_post_id"