Merge branch '41758-after-changing-username-url-still-redirects-to-old-route' into 'master'

Add confirmation modal to "Change username"

Closes #41758

See merge request gitlab-org/gitlab-ce!17405

app/assets/javascripts/profile/account/components/update_username.vue

+<script>
+import _ from 'underscore';
+import axios from '~/lib/utils/axios_utils';
+import GlModal from '~/vue_shared/components/gl_modal.vue';
+import { s__, sprintf } from '~/locale';
+import Flash from '~/flash';
+
+export default {
+  components: {
+    GlModal,
+  },
+  props: {
+    actionUrl: {
+      type: String,
+      required: true,
+    },
+    rootUrl: {
+      type: String,
+      required: true,
+    },
+    initialUsername: {
+      type: String,
+      required: true,
+    },
+  },
+  data() {
+    return {
+      isRequestPending: false,
+      username: this.initialUsername,
+      newUsername: this.initialUsername,
+    };
+  },
+  computed: {
+    path() {
+      return sprintf(s__('Profiles|Current path: %{path}'), {
+        path: `${this.rootUrl}${this.username}`,
+      });
+    },
+    modalText() {
+      return sprintf(
+        s__(`Profiles|
+You are going to change the username %{currentUsernameBold} to %{newUsernameBold}.
+Profile and projects will be redirected to the %{newUsername} namespace but this redirect will expire once the %{currentUsername} namespace is registered by another user or group.
+Please update your Git repository remotes as soon as possible.`),
+        {
+          currentUsernameBold: `<strong>${_.escape(this.username)}</strong>`,
+          newUsernameBold: `<strong>${_.escape(this.newUsername)}</strong>`,
+          currentUsername: _.escape(this.username),
+          newUsername: _.escape(this.newUsername),
+        },
+        false,
+      );
+    },
+  },
+  methods: {
+    onConfirm() {
+      this.isRequestPending = true;
+      const username = this.newUsername;
+      const putData = {
+        user: {
+          username,
+        },
+      };
+
+      return axios
+        .put(this.actionUrl, putData)
+        .then(result => {
+          Flash(result.data.message, 'notice');
+          this.username = username;
+          this.isRequestPending = false;
+        })
+        .catch(error => {
+          Flash(error.response.data.message);
+          this.isRequestPending = false;
+          throw error;
+        });
+    },
+  },
+  modalId: 'username-change-confirmation-modal',
+  inputId: 'username-change-input',
+  buttonText: s__('Profiles|Update username'),
+};
+</script>
+<template>
+  <div>
+    <div class="form-group">
+      <label :for="$options.inputId">{{ s__('Profiles|Path') }}</label>
+      <div class="input-group">
+        <div class="input-group-addon">{{ rootUrl }}</div>
+        <input
+          :id="$options.inputId"
+          class="form-control"
+          required="required"
+          v-model="newUsername"
+          :disabled="isRequestPending"
+        />
+      </div>
+      <p class="help-block">
+        {{ path }}
+      </p>
+    </div>
+    <button
+      :data-target="`#${$options.modalId}`"
+      class="btn btn-warning"
+      type="button"
+      data-toggle="modal"
+      :disabled="isRequestPending || newUsername === username"
+    >
+      {{ $options.buttonText }}
+    </button>
+    <gl-modal
+      :id="$options.modalId"
+      :header-title-text="s__('Profiles|Change username') + '?'"
+      footer-primary-button-variant="warning"
+      :footer-primary-button-text="$options.buttonText"
+      @submit="onConfirm"
+    >
+      <span v-html="modalText"></span>
+    </gl-modal>
+  </div>
+</template>

app/assets/javascripts/profile/account/index.js

 import Vue from 'vue';
 import Translate from '~/vue_shared/translate';
+import UpdateUsername from './components/update_username.vue';
 import deleteAccountModal from './components/delete_account_modal.vue';
 
 export default () => {
   Vue.use(Translate);
 
+  const updateUsernameElement = document.getElementById('update-username');
+  // eslint-disable-next-line no-new
+  new Vue({
+    el: updateUsernameElement,
+    components: {
+      UpdateUsername,
+    },
+    render(createElement) {
+      return createElement('update-username', {
+        props: { ...updateUsernameElement.dataset },
+      });
+    },
+  });
+
   const deleteAccountButton = document.getElementById('delete-account-button');
   const deleteAccountModalEl = document.getElementById('delete-account-modal');
   // eslint-disable-next-line no-new

app/assets/javascripts/vue_shared/components/gl_modal.vue

 <script>
-  const buttonVariants = [
-    'danger',
-    'primary',
-    'success',
-    'warning',
-  ];
+const buttonVariants = ['danger', 'primary', 'success', 'warning'];
 
-  export default {
+export default {
   name: 'GlModal',
 
   props: {
@@ -24,7 +19,7 @@
       type: String,
       required: false,
       default: 'primary',
-        validator: value => buttonVariants.indexOf(value) !== -1,
+      validator: value => buttonVariants.includes(value),
     },
     footerPrimaryButtonText: {
       type: String,
@@ -41,7 +36,7 @@
       this.$emit('submit', event);
     },
   },
-  };
+};
 </script>
 
 <template>
