Set the reported_user in a callback

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.

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index ffce14774b65fe2151db1055849669e386d23441..9b59a8fb1891c0a7ff61b5c34e2d1817234b4356 100644 (file)
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -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

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 40e0bb82aa9eb92a9bcc0f5ffcd3a2d1fe721b6b..472c860c68b29ccc476256d5053a1026e83e46e2 100644 (file)
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -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

diff --git a/app/views/browse/changeset.html.erb b/app/views/browse/changeset.html.erb
index b1401d5ac8aee14fdae4a96f7010d22e7afcb4c2..46d68c5d2d663ff896d8eb72fee59dc294818df0 100644 (file)
--- a/app/views/browse/changeset.html.erb
+++ b/app/views/browse/changeset.html.erb
@@ -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 %>

diff --git a/app/views/browse/note.html.erb b/app/views/browse/note.html.erb
index a3b5f0c544d8dd72de78be5d93ad3890128171d0..0884e3bdfc420690ce728d71ce24f0a77de3a41d 100644 (file)
--- a/app/views/browse/note.html.erb
+++ b/app/views/browse/note.html.erb
@@ -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 %>

diff --git a/app/views/diary_entry/_diary_comment.html.erb b/app/views/diary_entry/_diary_comment.html.erb
index e3375fd8b515db22ea20260a8d9d66ad9774ff5f..9ea035ba5ea8a9cd1a64618ef85f40d3934761ee 100644 (file)
--- a/app/views/diary_entry/_diary_comment.html.erb
+++ b/app/views/diary_entry/_diary_comment.html.erb
@@ -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 %>

diff --git a/app/views/diary_entry/_diary_entry.html.erb b/app/views/diary_entry/_diary_entry.html.erb
index 9ed87dbbd728b64521e05822f47004e5bd41ee76..e3bb20e36372b277f39612b4c678ea76cf06b8f9 100644 (file)
--- a/app/views/diary_entry/_diary_entry.html.erb
+++ b/app/views/diary_entry/_diary_entry.html.erb
@@ -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 %>

diff --git a/app/views/issues/new.html.erb b/app/views/issues/new.html.erb
index dc2ee044e53ab85f9b42baa2f1d8af055f70cb3a..4a33899599115f5bbf4058d9f214ce0282e8b7e8 100644 (file)
--- a/app/views/issues/new.html.erb
+++ b/app/views/issues/new.html.erb
@@ -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'>

diff --git a/app/views/user/view.html.erb b/app/views/user/view.html.erb
index 417b07bd0a618e1b8365600cf69b90cb0d1944d1..a23f15e55a2a4590a81e122e01749b7722b3e054 100644 (file)
--- a/app/views/user/view.html.erb
+++ b/app/views/user/view.html.erb
@@ -156,7 +156,7 @@
 
     <% 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>

diff --git a/test/models/issue_test.rb b/test/models/issue_test.rb
index 7e15ee90cf51075e5ed3012c5b74fc1cd334b201..7ee700124c7448a84a6bc02fa08a59f66069a6cd 100644 (file)
--- a/test/models/issue_test.rb
+++ b/test/models/issue_test.rb
@@ -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