From 2e36f8d375cbf36837d4c4162522b9f751b8150a Mon Sep 17 00:00:00 2001
From: Lukas Matt <lukas@zauberstuhl.de>
Date: Fri, 21 Mar 2014 09:21:12 -0400
Subject: [PATCH] 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 f65d043799..8f2c718d47 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 6fa29ce075..61816777a8 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 a5dcef21d7..7e379f343f 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 e7dbb911c3..70a4b6e534 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 b47a006dcf..149177b5be 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 0261857f9c..2e9168a803 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 8da7c73bdf..b27826a289 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
--
GitLab