Move users list to administrator module

.rubocop.yml

@@ -70,7 +70,7 @@ Rails/ReflectionClassName:
 Rails/SkipsModelValidations:
   Exclude:
     - 'db/migrate/*.rb'
-    - 'app/controllers/users_controller.rb'
+    - 'app/controllers/administrator/users_controller.rb'
 
 Style/Documentation:
   Enabled: false

app/controllers/administrator/users_controller.rb

@@ -0,0 +1,37 @@
+module Administrator
+  class UsersController < ApplicationController
+    include PaginationMethods
+
+    layout "site"
+
+    before_action :authorize_web
+    before_action :set_locale
+    before_action :check_database_readable
+
+    authorize_resource
+
+    ##
+    # display a list of users matching specified criteria
+    def index
+      if request.post?
+        ids = params[:user].keys.collect(&:to_i)
+
+        User.where(:id => ids).update_all(:status => "confirmed") if params[:confirm]
+        User.where(:id => ids).update_all(:status => "deleted") if params[:hide]
+
+        redirect_to url_for(:status => params[:status], :ip => params[:ip], :page => params[:page])
+      else
+        @params = params.permit(:status, :ip, :before, :after)
+
+        users = User.all
+        users = users.where(:status => @params[:status]) if @params[:status]
+        users = users.where(:creation_ip => @params[:ip]) if @params[:ip]
+
+        @users_count = users.count
+        @users, @newer_users_id, @older_users_id = get_page_items(users, :limit => 50)
+
+        render :partial => "page" if turbo_frame_request_id == "pagination"
+      end
+    end
+  end
+end

app/controllers/users_controller.rb

@@ -2,7 +2,6 @@ class UsersController < ApplicationController
   include EmailMethods
   include SessionMethods
   include UserMethods
-  include PaginationMethods
 
   layout "site"
 
@@ -21,30 +20,6 @@ class UsersController < ApplicationController
   allow_thirdparty_images :only => :show
   allow_social_login :only => :new
 
-  ##
-  # display a list of users matching specified criteria
-  def index
-    if request.post?
-      ids = params[:user].keys.collect(&:to_i)
-
-      User.where(:id => ids).update_all(:status => "confirmed") if params[:confirm]
-      User.where(:id => ids).update_all(:status => "deleted") if params[:hide]
-
-      redirect_to url_for(:status => params[:status], :ip => params[:ip], :page => params[:page])
-    else
-      @params = params.permit(:status, :ip, :before, :after)
-
-      users = User.all
-      users = users.where(:status => @params[:status]) if @params[:status]
-      users = users.where(:creation_ip => @params[:ip]) if @params[:ip]
-
-      @users_count = users.count
-      @users, @newer_users_id, @older_users_id = get_page_items(users, :limit => 50)
-
-      render :partial => "page" if turbo_frame_request_id == "pagination"
-    end
-  end
-
   def show
     @user = User.find_by(:display_name => params[:display_name])
 

app/views/users/_user.html.erb → app/views/administrator/users/_user.html.erb

@@ -5,12 +5,12 @@
   <td>
     <p>
       <% if user.creation_ip %>
-        <%= t "users.index.summary_html",
+        <%= t ".summary_html",
               :name => link_to(user.display_name, user),
               :ip_address => link_to(user.creation_ip, :ip => user.creation_ip),
               :date => l(user.created_at, :format => :friendly) %>
       <% else %>
-        <%= t "users.index.summary_no_ip_html",
+        <%= t ".summary_no_ip_html",
               :name => link_to(user.display_name, user),
               :date => l(user.created_at, :format => :friendly) %>
       <% end %>

config/locales/en.yml

@@ -2792,18 +2792,6 @@ en:
       report: Report this User
     go_public:
       flash success: "All your edits are now public, and you are now allowed to edit."
-    index:
-      title: Users
-      heading: Users
-      summary_html: "%{name} created from %{ip_address} on %{date}"
-      summary_no_ip_html: "%{name} created on %{date}"
-      empty: No matching users found
-    page:
-      found_users:
-        one: "%{count} user found"
-        other: "%{count} users found"
-      confirm: Confirm Selected Users
-      hide: Hide Selected Users
     suspended:
       title: Account Suspended
       heading: Account Suspended
