From 5dfb746cbe054c51b9a8595e5a286f356f4b6a9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saray=20Cabrera=20Padr=C3=B3n?= Date: Fri, 16 Oct 2020 16:25:15 +0200 Subject: [PATCH] Allow inline edition for user profile page When the authentication is handled by a proxy, biography is the only attribute editable. Otherwise, biography, name and email can be edited. Co-authored-by: Oleksandr Orlov --- .../app/controllers/webui/users_controller.rb | 62 +++++++++++++------ .../views/webui/user/_edit_account.html.haml | 10 ++- .../_basic_info.html.haml | 20 ++++++ .../_edit_account_form.html.haml | 28 +++++++++ .../user_profile_redesign/_info.html.haml | 33 +++------- .../app/views/webui/users/edit_account.js.erb | 10 +++ src/api/app/views/webui/users/show.js.erb | 8 +++ src/api/app/views/webui/users/update.js.haml | 19 ++++++ src/api/config/routes/webui_routes.rb | 1 + .../webui/users_controller_spec.rb | 2 +- 10 files changed, 144 insertions(+), 49 deletions(-) create mode 100644 src/api/app/views/webui/user/user_profile_redesign/_basic_info.html.haml create mode 100644 src/api/app/views/webui/user/user_profile_redesign/_edit_account_form.html.haml create mode 100644 src/api/app/views/webui/users/edit_account.js.erb create mode 100644 src/api/app/views/webui/users/show.js.erb create mode 100644 src/api/app/views/webui/users/update.js.haml diff --git a/src/api/app/controllers/webui/users_controller.rb b/src/api/app/controllers/webui/users_controller.rb index 4c3a6d5358a..cad830ba5fc 100644 --- a/src/api/app/controllers/webui/users_controller.rb +++ b/src/api/app/controllers/webui/users_controller.rb @@ -1,7 +1,9 @@ class Webui::UsersController < Webui::WebuiController - before_action :require_login, only: [:index, :edit, :destroy, :update, :change_password] + before_action :require_login, only: [:index, :edit, :destroy, :update, :change_password, :edit_account] before_action :require_admin, only: [:index, :edit, :destroy] - before_action :check_displayed_user, only: [:show, :edit, :update] + before_action :check_displayed_user, only: [:show, :edit, :update, :edit_account] + before_action :role_titles, only: [:show, :edit_account, :update] + before_action :account_edit_link, only: [:show, :edit_account, :update] def index respond_to do |format| @@ -15,8 +17,6 @@ def show @ipackages = @displayed_user.involved_packages.joins(:project).pluck(:name, 'projects.name as pname') @owned = @displayed_user.owned_packages @groups = @displayed_user.groups - @role_titles = @displayed_user.roles.global.pluck(:title) - @account_edit_link = CONFIG['proxy_auth_account_page'] return if CONFIG['contribution_graph'] == :off @@ -70,6 +70,12 @@ def destroy def edit; end + def edit_account + respond_to do |format| + format.js + end + end + def update unless User.admin_session? if User.session! != @displayed_user || !@configuration.accounts_editable?(@displayed_user) @@ -79,24 +85,21 @@ def update end end - if @configuration.accounts_editable?(@displayed_user) - @displayed_user.assign_attributes(params[:user].slice(:realname, :email, :biography).permit!) - @displayed_user.toggle(:in_beta) if params[:user][:in_beta] - end + assign_common_user_attributes if @configuration.accounts_editable?(@displayed_user) + assign_admin_attributes if User.admin_session? - if User.admin_session? - @displayed_user.assign_attributes(params[:user].slice(:state, :ignore_auth_services).permit!) - @displayed_user.update_globalroles(Role.global.where(id: params[:user][:role_ids])) unless params[:user][:role_ids].nil? - end - - begin - @displayed_user.save! - flash[:success] = "User data for user '#{@displayed_user.login}' successfully updated." - rescue ActiveRecord::RecordInvalid => e - flash[:error] = "Couldn't update user: #{e.message}." + respond_to do |format| + if @displayed_user.save + message = "User data for user '#{@displayed_user.login}' successfully updated." + format.html { flash[:success] = message } + format.js { flash.now[:success] = message } + else + message = "Couldn't update user: #{@displayed_user.errors.full_messages.to_sentence}." + format.html { flash[:error] = message } + format.js { flash.now[:error] = message } + end + redirect_back(fallback_location: user_path(@displayed_user)) if request.format.symbol == :html end - - redirect_back(fallback_location: user_path(@displayed_user)) end def autocomplete @@ -143,4 +146,23 @@ def create_params email: params[:email] } end + + def role_titles + @role_titles = @displayed_user.roles.global.pluck(:title) + end + + def account_edit_link + @account_edit_link = CONFIG['proxy_auth_account_page'] + end + + def assign_common_user_attributes + @displayed_user.assign_attributes(params[:user].slice(:biography).permit!) + @displayed_user.assign_attributes(params[:user].slice(:realname, :email).permit!) unless @account_edit_link + @displayed_user.toggle(:in_beta) if params[:user][:in_beta] + end + + def assign_admin_attributes + @displayed_user.assign_attributes(params[:user].slice(:state, :ignore_auth_services).permit!) + @displayed_user.update_globalroles(Role.global.where(id: params[:user][:role_ids])) unless params[:user][:role_ids].nil? + end end diff --git a/src/api/app/views/webui/user/_edit_account.html.haml b/src/api/app/views/webui/user/_edit_account.html.haml index 9fd2c3af0b6..e1eb4cfe997 100644 --- a/src/api/app/views/webui/user/_edit_account.html.haml +++ b/src/api/app/views/webui/user/_edit_account.html.haml @@ -1,6 +1,12 @@ - if feature_enabled?(:responsive_ux) - content_for :actions do - - if account_edit_link.present? + - if feature_enabled?(:user_profile_redesign) + -# Behind a proxy or not, with this Feature Flag `user_profile_redesign` we go to the edit account form. + %li.nav-item + = link_to(edit_account_user_path(user), class: 'nav-link', remote: true, title: 'Edit Your Account') do + %i.fas.fa-lg.mr-2.fa-user-edit + %span.nav-item-name Edit Your Account + - elsif account_edit_link.present? %li.nav-item = link_to(account_edit_link, class: 'nav-link', title: 'Edit Your Account') do %i.fas.fa-lg.mr-2.fa-user-edit @@ -10,7 +16,7 @@ = link_to('#', data: { toggle: 'modal', target: '#edit-modal' }, class: 'nav-link', title: 'Edit Your Account') do %i.fas.fa-lg.mr-2.fa-user-edit %span.nav-item-name Edit Your Account -- elsif account_edit_link.present? +- elsif account_edit_link.present? # Feature Flag `responsive_ux` is disabled = link_to(account_edit_link, class: 'd-block') do %i.fas.fa-user-edit Edit your account diff --git a/src/api/app/views/webui/user/user_profile_redesign/_basic_info.html.haml b/src/api/app/views/webui/user/user_profile_redesign/_basic_info.html.haml new file mode 100644 index 00000000000..6853900814b --- /dev/null +++ b/src/api/app/views/webui/user/user_profile_redesign/_basic_info.html.haml @@ -0,0 +1,20 @@ +.basic-info{ class: feature_enabled?(:user_profile_redesign) ? 'in-place-editing' : '' } + .row.mb-3.mb-md-0 + .col-4.col-sm-2.col-md-12.text-center + = image_tag_for(user, size: 200) + .col-8.col-sm-10.col-md-12.pl-0.p-md-3 + %h4#home-realname + = user.realname + %h5.text-muted#home-login + = user.login + + - role_titles.each do |title| + %span.badge.badge-secondary + = title.upcase + %p= render_as_markdown(user.biography) + + .mt-4 + - if User.session + = mail_to(user.email, title: "Send mail to #{user.name}", class: 'd-block') do + %i.fas.fa-envelope.mr-1 + = user.email diff --git a/src/api/app/views/webui/user/user_profile_redesign/_edit_account_form.html.haml b/src/api/app/views/webui/user/user_profile_redesign/_edit_account_form.html.haml new file mode 100644 index 00000000000..6def33f6116 --- /dev/null +++ b/src/api/app/views/webui/user/user_profile_redesign/_edit_account_form.html.haml @@ -0,0 +1,28 @@ +- behind_proxy = account_edit_link.present? + += form_for(displayed_user, url: user_path(displayed_user), method: :patch, remote: true) do |f| + = f.hidden_field(:login, id: 'user') + + .mb-3.text-center= image_tag_for(displayed_user, size: 200) + .form-group + = f.text_field(:realname, readonly: behind_proxy, placeholder: 'Name', class: 'form-control') + .form-group + = f.text_field(:login, readonly: true, class: 'form-control') + .form-group + - role_titles.each do |title| + %span.badge.badge-secondary + = title.upcase + .form-group + = f.text_area(:biography, rows: 6, placeholder: 'Biography', maxlength: User::MAX_BIOGRAPHY_LENGTH_ALLOWED, class: 'form-control') + = f.label(:biography, class: 'd-block text-right') do + %small.form-text Max. #{User::MAX_BIOGRAPHY_LENGTH_ALLOWED} characters. + .form-group + = f.text_field(:email, required: true, email: true, placeholder: 'Email', readonly: behind_proxy, class: 'form-control') + .form-group + - if behind_proxy + %p + You are behind a proxy. You can modify other data related to your profile by + = link_to(' this link.', account_edit_link) + .form-group.text-right + = link_to 'Cancel', user_path(displayed_user), class: 'cancel btn btn-outline-danger px-4', remote: true + = submit_tag('Update', class: 'btn btn-primary px-4') diff --git a/src/api/app/views/webui/user/user_profile_redesign/_info.html.haml b/src/api/app/views/webui/user/user_profile_redesign/_info.html.haml index 6b5c96d6f57..3baf99b1a21 100644 --- a/src/api/app/views/webui/user/user_profile_redesign/_info.html.haml +++ b/src/api/app/views/webui/user/user_profile_redesign/_info.html.haml @@ -1,31 +1,12 @@ .card.mb-3 .card-body - .row.mb-3.mb-md-0 - .col-4.col-sm-2.col-md-12.text-center - = image_tag_for(user, size: 200) - .col-8.col-sm-10.col-md-12.pl-0.p-md-3 - %h4#home-realname - = user.realname - %h5.text-muted#home-login - = user.login + = render partial: 'webui/user/user_profile_redesign/basic_info', locals: { user: user, role_titles: role_titles } - - role_titles.each do |title| - %span.badge.badge-secondary - = title.upcase - - %p= render_as_markdown(user.biography) - - .mt-4 - - if User.session - = mail_to(user.email, title: "Send mail to #{user.name}", class: 'd-block') do - %i.fas.fa-envelope.mr-1 - = user.email - - - if user.rss_token && is_user - = link_to(user_rss_notifications_path(token: user.rss_token.string, format: 'rss'), - title: 'RSS Feed for Notifications', class: 'd-block') do - %i.fas.fa-rss.mr-1 - RSS for Notifications + - if user.rss_token && is_user + = link_to(user_rss_notifications_path(token: user.rss_token.string, format: 'rss'), + title: 'RSS Feed for Notifications', class: 'd-block') do + %i.fas.fa-rss.mr-1 + RSS for Notifications - if groups.any? .h5.mt-4.mb-0 Member of the #{'group'.pluralize(groups.size)} @@ -50,7 +31,7 @@ .mt-4 - if configuration.accounts_editable?(user) - = render partial: 'webui/user/edit_account', locals: { account_edit_link: account_edit_link } + = render partial: 'webui/user/edit_account', locals: { account_edit_link: account_edit_link, user: user } - if configuration.passwords_changable?(user) = render partial: 'webui/user/change_password' - if feature_enabled?(:responsive_ux) diff --git a/src/api/app/views/webui/users/edit_account.js.erb b/src/api/app/views/webui/users/edit_account.js.erb new file mode 100644 index 00000000000..cc26378e216 --- /dev/null +++ b/src/api/app/views/webui/users/edit_account.js.erb @@ -0,0 +1,10 @@ +$('#flash').empty(); +$('.in-place-editing').animate({ + opacity: 0.25 +}, 400, function() { + scrollToInPlace(); + $('.in-place-editing').html("<%= escape_javascript(render(partial: 'webui/user/user_profile_redesign/edit_account_form', locals: { displayed_user: @displayed_user, role_titles: @role_titles, account_edit_link: @account_edit_link})) %>"); + $('.in-place-editing').animate({ opacity: 1 }, 400, function() { + $("form *[autofocus='autofocus']").focus(); + }); +}); diff --git a/src/api/app/views/webui/users/show.js.erb b/src/api/app/views/webui/users/show.js.erb new file mode 100644 index 00000000000..e6893078bc2 --- /dev/null +++ b/src/api/app/views/webui/users/show.js.erb @@ -0,0 +1,8 @@ +$('.in-place-editing').animate({ + opacity: 0.25 +}, 400, function() { + scrollToInPlace(); + $('.in-place-editing').html("<%= escape_javascript(render(partial: 'webui/user/user_profile_redesign/basic_info', locals: { user: @displayed_user, role_titles: @role_titles })) %>"); + setCollapsible(); + $('.in-place-editing').animate({ opacity: 1 }, 400); +}); diff --git a/src/api/app/views/webui/users/update.js.haml b/src/api/app/views/webui/users/update.js.haml new file mode 100644 index 00000000000..cf8908988e0 --- /dev/null +++ b/src/api/app/views/webui/users/update.js.haml @@ -0,0 +1,19 @@ +resetFormValidation(); +- if @displayed_user.errors.any? + - @displayed_user.errors.messages.each do |field, messages| + element = $("##{@displayed_user.class.name.underscore}_#{field}"); // Create strings like "user_email" + setFormValidation(element, "#{messages.to_sentence}"); +- else + - locals = { user: @displayed_user, role_titles: @role_titles } + :plain + $('.in-place-editing').animate({ + opacity: 0.25 + }, 400, function() { + scrollToInPlace(); + $('.in-place-editing').html( + "#{escape_javascript(render(partial: 'webui/user/user_profile_redesign/basic_info', locals: locals))}"); + setCollapsible(); + $('.in-place-editing').animate({ opacity: 1 }, 400, function() { + $('#flash').html("#{escape_javascript(render(layout: false, partial: 'layouts/webui/flash', object: flash))}"); + }); + }); diff --git a/src/api/config/routes/webui_routes.rb b/src/api/config/routes/webui_routes.rb index a63ab53feb8..fdc203a73dc 100644 --- a/src/api/config/routes/webui_routes.rb +++ b/src/api/config/routes/webui_routes.rb @@ -300,6 +300,7 @@ end member do post 'change_password' + get 'edit_account' end end diff --git a/src/api/spec/controllers/webui/users_controller_spec.rb b/src/api/spec/controllers/webui/users_controller_spec.rb index cec7f80eaeb..a8d4919c1bb 100644 --- a/src/api/spec/controllers/webui/users_controller_spec.rb +++ b/src/api/spec/controllers/webui/users_controller_spec.rb @@ -176,7 +176,7 @@ user.reload end - it { expect(flash[:error]).to eq("Couldn't update user: Validation failed: Email must be a valid email address.") } + it { expect(flash[:error]).to eq("Couldn't update user: Email must be a valid email address.") } it { expect(user.realname).to eq(user.realname) } it { expect(user.email).to eq(user.email) } it { expect(user.state).to eq('confirmed') }