Move profile-related settings to their own form

Refs #3167

app/abilities/ability.rb

@@ -47,6 +47,7 @@ class Ability
         can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message
         can [:close, :reopen], Note
         can [:show, :edit, :update], :preference
+        can [:edit, :update], :profile
         can [:new, :create], Report
         can [:mine, :new, :create, :edit, :update, :destroy], Trace
         can [:account, :go_public], User

app/controllers/confirmations_controller.rb

@@ -129,9 +129,9 @@ class ConfirmationsController < ApplicationController
   # display a message about th current status of the gravatar setting
   def gravatar_status_message(user)
     if user.image_use_gravatar
-      t "users.account.gravatar.enabled"
+      t "profiles.edit.gravatar.enabled"
     else
-      t "users.account.gravatar.disabled"
+      t "profiles.edit.gravatar.disabled"
     end
   end
 end

app/controllers/profiles_controller.rb

@@ -0,0 +1,43 @@
+class ProfilesController < ApplicationController
+  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]
+
+  def edit; end
+
+  def update
+    if params[:user][:description] != current_user.description
+      current_user.description = params[:user][:description]
+      current_user.description_format = "markdown"
+    end
+
+    case params[:avatar_action]
+    when "new"
+      current_user.avatar.attach(params[:user][:avatar])
+      current_user.image_use_gravatar = false
+    when "delete"
+      current_user.avatar.purge_later
+      current_user.image_use_gravatar = false
+    when "gravatar"
+      current_user.avatar.purge_later
+      current_user.image_use_gravatar = true
+    end
+
+    current_user.home_lat = params[:user][:home_lat]
+    current_user.home_lon = params[:user][:home_lon]
+
+    if current_user.save
+      flash[:notice] = t ".success"
+      redirect_to user_path(current_user)
+    else
+      flash[:error] = t ".failure"
+      render :edit
+    end
+  end
+end

app/controllers/users_controller.rb

@@ -363,26 +363,6 @@ class UsersController < ApplicationController
       user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation]
     end
 
-    if params[:user][:description] != user.description
-      user.description = params[:user][:description]
-      user.description_format = "markdown"
-    end
-
-    case params[:avatar_action]
-    when "new"
-      user.avatar.attach(params[:user][:avatar])
-      user.image_use_gravatar = false
-    when "delete"
-      user.avatar.purge_later
-      user.image_use_gravatar = false
-    when "gravatar"
-      user.avatar.purge_later
-      user.image_use_gravatar = true
-    end
-
-    user.home_lat = params[:user][:home_lat]
-    user.home_lon = params[:user][:home_lon]
-
     if params[:user][:auth_provider].nil? || params[:user][:auth_provider].blank?
       user.auth_provider = nil
       user.auth_uid = nil

app/views/profiles/edit.html.erb

