From: Tom Hughes <tom@compton.nu>
Date: Sun, 17 Jun 2018 10:14:19 +0000 (+0100)
Subject: Assign vandalism reports for users to moderators
X-Git-Tag: live~2937
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/e5604ce98e267cb0cec7dd27078a9e86c919eeb1?hp=6c225bd01c53ad4cc3429e05c7bb71da3b0a4b7b

Assign vandalism reports for users to moderators
---

diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb
index 08092c073..ef87a8699 100644
--- a/app/controllers/reports_controller.rb
+++ b/app/controllers/reports_controller.rb
@@ -16,11 +16,15 @@ class ReportsController < ApplicationController
 
   def create
     @report = current_user.reports.new(report_params)
-    @report.issue = Issue.find_or_initialize_by(:reportable_id => params[:report][:issue][:reportable_id], :reportable_type => params[:report][:issue][:reportable_type])
+    @report.issue = Issue
+                    .create_with(:assigned_role => default_assigned_role)
+                    .find_or_initialize_by(issue_params)
 
     if @report.save
-      @report.issue.save
-      @report.issue.reopen! unless @report.issue.open?
+      @report.issue.assigned_role = "administrator" if default_assigned_role == "administrator"
+      @report.issue.reopen unless @report.issue.open?
+      @report.issue.save!
+
       redirect_to helpers.reportable_url(@report.issue.reportable), :notice => t(".successful_report")
     else
       redirect_to new_report_path(:reportable_type => @report.issue.reportable_type, :reportable_id => @report.issue.reportable_id), :notice => t(".provide_details")
@@ -38,6 +42,21 @@ class ReportsController < ApplicationController
   end
 
   def report_params
-    params[:report].permit(:details, :category)
+    params.require(:report).permit(:details, :category)
+  end
+
+  def issue_params
+    params.require(:report).require(:issue).permit(:reportable_id, :reportable_type)
+  end
+
+  def default_assigned_role
+    case issue_params[:reportable_type]
+    when "Note" then "moderator"
+    when "User" then case report_params[:category]
+                     when "vandal" then "moderator"
+                     else "administrator"
+                     end
+    else "administrator"
+    end
   end
 end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index d893a6e3a..f18581b5f 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -44,7 +44,6 @@ class Issue < ActiveRecord::Base
   ASSIGNED_ROLES = %w[administrator moderator].freeze
   validates :assigned_role, :presence => true, :inclusion => ASSIGNED_ROLES
 
-  before_validation :set_default_assigned_role
   before_validation :set_reported_user
 
   scope :with_status, ->(issue_status) { where(:status => statuses[issue_status]) }
@@ -93,13 +92,4 @@ class Issue < ActiveRecord::Base
                            reportable.user
                          end
   end
-
-  def set_default_assigned_role
-    if assigned_role.blank?
-      self.assigned_role = case reportable
-                           when Note then "moderator"
-                           else "administrator"
-                           end
-    end
-  end
 end
diff --git a/app/views/reports/new.html.erb b/app/views/reports/new.html.erb
index 98d135890..fb338b497 100644
--- a/app/views/reports/new.html.erb
+++ b/app/views/reports/new.html.erb
@@ -32,7 +32,7 @@
     </div>
 
     <div class='form-row'>
-      <%= text_area :report, :details, :cols => 20, :rows => 5, placeholder: t('.details'), :required => true %>
+      <%= text_area :report, :details, :cols => 20, :rows => 5, placeholder: t('.details') %>
     </div>
 
     <div class='buttons'>
diff --git a/test/factories/issues.rb b/test/factories/issues.rb
index 913c47f2a..e209bd25d 100644
--- a/test/factories/issues.rb
+++ b/test/factories/issues.rb
@@ -3,5 +3,8 @@ FactoryBot.define do
     # Default to reporting users
     association :reportable, :factory => :user
     association :reported_user, :factory => :user
