diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e68c60f3f..32811a98f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -217,7 +217,7 @@ class ApplicationController < ActionController::Base def redirect_if_name_invalid? if request.format.html? && !CurrentUser.user.is_anonymous? && CurrentUser.user.name_invalid? flash[:notice] = "You must change your username to continue using #{Danbooru.config.app_name}" - redirect_to new_user_name_change_request_path + redirect_to change_name_user_path(CurrentUser.user) end end diff --git a/app/controllers/user_name_change_requests_controller.rb b/app/controllers/user_name_change_requests_controller.rb index 8953f91b8..a9ed1a234 100644 --- a/app/controllers/user_name_change_requests_controller.rb +++ b/app/controllers/user_name_change_requests_controller.rb @@ -6,15 +6,17 @@ class UserNameChangeRequestsController < ApplicationController skip_before_action :redirect_if_name_invalid? def new - @change_request = authorize UserNameChangeRequest.new(permitted_attributes(UserNameChangeRequest)) + user = User.find_by_name(params[:id]) || CurrentUser.user + @change_request = authorize UserNameChangeRequest.new(user: user, **permitted_attributes(UserNameChangeRequest)) respond_with(@change_request) end def create - @change_request = authorize UserNameChangeRequest.new(user: CurrentUser.user, original_name: CurrentUser.user.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) + user = User.find(params.dig(:user_name_change_request, :user_id)) + @change_request = authorize UserNameChangeRequest.new(updater: CurrentUser.user, original_name: user.name, **permitted_attributes(UserNameChangeRequest)) + @change_request.save + flash[:notice] = "Name changed" if @change_request.valid? + respond_with(@change_request, location: @change_request.user) end def show diff --git a/app/models/dmail.rb b/app/models/dmail.rb index de68e9bbf..1b6cbf788 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Dmail < ApplicationRecord - attr_accessor :creator_ip_addr + attr_accessor :creator_ip_addr, :disable_email_notifications validate :validate_sender_is_not_limited, on: :create validates :title, presence: true, length: { maximum: 200 }, if: :title_changed? @@ -145,7 +145,7 @@ class Dmail < ApplicationRecord end def send_email - if is_recipient? && !is_deleted? && to.receive_email_notifications? + if is_recipient? && !is_deleted? && to.receive_email_notifications? && !disable_email_notifications UserMailer.with(headers: { "X-Danbooru-Dmail": Routes.dmail_url(self) }).dmail_notice(self).deliver_later end end diff --git a/app/models/user_name_change_request.rb b/app/models/user_name_change_request.rb index 494385b50..2c6f94e39 100644 --- a/app/models/user_name_change_request.rb +++ b/app/models/user_name_change_request.rb @@ -2,13 +2,16 @@ class UserNameChangeRequest < ApplicationRecord belongs_to :user - belongs_to :approver, class_name: "User", optional: true + + attr_accessor :updater validate :not_limited, on: :create validates :original_name, presence: true validates :desired_name, user_name: true, presence: true, on: :create after_create :update_name! + after_create :create_mod_action + after_create :send_dmail def self.visible(user) if user.is_moderator? @@ -31,9 +34,22 @@ class UserNameChangeRequest < ApplicationRecord def not_limited return if user.name_invalid? + return if updater && updater != user - if UserNameChangeRequest.unscoped.where(user: user).exists?(["created_at >= ?", 1.week.ago]) + if user.user_name_change_requests.exists?(created_at: (1.week.ago..)) errors.add(:base, "You can only submit one name change request per week") end end + + def create_mod_action + return if updater.nil? || user == updater + ModAction.log("changed user ##{user.id}'s name from #{original_name} to #{desired_name}", :user_name_change, subject: user, user: updater) + end + + def send_dmail + return if updater.nil? || user == updater + Dmail.create_automated(to: user, disable_email_notifications: true, title: "Your username has been changed", body: <<~EOS) + Your username has been changed from #{original_name} to #{desired_name}. Your old name was either no longer valid or it violated site rules. You can change it to something else after one week. Please make sure your name follows the [[help:community rules|community rules]]. + EOS + end end diff --git a/app/policies/user_name_change_request_policy.rb b/app/policies/user_name_change_request_policy.rb index 84078526e..ea7460e07 100644 --- a/app/policies/user_name_change_request_policy.rb +++ b/app/policies/user_name_change_request_policy.rb @@ -9,7 +9,17 @@ class UserNameChangeRequestPolicy < ApplicationPolicy user.is_moderator? || (!user.is_anonymous? && !record.user.is_deleted?) || (record.user == user) end - def permitted_attributes - [:desired_name] + def create? + !user.is_anonymous? && (name_change.user == user || can_rename_user?) end + + def can_rename_user? + user.is_moderator? && name_change.user.level < User::Levels::MODERATOR + end + + def permitted_attributes_for_create + [:user_id, :desired_name] + end + + alias_method :name_change, :record end diff --git a/app/views/layouts/default.html.erb b/app/views/layouts/default.html.erb index 28a82a3f2..77bc85df8 100644 --- a/app/views/layouts/default.html.erb +++ b/app/views/layouts/default.html.erb @@ -160,7 +160,7 @@ <% if !CurrentUser.user.is_anonymous? && CurrentUser.user.name_invalid? %>

