Set the reported_user in a callback
authorAndy Allan <git@gravitystorm.co.uk>
Wed, 12 Jul 2017 12:36:48 +0000 (13:36 +0100)
committerAndy Allan <git@gravitystorm.co.uk>
Wed, 12 Jul 2017 12:36:48 +0000 (13:36 +0100)
This avoids passing around the reported_user via forms. There was no
validation anywhere that the reported_user corresponded to the object
being reported. This approach removes those worries too.

app/controllers/issues_controller.rb
app/models/issue.rb
app/views/browse/changeset.html.erb
app/views/browse/note.html.erb
app/views/diary_entry/_diary_comment.html.erb
app/views/diary_entry/_diary_entry.html.erb
app/views/issues/new.html.erb
app/views/user/view.html.erb
test/models/issue_test.rb

index ffce147..9b59a8f 100644 (file)
@@ -108,7 +108,7 @@ class IssuesController < ApplicationController
         redirect_back "/", :notice => t("issues.create.successful_report")
       end
     else
-      redirect_to new_issue_path(:reportable_type => @issue.reportable_type, :reportable_id => @issue.reportable_id, :reported_user_id => @issue.reported_user_id), :notice => t("issues.create.provide_details")
+      redirect_to new_issue_path(:reportable_type => @issue.reportable_type, :reportable_id => @issue.reportable_id), :notice => t("issues.create.provide_details")
     end
   end
 
@@ -142,7 +142,7 @@ class IssuesController < ApplicationController
         redirect_back "/", :notice => notice
       end
     else
-      redirect_to new_issue_path(:reportable_type => @issue.reportable_type, :reportable_id => @issue.reportable_id, :reported_user_id => @issue.reported_user_id), :notice => t("issues.update.provide_details")
+      redirect_to new_issue_path(:reportable_type => @issue.reportable_type, :reportable_id => @issue.reportable_id), :notice => t("issues.update.provide_details")
     end
   end
 
@@ -249,11 +249,11 @@ class IssuesController < ApplicationController
   end
 
   def create_new_issue_params
-    params.permit(:reportable_id, :reportable_type, :reported_user_id)
+    params.permit(:reportable_id, :reportable_type)
   end
 
   def issue_params
-    params[:issue].permit(:reportable_id, :reportable_type, :reported_user_id)
+    params[:issue].permit(:reportable_id, :reportable_type)
   end
 
   def report_params
index 40e0bb8..472c860 100644 (file)
@@ -9,6 +9,8 @@ class Issue < ActiveRecord::Base
   validates :reportable_id, :uniqueness => { :scope => [:reportable_type] }
   validates :reported_user_id, :presence => true
 
+  before_validation :set_reported_user
+
   # Check if more statuses are needed
   enum :status => %w[open ignored resolved]
   enum :type => %w[administrator moderator]
@@ -45,4 +47,17 @@ class Issue < ActiveRecord::Base
       transitions :from => :ignored, :to => :open
     end
   end
+
+  private
+
+  def set_reported_user
+    self.reported_user = case reportable.class.name
+                         when "User"
+                           reportable
+                         when "Note"
+                           reportable.author
+                         else
+                           reportable.user
+                         end
+  end
 end
index b1401d5..46d68c5 100644 (file)
@@ -4,7 +4,7 @@
   <a class="geolink" href="<%= root_path %>"><span class="icon close"></span></a>
   <%= t('browse.changeset.title', :id => @changeset.id) %>
   <% if @user and @user.id != @changeset.user.id %>
-    <%= link_to new_issue_url(reportable_id: @changeset.id, reportable_type: @changeset.class.name, reported_user_id: @changeset.user.id,referer: request.fullpath), :title => t('browse.changeset.report') do %>
+    <%= link_to new_issue_url(reportable_id: @changeset.id, reportable_type: @changeset.class.name, referer: request.fullpath), :title => t('browse.changeset.report') do %>
         &nbsp;&#9872;
     <% end %>
   <% end %>
index a3b5f0c..0884e3b 100644 (file)
@@ -4,7 +4,7 @@
   <a class="geolink" href="<%= root_path %>"><span class="icon close"></span></a>
   <%= t "browse.note.#{@note.status}_title", :note_name => @note.id %>
   <% if @user && @note.author && @user.id != @note.author.id %>
