Merge remote-tracking branch 'upstream/pull/3397'

.rubocop_todo.yml

@@ -40,6 +40,7 @@ Lint/AssignmentInCondition:
   Exclude:
     - 'app/controllers/api/traces_controller.rb'
     - 'app/controllers/api/user_preferences_controller.rb'
+    - 'app/controllers/accounts_controller.rb'
     - 'app/controllers/application_controller.rb'
     - 'app/controllers/geocoder_controller.rb'
     - 'app/controllers/notes_controller.rb'

app/abilities/ability.rb

@@ -42,6 +42,7 @@ class Ability
         can [:index, :new, :create, :show, :edit, :update, :destroy], :oauth2_application
         can [:index, :destroy], :oauth2_authorized_application
         can [:new, :show, :create, :destroy], :oauth2_authorization
+        can [:edit, :update], :account
         can [:show], :dashboard
         can [:new, :create, :edit, :update, :comment, :subscribe, :unsubscribe], DiaryEntry
         can [:make_friend, :remove_friend], Friendship

app/controllers/accounts_controller.rb

@@ -0,0 +1,52 @@
+class AccountsController < ApplicationController
+  include SessionMethods
+  include UserMethods
+
+  layout "site"
+
+  before_action :authorize_web
+  before_action :set_locale
+
+  authorize_resource :class => false
+
+  before_action :check_database_readable
+  before_action :check_database_writable, :only => [:update]
+  before_action :allow_thirdparty_images, :only => [:edit, :update]
+
+  def edit
+    @tokens = current_user.oauth_tokens.authorized
+
+    append_content_security_policy_directives(
+      :form_action => %w[accounts.google.com *.facebook.com login.live.com github.com meta.wikimedia.org]
+    )
+
+    if errors = session.delete(:user_errors)
+      errors.each do |attribute, error|
+        current_user.errors.add(attribute, error)
+      end
+    end
+    @title = t ".title"
+  end
+
+  def update
+    @tokens = current_user.oauth_tokens.authorized
+
+    append_content_security_policy_directives(
+      :form_action => %w[accounts.google.com *.facebook.com login.live.com github.com meta.wikimedia.org]
+    )
+
+    if params[:user][:auth_provider].blank? ||
+       (params[:user][:auth_provider] == current_user.auth_provider &&
+        params[:user][:auth_uid] == current_user.auth_uid)
+      update_user(current_user, params)
+      if current_user.errors.count.zero?
+        redirect_to edit_account_path
+      else
+        render :edit
+      end
+    else
+      session[:new_user_settings] = params
+      redirect_to auth_url(params[:user][:auth_provider], params[:user][:auth_uid]), :status => :temporary_redirect
+    end
+  end
+end

app/controllers/concerns/user_methods.rb

@@ -0,0 +1,47 @@
+module UserMethods
+  extend ActiveSupport::Concern
+
+  private
+
+  ##
+  # update a user's details
+  def update_user(user, params)
+    user.display_name = params[:user][:display_name]
+    user.new_email = params[:user][:new_email]
+
+    unless params[:user][:pass_crypt].empty? && params[:user][:pass_crypt_confirmation].empty?
+      user.pass_crypt = params[:user][:pass_crypt]
+      user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation]
+    end
+
+    if params[:user][:auth_provider].nil? || params[:user][:auth_provider].blank?
+      user.auth_provider = nil
+      user.auth_uid = nil
+    end
+
+    if user.save
+      session[:fingerprint] = user.fingerprint
+
+      if user.new_email.blank? || user.new_email == user.email
+        flash[:notice] = t "accounts.update.success"
+      else
+        user.email = user.new_email
+
+        if user.valid?
+          flash[:notice] = t "accounts.update.success_confirm_needed"
+
+          begin
+            UserMailer.email_confirm(user, user.tokens.create).deliver_later
+          rescue StandardError
+            # Ignore errors sending email
+          end
+        else
+          current_user.errors.add(:new_email, current_user.errors[:email])
+          current_user.errors.add(:email, [])
+        end
+
+        user.restore_email!
+      end
+    end
+  end
+end

app/controllers/confirmations_controller.rb

