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

app/abilities/ability.rb

@@ -36,7 +36,7 @@ class Ability
         can [:read, :create, :update, :destroy], :oauth2_application
         can [:read, :destroy], :oauth2_authorized_application
         can [:read, :create, :destroy], :oauth2_authorization
-        can [:update, :destroy], :account
+        can [:read, :update, :destroy], :account
         can :update, :account_terms
         can :create, :account_pd_declaration
         can :read, :dashboard

app/controllers/accounts/pd_declarations_controller.rb

@@ -22,7 +22,7 @@ module Accounts
         end
       end
 
-      redirect_to edit_account_path
+      redirect_to account_path
     end
   end
 end

app/controllers/accounts/terms_controller.rb

@@ -21,7 +21,7 @@ module Accounts
 
         if current_user.terms_agreed?
           # Already agreed to terms, so just show settings
-          redirect_to edit_account_path
+          redirect_to account_path
         end
       end
     end
@@ -43,7 +43,7 @@ module Accounts
 
       referer = safe_referer(params[:referer]) if params[:referer]
 
-      redirect_to referer || edit_account_path
+      redirect_to referer || account_path
     end
   end
 end

app/controllers/accounts_controller.rb

@@ -12,10 +12,10 @@ class AccountsController < ApplicationController
   before_action :check_database_readable
   before_action :check_database_writable, :only => [:update]
 
-  allow_thirdparty_images :only => [:edit, :update]
+  allow_thirdparty_images :only => [:show, :update]
-  allow_social_login :only => [:edit, :update]
+  allow_social_login :only => [:show, :update]
 
-  def edit
+  def show
     if errors = session.delete(:user_errors)
       errors.each do |attribute, error|
         current_user.errors.add(attribute, error)
@@ -32,9 +32,9 @@ class AccountsController < ApplicationController
         params[:user][:auth_uid] == current_user.auth_uid)
       update_user(current_user, user_params)
       if current_user.errors.empty?
-        redirect_to edit_account_path
+        redirect_to account_path
       else
-        render :edit
+        render :show
       end
     else
       session[:new_user_settings] = user_params.to_h

app/controllers/confirmations_controller.rb

@@ -91,7 +91,7 @@ class ConfirmationsController < ApplicationController
         flash[:error] = t ".unknown_token"
       end
 
-      redirect_to edit_account_path
+      redirect_to account_path
     end
   end
 

app/controllers/user_mutes_controller.rb

@@ -16,7 +16,7 @@ class UserMutesController < ApplicationController
     @muted_users = current_user.muted_users
     @title = t ".title"
 
-    redirect_to edit_account_path unless @muted_users.any?
+    redirect_to account_path unless @muted_users.any?
   end
 
   def create

app/controllers/users_controller.rb

@@ -101,7 +101,7 @@ class UsersController < ApplicationController
     current_user.data_public = true
     current_user.save
     flash[:notice] = t ".flash success"
-    redirect_to edit_account_path
+    redirect_to account_path
   end
 
   ##
@@ -135,7 +135,7 @@ class UsersController < ApplicationController
 
       session[:user_errors] = current_user.errors.as_json
 
-      redirect_to edit_account_path
+      redirect_to account_path
     else
       user = User.find_by(:auth_provider => provider, :auth_uid => uid)
 

app/views/accounts/deletions/show.html.erb

@@ -40,4 +40,4 @@
   <button class="btn btn-secondary" disabled><%= t(".delete_account") %></button>
 <% end %>
 
-<%= link_to t(".cancel"), edit_account_path, :class => "btn btn-link" %>
+<%= link_to t(".cancel"), account_path, :class => "btn btn-link" %>

app/views/application/_auth_providers.html.erb

@@ -37,7 +37,7 @@
       </label>
       <%= hidden_field_tag("referer", params[:referer], :autocomplete => "off") %>
       <%= text_field_tag("openid_url", "https://", :tabindex => 20, :autocomplete => "on", :class => "form-control") %>
