diff --git a/.rubocop.yml b/.rubocop.yml index c0f0c1fa08..d43c41b776 100644 --- a/.rubocop.yml +++ b/.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 diff --git a/app/controllers/administrator/users_controller.rb b/app/controllers/administrator/users_controller.rb new file mode 100644 index 0000000000..00e58d631a --- /dev/null +++ b/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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4ebeb1ec3a..775ca4ea02 100644 --- a/app/controllers/users_controller.rb +++ b/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]) diff --git a/app/views/users/_page.html.erb b/app/views/administrator/users/_page.html.erb similarity index 100% rename from app/views/users/_page.html.erb rename to app/views/administrator/users/_page.html.erb diff --git a/app/views/users/_user.html.erb b/app/views/administrator/users/_user.html.erb similarity index 87% rename from app/views/users/_user.html.erb rename to app/views/administrator/users/_user.html.erb index ef50ccaf2f..2667c9f85a 100644 --- a/app/views/users/_user.html.erb +++ b/app/views/administrator/users/_user.html.erb @@ -5,12 +5,12 @@

<% 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 %> diff --git a/app/views/users/index.html.erb b/app/views/administrator/users/index.html.erb similarity index 100% rename from app/views/users/index.html.erb rename to app/views/administrator/users/index.html.erb diff --git a/config/locales/en.yml b/config/locales/en.yml index 9a383569ee..0c7a882d29 100644 --- a/config/locales/en.yml +++ b/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: diff --git a/config/routes.rb b/config/routes.rb index 22c4ad6421..0fb1af98f3 100644 --- a/config/routes.rb +++ b/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" diff --git a/test/controllers/administrator/users_controller_test.rb b/test/controllers/administrator/users_controller_test.rb new file mode 100644 index 0000000000..f792813540 --- /dev/null +++ b/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 diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 4f4edf10b0..732745539a 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -47,23 +47,6 @@ def test_routes { :path => "/user/username", :method => :delete }, { :controller => "users", :action => "destroy", :display_name => "username" } ) - - assert_routing( - { :path => "/users", :method => :get }, - { :controller => "users", :action => "index" } - ) - assert_routing( - { :path => "/users", :method => :post }, - { :controller => "users", :action => "index" } - ) - assert_routing( - { :path => "/users/status", :method => :get }, - { :controller => "users", :action => "index", :status => "status" } - ) - assert_routing( - { :path => "/users/status", :method => :post }, - { :controller => "users", :action => "index", :status => "status" } - ) end # The user creation page loads @@ -471,204 +454,6 @@ def test_destroy assert_equal "deleted", user.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 - - 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 - - public - - 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 - def test_auth_failure_callback get auth_failure_path assert_redirected_to login_path