@@ -93,10 +93,10 @@ class ConfirmationsController < ApplicationController
         current_user.tokens.delete_all
         session[:user] = current_user.id
         session[:fingerprint] = current_user.fingerprint
-        redirect_to user_account_path(current_user)
+        redirect_to edit_account_path
       elsif token
         flash[:error] = t "confirmations.confirm_email.failure"
-        redirect_to user_account_path(token.user)
+        redirect_to edit_account_path
       else
         flash[:error] = t "confirmations.confirm_email.unknown_token"
       end

app/controllers/users_controller.rb

@@ -1,5 +1,6 @@
 class UsersController < ApplicationController
   include SessionMethods
+  include UserMethods
 
   layout "site"
 
@@ -11,11 +12,10 @@ class UsersController < ApplicationController
 
   authorize_resource
 
-  before_action :require_self, :only => [:account]
+  before_action :check_database_writable, :only => [:new, :go_public]
-  before_action :check_database_writable, :only => [:new, :account, :go_public]
   before_action :require_cookies, :only => [:new]
   before_action :lookup_user_by_name, :only => [:set_status, :destroy]
-  before_action :allow_thirdparty_images, :only => [:show, :account]
+  before_action :allow_thirdparty_images, :only => [:show]
 
   def terms
     @legale = params[:legale] || OSM.ip_to_country(request.remote_ip) || Settings.default_legale
@@ -28,7 +28,7 @@ class UsersController < ApplicationController
 
       if current_user&.terms_agreed?
         # Already agreed to terms, so just show settings
-        redirect_to user_account_path(current_user)
+        redirect_to edit_account_path
       elsif current_user.nil? && session[:new_user].nil?
         redirect_to login_path(:referer => request.fullpath)
       end
@@ -46,7 +46,7 @@ class UsersController < ApplicationController
 
         referer = safe_referer(params[:referer]) if params[:referer]
 
-        redirect_to referer || user_account_path(current_user)
+        redirect_to referer || edit_account_path
       elsif params[:decline]
         redirect_to t("users.terms.declined")
       else
@@ -64,7 +64,7 @@ class UsersController < ApplicationController
 
       referer = safe_referer(params[:referer]) if params[:referer]
 
-      redirect_to referer || user_account_path(current_user)
+      redirect_to referer || edit_account_path
     else
       self.current_user = session.delete(:new_user)
 
@@ -114,36 +114,11 @@ class UsersController < ApplicationController
     end
   end
 
-  def account
-    @tokens = current_user.oauth_tokens.authorized
-
-    append_content_security_policy_directives(
-      :form_action => %w[accounts.google.com *.facebook.com login.live.com github.com meta.wikimedia.org]
-    )
-
-    if request.post?
-      if params[:user][:auth_provider].blank? ||
-         (params[:user][:auth_provider] == current_user.auth_provider &&
-          params[:user][:auth_uid] == current_user.auth_uid)
-        update_user(current_user, params)
-        redirect_to user_account_url(current_user) if current_user.errors.count.zero?
-      else
-        session[:new_user_settings] = params
-        redirect_to auth_url(params[:user][:auth_provider], params[:user][:auth_uid]), :status => :temporary_redirect
-      end
-    elsif errors = session.delete(:user_errors)
-      errors.each do |attribute, error|
-        current_user.errors.add(attribute, error)
-      end
-    end
-    @title = t "users.account.title"
-  end
-
   def go_public
     current_user.data_public = true
     current_user.save
     flash[:notice] = t "users.go_public.flash success"
-    redirect_to user_account_path(current_user)
+    redirect_to edit_account_path
   end
 
   def new
@@ -293,7 +268,7 @@ class UsersController < ApplicationController
 
       session[:user_errors] = current_user.errors.as_json
 
-      redirect_to user_account_path(current_user)
+      redirect_to edit_account_path
     elsif session[:new_user]
       session[:new_user].auth_provider = provider
       session[:new_user].auth_uid = uid
@@ -340,54 +315,6 @@ class UsersController < ApplicationController
 
   private
 