-      <span class="form-text text-body-secondary">(<a href="<%= t "accounts.edit.openid.link" %>" target="_new"><%= t "accounts.edit.openid.link text" %></a>)</span>
+      <span class="form-text text-body-secondary">(<a href="<%= t "accounts.show.openid.link" %>" target="_new"><%= t "accounts.show.openid.link text" %></a>)</span>
     </div>
 
     <%= submit_tag t(".openid_login_button"), :tabindex => 21, :class => "btn btn-primary" %>

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"), edit_account_path, :class => "nav-link #{'active' if %w[accounts deletions].include?(controller_name)}" %>
+      <%= link_to t(".account_settings"), account_path, :class => "nav-link #{'active' if %w[accounts deletions].include?(controller_name)}" %>
     </li>
     <li class="nav-item">
       <%= link_to t(".oauth2_applications"), oauth_applications_path, :class => "nav-link #{'active' if controller_name == 'oauth2_applications'}" %>

app/views/layouts/_header.html.erb

@@ -85,7 +85,7 @@
             <span class='badge count-number'><%= number_with_delimiter(current_user.new_messages.size) %></span>
           <% end %>
           <%= link_to t("users.show.my profile"), current_user, :class => "dropdown-item" %>
-          <%= link_to t("users.show.my_account"), edit_account_path, :class => "dropdown-item" %>
+          <%= link_to t("users.show.my_account"), account_path, :class => "dropdown-item" %>
           <%= link_to t("users.show.my_preferences"), preferences_path, :class => "dropdown-item" %>
           <div class="dropdown-divider"></div>
           <% if current_user.home_location? %>

app/views/site/_not_public_flash.erb

@@ -1,3 +1,3 @@
 <p><%= t ".not_public" %></p>
-<p><%= t ".not_public_description_html", :user_page => (link_to t(".user_page_link"), edit_account_path(:anchor => "public")) %></p>
+<p><%= t ".not_public_description_html", :user_page => (link_to t(".user_page_link"), account_path(:anchor => "public")) %></p>
 <p><%= t ".anon_edits_html", :link => link_to(t(".anon_edits_link_text"), t(".anon_edits_link")) %></p>

app/views/users/show.html.erb

@@ -30,7 +30,7 @@
               <span class='badge count-number'><%= number_with_delimiter(current_user.diary_comments.size) %></span>
             </li>
             <li>
-              <%= link_to t(".my_account"), edit_account_path %>
+              <%= link_to t(".my_account"), account_path %>
             </li>
 
             <% if current_user.blocks.exists? %>

config/locales/en.yml

@@ -244,7 +244,7 @@ en:
         comment: Comment
         full: Full note
   accounts:
-    edit:
+    show:
       title: "Edit account"
       my_account: My Account
       current email address: "Current Email Address"

config/routes.rb

@@ -297,10 +297,10 @@ OpenStreetMap::Application.routes.draw do
       resource :status, :only => :update
     end
   end
-  get "/user/:display_name/account", :to => redirect(:path => "/account/edit")
+  get "/user/:display_name/account", :to => redirect(:path => "/account")
   get "/user/:display_name/diary/comments(/:page)", :page => /[1-9][0-9]*/, :to => redirect(:path => "/user/%{display_name}/diary_comments")
 
-  resource :account, :only => [:edit, :update, :destroy] do
+  resource :account, :only => [:show, :update, :destroy] do
     scope :module => :accounts do
       resource :terms, :only => [:show, :update]
       resource :pd_declaration, :only => [:show, :create]
@@ -308,6 +308,7 @@ OpenStreetMap::Application.routes.draw do
       resource :home, :only => :show
     end
   end
+  get "/account/edit", :to => redirect(:path => "/account"), :as => nil
 
   resource :dashboard, :only => [:show]
   resource :preferences, :only => [:show, :update]

test/controllers/accounts/pd_declarations_controller_test.rb

@@ -42,7 +42,7 @@ module Accounts
 
       post account_pd_declaration_path
 
