Just pass the user object, rather than the display_name, to the user_path helper
authorAndy Allan <git@gravitystorm.co.uk>
Mon, 10 Sep 2018 02:54:29 +0000 (10:54 +0800)
committerAndy Allan <git@gravitystorm.co.uk>
Mon, 10 Sep 2018 02:54:29 +0000 (10:54 +0800)
app/helpers/changeset_helper.rb
app/views/issues/_comments.html.erb
app/views/issues/_reports.html.erb
app/views/issues/index.html.erb
app/views/layouts/_header.html.erb
test/controllers/reports_controller_test.rb
test/controllers/user_controller_test.rb
test/controllers/user_roles_controller_test.rb
test/system/report_user_test.rb

index 539e6c3..bc19bc8 100644 (file)
@@ -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
index 4b5ae4a..5cb99f2 100644 (file)
@@ -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">
index b499288..427cbe9 100644 (file)
@@ -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)) %>
index e5e7ebc..bfb11ee 100644 (file)
         <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 %>
index a3e1846..cd3dfd4 100644 (file)
@@ -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 %>
index 5bf355d..612ead2 100644 (file)
@@ -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
 
index ba4b2ff..55b2c21 100644 (file)
@@ -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
index f9e3214..705f06a 100644 (file)
@@ -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
index 3dfbf4f..9d91952 100644 (file)
@@ -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"