-  ##
-  # update a user's details
-  def update_user(user, params)
-    user.display_name = params[:user][:display_name]
-    user.new_email = params[:user][:new_email]
-
-    unless params[:user][:pass_crypt].empty? && params[:user][:pass_crypt_confirmation].empty?
-      user.pass_crypt = params[:user][:pass_crypt]
-      user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation]
-    end
-
-    if params[:user][:auth_provider].nil? || params[:user][:auth_provider].blank?
-      user.auth_provider = nil
-      user.auth_uid = nil
-    end
-
-    if user.save
-      session[:fingerprint] = user.fingerprint
-
-      if user.new_email.blank? || user.new_email == user.email
-        flash[:notice] = t "users.account.flash update success"
-      else
-        user.email = user.new_email
-
-        if user.valid?
-          flash[:notice] = t "users.account.flash update success confirm needed"
-
-          begin
-            UserMailer.email_confirm(user, user.tokens.create).deliver_later
-          rescue StandardError
-            # Ignore errors sending email
-          end
-        else
-          current_user.errors.add(:new_email, current_user.errors[:email])
-          current_user.errors.add(:email, [])
-        end
-
-        user.restore_email!
-      end
-    end
-  end
-
-  ##
-  # require that the user in the URL is the logged in user
-  def require_self
-    head :forbidden if params[:display_name] != current_user.display_name
-  end
-
   ##
   # ensure that there is a "user" instance variable
   def lookup_user_by_name

app/views/accounts/edit.html.erb

@@ -6,9 +6,9 @@
   <h1><%= t ".my settings" %></h1>
 <% end %>
 
-<%= render :partial => "settings_menu", :locals => { :selected => "account" } %>
+<%= render :partial => "settings_menu" %>
 
-<%= bootstrap_form_for current_user, :url => { :action => :account }, :method => :post, :html => { :multipart => true, :id => "accountForm", :autocomplete => :off } do |f| %>
+<%= bootstrap_form_for current_user, :url => { :action => :update }, :html => { :multipart => true, :id => "accountForm", :autocomplete => :off } do |f| %>
 
   <%= f.text_field :display_name %>
   <%= f.email_field :email, :disabled => true, :label => t(".current email address") %>

app/views/application/_settings_menu.html.erb

@@ -3,7 +3,7 @@
 <% content_for :heading do %>
   <ul class="nav nav-tabs flex-column flex-sm-row">
     <li class="nav-item">
-      <%= link_to t(".account_settings"), user_account_path(current_user), :class => "nav-link #{'active' if controller_name == 'users'}" %>
+      <%= link_to t(".account_settings"), edit_account_path, :class => "nav-link #{'active' if controller_name == 'accounts'}" %>
     </li>
     <li class="nav-item">
       <%= link_to t(".oauth1_settings"), oauth_clients_path(current_user), :class => "nav-link #{'active' if controller_name == 'oauth_clients'}" %>

app/views/layouts/_header.html.erb

@@ -96,7 +96,7 @@
             <span class='count-number'><%= number_with_delimiter(current_user.new_messages.size) %></span>
           <% end %>
           <%= link_to t("users.show.my profile"), user_path(current_user), :class => "dropdown-item" %>
-          <%= link_to t("users.show.my settings"), user_account_path(current_user), :class => "dropdown-item" %>
+          <%= link_to t("users.show.my settings"), edit_account_path, :class => "dropdown-item" %>
           <%= link_to t("users.show.my_preferences"), preferences_path, :class => "dropdown-item" %>
           <div class="dropdown-divider"></div>
           <%= yield :greeting %>

app/views/sessions/new.html.erb

@@ -53,7 +53,7 @@
           <label for='openid_url'><%= t ".openid_html", :logo => openid_logo %></label>
           <%= hidden_field_tag("referer", params[:referer], :autocomplete => "off") %>
           <%= text_field_tag("openid_url", "", :tabindex => 3, :autocomplete => "on", :class => "openid_url form-control") %>
