From d375ebe7d90e3251b17f59ffba8fb1470923ebe8 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 12 Dec 2019 18:14:47 +0100 Subject: [PATCH] Write test/refactor tests/resolvers/middleware - write tests for userMiddleware - checks the functionality of nodes/locations middleware - refactor to not allow users to update to remove their name debatable whether we want that or not, but we do not allow users to create accounts with no name, so we should be consistent, before we were using neode to validate this, but we have are removing neode from production code, so we must validate ourselves - collate UpdateUser mutations to one --- backend/src/middleware/index.js | 2 +- backend/src/middleware/nodes/locations.js | 62 ++--- .../middleware/{ => user}/userMiddleware.js | 4 +- .../middleware/user/userMiddleware.spec.js | 213 ++++++++++++++++++ backend/src/middleware/userMiddleware.spec.js | 0 .../validation/validationMiddleware.js | 6 +- backend/src/schema/types/type/User.gql | 2 +- webapp/components/Embed/EmbedComponent.vue | 5 +- webapp/graphql/User.js | 49 ++-- webapp/pages/settings/embeds.vue | 5 +- webapp/pages/settings/index.vue | 30 +-- webapp/pages/settings/privacy.vue | 5 +- 12 files changed, 293 insertions(+), 90 deletions(-) rename backend/src/middleware/{ => user}/userMiddleware.js (75%) create mode 100644 backend/src/middleware/user/userMiddleware.spec.js delete mode 100644 backend/src/middleware/userMiddleware.spec.js diff --git a/backend/src/middleware/index.js b/backend/src/middleware/index.js index d09a96475..9c68d8c00 100644 --- a/backend/src/middleware/index.js +++ b/backend/src/middleware/index.js @@ -7,7 +7,7 @@ import sluggify from './sluggifyMiddleware' import excerpt from './excerptMiddleware' import xss from './xssMiddleware' import permissions from './permissionsMiddleware' -import user from './userMiddleware' +import user from './user/userMiddleware' import includedFields from './includedFieldsMiddleware' import orderBy from './orderByMiddleware' import validation from './validation/validationMiddleware' diff --git a/backend/src/middleware/nodes/locations.js b/backend/src/middleware/nodes/locations.js index d80d08a9a..47262d7ba 100644 --- a/backend/src/middleware/nodes/locations.js +++ b/backend/src/middleware/nodes/locations.js @@ -70,7 +70,6 @@ const createOrUpdateLocations = async (userId, locationName, driver) => { if (isEmpty(locationName)) { return } - const res = await fetch( `https://api.mapbox.com/geocoding/v5/mapbox.places/${encodeURIComponent( locationName, @@ -111,33 +110,44 @@ const createOrUpdateLocations = async (userId, locationName, driver) => { if (data.context) { await asyncForEach(data.context, async ctx => { await createLocation(session, ctx) - - await session.run( - 'MATCH (parent:Location {id: $parentId}), (child:Location {id: $childId}) ' + - 'MERGE (child)<-[:IS_IN]-(parent) ' + - 'RETURN child.id, parent.id', - { - parentId: parent.id, - childId: ctx.id, - }, - ) - - parent = ctx + try { + await session.writeTransaction(transaction => { + return transaction.run( + ` + MATCH (parent:Location {id: $parentId}), (child:Location {id: $childId}) + MERGE (child)<-[:IS_IN]-(parent) + RETURN child.id, parent.id + `, + { + parentId: parent.id, + childId: ctx.id, + }, + ) + }) + parent = ctx + } finally { + session.close() + } }) } - // delete all current locations from user - await session.run('MATCH (u:User {id: $userId})-[r:IS_IN]->(l:Location) DETACH DELETE r', { - userId: userId, - }) - // connect user with location - await session.run( - 'MATCH (u:User {id: $userId}), (l:Location {id: $locationId}) MERGE (u)-[:IS_IN]->(l) RETURN l.id, u.id', - { - userId: userId, - locationId: data.id, - }, - ) - session.close() + // delete all current locations from user and add new location + try { + await session.writeTransaction(transaction => { + return transaction.run( + ` + MATCH (user:User {id: $userId})-[relationship:IS_IN]->(location:Location) + DETACH DELETE relationship + WITH user + MATCH (location:Location {id: $locationId}) + MERGE (user)-[:IS_IN]->(location) + RETURN location.id, user.id + `, + { userId: userId, locationId: data.id }, + ) + }) + } finally { + session.close() + } } export default createOrUpdateLocations diff --git a/backend/src/middleware/userMiddleware.js b/backend/src/middleware/user/userMiddleware.js similarity index 75% rename from backend/src/middleware/userMiddleware.js rename to backend/src/middleware/user/userMiddleware.js index fafbd44e5..2ca61e69f 100644 --- a/backend/src/middleware/userMiddleware.js +++ b/backend/src/middleware/user/userMiddleware.js @@ -1,10 +1,10 @@ -import createOrUpdateLocations from './nodes/locations' +import createOrUpdateLocations from '../nodes/locations' export default { Mutation: { SignupVerification: async (resolve, root, args, context, info) => { const result = await resolve(root, args, context, info) - await createOrUpdateLocations(args.id, args.locationName, context.driver) + await createOrUpdateLocations(result.id, args.locationName, context.driver) return result }, UpdateUser: async (resolve, root, args, context, info) => { diff --git a/backend/src/middleware/user/userMiddleware.spec.js b/backend/src/middleware/user/userMiddleware.spec.js new file mode 100644 index 000000000..4ca8fd89f --- /dev/null +++ b/backend/src/middleware/user/userMiddleware.spec.js @@ -0,0 +1,213 @@ +import { gql } from '../../helpers/jest' +import Factory from '../../seed/factories' +import { getNeode, getDriver } from '../../bootstrap/neo4j' +import { createTestClient } from 'apollo-server-testing' +import createServer from '../../server' + +const factory = Factory() +const neode = getNeode() +const driver = getDriver() +let authenticatedUser, mutate, variables + +const signupVerificationMutation = gql` + mutation( + $name: String! + $password: String! + $email: String! + $nonce: String! + $termsAndConditionsAgreedVersion: String! + $locationName: String + ) { + SignupVerification( + name: $name + password: $password + email: $email + nonce: $nonce + termsAndConditionsAgreedVersion: $termsAndConditionsAgreedVersion + locationName: $locationName + ) { + locationName + } + } +` + +const updateUserMutation = gql` + mutation($id: ID!, $name: String!, $locationName: String) { + UpdateUser(id: $id, name: $name, locationName: $locationName) { + locationName + } + } +` + +let newlyCreatedNodesWithLocales = [ + { + city: { + lng: 41.1534, + nameES: 'Hamburg', + nameFR: 'Hamburg', + nameIT: 'Hamburg', + nameEN: 'Hamburg', + type: 'place', + namePT: 'Hamburg', + nameRU: 'Хамбург', + nameDE: 'Hamburg', + nameNL: 'Hamburg', + name: 'Hamburg', + namePL: 'Hamburg', + id: 'place.5977106083398860', + lat: -74.5763, + }, + state: { + namePT: 'Nova Jérsia', + nameRU: 'Нью-Джерси', + nameDE: 'New Jersey', + nameNL: 'New Jersey', + nameES: 'Nueva Jersey', + name: 'New Jersey', + namePL: 'New Jersey', + nameFR: 'New Jersey', + nameIT: 'New Jersey', + id: 'region.14919479731700330', + nameEN: 'New Jersey', + type: 'region', + }, + country: { + namePT: 'Estados Unidos', + nameRU: 'Соединённые Штаты Америки', + nameDE: 'Vereinigte Staaten', + nameNL: 'Verenigde Staten van Amerika', + nameES: 'Estados Unidos', + namePL: 'Stany Zjednoczone', + name: 'United States of America', + nameFR: 'États-Unis', + nameIT: "Stati Uniti d'America", + id: 'country.9053006287256050', + nameEN: 'United States of America', + type: 'country', + }, + }, +] + +beforeAll(() => { + const { server } = createServer({ + context: () => { + return { + user: authenticatedUser, + neode, + driver, + } + }, + }) + mutate = createTestClient(server).mutate +}) + +beforeEach(() => { + variables = {} + authenticatedUser = null +}) + +afterEach(() => { + factory.cleanDatabase() +}) + +describe('userMiddleware', () => { + describe('SignupVerification', () => { + beforeEach(async () => { + variables = { + ...variables, + name: 'John Doe', + password: '123', + email: 'john@example.org', + nonce: '123456', + termsAndConditionsAgreedVersion: '0.1.0', + locationName: 'Hamburg, New Jersey, United States of America', + } + const args = { + email: 'john@example.org', + nonce: '123456', + } + await neode.model('EmailAddress').create(args) + }) + it('creates a Location node with localised city/state/country names', async () => { + await mutate({ mutation: signupVerificationMutation, variables }) + const locations = await neode.cypher( + `MATCH (city:Location)-[:IS_IN]->(state:Location)-[:IS_IN]->(country:Location) return city, state, country`, + ) + expect( + locations.records.map(record => { + return { + city: record.get('city').properties, + state: record.get('state').properties, + country: record.get('country').properties, + } + }), + ).toEqual(newlyCreatedNodesWithLocales) + }) + }) + + describe('UpdateUser', () => { + let user, userParams + beforeEach(async () => { + newlyCreatedNodesWithLocales = [ + { + city: { + lng: 53.55, + nameES: 'Hamburgo', + nameFR: 'Hambourg', + nameIT: 'Amburgo', + nameEN: 'Hamburg', + type: 'region', + namePT: 'Hamburgo', + nameRU: 'Гамбург', + nameDE: 'Hamburg', + nameNL: 'Hamburg', + namePL: 'Hamburg', + name: 'Hamburg', + id: 'region.10793468240398860', + lat: 10, + }, + country: { + namePT: 'Alemanha', + nameRU: 'Германия', + nameDE: 'Deutschland', + nameNL: 'Duitsland', + nameES: 'Alemania', + name: 'Germany', + namePL: 'Niemcy', + nameFR: 'Allemagne', + nameIT: 'Germania', + id: 'country.10743216036480410', + nameEN: 'Germany', + type: 'country', + }, + }, + ] + userParams = { + id: 'updating-user', + } + user = await factory.create('User', userParams) + authenticatedUser = await user.toJson() + }) + + it('creates a Location node with localised city/state/country names', async () => { + variables = { + ...variables, + id: 'updating-user', + name: 'Updating user', + locationName: 'Hamburg, Germany', + } + await mutate({ mutation: updateUserMutation, variables }) + const locations = await neode.cypher( + `MATCH (city:Location)-[:IS_IN]->(country:Location) return city, country`, + ) + expect( + locations.records.map(record => { + return { + city: record.get('city').properties, + country: record.get('country').properties, + } + }), + ).toEqual(newlyCreatedNodesWithLocales) + }) + }) +}) diff --git a/backend/src/middleware/userMiddleware.spec.js b/backend/src/middleware/userMiddleware.spec.js deleted file mode 100644 index e69de29bb..000000000 diff --git a/backend/src/middleware/validation/validationMiddleware.js b/backend/src/middleware/validation/validationMiddleware.js index 9e6adc5a0..8caf73e0c 100644 --- a/backend/src/middleware/validation/validationMiddleware.js +++ b/backend/src/middleware/validation/validationMiddleware.js @@ -128,9 +128,11 @@ export const validateNotifyUsers = async (label, reason) => { } const validateUpdateUser = async (resolve, root, params, context, info) => { - if (!params.name || params.name.length < USERNAME_MIN_LENGTH) + const { name } = params + if (typeof name === 'string' && name.trim().length > USERNAME_MIN_LENGTH) + return resolve(root, params, context, info) + if (typeof name !== 'string' || name.trim().length < USERNAME_MIN_LENGTH) throw new UserInputError(`Username must be at least ${USERNAME_MIN_LENGTH} character long!`) - return resolve(root, params, context, info) } export default { diff --git a/backend/src/schema/types/type/User.gql b/backend/src/schema/types/type/User.gql index 243f45322..df6d831fa 100644 --- a/backend/src/schema/types/type/User.gql +++ b/backend/src/schema/types/type/User.gql @@ -26,7 +26,7 @@ enum _UserOrdering { type User { id: ID! actorId: String - name: String + name: String! email: String! @cypher(statement: "MATCH (this)-[: PRIMARY_EMAIL]->(e: EmailAddress) RETURN e.email") slug: String! avatar: String diff --git a/webapp/components/Embed/EmbedComponent.vue b/webapp/components/Embed/EmbedComponent.vue index 5dc8ad00c..0ce33682c 100644 --- a/webapp/components/Embed/EmbedComponent.vue +++ b/webapp/components/Embed/EmbedComponent.vue @@ -46,7 +46,7 @@