From: Andy Allan Date: Wed, 12 Jul 2017 12:36:48 +0000 (+0100) Subject: Set the reported_user in a callback X-Git-Tag: live~3501^2~112 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/506c0b5f0db8aca79ef61fa45cad508b7167817a 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 ffce14774..9b59a8fb1 100644 --- 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 40e0bb82a..472c860c6 100644 --- 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 b1401d5ac..46d68c5d2 100644 --- a/app/views/browse/changeset.html.erb +++ b/app/views/browse/changeset.html.erb @@ -4,7 +4,7 @@ <%= 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 %>  ⚐ <% end %> <% end %> diff --git a/app/views/browse/note.html.erb b/app/views/browse/note.html.erb index a3b5f0c54..0884e3bdf 100644 --- a/app/views/browse/note.html.erb +++ b/app/views/browse/note.html.erb @@ -4,7 +4,7 @@ <%= 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 %>  ⚐ <% end %> <% end %> diff --git a/app/views/diary_entry/_diary_comment.html.erb b/app/views/diary_entry/_diary_comment.html.erb index e3375fd8b..9ea035ba5 100644 --- 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 %>

<%= 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 %>  ⚐ <% end %> <% end %> diff --git a/app/views/diary_entry/_diary_entry.html.erb b/app/views/diary_entry/_diary_entry.html.erb index 9ed87dbbd..e3bb20e36 100644 --- a/app/views/diary_entry/_diary_entry.html.erb +++ b/app/views/diary_entry/_diary_entry.html.erb @@ -7,7 +7,7 @@

<%= link_to h(diary_entry.title), :action => 'view', :display_name => diary_entry.user.display_name, :id => diary_entry.id %>

<% 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 %>  ⚐ <% end %> <% end %> diff --git a/app/views/issues/new.html.erb b/app/views/issues/new.html.erb index dc2ee044e..4a3389959 100644 --- a/app/views/issues/new.html.erb +++ b/app/views/issues/new.html.erb @@ -17,7 +17,6 @@
<%= f.hidden_field :reportable_id %> <%= f.hidden_field :reportable_type %> - <%= f.hidden_field :reported_user_id %>
diff --git a/app/views/user/view.html.erb b/app/views/user/view.html.erb index 417b07bd0..a23f15e55 100644 --- 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 %>
- <%= 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%>  ⚐ <% end %>
diff --git a/test/models/issue_test.rb b/test/models/issue_test.rb index 7e15ee90c..7ee700124 100644 --- 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