-          <span class="form-text text-muted">(<a href="<%= t "users.account.openid.link" %>" target="_new"><%= t "users.account.openid.link text" %></a>)</span>
+          <span class="form-text text-muted">(<a href="<%= t "accounts.edit.openid.link" %>" target="_new"><%= t "accounts.edit.openid.link text" %></a>)</span>
         </div>
 
         <%= submit_tag t(".login_button"), :tabindex => 6, :id => "login_openid_submit", :class => "btn btn-primary" %>

app/views/site/edit.html.erb

@@ -5,7 +5,7 @@
     <p><%= t "layouts.osm_read_only" %></p>
   <% elsif !current_user.data_public? %>
     <p><%= t ".not_public" %></p>
-    <p><%= t ".not_public_description_html", :user_page => (link_to t(".user_page_link"), user_account_path(current_user, :anchor => "public")) %></p>
+    <p><%= t ".not_public_description_html", :user_page => (link_to t(".user_page_link"), edit_account_path(:anchor => "public")) %></p>
     <p><%= t ".anon_edits_html", :link => link_to(t(".anon_edits_link_text"), t(".anon_edits_link")) %></p>
   <% else %>
     <%= render :partial => preferred_editor %>

app/views/users/show.html.erb

@@ -28,7 +28,7 @@
               <%= link_to t(".my comments"), diary_comments_path(current_user) %>
             </li>
             <li>
-              <%= link_to t(".my settings"), user_account_path(current_user) %>
+              <%= link_to t(".my settings"), edit_account_path %>
             </li>
 
             <% if current_user.blocks.exists? %>

config/locales/en.yml

@@ -239,6 +239,38 @@ en:
       entry:
         comment: Comment
         full: Full note
+  accounts:
+    edit:
+      title: "Edit account"
+      my settings: My Settings
+      current email address: "Current Email Address"
+      external auth: "External Authentication"
+      openid:
+        link: "https://wiki.openstreetmap.org/wiki/OpenID"
+        link text: "what is this?"
+      public editing:
+        heading: "Public editing"
+        enabled: "Enabled. Not anonymous and can edit data."
+        enabled link: "https://wiki.openstreetmap.org/wiki/Anonymous_edits"
+        enabled link text: "what is this?"
+        disabled: "Disabled and cannot edit data, all previous edits are anonymous."
+        disabled link text: "why can't I edit?"
+      public editing note:
+        heading: "Public editing"
+        html: "Currently your edits are anonymous and people cannot send you messages or see your location. To show what you edited and allow people to contact you through the website, click the button below. <b>Since the 0.6 API changeover, only public users can edit map data</b>. (<a href=\"https://wiki.openstreetmap.org/wiki/Anonymous_edits\">find out why</a>).<ul><li>Your email address will not be revealed by becoming public.</li><li>This action cannot be reversed and all new users are now public by default.</li></ul>"
+      contributor terms:
+        heading: "Contributor Terms"
+        agreed: "You have agreed to the new Contributor Terms."
+        not yet agreed: "You have not yet agreed to the new Contributor Terms."
+        review link text: "Please follow this link at your convenience to review and accept the new Contributor Terms."
+        agreed_with_pd: "You have also declared that you consider your edits to be in the Public Domain."
+        link: "https://www.osmfoundation.org/wiki/License/Contributor_Terms"
+        link text: "what is this?"
+      save changes button: Save Changes
+      make edits public button: Make all my edits public
+    update:
+      success_confirm_needed: "User information updated successfully. Check your email for a note to confirm your new email address."
+      success: "User information updated successfully."
   browse:
     created: "Created"
     closed: "Closed"
@@ -2561,36 +2593,6 @@ en:
       delete_user: "Delete this User"
       confirm: "Confirm"
       report: Report this User
