From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 28 Feb 2018 06:26:40 +0000 (+0800)
Subject: Allow reporting of anonymous notes
X-Git-Tag: live~2981^2~50
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/80a241f7985fec6ec853e48a25823446b59b7682

Allow reporting of anonymous notes
---

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index e79a03203..dc04606f7 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -50,7 +50,7 @@ class IssuesController < ApplicationController
     @read_reports = @issue.read_reports
     @unread_reports = @issue.unread_reports
     @comments = @issue.comments
-    @related_issues = @issue.reported_user.issues.where(:assigned_role => current_user.roles.map(&:role))
+    @related_issues = @issue.reported_user.issues.where(:assigned_role => current_user.roles.map(&:role)) if @issue.reported_user
     @new_comment = IssueComment.new(:issue => @issue)
   end
 
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 83923f242..9eb6aac4c 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -5,7 +5,7 @@
 #  id               :integer          not null, primary key
 #  reportable_type  :string           not null
 #  reportable_id    :integer          not null
-#  reported_user_id :integer          not null
+#  reported_user_id :integer
 #  status           :integer
 #  assigned_role    :enum             not null
 #  resolved_at      :datetime
@@ -36,7 +36,6 @@ class Issue < ActiveRecord::Base
   has_many :comments, :class_name => "IssueComment", :dependent => :destroy
 
   validates :reportable_id, :uniqueness => { :scope => [:reportable_type] }
-  validates :reported_user_id, :presence => true
 
   ASSIGNED_ROLES = %w[administrator moderator].freeze
   validates :assigned_role, :presence => true, :inclusion => ASSIGNED_ROLES
diff --git a/app/views/browse/note.html.erb b/app/views/browse/note.html.erb
index f292ccd19..723f9d197 100644
--- a/app/views/browse/note.html.erb
+++ b/app/views/browse/note.html.erb
@@ -3,7 +3,7 @@
 <h2>
   <a class="geolink" href="<%= root_path %>"><span class="icon close"></span></a>
   <%= t "browse.note.#{@note.status}_title", :note_name => @note.id %>
-  <% if current_user && @note.author && current_user.id != @note.author.id %>
+  <% if current_user && current_user != @note.author %>
     <%= link_to new_report_url(reportable_id: @note.id, reportable_type: @note.class.name), :title => t('browse.note.report') do %>
         &nbsp;&#9872;
     <% end %>
diff --git a/app/views/issues/index.html.erb b/app/views/issues/index.html.erb
index 4b7a2de50..0edf364ec 100644
--- a/app/views/issues/index.html.erb
+++ b/app/views/issues/index.html.erb
@@ -38,7 +38,7 @@
         <td><%= l(issue.updated_at.to_datetime, :format => :friendly) %></td>
         <td><% if issue.user_updated %> <%= issue.user_updated.display_name %> <% else %> - <% end %></td>
         <td><%= link_to reportable_title(issue.reportable), issue %></td>
-        <td><%= link_to issue.reported_user.display_name , :controller => :user, :action => :view, :display_name => issue.reported_user.display_name %></td>
+        <td><%= link_to issue.reported_user.display_name, :controller => :user, :action => :view, :display_name => issue.reported_user.display_name if issue.reported_user %></td>
         <td><%= link_to t(".show_instance"), reportable_url(issue.reportable) %></td>
       </tr>
     <% end %>
diff --git a/app/views/issues/show.html.erb b/app/views/issues/show.html.erb
index b69203c43..732fd2ef2 100644
--- a/app/views/issues/show.html.erb
+++ b/app/views/issues/show.html.erb
@@ -40,20 +40,22 @@
     <br/>
   </div>
 
-  <div class="related-block">
-    <h3><%= t ".other_issues_against_this_user" %></h3>
-    <div class="unread-reports">
-      <% if @related_issues.count > 1 %>
-        <% @related_issues.each do |issue| %>
-          <% if issue.id != @issue.id %>
-            <%= link_to reportable_title(issue.reportable), issue %> <br/>
+  <% if @issue.reported_user %>
+    <div class="related-block">
+      <h3><%= t ".other_issues_against_this_user" %></h3>
+      <div class="unread-reports">
+        <% if @related_issues.count > 1 %>
+          <% @related_issues.each do |issue| %>
+            <% if issue.id != @issue.id %>
+              <%= link_to reportable_title(issue.reportable), issue %> <br/>
+            <% end %>
           <% end %>