-      assert_redirected_to edit_account_path
+      assert_redirected_to account_path
       assert_nil flash[:notice]
       assert_equal "You didn't confirm that you consider your edits to be in the Public Domain.", flash[:warning]
 

test/controllers/accounts/terms_controller_test.rb

@@ -29,7 +29,7 @@ module Accounts
       session_for(user)
 
       get account_terms_path
-      assert_redirected_to edit_account_path
+      assert_redirected_to account_path
     end
 
     def test_show_not_seen_without_referer
@@ -53,7 +53,7 @@ module Accounts
       session_for(user)
 
       put account_terms_path, :params => { :read_ct => 1, :read_tou => 1 }
-      assert_redirected_to edit_account_path
+      assert_redirected_to account_path
       assert_equal "Thanks for accepting the new contributor terms!", flash[:notice]
 
       user.reload
@@ -82,8 +82,8 @@ module Accounts
       user = create(:user, :terms_seen => false, :terms_agreed => nil)
       session_for(user)
 
-      get edit_account_path
+      get account_path
-      assert_redirected_to account_terms_path(:referer => "/account/edit")
+      assert_redirected_to account_terms_path(:referer => account_path)
     end
   end
 end

test/controllers/accounts_controller_test.rb

@@ -5,16 +5,23 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
   # test all routes which lead to this controller
   def test_routes
     assert_routing(
-      { :path => "/account/edit", :method => :get },
+      { :path => "/account", :method => :get },
-      { :controller => "accounts", :action => "edit" }
+      { :controller => "accounts", :action => "show" }
     )
     assert_routing(
       { :path => "/account", :method => :put },
       { :controller => "accounts", :action => "update" }
     )
+    assert_routing(
+      { :path => "/account", :method => :delete },
+      { :controller => "accounts", :action => "destroy" }
+    )
+
+    get "/account/edit"
+    assert_redirected_to "/account"
   end
 
-  def test_account
+  def test_show_and_update
     # 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
@@ -24,14 +31,14 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
 
     # Make sure that you are redirected to the login page when
     # you are not logged in
-    get edit_account_path
+    get account_path
-    assert_redirected_to login_path(:referer => "/account/edit")
+    assert_redirected_to login_path(:referer => account_path)
 
     # Make sure we get the page when we are logged in as the right user
     session_for(user)
-    get edit_account_path
+    get account_path
     assert_response :success
-    assert_template :edit
+    assert_template :show
     assert_select "form#accountForm" do |form|
       assert_equal "post", form.attr("method").to_s
       assert_select "input[name='_method']", true
@@ -41,9 +48,9 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
     # Updating the description using GET should fail
     user.description = "new description"
     user.preferred_editor = "default"
-    get edit_account_path, :params => { :user => user.attributes }
+    get account_path, :params => { :user => user.attributes }
     assert_response :success
-    assert_template :edit
+    assert_template :show
     assert_not_equal user.description, User.find(user.id).description
 
     # Adding external authentication should redirect to the auth provider
@@ -56,7 +63,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
     new_attributes = user.attributes.dup.merge(:display_name => create(:user).display_name)
     patch account_path, :params => { :user => new_attributes }
     assert_response :success
-    assert_template :edit
+    assert_template :show
     assert_select ".alert-success", false
     assert_select "form#accountForm > div > input.is-invalid#user_display_name"
 
@@ -64,17 +71,17 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
     new_attributes = user.attributes.dup.merge(:display_name => create(:user).display_name.upcase)
     patch account_path, :params => { :user => new_attributes }
     assert_response :success
-    assert_template :edit
+    assert_template :show
     assert_select ".alert-success", false
     assert_select "form#accountForm > div > 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")
     patch account_path, :params => { :user => new_attributes }
-    assert_redirected_to edit_account_url
+    assert_redirected_to account_path
-    get edit_account_path
+    follow_redirect!
     assert_response :success
-    assert_template :edit
+    assert_template :show
     assert_select ".alert-success", /^User information updated successfully/
     assert_select "form#accountForm > div > input#user_display_name[value=?]", "new tester"
 
@@ -89,7 +96,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
       end
     end
     assert_response :success