-    account:
-      title: "Edit account"
-      my settings: My Settings
-      current email address: "Current Email Address"
-      external auth: "External Authentication"
-      openid:
-        link: "https://wiki.openstreetmap.org/wiki/OpenID"
-        link text: "what is this?"
-      public editing:
-        heading: "Public editing"
-        enabled: "Enabled. Not anonymous and can edit data."
-        enabled link: "https://wiki.openstreetmap.org/wiki/Anonymous_edits"
-        enabled link text: "what is this?"
-        disabled: "Disabled and cannot edit data, all previous edits are anonymous."
-        disabled link text: "why can't I edit?"
-      public editing note:
-        heading: "Public editing"
-        html: "Currently your edits are anonymous and people cannot send you messages or see your location. To show what you edited and allow people to contact you through the website, click the button below. <b>Since the 0.6 API changeover, only public users can edit map data</b>. (<a href=\"https://wiki.openstreetmap.org/wiki/Anonymous_edits\">find out why</a>).<ul><li>Your email address will not be revealed by becoming public.</li><li>This action cannot be reversed and all new users are now public by default.</li></ul>"
-      contributor terms:
-        heading: "Contributor Terms"
-        agreed: "You have agreed to the new Contributor Terms."
-        not yet agreed: "You have not yet agreed to the new Contributor Terms."
-        review link text: "Please follow this link at your convenience to review and accept the new Contributor Terms."
-        agreed_with_pd: "You have also declared that you consider your edits to be in the Public Domain."
-        link: "https://www.osmfoundation.org/wiki/License/Contributor_Terms"
-        link text: "what is this?"
-      save changes button: Save Changes
-      make edits public button: Make all my edits public
-      flash update success confirm needed: "User information updated successfully. Check your email for a note to confirm your new email address."
-      flash update success: "User information updated successfully."
     set_home:
       flash success: "Home location saved successfully"
     go_public:

config/routes.rb

@@ -237,9 +237,10 @@ OpenStreetMap::Application.routes.draw do
 
   # user pages
   resources :users, :path => "user", :param => :display_name, :only => [:show, :destroy]
-  match "/user/:display_name/account" => "users#account", :via => [:get, :post], :as => "user_account"
+  get "/user/:display_name/account", :to => redirect(:path => "/account/edit")
   post "/user/:display_name/set_status" => "users#set_status", :as => :set_status_user
 
+  resource :account, :only => [:edit, :update]
   resource :dashboard, :only => [:show]
   resource :preferences, :only => [:show, :edit, :update]
   resource :profile, :only => [:edit, :update]

test/controllers/accounts_controller_test.rb

