Merge pull request #10321 from saraycp/inline-edit-user-profile

Allow inline edition for user profile page

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

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

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

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')

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)

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();
+  });
+});

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);
+});

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))}");
+      });
+    });

src/api/config/routes/webui_routes.rb

@@ -300,6 +300,7 @@
       end
       member do
         post 'change_password'
+        get 'edit_account'
       end
     end
 

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') }