From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 6 Dec 2017 16:28:13 +0000 (+0000)
Subject: Rename 'issue_type' to 'assigned_role', set defaults in the model and test.
X-Git-Tag: live~3501^2~73
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/0df788d74232330eea5c2d4854ba0d10796053eb

Rename 'issue_type' to 'assigned_role', set defaults in the model and test.
---

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index edf8f1121..253587965 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -17,7 +17,7 @@ class IssuesController < ApplicationController
       @users = User.joins(:roles).where(:user_roles => { :role => "administrator" })
     end
 
-    @issues = Issue.where(:issue_type => @user_role)
+    @issues = Issue.where(:assigned_role => @user_role)
 
     # If search
     if params[:search_by_user] && params[:search_by_user].present?
@@ -55,7 +55,7 @@ class IssuesController < ApplicationController
     @read_reports = @issue.read_reports
     @unread_reports = @issue.unread_reports
     @comments = @issue.comments
-    @related_issues = @issue.reported_user.issues.where(:issue_type => @user_role)
+    @related_issues = @issue.reported_user.issues.where(:assigned_role => @user_role)
     @new_comment = IssueComment.new(:issue => @issue)
   end
 
@@ -125,7 +125,7 @@ class IssuesController < ApplicationController
 
   # Reassign Issues between Administrators and Moderators
   def reassign_issue
-    @issue.issue_type = upgrade_issue(@issue.issue_type)
+    @issue.assigned_role = upgrade_issue(@issue.assigned_role)
     @issue.save!
   end
 
diff --git a/app/models/issue.rb b/app/models/issue.rb
index a08bfd6a4..7294b07f5 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -7,7 +7,7 @@
 #  reportable_id    :integer          not null
 #  reported_user_id :integer          not null
 #  status           :integer
-#  issue_type       :string
+#  assigned_role    :enum             not null
 #  resolved_at      :datetime
 #  resolved_by      :integer
 #  updated_by       :integer
@@ -38,6 +38,7 @@ class Issue < ActiveRecord::Base
   validates :reportable_id, :uniqueness => { :scope => [:reportable_type] }
   validates :reported_user_id, :presence => true
 
+  before_validation :set_default_assigned_role
   before_validation :set_reported_user
 
   # Check if more statuses are needed
@@ -89,4 +90,9 @@ class Issue < ActiveRecord::Base
                            reportable.user
                          end
   end
+
+  def set_default_assigned_role
+    role = %w[Changeset Note].include?(reportable.class.name) ? "moderator" : "administrator"
+    self.assigned_role = role if assigned_role.blank?
+  end
 end
diff --git a/db/migrate/20160822153055_create_issues_and_reports.rb b/db/migrate/20160822153055_create_issues_and_reports.rb
index cdd3ea7c7..a9ba96554 100644
--- a/db/migrate/20160822153055_create_issues_and_reports.rb
+++ b/db/migrate/20160822153055_create_issues_and_reports.rb
@@ -5,7 +5,7 @@ class CreateIssuesAndReports < ActiveRecord::Migration[5.0]
       t.integer :reportable_id, :null => false
       t.integer :reported_user_id, :null => false
       t.integer :status
-      t.string :issue_type
+      t.column :assigned_role, :user_role_enum, :null => false
       t.datetime :resolved_at
       t.integer :resolved_by
       t.integer :updated_by
diff --git a/db/structure.sql b/db/structure.sql
index 742791df5..f01cbfbbc 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -728,7 +728,7 @@ CREATE TABLE issues (
     reportable_id integer NOT NULL,
     reported_user_id integer NOT NULL,
     status integer,
-    issue_type character varying,
+    assigned_role user_role_enum NOT NULL,
     resolved_at timestamp without time zone,
     resolved_by integer,
     updated_by integer,
@@ -2877,3 +2877,5 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('7'),
 ('8'),
 ('9');
+
+
diff --git a/test/models/issue_test.rb b/test/models/issue_test.rb
index 7ee700124..49649eede 100644
--- a/test/models/issue_test.rb
+++ b/test/models/issue_test.rb
@@ -21,4 +21,26 @@ class IssueTest < ActiveSupport::TestCase
     issue.save!
     assert_equal issue.reported_user, diary_entry.user
   end
+
+  def test_default_assigned_role
+    create(:language, :code => "en")
+    diary_entry = create(:diary_entry)
+    changeset = create(:changeset)
+
+    issue = Issue.new
+    issue.reportable = diary_entry
+    issue.save!
+    assert_equal "administrator", issue.assigned_role
+
+    issue = Issue.new
+    issue.reportable = changeset
+    issue.save!
+    assert_equal "moderator", issue.assigned_role
+
+    # check the callback doesn't override an explicitly set role
+    issue.assigned_role = "administrator"
+    issue.save!
+    issue.reload
+    assert_equal "administrator", issue.assigned_role
+  end
 end
diff --git a/test/system/issues_test.rb b/test/system/issues_test.rb
index 9eb2fbbcb..ab3e619c9 100644
--- a/test/system/issues_test.rb
+++ b/test/system/issues_test.rb
@@ -17,7 +17,7 @@ class IssuesTest < ApplicationSystemTestCase
 
   def test_view_issues
     sign_in_as(create(:moderator_user))
-    issues = create_list(:issue, 3, :issue_type => "moderator")
+    issues = create_list(:issue, 3, :assigned_role => "moderator")
 
     visit issues_path
     assert page.has_content?(issues.first.reported_user.display_name)
@@ -26,7 +26,7 @@ class IssuesTest < ApplicationSystemTestCase
   def test_search_issues_by_user
     good_user = create(:user)
     bad_user = create(:user)
-    create(:issue, :reportable => bad_user, :reported_user => bad_user, :issue_type => "administrator")
+    create(:issue, :reportable => bad_user, :reported_user => bad_user, :assigned_role => "administrator")
 
     sign_in_as(create(:administrator_user))