+
+    # Default to assigning to an administrator
+    assigned_role "administrator"
   end
 end
diff --git a/test/models/issue_test.rb b/test/models/issue_test.rb
index 4203d02c1..073dc2644 100644
--- a/test/models/issue_test.rb
+++ b/test/models/issue_test.rb
@@ -17,72 +17,24 @@ class IssueTest < ActiveSupport::TestCase
     diary_entry = create(:diary_entry)
     diary_comment = create(:diary_comment, :diary_entry => diary_entry)
 
-    issue = Issue.new(:reportable => user)
+    issue = Issue.new(:reportable => user, :assigned_role => "administrator")
     issue.save!
     assert_equal issue.reported_user, user
 
-    issue = Issue.new(:reportable => note)
+    issue = Issue.new(:reportable => note, :assigned_role => "administrator")
     issue.save!
     assert_equal issue.reported_user, note.author
 
-    issue = Issue.new(:reportable => anonymous_note)
+    issue = Issue.new(:reportable => anonymous_note, :assigned_role => "administrator")
     issue.save!
     assert_nil issue.reported_user
 
-    issue = Issue.new(:reportable => diary_entry)
+    issue = Issue.new(:reportable => diary_entry, :assigned_role => "administrator")
     issue.save!
     assert_equal issue.reported_user, diary_entry.user
 
-    issue = Issue.new(:reportable => diary_comment)
+    issue = Issue.new(:reportable => diary_comment, :assigned_role => "administrator")
     issue.save!
     assert_equal issue.reported_user, diary_comment.user
   end
-
-  def test_default_assigned_role
-    create(:language, :code => "en")
-    user = create(:user)
-    note = create(:note_with_comments)
-    diary_entry = create(:diary_entry)
-    diary_comment = create(:diary_comment, :diary_entry => diary_entry)
-
-    issue = Issue.new(:reportable => user)
-    issue.save!
-    assert_equal "administrator", issue.assigned_role
-
-    issue = Issue.new(:reportable => note)
-    issue.save!
-    assert_equal "moderator", issue.assigned_role
-
-    issue = Issue.new(:reportable => diary_entry)
-    issue.save!
-    assert_equal "administrator", issue.assigned_role
-
-    issue = Issue.new(:reportable => diary_comment)
-    issue.save!
-    assert_equal "administrator", issue.assigned_role
-  end
-
-  def test_no_default_explicit_role
-    create(:language, :code => "en")
-    user = create(:user)
-    note = create(:note_with_comments)
-    diary_entry = create(:diary_entry)
-    diary_comment = create(:diary_comment, :diary_entry => diary_entry)
-
-    issue = Issue.new(:reportable => user, :assigned_role => "moderator")
-    issue.save!
-    assert_equal "moderator", issue.reload.assigned_role
-
-    issue = Issue.new(:reportable => note, :assigned_role => "administrator")
-    issue.save!
-    assert_equal "administrator", issue.reload.assigned_role
-
-    issue = Issue.new(:reportable => diary_entry, :assigned_role => "moderator")
-    issue.save!
-    assert_equal "moderator", issue.reload.assigned_role
-
-    issue = Issue.new(:reportable => diary_comment, :assigned_role => "moderator")
-    issue.save!
-    assert_equal "moderator", issue.reload.assigned_role
-  end
 end
diff --git a/test/system/issues_test.rb b/test/system/issues_test.rb
index 3cecf58d9..9aee867a9 100644
--- a/test/system/issues_test.rb
+++ b/test/system/issues_test.rb
@@ -33,7 +33,7 @@ class IssuesTest < ApplicationSystemTestCase
   def test_view_issues_with_no_reported_user
     sign_in_as(create(:moderator_user))
     anonymous_note = create(:note_with_comments)
-    issue = create(:issue, :reportable => anonymous_note)
+    issue = create(:issue, :reportable => anonymous_note, :assigned_role => "moderator")
 
     visit issues_path
     assert page.has_content?(reportable_title(anonymous_note))
