From: Andy Allan <git@gravitystorm.co.uk>
Date: Mon, 10 Sep 2018 02:54:29 +0000 (+0800)
Subject: Just pass the user object, rather than the display_name, to the user_path helper
X-Git-Tag: live~2857^2~1
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/4dd4831c0aedee97b69fd0c942d559cd0d1f7ed9

Just pass the user object, rather than the display_name, to the user_path helper
---

diff --git a/app/helpers/changeset_helper.rb b/app/helpers/changeset_helper.rb
index 539e6c309..bc19bc857 100644
--- a/app/helpers/changeset_helper.rb
+++ b/app/helpers/changeset_helper.rb
@@ -3,7 +3,7 @@ module ChangesetHelper
     if changeset.user.status == "deleted"
       t("user.no_such_user.deleted")
     elsif changeset.user.data_public?
-      link_to(changeset.user.display_name, user_path(changeset.user.display_name))
+      link_to(changeset.user.display_name, user_path(changeset.user))
     else
       t("browse.anonymous")
     end
diff --git a/app/views/issues/_comments.html.erb b/app/views/issues/_comments.html.erb
index 4b5ae4a8f..5cb99f2f1 100644
--- a/app/views/issues/_comments.html.erb
+++ b/app/views/issues/_comments.html.erb
@@ -2,9 +2,9 @@
   <% comments.each do |comment| %>
     <div class="comment">
       <div style="float:left">
-        <%= link_to user_thumbnail(comment.user), user_path(comment.user.display_name) %>
+        <%= link_to user_thumbnail(comment.user), user_path(comment.user) %>
       </div>
-      <b> <%= link_to comment.user.display_name, user_path(comment.user.display_name) %> </b> <br/>
+      <b> <%= link_to comment.user.display_name, user_path(comment.user) %> </b> <br/>
       <%= comment.body %>
     </div>
     <span class="deemphasize">
diff --git a/app/views/issues/_reports.html.erb b/app/views/issues/_reports.html.erb
index b499288a2..427cbe909 100644
--- a/app/views/issues/_reports.html.erb
+++ b/app/views/issues/_reports.html.erb
@@ -1,9 +1,9 @@
 <% reports.each do |report| %>
   <div class="report">
     <div style="float:left">
-      <%= link_to user_thumbnail(report.user), user_path(report.user.display_name) %>
+      <%= link_to user_thumbnail(report.user), user_path(report.user) %>
     </div>
-    <%= t ".reported_by_html", :category => report.category, :user => link_to(report.user.display_name, user_path(report.user.display_name)) %>
+    <%= t ".reported_by_html", :category => report.category, :user => link_to(report.user.display_name, user_path(report.user)) %>
     <br/>
     <span class="deemphasize">
       <%= t(".updated_at", :datetime => l(report.updated_at.to_datetime, :format => :friendly)) %>
diff --git a/app/views/issues/index.html.erb b/app/views/issues/index.html.erb
index e5e7ebc00..bfb11ee5b 100644
--- a/app/views/issues/index.html.erb
+++ b/app/views/issues/index.html.erb
@@ -34,10 +34,10 @@
         <td><%= t ".states.#{issue.status}" %></td>
         <td><%= link_to t(".reports_count", :count => issue.reports_count), issue %></td>
         <td><%= link_to reportable_title(issue.reportable), reportable_url(issue.reportable) %></td>
-        <td><%= link_to issue.reported_user.display_name, user_path(issue.reported_user.display_name) if issue.reported_user %></td>
+        <td><%= link_to issue.reported_user.display_name, user_path(issue.reported_user) if issue.reported_user %></td>
         <td>
           <% if issue.user_updated %>
-            <%= t ".last_updated_time_user_html", :user => link_to(issue.user_updated.display_name, user_path(issue.user_updated.display_name)),
+            <%= t ".last_updated_time_user_html", :user => link_to(issue.user_updated.display_name, user_path(issue.user_updated)),
                                                   :time => distance_of_time_in_words_to_now(issue.updated_at),
                                                   :title => l(issue.updated_at) %>
           <% else %>
diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb
index a3e1846a9..cd3dfd468 100644
--- a/app/views/layouts/_header.html.erb
+++ b/app/views/layouts/_header.html.erb
@@ -86,7 +86,7 @@
             <% end %>
           </li>
           <li>
-            <%= link_to t('user.show.my profile'), user_path(:display_name => current_user.display_name) %>
+            <%= link_to t('user.show.my profile'), user_path(current_user) %>
           </li>
           <li>
             <%= link_to t('user.show.my settings'), :controller => 'user', :action => 'account', :display_name => current_user.display_name %>
