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

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