-    assert_template :edit
+    assert_template :show
     assert_select ".alert-success", false
     assert_select "form#accountForm > div > input.is-invalid#user_new_email"
 
@@ -101,7 +108,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
       end
     end
     assert_response :success
-    assert_template :edit
+    assert_template :show
     assert_select ".alert-success", false
     assert_select "form#accountForm > div > input.is-invalid#user_new_email"
 
@@ -112,10 +119,10 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
         patch account_path, :params => { :user => user.attributes }
       end
     end
-    assert_redirected_to edit_account_url
+    assert_redirected_to account_path
-    get edit_account_path
+    follow_redirect!
     assert_response :success
-    assert_template :edit
+    assert_template :show
     assert_select ".alert-success", /^User information updated successfully/
     assert_select "form#accountForm > div > input#user_new_email[value=?]", user.new_email
     email = ActionMailer::Base.deliveries.first
@@ -123,19 +130,19 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
     assert_equal user.new_email, email.to.first
   end
 
-  def test_private_account
+  def test_show_private_account
     user = create(:user, :data_public => false)
 
     # Make sure that you are redirected to the login page when
     # you are not logged in
-    get edit_account_path
+    get account_path
-    assert_redirected_to login_path(:referer => "/account/edit")
+    assert_redirected_to login_path(:referer => account_path)
 
     # Make sure we get the page when we are logged in as the right user
     session_for(user)
-    get edit_account_path
+    get account_path
     assert_response :success
-    assert_template :edit
+    assert_template :show
     assert_select "form#accountForm" do |form|
       assert_equal "post", form.attr("method").to_s
       assert_select "input[name='_method']", true

test/controllers/confirmations_controller_test.rb

@@ -248,7 +248,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     confirm_string = user.generate_token_for(:new_email)
 
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
-    assert_redirected_to edit_account_path
+    assert_redirected_to account_path
     assert_match(/Confirmed your change of email address/, flash[:notice])
   end
 
@@ -257,13 +257,13 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     confirm_string = user.generate_token_for(:new_email)
 
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
-    assert_redirected_to edit_account_path
+    assert_redirected_to account_path
     assert_match(/already been confirmed/, flash[:error])
   end
 
   def test_confirm_email_bad_token
     post user_confirm_email_path, :params => { :confirm_string => "XXXXX" }
-    assert_redirected_to edit_account_path
+    assert_redirected_to account_path
     assert_match(/confirmation code has expired or does not exist/, flash[:error])
   end
 
@@ -279,7 +279,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     # precondition gravatar should be turned off
     assert_not user.image_use_gravatar
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
-    assert_redirected_to edit_account_path
+    assert_redirected_to 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
@@ -293,7 +293,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     # precondition gravatar should be turned on
     assert user.image_use_gravatar
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
-    assert_redirected_to edit_account_path
+    assert_redirected_to 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

@@ -195,7 +195,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     post user_go_public_path
 
-    assert_redirected_to edit_account_path
+    assert_redirected_to account_path
     assert User.find(user.id).data_public
   end
 
@@ -249,7 +249,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/traces']", 1
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/diary']", 1
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/diary_comments']", 1
-      assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/account']", 0
+      assert_select "a[href='/account']", 0
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/blocks']", 0
       assert_select "a[href='/user/#{ERB::Util.u(moderator_user.display_name)}/blocks_by']", 1
       assert_select "a[href='/user_blocks/new/#{ERB::Util.u(moderator_user.display_name)}']", 0
@@ -266,7 +266,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='/account/edit']", 1
+      assert_select "a[href='/account']", 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='/user_blocks/new/#{ERB::Util.u(user.display_name)}']", 0
@@ -284,7 +284,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='/account/edit']", 0
+      assert_select "a[href='/account']", 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='/user_blocks/new/#{ERB::Util.u(user.display_name)}']", 1

test/system/account_deletion_test.rb

@@ -7,7 +7,7 @@ class AccountDeletionTest < ApplicationSystemTestCase
   end
 
   test "the status is deleted and the personal data removed" do