@@ -0,0 +1,128 @@
+require "test_helper"
+
+class AccountsControllerTest < ActionDispatch::IntegrationTest
+  ##
+  # test all routes which lead to this controller
+  def test_routes
+    assert_routing(
+      { :path => "/account/edit", :method => :get },
+      { :controller => "accounts", :action => "edit" }
+    )
+    assert_routing(
+      { :path => "/account", :method => :put },
+      { :controller => "accounts", :action => "update" }
+    )
+  end
+
+  def test_account
+    # Get a user to work with - note that this user deliberately
+    # conflicts with uppercase_user in the email and display name
+    # fields to test that we can change other fields without any
+    # validation errors being reported
+    user = create(:user, :languages => [])
+    _uppercase_user = build(:user, :email => user.email.upcase, :display_name => user.display_name.upcase).tap { |u| u.save(:validate => false) }
+
+    # Make sure that you are redirected to the login page when
+    # you are not logged in
+    get edit_account_path
+    assert_response :redirect
+    assert_redirected_to login_path(:referer => "/account/edit")
+
+    # Make sure we get the page when we are logged in as the right user
+    session_for(user)
+    get edit_account_path
+    assert_response :success
+    assert_template :edit
+    assert_select "form#accountForm" do |form|
+      assert_equal "post", form.attr("method").to_s
+      assert_select "input[name='_method']", true
+      assert_equal "/account", form.attr("action").to_s
+    end
+
+    # Updating the description using GET should fail
+    user.description = "new description"
+    user.preferred_editor = "default"
+    get edit_account_path, :params => { :user => user.attributes }
+    assert_response :success
+    assert_template :edit
+    assert_not_equal user.description, User.find(user.id).description
+
+    # Adding external authentication should redirect to the auth provider
+    put account_path, :params => { :user => user.attributes.merge(:auth_provider => "openid", :auth_uid => "gmail.com") }
+    assert_response :redirect
+    assert_redirected_to auth_path(:provider => "openid", :openid_url => "https://www.google.com/accounts/o8/id", :origin => "/account")
+
+    # Changing name to one that exists should fail
+    new_attributes = user.attributes.dup.merge(:display_name => create(:user).display_name)
+    put account_path, :params => { :user => new_attributes }
+    assert_response :success
+    assert_template :edit
+    assert_select ".notice", false
+    assert_select "form#accountForm > div.form-group > input.is-invalid#user_display_name"
+
+    # Changing name to one that exists should fail, regardless of case
+    new_attributes = user.attributes.dup.merge(:display_name => create(:user).display_name.upcase)
+    put account_path, :params => { :user => new_attributes }
+    assert_response :success
+    assert_template :edit
+    assert_select ".notice", false
+    assert_select "form#accountForm > div.form-group > input.is-invalid#user_display_name"
+
+    # Changing name to one that doesn't exist should work
+    new_attributes = user.attributes.dup.merge(:display_name => "new tester")
+    put account_path, :params => { :user => new_attributes }
+    assert_response :redirect
+    assert_redirected_to edit_account_url
+    get edit_account_path
+    assert_response :success
+    assert_template :edit
+    assert_select ".notice", /^User information updated successfully/
+    assert_select "form#accountForm > div.form-group > input#user_display_name[value=?]", "new tester"
+
+    # Record the change of name
+    user.display_name = "new tester"
+
+    # Changing email to one that exists should fail
+    user.new_email = create(:user).email
+    assert_no_difference "ActionMailer::Base.deliveries.size" do
+      perform_enqueued_jobs do
+        put account_path, :params => { :user => user.attributes }
+      end
+    end
+    assert_response :success
+    assert_template :edit
+    assert_select ".notice", false
+    assert_select "form#accountForm > div.form-group > input.is-invalid#user_new_email"
+
+    # Changing email to one that exists should fail, regardless of case
+    user.new_email = create(:user).email.upcase
+    assert_no_difference "ActionMailer::Base.deliveries.size" do
+      perform_enqueued_jobs do
+        put account_path, :params => { :user => user.attributes }
+      end
+    end
+    assert_response :success
+    assert_template :edit
+    assert_select ".notice", false
+    assert_select "form#accountForm > div.form-group > input.is-invalid#user_new_email"
+
+    # Changing email to one that doesn't exist should work
+    user.new_email = "new_tester@example.com"
+    assert_difference "ActionMailer::Base.deliveries.size", 1 do
+      perform_enqueued_jobs do
+        put account_path, :params => { :user => user.attributes }
+      end
+    end
+    assert_response :redirect
+    assert_redirected_to edit_account_url
+    get edit_account_path
+    assert_response :success
+    assert_template :edit
+    assert_select ".notice", /^User information updated successfully/
+    assert_select "form#accountForm > div.form-group > input#user_new_email[value=?]", user.new_email
+    email = ActionMailer::Base.deliveries.first
+    assert_equal 1, email.to.count
+    assert_equal user.new_email, email.to.first
+    ActionMailer::Base.deliveries.clear
+  end
+end

test/controllers/confirmations_controller_test.rb

@@ -283,7 +283,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
     assert_response :redirect
-    assert_redirected_to :controller => :users, :action => :account, :display_name => user.display_name
+    assert_redirected_to edit_account_path
     assert_match(/Confirmed your change of email address/, flash[:notice])
   end
 
@@ -293,7 +293,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
     assert_response :redirect
-    assert_redirected_to :controller => :users, :action => :account, :display_name => user.display_name
+    assert_redirected_to edit_account_path
     assert_match(/already been confirmed/, flash[:error])
   end
 
@@ -317,7 +317,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     assert_not user.image_use_gravatar
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
     assert_response :redirect
-    assert_redirected_to :controller => :users, :action => :account, :display_name => user.display_name
+    assert_redirected_to edit_account_path
     assert_match(/Confirmed your change of email address/, flash[:notice])
     # gravatar use should now be enabled
     assert User.find(user.id).image_use_gravatar
@@ -332,7 +332,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     assert user.image_use_gravatar
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
     assert_response :redirect
-    assert_redirected_to :controller => :users, :action => :account, :display_name => user.display_name
+    assert_redirected_to edit_account_path
     assert_match(/Confirmed your change of email address/, flash[:notice])
     # gravatar use should now be disabled
     assert_not User.find(user.id).image_use_gravatar