diff --git a/test/controllers/reports_controller_test.rb b/test/controllers/reports_controller_test.rb
index 5bf355d08..612ead2e6 100644
--- a/test/controllers/reports_controller_test.rb
+++ b/test/controllers/reports_controller_test.rb
@@ -29,7 +29,7 @@ class ReportsControllerTest < ActionController::TestCase
            }
     end
     assert_response :redirect
-    assert_redirected_to user_path(target_user.display_name)
+    assert_redirected_to user_path(target_user)
   end
 
   def test_new_report_with_incomplete_details
@@ -55,7 +55,7 @@ class ReportsControllerTest < ActionController::TestCase
            }
     end
     assert_response :redirect
-    assert_redirected_to user_path(target_user.display_name)
+    assert_redirected_to user_path(target_user)
 
     issue = Issue.last
 
@@ -103,7 +103,7 @@ class ReportsControllerTest < ActionController::TestCase
            }
     end
     assert_response :redirect
-    assert_redirected_to user_path(target_user.display_name)
+    assert_redirected_to user_path(target_user)
 
     issue = Issue.last
 
diff --git a/test/controllers/user_controller_test.rb b/test/controllers/user_controller_test.rb
index ba4b2ff3a..55b2c2114 100644
--- a/test/controllers/user_controller_test.rb
+++ b/test/controllers/user_controller_test.rb
@@ -1250,7 +1250,7 @@ class UserControllerTest < ActionController::TestCase
     assert_difference "ActionMailer::Base.deliveries.size", 1 do
       post :make_friend, :params => { :display_name => friend.display_name }, :session => { :user => user }
     end
-    assert_redirected_to user_path(:display_name => friend.display_name)
+    assert_redirected_to user_path(friend)
     assert_match /is now your friend/, flash[:notice]
     assert Friend.where(:user_id => user.id, :friend_user_id => friend.id).first
     email = ActionMailer::Base.deliveries.first
@@ -1262,7 +1262,7 @@ class UserControllerTest < ActionController::TestCase
     assert_no_difference "ActionMailer::Base.deliveries.size" do
       post :make_friend, :params => { :display_name => friend.display_name }, :session => { :user => user }
     end
-    assert_redirected_to user_path(:display_name => friend.display_name)
+    assert_redirected_to user_path(friend)
     assert_match /You are already friends with/, flash[:warning]
     assert Friend.where(:user_id => user.id, :friend_user_id => friend.id).first
   end
@@ -1335,13 +1335,13 @@ class UserControllerTest < ActionController::TestCase
 
     # When logged in a POST should remove the friendship
     post :remove_friend, :params => { :display_name => friend.display_name }, :session => { :user => user }
-    assert_redirected_to user_path(:display_name => friend.display_name)
+    assert_redirected_to user_path(friend)
     assert_match /was removed from your friends/, flash[:notice]
     assert_nil Friend.where(:user_id => user.id, :friend_user_id => friend.id).first
 
     # A second POST should report that the friendship does not exist
     post :remove_friend, :params => { :display_name => friend.display_name }, :session => { :user => user }
-    assert_redirected_to user_path(:display_name => friend.display_name)
+    assert_redirected_to user_path(friend)
     assert_match /is not one of your friends/, flash[:error]
     assert_nil Friend.where(:user_id => user.id, :friend_user_id => friend.id).first
   end
diff --git a/test/controllers/user_roles_controller_test.rb b/test/controllers/user_roles_controller_test.rb
index f9e321406..705f06a1c 100644
--- a/test/controllers/user_roles_controller_test.rb
+++ b/test/controllers/user_roles_controller_test.rb
@@ -31,7 +31,7 @@ class UserRolesControllerTest < ActionController::TestCase
 
     # Granting should still fail
     post :grant, :params => { :display_name => target_user.display_name, :role => "moderator" }
-    assert_redirected_to user_path(target_user.display_name)
+    assert_redirected_to user_path(target_user)
     assert_equal "Only administrators can perform user role management, and you are not an administrator.", flash[:error]
 
     # Login as an administrator
@@ -50,20 +50,20 @@ class UserRolesControllerTest < ActionController::TestCase
       assert_no_difference "UserRole.count" do
         post :grant, :params => { :display_name => super_user.display_name, :role => role }
       end
-      assert_redirected_to user_path(super_user.display_name)
+      assert_redirected_to user_path(super_user)
       assert_equal "The user already has role #{role}.", flash[:error]
 
       # Granting a role to a user that doesn't have it should work...
       assert_difference "UserRole.count", 1 do
         post :grant, :params => { :display_name => target_user.display_name, :role => role }
       end
