pundit: convert user name change requests to pundit.

Fix discrepancy between index action and show action. The index
action allowed members to see name changes for undeleted users, but the
show action didn't.

app/controllers/user_name_change_requests_controller.rb

@@ -1,37 +1,25 @@
 class UserNameChangeRequestsController < ApplicationController
-  before_action :member_only, :only => [:index, :show, :new, :create]
   respond_to :html, :json, :xml
 
   def new
-    @change_request = UserNameChangeRequest.new(change_request_params)
+    @change_request = authorize UserNameChangeRequest.new(permitted_attributes(UserNameChangeRequest))
     respond_with(@change_request)
   end
 
   def create
-    @change_request = UserNameChangeRequest.create_with(user: CurrentUser.user, original_name: CurrentUser.name).create(change_request_params)
+    @change_request = authorize UserNameChangeRequest.new(user: CurrentUser.user, original_name: CurrentUser.name)
+    @change_request.update(permitted_attributes(@change_request))
     flash[:notice] = "Your name has been changed" if @change_request.valid?
     respond_with(@change_request, location: profile_path)
   end
 
   def show
-    @change_request = UserNameChangeRequest.find(params[:id])
-    check_privileges!(@change_request)
+    @change_request = authorize UserNameChangeRequest.find(params[:id])
     respond_with(@change_request)
   end
 
   def index
-    @change_requests = UserNameChangeRequest.visible(CurrentUser.user).order("id desc").paginate(params[:page], :limit => params[:limit])
+    @change_requests = authorize UserNameChangeRequest.visible(CurrentUser.user).order("id desc").paginate(params[:page], :limit => params[:limit])
     respond_with(@change_requests)
   end
-
-  private
-
-  def check_privileges!(change_request)
-    return if CurrentUser.is_admin?
-    raise User::PrivilegeError if change_request.user_id != CurrentUser.user.id
-  end
-
-  def change_request_params
-    params.fetch(:user_name_change_request, {}).permit(%i[desired_name desired_name_confirmation])
-  end
 end

app/models/user.rb

@@ -322,6 +322,10 @@ class User < ApplicationRecord
       User.level_string(value || level)
     end
 
+    def is_deleted?
+      name.match?(/\Auser_[0-9]+~*\z/)
+    end
+
     def is_anonymous?
       level == Levels::ANONYMOUS
     end

app/policies/user_name_change_request_policy.rb

@@ -0,0 +1,13 @@
+class UserNameChangeRequestPolicy < ApplicationPolicy
+  def index?
+    user.is_member?
+  end
+
+  def show?
+    user.is_admin? || (user.is_member? && !record.user.is_deleted?) || (record.user == user)
+  end
+
+  def permitted_attributes
+    [:desired_name, :desired_name_confirmation]
+  end
+end

app/views/static/site_map.html.erb

@@ -121,7 +121,7 @@
         <% else %>
           <li><%= link_to "Profile", profile_path %></li>
           <li><%= link_to "Settings", settings_path %></li>
-          <% if CurrentUser.is_gold? %>
+          <% if policy(UserNameChangeRequest).create? %>
             <li><%= link_to "Change name", new_user_name_change_request_path %></li>
           <% end %>
           <li><%= link_to "Delete account", maintenance_user_deletion_path %></li>
@@ -150,7 +150,7 @@
         <li><%= link_to("Jobs", delayed_jobs_path) %></li>
         <li><%= link_to("Bulk Update Requests", bulk_update_requests_path) %></li>
 
-        <% if CurrentUser.is_member? %>
+        <% if policy(UserNameChangeRequest).index? %>
           <li><%= link_to("User Name Change Requests", user_name_change_requests_path) %></li>
         <% end %>
 

test/functional/user_name_change_requests_controller_test.rb

@@ -26,6 +26,7 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest
     context "show action" do
       setup do
         @change_request = as(@user) { create(:user_name_change_request, user_id: @user.id) }
+        @user.update!(name: "user_#{@user.id}")
       end
 
       should "render" do
@@ -33,7 +34,7 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest
         assert_response :success
       end
 
-      context "when the current user is not an admin and does not own the request" do
+      context "when the current user is not an admin, doesn't own the request, and the other user is deleted" do
         should "fail" do
           @another_user = create(:user)
           get_auth user_name_change_request_path(@change_request), @another_user
@@ -42,15 +43,20 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest
       end
     end
 
-    context "for actions restricted to admins" do
-      context "index action" do
-        should "render" do
-          post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { desired_name: "zun", desired_name_confirmation: "zun" }}
-          get user_name_change_requests_path
+    context "index action" do
+      should "allows members to see name changes" do
+        create(:user_name_change_request)
+        get_auth user_name_change_requests_path, @user
 
-          assert_response :success
-          assert_select "table tbody tr", 1
-        end
+        assert_response :success
+        assert_select "table tbody tr", 1
+      end
+
+      should "not allow anonymous users to see name changes" do
+        create(:user_name_change_request)
+        get user_name_change_requests_path
+
+        assert_response 403
       end
     end
   end