user name changes: remove unused reason, status fields.

Remove all infrastructure around approving or rejecting user name
changes. Name changes haven't been moderated for several years.

* Remove status, approver_id, change_reason, and rejection_reason fields.
* Remove approve and reject controller actions.

app/controllers/user_name_change_requests_controller.rb

@@ -1,7 +1,6 @@
 class UserNameChangeRequestsController < ApplicationController
   before_action :member_only, :only => [:index, :show]
   before_action :gold_only, :only => [:new, :create]
-  before_action :admin_only, :only => [:approve, :reject]
   respond_to :html, :json, :xml
 
   def new
@@ -10,14 +9,9 @@ class UserNameChangeRequestsController < ApplicationController
   end
 
   def create
-    @change_request = UserNameChangeRequest.create(change_request_params)
+    @change_request = UserNameChangeRequest.create_with(user: CurrentUser.user, original_name: CurrentUser.name).create(change_request_params)
-    
+    flash[:notice] = "Your name has been changed"
-    if @change_request.errors.any?
+    respond_with(@change_request, location: profile_path)
-      render :action => "new"
-    else
-      @change_request.approve! 
-      redirect_to user_name_change_request_path(@change_request), :notice => "Your name has been changed"
-    end
   end
 
   def show
@@ -31,18 +25,6 @@ class UserNameChangeRequestsController < ApplicationController
     respond_with(@change_requests)
   end
 
-  def approve
-    @change_request = UserNameChangeRequest.find(params[:id])
-    @change_request.approve!
-    redirect_to user_name_change_request_path(@change_request), :notice => "Name change request approved"
-  end
-  
-  def reject
-    @change_request = UserNameChangeRequest.find(params[:id])
-    @change_request.reject!(params[:reason])
-    redirect_to user_name_change_request_path(@change_request), :notice => "Name change request rejected"
-  end
-
   private
 
   def check_privileges!(change_request)
@@ -51,6 +33,6 @@ class UserNameChangeRequestsController < ApplicationController
   end
 
   def change_request_params
-    params.fetch(:user_name_change_request, {}).permit(%i[desired_name change_reason])
+    params.fetch(:user_name_change_request, {}).permit(%i[desired_name])
   end
 end

app/models/user_name_change_request.rb

@@ -1,53 +1,24 @@
 class UserNameChangeRequest < ApplicationRecord
-  after_initialize :initialize_attributes, if: :new_record?
-  validates_presence_of :original_name, :desired_name
-  validates_inclusion_of :status, :in => %w(pending approved rejected)
   belongs_to :user
-  belongs_to :approver, :class_name => "User", optional: true
+  belongs_to :approver, class_name: "User", optional: true
-  validate :not_limited, :on => :create
+
+  validate :not_limited, on: :create
   validates :desired_name, user_name: true
+  validates_presence_of :original_name, :desired_name
 
-  def initialize_attributes
+  after_create :approve!
-    self.user_id ||= CurrentUser.user.id
-    self.original_name ||= CurrentUser.user.name
-  end
-  
-  def self.pending
-    where(:status => "pending")
-  end
-
-  def self.approved
-    where(:status => "approved")
-  end
 
   def self.visible(viewer = CurrentUser.user)
     if viewer.is_admin?
       all
     elsif viewer.is_member?
-      joins(:user).merge(User.undeleted).where("user_name_change_requests.status = 'approved' OR user_name_change_requests.user_id = ?", viewer.id)
+      joins(:user).merge(User.undeleted).where("user_name_change_requests.user_id = ?", viewer.id)
     else
       none
     end
   end
 
-  def rejected?
-    status == "rejected"
-  end
-  
-  def approved?
-    status == "approved"
-  end
-
-  def pending?
-    status == "pending"
-  end
-  
-  def feedback
-    user.feedback.order("id desc")
-  end
-  
   def approve!
-    update(status: "approved", approver_id: CurrentUser.user.id)
     user.update_attribute(:name, desired_name)
     body = "Your name change request has been approved. Be sure to log in with your new user name."
     Dmail.create_automated(:title => "Name change request approved", :body => body, :to_id => user_id)
@@ -55,21 +26,9 @@ class UserNameChangeRequest < ApplicationRecord
     ModAction.log("Name changed from #{original_name} to #{desired_name}",:user_name_change)
   end
 
-  def reject!(reason)
-    update(status: "rejected", rejection_reason: reason)
-    body = "Your name change request has been rejected for the following reason: #{rejection_reason}"
-    Dmail.create_automated(:title => "Name change request rejected", :body => body, :to_id => user_id)
-  end
-  
   def not_limited
