From 165b29649952d0e18e955b631c6cd759e74c2d9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 26 Feb 2019 15:40:55 +0100 Subject: [PATCH 01/12] Configure jest to e verbose --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index cb3bda594..df4e9ff66 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "nonGlobalStepDefinitions": true }, "jest": { + "verbose": true, "moduleFileExtensions": [ "js", "json", From 71ab2f3828492c52fcb4f7a0cd066650276f197d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 26 Feb 2019 15:43:03 +0100 Subject: [PATCH 02/12] Get rid of JWT_SECRET once and for all * refactor store/auth.js not to use `delete Object` * refactor store/auth.js to have less redundancy * implement fetchCurrentUser without knowing the id beforehand * test fetchCurrentUser and init --- .env.template | 1 - docker-compose.yml | 1 - store/auth.js | 74 +++++++++++---------------- store/auth.test.js | 123 ++++++++++++++++++++++++++++++++++++++++----- 4 files changed, 141 insertions(+), 58 deletions(-) diff --git a/.env.template b/.env.template index 4313645bb..1fa2a542a 100644 --- a/.env.template +++ b/.env.template @@ -1,2 +1 @@ -JWT_SECRET="b/&&7b78BF&fv/Vd" MAPBOX_TOKEN="pk.eyJ1IjoiaHVtYW4tY29ubmVjdGlvbiIsImEiOiJjajl0cnBubGoweTVlM3VwZ2lzNTNud3ZtIn0.KZ8KK9l70omjXbEkkbHGsQ" diff --git a/docker-compose.yml b/docker-compose.yml index 64b743698..3b09fa57d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,7 +14,6 @@ services: environment: - HOST=0.0.0.0 - BACKEND_URL=http://backend:4000 - - JWT_SECRET=b/&&7b78BF&fv/Vd - MAPBOX_TOKEN="pk.eyJ1IjoiaHVtYW4tY29ubmVjdGlvbiIsImEiOiJjajl0cnBubGoweTVlM3VwZ2lzNTNud3ZtIn0.bZ8KK9l70omjXbEkkbHGsQ" networks: diff --git a/store/auth.js b/store/auth.js index a6ab3b3f8..b1b029468 100644 --- a/store/auth.js +++ b/store/auth.js @@ -57,56 +57,44 @@ export const actions = { if (!token) { return } - - const payload = await jwt.verify(token, process.env.JWT_SECRET) - if (!payload.id) { - return - } commit('SET_TOKEN', token) - commit('SET_USER', { - id: payload.id - }) await dispatch('fetchCurrentUser') }, + async check({ commit, dispatch, getters }) { if (!this.app.$apolloHelpers.getToken()) { await dispatch('logout') } return getters.isLoggedIn }, - async fetchCurrentUser({ commit, getters }) { - await this.app.apolloProvider.defaultClient - .query({ - query: gql(` - query User($id: ID!) { - User(id: $id) { - id - name - slug - email - avatar - role - locationName - about - } + async fetchCurrentUser({ commit }) { + const client = this.app.apolloProvider.defaultClient + const { + data: { currentUser } + } = await client.query({ + query: gql(`{ + currentUser { + id + name + slug + email + avatar + role } - `), - variables: { id: getters.user.id } - }) - .then(({ data }) => { - const user = data.User.pop() - if (user.id && user.email) { - commit('SET_USER', user) - } - }) - return getters.user + }`) + }) + const { token, ...user } = currentUser + commit('SET_USER', user) + return user }, async login({ commit }, { email, password }) { commit('SET_PENDING', true) try { - const res = await this.app.apolloProvider.defaultClient - .mutate({ - mutation: gql(` + const client = this.app.apolloProvider.defaultClient + const { + data: { login } + } = await client.mutate({ + mutation: gql(` mutation($email: String!, $password: String!) { login(email: $email, password: $password) { id @@ -121,15 +109,13 @@ export const actions = { } } `), - variables: { email, password } - }) - .then(({ data }) => data && data.login) + variables: { email, password } + }) + const { token, ...user } = login - await this.app.$apolloHelpers.onLogin(res.token) - commit('SET_TOKEN', res.token) - const userData = Object.assign({}, res) - delete userData.token - commit('SET_USER', userData) + await this.app.$apolloHelpers.onLogin(token) + commit('SET_TOKEN', token) + commit('SET_USER', user) } catch (err) { throw new Error(err) } finally { diff --git a/store/auth.test.js b/store/auth.test.js index 98bd1ce13..290938b7e 100644 --- a/store/auth.test.js +++ b/store/auth.test.js @@ -2,23 +2,35 @@ import { getters, mutations, actions } from './auth.js' let state let commit +let dispatch const token = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6InUzIiwic2x1ZyI6Implbm55LXJvc3RvY2siLCJuYW1lIjoiSmVubnkgUm9zdG9jayIsImF2YXRhciI6Imh0dHBzOi8vczMuYW1hem9uYXdzLmNvbS91aWZhY2VzL2ZhY2VzL3R3aXR0ZXIvbXV0dV9rcmlzaC8xMjguanBnIiwiZW1haWwiOiJ1c2VyQGV4YW1wbGUub3JnIiwicm9sZSI6InVzZXIiLCJpYXQiOjE1NDUxNDQ2ODgsImV4cCI6MTYzMTU0NDY4OCwiYXVkIjoiaHR0cDovL2xvY2FsaG9zdDozMDAwIiwiaXNzIjoiaHR0cDovL2xvY2FsaG9zdDo0MDAwIiwic3ViIjoidTMifQ.s5_JeQN9TaUPfymAXPOpbMAwhmTIg9cnOvNEcj4z75k' +const currentUser = { + id: 'u3', + name: 'Jenny Rostock', + slug: 'jenny-rostock', + email: 'user@example.org', + avatar: 'https://s3.amazonaws.com/uifaces/faces/twitter/mutu_krish/128.jpg', + role: 'user' +} const successfulLoginResponse = { data: { login: { - id: 'u3', - name: 'Jenny Rostock', - slug: 'jenny-rostock', - email: 'user@example.org', - avatar: - 'https://s3.amazonaws.com/uifaces/faces/twitter/mutu_krish/128.jpg', - role: 'user', + ...currentUser, token } } } +const successfulCurrentUserResponse = { + data: { + currentUser: { + ...currentUser, + token + } + } +} + const incorrectPasswordResponse = { data: { login: null @@ -39,6 +51,7 @@ const incorrectPasswordResponse = { beforeEach(() => { commit = jest.fn() + dispatch = jest.fn(() => Promise.resolve()) }) describe('getters', () => { @@ -55,16 +68,102 @@ describe('getters', () => { describe('actions', () => { let action + describe('init', () => { + const theAction = () => { + const module = { + app: { + $apolloHelpers: { + getToken: () => token + } + } + } + action = actions.init.bind(module) + return action({ commit, dispatch }) + } + + describe('client-side', () => { + beforeEach(() => { + process.server = false + }) + + it('returns', async () => { + await theAction() + expect(dispatch.mock.calls).toEqual([]) + expect(commit.mock.calls).toEqual([]) + }) + }) + + describe('server-side', () => { + beforeEach(() => { + process.server = true + }) + + it('fetches the current user', async () => { + await theAction() + expect(dispatch.mock.calls).toEqual([['fetchCurrentUser']]) + }) + + it('saves the JWT Bearer token', async () => { + await theAction() + expect(commit.mock.calls).toEqual( + expect.arrayContaining([['SET_TOKEN', token]]) + ) + }) + }) + }) + + describe('fetchCurrentUser', () => { + describe('given a successful response', () => { + beforeEach(async () => { + const module = { + app: { + apolloProvider: { + defaultClient: { + query: jest.fn(() => + Promise.resolve(successfulCurrentUserResponse) + ) + } + } + } + } + action = actions.fetchCurrentUser.bind(module) + await action({ commit }) + }) + + it('saves user data without token', () => { + expect(commit.mock.calls).toEqual( + expect.arrayContaining([ + [ + 'SET_USER', + { + id: 'u3', + name: 'Jenny Rostock', + slug: 'jenny-rostock', + email: 'user@example.org', + avatar: + 'https://s3.amazonaws.com/uifaces/faces/twitter/mutu_krish/128.jpg', + role: 'user' + } + ] + ]) + ) + }) + }) + }) + describe('login', () => { describe('given valid credentials and a successful response', () => { beforeEach(async () => { - const response = Object.assign({}, successfulLoginResponse) - const mutate = jest.fn(() => Promise.resolve(response)) - const onLogin = jest.fn(() => Promise.resolve()) const module = { app: { - apolloProvider: { defaultClient: { mutate } }, - $apolloHelpers: { onLogin } + apolloProvider: { + defaultClient: { + mutate: jest.fn(() => Promise.resolve(successfulLoginResponse)) + } + }, + $apolloHelpers: { + onLogin: jest.fn(() => Promise.resolve()) + } } } action = actions.login.bind(module) From b0262e9b769308feb46f85fc26a6fcd66f7e04b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 26 Feb 2019 15:55:50 +0100 Subject: [PATCH 03/12] Remove locationName and about as default properties of the currentUser query. We don't need it all the time. Somewhere in the frontend, the call to `fetchCurrentUser` is abused to refresh some data of the user to be edited. --- store/auth.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/store/auth.js b/store/auth.js index b1b029468..a2b53a196 100644 --- a/store/auth.js +++ b/store/auth.js @@ -103,8 +103,6 @@ export const actions = { email avatar role - locationName - about token } } From 2c606ec304a67a68fa4d12c0982fd11b1cc70c71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 26 Feb 2019 21:16:54 +0100 Subject: [PATCH 04/12] Settings page component without fetchCurrentUser --- cypress/integration/common/admin.js | 35 ++++++---- cypress/integration/common/report.js | 2 +- cypress/support/factories.js | 2 +- pages/settings/index.vue | 99 +++++++++++++--------------- 4 files changed, 70 insertions(+), 68 deletions(-) diff --git a/cypress/integration/common/admin.js b/cypress/integration/common/admin.js index 03cbe5fca..d922d909f 100644 --- a/cypress/integration/common/admin.js +++ b/cypress/integration/common/admin.js @@ -2,7 +2,6 @@ import { When, Then } from 'cypress-cucumber-preprocessor/steps' /* global cy */ - When('I navigate to the administration dashboard', () => { cy.get('.avatar-menu').click() cy.get('.avatar-menu-popover') @@ -14,11 +13,15 @@ Then('I can see a list of categories ordered by post count:', table => { cy.get('thead') .find('tr th') .should('have.length', 3) - table.hashes().forEach(({Name, Posts}, index) => { - cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(2)`) - .should('contain', Name) - cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(3)`) - .should('contain', Posts) + table.hashes().forEach(({ Name, Posts }, index) => { + cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(2)`).should( + 'contain', + Name + ) + cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(3)`).should( + 'contain', + Posts + ) }) }) @@ -26,12 +29,18 @@ Then('I can see a list of tags ordered by user count:', table => { cy.get('thead') .find('tr th') .should('have.length', 4) - table.hashes().forEach(({Name, Users, Posts}, index) => { - cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(2)`) - .should('contain', Name) - cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(3)`) - .should('contain', Users) - cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(4)`) - .should('contain', Posts) + table.hashes().forEach(({ Name, Users, Posts }, index) => { + cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(2)`).should( + 'contain', + Name + ) + cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(3)`).should( + 'contain', + Users + ) + cy.get(`tbody > :nth-child(${index + 1}) > :nth-child(4)`).should( + 'contain', + Posts + ) }) }) diff --git a/cypress/integration/common/report.js b/cypress/integration/common/report.js index 3f2895dd9..db264ddde 100644 --- a/cypress/integration/common/report.js +++ b/cypress/integration/common/report.js @@ -111,7 +111,7 @@ Then(`I can't see the moderation menu item`, () => { .should('not.exist') }) -When(/^I confirm the reporting dialog .*:$/, (message) => { +When(/^I confirm the reporting dialog .*:$/, message => { cy.contains(message) // wait for element to become visible cy.get('.ds-modal').within(() => { cy.get('button') diff --git a/cypress/support/factories.js b/cypress/support/factories.js index 95355f414..87bcd1853 100644 --- a/cypress/support/factories.js +++ b/cypress/support/factories.js @@ -15,7 +15,7 @@ beforeEach(async () => { }) Cypress.Commands.add('factory', () => { - return Factory({seedServerHost}) + return Factory({ seedServerHost }) }) Cypress.Commands.add( diff --git a/pages/settings/index.vue b/pages/settings/index.vue index c018061a9..57b5a4fff 100644 --- a/pages/settings/index.vue +++ b/pages/settings/index.vue @@ -55,12 +55,40 @@ import find from 'lodash/find' let timeout const mapboxToken = process.env.MAPBOX_TOKEN +const query = gql` + query getUser($id: ID) { + User(id: $id) { + id + name + locationName + about + } + } +` + +const mutation = gql` + mutation($id: ID!, $name: String, $locationName: String, $about: String) { + UpdateUser( + id: $id + name: $name + locationName: $locationName + about: $about + ) { + id + name + locationName + about + } + } +` + export default { data() { return { axiosSource: null, cities: [], sending: false, + users: [], form: { name: null, locationName: null, @@ -70,18 +98,23 @@ export default { }, computed: { ...mapGetters({ - user: 'auth/user' + currentUser: 'auth/user' }) }, watch: { - user: { - immediate: true, - handler: function(user) { - this.form = { - name: user.name, - locationName: user.locationName, - about: user.about - } + users: function(users) { + const { name, locationName, about } = users[0] + this.form = { name, locationName, about } + } + }, + apollo: { + users: function() { + return { + query, + variables: { + id: this.currentUser.id + }, + update: data => data.User } } }, @@ -90,59 +123,19 @@ export default { this.sending = true this.$apollo .mutate({ - mutation: gql` - mutation( - $id: ID! - $name: String - $locationName: String - $about: String - ) { - UpdateUser( - id: $id - name: $name - locationName: $locationName - about: $about - ) { - id - name - locationName - about - } - } - `, - // Parameters + mutation, variables: { - id: this.user.id, + id: this.currentUser.id, name: this.form.name, locationName: this.form.locationName ? this.form.locationName['label'] || this.form.locationName : null, about: this.form.about }, - // Update the cache with the result - // The query will be updated with the optimistic response - // and then with the real result of the mutation update: (store, { data: { UpdateUser } }) => { - this.$store.dispatch('auth/fetchCurrentUser') - - // Read the data from our cache for this query. - // const data = store.readQuery({ query: TAGS_QUERY }) - // Add our tag from the mutation to the end - // data.tags.push(addTag) - // Write our data back to the cache. - // store.writeQuery({ query: TAGS_QUERY, data }) + const { name, locationName, about } = UpdateUser + this.form = { name, locationName, about } } - // Optimistic UI - // Will be treated as a 'fake' result as soon as the request is made - // so that the UI can react quickly and the user be happy - /* optimisticResponse: { - __typename: 'Mutation', - addTag: { - __typename: 'Tag', - id: -1, - label: newTag - } - } */ }) .then(data => { this.$toast.success('Updated user') From fba1072964d67654cd5788c18335f491b3668e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 26 Feb 2019 22:37:12 +0100 Subject: [PATCH 05/12] Add back the `fetchCurrentUser` There was another reason: The user menu in the top right listens on `auth/user` which does not get updated otherwise. We should overthink having two separate result types in the backend: * currentUser: [LoggedInUser] * User: [User] Why not return `User` in both cases? --- pages/settings/index.vue | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pages/settings/index.vue b/pages/settings/index.vue index 57b5a4fff..32824c4c3 100644 --- a/pages/settings/index.vue +++ b/pages/settings/index.vue @@ -135,6 +135,9 @@ export default { update: (store, { data: { UpdateUser } }) => { const { name, locationName, about } = UpdateUser this.form = { name, locationName, about } + // update the user menu, too + // which listens on auth/user + this.$store.dispatch('auth/fetchCurrentUser') } }) .then(data => { From 34d79dfc6400f1386725f55ddd59b3f1120910a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Wed, 27 Feb 2019 10:45:46 +0100 Subject: [PATCH 06/12] Log out the user if fetchUser was unsuccessful --- store/auth.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/store/auth.js b/store/auth.js index a2b53a196..91baad151 100644 --- a/store/auth.js +++ b/store/auth.js @@ -67,7 +67,7 @@ export const actions = { } return getters.isLoggedIn }, - async fetchCurrentUser({ commit }) { + async fetchCurrentUser({ commit, dispatch }) { const client = this.app.apolloProvider.defaultClient const { data: { currentUser } @@ -83,6 +83,7 @@ export const actions = { } }`) }) + if (!currentUser) return dispatch('logout') const { token, ...user } = currentUser commit('SET_USER', user) return user From ea9f896ef8dce4e0fb6d99256815612d6cee9f45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Wed, 27 Feb 2019 11:06:02 +0100 Subject: [PATCH 07/12] Refactor store to use the new backend API --- store/auth.js | 31 +++++++++++++------------------ store/auth.test.js | 39 +++++---------------------------------- 2 files changed, 18 insertions(+), 52 deletions(-) diff --git a/store/auth.js b/store/auth.js index 91baad151..4785ff0c0 100644 --- a/store/auth.js +++ b/store/auth.js @@ -67,6 +67,7 @@ export const actions = { } return getters.isLoggedIn }, + async fetchCurrentUser({ commit, dispatch }) { const client = this.app.apolloProvider.defaultClient const { @@ -80,15 +81,17 @@ export const actions = { email avatar role + about + locationName } }`) }) if (!currentUser) return dispatch('logout') - const { token, ...user } = currentUser - commit('SET_USER', user) - return user + commit('SET_USER', currentUser) + return currentUser }, - async login({ commit }, { email, password }) { + + async login({ commit, dispatch }, { email, password }) { commit('SET_PENDING', true) try { const client = this.app.apolloProvider.defaultClient @@ -97,35 +100,27 @@ export const actions = { } = await client.mutate({ mutation: gql(` mutation($email: String!, $password: String!) { - login(email: $email, password: $password) { - id - name - slug - email - avatar - role - token - } + login(email: $email, password: $password) } `), variables: { email, password } }) - const { token, ...user } = login - - await this.app.$apolloHelpers.onLogin(token) - commit('SET_TOKEN', token) - commit('SET_USER', user) + await this.app.$apolloHelpers.onLogin(login) + commit('SET_TOKEN', login) + await dispatch('fetchCurrentUser') } catch (err) { throw new Error(err) } finally { commit('SET_PENDING', false) } }, + async logout({ commit }) { commit('SET_USER', null) commit('SET_TOKEN', null) return this.app.$apolloHelpers.onLogout() }, + register( { dispatch, commit }, { email, password, inviteCode, invitedByUserId } diff --git a/store/auth.test.js b/store/auth.test.js index 290938b7e..6758e3427 100644 --- a/store/auth.test.js +++ b/store/auth.test.js @@ -14,22 +14,8 @@ const currentUser = { avatar: 'https://s3.amazonaws.com/uifaces/faces/twitter/mutu_krish/128.jpg', role: 'user' } -const successfulLoginResponse = { - data: { - login: { - ...currentUser, - token - } - } -} -const successfulCurrentUserResponse = { - data: { - currentUser: { - ...currentUser, - token - } - } -} +const successfulLoginResponse = { data: { login: token } } +const successfulCurrentUserResponse = { data: { currentUser } } const incorrectPasswordResponse = { data: { @@ -168,7 +154,7 @@ describe('actions', () => { } action = actions.login.bind(module) await action( - { commit }, + { commit, dispatch }, { email: 'user@example.org', password: '1234' } ) }) @@ -183,23 +169,8 @@ describe('actions', () => { ) }) - it('saves user data without token', () => { - expect(commit.mock.calls).toEqual( - expect.arrayContaining([ - [ - 'SET_USER', - { - id: 'u3', - name: 'Jenny Rostock', - slug: 'jenny-rostock', - email: 'user@example.org', - avatar: - 'https://s3.amazonaws.com/uifaces/faces/twitter/mutu_krish/128.jpg', - role: 'user' - } - ] - ]) - ) + it('fetches the user', () => { + expect(dispatch.mock.calls).toEqual( [['fetchCurrentUser']]) }) it('saves pending flags in order', () => { From 41072e6fa8ae37b41064c90a58b89af6989b8f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Wed, 27 Feb 2019 11:06:23 +0100 Subject: [PATCH 08/12] Update currentUser with $store.commit I find this much cleaner that to dispatch `fetchCurrentUser`. It gets along with one less request, too. --- pages/settings/index.vue | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pages/settings/index.vue b/pages/settings/index.vue index 32824c4c3..61e3f9a01 100644 --- a/pages/settings/index.vue +++ b/pages/settings/index.vue @@ -48,7 +48,7 @@