-    <%= link_to new_issue_url(reportable_id: @note.id, reportable_type: @note.class.name, reported_user_id: @note.author.id,referer: request.fullpath), :title => t('browse.note.report') do %>
+    <%= link_to new_issue_url(reportable_id: @note.id, reportable_type: @note.class.name, referer: request.fullpath), :title => t('browse.note.report') do %>
         &nbsp;&#9872;
     <% end %>
   <% end %>
index e3375fd..9ea035b 100644 (file)
@@ -2,7 +2,7 @@
   <%= user_thumbnail diary_comment.user %>
   <p class="deemphasize comment-heading" id="comment<%= diary_comment.id %>"><%= raw(t('diary_entry.diary_comment.comment_from', :link_user => (link_to h(diary_comment.user.display_name), :controller => 'user', :action => 'view', :display_name => diary_comment.user.display_name), :comment_created_at => link_to(l(diary_comment.created_at, :format => :friendly), :anchor => "comment#{diary_comment.id}"))) %>
     <% if @user and diary_comment.user.id != @user.id %>
-      <%= link_to new_issue_url(reportable_id: diary_comment.id, reportable_type: diary_comment.class.name, reported_user_id: diary_comment.user.id, referer: request.fullpath), :title => t('diary_entry.diary_comment.report') do %>
+      <%= link_to new_issue_url(reportable_id: diary_comment.id, reportable_type: diary_comment.class.name, referer: request.fullpath), :title => t('diary_entry.diary_comment.report') do %>
               &nbsp;&#9872;
       <% end %>
     <% end %>
index 9ed87db..e3bb20e 100644 (file)
@@ -7,7 +7,7 @@
     <h2><%= link_to h(diary_entry.title), :action => 'view', :display_name => diary_entry.user.display_name, :id => diary_entry.id %></h2>
 
     <% if @user and diary_entry.user.id != @user.id %>
-           <%= link_to new_issue_url(reportable_id: diary_entry.id, reportable_type: diary_entry.class.name, reported_user_id: diary_entry.user.id,referer: request.fullpath), :title => t('diary_entry.diary_entry.report') do %>
+           <%= link_to new_issue_url(reportable_id: diary_entry.id, reportable_type: diary_entry.class.name, referer: request.fullpath), :title => t('diary_entry.diary_entry.report') do %>
             &nbsp;&#9872;
           <% end %>
     <% end %>
index dc2ee04..4a33899 100644 (file)
@@ -17,7 +17,6 @@
     <div class='form-row'>
       <%= f.hidden_field :reportable_id %>
       <%= f.hidden_field :reportable_type %>
-      <%= f.hidden_field :reported_user_id %>
     </div>
 
     <div class='form-row' style='width:600px'>
index 417b07b..a23f15e 100644 (file)
 
     <% if @user and @this_user.id != @user.id %>
       <div class="report-button">
-         <%= link_to new_issue_url(reportable_id: @this_user.id, reportable_type: @this_user.class.name, reported_user_id: @this_user.id,referer: request.fullpath), :title => t('user.view.report') do%>
+         <%= link_to new_issue_url(reportable_id: @this_user.id, reportable_type: @this_user.class.name, referer: request.fullpath), :title => t('user.view.report') do%>
             &nbsp;&#9872;
          <% end %>
       </div>
index 7e15ee9..7ee7001 100644 (file)
@@ -1,7 +1,24 @@
 require "test_helper"
 
 class IssueTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  def test_reported_user
+    note = create(:note_comment, :author => create(:user)).note
+    user = create(:user)
+    create(:language, :code => "en")
+    diary_entry = create(:diary_entry)
+    issue = Issue.new
+
+    issue.reportable = user
+    issue.save!
+    assert_equal issue.reported_user, user
+
+    # FIXME: doesn't handle anonymous notes
+    issue.reportable = note
+    issue.save!
+    assert_equal issue.reported_user, note.author
+
+    issue.reportable = diary_entry
+    issue.save!
+    assert_equal issue.reported_user, diary_entry.user
+  end
 end