+        <% else %>
+          <p><%= t ".no_other_issues" %></p>
         <% end %>
-      <% else %>
-        <p><%= t ".no_other_issues" %></p>
-      <% end %>
+      </div>
     </div>
-  </div>
+  <% end %>
 </div>
 
 <h3><%= t ".comments_on_this_issue" %></h3>
diff --git a/db/migrate/20160822153055_create_issues_and_reports.rb b/db/migrate/20160822153055_create_issues_and_reports.rb
index 4361c9528..ce2374c31 100644
--- a/db/migrate/20160822153055_create_issues_and_reports.rb
+++ b/db/migrate/20160822153055_create_issues_and_reports.rb
@@ -3,7 +3,7 @@ class CreateIssuesAndReports < ActiveRecord::Migration[5.0]
     create_table :issues do |t|
       t.string :reportable_type, :null => false
       t.integer :reportable_id, :null => false
-      t.integer :reported_user_id, :null => false
+      t.integer :reported_user_id
       t.integer :status
       t.column :assigned_role, :user_role_enum, :null => false
       t.datetime :resolved_at
diff --git a/db/structure.sql b/db/structure.sql
index c7613fca4..eac2acaaf 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -726,7 +726,7 @@ CREATE TABLE issues (
     id integer NOT NULL,
     reportable_type character varying NOT NULL,
     reportable_id integer NOT NULL,
-    reported_user_id integer NOT NULL,
+    reported_user_id integer,
     status integer,
     assigned_role user_role_enum NOT NULL,
     resolved_at timestamp without time zone,
diff --git a/test/system/issues_test.rb b/test/system/issues_test.rb
index efe523634..f11b05a2d 100644
--- a/test/system/issues_test.rb
+++ b/test/system/issues_test.rb
@@ -1,6 +1,8 @@
 require "application_system_test_case"
 
 class IssuesTest < ApplicationSystemTestCase
+  include IssuesHelper
+
   def test_view_issues_normal_user
     sign_in_as(create(:user))
 
@@ -23,6 +25,18 @@ class IssuesTest < ApplicationSystemTestCase
     assert page.has_content?(issues.first.reported_user.display_name)
   end
 
+  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)
+
+    visit issues_path
+    assert page.has_content?(reportable_title(anonymous_note))
+
+    visit issue_path(issue)
+    assert page.has_content?(reportable_title(anonymous_note))
+  end
+
   def test_search_issues_by_user
     good_user = create(:user)
     bad_user = create(:user)
diff --git a/test/system/report_anonymous_note_test.rb b/test/system/report_anonymous_note_test.rb
new file mode 100644
index 000000000..a5a15d3a0
--- /dev/null
+++ b/test/system/report_anonymous_note_test.rb
@@ -0,0 +1,30 @@
+require "application_system_test_case"
+
+class ReportAnonymousNoteTest < ApplicationSystemTestCase
+  def test_no_flag_when_not_logged_in
+    note = create(:note_with_comments)
+    visit browse_note_path(note)
+    assert page.has_content?(note.comments.first.body)
+
+    assert !page.has_content?("\u2690")
+  end
+
+  def test_can_report_anonymous_notes
+    note = create(:note_with_comments)
+    sign_in_as(create(:user))
+    visit browse_note_path(note)
+
+    click_on "\u2690"
+    assert page.has_content? "Report"
+    assert page.has_content? I18n.t("issues.new.disclaimer.intro")
+
+    choose I18n.t("reports.categories.Note.spam")
+    fill_in "report_details", :with => "This is spam"
+    click_on "Create Report"
+
+    assert page.has_content? "Your report has been registered sucessfully"
+
+    assert_equal 1, Issue.count
+    assert Issue.last.reportable == note
+  end
+end