@@ -0,0 +1,61 @@
+<% content_for :head do %>
+  <%= javascript_include_tag "user" %>
+<% end %>
+
+<% content_for :heading do %>
+  <h1><%= t ".title" %></h1>
+<% end %>
+
+<%= bootstrap_form_for current_user, :url => { :action => :update }, :html => { :multipart => true, :autocomplete => :off } do |f| %>
+  <%= f.richtext_field :description, :cols => 80, :rows => 20 %>
+
+  <fieldset class="form-group">
+    <%= f.label t(".image") %>
+    <div class="form-row">
+      <div class="col-sm-2">
+        <%= user_image current_user %>
+      </div>
+      <div class="col-sm-10">
+        <% if current_user.avatar.attached? %>
+          <%= f.radio_button "avatar_action", "keep", :name => "avatar_action", :label => t(".keep image"), :checked => !current_user.image_use_gravatar %>
+        <% end %>
+        <% if current_user.avatar.attached? || current_user.image_use_gravatar? %>
+          <%= f.radio_button "avatar_action", "delete", :name => "avatar_action", :label => t(".delete image"), :checked => false %>
+        <% end %>
+        <% if current_user.avatar.attached? %>
+          <%= f.form_group :help => t(".image size hint"), :class => "mb-0" do %>
+            <%= f.radio_button "avatar_action", "new", :name => "avatar_action", :label => t(".replace image"), :checked => false %>
+            <%= f.file_field :avatar, :hide_label => true, :wrapper => { :class => "mb-0" } %>
+          <% end %>
+        <% else %>
+          <%= f.form_group :help => t(".image size hint"), :class => "mb-0" do %>
+            <%= f.radio_button "avatar_action", "new", :name => "avatar_action", :label => t(".new image"), :checked => false %>
+            <%= f.file_field :avatar, :hide_label => true, :wrapper => { :class => "mb-0" } %>
+          <% end %>
+        <% end %>
+        <%= f.form_group :help => link_to(t(".gravatar.what_is_gravatar"), t(".gravatar.link")) do %>
+          <%= f.radio_button "avatar_action", "gravatar", :name => "avatar_action", :label => t(".gravatar.gravatar"), :checked => current_user.image_use_gravatar %>
+        <% end %>
+      </div>
+    </div>
+  </fieldset>
+
+  <fieldset>
+    <legend><%= t ".home location" -%></legend>
+    <div id="homerow" <% unless current_user.home_lat and current_user.home_lon %> class="nohome"<% end %>>
+      <p class="message text-muted"><%= t ".no home location" %></p>
+      <div class="form-row">
+        <%= f.text_field :home_lat, :wrapper_class => "col-sm-4", :id => "home_lat" %>
+        <%= f.text_field :home_lon, :wrapper_class => "col-sm-4", :id => "home_lon" %>
+      </div>
+    </div>
+    <div class="form-check">
+      <input class="form-check-input" type="checkbox" name="updatehome" value="1" <% unless current_user.home_lat and current_user.home_lon %> checked="checked" <% end %> id="updatehome" />
+      <label class="form-check-label" for="updatehome"><%= t ".update home location on click" %></label>
+    </div>
+    <%= tag.div "", :id => "map", :class => "content_map set_location" %>
+  </fieldset>
+
+  <%= f.primary t(".save") %>
+    <%= link_to t(".cancel"), user_path(current_user), :class => "btn btn-link" %>
+<% end %>

app/views/users/account.html.erb

@@ -58,55 +58,6 @@
     </span>
   </div>
 
-  <%= f.richtext_field :description, :cols => 80, :rows => 20 %>
-
-  <fieldset class="form-group">
-    <%= f.label t(".image") %>
-    <div class="form-row">
-      <div class="col-sm-2">
-        <%= user_image current_user %>
-      </div>
-      <div class="col-sm-10">
-        <% if current_user.avatar.attached? %>
-          <%= f.radio_button "avatar_action", "keep", :name => "avatar_action", :label => t(".keep image"), :checked => !current_user.image_use_gravatar %>
-        <% end %>
-        <% if current_user.avatar.attached? || current_user.image_use_gravatar? %>
-          <%= f.radio_button "avatar_action", "delete", :name => "avatar_action", :label => t(".delete image"), :checked => false %>
-        <% end %>
-        <% if current_user.avatar.attached? %>
-          <%= f.form_group :help => t(".image size hint"), :class => "mb-0" do %>
-            <%= f.radio_button "avatar_action", "new", :name => "avatar_action", :label => t(".replace image"), :checked => false %>
-            <%= f.file_field :avatar, :hide_label => true, :wrapper => { :class => "mb-0" } %>
-          <% end %>
-        <% else %>
-          <%= f.form_group :help => t(".image size hint"), :class => "mb-0" do %>
-            <%= f.radio_button "avatar_action", "new", :name => "avatar_action", :label => t(".new image"), :checked => false %>
-            <%= f.file_field :avatar, :hide_label => true, :wrapper => { :class => "mb-0" } %>
-          <% end %>
-        <% end %>
-        <%= f.form_group :help => link_to(t(".gravatar.what_is_gravatar"), t(".gravatar.link")) do %>
-          <%= f.radio_button "avatar_action", "gravatar", :name => "avatar_action", :label => t(".gravatar.gravatar"), :checked => current_user.image_use_gravatar %>
-        <% end %>
-      </div>
-    </div>
-  </fieldset>
-
-  <fieldset>
-    <legend><%= t ".home location" -%></legend>
-    <div id="homerow" <% unless current_user.home_lat and current_user.home_lon %> class="nohome"<% end %>>
-      <p class="message text-muted"><%= t ".no home location" %></p>
-      <div class="form-row">
-        <%= f.text_field :home_lat, :wrapper_class => "col-sm-4", :id => "home_lat" %>
-        <%= f.text_field :home_lon, :wrapper_class => "col-sm-4", :id => "home_lon" %>
-      </div>
-    </div>
-    <div class="form-check">
-      <input class="form-check-input" type="checkbox" name="updatehome" value="1" <% unless current_user.home_lat and current_user.home_lon %> checked="checked" <% end %> id="updatehome" />
-      <label class="form-check-label" for="updatehome"><%= t ".update home location on click" %></label>
-    </div>
-    <%= tag.div "", :id => "map", :class => "content_map set_location" %>
-  </fieldset>
-
   <%= f.primary t(".save changes button") %>
 <% end %>
 