-      assert_redirected_to user_path(target_user.display_name)
+      assert_redirected_to user_path(target_user)
 
       # ...but trying a second time should fail
       assert_no_difference "UserRole.count" do
         post :grant, :params => { :display_name => target_user.display_name, :role => role }
       end
-      assert_redirected_to user_path(target_user.display_name)
+      assert_redirected_to user_path(target_user)
       assert_equal "The user already has role #{role}.", flash[:error]
     end
 
@@ -71,7 +71,7 @@ class UserRolesControllerTest < ActionController::TestCase
     assert_difference "UserRole.count", 0 do
       post :grant, :params => { :display_name => target_user.display_name, :role => "no_such_role" }
     end
-    assert_redirected_to user_path(target_user.display_name)
+    assert_redirected_to user_path(target_user)
     assert_equal "The string `no_such_role' is not a valid role.", flash[:error]
   end
 
@@ -92,7 +92,7 @@ class UserRolesControllerTest < ActionController::TestCase
 
     # Revoking should still fail
     post :revoke, :params => { :display_name => target_user.display_name, :role => "moderator" }
-    assert_redirected_to user_path(target_user.display_name)
+    assert_redirected_to user_path(target_user)
     assert_equal "Only administrators can perform user role management, and you are not an administrator.", flash[:error]
 
     # Login as an administrator
@@ -111,20 +111,20 @@ class UserRolesControllerTest < ActionController::TestCase
       assert_no_difference "UserRole.count" do
         post :revoke, :params => { :display_name => target_user.display_name, :role => role }
       end
-      assert_redirected_to user_path(target_user.display_name)
+      assert_redirected_to user_path(target_user)
       assert_equal "The user does not have role #{role}.", flash[:error]
 
       # Removing a role from a user that has it should work...
       assert_difference "UserRole.count", -1 do
         post :revoke, :params => { :display_name => super_user.display_name, :role => role }
       end
-      assert_redirected_to user_path(super_user.display_name)
+      assert_redirected_to user_path(super_user)
 
       # ...but trying a second time should fail
       assert_no_difference "UserRole.count" do
         post :revoke, :params => { :display_name => super_user.display_name, :role => role }
       end
-      assert_redirected_to user_path(super_user.display_name)
+      assert_redirected_to user_path(super_user)
       assert_equal "The user does not have role #{role}.", flash[:error]
     end
 
@@ -132,12 +132,12 @@ class UserRolesControllerTest < ActionController::TestCase
     assert_difference "UserRole.count", 0 do
       post :revoke, :params => { :display_name => target_user.display_name, :role => "no_such_role" }
     end
-    assert_redirected_to user_path(target_user.display_name)
+    assert_redirected_to user_path(target_user)
     assert_equal "The string `no_such_role' is not a valid role.", flash[:error]
 
     # Revoking administrator role from current user should fail
     post :revoke, :params => { :display_name => administrator_user.display_name, :role => "administrator" }
-    assert_redirected_to user_path(administrator_user.display_name)
+    assert_redirected_to user_path(administrator_user)
     assert_equal "Cannot revoke administrator role from current user.", flash[:error]
   end
 end
diff --git a/test/system/report_user_test.rb b/test/system/report_user_test.rb
index 3dfbf4f1f..9d919523b 100644
--- a/test/system/report_user_test.rb
+++ b/test/system/report_user_test.rb
@@ -12,7 +12,7 @@ class ReportUserTest < ApplicationSystemTestCase
   def test_can_report_user
     user = create(:user)
     sign_in_as(create(:user))
-    visit user_path(user.display_name)
+    visit user_path(user)
 
     click_on I18n.t("user.show.report")
     assert page.has_content? "Report"
@@ -33,7 +33,7 @@ class ReportUserTest < ApplicationSystemTestCase
   def test_it_promotes_issues
     user = create(:user)
     sign_in_as(create(:user))
-    visit user_path(user.display_name)
+    visit user_path(user)
 
     click_on I18n.t("user.show.report")
     assert page.has_content? "Report"
@@ -50,7 +50,7 @@ class ReportUserTest < ApplicationSystemTestCase
     assert_equal user, Issue.last.reportable
     assert_equal "moderator", Issue.last.assigned_role
 
-    visit user_path(user.display_name)
+    visit user_path(user)
 
     click_on I18n.t("user.show.report")
     assert page.has_content? "Report"