From 5117a97b1c63d1ea856f6d704677a270f1d567c7 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 5 Dec 2019 13:17:26 +0100 Subject: [PATCH 01/32] Use readTransaction for non-write transactions --- backend/src/middleware/validation/validationMiddleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/middleware/validation/validationMiddleware.js b/backend/src/middleware/validation/validationMiddleware.js index f36458e61..4413b7747 100644 --- a/backend/src/middleware/validation/validationMiddleware.js +++ b/backend/src/middleware/validation/validationMiddleware.js @@ -72,7 +72,7 @@ const validateReview = async (resolve, root, args, context, info) => { const { user, driver } = context if (resourceId === user.id) throw new Error('You cannot review yourself!') const session = driver.session() - const reportReadTxPromise = session.writeTransaction(async txc => { + const reportReadTxPromise = session.readTransaction(async txc => { const validateReviewTransactionResponse = await txc.run( ` MATCH (resource {id: $resourceId}) From f2743c992f4c53df294ada1ef564c9b1dd5769ff Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 5 Dec 2019 13:18:06 +0100 Subject: [PATCH 02/32] Use transaction function jwt/decode --- backend/src/jwt/decode.js | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/backend/src/jwt/decode.js b/backend/src/jwt/decode.js index 5b7881d20..3b0efea57 100644 --- a/backend/src/jwt/decode.js +++ b/backend/src/jwt/decode.js @@ -11,27 +11,30 @@ export default async (driver, authorizationHeader) => { } catch (err) { return null } - const query = ` - MATCH (user:User {id: $id, deleted: false, disabled: false }) - SET user.lastActiveAt = toString(datetime()) - RETURN user {.id, .slug, .name, .avatar, .email, .role, .disabled, .actorId} - LIMIT 1 - ` const session = driver.session() - let result + const writeTxResultPromise = session.writeTransaction(async txc => { + const updateUserLastActiveTransactionResponse = await txc.run( + ` + MATCH (user:User {id: $id, deleted: false, disabled: false }) + SET user.lastActiveAt = toString(datetime()) + RETURN user {.id, .slug, .name, .avatar, .email, .role, .disabled, .actorId} + LIMIT 1 + `, + { id }, + ) + return updateUserLastActiveTransactionResponse.records.map( + record => record.get('user'), + ) + }) try { - result = await session.run(query, { id }) + const [currentUser] = await writeTxResultPromise + if (!currentUser) return null + return { + token, + ...currentUser, + } } finally { session.close() } - - const [currentUser] = await result.records.map(record => { - return record.get('user') - }) - if (!currentUser) return null - return { - token, - ...currentUser, - } } From de64f1dd4ab850320d3e1e1859a3412f284174f9 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 5 Dec 2019 13:43:55 +0100 Subject: [PATCH 03/32] Refactor hashtagsMiddleware updateHashtagsOfPost - Favor transaction functions for production environment - Use one transaction instead of two as we can use optional match to delete potential previous relationships --- .../middleware/hashtags/hashtagsMiddleware.js | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/backend/src/middleware/hashtags/hashtagsMiddleware.js b/backend/src/middleware/hashtags/hashtagsMiddleware.js index 53a8fed20..cbb5e5320 100644 --- a/backend/src/middleware/hashtags/hashtagsMiddleware.js +++ b/backend/src/middleware/hashtags/hashtagsMiddleware.js @@ -2,30 +2,23 @@ import extractHashtags from '../hashtags/extractHashtags' const updateHashtagsOfPost = async (postId, hashtags, context) => { if (!hashtags.length) return - - // We need two Cypher statements, because the 'MATCH' in the 'cypherDeletePreviousRelations' statement - // functions as an 'if'. In case there is no previous relation, the rest of the commands are omitted - // and no new Hashtags and relations will be created. - const cypherDeletePreviousRelations = ` - MATCH (p: Post { id: $postId })-[previousRelations: TAGGED]->(t: Tag) - DELETE previousRelations - RETURN p, t - ` - const cypherCreateNewTagsAndRelations = ` - MATCH (p: Post { id: $postId}) - UNWIND $hashtags AS tagName - MERGE (t: Tag { id: tagName, disabled: false, deleted: false }) - MERGE (p)-[:TAGGED]->(t) - RETURN p, t - ` const session = context.driver.session() + try { - await session.run(cypherDeletePreviousRelations, { - postId, - }) - await session.run(cypherCreateNewTagsAndRelations, { - postId, - hashtags, + await session.writeTransaction(txc => { + return txc.run( + ` + MATCH (post:Post { id: $postId}) + OPTIONAL MATCH (post)-[previousRelations:TAGGED]->(tag:Tag) + DELETE previousRelations + WITH post + UNWIND $hashtags AS tagName + MERGE (tag:Tag {id: tagName, disabled: false, deleted: false }) + MERGE (post)-[:TAGGED]->(tag) + RETURN post, tag + `, + { postId, hashtags }, + ) }) } finally { session.close() From 1827889582e1ddb8749dbcd228f4ab975682e151 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 5 Dec 2019 20:42:45 +0100 Subject: [PATCH 04/32] Refactor notificationsMiddleware/locations - start refactoring - locations does not have any automated tests, which makes it more difficult to refactor and have confidence that functionality will not be broken - notificationsMiddleware in progress --- backend/src/jwt/decode.js | 8 +- .../middleware/hashtags/hashtagsMiddleware.js | 2 +- backend/src/middleware/nodes/locations.js | 15 ++-- .../notifications/notificationsMiddleware.js | 83 ++++++------------- .../notificationsMiddleware.spec.js | 6 +- 5 files changed, 39 insertions(+), 75 deletions(-) diff --git a/backend/src/jwt/decode.js b/backend/src/jwt/decode.js index 3b0efea57..41e9d005e 100644 --- a/backend/src/jwt/decode.js +++ b/backend/src/jwt/decode.js @@ -23,14 +23,12 @@ export default async (driver, authorizationHeader) => { `, { id }, ) - return updateUserLastActiveTransactionResponse.records.map( - record => record.get('user'), - ) + return updateUserLastActiveTransactionResponse.records.map(record => record.get('user')) }) try { const [currentUser] = await writeTxResultPromise - if (!currentUser) return null - return { + if (!currentUser) return null + return { token, ...currentUser, } diff --git a/backend/src/middleware/hashtags/hashtagsMiddleware.js b/backend/src/middleware/hashtags/hashtagsMiddleware.js index cbb5e5320..7d8593fd5 100644 --- a/backend/src/middleware/hashtags/hashtagsMiddleware.js +++ b/backend/src/middleware/hashtags/hashtagsMiddleware.js @@ -3,7 +3,7 @@ import extractHashtags from '../hashtags/extractHashtags' const updateHashtagsOfPost = async (postId, hashtags, context) => { if (!hashtags.length) return const session = context.driver.session() - + try { await session.writeTransaction(txc => { return txc.run( diff --git a/backend/src/middleware/nodes/locations.js b/backend/src/middleware/nodes/locations.js index 3e0ca6855..d80d08a9a 100644 --- a/backend/src/middleware/nodes/locations.js +++ b/backend/src/middleware/nodes/locations.js @@ -38,7 +38,7 @@ const createLocation = async (session, mapboxData) => { lng: mapboxData.center && mapboxData.center.length ? mapboxData.center[1] : null, } - let query = + let mutation = 'MERGE (l:Location {id: $id}) ' + 'SET l.name = $nameEN, ' + 'l.nameEN = $nameEN, ' + @@ -53,12 +53,17 @@ const createLocation = async (session, mapboxData) => { 'l.type = $type' if (data.lat && data.lng) { - query += ', l.lat = $lat, l.lng = $lng' + mutation += ', l.lat = $lat, l.lng = $lng' } - query += ' RETURN l.id' + mutation += ' RETURN l.id' - await session.run(query, data) - session.close() + try { + await session.writeTransaction(transaction => { + return transaction.run(mutation, data) + }) + } finally { + session.close() + } } const createOrUpdateLocations = async (userId, locationName, driver) => { diff --git a/backend/src/middleware/notifications/notificationsMiddleware.js b/backend/src/middleware/notifications/notificationsMiddleware.js index ac199a67d..3d6d13790 100644 --- a/backend/src/middleware/notifications/notificationsMiddleware.js +++ b/backend/src/middleware/notifications/notificationsMiddleware.js @@ -1,23 +1,22 @@ import extractMentionedUsers from './mentions/extractMentionedUsers' -const postAuthorOfComment = async (comment, { context }) => { - const cypherFindUser = ` - MATCH (user: User)-[:WROTE]->(:Post)<-[:COMMENTS]-(:Comment { id: $commentId }) - RETURN user { .id } - ` +const postAuthorOfComment = async (commentId, { context }) => { const session = context.driver.session() - let result + let postAuthorId try { - result = await session.run(cypherFindUser, { - commentId: comment.id, + postAuthorId = await session.readTransaction(transaction => { + return transaction.run( + ` + MATCH (author:User)-[:WROTE]->(:Post)<-[:COMMENTS]-(:Comment { id: $commentId }) + RETURN author { .id } as authorId + `, + { commentId }, + ) }) + return postAuthorId.records.map(record => record.get('authorId')) } finally { session.close() } - const [postAuthor] = await result.records.map(record => { - return record.get('user') - }) - return postAuthor } const notifyUsers = async (label, id, idsOfUsers, reason, context) => { @@ -90,10 +89,8 @@ const notifyUsers = async (label, id, idsOfUsers, reason, context) => { } const session = context.driver.session() try { - await session.run(cypher, { - id, - idsOfUsers, - reason, + await session.writeTransaction(transaction => { + return transaction.run(cypher, { id, idsOfUsers, reason }) }) } finally { session.close() @@ -102,56 +99,24 @@ const notifyUsers = async (label, id, idsOfUsers, reason, context) => { const handleContentDataOfPost = async (resolve, root, args, context, resolveInfo) => { const idsOfUsers = extractMentionedUsers(args.content) - const post = await resolve(root, args, context, resolveInfo) - - if (post) { - await notifyUsers('Post', post.id, idsOfUsers, 'mentioned_in_post', context) - } - - return post + if (post) return notifyUsers('Post', post.id, idsOfUsers, 'mentioned_in_post', context) } const handleContentDataOfComment = async (resolve, root, args, context, resolveInfo) => { - let idsOfUsers = extractMentionedUsers(args.content) - const comment = await resolve(root, args, context, resolveInfo) - - if (comment) { - const postAuthor = await postAuthorOfComment(comment, { context }) - idsOfUsers = idsOfUsers.filter(id => id !== postAuthor.id) - - await notifyUsers('Comment', comment.id, idsOfUsers, 'mentioned_in_comment', context) - } - - return comment + const { content, id: commentId } = args + let idsOfUsers = extractMentionedUsers(content) + const [postAuthor] = await postAuthorOfComment(commentId, { context }) + idsOfUsers = idsOfUsers.filter(id => id !== postAuthor.id) + if (idsOfUsers && idsOfUsers.length) + await notifyUsers('Comment', commentId, idsOfUsers, 'mentioned_in_comment', context) + if (context.user.id !== postAuthor.id) + await notifyUsers('Comment', commentId, [postAuthor.id], 'commented_on_post', context) } const handleCreateComment = async (resolve, root, args, context, resolveInfo) => { - const comment = await handleContentDataOfComment(resolve, root, args, context, resolveInfo) - - if (comment) { - const cypherFindUser = ` - MATCH (user: User)-[:WROTE]->(:Post)<-[:COMMENTS]-(:Comment { id: $commentId }) - RETURN user { .id } - ` - const session = context.driver.session() - let result - try { - result = await session.run(cypherFindUser, { - commentId: comment.id, - }) - } finally { - session.close() - } - const [postAuthor] = await result.records.map(record => { - return record.get('user') - }) - if (context.user.id !== postAuthor.id) { - await notifyUsers('Comment', comment.id, [postAuthor.id], 'commented_on_post', context) - } - } - - return comment + const comment = await resolve(root, args, context, resolveInfo) + if (comment) return handleContentDataOfComment(resolve, root, args, context, resolveInfo) } export default { diff --git a/backend/src/middleware/notifications/notificationsMiddleware.spec.js b/backend/src/middleware/notifications/notificationsMiddleware.spec.js index 502ddaa8e..6fd94ab69 100644 --- a/backend/src/middleware/notifications/notificationsMiddleware.spec.js +++ b/backend/src/middleware/notifications/notificationsMiddleware.spec.js @@ -4,11 +4,7 @@ import { createTestClient } from 'apollo-server-testing' import { neode, getDriver } from '../../bootstrap/neo4j' import createServer from '../../server' -let server -let query -let mutate -let notifiedUser -let authenticatedUser +let server, query, mutate, notifiedUser, authenticatedUser const factory = Factory() const driver = getDriver() const instance = neode() From 132951c525028c74ce46c9f7f4bbf6f8bc5080a3 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Fri, 6 Dec 2019 13:03:05 +0100 Subject: [PATCH 05/32] Update handleContentDataOfPost to return post - fix tests, functionality --- .../src/middleware/notifications/notificationsMiddleware.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/src/middleware/notifications/notificationsMiddleware.js b/backend/src/middleware/notifications/notificationsMiddleware.js index 3d6d13790..564ddc370 100644 --- a/backend/src/middleware/notifications/notificationsMiddleware.js +++ b/backend/src/middleware/notifications/notificationsMiddleware.js @@ -100,7 +100,8 @@ const notifyUsers = async (label, id, idsOfUsers, reason, context) => { const handleContentDataOfPost = async (resolve, root, args, context, resolveInfo) => { const idsOfUsers = extractMentionedUsers(args.content) const post = await resolve(root, args, context, resolveInfo) - if (post) return notifyUsers('Post', post.id, idsOfUsers, 'mentioned_in_post', context) + if (post) await notifyUsers('Post', post.id, idsOfUsers, 'mentioned_in_post', context) + return post } const handleContentDataOfComment = async (resolve, root, args, context, resolveInfo) => { From da150e0b2791dadc592a1a15c8e3a7ce5c85eb68 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Fri, 6 Dec 2019 14:48:32 +0100 Subject: [PATCH 06/32] Refactor notificationsMiddleware - Use transaction functions - extract validations into validationsMiddleware - break notifyUsers into notifyUsersOfMention/notifyUsersOfComment --- .../notifications/notificationsMiddleware.js | 142 +++++++++--------- .../notificationsMiddleware.spec.js | 18 +-- .../validation/validationMiddleware.js | 11 ++ 3 files changed, 88 insertions(+), 83 deletions(-) diff --git a/backend/src/middleware/notifications/notificationsMiddleware.js b/backend/src/middleware/notifications/notificationsMiddleware.js index 564ddc370..4daf2d770 100644 --- a/backend/src/middleware/notifications/notificationsMiddleware.js +++ b/backend/src/middleware/notifications/notificationsMiddleware.js @@ -1,4 +1,29 @@ import extractMentionedUsers from './mentions/extractMentionedUsers' +import { validateNotifyUsers } from '../validation/validationMiddleware' + +const handleContentDataOfPost = async (resolve, root, args, context, resolveInfo) => { + const idsOfUsers = extractMentionedUsers(args.content) + const post = await resolve(root, args, context, resolveInfo) + if (post && idsOfUsers && idsOfUsers.length) + await notifyUsersOfMention('Post', post.id, idsOfUsers, 'mentioned_in_post', context) + return post +} + +const handleCreateComment = async (resolve, root, args, context, resolveInfo) => { + const comment = await resolve(root, args, context, resolveInfo) + if (comment) return handleContentDataOfComment(resolve, root, args, context, resolveInfo) +} + +const handleContentDataOfComment = async (resolve, root, args, context, resolveInfo) => { + const { content, id: commentId } = args + let idsOfUsers = extractMentionedUsers(content) + const [postAuthor] = await postAuthorOfComment(commentId, { context }) + idsOfUsers = idsOfUsers.filter(id => id !== postAuthor.id) + if (idsOfUsers && idsOfUsers.length) + await notifyUsersOfMention('Comment', commentId, idsOfUsers, 'mentioned_in_comment', context) + if (context.user.id !== postAuthor.id) + await notifyUsersOfComment('Comment', commentId, postAuthor.id, 'commented_on_post', context) +} const postAuthorOfComment = async (commentId, { context }) => { const session = context.driver.session() @@ -19,105 +44,74 @@ const postAuthorOfComment = async (commentId, { context }) => { } } -const notifyUsers = async (label, id, idsOfUsers, reason, context) => { - if (!idsOfUsers.length) return - - // Checked here, because it does not go through GraphQL checks at all in this file. - const reasonsAllowed = ['mentioned_in_post', 'mentioned_in_comment', 'commented_on_post'] - if (!reasonsAllowed.includes(reason)) { - throw new Error('Notification reason is not allowed!') - } - if ( - (label === 'Post' && reason !== 'mentioned_in_post') || - (label === 'Comment' && !['mentioned_in_comment', 'commented_on_post'].includes(reason)) - ) { - throw new Error('Notification does not fit the reason!') - } - - let cypher +const notifyUsersOfMention = async (label, id, idsOfUsers, reason, context) => { + await validateNotifyUsers(label, reason) + let mentionedCypher switch (reason) { case 'mentioned_in_post': { - cypher = ` + mentionedCypher = ` MATCH (post: Post { id: $id })<-[:WROTE]-(author: User) MATCH (user: User) WHERE user.id in $idsOfUsers AND NOT (user)<-[:BLOCKED]-(author) MERGE (post)-[notification:NOTIFIED {reason: $reason}]->(user) - SET notification.read = FALSE - SET ( - CASE - WHEN notification.createdAt IS NULL - THEN notification END ).createdAt = toString(datetime()) - SET notification.updatedAt = toString(datetime()) ` break } case 'mentioned_in_comment': { - cypher = ` - MATCH (postAuthor: User)-[:WROTE]->(post: Post)<-[:COMMENTS]-(comment: Comment { id: $id })<-[:WROTE]-(author: User) - MATCH (user: User) - WHERE user.id in $idsOfUsers - AND NOT (user)<-[:BLOCKED]-(author) - AND NOT (user)<-[:BLOCKED]-(postAuthor) - MERGE (comment)-[notification:NOTIFIED {reason: $reason}]->(user) - SET notification.read = FALSE - SET ( - CASE - WHEN notification.createdAt IS NULL - THEN notification END ).createdAt = toString(datetime()) - SET notification.updatedAt = toString(datetime()) - ` - break - } - case 'commented_on_post': { - cypher = ` - MATCH (postAuthor: User)-[:WROTE]->(post: Post)<-[:COMMENTS]-(comment: Comment { id: $id })<-[:WROTE]-(author: User) - MATCH (user: User) - WHERE user.id in $idsOfUsers - AND NOT (user)<-[:BLOCKED]-(author) - AND NOT (author)<-[:BLOCKED]-(user) - MERGE (comment)-[notification:NOTIFIED {reason: $reason}]->(user) - SET notification.read = FALSE - SET ( - CASE - WHEN notification.createdAt IS NULL - THEN notification END ).createdAt = toString(datetime()) - SET notification.updatedAt = toString(datetime()) + mentionedCypher = ` + MATCH (postAuthor: User)-[:WROTE]->(post: Post)<-[:COMMENTS]-(comment: Comment { id: $id })<-[:WROTE]-(author: User) + MATCH (user: User) + WHERE user.id in $idsOfUsers + AND NOT (user)<-[:BLOCKED]-(author) + AND NOT (user)<-[:BLOCKED]-(postAuthor) + MERGE (comment)-[notification:NOTIFIED {reason: $reason}]->(user) ` break } } + mentionedCypher += ` + SET notification.read = FALSE + SET ( + CASE + WHEN notification.createdAt IS NULL + THEN notification END ).createdAt = toString(datetime()) + SET notification.updatedAt = toString(datetime()) + ` const session = context.driver.session() try { await session.writeTransaction(transaction => { - return transaction.run(cypher, { id, idsOfUsers, reason }) + return transaction.run(mentionedCypher, { id, idsOfUsers, reason }) }) } finally { session.close() } } -const handleContentDataOfPost = async (resolve, root, args, context, resolveInfo) => { - const idsOfUsers = extractMentionedUsers(args.content) - const post = await resolve(root, args, context, resolveInfo) - if (post) await notifyUsers('Post', post.id, idsOfUsers, 'mentioned_in_post', context) - return post -} +const notifyUsersOfComment = async (label, commentId, postAuthorId, reason, context) => { + await validateNotifyUsers(label, reason) + const session = context.driver.session() -const handleContentDataOfComment = async (resolve, root, args, context, resolveInfo) => { - const { content, id: commentId } = args - let idsOfUsers = extractMentionedUsers(content) - const [postAuthor] = await postAuthorOfComment(commentId, { context }) - idsOfUsers = idsOfUsers.filter(id => id !== postAuthor.id) - if (idsOfUsers && idsOfUsers.length) - await notifyUsers('Comment', commentId, idsOfUsers, 'mentioned_in_comment', context) - if (context.user.id !== postAuthor.id) - await notifyUsers('Comment', commentId, [postAuthor.id], 'commented_on_post', context) -} - -const handleCreateComment = async (resolve, root, args, context, resolveInfo) => { - const comment = await resolve(root, args, context, resolveInfo) - if (comment) return handleContentDataOfComment(resolve, root, args, context, resolveInfo) + try { + return session.writeTransaction(async transaction => { + await transaction.run( + ` + MATCH (postAuthor:User {id: $postAuthorId})-[:WROTE]->(post:Post)<-[:COMMENTS]-(comment:Comment { id: $commentId })<-[:WROTE]-(commenter:User) + WHERE NOT (postAuthor)-[:BLOCKED]-(commenter) + MERGE (comment)-[notification:NOTIFIED {reason: $reason}]->(postAuthor) + SET notification.read = FALSE + SET ( + CASE + WHEN notification.createdAt IS NULL + THEN notification END ).createdAt = toString(datetime()) + SET notification.updatedAt = toString(datetime()) + `, + { commentId, postAuthorId, reason }, + ) + }) + } finally { + session.close() + } } export default { diff --git a/backend/src/middleware/notifications/notificationsMiddleware.spec.js b/backend/src/middleware/notifications/notificationsMiddleware.spec.js index 6fd94ab69..c218ed1cc 100644 --- a/backend/src/middleware/notifications/notificationsMiddleware.spec.js +++ b/backend/src/middleware/notifications/notificationsMiddleware.spec.js @@ -35,7 +35,8 @@ const createCommentMutation = gql` } ` -beforeAll(() => { +beforeAll(async () => { + await factory.cleanDatabase() const createServerResult = createServer({ context: () => { return { @@ -169,7 +170,6 @@ describe('notifications', () => { ], }, }) - const { query } = createTestClient(server) await expect( query({ query: notificationQuery, @@ -186,7 +186,7 @@ describe('notifications', () => { const expected = expect.objectContaining({ data: { notifications: [] }, }) - const { query } = createTestClient(server) + await expect( query({ query: notificationQuery, @@ -210,7 +210,7 @@ describe('notifications', () => { const expected = expect.objectContaining({ data: { notifications: [] }, }) - const { query } = createTestClient(server) + await expect( query({ query: notificationQuery, @@ -261,7 +261,7 @@ describe('notifications', () => { ], }, }) - const { query } = createTestClient(server) + await expect( query({ query: notificationQuery, @@ -405,7 +405,7 @@ describe('notifications', () => { const expected = expect.objectContaining({ data: { notifications: [] }, }) - const { query } = createTestClient(server) + await expect( query({ query: notificationQuery, @@ -463,7 +463,7 @@ describe('notifications', () => { ], }, }) - const { query } = createTestClient(server) + await expect( query({ query: notificationQuery, @@ -497,7 +497,7 @@ describe('notifications', () => { ], }, }) - const { query } = createTestClient(server) + await expect( query({ query: notificationQuery, @@ -528,7 +528,7 @@ describe('notifications', () => { const expected = expect.objectContaining({ data: { notifications: [] }, }) - const { query } = createTestClient(server) + await expect( query({ query: notificationQuery, diff --git a/backend/src/middleware/validation/validationMiddleware.js b/backend/src/middleware/validation/validationMiddleware.js index 4413b7747..7a334db5d 100644 --- a/backend/src/middleware/validation/validationMiddleware.js +++ b/backend/src/middleware/validation/validationMiddleware.js @@ -115,6 +115,17 @@ const validateReview = async (resolve, root, args, context, info) => { return resolve(root, args, context, info) } +export const validateNotifyUsers = async (label, reason) => { + const reasonsAllowed = ['mentioned_in_post', 'mentioned_in_comment', 'commented_on_post'] + if (!reasonsAllowed.includes(reason)) throw new Error('Notification reason is not allowed!') + if ( + (label === 'Post' && reason !== 'mentioned_in_post') || + (label === 'Comment' && !['mentioned_in_comment', 'commented_on_post'].includes(reason)) + ) { + throw new Error('Notification does not fit the reason!') + } +} + export default { Mutation: { CreateComment: validateCreateComment, From a1acd488214befcf65819890cbb008950215793a Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Fri, 6 Dec 2019 14:56:14 +0100 Subject: [PATCH 07/32] Use verbose variables --- backend/src/jwt/decode.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/jwt/decode.js b/backend/src/jwt/decode.js index 41e9d005e..5433a8c76 100644 --- a/backend/src/jwt/decode.js +++ b/backend/src/jwt/decode.js @@ -13,8 +13,8 @@ export default async (driver, authorizationHeader) => { } const session = driver.session() - const writeTxResultPromise = session.writeTransaction(async txc => { - const updateUserLastActiveTransactionResponse = await txc.run( + const writeTxResultPromise = session.writeTransaction(async transaction => { + const updateUserLastActiveTransactionResponse = await transaction.run( ` MATCH (user:User {id: $id, deleted: false, disabled: false }) SET user.lastActiveAt = toString(datetime()) From 0e757cf94facdfd0381d0407f01211f8cd756013 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Fri, 6 Dec 2019 17:48:14 +0100 Subject: [PATCH 08/32] Refactor to fix test, DRY out code --- .../notifications/notificationsMiddleware.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/backend/src/middleware/notifications/notificationsMiddleware.js b/backend/src/middleware/notifications/notificationsMiddleware.js index 4daf2d770..837193773 100644 --- a/backend/src/middleware/notifications/notificationsMiddleware.js +++ b/backend/src/middleware/notifications/notificationsMiddleware.js @@ -9,20 +9,17 @@ const handleContentDataOfPost = async (resolve, root, args, context, resolveInfo return post } -const handleCreateComment = async (resolve, root, args, context, resolveInfo) => { - const comment = await resolve(root, args, context, resolveInfo) - if (comment) return handleContentDataOfComment(resolve, root, args, context, resolveInfo) -} - const handleContentDataOfComment = async (resolve, root, args, context, resolveInfo) => { - const { content, id: commentId } = args + const { content } = args let idsOfUsers = extractMentionedUsers(content) - const [postAuthor] = await postAuthorOfComment(commentId, { context }) + const comment = await resolve(root, args, context, resolveInfo) + const [postAuthor] = await postAuthorOfComment(comment.id, { context }) idsOfUsers = idsOfUsers.filter(id => id !== postAuthor.id) if (idsOfUsers && idsOfUsers.length) - await notifyUsersOfMention('Comment', commentId, idsOfUsers, 'mentioned_in_comment', context) + await notifyUsersOfMention('Comment', comment.id, idsOfUsers, 'mentioned_in_comment', context) if (context.user.id !== postAuthor.id) - await notifyUsersOfComment('Comment', commentId, postAuthor.id, 'commented_on_post', context) + await notifyUsersOfComment('Comment', comment.id, postAuthor.id, 'commented_on_post', context) + return comment } const postAuthorOfComment = async (commentId, { context }) => { @@ -93,7 +90,7 @@ const notifyUsersOfComment = async (label, commentId, postAuthorId, reason, cont const session = context.driver.session() try { - return session.writeTransaction(async transaction => { + await session.writeTransaction(async transaction => { await transaction.run( ` MATCH (postAuthor:User {id: $postAuthorId})-[:WROTE]->(post:Post)<-[:COMMENTS]-(comment:Comment { id: $commentId })<-[:WROTE]-(commenter:User) @@ -118,7 +115,7 @@ export default { Mutation: { CreatePost: handleContentDataOfPost, UpdatePost: handleContentDataOfPost, - CreateComment: handleCreateComment, + CreateComment: handleContentDataOfComment, UpdateComment: handleContentDataOfComment, }, } From 6f4ee5f3b725b3ccd6d59a13d32b76b370c1396e Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Fri, 6 Dec 2019 17:48:34 +0100 Subject: [PATCH 09/32] Refactor resolver/spec - favor transaction functions - add errors undefined, clean up specs --- backend/src/schema/resolvers/comments.js | 89 +++++++++++-------- backend/src/schema/resolvers/comments.spec.js | 10 ++- 2 files changed, 59 insertions(+), 40 deletions(-) diff --git a/backend/src/schema/resolvers/comments.js b/backend/src/schema/resolvers/comments.js index 97b461511..864d9412c 100644 --- a/backend/src/schema/resolvers/comments.js +++ b/backend/src/schema/resolvers/comments.js @@ -5,6 +5,7 @@ export default { Mutation: { CreateComment: async (object, params, context, resolveInfo) => { const { postId } = params + const { user, driver } = context // Adding relationship from comment to post by passing in the postId, // but we do not want to create the comment with postId as an attribute // because we use relationships for this. So, we are deleting it from params @@ -12,26 +13,28 @@ export default { delete params.postId params.id = params.id || uuid() - const session = context.driver.session() + const session = driver.session() + + const writeTxResultPromise = session.writeTransaction(async transaction => { + const createCommentTransactionResponse = await transaction.run( + ` + MATCH (post:Post {id: $postId}) + MATCH (author:User {id: $userId}) + WITH post, author + CREATE (comment:Comment {params}) + SET comment.createdAt = toString(datetime()) + SET comment.updatedAt = toString(datetime()) + MERGE (post)<-[:COMMENTS]-(comment)<-[:WROTE]-(author) + RETURN comment + `, + { userId: user.id, postId, params }, + ) + return createCommentTransactionResponse.records.map( + record => record.get('comment').properties, + ) + }) try { - const createCommentCypher = ` - MATCH (post:Post {id: $postId}) - MATCH (author:User {id: $userId}) - WITH post, author - CREATE (comment:Comment {params}) - SET comment.createdAt = toString(datetime()) - SET comment.updatedAt = toString(datetime()) - MERGE (post)<-[:COMMENTS]-(comment)<-[:WROTE]-(author) - RETURN comment - ` - const transactionRes = await session.run(createCommentCypher, { - userId: context.user.id, - postId, - params, - }) - - const [comment] = transactionRes.records.map(record => record.get('comment').properties) - + const [comment] = await writeTxResultPromise return comment } finally { session.close() @@ -39,15 +42,22 @@ export default { }, UpdateComment: async (_parent, params, context, _resolveInfo) => { const session = context.driver.session() + const writeTxResultPromise = session.writeTransaction(async transaction => { + const updateCommentTransactionResponse = await transaction.run( + ` + MATCH (comment:Comment {id: $params.id}) + SET comment += $params + SET comment.updatedAt = toString(datetime()) + RETURN comment + `, + { params }, + ) + return updateCommentTransactionResponse.records.map( + record => record.get('comment').properties, + ) + }) try { - const updateCommentCypher = ` - MATCH (comment:Comment {id: $params.id}) - SET comment += $params - SET comment.updatedAt = toString(datetime()) - RETURN comment - ` - const transactionRes = await session.run(updateCommentCypher, { params }) - const [comment] = transactionRes.records.map(record => record.get('comment').properties) + const [comment] = await writeTxResultPromise return comment } finally { session.close() @@ -55,18 +65,23 @@ export default { }, DeleteComment: async (_parent, args, context, _resolveInfo) => { const session = context.driver.session() - try { - const transactionRes = await session.run( - ` - MATCH (comment:Comment {id: $commentId}) - SET comment.deleted = TRUE - SET comment.content = 'UNAVAILABLE' - SET comment.contentExcerpt = 'UNAVAILABLE' - RETURN comment - `, + const writeTxResultPromise = session.writeTransaction(async transaction => { + const deleteCommentTransactionResponse = await transaction.run( + ` + MATCH (comment:Comment {id: $commentId}) + SET comment.deleted = TRUE + SET comment.content = 'UNAVAILABLE' + SET comment.contentExcerpt = 'UNAVAILABLE' + RETURN comment + `, { commentId: args.id }, ) - const [comment] = transactionRes.records.map(record => record.get('comment').properties) + return deleteCommentTransactionResponse.records.map( + record => record.get('comment').properties, + ) + }) + try { + const [comment] = await writeTxResultPromise return comment } finally { session.close() diff --git a/backend/src/schema/resolvers/comments.spec.js b/backend/src/schema/resolvers/comments.spec.js index d2692aa8a..aa504e19e 100644 --- a/backend/src/schema/resolvers/comments.spec.js +++ b/backend/src/schema/resolvers/comments.spec.js @@ -10,7 +10,8 @@ const factory = Factory() let variables, mutate, authenticatedUser, commentAuthor, newlyCreatedComment -beforeAll(() => { +beforeAll(async () => { + await factory.cleanDatabase() const { server } = createServer({ context: () => { return { @@ -19,8 +20,7 @@ beforeAll(() => { } }, }) - const client = createTestClient(server) - mutate = client.mutate + mutate = createTestClient(server).mutate }) beforeEach(async () => { @@ -100,6 +100,7 @@ describe('CreateComment', () => { await expect(mutate({ mutation: createCommentMutation, variables })).resolves.toMatchObject( { data: { CreateComment: { content: "I'm authorised to comment" } }, + errors: undefined, }, ) }) @@ -108,6 +109,7 @@ describe('CreateComment', () => { await expect(mutate({ mutation: createCommentMutation, variables })).resolves.toMatchObject( { data: { CreateComment: { author: { name: 'Author' } } }, + errors: undefined, }, ) }) @@ -157,6 +159,7 @@ describe('UpdateComment', () => { it('updates the comment', async () => { const expected = { data: { UpdateComment: { id: 'c456', content: 'The comment is updated' } }, + errors: undefined, } await expect(mutate({ mutation: updateCommentMutation, variables })).resolves.toMatchObject( expected, @@ -172,6 +175,7 @@ describe('UpdateComment', () => { createdAt: expect.any(String), }, }, + errors: undefined, } await expect(mutate({ mutation: updateCommentMutation, variables })).resolves.toMatchObject( expected, From 6ed435364caf27a2652add711ab4143ffc80e1d8 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Fri, 6 Dec 2019 18:29:50 +0100 Subject: [PATCH 10/32] Start updating posts resolver --- backend/src/schema/resolvers/posts.js | 121 +++++++++++++++----------- 1 file changed, 68 insertions(+), 53 deletions(-) diff --git a/backend/src/schema/resolvers/posts.js b/backend/src/schema/resolvers/posts.js index 2bd229b84..eaab1283c 100644 --- a/backend/src/schema/resolvers/posts.js +++ b/backend/src/schema/resolvers/posts.js @@ -57,17 +57,20 @@ export default { PostsEmotionsCountByEmotion: async (object, params, context, resolveInfo) => { const { postId, data } = params const session = context.driver.session() - try { - const transactionRes = await session.run( - `MATCH (post:Post {id: $postId})<-[emoted:EMOTED {emotion: $data.emotion}]-() - RETURN COUNT(DISTINCT emoted) as emotionsCount - `, + const readTxResultPromise = session.readTransaction(async transaction => { + const emotionsCountTransactionResponse = await transaction.run( + ` + MATCH (post:Post {id: $postId})<-[emoted:EMOTED {emotion: $data.emotion}]-() + RETURN COUNT(DISTINCT emoted) as emotionsCount + `, { postId, data }, ) - - const [emotionsCount] = transactionRes.records.map(record => { - return record.get('emotionsCount').low - }) + return emotionsCountTransactionResponse.records.map( + record => record.get('emotionsCount').low, + ) + }) + try { + const [emotionsCount] = await readTxResultPromise return emotionsCount } finally { session.close() @@ -76,16 +79,18 @@ export default { PostsEmotionsByCurrentUser: async (object, params, context, resolveInfo) => { const { postId } = params const session = context.driver.session() - try { - const transactionRes = await session.run( - `MATCH (user:User {id: $userId})-[emoted:EMOTED]->(post:Post {id: $postId}) - RETURN collect(emoted.emotion) as emotion`, + const readTxResultPromise = session.readTransaction(async transaction => { + const emotionsTransactionResponse = await transaction.run( + ` + MATCH (user:User {id: $userId})-[emoted:EMOTED]->(post:Post {id: $postId}) + RETURN collect(emoted.emotion) as emotion + `, { userId: context.user.id, postId }, ) - - const [emotions] = transactionRes.records.map(record => { - return record.get('emotion') - }) + return emotionsTransactionResponse.records.map(record => record.get('emotion')) + }) + try { + const [emotions] = await readTxResultPromise return emotions } finally { session.close() @@ -98,25 +103,29 @@ export default { delete params.categoryIds params = await fileUpload(params, { file: 'imageUpload', url: 'image' }) params.id = params.id || uuid() - const createPostCypher = `CREATE (post:Post {params}) - SET post.createdAt = toString(datetime()) - SET post.updatedAt = toString(datetime()) - WITH post - MATCH (author:User {id: $userId}) - MERGE (post)<-[:WROTE]-(author) - WITH post - UNWIND $categoryIds AS categoryId - MATCH (category:Category {id: categoryId}) - MERGE (post)-[:CATEGORIZED]->(category) - RETURN post` - - const createPostVariables = { userId: context.user.id, categoryIds, params } - const session = context.driver.session() + const writeTxResultPromise = session.writeTransaction(async transaction => { + const createPostTransactionResponse = await transaction.run( + ` + CREATE (post:Post {params}) + SET post.createdAt = toString(datetime()) + SET post.updatedAt = toString(datetime()) + WITH post + MATCH (author:User {id: $userId}) + MERGE (post)<-[:WROTE]-(author) + WITH post + UNWIND $categoryIds AS categoryId + MATCH (category:Category {id: categoryId}) + MERGE (post)-[:CATEGORIZED]->(category) + RETURN post + `, + { userId: context.user.id, categoryIds, params }, + ) + return createPostTransactionResponse.records.map(record => record.get('post').properties) + }) try { - const transactionRes = await session.run(createPostCypher, createPostVariables) - const posts = transactionRes.records.map(record => record.get('post').properties) - return posts[0] + const [posts] = await writeTxResultPromise + return posts } catch (e) { if (e.code === 'Neo.ClientError.Schema.ConstraintValidationFailed') throw new UserInputError('Post with this slug already exists!') @@ -129,38 +138,44 @@ export default { const { categoryIds } = params delete params.categoryIds params = await fileUpload(params, { file: 'imageUpload', url: 'image' }) - let updatePostCypher = `MATCH (post:Post {id: $params.id}) - SET post += $params - SET post.updatedAt = toString(datetime()) - WITH post - ` - const session = context.driver.session() - try { - if (categoryIds && categoryIds.length) { - const cypherDeletePreviousRelations = ` + let updatePostCypher = ` + MATCH (post:Post {id: $params.id}) + SET post += $params + SET post.updatedAt = toString(datetime()) + WITH post + ` + + if (categoryIds && categoryIds.length) { + const cypherDeletePreviousRelations = ` MATCH (post:Post { id: $params.id })-[previousRelations:CATEGORIZED]->(category:Category) DELETE previousRelations RETURN post, category - ` + ` - await session.run(cypherDeletePreviousRelations, { params }) + await session.writeTransaction(transaction => { + return transaction.run(cypherDeletePreviousRelations, { params }) + }) - updatePostCypher += ` + updatePostCypher += ` UNWIND $categoryIds AS categoryId MATCH (category:Category {id: categoryId}) MERGE (post)-[:CATEGORIZED]->(category) WITH post ` - } + } - updatePostCypher += `RETURN post` - const updatePostVariables = { categoryIds, params } - - const transactionRes = await session.run(updatePostCypher, updatePostVariables) - const [post] = transactionRes.records.map(record => { - return record.get('post').properties + updatePostCypher += `RETURN post` + const updatePostVariables = { categoryIds, params } + try { + const writeTxResultPromise = session.writeTransaction(async transaction => { + const updatePostTransactionResponse = await transaction.run( + updatePostCypher, + updatePostVariables, + ) + return updatePostTransactionResponse.records.map(record => record.get('post').properties) }) + const [post] = await writeTxResultPromise return post } finally { session.close() From 3bc944b06da64db22e9c493c2044f6dfa7dc5f17 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Fri, 6 Dec 2019 18:56:20 +0100 Subject: [PATCH 11/32] Finish refactoring posts resolver --- backend/src/schema/resolvers/posts.js | 95 ++++++++++++++++----------- 1 file changed, 56 insertions(+), 39 deletions(-) diff --git a/backend/src/schema/resolvers/posts.js b/backend/src/schema/resolvers/posts.js index eaab1283c..7f4666450 100644 --- a/backend/src/schema/resolvers/posts.js +++ b/backend/src/schema/resolvers/posts.js @@ -124,8 +124,8 @@ export default { return createPostTransactionResponse.records.map(record => record.get('post').properties) }) try { - const [posts] = await writeTxResultPromise - return posts + const [post] = await writeTxResultPromise + return post } catch (e) { if (e.code === 'Neo.ClientError.Schema.ConstraintValidationFailed') throw new UserInputError('Post with this slug already exists!') @@ -184,23 +184,25 @@ export default { DeletePost: async (object, args, context, resolveInfo) => { const session = context.driver.session() - try { - // we cannot set slug to 'UNAVAILABE' because of unique constraints - const transactionRes = await session.run( + const writeTxResultPromise = session.writeTransaction(async transaction => { + const deletePostTransactionResponse = await transaction.run( ` - MATCH (post:Post {id: $postId}) - OPTIONAL MATCH (post)<-[:COMMENTS]-(comment:Comment) - SET post.deleted = TRUE - SET post.content = 'UNAVAILABLE' - SET post.contentExcerpt = 'UNAVAILABLE' - SET post.title = 'UNAVAILABLE' - SET comment.deleted = TRUE - REMOVE post.image - RETURN post - `, + MATCH (post:Post {id: $postId}) + OPTIONAL MATCH (post)<-[:COMMENTS]-(comment:Comment) + SET post.deleted = TRUE + SET post.content = 'UNAVAILABLE' + SET post.contentExcerpt = 'UNAVAILABLE' + SET post.title = 'UNAVAILABLE' + SET comment.deleted = TRUE + REMOVE post.image + RETURN post + `, { postId: args.id }, ) - const [post] = transactionRes.records.map(record => record.get('post').properties) + return deletePostTransactionResponse.records.map(record => record.get('post').properties) + }) + try { + const [post] = await writeTxResultPromise return post } finally { session.close() @@ -210,21 +212,24 @@ export default { const { to, data } = params const { user } = context const session = context.driver.session() - try { - const transactionRes = await session.run( - `MATCH (userFrom:User {id: $user.id}), (postTo:Post {id: $to.id}) - MERGE (userFrom)-[emotedRelation:EMOTED {emotion: $data.emotion}]->(postTo) - RETURN userFrom, postTo, emotedRelation`, + const writeTxResultPromise = session.writeTransaction(async transaction => { + const addPostEmotionsTransactionResponse = await transaction.run( + ` + MATCH (userFrom:User {id: $user.id}), (postTo:Post {id: $to.id}) + MERGE (userFrom)-[emotedRelation:EMOTED {emotion: $data.emotion}]->(postTo) + RETURN userFrom, postTo, emotedRelation`, { user, to, data }, ) - - const [emoted] = transactionRes.records.map(record => { + return addPostEmotionsTransactionResponse.records.map(record => { return { from: { ...record.get('userFrom').properties }, to: { ...record.get('postTo').properties }, ...record.get('emotedRelation').properties, } }) + }) + try { + const [emoted] = await writeTxResultPromise return emoted } finally { session.close() @@ -234,20 +239,25 @@ export default { const { to, data } = params const { id: from } = context.user const session = context.driver.session() - try { - const transactionRes = await session.run( - `MATCH (userFrom:User {id: $from})-[emotedRelation:EMOTED {emotion: $data.emotion}]->(postTo:Post {id: $to.id}) - DELETE emotedRelation - RETURN userFrom, postTo`, + const writeTxResultPromise = session.writeTransaction(async transaction => { + const removePostEmotionsTransactionResponse = await transaction.run( + ` + MATCH (userFrom:User {id: $from})-[emotedRelation:EMOTED {emotion: $data.emotion}]->(postTo:Post {id: $to.id}) + DELETE emotedRelation + RETURN userFrom, postTo + `, { from, to, data }, ) - const [emoted] = transactionRes.records.map(record => { + return removePostEmotionsTransactionResponse.records.map(record => { return { from: { ...record.get('userFrom').properties }, to: { ...record.get('postTo').properties }, emotion: data.emotion, } }) + }) + try { + const [emoted] = await writeTxResultPromise return emoted } finally { session.close() @@ -351,21 +361,28 @@ export default { relatedContributions: async (parent, params, context, resolveInfo) => { if (typeof parent.relatedContributions !== 'undefined') return parent.relatedContributions const { id } = parent - const statement = ` - MATCH (p:Post {id: $id})-[:TAGGED|CATEGORIZED]->(categoryOrTag)<-[:TAGGED|CATEGORIZED]-(post:Post) - WHERE NOT post.deleted AND NOT post.disabled - RETURN DISTINCT post - LIMIT 10 - ` - let relatedContributions const session = context.driver.session() + + const writeTxResultPromise = session.writeTransaction(async transaction => { + const relatedContributionsTransactionResponse = await transaction.run( + ` + MATCH (p:Post {id: $id})-[:TAGGED|CATEGORIZED]->(categoryOrTag)<-[:TAGGED|CATEGORIZED]-(post:Post) + WHERE NOT post.deleted AND NOT post.disabled + RETURN DISTINCT post + LIMIT 10 + `, + { id }, + ) + return relatedContributionsTransactionResponse.records.map( + record => record.get('post').properties, + ) + }) try { - const result = await session.run(statement, { id }) - relatedContributions = result.records.map(r => r.get('post').properties) + const relatedContributions = await writeTxResultPromise + return relatedContributions } finally { session.close() } - return relatedContributions }, }, } From 760fd0171573cf4865bce3b17b9ea456bad6a25c Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 11:06:44 +0100 Subject: [PATCH 12/32] Finish refactor of notifications resolver --- backend/src/schema/resolvers/notifications.js | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/backend/src/schema/resolvers/notifications.js b/backend/src/schema/resolvers/notifications.js index eca12900d..31369a8c7 100644 --- a/backend/src/schema/resolvers/notifications.js +++ b/backend/src/schema/resolvers/notifications.js @@ -76,16 +76,21 @@ export default { markAsRead: async (parent, args, context, resolveInfo) => { const { user: currentUser } = context const session = context.driver.session() + const writeTxResultPromise = session.writeTransaction(async transaction => { + const markNotificationAsReadTransactionResponse = await transaction.run( + ` + MATCH (resource {id: $resourceId})-[notification:NOTIFIED {read: FALSE}]->(user:User {id:$id}) + SET notification.read = TRUE + RETURN resource, notification, user + `, + { resourceId: args.id, id: currentUser.id }, + ) + log(markNotificationAsReadTransactionResponse) + return markNotificationAsReadTransactionResponse.records.map(transformReturnType) + }) try { - const cypher = ` - MATCH (resource {id: $resourceId})-[notification:NOTIFIED {read: FALSE}]->(user:User {id:$id}) - SET notification.read = TRUE - RETURN resource, notification, user - ` - const result = await session.run(cypher, { resourceId: args.id, id: currentUser.id }) - log(result) - const notifications = await result.records.map(transformReturnType) - return notifications[0] + const [notifications] = await writeTxResultPromise + return notifications } finally { session.close() } From ca9c58c06bf8392561fa73d00771e9683fea7823 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 12:50:46 +0100 Subject: [PATCH 13/32] Add errors undefined to tests - helps with debugging --- backend/src/schema/resolvers/posts.spec.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/backend/src/schema/resolvers/posts.spec.js b/backend/src/schema/resolvers/posts.spec.js index 752602fd9..dcbd16d5d 100644 --- a/backend/src/schema/resolvers/posts.spec.js +++ b/backend/src/schema/resolvers/posts.spec.js @@ -383,7 +383,10 @@ describe('UpdatePost', () => { }) it('updates a post', async () => { - const expected = { data: { UpdatePost: { id: 'p9876', content: 'New content' } } } + const expected = { + data: { UpdatePost: { id: 'p9876', content: 'New content' } }, + errors: undefined, + } await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject( expected, ) @@ -394,6 +397,7 @@ describe('UpdatePost', () => { data: { UpdatePost: { id: 'p9876', content: 'New content', createdAt: expect.any(String) }, }, + errors: undefined, } await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject( expected, @@ -421,6 +425,7 @@ describe('UpdatePost', () => { categories: expect.arrayContaining([{ id: 'cat9' }, { id: 'cat4' }, { id: 'cat15' }]), }, }, + errors: undefined, } await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject( expected, @@ -441,6 +446,7 @@ describe('UpdatePost', () => { categories: expect.arrayContaining([{ id: 'cat27' }]), }, }, + errors: undefined, } await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject( expected, @@ -722,6 +728,7 @@ describe('UpdatePost', () => { }, ], }, + errors: undefined, } variables = { orderBy: ['pinned_desc', 'createdAt_desc'] } await expect(query({ query: postOrderingQuery, variables })).resolves.toMatchObject( From 73a5b394d63a6c3a45792373cd0946edd9413a97 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 12:56:49 +0100 Subject: [PATCH 14/32] Refactor isAuthor to use transaction function --- backend/src/middleware/permissionsMiddleware.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/backend/src/middleware/permissionsMiddleware.js b/backend/src/middleware/permissionsMiddleware.js index 8f139f4c7..3b42ae7fe 100644 --- a/backend/src/middleware/permissionsMiddleware.js +++ b/backend/src/middleware/permissionsMiddleware.js @@ -47,17 +47,18 @@ const isAuthor = rule({ if (!user) return false const { id: resourceId } = args const session = driver.session() - try { - const result = await session.run( + const authorReadTxPromise = session.readTransaction(async transaction => { + const authorTransactionResponse = await transaction.run( ` - MATCH (resource {id: $resourceId})<-[:WROTE]-(author {id: $userId}) - RETURN author - `, + MATCH (resource {id: $resourceId})<-[:WROTE]-(author {id: $userId}) + RETURN author + `, { resourceId, userId: user.id }, ) - const [author] = result.records.map(record => { - return record.get('author') - }) + return authorTransactionResponse.records.map(record => record.get('author')) + }) + try { + const [author] = await authorReadTxPromise return !!author } finally { session.close() From cc0a33ec7d6204e73f65804ff6c0af66b34d1f1e Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 13:26:18 +0100 Subject: [PATCH 15/32] Use transaction function in isUniqueFor --- backend/src/middleware/sluggifyMiddleware.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/backend/src/middleware/sluggifyMiddleware.js b/backend/src/middleware/sluggifyMiddleware.js index cda3fd335..1cd3c0b9c 100644 --- a/backend/src/middleware/sluggifyMiddleware.js +++ b/backend/src/middleware/sluggifyMiddleware.js @@ -4,10 +4,16 @@ const isUniqueFor = (context, type) => { return async slug => { const session = context.driver.session() try { - const response = await session.run(`MATCH(p:${type} {slug: $slug }) return p.slug`, { - slug, + const existingSlug = await session.readTransaction(transaction => { + return transaction.run( + ` + MATCH(p:${type} {slug: $slug }) + RETURN p.slug + `, + { slug }, + ) }) - return response.records.length === 0 + return existingSlug.records.length === 0 } finally { session.close() } From c871ec2632d713713ab24c768bf735601038c285 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 16:29:20 +0100 Subject: [PATCH 16/32] Refactor validationMiddleware --- .../validation/validationMiddleware.js | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/backend/src/middleware/validation/validationMiddleware.js b/backend/src/middleware/validation/validationMiddleware.js index 7a334db5d..606e14c23 100644 --- a/backend/src/middleware/validation/validationMiddleware.js +++ b/backend/src/middleware/validation/validationMiddleware.js @@ -14,14 +14,15 @@ const validateCreateComment = async (resolve, root, args, context, info) => { } const session = context.driver.session() try { - const postQueryRes = await session.run( - ` - MATCH (post:Post {id: $postId}) - RETURN post`, - { - postId, - }, - ) + const postQueryRes = await session.readTransaction(transaction => { + return transaction.run( + ` + MATCH (post:Post {id: $postId}) + RETURN post + `, + { postId }, + ) + }) const [post] = postQueryRes.records.map(record => { return record.get('post') }) @@ -72,8 +73,8 @@ const validateReview = async (resolve, root, args, context, info) => { const { user, driver } = context if (resourceId === user.id) throw new Error('You cannot review yourself!') const session = driver.session() - const reportReadTxPromise = session.readTransaction(async txc => { - const validateReviewTransactionResponse = await txc.run( + const reportReadTxPromise = session.readTransaction(async transaction => { + const validateReviewTransactionResponse = await transaction.run( ` MATCH (resource {id: $resourceId}) WHERE resource:User OR resource:Post OR resource:Comment From b583b02fb4276e5c5d69477844cab760e9b58e04 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 17:57:25 +0100 Subject: [PATCH 17/32] Update createPasswordReset helper function - the test is broken, can you have a look @roschaefer?? - I tried to get it to work, but it's complicated with multiple promises... I'm ok if we remove this test as well as it's only testing that normalizeEmail works as it's supposed to... but that hopefully is tested on the side of the validator library --- .../resolvers/helpers/createPasswordReset.js | 39 +++++++++++-------- .../helpers/createPasswordReset.spec.js | 12 +++--- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/backend/src/schema/resolvers/helpers/createPasswordReset.js b/backend/src/schema/resolvers/helpers/createPasswordReset.js index 41214b501..dec55c893 100644 --- a/backend/src/schema/resolvers/helpers/createPasswordReset.js +++ b/backend/src/schema/resolvers/helpers/createPasswordReset.js @@ -5,24 +5,29 @@ export default async function createPasswordReset(options) { const normalizedEmail = normalizeEmail(email) const session = driver.session() try { - const cypher = ` - MATCH (u:User)-[:PRIMARY_EMAIL]->(e:EmailAddress {email:$email}) - CREATE(pr:PasswordReset {nonce: $nonce, issuedAt: datetime($issuedAt), usedAt: NULL}) - MERGE (u)-[:REQUESTED]->(pr) - RETURN e, pr, u - ` - const transactionRes = await session.run(cypher, { - issuedAt: issuedAt.toISOString(), - nonce, - email: normalizedEmail, + const createPasswordResetTxPromise = session.writeTransaction(async transaction => { + const createPasswordResetTransactionResponse = await transaction.run( + ` + MATCH (user:User)-[:PRIMARY_EMAIL]->(email:EmailAddress {email:$email}) + CREATE(passwordReset:PasswordReset {nonce: $nonce, issuedAt: datetime($issuedAt), usedAt: NULL}) + MERGE (user)-[:REQUESTED]->(passwordReset) + RETURN email, passwordReset, user + `, + { + issuedAt: issuedAt.toISOString(), + nonce, + email: normalizedEmail, + }, + ) + return createPasswordResetTransactionResponse.records.map(record => { + const { email } = record.get('email').properties + const { nonce } = record.get('passwordReset').properties + const { name } = record.get('user').properties + return { email, nonce, name } + }) }) - const records = transactionRes.records.map(record => { - const { email } = record.get('e').properties - const { nonce } = record.get('pr').properties - const { name } = record.get('u').properties - return { email, nonce, name } - }) - return records[0] || {} + const [records] = await createPasswordResetTxPromise + return records || {} } finally { session.close() } diff --git a/backend/src/schema/resolvers/helpers/createPasswordReset.spec.js b/backend/src/schema/resolvers/helpers/createPasswordReset.spec.js index a566e225a..5f673f028 100644 --- a/backend/src/schema/resolvers/helpers/createPasswordReset.spec.js +++ b/backend/src/schema/resolvers/helpers/createPasswordReset.spec.js @@ -10,10 +10,12 @@ describe('createPasswordReset', () => { beforeEach(() => { mockSession = { close() {}, - run: jest.fn().mockReturnValue({ - records: { - map: jest.fn(() => []), - }, + writeTransaction: jest.fn().mockReturnValue({ + run: jest.fn().mockReturnValue({ + records: { + map: jest.fn(() => []), + }, + }), }), } driver = { session: () => mockSession } @@ -22,7 +24,7 @@ describe('createPasswordReset', () => { it('lowercases email address', async () => { const email = 'stRaNGeCaSiNG@ExAmplE.ORG' await createPasswordReset({ driver, email, issuedAt, nonce }) - expect(mockSession.run.mock.calls).toEqual([ + expect(mockSession.writeTransaction.run.mock.calls).toEqual([ [ expect.any(String), expect.objectContaining({ From d39e702e70671ea8271d136227e12fc07b535d7a Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 18:19:40 +0100 Subject: [PATCH 18/32] Update exisitingEmailAddress --- .../resolvers/helpers/existingEmailAddress.js | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/backend/src/schema/resolvers/helpers/existingEmailAddress.js b/backend/src/schema/resolvers/helpers/existingEmailAddress.js index ee1a6af82..960b2066f 100644 --- a/backend/src/schema/resolvers/helpers/existingEmailAddress.js +++ b/backend/src/schema/resolvers/helpers/existingEmailAddress.js @@ -1,25 +1,29 @@ import { UserInputError } from 'apollo-server' export default async function alreadyExistingMail({ args, context }) { - const cypher = ` - MATCH (email:EmailAddress {email: $email}) - OPTIONAL MATCH (email)-[:BELONGS_TO]-(user) - RETURN email, user - ` - let transactionRes const session = context.driver.session() try { - transactionRes = await session.run(cypher, { email: args.email }) + const existingEmailAddressTxPromise = session.writeTransaction(async transaction => { + const existingEmailAddressTransactionResponse = await transaction.run( + ` + MATCH (email:EmailAddress {email: $email}) + OPTIONAL MATCH (email)-[:BELONGS_TO]-(user) + RETURN email, user + `, + { email: args.email }, + ) + return existingEmailAddressTransactionResponse.records.map(record => { + return { + alreadyExistingEmail: record.get('email').properties, + user: record.get('user') && record.get('user').properties, + } + }) + }) + const [emailBelongsToUser] = await existingEmailAddressTxPromise + const { alreadyExistingEmail, user } = emailBelongsToUser || {} + if (user) throw new UserInputError('A user account with this email already exists.') + return alreadyExistingEmail } finally { session.close() } - const [result] = transactionRes.records.map(record => { - return { - alreadyExistingEmail: record.get('email').properties, - user: record.get('user') && record.get('user').properties, - } - }) - const { alreadyExistingEmail, user } = result || {} - if (user) throw new UserInputError('A user account with this email already exists.') - return alreadyExistingEmail } From 3c6932e21a4361bd774921b46593a402554b7629 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 18:43:36 +0100 Subject: [PATCH 19/32] Update passwordReset resolver/spec --- backend/src/schema/resolvers/passwordReset.js | 36 ++++++++++--------- .../schema/resolvers/passwordReset.spec.js | 13 +++---- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/backend/src/schema/resolvers/passwordReset.js b/backend/src/schema/resolvers/passwordReset.js index dfbfe8183..74c71e011 100644 --- a/backend/src/schema/resolvers/passwordReset.js +++ b/backend/src/schema/resolvers/passwordReset.js @@ -12,25 +12,29 @@ export default { const stillValid = new Date() stillValid.setDate(stillValid.getDate() - 1) const encryptedNewPassword = await bcrypt.hashSync(newPassword, 10) - const cypher = ` - MATCH (pr:PasswordReset {nonce: $nonce}) - MATCH (e:EmailAddress {email: $email})<-[:PRIMARY_EMAIL]-(u:User)-[:REQUESTED]->(pr) - WHERE duration.between(pr.issuedAt, datetime()).days <= 0 AND pr.usedAt IS NULL - SET pr.usedAt = datetime() - SET u.encryptedPassword = $encryptedNewPassword - RETURN pr - ` const session = driver.session() try { - const transactionRes = await session.run(cypher, { - stillValid, - email, - nonce, - encryptedNewPassword, + const passwordResetTxPromise = session.writeTransaction(async transaction => { + const passwordResetTransactionResponse = await transaction.run( + ` + MATCH (passwordReset:PasswordReset {nonce: $nonce}) + MATCH (email:EmailAddress {email: $email})<-[:PRIMARY_EMAIL]-(user:User)-[:REQUESTED]->(passwordReset) + WHERE duration.between(passwordReset.issuedAt, datetime()).days <= 0 AND passwordReset.usedAt IS NULL + SET passwordReset.usedAt = datetime() + SET user.encryptedPassword = $encryptedNewPassword + RETURN passwordReset + `, + { + stillValid, + email, + nonce, + encryptedNewPassword, + }, + ) + return passwordResetTransactionResponse.records.map(record => record.get('passwordReset')) }) - const [reset] = transactionRes.records.map(record => record.get('pr')) - const response = !!(reset && reset.properties.usedAt) - return response + const [reset] = await passwordResetTxPromise + return !!(reset && reset.properties.usedAt) } finally { session.close() } diff --git a/backend/src/schema/resolvers/passwordReset.spec.js b/backend/src/schema/resolvers/passwordReset.spec.js index a1968d288..be3c8c085 100644 --- a/backend/src/schema/resolvers/passwordReset.spec.js +++ b/backend/src/schema/resolvers/passwordReset.spec.js @@ -14,14 +14,11 @@ let authenticatedUser let variables const getAllPasswordResets = async () => { - const session = driver.session() - try { - const transactionRes = await session.run('MATCH (r:PasswordReset) RETURN r') - const resets = transactionRes.records.map(record => record.get('r')) - return resets - } finally { - session.close() - } + const passwordResetQuery = await neode.cypher( + 'MATCH (passwordReset:PasswordReset) RETURN passwordReset', + ) + const resets = passwordResetQuery.records.map(record => record.get('passwordReset')) + return resets } beforeEach(() => { From b1c5c4dbf9aebb5fa661cb99fc02e2e394ae24c9 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 18:44:01 +0100 Subject: [PATCH 20/32] Avoid testing third-party code - This test, though I understand why it was added, is not necessary in my opinion. It's more difficult to get this test to pass since we don't call session.run, we call session.writeTransaction which has a callback that calls transaction.run... - I think we don't need to test that our third party library does what it was added to do... they have their own tests, which can be found here @roschaefer, which I think are sufficient https://github.com/validatorjs/validator.js/blob/master/test/sanitizers.js - We can always add another type of test, if you feel necessary, maybe an e2e? --- .../helpers/createPasswordReset.spec.js | 37 ------------------- 1 file changed, 37 deletions(-) delete mode 100644 backend/src/schema/resolvers/helpers/createPasswordReset.spec.js diff --git a/backend/src/schema/resolvers/helpers/createPasswordReset.spec.js b/backend/src/schema/resolvers/helpers/createPasswordReset.spec.js deleted file mode 100644 index 5f673f028..000000000 --- a/backend/src/schema/resolvers/helpers/createPasswordReset.spec.js +++ /dev/null @@ -1,37 +0,0 @@ -import createPasswordReset from './createPasswordReset' - -describe('createPasswordReset', () => { - const issuedAt = new Date() - const nonce = 'abcdef' - - describe('email lookup', () => { - let driver - let mockSession - beforeEach(() => { - mockSession = { - close() {}, - writeTransaction: jest.fn().mockReturnValue({ - run: jest.fn().mockReturnValue({ - records: { - map: jest.fn(() => []), - }, - }), - }), - } - driver = { session: () => mockSession } - }) - - it('lowercases email address', async () => { - const email = 'stRaNGeCaSiNG@ExAmplE.ORG' - await createPasswordReset({ driver, email, issuedAt, nonce }) - expect(mockSession.writeTransaction.run.mock.calls).toEqual([ - [ - expect.any(String), - expect.objectContaining({ - email: 'strangecasing@example.org', - }), - ], - ]) - }) - }) -}) From 18ab7186f56e60b997b432498d1b226db2ff10c6 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 18:53:08 +0100 Subject: [PATCH 21/32] Favor transaction functions over auto-commit --- backend/src/schema/resolvers/rewards.js | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/backend/src/schema/resolvers/rewards.js b/backend/src/schema/resolvers/rewards.js index 4d5d62aea..44bdab770 100644 --- a/backend/src/schema/resolvers/rewards.js +++ b/backend/src/schema/resolvers/rewards.js @@ -24,18 +24,19 @@ export default { const { user } = await getUserAndBadge(params) const session = context.driver.session() try { - // silly neode cannot remove relationships - await session.run( - ` - MATCH (badge:Badge {id: $badgeKey})-[reward:REWARDED]->(rewardedUser:User {id: $userId}) - DELETE reward - RETURN rewardedUser - `, - { - badgeKey, - userId, - }, - ) + await session.writeTransaction(transaction => { + return transaction.run( + ` + MATCH (badge:Badge {id: $badgeKey})-[reward:REWARDED]->(rewardedUser:User {id: $userId}) + DELETE reward + RETURN rewardedUser + `, + { + badgeKey, + userId, + }, + ) + }) } finally { session.close() } From 1e85cbb6a2b061004022c4cf38564c8d97cb3e95 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 19:01:29 +0100 Subject: [PATCH 22/32] Refactor shout/unshout mutations - Remove unrecommended auto-commit transactions from code base - Favor transaction functions --- backend/src/schema/resolvers/shout.js | 64 +++++++++++++++------------ 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/backend/src/schema/resolvers/shout.js b/backend/src/schema/resolvers/shout.js index ada1172a4..70ebdf7ae 100644 --- a/backend/src/schema/resolvers/shout.js +++ b/backend/src/schema/resolvers/shout.js @@ -1,3 +1,5 @@ +import log from './helpers/databaseLogger' + export default { Mutation: { shout: async (_object, params, context, _resolveInfo) => { @@ -5,22 +7,24 @@ export default { const session = context.driver.session() try { - const transactionRes = await session.run( - `MATCH (node {id: $id})<-[:WROTE]-(userWritten:User), (user:User {id: $userId}) - WHERE $type IN labels(node) AND NOT userWritten.id = $userId - MERGE (user)-[relation:SHOUTED{createdAt:toString(datetime())}]->(node) - RETURN COUNT(relation) > 0 as isShouted`, - { - id, - type, - userId: context.user.id, - }, - ) - - const [isShouted] = transactionRes.records.map(record => { - return record.get('isShouted') + const shoutWriteTxResultPromise = session.writeTransaction(async transaction => { + const shoutTransactionResponse = await transaction.run( + ` + MATCH (node {id: $id})<-[:WROTE]-(userWritten:User), (user:User {id: $userId}) + WHERE $type IN labels(node) AND NOT userWritten.id = $userId + MERGE (user)-[relation:SHOUTED{createdAt:toString(datetime())}]->(node) + RETURN COUNT(relation) > 0 as isShouted + `, + { + id, + type, + userId: context.user.id, + }, + ) + log(shoutTransactionResponse) + return shoutTransactionResponse.records.map(record => record.get('isShouted')) }) - + const [isShouted] = await shoutWriteTxResultPromise return isShouted } finally { session.close() @@ -31,20 +35,24 @@ export default { const { id, type } = params const session = context.driver.session() try { - const transactionRes = await session.run( - `MATCH (user:User {id: $userId})-[relation:SHOUTED]->(node {id: $id}) - WHERE $type IN labels(node) - DELETE relation - RETURN COUNT(relation) > 0 as isShouted`, - { - id, - type, - userId: context.user.id, - }, - ) - const [isShouted] = transactionRes.records.map(record => { - return record.get('isShouted') + const unshoutWriteTxResultPromise = session.writeTransaction(async transaction => { + const unshoutTransactionResponse = await transaction.run( + ` + MATCH (user:User {id: $userId})-[relation:SHOUTED]->(node {id: $id}) + WHERE $type IN labels(node) + DELETE relation + RETURN COUNT(relation) > 0 as isShouted + `, + { + id, + type, + userId: context.user.id, + }, + ) + log(unshoutTransactionResponse) + return unshoutTransactionResponse.records.map(record => record.get('isShouted')) }) + const [isShouted] = await unshoutWriteTxResultPromise return isShouted } finally { session.close() From 6ef9ca3343c8e6976dce06e563f49326b7665e00 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 19:09:38 +0100 Subject: [PATCH 23/32] Refactor to use readTransaction --- backend/src/schema/resolvers/statistics.js | 39 ++++++++++++---------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/backend/src/schema/resolvers/statistics.js b/backend/src/schema/resolvers/statistics.js index 07b9e4cea..7ca9239f3 100644 --- a/backend/src/schema/resolvers/statistics.js +++ b/backend/src/schema/resolvers/statistics.js @@ -1,8 +1,10 @@ +import log from './helpers/databaseLogger' + export default { Query: { statistics: async (_parent, _args, { driver }) => { const session = driver.session() - const response = {} + const counts = {} try { const mapping = { countUsers: 'User', @@ -13,27 +15,28 @@ export default { countFollows: 'FOLLOWS', countShouts: 'SHOUTED', } - const cypher = ` - CALL apoc.meta.stats() YIELD labels, relTypesCount - RETURN labels, relTypesCount - ` - const result = await session.run(cypher) - const [statistics] = await result.records.map(record => { - return { - ...record.get('labels'), - ...record.get('relTypesCount'), - } + const statisticsReadTxResultPromise = session.readTransaction(async transaction => { + const statisticsTransactionResponse = await transaction.run( + ` + CALL apoc.meta.stats() YIELD labels, relTypesCount + RETURN labels, relTypesCount + `, + ) + log(statisticsTransactionResponse) + return statisticsTransactionResponse.records.map(record => { + return { + ...record.get('labels'), + ...record.get('relTypesCount'), + } + }) }) + const [statistics] = await statisticsReadTxResultPromise Object.keys(mapping).forEach(key => { const stat = statistics[mapping[key]] - response[key] = stat ? stat.toNumber() : 0 + counts[key] = stat ? stat.toNumber() : 0 }) - - /* - * Note: invites count is calculated this way because invitation codes are not in use yet - */ - response.countInvites = response.countEmails - response.countUsers - return response + counts.countInvites = counts.countEmails - counts.countUsers + return counts } finally { session.close() } From d38131e24a2bae3c87cb12967aa4d342c46fe1ba Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 19:27:37 +0100 Subject: [PATCH 24/32] Refactor to use transaction functions/logging --- backend/src/schema/resolvers/users.js | 68 ++++++++++++---------- backend/src/schema/resolvers/users.spec.js | 4 ++ 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/backend/src/schema/resolvers/users.js b/backend/src/schema/resolvers/users.js index d8d5fbb73..d7f438dd4 100644 --- a/backend/src/schema/resolvers/users.js +++ b/backend/src/schema/resolvers/users.js @@ -3,6 +3,7 @@ import fileUpload from './fileUpload' import { getNeode } from '../../bootstrap/neo4j' import { UserInputError, ForbiddenError } from 'apollo-server' import Resolver from './helpers/Resolver' +import log from './helpers/databaseLogger' const neode = getNeode() @@ -122,50 +123,53 @@ export default { DeleteUser: async (object, params, context, resolveInfo) => { const { resource } = params const session = context.driver.session() - - let user try { if (resource && resource.length) { - await Promise.all( - resource.map(async node => { - await session.run( + await session.writeTransaction(transaction => { + resource.map(node => { + return transaction.run( ` - MATCH (resource:${node})<-[:WROTE]-(author:User {id: $userId}) - OPTIONAL MATCH (resource)<-[:COMMENTS]-(comment:Comment) - SET resource.deleted = true - SET resource.content = 'UNAVAILABLE' - SET resource.contentExcerpt = 'UNAVAILABLE' - SET comment.deleted = true - RETURN author`, + MATCH (resource:${node})<-[:WROTE]-(author:User {id: $userId}) + OPTIONAL MATCH (resource)<-[:COMMENTS]-(comment:Comment) + SET resource.deleted = true + SET resource.content = 'UNAVAILABLE' + SET resource.contentExcerpt = 'UNAVAILABLE' + SET comment.deleted = true + RETURN author + `, { userId: context.user.id, }, ) - }), - ) + }) + }) } - // we cannot set slug to 'UNAVAILABE' because of unique constraints - const transactionResult = await session.run( - ` - MATCH (user:User {id: $userId}) - SET user.deleted = true - SET user.name = 'UNAVAILABLE' - SET user.about = 'UNAVAILABLE' - WITH user - OPTIONAL MATCH (user)<-[:BELONGS_TO]-(email:EmailAddress) - DETACH DELETE email - WITH user - OPTIONAL MATCH (user)<-[:OWNED_BY]-(socialMedia:SocialMedia) - DETACH DELETE socialMedia - RETURN user`, - { userId: context.user.id }, - ) - user = transactionResult.records.map(r => r.get('user').properties)[0] + const deleteUserTxResultPromise = session.writeTransaction(async transaction => { + const deleteUserTransactionResponse = await transaction.run( + ` + MATCH (user:User {id: $userId}) + SET user.deleted = true + SET user.name = 'UNAVAILABLE' + SET user.about = 'UNAVAILABLE' + WITH user + OPTIONAL MATCH (user)<-[:BELONGS_TO]-(email:EmailAddress) + DETACH DELETE email + WITH user + OPTIONAL MATCH (user)<-[:OWNED_BY]-(socialMedia:SocialMedia) + DETACH DELETE socialMedia + RETURN user + `, + { userId: context.user.id }, + ) + log(deleteUserTransactionResponse) + return deleteUserTransactionResponse.records.map(record => record.get('user').properties) + }) + const [user] = await deleteUserTxResultPromise + return user } finally { session.close() } - return user }, }, User: { diff --git a/backend/src/schema/resolvers/users.spec.js b/backend/src/schema/resolvers/users.spec.js index 26e977a31..ab310c47a 100644 --- a/backend/src/schema/resolvers/users.spec.js +++ b/backend/src/schema/resolvers/users.spec.js @@ -372,6 +372,7 @@ describe('DeleteUser', () => { ], }, }, + errors: undefined, } await expect(mutate({ mutation: deleteUserMutation, variables })).resolves.toMatchObject( expectedResponse, @@ -418,6 +419,7 @@ describe('DeleteUser', () => { ], }, }, + errors: undefined, } await expect( mutate({ mutation: deleteUserMutation, variables }), @@ -465,6 +467,7 @@ describe('DeleteUser', () => { ], }, }, + errors: undefined, } await expect( mutate({ mutation: deleteUserMutation, variables }), @@ -511,6 +514,7 @@ describe('DeleteUser', () => { ], }, }, + errors: undefined, } await expect( mutate({ mutation: deleteUserMutation, variables }), From 53791c83e89901b5dc7af2bd49830da98b505948 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 19:31:50 +0100 Subject: [PATCH 25/32] Favor transaction functions in login mutation --- .../src/schema/resolvers/user_management.js | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/backend/src/schema/resolvers/user_management.js b/backend/src/schema/resolvers/user_management.js index d5c6cd5ad..4d035d9fa 100644 --- a/backend/src/schema/resolvers/user_management.js +++ b/backend/src/schema/resolvers/user_management.js @@ -3,6 +3,7 @@ import bcrypt from 'bcryptjs' import { AuthenticationError } from 'apollo-server' import { getNeode } from '../../bootstrap/neo4j' import normalizeEmail from './helpers/normalizeEmail' +import log from './helpers/databaseLogger' const neode = getNeode() @@ -25,17 +26,18 @@ export default { email = normalizeEmail(email) const session = driver.session() try { - const result = await session.run( - ` - MATCH (user:User {deleted: false})-[:PRIMARY_EMAIL]->(e:EmailAddress {email: $userEmail}) - RETURN user {.id, .slug, .name, .avatar, .encryptedPassword, .role, .disabled, email:e.email} as user LIMIT 1 - `, - { userEmail: email }, - ) - const [currentUser] = await result.records.map(record => { - return record.get('user') + const loginReadTxResultPromise = session.readTransaction(async transaction => { + const loginTransactionResponse = await transaction.run( + ` + MATCH (user:User {deleted: false})-[:PRIMARY_EMAIL]->(e:EmailAddress {email: $userEmail}) + RETURN user {.id, .slug, .name, .avatar, .encryptedPassword, .role, .disabled, email:e.email} as user LIMIT 1 + `, + { userEmail: email }, + ) + log(loginTransactionResponse) + return loginTransactionResponse.records.map(record => record.get('user')) }) - + const [currentUser] = await loginReadTxResultPromise if ( currentUser && (await bcrypt.compareSync(password, currentUser.encryptedPassword)) && From 6903a6cc7174458983ded56c96901547f30f8044 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 19:38:39 +0100 Subject: [PATCH 26/32] Favor transaction functions even for seeds --- backend/src/seed/factories/index.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/backend/src/seed/factories/index.js b/backend/src/seed/factories/index.js index 10db5cc03..8b80a4b4f 100644 --- a/backend/src/seed/factories/index.js +++ b/backend/src/seed/factories/index.js @@ -29,10 +29,16 @@ const factories = { export const cleanDatabase = async (options = {}) => { const { driver = getDriver() } = options - const cypher = 'MATCH (n) DETACH DELETE n' const session = driver.session() try { - return await session.run(cypher) + await session.writeTransaction(transaction => { + return transaction.run( + ` + MATCH (everything) + DETACH DELETE everything + `, + ) + }) } finally { session.close() } From 8a93e402b9c810858b0e3e36cd50bcad0127b160 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 12 Dec 2019 14:24:43 +0100 Subject: [PATCH 27/32] Remove neode update from production code - Favor transaction functions we have more control over --- backend/src/schema/resolvers/users.js | 34 +++++++++++++++++++-------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/backend/src/schema/resolvers/users.js b/backend/src/schema/resolvers/users.js index d7f438dd4..be9a69e80 100644 --- a/backend/src/schema/resolvers/users.js +++ b/backend/src/schema/resolvers/users.js @@ -101,23 +101,37 @@ export default { const blockedUser = await neode.find('User', args.id) return blockedUser.toJson() }, - UpdateUser: async (object, args, context, resolveInfo) => { - const { termsAndConditionsAgreedVersion } = args + UpdateUser: async (_parent, params, context, _resolveInfo) => { + const { termsAndConditionsAgreedVersion } = params if (termsAndConditionsAgreedVersion) { const regEx = new RegExp(/^[0-9]+\.[0-9]+\.[0-9]+$/g) if (!regEx.test(termsAndConditionsAgreedVersion)) { throw new ForbiddenError('Invalid version format!') } - args.termsAndConditionsAgreedAt = new Date().toISOString() + params.termsAndConditionsAgreedAt = new Date().toISOString() } - args = await fileUpload(args, { file: 'avatarUpload', url: 'avatar' }) + params = await fileUpload(params, { file: 'avatarUpload', url: 'avatar' }) + const session = context.driver.session() + + const writeTxResultPromise = session.writeTransaction(async transaction => { + const updateUserTransactionResponse = await transaction.run( + ` + MATCH (user:User {id: $params.id}) + SET user += $params + SET user.updatedAt = toString(datetime()) + RETURN user + `, + { params }, + ) + return updateUserTransactionResponse.records.map(record => record.get('user').properties) + }) try { - const user = await neode.find('User', args.id) - if (!user) return null - await user.update({ ...args, updatedAt: new Date().toISOString() }) - return user.toJson() - } catch (e) { - throw new UserInputError(e.message) + const [user] = await writeTxResultPromise + return user + } catch (error) { + throw new UserInputError(error.message) + } finally { + session.close() } }, DeleteUser: async (object, params, context, resolveInfo) => { From 3e15ecdfa2c1c4d05fd7ad7d933a2968b6f2abe2 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 12 Dec 2019 14:25:28 +0100 Subject: [PATCH 28/32] Refactor tests, extract validation to middleware --- backend/src/middleware/userMiddleware.spec.js | 0 .../validation/validationMiddleware.js | 9 +++- .../validation/validationMiddleware.spec.js | 48 ++++++++++++++++++ backend/src/schema/resolvers/users.spec.js | 50 ++++++++----------- 4 files changed, 78 insertions(+), 29 deletions(-) create mode 100644 backend/src/middleware/userMiddleware.spec.js diff --git a/backend/src/middleware/userMiddleware.spec.js b/backend/src/middleware/userMiddleware.spec.js new file mode 100644 index 000000000..e69de29bb diff --git a/backend/src/middleware/validation/validationMiddleware.js b/backend/src/middleware/validation/validationMiddleware.js index 606e14c23..9e6adc5a0 100644 --- a/backend/src/middleware/validation/validationMiddleware.js +++ b/backend/src/middleware/validation/validationMiddleware.js @@ -4,7 +4,7 @@ const COMMENT_MIN_LENGTH = 1 const NO_POST_ERR_MESSAGE = 'Comment cannot be created without a post!' const NO_CATEGORIES_ERR_MESSAGE = 'You cannot save a post without at least one category or more than three' - +const USERNAME_MIN_LENGTH = 3 const validateCreateComment = async (resolve, root, args, context, info) => { const content = args.content.replace(/<(?:.|\n)*?>/gm, '').trim() const { postId } = args @@ -127,12 +127,19 @@ export const validateNotifyUsers = async (label, reason) => { } } +const validateUpdateUser = async (resolve, root, params, context, info) => { + if (!params.name || params.name.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 { Mutation: { CreateComment: validateCreateComment, UpdateComment: validateUpdateComment, CreatePost: validatePost, UpdatePost: validateUpdatePost, + UpdateUser: validateUpdateUser, fileReport: validateReport, review: validateReview, }, diff --git a/backend/src/middleware/validation/validationMiddleware.spec.js b/backend/src/middleware/validation/validationMiddleware.spec.js index c3d0512ad..8aabc6b54 100644 --- a/backend/src/middleware/validation/validationMiddleware.spec.js +++ b/backend/src/middleware/validation/validationMiddleware.spec.js @@ -71,6 +71,14 @@ const reviewMutation = gql` } } ` + +const updateUserMutation = gql` + mutation($id: ID!, $name: String) { + UpdateUser(id: $id, name: $name) { + name + } + } +` beforeAll(() => { const { server } = createServer({ context: () => { @@ -397,4 +405,44 @@ describe('validateReview', () => { }) }) }) + + describe('validateUpdateUser', () => { + let userParams, variables, updatingUser + + beforeEach(async () => { + userParams = { + id: 'updating-user', + name: 'John Doe', + } + + variables = { + id: 'updating-user', + name: 'John Doughnut', + } + updatingUser = await factory.create('User', userParams) + authenticatedUser = await updatingUser.toJson() + }) + + it('with `null` as name', async () => { + variables = { + ...variables, + name: null, + } + await expect(mutate({ mutation: updateUserMutation, variables })).resolves.toMatchObject({ + data: { UpdateUser: null }, + errors: [{ message: 'Username must be at least 3 character long!' }], + }) + }) + + it('with name too short', async () => { + variables = { + ...variables, + name: ' ', + } + await expect(mutate({ mutation: updateUserMutation, variables })).resolves.toMatchObject({ + data: { UpdateUser: null }, + errors: [{ message: 'Username must be at least 3 character long!' }], + }) + }) + }) }) diff --git a/backend/src/schema/resolvers/users.spec.js b/backend/src/schema/resolvers/users.spec.js index ab310c47a..5d1ebd8e2 100644 --- a/backend/src/schema/resolvers/users.spec.js +++ b/backend/src/schema/resolvers/users.spec.js @@ -68,6 +68,7 @@ describe('User', () => { it('is permitted', async () => { await expect(query({ query: userQuery, variables })).resolves.toMatchObject({ data: { User: [{ name: 'Johnny' }] }, + errors: undefined, }) }) @@ -90,8 +91,7 @@ describe('User', () => { }) describe('UpdateUser', () => { - let userParams - let variables + let userParams, variables beforeEach(async () => { userParams = { @@ -111,16 +111,23 @@ describe('UpdateUser', () => { }) const updateUserMutation = gql` - mutation($id: ID!, $name: String, $termsAndConditionsAgreedVersion: String) { + mutation( + $id: ID! + $name: String + $termsAndConditionsAgreedVersion: String + $locationName: String + ) { UpdateUser( id: $id name: $name termsAndConditionsAgreedVersion: $termsAndConditionsAgreedVersion + locationName: $locationName ) { id name termsAndConditionsAgreedVersion termsAndConditionsAgreedAt + locationName } } ` @@ -152,7 +159,7 @@ describe('UpdateUser', () => { authenticatedUser = await user.toJson() }) - it('name within specifications', async () => { + it('updates the name', async () => { const expected = { data: { UpdateUser: { @@ -160,36 +167,13 @@ describe('UpdateUser', () => { name: 'John Doughnut', }, }, + errors: undefined, } await expect(mutate({ mutation: updateUserMutation, variables })).resolves.toMatchObject( expected, ) }) - it('with `null` as name', async () => { - const variables = { - id: 'u47', - name: null, - } - const { errors } = await mutate({ mutation: updateUserMutation, variables }) - expect(errors[0]).toHaveProperty( - 'message', - 'child "name" fails because ["name" contains an invalid value, "name" must be a string]', - ) - }) - - it('with too short name', async () => { - const variables = { - id: 'u47', - name: ' ', - } - const { errors } = await mutate({ mutation: updateUserMutation, variables }) - expect(errors[0]).toHaveProperty( - 'message', - 'child "name" fails because ["name" length must be at least 3 characters long]', - ) - }) - describe('given a new agreed version of terms and conditions', () => { beforeEach(async () => { variables = { ...variables, termsAndConditionsAgreedVersion: '0.0.2' } @@ -202,6 +186,7 @@ describe('UpdateUser', () => { termsAndConditionsAgreedAt: expect.any(String), }), }, + errors: undefined, } await expect(mutate({ mutation: updateUserMutation, variables })).resolves.toMatchObject( @@ -222,6 +207,7 @@ describe('UpdateUser', () => { termsAndConditionsAgreedAt: null, }), }, + errors: undefined, } await expect(mutate({ mutation: updateUserMutation, variables })).resolves.toMatchObject( @@ -238,6 +224,14 @@ describe('UpdateUser', () => { const { errors } = await mutate({ mutation: updateUserMutation, variables }) expect(errors[0]).toHaveProperty('message', 'Invalid version format!') }) + + it('supports updating location', async () => { + variables = { ...variables, locationName: 'Hamburg, New Jersey, United States of America' } + await expect(mutate({ mutation: updateUserMutation, variables })).resolves.toMatchObject({ + data: { UpdateUser: { locationName: 'Hamburg, New Jersey, United States of America' } }, + errors: undefined, + }) + }) }) }) From d375ebe7d90e3251b17f59ffba8fb1470923ebe8 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 12 Dec 2019 18:14:47 +0100 Subject: [PATCH 29/32] 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 @@