diff --git a/test/system/report_diary_comment_test.rb b/test/system/report_diary_comment_test.rb
index 5b66bcee7..18be7e649 100644
--- a/test/system/report_diary_comment_test.rb
+++ b/test/system/report_diary_comment_test.rb
@@ -32,5 +32,6 @@ class ReportDiaryCommentTest < ApplicationSystemTestCase
     assert page.has_content? "Your report has been registered sucessfully"
 
     assert_equal @comment, Issue.last.reportable
+    assert_equal "administrator", Issue.last.assigned_role
   end
 end
diff --git a/test/system/report_diary_entry_test.rb b/test/system/report_diary_entry_test.rb
index 1aac8ba70..1db733ab7 100644
--- a/test/system/report_diary_entry_test.rb
+++ b/test/system/report_diary_entry_test.rb
@@ -31,6 +31,7 @@ class ReportDiaryEntryTest < ApplicationSystemTestCase
     assert page.has_content? "Your report has been registered sucessfully"
 
     assert_equal @diary_entry, Issue.last.reportable
+    assert_equal "administrator", Issue.last.assigned_role
   end
 
   def test_it_reopens_issue
diff --git a/test/system/report_note_test.rb b/test/system/report_note_test.rb
index 24deac2cb..93e73cf5a 100644
--- a/test/system/report_note_test.rb
+++ b/test/system/report_note_test.rb
@@ -27,6 +27,7 @@ class ReportNoteTest < ApplicationSystemTestCase
     assert page.has_content? "Your report has been registered sucessfully"
 
     assert_equal note, Issue.last.reportable
+    assert_equal "moderator", Issue.last.assigned_role
   end
 
   def test_can_report_notes_with_author
@@ -47,5 +48,6 @@ class ReportNoteTest < ApplicationSystemTestCase
     assert page.has_content? "Your report has been registered sucessfully"
 
     assert_equal note, Issue.last.reportable
+    assert_equal "moderator", Issue.last.assigned_role
   end
 end
diff --git a/test/system/report_user_test.rb b/test/system/report_user_test.rb
index 8a30c80e0..99533353b 100644
--- a/test/system/report_user_test.rb
+++ b/test/system/report_user_test.rb
@@ -27,5 +27,44 @@ class ReportUserTest < ApplicationSystemTestCase
     assert page.has_content? "Your report has been registered sucessfully"
 
     assert_equal user, Issue.last.reportable
+    assert_equal "moderator", Issue.last.assigned_role
+  end
+
+  def test_it_promotes_issues
+    user = create(:user)
+    sign_in_as(create(:user))
+    visit user_path(user.display_name)
+
+    click_on I18n.t("user.view.report")
+    assert page.has_content? "Report"
+    assert page.has_content? I18n.t("reports.new.disclaimer.intro")
+
+    choose I18n.t("reports.new.categories.user.vandal_label")
+    fill_in "report_details", :with => "This user is a vandal"
+    assert_difference "Issue.count", 1 do
+      click_on "Create Report"
+    end
+
+    assert page.has_content? "Your report has been registered sucessfully"
+
+    assert_equal user, Issue.last.reportable
+    assert_equal "moderator", Issue.last.assigned_role
+
+    visit user_path(user.display_name)
+
+    click_on I18n.t("user.view.report")
+    assert page.has_content? "Report"
+    assert page.has_content? I18n.t("reports.new.disclaimer.intro")
+
+    choose I18n.t("reports.new.categories.user.spam_label")
+    fill_in "report_details", :with => "This user is a spammer"
+    assert_no_difference "Issue.count" do
+      click_on "Create Report"
+    end
+
+    assert page.has_content? "Your report has been registered sucessfully"
+
+    assert_equal user, Issue.last.reportable
+    assert_equal "administrator", Issue.last.assigned_role
   end
 end