test/controllers/users_controller_test.rb

@@ -39,15 +39,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       { :controller => "users", :action => "show", :display_name => "username" }
     )
 
-    assert_routing(
-      { :path => "/user/username/account", :method => :get },
-      { :controller => "users", :action => "account", :display_name => "username" }
-    )
-    assert_routing(
-      { :path => "/user/username/account", :method => :post },
-      { :controller => "users", :action => "account", :display_name => "username" }
-    )
-
     assert_routing(
       { :path => "/user/username/set_status", :method => :post },
       { :controller => "users", :action => "set_status", :display_name => "username" }
@@ -358,7 +349,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     get user_terms_path
     assert_response :redirect
-    assert_redirected_to :action => :account, :display_name => user.display_name
+    assert_redirected_to edit_account_path
   end
 
   def test_terms_not_seen_without_referer
@@ -372,7 +363,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     post user_save_path, :params => { :user => { :consider_pd => true }, :read_ct => 1, :read_tou => 1 }
     assert_response :redirect
-    assert_redirected_to :action => :account, :display_name => user.display_name
+    assert_redirected_to edit_account_path
     assert_equal "Thanks for accepting the new contributor terms!", flash[:notice]
 
     user.reload
@@ -409,9 +400,9 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     user = create(:user, :terms_seen => false, :terms_agreed => nil)
     session_for(user)
 
-    get user_account_path(user)
+    get edit_account_path
     assert_response :redirect
-    assert_redirected_to :action => :terms, :referer => "/user/#{ERB::Util.u(user.display_name)}/account"
+    assert_redirected_to :controller => :users, :action => :terms, :referer => "/account/edit"
   end
 
   def test_terms_not_logged_in
@@ -427,127 +418,10 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     post user_go_public_path
 
     assert_response :redirect
-    assert_redirected_to :action => :account, :display_name => user.display_name
+    assert_redirected_to edit_account_path
     assert User.find(user.id).data_public
   end
 
-  def test_account
-    # Get a user to work with - note that this user deliberately
-    # conflicts with uppercase_user in the email and display name
-    # fields to test that we can change other fields without any
-    # validation errors being reported
-    user = create(:user, :languages => [])
-    _uppercase_user = build(:user, :email => user.email.upcase, :display_name => user.display_name.upcase).tap { |u| u.save(:validate => false) }
-
-    # Make sure that you are redirected to the login page when
-    # you are not logged in
-    get user_account_path(user)
-    assert_response :redirect
-    assert_redirected_to login_path(:referer => "/user/#{ERB::Util.u(user.display_name)}/account")
-
-    # Make sure that you are blocked when not logged in as the right user
-    session_for(create(:user))
-    get user_account_path(user)
-    assert_response :forbidden
-
-    # Make sure we get the page when we are logged in as the right user
-    session_for(user)
-    get user_account_path(user)
-    assert_response :success
-    assert_template :account
-    assert_select "form#accountForm" do |form|
-      assert_equal "post", form.attr("method").to_s
-      assert_select "input[name='_method']", false
-      assert_equal "/user/#{ERB::Util.u(user.display_name)}/account", form.attr("action").to_s
-    end
-
-    # Updating the description using GET should fail
-    user.description = "new description"
-    user.preferred_editor = "default"
-    get user_account_path(user), :params => { :user => user.attributes }
-    assert_response :success
-    assert_template :account
-    assert_not_equal user.description, User.find(user.id).description
-
-    # Adding external authentication should redirect to the auth provider
-    post user_account_path(user), :params => { :user => user.attributes.merge(:auth_provider => "openid", :auth_uid => "gmail.com") }
-    assert_response :redirect
-    assert_redirected_to auth_path(:provider => "openid", :openid_url => "https://www.google.com/accounts/o8/id", :origin => "/user/#{ERB::Util.u(user.display_name)}/account")
-
-    # Changing name to one that exists should fail
-    new_attributes = user.attributes.dup.merge(:display_name => create(:user).display_name)
-    post user_account_path(user), :params => { :user => new_attributes }
-    assert_response :success
-    assert_template :account
-    assert_select ".notice", false
-    assert_select "form#accountForm > div.form-group > input.is-invalid#user_display_name"
-
-    # Changing name to one that exists should fail, regardless of case
-    new_attributes = user.attributes.dup.merge(:display_name => create(:user).display_name.upcase)
-    post user_account_path(user), :params => { :user => new_attributes }
-    assert_response :success
-    assert_template :account
-    assert_select ".notice", false
-    assert_select "form#accountForm > div.form-group > input.is-invalid#user_display_name"
-
-    # Changing name to one that doesn't exist should work
-    new_attributes = user.attributes.dup.merge(:display_name => "new tester")
-    post user_account_path(user), :params => { :user => new_attributes }
-    assert_response :redirect
-    assert_redirected_to user_account_url(:display_name => "new tester")
-    get user_account_path(:display_name => "new tester")
-    assert_response :success
-    assert_template :account
-    assert_select ".notice", /^User information updated successfully/
-    assert_select "form#accountForm > div.form-group > input#user_display_name[value=?]", "new tester"
-
-    # Record the change of name
-    user.display_name = "new tester"
-
-    # Changing email to one that exists should fail
-    user.new_email = create(:user).email
-    assert_no_difference "ActionMailer::Base.deliveries.size" do
-      perform_enqueued_jobs do
-        post user_account_path(user), :params => { :user => user.attributes }
-      end
-    end
-    assert_response :success
-    assert_template :account
-    assert_select ".notice", false
-    assert_select "form#accountForm > div.form-group > input.is-invalid#user_new_email"
-
-    # Changing email to one that exists should fail, regardless of case
-    user.new_email = create(:user).email.upcase
-    assert_no_difference "ActionMailer::Base.deliveries.size" do
-      perform_enqueued_jobs do
-        post user_account_path(user), :params => { :user => user.attributes }
-      end
-    end
-    assert_response :success
-    assert_template :account
-    assert_select ".notice", false
-    assert_select "form#accountForm > div.form-group > input.is-invalid#user_new_email"
-
-    # Changing email to one that doesn't exist should work
-    user.new_email = "new_tester@example.com"
-    assert_difference "ActionMailer::Base.deliveries.size", 1 do
-      perform_enqueued_jobs do
-        post user_account_path(user), :params => { :user => user.attributes }
-      end
-    end
-    assert_response :redirect
-    assert_redirected_to user_account_url(user)
-    get user_account_path(user)
-    assert_response :success
-    assert_template :account
-    assert_select ".notice", /^User information updated successfully/
-    assert_select "form#accountForm > div.form-group > input#user_new_email[value=?]", user.new_email
-    email = ActionMailer::Base.deliveries.first
-    assert_equal 1, email.to.count
-    assert_equal user.new_email, email.to.first
-    ActionMailer::Base.deliveries.clear
-  end
-
   # Check that the user account page will display and contains some relevant
   # information for the user
   def test_show
@@ -617,7 +491,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       assert_select "a[href='/traces/mine']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1
-      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/account']", 1
+      assert_select "a[href='/account/edit']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0
       assert_select "a[href='/blocks/new/#{ERB::Util.u(user.display_name)}']", 0
@@ -634,7 +508,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/traces']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary']", 1
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/diary/comments']", 1
-      assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/account']", 0
+      assert_select "a[href='/account/edit']", 0
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks']", 0
       assert_select "a[href='/user/#{ERB::Util.u(user.display_name)}/blocks_by']", 0
       assert_select "a[href='/blocks/new/#{ERB::Util.u(user.display_name)}']", 1

test/integration/client_applications_test.rb

@@ -17,9 +17,9 @@ class ClientApplicationsTest < ActionDispatch::IntegrationTest
     follow_redirect!
     assert_response :success
     assert_template "users/show"
-    get "/user/#{ERB::Util.u(user.display_name)}/account"
+    get "/account/edit"
     assert_response :success
-    assert_template "users/account"
+    assert_template "accounts/edit"
 
     # check that the form to allow new client application creations exists
     assert_in_heading do