-    visit edit_account_path
+    visit account_path
 
     click_on "Delete Account..."
     accept_confirm do
@@ -21,7 +21,7 @@ class AccountDeletionTest < ApplicationSystemTestCase
   end
 
   test "the user is signed out after deletion" do
-    visit edit_account_path
+    visit account_path
 
     click_on "Delete Account..."
     accept_confirm do
@@ -32,7 +32,7 @@ class AccountDeletionTest < ApplicationSystemTestCase
   end
 
   test "the user is shown a confirmation flash message" do
-    visit edit_account_path
+    visit account_path
 
     click_on "Delete Account..."
     accept_confirm do
@@ -45,7 +45,7 @@ class AccountDeletionTest < ApplicationSystemTestCase
   test "can delete with any delay setting value if the user has no changesets" do
     with_user_account_deletion_delay(10000) do
       travel 1.hour do
-        visit edit_account_path
+        visit account_path
 
         click_on "Delete Account..."
 
@@ -59,7 +59,7 @@ class AccountDeletionTest < ApplicationSystemTestCase
       create(:changeset, :user => @user)
 
       travel 1.hour do
-        visit edit_account_path
+        visit account_path
 
         click_on "Delete Account..."
 
@@ -73,7 +73,7 @@ class AccountDeletionTest < ApplicationSystemTestCase
       create(:changeset, :user => @user, :created_at => Time.now.utc, :closed_at => Time.now.utc + 1.hour)
 
       travel 12.hours do
-        visit edit_account_path
+        visit account_path
 
         click_on "Delete Account..."
 
@@ -87,7 +87,7 @@ class AccountDeletionTest < ApplicationSystemTestCase
       create(:changeset, :user => @user, :created_at => Time.now.utc, :closed_at => Time.now.utc + 1.hour)
 
       travel 10.hours do
-        visit edit_account_path
+        visit account_path
 
         click_on "Delete Account..."
 

test/system/user_email_change_test.rb

@@ -12,7 +12,7 @@ class UserEmailChangeTest < ApplicationSystemTestCase
     sign_in_as(user)
 
     assert_emails 1 do
-      visit edit_account_path
+      visit account_path
       fill_in "New Email Address", :with => "new_tester@example.com"
       click_on "Save Changes"
       assert_equal "new_tester@example.com", user.reload.new_email
@@ -25,7 +25,7 @@ class UserEmailChangeTest < ApplicationSystemTestCase
 
     if email.parts[0].parts[0].decoded =~ %r{(/user/confirm-email\?confirm_string=[A-Za-z0-9\-_%]+)\s}
       visit Regexp.last_match(1)
-      assert page.has_css?("body.accounts-edit")
+      assert page.has_css?("body.accounts-show")
     end
 
     assert_equal "new_tester@example.com", user.reload.email

test/system/user_muting_test.rb

@@ -13,7 +13,7 @@ class UserMutingTest < ApplicationSystemTestCase
     click_on "Mute this User"
     assert_content "You muted #{other_user.display_name}"
 
-    visit edit_account_path
+    visit account_path
     assert_content "Muted Users"
     click_on "Muted Users"
     assert_content "You have muted 1 User"
@@ -21,7 +21,7 @@ class UserMutingTest < ApplicationSystemTestCase
 
     assert_content "You unmuted #{other_user.display_name}"
     refute_content "Muted Users"
-    assert_current_path edit_account_path
+    assert_current_path account_path
   end
 
   test "messages sent by muted users are set `muted` and do not cause notification emails" do

test/system/user_suspension_test.rb

@@ -4,12 +4,12 @@ class UserSuspensionTest < ApplicationSystemTestCase
   test "User shown a message when suspended mid-session" do
     user = create(:user)
     sign_in_as(user)
-    visit edit_account_path
+    visit account_path
     assert_content "My Account"
 
     user.suspend!
 
-    visit edit_account_path
+    visit account_path
     assert_content "This decision will be reviewed by an administrator shortly"
   end
 end