@@ -60,7 +55,7 @@
           <slot name="header">
             <button
               type="button"
-              class="close"
+              class="close js-modal-close-action"
               data-dismiss="modal"
               :aria-label="s__('Modal|Close')"
               @click="emitCancel($event)"
@@ -83,7 +78,7 @@
           <slot name="footer">
             <button
               type="button"
-              class="btn"
+              class="btn js-modal-cancel-action"
               data-dismiss="modal"
               @click="emitCancel($event)"
             >
@@ -91,7 +86,7 @@
             </button>
             <button
               type="button"
-              class="btn"
+              class="btn js-modal-primary-action"
               :class="`btn-${footerPrimaryButtonVariant}`"
               data-dismiss="modal"
               @click="emitSubmit($event)"

app/controllers/profiles_controller.rb

@@ -53,13 +53,19 @@ class ProfilesController < Profiles::ApplicationController
   def update_username
     result = Users::UpdateService.new(current_user, user: @user, username: username_param).execute
 
-    options = if result[:status] == :success
-                { notice: "Username successfully changed" }
+    respond_to do |format|
+      if result[:status] == :success
+        message = s_("Profiles|Username successfully changed")
+
+        format.html { redirect_back_or_default(default: { action: 'show' }, options: { notice: message }) }
+        format.json { render json: { message: message }, status: :ok }
       else
-                { alert: "Username change failed - #{result[:message]}" }
-              end
+        message = s_("Profiles|Username change failed - %{message}") % { message: result[:message] }
 
-    redirect_back_or_default(default: { action: 'show' }, options: options)
+        format.html { redirect_back_or_default(default: { action: 'show' }, options: { alert: message }) }
+        format.json { render json: { message: message }, status: :unprocessable_entity }
+      end
+    end
   end
 
   private

app/views/profiles/accounts/show.html.haml

@@ -57,20 +57,8 @@
         = succeed '.' do
           = link_to 'Learn more', help_page_path('user/profile/index', anchor: 'changing-your-username'), target: '_blank'
     .col-lg-8
-      = form_for @user, url: update_username_profile_path, method: :put, html: {class: "update-username"} do |f|
-        .form-group
-          = f.label :username, "Path", class: "label-light"
-          .input-group
-            .input-group-addon
-              = root_url
-            = f.text_field :username, required: true, class: 'form-control'
-        .help-block
-          Current path:
-          #{root_url}#{current_user.username}
-        .prepend-top-default
-          = f.button class: "btn btn-warning", type: "submit" do
-            = icon "spinner spin", class: "hidden loading-username"
-            Update username
+      - data = { initial_username: current_user.username, root_url: root_url, action_url: update_username_profile_path(format: :json) }
+      #update-username{ data: data }
   %hr
 
 .row.prepend-top-default

changelogs/unreleased/41758-after-changing-username-url-still-redirects-to-old-route.yml

+---
+title: Added confirmation modal for changing username
+merge_request: 17405
+author:
+type: added

spec/controllers/profiles_controller_spec.rb

@@ -84,6 +84,28 @@ describe ProfilesController, :request_store do
       expect(user.username).to eq(new_username)
     end
 
+    it 'updates a username using JSON request' do
+      sign_in(user)
+
+      put :update_username,
+          user: { username: new_username },
+          format: :json
+
+      expect(response.status).to eq(200)
+      expect(json_response['message']).to eq('Username successfully changed')
+    end
+
+    it 'renders an error message when the username was not updated' do
+      sign_in(user)
+
+      put :update_username,
+          user: { username: 'invalid username.git' },
+          format: :json
+
+      expect(response.status).to eq(422)
+      expect(json_response['message']).to match(/Username change failed/)
+    end
+
     it 'raises a correct error when the username is missing' do
       sign_in(user)
 

spec/features/profile_spec.rb

@@ -97,9 +97,13 @@ describe 'Profile account page', :js do
     end
 
     it 'changes my username' do
-      fill_in 'user_username', with: 'new-username'
+      fill_in 'username-change-input', with: 'new-username'
 
-      click_button('Update username')
+      page.find('[data-target="#username-change-confirmation-modal"]').click
+
+      page.within('.modal') do
+        find('.js-modal-primary-action').click
+      end
 
       expect(page).to have_content('new-username')
     end

spec/features/profiles/account_spec.rb

 require 'rails_helper'
 
-feature 'Profile > Account' do
+feature 'Profile > Account', :js do
   given(:user) { create(:user, username: 'foo') }
 
   before do
@@ -59,6 +59,12 @@ end
 def update_username(new_username)
   allow(user.namespace).to receive(:move_dir)
   visit profile_account_path
-  fill_in 'user_username', with: new_username
-  click_button 'Update username'
+
+  fill_in 'username-change-input', with: new_username
+
+  page.find('[data-target="#username-change-confirmation-modal"]').click
+
+  page.within('.modal') do
+    find('.js-modal-primary-action').click
+  end
 end

spec/javascripts/profile/account/components/update_username_spec.js

