From a900435d58296680fb7384a63011d51c7d8edfaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Wed, 13 Mar 2019 17:44:07 +0100 Subject: [PATCH 1/8] Sketch ChangePassword component + spec --- components/ChangePassword.spec.js | 72 +++++++++++++++++++++++++++++++ components/ChangePassword.vue | 2 + 2 files changed, 74 insertions(+) create mode 100644 components/ChangePassword.spec.js create mode 100644 components/ChangePassword.vue diff --git a/components/ChangePassword.spec.js b/components/ChangePassword.spec.js new file mode 100644 index 000000000..fda0e3f42 --- /dev/null +++ b/components/ChangePassword.spec.js @@ -0,0 +1,72 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils' +import ChangePassword from './ChangePassword.vue' +import Vue from 'vue' +import Styleguide from '@human-connection/styleguide' + +const localVue = createLocalVue() + +localVue.use(Styleguide) + +describe('ChangePassword.vue', () => { + let store + let mocks + let wrapper + + beforeEach(() => { + mocks = { + $apollo: { + mutate: jest.fn().mockResolvedValue() + } + } + }) + + describe('shallowMount', () => { + const Wrapper = () => { + return shallowMount(ChangePassword, { mocks, localVue }) + } + + it.todo('renders') + + describe('validations', () => { + it.todo('is disabled') + + describe('old password and new password', () => { + describe('match', () => { + it.todo('invalid') + it.todo('displays a warning') + }) + }) + + describe('new password and confirmation', () => { + describe('mismatch', () => { + it.todo('invalid') + it.todo('displays a warning') + }) + + describe('match', () => { + describe('and old password mismatch', () => { + it.todo('valid') + }) + + describe('clicked', () => { + it.todo('sets loading') + }) + }) + }) + }) + + describe('given valid input', () => { + describe('click on submit button', () => { + it.todo('calls changePassword mutation') + + describe('mutation resolves', () => { + it.todo('calls auth/SET_TOKEN with response') + }) + + describe('mutation rejects', () => { + it.todo('displays error message') + }) + }) + }) + }) +}) diff --git a/components/ChangePassword.vue b/components/ChangePassword.vue new file mode 100644 index 000000000..6beff5199 --- /dev/null +++ b/components/ChangePassword.vue @@ -0,0 +1,2 @@ + From 51a1678a38448923f6f10e4df82df760c15ab482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Wed, 13 Mar 2019 17:56:55 +0100 Subject: [PATCH 2/8] [ChangePassword] Implement disabled property --- components/ChangePassword.spec.js | 13 +++++++++++-- components/ChangePassword.vue | 11 +++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/components/ChangePassword.spec.js b/components/ChangePassword.spec.js index fda0e3f42..36ad3fdce 100644 --- a/components/ChangePassword.spec.js +++ b/components/ChangePassword.spec.js @@ -21,14 +21,23 @@ describe('ChangePassword.vue', () => { }) describe('shallowMount', () => { + let wrapper const Wrapper = () => { return shallowMount(ChangePassword, { mocks, localVue }) } - it.todo('renders') + beforeEach(() => { + wrapper = Wrapper() + }) + + it('renders', () => { + expect(wrapper.is('div')).toBe(true) + }) describe('validations', () => { - it.todo('is disabled') + it('invalid', () => { + expect(wrapper.vm.disabled).toBe(true) + }) describe('old password and new password', () => { describe('match', () => { diff --git a/components/ChangePassword.vue b/components/ChangePassword.vue index 6beff5199..9ef85b3de 100644 --- a/components/ChangePassword.vue +++ b/components/ChangePassword.vue @@ -1,2 +1,13 @@ + + From e0432b2fd9e3b662979842a181227c540ee9cca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Wed, 13 Mar 2019 18:56:34 +0100 Subject: [PATCH 3/8] Styleguide blocks development cc @appinteractive We're trying to cross-validate two form fields and don't know how. We (ie. @kachulio1, @aonomike, myself) gave up after we discovered https://github.com/Human-Connection/Nitro-Styleguide/issues/46 --- components/ChangePassword.spec.js | 29 ++++++++++++---- components/ChangePassword.vue | 54 +++++++++++++++++++++++++++++- webapp/pages/settings/security.vue | 8 ++--- 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/components/ChangePassword.spec.js b/components/ChangePassword.spec.js index 36ad3fdce..a6be60880 100644 --- a/components/ChangePassword.spec.js +++ b/components/ChangePassword.spec.js @@ -1,4 +1,4 @@ -import { shallowMount, createLocalVue } from '@vue/test-utils' +import { mount, createLocalVue } from '@vue/test-utils' import ChangePassword from './ChangePassword.vue' import Vue from 'vue' import Styleguide from '@human-connection/styleguide' @@ -14,24 +14,25 @@ describe('ChangePassword.vue', () => { beforeEach(() => { mocks = { + $t: jest.fn(), $apollo: { mutate: jest.fn().mockResolvedValue() } } }) - describe('shallowMount', () => { + describe('mount', () => { let wrapper const Wrapper = () => { - return shallowMount(ChangePassword, { mocks, localVue }) + return mount(ChangePassword, { mocks, localVue }) } beforeEach(() => { wrapper = Wrapper() }) - it('renders', () => { - expect(wrapper.is('div')).toBe(true) + it('renders three input fields', () => { + expect(wrapper.findAll('input')).toHaveLength(3) }) describe('validations', () => { @@ -41,8 +42,22 @@ describe('ChangePassword.vue', () => { describe('old password and new password', () => { describe('match', () => { - it.todo('invalid') - it.todo('displays a warning') + beforeEach(() => { + wrapper.find('input#oldPassword').setValue('some secret') + wrapper.find('input#newPassword').setValue('some secret') + }) + + it('invalid', () => { + expect(wrapper.vm.disabled).toBe(true) + }) + + it.skip('displays a warning', () => { + const calls = mocks.$t.mock.calls + const expected = [ + ['change-password.validations.old-and-new-password-match'] + ] + expect(calls).toEqual(expect.arrayContaining(expected)) + }) }) }) diff --git a/components/ChangePassword.vue b/components/ChangePassword.vue index 9ef85b3de..0a3510091 100644 --- a/components/ChangePassword.vue +++ b/components/ChangePassword.vue @@ -1,13 +1,65 @@ diff --git a/webapp/pages/settings/security.vue b/webapp/pages/settings/security.vue index 937aac9dd..376f104e5 100644 --- a/webapp/pages/settings/security.vue +++ b/webapp/pages/settings/security.vue @@ -1,18 +1,16 @@ From eb2552c9a902699495ff7d0b148480dee7763c04 Mon Sep 17 00:00:00 2001 From: Matt Rider Date: Wed, 13 Mar 2019 19:23:24 -0300 Subject: [PATCH 4/8] Add cypress test, update variables - change to confirmPassword to be more consistent with oldPassword, newPassword - change to validate, $t is used for a function for translation --- components/ChangePassword.spec.js | 4 +-- components/ChangePassword.vue | 8 +++--- cypress/integration/ChangePassword.feature | 14 +++++++++++ cypress/integration/common/settings.js | 29 ++++++++++++++++++++++ 4 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 cypress/integration/ChangePassword.feature diff --git a/components/ChangePassword.spec.js b/components/ChangePassword.spec.js index a6be60880..3b8e6791b 100644 --- a/components/ChangePassword.spec.js +++ b/components/ChangePassword.spec.js @@ -14,7 +14,7 @@ describe('ChangePassword.vue', () => { beforeEach(() => { mocks = { - $t: jest.fn(), + validate: jest.fn(), $apollo: { mutate: jest.fn().mockResolvedValue() } @@ -52,7 +52,7 @@ describe('ChangePassword.vue', () => { }) it.skip('displays a warning', () => { - const calls = mocks.$t.mock.calls + const calls = mocks.validate.mock.calls const expected = [ ['change-password.validations.old-and-new-password-match'] ] diff --git a/components/ChangePassword.vue b/components/ChangePassword.vue index 0a3510091..f6e5237a0 100644 --- a/components/ChangePassword.vue +++ b/components/ChangePassword.vue @@ -19,8 +19,8 @@ label="Your new password" /> @@ -41,12 +41,12 @@ export default { formData: { oldPassword: '', newPassword: '', - passwordConfirmation: '' + confirmPassword: '' }, formSchema: { oldPassword: { required: true }, newPassword: { required: true }, - passwordConfirmation: { required: true } + confirmPassword: { required: true } }, disabled: true } diff --git a/cypress/integration/ChangePassword.feature b/cypress/integration/ChangePassword.feature new file mode 100644 index 000000000..cecfaeb84 --- /dev/null +++ b/cypress/integration/ChangePassword.feature @@ -0,0 +1,14 @@ +Feature: Change password + As a user + I want to change my password in my settings + Because this is a basic security feature, e.g. if I exposed my password by accident + + Background: + Given I have a user account + And I am logged in + And I am on the "settings" page + + Scenario: Change my password + Given I click on the "Security" link + Then I should be on the "Security" settings page + And I should be able to change my password \ No newline at end of file diff --git a/cypress/integration/common/settings.js b/cypress/integration/common/settings.js index 3aa6022a8..9bf620024 100644 --- a/cypress/integration/common/settings.js +++ b/cypress/integration/common/settings.js @@ -61,3 +61,32 @@ Then( 'I can see my new name {string} when I click on my profile picture in the top right', name => matchNameInUserMenu(name) ) + +When('I click on the {string} link', link => { + cy.get('a') + .contains(link) + .click() +}) + +Then('I should be on the {string} settings page', page => { + const pathname = `/settings/${page.toLowerCase()}` + cy.location() + .should(loc => { + expect(loc.pathname).to.eq(pathname) + }) + .get('h3') + .should('contain', page) +}) + +Then('I should be able to change my password', () => { + cy.get('input[id=oldPassword]') + .type('1234') + .get('input[id=newPassword]') + .type('12345') + .get('input[id=confirmPassword]') + .type('12345') + .get('button') + .contains('Submit') + .get('.iziToast-message') + .should('contain', 'Password updated successfully.') +}) From 26ab3b9bbe79a4d63677a1c47ce9c8ae7a1a3f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Fri, 22 Mar 2019 16:03:34 +0100 Subject: [PATCH 5/8] Oops, moved the components to a wrong folder --- {components => webapp/components}/ChangePassword.spec.js | 0 {components => webapp/components}/ChangePassword.vue | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename {components => webapp/components}/ChangePassword.spec.js (100%) rename {components => webapp/components}/ChangePassword.vue (100%) diff --git a/components/ChangePassword.spec.js b/webapp/components/ChangePassword.spec.js similarity index 100% rename from components/ChangePassword.spec.js rename to webapp/components/ChangePassword.spec.js diff --git a/components/ChangePassword.vue b/webapp/components/ChangePassword.vue similarity index 100% rename from components/ChangePassword.vue rename to webapp/components/ChangePassword.vue From 8827add1c7271051e2c94f51c303aa87670b5e53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Fri, 22 Mar 2019 20:01:01 +0100 Subject: [PATCH 6/8] Improve :cucumber: *must* fail if not implemented cc @mattwr18 --- cypress/integration/ChangePassword.feature | 14 ------- cypress/integration/common/settings.js | 29 ------------- cypress/integration/common/steps.js | 42 ++++++++++++++++++- .../settings/ChangePassword.feature | 31 ++++++++++++++ 4 files changed, 72 insertions(+), 44 deletions(-) delete mode 100644 cypress/integration/ChangePassword.feature create mode 100644 cypress/integration/settings/ChangePassword.feature diff --git a/cypress/integration/ChangePassword.feature b/cypress/integration/ChangePassword.feature deleted file mode 100644 index cecfaeb84..000000000 --- a/cypress/integration/ChangePassword.feature +++ /dev/null @@ -1,14 +0,0 @@ -Feature: Change password - As a user - I want to change my password in my settings - Because this is a basic security feature, e.g. if I exposed my password by accident - - Background: - Given I have a user account - And I am logged in - And I am on the "settings" page - - Scenario: Change my password - Given I click on the "Security" link - Then I should be on the "Security" settings page - And I should be able to change my password \ No newline at end of file diff --git a/cypress/integration/common/settings.js b/cypress/integration/common/settings.js index 9bf620024..3aa6022a8 100644 --- a/cypress/integration/common/settings.js +++ b/cypress/integration/common/settings.js @@ -61,32 +61,3 @@ Then( 'I can see my new name {string} when I click on my profile picture in the top right', name => matchNameInUserMenu(name) ) - -When('I click on the {string} link', link => { - cy.get('a') - .contains(link) - .click() -}) - -Then('I should be on the {string} settings page', page => { - const pathname = `/settings/${page.toLowerCase()}` - cy.location() - .should(loc => { - expect(loc.pathname).to.eq(pathname) - }) - .get('h3') - .should('contain', page) -}) - -Then('I should be able to change my password', () => { - cy.get('input[id=oldPassword]') - .type('1234') - .get('input[id=newPassword]') - .type('12345') - .get('input[id=confirmPassword]') - .type('12345') - .get('button') - .contains('Submit') - .get('.iziToast-message') - .should('contain', 'Password updated successfully.') -}) diff --git a/cypress/integration/common/steps.js b/cypress/integration/common/steps.js index eeb3a49d3..726aca86c 100644 --- a/cypress/integration/common/steps.js +++ b/cypress/integration/common/steps.js @@ -5,7 +5,7 @@ import { getLangByName } from '../../support/helpers' let lastPost = {} -const loginCredentials = { +let loginCredentials = { email: 'peterpan@example.org', password: '1234' } @@ -244,3 +244,43 @@ Then( cy.get('.error').should('contain', message) } ) + +Given('my user account has the following login credentials:', table => { + loginCredentials = { + ...loginCredentials, + ...table.hashes()[0] + } + cy.factory().create('User', { + ...loginCredentials + }) +}) + +When('I fill the password form with:', table => { + table = table.rowsHash() + cy.get('input[id=oldPassword]') + .type(table['Your old password']) + .get('input[id=newPassword]') + .type(table['Your new passsword']) + .get('input[id=confirmPassword]') + .type(table['Confirm new password']) +}) + +When('submit the form', () => { + cy.get('form').submit() +}) + +Then('I cannot login anymore with password {string}', password => { + cy.login({ + ...loginCredentials, + ...{password} + }) + cy.get('.iziToast-wrapper').should('contain', "Incorrect email or password") +}) + +Then('I can login successfully with password {string}', password => { + cy.login({ + ...loginCredentials, + ...{password} + }) + cy.get('.iziToast-wrapper').should('contain', "You are logged in!") +}) diff --git a/cypress/integration/settings/ChangePassword.feature b/cypress/integration/settings/ChangePassword.feature new file mode 100644 index 000000000..936bbed74 --- /dev/null +++ b/cypress/integration/settings/ChangePassword.feature @@ -0,0 +1,31 @@ +Feature: Change password + As a user + I want to change my password in my settings + For security, e.g. if I exposed my password by accident + + Login via email and password is a well-known authentication procedure and you + can assure to the server that you are who you claim to be. Either if you + exposed your password by acccident and you want to invalidate the exposed + password or just out of an good habit, you want to change your password. + + Background: + Given my user account has the following login credentials: + | email | passsword | + | user@example.org | 1234 | + And I am logged in + + Scenario: Change my password + Given I am on the "settings" page + And I click on "Security" + When I fill the password form with: + | Your old password | 1234 | + | Your new passsword | 12345 | + | Confirm new password | 12345 | + And submit the form + And I see a success message: + """ + Password updated successfully + """ + And I log out through the menu in the top right corner + Then I cannot login anymore with password "1234" + But I can login successfully with password "12345" From 22367417de9eeb892685e9b2074361befd0c6714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Sat, 23 Mar 2019 01:37:36 +0100 Subject: [PATCH 7/8] Implement component test --- webapp/components/ChangePassword.spec.js | 70 ++++++++++++++++++++++-- webapp/components/ChangePassword.vue | 38 +++++++++---- webapp/locales/de.json | 6 +- webapp/locales/en.json | 6 +- 4 files changed, 102 insertions(+), 18 deletions(-) diff --git a/webapp/components/ChangePassword.spec.js b/webapp/components/ChangePassword.spec.js index 3b8e6791b..98a66da72 100644 --- a/webapp/components/ChangePassword.spec.js +++ b/webapp/components/ChangePassword.spec.js @@ -8,15 +8,25 @@ const localVue = createLocalVue() localVue.use(Styleguide) describe('ChangePassword.vue', () => { - let store let mocks let wrapper beforeEach(() => { mocks = { validate: jest.fn(), + $toast: { + error: jest.fn(), + success: jest.fn() + }, + $t: jest.fn(), + $store: { + commit: jest.fn() + }, $apollo: { - mutate: jest.fn().mockResolvedValue() + mutate: jest + .fn() + .mockRejectedValue({ message: 'Ouch!' }) + .mockResolvedValueOnce({ data: { changePassword: 'NEWTOKEN' } }) } } }) @@ -80,15 +90,63 @@ describe('ChangePassword.vue', () => { }) describe('given valid input', () => { - describe('click on submit button', () => { - it.todo('calls changePassword mutation') + beforeEach(() => { + wrapper.find('input#oldPassword').setValue('supersecret') + wrapper.find('input#newPassword').setValue('superdupersecret') + wrapper.find('input#confirmPassword').setValue('superdupersecret') + }) + + describe('submit form', () => { + beforeEach(() => { + wrapper.find('form').trigger('submit') + }) + + it('calls changePassword mutation', () => { + expect(mocks.$apollo.mutate).toHaveBeenCalled() + }) + + it('passes form data as variables', () => { + expect(mocks.$apollo.mutate.mock.calls[0][0]).toEqual( + expect.objectContaining({ + variables: { + oldPassword: 'supersecret', + newPassword: 'superdupersecret', + confirmPassword: 'superdupersecret' + } + }) + ) + }) describe('mutation resolves', () => { - it.todo('calls auth/SET_TOKEN with response') + beforeEach(() => { + mocks.$apollo.mutate = jest.fn().mockResolvedValue() + wrapper = Wrapper() + }) + + it('calls auth/SET_TOKEN with response', () => { + expect(mocks.$store.commit).toHaveBeenCalledWith( + 'auth/SET_TOKEN', + 'NEWTOKEN' + ) + }) + + it('displays success message', () => { + expect(mocks.$t).toHaveBeenCalledWith( + 'settings.security.change-password.success' + ) + expect(mocks.$toast.success).toHaveBeenCalled() + }) }) describe('mutation rejects', () => { - it.todo('displays error message') + beforeEach(() => { + // second call will reject + wrapper.find('form').trigger('submit') + }) + + it('displays error message', () => { + expect(mocks.$toast.error).toHaveBeenCalledWith('Ouch!') + }) }) }) }) diff --git a/webapp/components/ChangePassword.vue b/webapp/components/ChangePassword.vue index f6e5237a0..dc8cc09da 100644 --- a/webapp/components/ChangePassword.vue +++ b/webapp/components/ChangePassword.vue @@ -3,7 +3,6 @@ v-model="formData" :schema="formSchema" @submit="handleSubmit" - @input="validate" > @@ -34,6 +36,8 @@