Action required

-
You must <%= link_to "change your username", new_user_name_change_request_path %> to continue using <%= Danbooru.config.canonical_app_name %>.
+
You must <%= link_to "change your username", change_name_user_path(CurrentUser.user) %> to continue using <%= Danbooru.config.canonical_app_name %>.
<% end %> diff --git a/app/views/static/site_map.html.erb b/app/views/static/site_map.html.erb index cb811ed8a..cd2407b29 100644 --- a/app/views/static/site_map.html.erb +++ b/app/views/static/site_map.html.erb @@ -131,9 +131,6 @@ <% else %>
  • <%= link_to "Profile", profile_path %>
  • <%= link_to "Settings", settings_path %>
  • - <% if policy(UserNameChangeRequest).create? %> -
  • <%= link_to "Change name", new_user_name_change_request_path %>
  • - <% end %>
  • <%= link_to "Uploads", user_uploads_path(CurrentUser.user) %>
  • <%= link_to "Dmails", dmails_path(search: { folder: "received" }) %>
  • <%= link_to "Favorites", favorites_path %>
  • diff --git a/app/views/user_name_change_requests/_secondary_links.html.erb b/app/views/user_name_change_requests/_secondary_links.html.erb index 47803bb0c..569308136 100644 --- a/app/views/user_name_change_requests/_secondary_links.html.erb +++ b/app/views/user_name_change_requests/_secondary_links.html.erb @@ -1,5 +1,5 @@ <% content_for(:secondary_links) do %> <%= subnav_link_to "Listing", user_name_change_requests_path %> - <%= subnav_link_to "New", new_user_name_change_request_path %> + <%= subnav_link_to "New", change_name_user_path(CurrentUser.user) unless CurrentUser.user.is_anonymous? %> <%= subnav_link_to "Help", wiki_page_path("help:user_name_change_requests") %> <% end %> diff --git a/app/views/user_name_change_requests/new.html.erb b/app/views/user_name_change_requests/new.html.erb index 5d8270f19..a29db248b 100644 --- a/app/views/user_name_change_requests/new.html.erb +++ b/app/views/user_name_change_requests/new.html.erb @@ -1,6 +1,10 @@
    -

    Change Name

    + <% if CurrentUser.user != @change_request.user %> +

    Change Name: <%= link_to_user @change_request.user %>

    + <% else %> +

    Change Name

    + <% end %> <% if CurrentUser.user.name_invalid? %>

    Your current username is invalid. You must change your username to continue @@ -26,6 +30,7 @@

    <%= edit_form_for(@change_request) do |f| %> + <%= f.input :user_id, as: :hidden, input_html: { value: @change_request.user_id } %> <%= f.input :desired_name, label: "New name" %> <%= f.submit "Submit" %> <% end %> diff --git a/app/views/user_name_change_requests/show.html.erb b/app/views/user_name_change_requests/show.html.erb index e95f2bc96..6750b1066 100644 --- a/app/views/user_name_change_requests/show.html.erb +++ b/app/views/user_name_change_requests/show.html.erb @@ -8,7 +8,6 @@ Date <%= compact_time @change_request.created_at %> - <%= render "application/update_notice", record: @change_request %> diff --git a/app/views/users/_secondary_links.html.erb b/app/views/users/_secondary_links.html.erb index 66580a933..fd5b4bce3 100644 --- a/app/views/users/_secondary_links.html.erb +++ b/app/views/users/_secondary_links.html.erb @@ -27,6 +27,10 @@ <%= subnav_link_to "Promote", edit_admin_user_path(@user) %> <% end %> + <% if policy(UserNameChangeRequest.new(user: @user)).can_rename_user? %> + <%= subnav_link_to "Rename", change_name_user_path(@user) %> + <% end %> + <% if policy(Ban.new(user: @user)).create? %> <% if @user.is_banned? && @user.active_ban.present? %> <%= subnav_link_to "Unban", ban_path(@user.active_ban) %> diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 351ba0a2b..046ebfe7f 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -14,7 +14,7 @@
    -

    <%= link_to "Change your name", new_user_name_change_request_path %>

    +

    <%= link_to "Change your name", change_name_user_path(@user) %>

    diff --git a/app/views/users/show.html+tooltip.erb b/app/views/users/show.html+tooltip.erb index 75c83966d..63849a4fc 100644 --- a/app/views/users/show.html+tooltip.erb +++ b/app/views/users/show.html+tooltip.erb @@ -83,6 +83,14 @@ <% end %> <% end %> <% end %> + + <% if policy(UserNameChangeRequest.new(user: @user)).can_rename_user? %> + <% menu.item do %> + <%= link_to change_name_user_path(@user) do %> + <%= feedback_icon %> Rename User + <% end %> + <% end %> + <% end %> <% end %>
    diff --git a/config/routes.rb b/config/routes.rb index 7f80ec25d..682c82153 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -270,9 +270,8 @@ Rails.application.routes.draw do resources :api_keys, only: [:new, :create, :edit, :update, :index, :destroy] resources :uploads, only: [:index] - collection do - get :custom_style - end + get :change_name, on: :member, to: "user_name_change_requests#new" + get :custom_style, on: :collection end get "/upgrade", to: "user_upgrades#new", as: "new_user_upgrade" get "/user_upgrades/new", to: redirect("/upgrade") diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index f0ab1423a..d8eb31429 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -95,7 +95,7 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest @user.update_columns(name: "foo__bar") get_auth posts_path, @user - assert_redirected_to new_user_name_change_request_path + assert_redirected_to change_name_user_path(@user) end end diff --git a/test/functional/user_name_change_requests_controller_test.rb b/test/functional/user_name_change_requests_controller_test.rb index 9539ae52d..c3d688de1 100644 --- a/test/functional/user_name_change_requests_controller_test.rb +++ b/test/functional/user_name_change_requests_controller_test.rb @@ -9,33 +9,70 @@ class UserNameChangeRequestsControllerTest < ActionDispatch::IntegrationTest context "new action" do should "render" do - get_auth new_user_name_change_request_path, @user + get_auth change_name_user_path(@user), @user assert_response :success end should "render when the current user's name is invalid" do @user.update_columns(name: "foo__bar") - get_auth new_user_name_change_request_path, @user + get_auth change_name_user_path(@user), @user assert_response :success end end context "create action" do - should "work" do - post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { desired_name: "zun" }} + should "work for a user changing their own name" do + post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { user_id: @user.id, desired_name: "zun" }} - assert_redirected_to profile_path + assert_redirected_to @user assert_equal("zun", @user.reload.name) + + assert_equal(0, ModAction.user_name_change.count) + assert_equal(0, @user.dmails.received.count) + end + + should "work for a moderator changing a regular user's name" do + @user = create(:user, name: "bkub") + @mod = create(:moderator_user) + post_auth user_name_change_requests_path, @mod, params: { user_name_change_request: { user_id: @user.id, desired_name: "zun" }} + + assert_redirected_to @user + assert_equal("zun", @user.reload.name) + + assert_equal("user_name_change", ModAction.last.category) + assert_equal(@mod, ModAction.last.creator) + assert_equal(@user, ModAction.last.subject) + assert_equal("changed user ##{@user.id}'s name from bkub to zun", ModAction.last.description) + + assert_equal(1, @user.dmails.received.count) + assert_equal("Your username has been changed", @user.dmails.received.last.title) + assert_no_enqueued_emails end should "fail if the new name is invalid" do assert_no_changes(-> { @user.reload.name }) do - post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { desired_name: "foo__bar" }} + post_auth user_name_change_requests_path, @user, params: { user_name_change_request: { user_id: @user.id, desired_name: "foo__bar" }} assert_response :success end end + + should "fail for a regular user trying to change another user's name" do + @user = create(:user, name: "bkub") + post_auth user_name_change_requests_path, create(:builder_user), params: { user_name_change_request: { user_id: @user.id, desired_name: "zun" }} + + assert_response 403 + assert_equal("bkub", @user.reload.name) + end + + should "fail for a moderator trying to change the name of someone above Builder level" do + @user = create(:moderator_user, name: "mod") + post_auth user_name_change_requests_path, create(:moderator_user), params: { user_name_change_request: { user_id: @user.id, desired_name: "zun" }} + + assert_response 403 + assert_equal("mod", @user.reload.name) + end end context "show action" do