app/views/users/show.html.erb

@@ -165,6 +165,12 @@
 
     <div class="user-description richtext text-break"><%= @user.description.to_html %></div>
 
+    <% if current_user and @user.id == current_user.id %>
+      <div class="my-3">
+        <%= link_to t(".edit_profile"), edit_profile_path, :class => "btn btn-outline-primary" %>
+      </div>
+    <% end %>
+
   </div>
 
   <% if current_user and current_user.administrator? -%>

config/locales/en.yml

@@ -1657,6 +1657,29 @@ en:
     update:
       success: Preferences updated.
       failure: Couldn't update preferences.
+  profiles:
+    edit:
+      title: Edit Profile
+      save: Update Profile
+      cancel: Cancel
+      image: Image
+      gravatar:
+        gravatar: "Use Gravatar"
+        link: "https://wiki.openstreetmap.org/wiki/Gravatar"
+        what_is_gravatar: "What is Gravatar?"
+        disabled: "Gravatar has been disabled."
+        enabled: "Display of your Gravatar has been enabled."
+      new image: "Add an image"
+      keep image: "Keep the current image"
+      delete image: "Remove the current image"
+      replace image: "Replace the current image"
+      image size hint: "(square images at least 100x100 work best)"
+      home location: "Home Location"
+      no home location: "You have not entered your home location."
+      update home location on click: "Update home location when I click on the map?"
+    update:
+      success: Profile updated.
+      failure: Couldn't update profile.
   sessions:
     new:
       title: "Login"
@@ -2461,6 +2484,7 @@ en:
       my_preferences: My Preferences
       blocks on me: Blocks on Me
       blocks by me: Blocks by Me
+      edit_profile: Edit Profile
       send message: Send Message
       diary: Diary
       edits: Edits
@@ -2542,21 +2566,6 @@ en:
         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?"
-      image: Image
-      gravatar:
-        gravatar: "Use Gravatar"
-        link: "https://wiki.openstreetmap.org/wiki/Gravatar"
-        what_is_gravatar: "What is Gravatar?"
-        disabled: "Gravatar has been disabled."
-        enabled: "Display of your Gravatar has been enabled."
-      new image: "Add an image"
-      keep image: "Keep the current image"
-      delete image: "Remove the current image"
-      replace image: "Replace the current image"
-      image size hint: "(square images at least 100x100 work best)"
-      home location: "Home Location"
-      no home location: "You have not entered your home location."
-      update home location on click: "Update home location when I click on the map?"
       save changes button: Save Changes
       make edits public button: Make all my edits public
       return to profile: Return to profile

config/routes.rb

@@ -241,6 +241,7 @@ OpenStreetMap::Application.routes.draw do
   post "/user/:display_name/set_status" => "users#set_status", :as => :set_status_user
 
   resource :preferences, :only => [:show, :edit, :update]
