diff options
10 files changed, 400 insertions, 66 deletions
diff --git a/app/assets/javascripts/profile/account/components/update_username.vue b/app/assets/javascripts/profile/account/components/update_username.vue new file mode 100644 index 00000000000..e5de3f69b01 --- /dev/null +++ b/app/assets/javascripts/profile/account/components/update_username.vue @@ -0,0 +1,121 @@ +<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> diff --git a/app/assets/javascripts/profile/account/index.js b/app/assets/javascripts/profile/account/index.js index 84049a1f0b7..59c13e1a042 100644 --- a/app/assets/javascripts/profile/account/index.js +++ b/app/assets/javascripts/profile/account/index.js @@ -1,10 +1,25 @@ 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 diff --git a/app/assets/javascripts/vue_shared/components/gl_modal.vue b/app/assets/javascripts/vue_shared/components/gl_modal.vue index 67c9181c7b1..f28e5e2715d 100644 --- a/app/assets/javascripts/vue_shared/components/gl_modal.vue +++ b/app/assets/javascripts/vue_shared/components/gl_modal.vue @@ -1,47 +1,42 @@ <script> - const buttonVariants = [ - 'danger', - 'primary', - 'success', - 'warning', - ]; +const buttonVariants = ['danger', 'primary', 'success', 'warning']; - export default { - name: 'GlModal', +export default { + name: 'GlModal', - props: { - id: { - type: String, - required: false, - default: null, - }, - headerTitleText: { - type: String, - required: false, - default: '', - }, - footerPrimaryButtonVariant: { - type: String, - required: false, - default: 'primary', - validator: value => buttonVariants.indexOf(value) !== -1, - }, - footerPrimaryButtonText: { - type: String, - required: false, - default: '', - }, + props: { + id: { + type: String, + required: false, + default: null, }, + headerTitleText: { + type: String, + required: false, + default: '', + }, + footerPrimaryButtonVariant: { + type: String, + required: false, + default: 'primary', + validator: value => buttonVariants.includes(value), + }, + footerPrimaryButtonText: { + type: String, + required: false, + default: '', + }, + }, - methods: { - emitCancel(event) { - this.$emit('cancel', event); - }, - emitSubmit(event) { - this.$emit('submit', event); - }, + methods: { + emitCancel(event) { + this.$emit('cancel', event); + }, + emitSubmit(event) { + 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)" diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 3d27ae18b17..ac71f72e624 100644 --- a/app/controllers/profiles_controller.rb +++ b/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" } - else - { alert: "Username change failed - #{result[:message]}" } - end + respond_to do |format| + if result[:status] == :success + message = s_("Profiles|Username successfully changed") - redirect_back_or_default(default: { action: 'show' }, options: options) + format.html { redirect_back_or_default(default: { action: 'show' }, options: { notice: message }) } + format.json { render json: { message: message }, status: :ok } + else + message = s_("Profiles|Username change failed - %{message}") % { message: result[:message] } + + 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 diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 02263095599..9c95b6281ba 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/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 diff --git a/changelogs/unreleased/41758-after-changing-username-url-still-redirects-to-old-route.yml b/changelogs/unreleased/41758-after-changing-username-url-still-redirects-to-old-route.yml new file mode 100644 index 00000000000..36e79ea1ed4 --- /dev/null +++ b/changelogs/unreleased/41758-after-changing-username-url-still-redirects-to-old-route.yml @@ -0,0 +1,5 @@ +--- +title: Added confirmation modal for changing username +merge_request: 17405 +author: +type: added diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index 891485406c6..de6ef919221 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/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) diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb index 0848857ed1e..15dcb30cbdd 100644 --- a/spec/features/profile_spec.rb +++ b/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 diff --git a/spec/features/profiles/account_spec.rb b/spec/features/profiles/account_spec.rb index e8eb0d17ca4..215b658eb7b 100644 --- a/spec/features/profiles/account_spec.rb +++ b/spec/features/profiles/account_spec.rb @@ -1,6 +1,6 @@ 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 diff --git a/spec/javascripts/profile/account/components/update_username_spec.js b/spec/javascripts/profile/account/components/update_username_spec.js new file mode 100644 index 00000000000..bac306edf5a --- /dev/null +++ b/spec/javascripts/profile/account/components/update_username_spec.js @@ -0,0 +1,172 @@ +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('<i>Italic</i>'); + 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); + }); +}); |