+import Vue from 'vue';
+import axios from '~/lib/utils/axios_utils';
+import MockAdapter from 'axios-mock-adapter';
+
+import updateUsername from '~/profile/account/components/update_username.vue';
+import mountComponent from 'spec/helpers/vue_mount_component_helper';
+
+describe('UpdateUsername component', () => {
+  const rootUrl = gl.TEST_HOST;
+  const actionUrl = `${gl.TEST_HOST}/update/username`;
+  const username = 'hasnoname';
+  const newUsername = 'new_username';
+  let Component;
+  let vm;
+  let axiosMock;
+
+  beforeEach(() => {
+    axiosMock = new MockAdapter(axios);
+    Component = Vue.extend(updateUsername);
+    vm = mountComponent(Component, {
+      actionUrl,
+      rootUrl,
+      initialUsername: username,
+    });
+  });
+
+  afterEach(() => {
+    vm.$destroy();
+    axiosMock.restore();
+  });
+
+  const findElements = () => {
+    const modalSelector = `#${vm.$options.modalId}`;
+
+    return {
+      input: vm.$el.querySelector(`#${vm.$options.inputId}`),
+      openModalBtn: vm.$el.querySelector(`[data-target="${modalSelector}"]`),
+      modal: vm.$el.querySelector(modalSelector),
+      modalBody: vm.$el.querySelector(`${modalSelector} .modal-body`),
+      modalHeader: vm.$el.querySelector(`${modalSelector} .modal-title`),
+      confirmModalBtn: vm.$el.querySelector(`${modalSelector} .btn-warning`),
+    };
+  };
+
+  it('has a disabled button if the username was not changed', done => {
+    const { input, openModalBtn } = findElements();
+    input.dispatchEvent(new Event('input'));
+
+    Vue.nextTick()
+      .then(() => {
+        expect(vm.username).toBe(username);
+        expect(vm.newUsername).toBe(username);
+        expect(openModalBtn).toBeDisabled();
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+
+  it('has an enabled button which if the username was changed', done => {
+    const { input, openModalBtn } = findElements();
+    input.value = newUsername;
+    input.dispatchEvent(new Event('input'));
+
+    Vue.nextTick()
+      .then(() => {
+        expect(vm.username).toBe(username);
+        expect(vm.newUsername).toBe(newUsername);
+        expect(openModalBtn).not.toBeDisabled();
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+
+  it('confirmation modal contains proper header and body', done => {
+    const { modalBody, modalHeader } = findElements();
+
+    vm.newUsername = newUsername;
+
+    Vue.nextTick()
+      .then(() => {
+        expect(modalHeader.textContent).toContain('Change username?');
+        expect(modalBody.textContent).toContain(
+          `You are going to change the username ${username} to ${newUsername}`,
+        );
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+
+  it('confirmation modal should escape usernames properly', done => {
+    const { modalBody } = findElements();
+
+    vm.username = vm.newUsername = '<i>Italic</i>';
+
+    Vue.nextTick()
+      .then(() => {
+        expect(modalBody.innerHTML).toContain('&lt;i&gt;Italic&lt;/i&gt;');
+        expect(modalBody.innerHTML).not.toContain(vm.username);
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+
+  it('executes API call on confirmation button click', done => {
+    const { confirmModalBtn } = findElements();
+
+    axiosMock.onPut(actionUrl).replyOnce(() => [200, { message: 'Username changed' }]);
+    spyOn(axios, 'put').and.callThrough();
+
+    vm.newUsername = newUsername;
+
+    Vue.nextTick()
+      .then(() => {
+        confirmModalBtn.click();
+        expect(axios.put).toHaveBeenCalledWith(actionUrl, { user: { username: newUsername } });
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+
+  it('sets the username after a successful update', done => {
+    const { input, openModalBtn } = findElements();
+
+    axiosMock.onPut(actionUrl).replyOnce(() => {
+      expect(input).toBeDisabled();
+      expect(openModalBtn).toBeDisabled();
+
+      return [200, { message: 'Username changed' }];
+    });
+
+    vm.newUsername = newUsername;
+
+    vm
+      .onConfirm()
+      .then(() => {
+        expect(vm.username).toBe(newUsername);
+        expect(vm.newUsername).toBe(newUsername);
+        expect(input).not.toBeDisabled();
+        expect(input.value).toBe(newUsername);
+        expect(openModalBtn).toBeDisabled();
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+
+  it('does not set the username after a erroneous update', done => {
+    const { input, openModalBtn } = findElements();
+
+    axiosMock.onPut(actionUrl).replyOnce(() => {
+      expect(input).toBeDisabled();
+      expect(openModalBtn).toBeDisabled();
+
+      return [400, { message: 'Invalid username' }];
+    });
+
+    const invalidUsername = 'anything.git';
+    vm.newUsername = invalidUsername;
+
+    vm
+      .onConfirm()
+      .then(() => done.fail('Expected onConfirm to throw!'))
+      .catch(() => {
+        expect(vm.username).toBe(username);
+        expect(vm.newUsername).toBe(invalidUsername);
+        expect(input).not.toBeDisabled();
+        expect(input.value).toBe(invalidUsername);
+        expect(openModalBtn).not.toBeDisabled();
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+});