+  resource :profile, :only => [:edit, :update]
 
   # friendships
   match "/user/:display_name/make_friend" => "friendships#make_friend", :via => [:get, :post], :as => "make_friend"

test/controllers/profiles_controller_test.rb

@@ -0,0 +1,67 @@
+require "test_helper"
+
+class ProfilesControllerTest < ActionDispatch::IntegrationTest
+  ##
+  # test all routes which lead to this controller
+  def test_routes
+    assert_routing(
+      { :path => "/profile/edit", :method => :get },
+      { :controller => "profiles", :action => "edit" }
+    )
+
+    assert_routing(
+      { :path => "/profile", :method => :put },
+      { :controller => "profiles", :action => "update" }
+    )
+  end
+
+  def test_update
+    user = create(:user)
+    session_for(user)
+
+    # Updating the description should work
+    put profile_path, :params => { :user => { :description => "new description" } }
+    assert_response :redirect
+    assert_redirected_to user_path(user)
+    follow_redirect!
+    assert_response :success
+    assert_template :show
+    assert_select ".notice", /^Profile updated./
+    assert_select "div", "new description"
+
+    # Changing to an uploaded image should work
+    image = Rack::Test::UploadedFile.new("test/gpx/fixtures/a.gif", "image/gif")
+    put profile_path, :params => { :avatar_action => "new", :user => { :avatar => image, :description => user.description } }
+    assert_response :redirect
+    assert_redirected_to user_path(user)
+    follow_redirect!
+    assert_response :success
+    assert_template :show
+    assert_select ".notice", /^Profile updated./
+    get edit_profile_path
+    assert_select "form > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-check > input[name=avatar_action][checked][value=?]", "keep"
+
+    # Changing to a gravatar image should work
+    put profile_path, :params => { :avatar_action => "gravatar", :user => { :description => user.description } }
+    assert_response :redirect
+    assert_redirected_to user_path(user)
+    follow_redirect!
+    assert_response :success
+    assert_template :show
+    assert_select ".notice", /^Profile updated./
+    get edit_profile_path
+    assert_select "form > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-group > div.form-check > input[name=avatar_action][checked][value=?]", "gravatar"
+
+    # Removing the image should work
+    put profile_path, :params => { :avatar_action => "delete", :user => { :description => user.description } }
+    assert_response :redirect
+    assert_redirected_to user_path(user)
+    follow_redirect!
+    assert_response :success
+    assert_template :show
+    assert_select ".notice", /^Profile updated./
+    get edit_profile_path
+    assert_select "form > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-check > input[name=avatar_action][checked]", false
+    assert_select "form > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-group > div.form-check > input[name=avatar_action][checked]", false
+  end
+end

test/controllers/users_controller_test.rb

@@ -463,50 +463,6 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     assert_template :account
     assert_not_equal user.description, User.find(user.id).description
 
-    # Updating the description should work
-    user.description = "new description"
-    user.preferred_editor = "default"
-    post user_account_path(user), :params => { :user => user.attributes }
-    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 > div#user_description_container > div#user_description_content > textarea#user_description", user.description
-
-    # Changing to an uploaded image should work
-    image = Rack::Test::UploadedFile.new("test/gpx/fixtures/a.gif", "image/gif")
-    post user_account_path(user), :params => { :avatar_action => "new", :user => user.attributes.merge(:avatar => image) }
-    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 > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-check > input[name=avatar_action][checked][value=?]", "keep"
-
-    # Changing to a gravatar image should work
-    post user_account_path(user), :params => { :avatar_action => "gravatar", :user => user.attributes }
-    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 > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-group > div.form-check > input[name=avatar_action][checked][value=?]", "gravatar"
-
-    # Removing the image should work
-    post user_account_path(user), :params => { :avatar_action => "delete", :user => user.attributes }
-    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 > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-check > input[name=avatar_action][checked]", false
-    assert_select "form#accountForm > fieldset.form-group > div.form-row > div.col-sm-10 > div.form-group > div.form-check > input[name=avatar_action][checked]", false
-
     # 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