Merge pull request #2842 from evazion/fix-user-name-changes

Minor user name changes improvements.

app/assets/stylesheets/common/tables.css.scss

@@ -55,3 +55,11 @@ table.search {
     vertical-align: top;
   }
 }
+
+table.aligned-vertical {
+  @extend table.search;
+
+  tr {
+    height: 1.75em;
+  }
+}

app/assets/stylesheets/specific/user_name_change_requests.css.scss

@@ -28,4 +28,4 @@
   section {
     margin-bottom: 1em;
   }
-}
+}

app/controllers/user_name_change_requests_controller.rb

@@ -1,7 +1,9 @@
 class UserNameChangeRequestsController < ApplicationController
-  before_filter :gold_only, :only => [:new, :create, :show]
+  before_filter :member_only, :only => [:index, :show]
-  before_filter :admin_only, :only => [:index, :approve, :reject, :destroy]
+  before_filter :gold_only, :only => [:new, :create]
+  before_filter :admin_only, :only => [:approve, :reject]
   rescue_from User::PrivilegeError, :with => :access_denied
+  respond_to :html, :json, :xml
 
   def new
   end
@@ -26,10 +28,12 @@ class UserNameChangeRequestsController < ApplicationController
   def show
     @change_request = UserNameChangeRequest.find(params[:id])
     check_privileges!(@change_request)
+    respond_with(@change_request)
   end
   
   def index
-    @change_requests = UserNameChangeRequest.order("id desc").paginate(params[:page], :limit => params[:limit])
+    @change_requests = UserNameChangeRequest.visible.order("id desc").paginate(params[:page], :limit => params[:limit])
+    respond_with(@change_requests)
   end
   
   def approve
@@ -38,12 +42,6 @@ class UserNameChangeRequestsController < ApplicationController
     redirect_to user_name_change_request_path(@change_request), :notice => "Name change request approved"
   end
   
-  def destroy
-    @change_request = UserNameChangeRequest.find(params[:id])
-    @change_request.destroy
-    redirect_to user_name_change_requests_path
-  end
-  
   def reject
     @change_request = UserNameChangeRequest.find(params[:id])
     @change_request.reject!(params[:reason])

app/models/user_name_change_request.rb

@@ -36,6 +36,10 @@ class UserNameChangeRequest < ActiveRecord::Base
   def approved?
     status == "approved"
   end
+
+  def pending?
+    status == "pending"
+  end
   
   def normalize_name
     self.desired_name = desired_name.strip.gsub(/ /, "_")

app/views/user_name_change_requests/_secondary_links.html.erb

@@ -0,0 +1,7 @@
+<% content_for(:secondary_links) do %>
+  <menu>
+    <li><%= link_to "Listing", user_name_change_requests_path %></li>
+    <li><%= link_to "New", new_user_name_change_request_path %></li>
+    <li><%= link_to "Help", wiki_pages_path(:search => {:title => "help:user_name_change_requests"}) %></li>
+  </menu>
+<% end %>

app/views/user_name_change_requests/index.html.erb

@@ -5,15 +5,35 @@
     <thead>
       <tr>
         <th>User</th>
-        <th>Desired</th>
+        <th>Request</th>
+        <th>Reason</th>
+        <th>Status</th>
+        <th>Date</th>
         <th></th>
       </tr>
     </thead>
     <tbody>
       <% @change_requests.each do |change_request| %>
         <tr>
-          <td><%= link_to change_request.original_name, user_path(change_request.user_id) %></td>
+          <td><%= link_to_user change_request.user %></td>
-          <td><%= change_request.desired_name %></td>
+          <td>
+            <strong><%= change_request.original_name %></strong> ->
+            <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>
       <% end %>
@@ -22,3 +42,5 @@
   
   <%= numbered_paginator(@change_requests) %>
 </div>
+
+<%= render "secondary_links" %>

app/views/user_name_change_requests/new.html.erb

@@ -19,3 +19,5 @@
     <%= submit_tag "Submit", :data => { :disable_with => "Submitting..." } %>
   </div>
 <% end %>
+
+<%= render "secondary_links" %>

app/views/user_name_change_requests/show.html.erb

@@ -1,49 +1,70 @@
 <div id="c-user-name-change-requests">
   <h1>Name Change Request</h1>
-  
-  <section>
-    <p><%= @change_request.original_name %> is requesting to change their name to <%= @change_request.desired_name %>.</p>
-  </section>
-  
-  <section>
-    <h2>Reason</h2>
-    <p><%= @change_request.change_reason %></p>
-  </section>
 
-  <section>
+  <table class="aligned-vertical">
-    <h2>Feedback</h2>
+    <tbody>
-    <ul>
+      <tr>
-      <% @change_request.feedback.each do |feedback| %>
+        <th>Date</th>
-        <li class="feedback-category-<%= feedback.category %>">
+        <td>
-          <p><%= feedback.body %></p>
+          <%= compact_time @change_request.created_at %>
-          <p class="author">Submitted by <%= feedback.creator.name %> <%= time_ago_in_words_tagged feedback.created_at %></p>
+          <% if @change_request.updated_at - @change_request.created_at > 5.minutes %>
-        </li>
+            (updated <%= time_ago_in_words_tagged @change_request.updated_at %>)
-      <% end %>
+          <% end %>
-    </ul>
+        </td>
-  </section>
+      </tr>
+      <tr>
+        <th>User</th>
+        <td><%= link_to_user @change_request.user %></td>
+      </tr>
+      <tr>
+        <th>Request</th>
+        <td>
+          <strong><%= @change_request.original_name %></strong> ->
+          <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="author">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>
-  
-  <% if @change_request.rejected? %>
     <section>
-      <h2>Request was rejected</h2>
+      <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>
-  
+
-  <% elsif @change_request.approved? && @change_request.approver %>
-    <section>
-      <h2>Request was approved by <%= @change_request.approver.name %></h2>
-    </section>
-  
-  <% elsif CurrentUser.user.is_admin? %>
     <section>
       <h2>Options</h2>
       <%= form_tag(approve_user_name_change_request_path(@change_request)) do %>
@@ -62,11 +83,6 @@
       <% end %>
     </section>
   <% end %>
-  
-  <section>
-    <%= form_tag(user_name_change_request_path(@change_request), :method => :delete) do %>
-      <%= submit_tag "Delete" %>
-    <% end %>
-  </section>
 </div>
 
+<%= render "secondary_links" %>

test/functional/user_name_change_requests_controller_test.rb

@@ -61,13 +61,6 @@ class UserNameChangeRequestsControllerTest < ActionController::TestCase
           assert_redirected_to(user_name_change_request_path(@change_request))
         end
       end
-      
-      context "destroy action" do
-        should "destroy" do
-          post :destroy, {:id => @change_request.id}, {:user_id => @admin.id}
-          assert_redirected_to(user_name_change_requests_path)
-        end
-      end
     end
   end
 end