@@ -3017,6 +3005,21 @@ en:
       showing_page: "Page %{page}"
       next: "Next"
       previous: "Previous"
+  administrator:
+    users:
+      index:
+        title: Users
+        heading: Users
+        empty: No matching users found
+      page:
+        found_users:
+          one: "%{count} user found"
+          other: "%{count} users found"
+        confirm: Confirm Selected Users
+        hide: Hide Selected Users
+      user:
+        summary_html: "%{name} created from %{ip_address} on %{date}"
+        summary_no_ip_html: "%{name} created on %{date}"
   javascripts:
     close: Close
     share:

config/routes.rb

@@ -284,8 +284,10 @@
   match "/user/:display_name/remove_friend" => "friendships#remove_friend", :via => [:get, :post], :as => "remove_friend"
 
   # user lists
-  match "/users" => "users#index", :via => [:get, :post]
-  match "/users/:status" => "users#index", :via => [:get, :post]
+  scope :module => :administrator do
+    match "users" => "users#index", :via => [:get, :post]
+    match "users/:status" => "users#index", :via => [:get, :post]
+  end
 
   # geocoder
   get "/search" => "geocoder#search"

test/controllers/administrator/users_controller_test.rb

@@ -0,0 +1,222 @@
+require "test_helper"
+
+module Administrator
+  class UsersControllerTest < ActionDispatch::IntegrationTest
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/users", :method => :get },
+        { :controller => "administrator/users", :action => "index" }
+      )
+      assert_routing(
+        { :path => "/users", :method => :post },
+        { :controller => "administrator/users", :action => "index" }
+      )
+      assert_routing(
+        { :path => "/users/status", :method => :get },
+        { :controller => "administrator/users", :action => "index", :status => "status" }
+      )
+      assert_routing(
+        { :path => "/users/status", :method => :post },
+        { :controller => "administrator/users", :action => "index", :status => "status" }
+      )
+    end
+
+    def test_index_get
+      user = create(:user)
+      moderator_user = create(:moderator_user)
+      administrator_user = create(:administrator_user)
+      _suspended_user = create(:user, :suspended)
+      _ip_user = create(:user, :creation_ip => "1.2.3.4")
+
+      # There are now 7 users - the five above, plus two extra "granters" for the
+      # moderator_user and administrator_user
+      assert_equal 7, User.count
+
+      # Shouldn't work when not logged in
+      get users_path
+      assert_redirected_to login_path(:referer => users_path)
+
+      session_for(user)
+
+      # Shouldn't work when logged in as a normal user
+      get users_path
+      assert_redirected_to :controller => "/errors", :action => :forbidden
+
+      session_for(moderator_user)
+
+      # Shouldn't work when logged in as a moderator
+      get users_path
+      assert_redirected_to :controller => "/errors", :action => :forbidden
+
+      session_for(administrator_user)
+
+      # Note there is a header row, so all row counts are users + 1
+      # Should work when logged in as an administrator
+      get users_path
+      assert_response :success
+      assert_template :index
+      assert_select "table#user_list tbody tr", :count => 7
+
+      # Should be able to limit by status
+      get users_path, :params => { :status => "suspended" }
+      assert_response :success
+      assert_template :index
+      assert_select "table#user_list tbody tr", :count => 1
+
+      # Should be able to limit by IP address
+      get users_path, :params => { :ip => "1.2.3.4" }
+      assert_response :success
+      assert_template :index
+      assert_select "table#user_list tbody tr", :count => 1
+    end
+
+    def test_index_get_paginated
+      1.upto(100).each do |n|
+        User.create(:display_name => "extra_#{n}",
+                    :email => "extra#{n}@example.com",
+                    :pass_crypt => "extraextra")
+      end
+
+      session_for(create(:administrator_user))
+
+      # 100 examples, an administrator, and a granter for the admin.
+      assert_equal 102, User.count
+      next_path = users_path
+
+      get next_path
+      assert_response :success
+      assert_template :index
+      assert_select "table#user_list tbody tr", :count => 50
+      check_no_page_link "Newer Users"
+      next_path = check_page_link "Older Users"
+
+      get next_path
+      assert_response :success
+      assert_template :index
+      assert_select "table#user_list tbody tr", :count => 50
+      check_page_link "Newer Users"
+      next_path = check_page_link "Older Users"
+
+      get next_path
+      assert_response :success
+      assert_template :index
+      assert_select "table#user_list tbody tr", :count => 2
+      check_page_link "Newer Users"
+      check_no_page_link "Older Users"
+    end
+
+    def test_index_get_invalid_paginated
+      session_for(create(:administrator_user))
+
+      %w[-1 0 fred].each do |id|
+        get users_path(:before => id)
+        assert_redirected_to :controller => "/errors", :action => :bad_request
+
+        get users_path(:after => id)
+        assert_redirected_to :controller => "/errors", :action => :bad_request
+      end
+    end
+
+    def test_index_post_confirm
+      inactive_user = create(:user, :pending)
+      suspended_user = create(:user, :suspended)
+
+      # Shouldn't work when not logged in
+      assert_no_difference "User.active.count" do
+        post users_path, :params => { :confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 } }
+      end
+      assert_response :forbidden
+
+      assert_equal "pending", inactive_user.reload.status
+      assert_equal "suspended", suspended_user.reload.status
+
+      session_for(create(:user))
+
+      # Shouldn't work when logged in as a normal user
+      assert_no_difference "User.active.count" do
+        post users_path, :params => { :confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 } }
+      end
+      assert_redirected_to :controller => "/errors", :action => :forbidden
+      assert_equal "pending", inactive_user.reload.status
+      assert_equal "suspended", suspended_user.reload.status
+
+      session_for(create(:moderator_user))
+
+      # Shouldn't work when logged in as a moderator
+      assert_no_difference "User.active.count" do
+        post users_path, :params => { :confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 } }
+      end
+      assert_redirected_to :controller => "/errors", :action => :forbidden
+      assert_equal "pending", inactive_user.reload.status
+      assert_equal "suspended", suspended_user.reload.status
+
+      session_for(create(:administrator_user))
+
+      # Should work when logged in as an administrator
+      assert_difference "User.active.count", 2 do
+        post users_path, :params => { :confirm => 1, :user => { inactive_user.id => 1, suspended_user.id => 1 } }
+      end
+      assert_redirected_to :action => :index
+      assert_equal "confirmed", inactive_user.reload.status
+      assert_equal "confirmed", suspended_user.reload.status
+    end
+
+    def test_index_post_hide
+      normal_user = create(:user)
+      confirmed_user = create(:user, :confirmed)
+
+      # Shouldn't work when not logged in
+      assert_no_difference "User.active.count" do
+        post users_path, :params => { :hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 } }
+      end
+      assert_response :forbidden
+
+      assert_equal "active", normal_user.reload.status
+      assert_equal "confirmed", confirmed_user.reload.status
+
+      session_for(create(:user))
+
+      # Shouldn't work when logged in as a normal user
+      assert_no_difference "User.active.count" do
+        post users_path, :params => { :hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 } }
+      end
+      assert_redirected_to :controller => "/errors", :action => :forbidden
+      assert_equal "active", normal_user.reload.status
+      assert_equal "confirmed", confirmed_user.reload.status
+
+      session_for(create(:moderator_user))
+
+      # Shouldn't work when logged in as a moderator
+      assert_no_difference "User.active.count" do
+        post users_path, :params => { :hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 } }
+      end
+      assert_redirected_to :controller => "/errors", :action => :forbidden
+      assert_equal "active", normal_user.reload.status
+      assert_equal "confirmed", confirmed_user.reload.status
+
+      session_for(create(:administrator_user))
+
+      # Should work when logged in as an administrator
+      assert_difference "User.active.count", -2 do
+        post users_path, :params => { :hide => 1, :user => { normal_user.id => 1, confirmed_user.id => 1 } }
+      end
+      assert_redirected_to :action => :index
+      assert_equal "deleted", normal_user.reload.status
+      assert_equal "deleted", confirmed_user.reload.status
+    end
+
+    private
+
+    def check_no_page_link(name)
+      assert_select "a.page-link", { :text => /#{Regexp.quote(name)}/, :count => 0 }, "unexpected #{name} page link"
+    end
+
+    def check_page_link(name)
+      assert_select "a.page-link", { :text => /#{Regexp.quote(name)}/ }, "missing #{name} page link" do |buttons|
+        return buttons.first.attributes["href"].value
+      end
+    end
+  end
+end