-    if UserNameChangeRequest.where("user_id = ? and created_at >= ?", CurrentUser.user.id, 1.week.ago).exists?
+    if UserNameChangeRequest.unscoped.where(user: user).where("created_at >= ?", 1.week.ago).exists?
-      errors.add(:base, "You can only submit one name change request per week")
+      errors[:base] << "You can only submit one name change request per week"
     end
   end
-
-  def api_attributes
-    attributes = super
-    attributes -= [:change_reason, :rejection_reason] unless CurrentUser.is_admin? || user == CurrentUser.user
-    attributes
-  end
 end

app/views/user_name_change_requests/index.html.erb

@@ -6,9 +6,8 @@
       <thead>
         <tr>
           <th>User</th>
-          <th>Request</th>
+          <th>Old Name</th>
-          <th>Reason</th>
+          <th>New Name</th>
-          <th>Status</th>
           <th>Date</th>
           <th></th>
         </tr>
@@ -17,23 +16,8 @@
         <% @change_requests.each do |change_request| %>
           <tr>
             <td><%= link_to_user change_request.user %></td>
-            <td>
+            <td><%= change_request.original_name %></td>
-              <strong><%= change_request.original_name %></strong> ->
+            <td><%= change_request.desired_name %></td>
-              <strong><%= change_request.desired_name %></strong>
-            </td>
-            <td>
-              <% if CurrentUser.is_admin? || CurrentUser.user == change_request.user %>
-                <%= change_request.change_reason %>
-              <% end %>
-            </td>
-            <td>
-              <%= change_request.status %>
-              <% if change_request.approved? %>
-                by <%= link_to_user change_request.approver %>
-              <% elsif change_request.rejected? %>
-                for reason: <%= link_to change_request.rejection_reason %>
-              <% end %>
-            </td>
             <td><%= compact_time change_request.created_at %></td>
             <td><%= link_to "view", user_name_change_request_path(change_request) %></td>
           </tr>

app/views/user_name_change_requests/show.html.erb

@@ -22,66 +22,8 @@
             <strong><%= @change_request.desired_name %></strong>
           </td>
         </tr>
-        <tr>
-          <th>Reason</th>
-          <td><%= @change_request.change_reason %></td>
-        </tr>
-        <tr>
-          <th>Status</th>
-          <td>
-            <%= @change_request.status %>
-            <% if @change_request.approved? %>
-              by <%= link_to_user @change_request.approver %>
-            <% elsif @change_request.rejected? %>
-              for reason: <%= link_to @change_request.rejection_reason %>
-            <% end %>
-          </td>
-        </tr>
       </tbody>
     </table>
-
-    <% if @change_request.pending? && CurrentUser.user.is_admin? %>
-      <section>
-        <h2>Feedback</h2>
-        <ul>
-          <% @change_request.feedback.each do |feedback| %>
-            <li class="feedback-category-<%= feedback.category %>">
-              <p><%= feedback.body %></p>
-              <p class="fineprint">Submitted by <%= link_to_user feedback.creator.name %> <%= time_ago_in_words_tagged feedback.created_at %></p>
-            </li>
-          <% end %>
-        </ul>
-      </section>
-
-      <section>
-        <h2>Statistics</h2>
-        <ul>
-          <li>Level: <%= @change_request.user.level_string %></li>
-          <li>Uploads: <%= link_to @change_request.user.post_upload_count, posts_path("user:#{@change_request.user.name}") %></li>
-          <li>Updates: <%= link_to @change_request.user.post_update_count, post_versions_path(:search => {:updater_id => @change_request.user.id}) %></li>
-          <li>Notes: <%= link_to @change_request.user.note_update_count, note_versions_path(:search => {:updater_id => @change_request.user.id}) %></li>
-          <li>Favorites: <%= @change_request.user.favorite_count %></li>
-        </ul>
-      </section>
-
-      <section>
-        <h2>Options</h2>
-        <%= form_tag(approve_user_name_change_request_path(@change_request)) do %>
-          <%= submit_tag "Approve" %>
-        <% end %>
-
-        <%= form_tag(reject_user_name_change_request_path(@change_request), :class => "simple_form") do %>
-          <div class="input">
-            <label>Reason</label>
-            <%= text_field_tag "reason" %>
-          </div>
-
-          <div class="input">
-            <%= submit_tag "Reject" %>
-          </div>
-        <% end %>
-      </section>
-    <% end %>
   </div>
 </div>
 

config/routes.rb

@@ -300,12 +300,7 @@ Rails.application.routes.draw do
       get :search
     end
   end
-  resources :user_name_change_requests do
+  resources :user_name_change_requests, only: [:new, :create, :show, :index]
-    member do
-      post :approve
-      post :reject
-    end
-  end
   resource :user_revert, :only => [:new, :create]
   resources :wiki_pages do
     member do

