From caf2e2a242182f2c0dac78fc3f2f39b3fbbc8299 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 14 Jul 2021 17:39:09 +0100 Subject: [PATCH] Move profile-related settings to their own form Refs #3167 --- app/abilities/ability.rb | 1 + app/controllers/confirmations_controller.rb | 4 +- app/controllers/profiles_controller.rb | 43 +++++++++++++ app/controllers/users_controller.rb | 20 ------ app/views/profiles/edit.html.erb | 61 ++++++++++++++++++ app/views/users/account.html.erb | 49 -------------- app/views/users/show.html.erb | 6 ++ config/locales/en.yml | 39 +++++++----- config/routes.rb | 1 + test/controllers/profiles_controller_test.rb | 67 ++++++++++++++++++++ test/controllers/users_controller_test.rb | 44 ------------- 11 files changed, 205 insertions(+), 130 deletions(-) create mode 100644 app/controllers/profiles_controller.rb create mode 100644 app/views/profiles/edit.html.erb create mode 100644 test/controllers/profiles_controller_test.rb diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index b77f68be7..018ce0096 100644 --- a/app/abilities/ability.rb +++ b/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 diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index 372ac2a70..7b1c52ca6 100644 --- a/app/controllers/confirmations_controller.rb +++ b/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 diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb new file mode 100644 index 000000000..b48d08ad2 --- /dev/null +++ b/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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 1f3eb2f7a..ec30eb4e7 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb new file mode 100644 index 000000000..8eca6f4f7 --- /dev/null +++ b/app/views/profiles/edit.html.erb @@ -0,0 +1,61 @@ +<% content_for :head do %> + <%= javascript_include_tag "user" %> +<% end %> + +<% content_for :heading do %> +

<%= t ".title" %>

+<% end %> + +<%= bootstrap_form_for current_user, :url => { :action => :update }, :html => { :multipart => true, :autocomplete => :off } do |f| %> + <%= f.richtext_field :description, :cols => 80, :rows => 20 %> + +
+ <%= f.label t(".image") %> +
+
+ <%= user_image current_user %> +
+
+ <% 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 %> +
+
+
+ +
+ <%= t ".home location" -%> +
class="nohome"<% end %>> +

<%= t ".no home location" %>

+
+ <%= 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" %> +
+
+
+ checked="checked" <% end %> id="updatehome" /> + +
+ <%= tag.div "", :id => "map", :class => "content_map set_location" %> +
+ + <%= f.primary t(".save") %> + <%= link_to t(".cancel"), user_path(current_user), :class => "btn btn-link" %> +<% end %> diff --git a/app/views/users/account.html.erb b/app/views/users/account.html.erb index f1c46c710..26d840210 100644 --- a/app/views/users/account.html.erb +++ b/app/views/users/account.html.erb @@ -58,55 +58,6 @@ - <%= f.richtext_field :description, :cols => 80, :rows => 20 %> - -
- <%= f.label t(".image") %> -
-
- <%= user_image current_user %> -
-
- <% 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 %> -
-
-
- -
- <%= t ".home location" -%> -
class="nohome"<% end %>> -

<%= t ".no home location" %>

-
- <%= 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" %> -
-
-
- checked="checked" <% end %> id="updatehome" /> - -
- <%= tag.div "", :id => "map", :class => "content_map set_location" %> -
- <%= f.primary t(".save changes button") %> <% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 302ec5bd3..3974a2f9c 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -165,6 +165,12 @@
<%= @user.description.to_html %>
+ <% if current_user and @user.id == current_user.id %> +
+ <%= link_to t(".edit_profile"), edit_profile_path, :class => "btn btn-outline-primary" %> +
+ <% end %> + <% if current_user and current_user.administrator? -%> diff --git a/config/locales/en.yml b/config/locales/en.yml index 74d68263c..1ef7db9c4 100644 --- a/config/locales/en.yml +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index b68882dbf..dc10818b9 100644 --- a/config/routes.rb +++ b/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" diff --git a/test/controllers/profiles_controller_test.rb b/test/controllers/profiles_controller_test.rb new file mode 100644 index 000000000..38e73a03f --- /dev/null +++ b/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 diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 6eebd7f1a..a73e48211 100644 --- a/test/controllers/users_controller_test.rb +++ b/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 -- 2.39.5