db/migrate/20190926000912_remove_reason_and_status_from_user_name_change_requests.rb

@@ -0,0 +1,8 @@
+class RemoveReasonAndStatusFromUserNameChangeRequests < ActiveRecord::Migration[6.0]
+  def change
+    remove_column :user_name_change_requests, :change_reason, :text
+    remove_column :user_name_change_requests, :rejection_reason, :text
+    remove_column :user_name_change_requests, :approver_id, :integer
+    remove_column :user_name_change_requests, :status, :string, null: false, default: "pending"
+  end
+end

db/structure.sql

@@ -3010,13 +3010,9 @@ ALTER SEQUENCE public.user_feedback_id_seq OWNED BY public.user_feedback.id;
 
 CREATE TABLE public.user_name_change_requests (
     id integer NOT NULL,
-    status character varying DEFAULT 'pending'::character varying NOT NULL,
     user_id integer NOT NULL,
-    approver_id integer,
     original_name character varying,
     desired_name character varying,
-    change_reason text,
-    rejection_reason text,
     created_at timestamp without time zone,
     updated_at timestamp without time zone
 );
@@ -7340,6 +7336,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20190908031103'),
 ('20190908035317'),
 ('20190919175836'),
-('20190923071044');
+('20190923071044'),
+('20190926000912');
 
 

test/factories/user_name_change_request.rb

@@ -1,6 +1,7 @@
 FactoryBot.define do
   factory(:user_name_change_request) do
+    user
+    original_name {FFaker::Internet.user_name}
     desired_name {FFaker::Internet.user_name}
-    change_reason {FFaker::Lorem.sentence}
   end
 end

test/functional/user_name_change_requests_controller_test.rb

@@ -5,14 +5,6 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest
     setup do
       @user = create(:gold_user)
       @admin = create(:admin_user)
-      as(@user) do
-        @change_request = UserNameChangeRequest.create!(
-          :user_id => @user.id,
-          :original_name => @user.name,
-          :desired_name => "abc",
-          :change_reason => "hello"
-        )
-      end
     end
 
     context "new action" do
@@ -25,13 +17,19 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest
     context "create action" do
       should "work" do
         post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { desired_name: "zun" }}
-        assert_response :success
+
+        assert_response :redirect
+        assert_equal("zun", @user.reload.name)
       end
     end
 
     context "show action" do
+      setup do
+        @change_request = as(@user) { create(:user_name_change_request, user_id: @user.id) }
+      end
+
       should "render" do
-        get_auth user_name_change_request_path(@change_request), @user
+        get_auth user_name_change_request_path(@change_request), @admin
         assert_response :success
       end
 
@@ -51,20 +49,6 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest
           assert_response :success
         end
       end
-      
-      context "approve action" do
-        should "succeed" do
-          post_auth approve_user_name_change_request_path(@change_request), @admin
-          assert_redirected_to(user_name_change_request_path(@change_request))
-        end
-      end
-      
-      context "reject action" do
-        should "succeed" do
-          post_auth reject_user_name_change_request_path(@change_request), @admin
-          assert_redirected_to(user_name_change_request_path(@change_request))
-        end
-      end
     end
   end
 end

test/unit/user_name_change_request_test.rb

@@ -19,7 +19,6 @@ class UserNameChangeRequestTest < ActiveSupport::TestCase
         @change_request = UserNameChangeRequest.create(
           :user_id => @requester.id,
           :original_name => @requester.name,
-          :status => "pending",
           :desired_name => "abc"
         )
         CurrentUser.user = @admin
@@ -50,37 +49,12 @@ class UserNameChangeRequestTest < ActiveSupport::TestCase
       end
     end
     
-    context "rejecting a request" do
-      setup do
-        @change_request = UserNameChangeRequest.create(
-          :user_id => @requester.id,
-          :original_name => @requester.name,
-          :status => "pending",
-          :desired_name => "abc"
-        )
-        CurrentUser.user = @admin
-      end
-      
-      should "create a dmail" do
-        assert_difference("Dmail.count", 1) do
-          @change_request.reject!("msg")
-        end
-      end
-      
-      should "preserve the username" do
-        @change_request.reject!("msg")
-        @requester.reload
-        assert_not_equal("abc", @requester.name)
-      end
-    end
-    
     context "creating a new request" do
       should "not validate if the desired name already exists" do
         assert_difference("UserNameChangeRequest.count", 0) do
           req = UserNameChangeRequest.create(
             :user_id => @requester.id,
             :original_name => @requester.name,
-            :status => "pending",
             :desired_name => @requester.name
           )
           assert_equal(["Desired name already exists"], req.errors.full_messages)