From 5117a97b1c63d1ea856f6d704677a270f1d567c7 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 5 Dec 2019 13:17:26 +0100 Subject: [PATCH 01/65] 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/65] 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/65] 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/65] 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/65] 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/65] 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/65] 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/65] 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/65] 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/65] 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/65] 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 08dbfb31c9ea77894e6b7a77313a2016e69f3cdc Mon Sep 17 00:00:00 2001 From: lokalise-bot Date: Tue, 10 Dec 2019 21:55:15 +0100 Subject: [PATCH 12/65] Lokalise: update of webapp/locales/es.json --- webapp/locales/es.json | 66 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/webapp/locales/es.json b/webapp/locales/es.json index 7223024bf..ffbe48353 100644 --- a/webapp/locales/es.json +++ b/webapp/locales/es.json @@ -321,7 +321,63 @@ "disabledBy": "desactivado por", "reasonCategory": "Categoría", "reasonDescription": "Descripción", - "createdAt": "Fecha" + "status": "Estado actual", + "content": "Contenido", + "author": "Autor", + "decision": "Decisión", + "enabled": "Habilitado", + "disabled": "Deshabilitado", + "decided": "Decidido", + "noDecision": "¡No hay decisión!", + "decideButton": "Confirmar", + "DecisionSuccess": "Decidido con éxito!", + "enabledBy": "Habilitado por", + "previousDecision": "Decisión previa:", + "enabledAt": "Habilitado el", + "disabledAt": "Deshabilitado el", + "numberOfUsers": "{count} usuarios", + "filterLabel": { + "all": "Todos", + "unreviewed": "Sin revisar", + "reviewed": "Revisado", + "closed": "Cerrado" + }, + "reportedOn": "Fecha", + "moreDetails": "Ver Detalles", + "decideModal": { + "submit": "Confirmar decisión", + "cancel": "Cancelar", + "User": { + "disable": { + "title": "Finalmente Deshabilitar Usuario", + "message": "¿Realmente quiere que el usuario \"{nombre}<\/b>\" permanezca desactivado<\/b>?" + }, + "enable": { + "title": "Finalmente Habilitar Usuario", + "message": "¿Realmente quiere que el usuario \"{nombre}<\/b>\" permanezca habilitado<\/b>?" + } + }, + "Post": { + "disable": { + "title": "Finalmente Desactivar Contribución", + "message": "¿Realmente quiere que la entrada \"{nombre}<\/b>\" permanezca desactivada<\/b>?" + }, + "enable": { + "title": "Finalmente Habilitar Contribución", + "message": "¿Realmente quiere que la contribución \"{nombre}<\/b>\" permanezca activada<\/b>?" + } + }, + "Comment": { + "disable": { + "title": "Desactivar finalmente Comentario", + "message": "¿Realmente quiere que el comentario \"{nombre}<\/b>\" permanezca desactivado<\/b>?" + }, + "enable": { + "title": "Finalmente Habilitar Comentario", + "message": "¿Realmente quiere que el comentario \"{nombre}<\/b>\" permanezca habilitado<\/b>?" + } + } + } } }, "disable": { @@ -501,7 +557,9 @@ "invalid-invitation-token": "Parece que el código de invitación ya ha sido canjeado. Cada código sólo se puede utilizar una vez." }, "submit": "Crear una cuenta", - "success": "Se ha enviado un correo electrónico con un enlace de confirmación para el registro a {email}<\/b>." + "success": "Se ha enviado un correo electrónico con un enlace de confirmación para el registro a {email}<\/b>.", + "no-commercial": "No tengo intensiones comerciales y no represento una empresa u organización comercial.", + "no-political": "No estoy en la red en nombre de un partido o una organización política." } }, "create-user-account": { @@ -742,6 +800,10 @@ "addition": { "title": "Además, regularmente celebramos eventos donde también puede dar impresiones y hacer preguntas. Puede encontrar un resumen actualizado aquí:", "description": " https:\/\/human-connection.org\/events\/ <\/a>" + }, + "no-commercial-use": { + "title": "Sin uso comercial", + "description": "El uso de la red Human Connection no está permitido para fines comerciales. Esto incluye, pero no se limita a, publicitar productos con intención comercial, publicar enlaces de afiliados, solicitar donaciones directamente o brindar apoyo financiero para fines que no se reconocen como caritativos para fines fiscales." } }, "donations": { From f672abfaa9b0b655c47acc749cc84cfd3e69180c Mon Sep 17 00:00:00 2001 From: lokalise-bot Date: Tue, 10 Dec 2019 21:55:16 +0100 Subject: [PATCH 13/65] Lokalise: update of webapp/locales/ru.json --- webapp/locales/ru.json | 66 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/webapp/locales/ru.json b/webapp/locales/ru.json index b1296839c..dc8562899 100644 --- a/webapp/locales/ru.json +++ b/webapp/locales/ru.json @@ -321,7 +321,63 @@ "disabledBy": "отключены", "reasonCategory": "Категория", "reasonDescription": "Описание", - "createdAt": "Дата" + "status": "Текущее состояние", + "content": "Содержа́ние", + "author": "Автор", + "decision": "Решение", + "enabled": "Включен", + "disabled": "Отключен", + "decided": "Решил", + "noDecision": "Нет решения!", + "decideButton": "Подтвердить", + "DecisionSuccess": "Решил успешно!", + "enabledBy": "Включено с", + "previousDecision": "Предыдущее решение:", + "enabledAt": "Включено на", + "disabledAt": "Отключено на", + "numberOfUsers": "{count} пользователи", + "filterLabel": { + "all": "Все", + "unreviewed": "Нерассмотренный", + "reviewed": "Рассмотренный", + "closed": "Закрыто" + }, + "reportedOn": "Дата", + "moreDetails": "Посмотреть подробности", + "decideModal": { + "submit": "Подтвердить решение", + "cancel": "Отменить", + "User": { + "disable": { + "title": "Окончательно отключить пользователя", + "message": "Ты действительно хочешь, чтобы пользователь \"{name}<\/b>\" остановиться и отключен<\/b>?" + }, + "enable": { + "title": "Окончательно включить пост", + "message": "Ты уверены, что хочешь поделиться пользователем \"{name}<\/b>\"?" + } + }, + "Post": { + "disable": { + "title": "Окончательно отключить пост", + "message": "Ты действительно хочешь, чтобы пост \"{name}<\/b>\" остановиться и отключен<\/b>?" + }, + "enable": { + "title": "Окончательно включить пост", + "message": "Ты действительно хочешь, чтобы пост \"{name}<\/b>\" остановиться и включен<\/b>?" + } + }, + "Comment": { + "disable": { + "title": "Окончательно отключить комментарий", + "message": "Ты действительно хочешь, чтобы комментарий \"{name}<\/b>\" остановиться и отключен<\/b>?" + }, + "enable": { + "title": "Окончательно включить комментарий", + "message": "Ты действительно хочешь, чтобы комментарий \"{name}<\/b>\" остановиться и включен<\/b>?" + } + } + } } }, "disable": { @@ -501,7 +557,9 @@ "invalid-invitation-token": "Похоже, что приглашение уже использовалось. Ссылки на приглашения можно использовать только один раз." }, "submit": "Создать учетную запись", - "success": "Письмо со ссылкой для завершения регистрации было отправлено на {email} <\/b>" + "success": "Письмо со ссылкой для завершения регистрации было отправлено на {email} <\/b>", + "no-commercial": "У меня нет коммерческих намерений, и я не представляю коммерческое предприятие или организацию.", + "no-political": "Я не от имени какой-либо партии или политической организации в сети." } }, "create-user-account": { @@ -742,6 +800,10 @@ "addition": { "title": "Кроме того, мы регулярно проводим мероприятия, где ты также можешь\nподелиться своими впечатлениями и задать вопросы. Здесь ты можешь найти текущий обзор:", "description": " https:\/\/human-connection.org\/events\/ <\/a>" + }, + "no-commercial-use": { + "title": "Нет коммерческого использования", + "description": "Использование Human Connection сети не допускается в коммерческих целях. Это включает, но не ограничивается рекламой продуктов с коммерческими целями, размещением партнерских ссылок, прямым привлечением пожертвований или предоставлением финансовой поддержки для целей, которые не признаются благотворительными для целей налогообложения." } }, "donations": { From e24d8035b13040dc29f5f9cb033de8c1a401ac34 Mon Sep 17 00:00:00 2001 From: roschaefer Date: Tue, 10 Dec 2019 21:47:34 +0100 Subject: [PATCH 14/65] Fix this annoying bug with a tested helper I don't know where the bug originates. But it can only be that either `previousResult` or `fetchMore` result is sometimes undefined. This should make the function bullet-proof for these situations. --- webapp/components/utils/UpdateQuery.js | 17 ++++ webapp/components/utils/UpdateQuery.spec.js | 86 +++++++++++++++++++++ webapp/pages/index.vue | 23 +----- webapp/pages/profile/_id/_slug.vue | 23 +----- 4 files changed, 107 insertions(+), 42 deletions(-) create mode 100644 webapp/components/utils/UpdateQuery.js create mode 100644 webapp/components/utils/UpdateQuery.spec.js diff --git a/webapp/components/utils/UpdateQuery.js b/webapp/components/utils/UpdateQuery.js new file mode 100644 index 000000000..d601d0ac3 --- /dev/null +++ b/webapp/components/utils/UpdateQuery.js @@ -0,0 +1,17 @@ +import unionBy from 'lodash/unionBy' + +export default function UpdateQuery(component, { $state, pageKey }) { + if (!pageKey) throw new Error('No key given for the graphql query { data } object') + return (previousResult, { fetchMoreResult }) => { + const oldData = (previousResult && previousResult[pageKey]) || [] + const newData = (fetchMoreResult && fetchMoreResult[pageKey]) || [] + if (newData.length < component.pageSize) { + component.hasMore = false + $state.complete() + } + const result = {} + result[pageKey] = unionBy(oldData, newData, item => item.id) + $state.loaded() + return result + } +} diff --git a/webapp/components/utils/UpdateQuery.spec.js b/webapp/components/utils/UpdateQuery.spec.js new file mode 100644 index 000000000..295b256a4 --- /dev/null +++ b/webapp/components/utils/UpdateQuery.spec.js @@ -0,0 +1,86 @@ +import UpdateQuery from './UpdateQuery' + +let $state +let component +let pageKey +let updateQuery +let previousResult +let fetchMoreResult + +beforeEach(() => { + component = { + hasMore: true, + pageSize: 1, + } + + $state = { + complete: jest.fn(), + loaded: jest.fn(), + } + previousResult = { Post: [{ id: 1, foo: 'bar' }] } + fetchMoreResult = { Post: [{ id: 2, foo: 'baz' }] } + updateQuery = () => UpdateQuery(component, { $state, pageKey }) +}) + +describe('UpdateQuery', () => { + it('throws error because no key is given', () => { + expect(() => { + updateQuery()({ Post: [] }, { fetchMoreResult: { Post: [] } }) + }).toThrow(/No key given/) + }) + + describe('with a page key', () => { + beforeEach(() => (pageKey = 'Post')) + + describe('given two arrays of things', () => { + it('merges the arrays', () => { + expect(updateQuery()(previousResult, { fetchMoreResult })).toEqual({ + Post: [ + { id: 1, foo: 'bar' }, + { id: 2, foo: 'baz' }, + ], + }) + }) + + it('does not create duplicates', () => { + fetchMoreResult = { Post: [{ id: 1, foo: 'baz' }] } + expect(updateQuery()(previousResult, { fetchMoreResult })).toEqual({ + Post: [{ id: 1, foo: 'bar' }], + }) + }) + + it('does not call $state.complete()', () => { + expect(updateQuery()(previousResult, { fetchMoreResult })) + expect($state.complete).not.toHaveBeenCalled() + }) + + describe('in case of fewer records than pageSize', () => { + beforeEach(() => (component.pageSize = 10)) + it('calls $state.complete()', () => { + expect(updateQuery()(previousResult, { fetchMoreResult })) + expect($state.complete).toHaveBeenCalled() + }) + + it('changes component.hasMore to `false`', () => { + expect(component.hasMore).toBe(true) + expect(updateQuery()(previousResult, { fetchMoreResult })) + expect(component.hasMore).toBe(false) + }) + }) + }) + + describe('given one array is undefined', () => { + describe('does not crash', () => { + it('neither if the previous data was undefined', () => { + expect(updateQuery()(undefined, { fetchMoreResult })).toEqual({ + Post: [{ id: 2, foo: 'baz' }], + }) + }) + + it('not if the new data is undefined', () => { + expect(updateQuery()(previousResult, {})).toEqual({ Post: [{ id: 1, foo: 'bar' }] }) + }) + }) + }) + }) +}) diff --git a/webapp/pages/index.vue b/webapp/pages/index.vue index 2283a3453..c3f01d548 100644 --- a/webapp/pages/index.vue +++ b/webapp/pages/index.vue @@ -68,6 +68,7 @@ import MasonryGridItem from '~/components/MasonryGrid/MasonryGridItem.vue' import { mapGetters, mapMutations } from 'vuex' import { filterPosts } from '~/graphql/PostQuery.js' import PostMutations from '~/graphql/PostMutations' +import UpdateQuery from '~/components/utils/UpdateQuery' export default { components: { @@ -151,27 +152,7 @@ export default { first: this.pageSize, orderBy: ['pinned_asc', this.orderBy], }, - updateQuery: (previousResult, { fetchMoreResult }) => { - if (!fetchMoreResult || fetchMoreResult.Post.length < this.pageSize) { - this.hasMore = false - $state.complete() - } - - const { Post = [] } = previousResult - const result = { - ...previousResult, - Post: [ - ...Post.filter(prevPost => { - return ( - fetchMoreResult.Post.filter(newPost => newPost.id === prevPost.id).length === 0 - ) - }), - ...fetchMoreResult.Post, - ], - } - $state.loaded() - return result - }, + updateQuery: UpdateQuery(this, { $state, pageKey: 'Post' }), }) }, deletePost(deletedPost) { diff --git a/webapp/pages/profile/_id/_slug.vue b/webapp/pages/profile/_id/_slug.vue index 44ac7f47d..3f7ff068a 100644 --- a/webapp/pages/profile/_id/_slug.vue +++ b/webapp/pages/profile/_id/_slug.vue @@ -283,6 +283,7 @@ import { profilePagePosts } from '~/graphql/PostQuery' import UserQuery from '~/graphql/User' import { Block, Unblock } from '~/graphql/settings/BlockedUsers' import PostMutations from '~/graphql/PostMutations' +import UpdateQuery from '~/components/utils/UpdateQuery' const tabToFilterMapping = ({ tab, id }) => { return { @@ -385,27 +386,7 @@ export default { first: this.pageSize, orderBy: 'createdAt_desc', }, - updateQuery: (previousResult, { fetchMoreResult }) => { - if (!fetchMoreResult || fetchMoreResult.profilePagePosts.length < this.pageSize) { - this.hasMore = false - $state.complete() - } - const { profilePagePosts = [] } = previousResult - const result = { - ...previousResult, - profilePagePosts: [ - ...profilePagePosts.filter(prevPost => { - return ( - fetchMoreResult.profilePagePosts.filter(newPost => newPost.id === prevPost.id) - .length === 0 - ) - }), - ...fetchMoreResult.profilePagePosts, - ], - } - $state.loaded() - return result - }, + updateQuery: UpdateQuery(this, { $state, pageKey: 'profilePagePosts' }), }) }, resetPostList() { From 28352675ead1babf4ad187650b1ebb3a37d3fe3a Mon Sep 17 00:00:00 2001 From: roschaefer Date: Tue, 10 Dec 2019 23:57:05 +0100 Subject: [PATCH 15/65] Fix docker manifest on Travis CI So I had `DOCKER_CLI_EXPERIMENTAL=enabled` enabled by default and couldn't reproduce the error on my machine. This time I'm pretty sure it works as expected. --- scripts/docker_push.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/docker_push.sh b/scripts/docker_push.sh index 16a5e21ab..b342278b9 100755 --- a/scripts/docker_push.sh +++ b/scripts/docker_push.sh @@ -1,6 +1,5 @@ #!/usr/bin/env bash ROOT_DIR=$(dirname "$0")/.. -DOCKER_CLI_EXPERIMENTAL=enabled # BUILD_COMMIT=${TRAVIS_COMMIT:-$(git rev-parse HEAD)} IFS='.' read -r major minor patch < $ROOT_DIR/VERSION @@ -24,7 +23,7 @@ do for tag in "${tags[@]}" do TARGET="humanconnection/${app}:${tag}" - if docker manifest inspect $TARGET >/dev/null; then + if DOCKER_CLI_EXPERIMENTAL=enabled docker manifest inspect $TARGET >/dev/null; then echo "docker image ${TARGET} already present, skipping ..." else echo -e "docker tag $SOURCE $TARGET\ndocker push $TARGET" From 232787d41270ff45ccc3ba35854eafef01243d00 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 11 Dec 2019 09:14:16 +0000 Subject: [PATCH 16/65] build(deps): bump node from 13.1.0-alpine to 13.3.0-alpine in /webapp Bumps node from 13.1.0-alpine to 13.3.0-alpine. Signed-off-by: dependabot-preview[bot] --- webapp/Dockerfile | 2 +- webapp/Dockerfile.maintenance | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/Dockerfile b/webapp/Dockerfile index 37a31d6f4..91f19a486 100644 --- a/webapp/Dockerfile +++ b/webapp/Dockerfile @@ -1,4 +1,4 @@ -FROM node:13.1.0-alpine as base +FROM node:13.3.0-alpine as base LABEL Description="Web Frontend of the Social Network Human-Connection.org" Vendor="Human-Connection gGmbH" Version="0.0.1" Maintainer="Human-Connection gGmbH (developer@human-connection.org)" EXPOSE 3000 diff --git a/webapp/Dockerfile.maintenance b/webapp/Dockerfile.maintenance index 7195d0f1c..adcaca13a 100644 --- a/webapp/Dockerfile.maintenance +++ b/webapp/Dockerfile.maintenance @@ -1,4 +1,4 @@ -FROM node:13.1.0-alpine as build +FROM node:13.3.0-alpine as build LABEL Description="Maintenance page of the Social Network Human-Connection.org" Vendor="Human-Connection gGmbH" Version="0.0.1" Maintainer="Human-Connection gGmbH (developer@human-connection.org)" EXPOSE 3000 From 760fd0171573cf4865bce3b17b9ea456bad6a25c Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 11:06:44 +0100 Subject: [PATCH 17/65] 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 61862905beecb8a990a117998b9adaa1c809b99b Mon Sep 17 00:00:00 2001 From: Erhardt Stiefel <49943365+erhardtstiefel@users.noreply.github.com> Date: Wed, 11 Dec 2019 11:19:07 +0100 Subject: [PATCH 18/65] Update en.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a longer discussion I understood just to change the title of the function deleteUserAccount by renaming "name": "Delete data", to "name": "Delete user account", to solve 🐛 [Bug] Misleading description on "Delete your account" #2355 --- webapp/locales/en.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/locales/en.json b/webapp/locales/en.json index c10164a33..94183d7e7 100644 --- a/webapp/locales/en.json +++ b/webapp/locales/en.json @@ -271,10 +271,10 @@ "name": "Download Data" }, "deleteUserAccount": { - "name": "Delete data", + "name": "Delete user account", "contributionsCount": "Delete my {count} posts", "commentedCount": "Delete my {count} comments", - "accountDescription": "Be aware that your Post and Comments are important to our community. If you still choose to delete them, you have to mark them below.", + "accountDescription": "Be aware that your Posts and Comments are important to our community. If you still choose to delete them, you have to mark them below.", "accountWarning": "You CAN'T MANAGE and CAN'T RECOVER your Account, Posts, or Comments after deleting your account!", "success": "Account successfully deleted!", "pleaseConfirm": "Destructive action! Type {confirm} to confirm" From aed15f0be9ebc57a2812a2bc1da3c9cfb088a08a Mon Sep 17 00:00:00 2001 From: Erhardt Stiefel <49943365+erhardtstiefel@users.noreply.github.com> Date: Wed, 11 Dec 2019 12:28:41 +0100 Subject: [PATCH 19/65] Update de.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a longer discussion I understood just to change the title of the function deleteUserAccount by renaming "name": "Daten löschen", to "name": "Benutzerkonto löschen", to solve the bug --- webapp/locales/de.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/locales/de.json b/webapp/locales/de.json index 663106c6c..880d22c87 100644 --- a/webapp/locales/de.json +++ b/webapp/locales/de.json @@ -113,7 +113,7 @@ } }, "deleteUserAccount": { - "name": "Daten löschen", + "name": "Benutzerkonto löschen", "contributionsCount": "Meine {count} Beiträge löschen", "commentedCount": "Meine {count} Kommentare löschen", "accountDescription": "Sei dir bewusst, dass deine Beiträge und Kommentare für unsere Community wichtig sind. Wenn du sie trotzdem löschen möchtest, musst du sie unten markieren.", From ca9c58c06bf8392561fa73d00771e9683fea7823 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 12:50:46 +0100 Subject: [PATCH 20/65] 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 d6442b123e7733122b2b60e466566ca0cec0378c Mon Sep 17 00:00:00 2001 From: Erhardt Stiefel <49943365+erhardtstiefel@users.noreply.github.com> Date: Wed, 11 Dec 2019 12:56:38 +0100 Subject: [PATCH 21/65] Update es.json After a longer discussion I understood just to change the title of the function deleteUserAccount by renaming "name": "Borrar datos", to "name": "Eliminar cuenta de usuario", to solve the bug --- webapp/locales/es.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/locales/es.json b/webapp/locales/es.json index 7223024bf..50558c851 100644 --- a/webapp/locales/es.json +++ b/webapp/locales/es.json @@ -113,7 +113,7 @@ } }, "deleteUserAccount": { - "name": "Borrar datos", + "name": "Eliminar cuenta de usuario", "contributionsCount": "Eliminar mis {count} contribuciones", "commentedCount": "Eliminar mis {count} comentarios", "accountDescription": "Tenga en cuenta que su contribución y sus comentarios son importantes para nuestra comunidad. Si aún decide borrarlos, debe marcarlos a continuación.", @@ -749,4 +749,4 @@ "donate-now": "Donar ahora", "amount-of-total": "{amount} de {total} € recaudados" } -} \ No newline at end of file +} From 73a5b394d63a6c3a45792373cd0946edd9413a97 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 12:56:49 +0100 Subject: [PATCH 22/65] 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 eceae24683623803596be76451e8b68a9306fa2e Mon Sep 17 00:00:00 2001 From: Erhardt Stiefel <49943365+erhardtstiefel@users.noreply.github.com> Date: Wed, 11 Dec 2019 13:10:17 +0100 Subject: [PATCH 23/65] Update fr.json --- webapp/locales/fr.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/locales/fr.json b/webapp/locales/fr.json index c5967eb8b..6d31c2467 100644 --- a/webapp/locales/fr.json +++ b/webapp/locales/fr.json @@ -113,7 +113,7 @@ } }, "deleteUserAccount": { - "name": "Effacer les données", + "name": "Supprimer un compte utilisateur", "contributionsCount": "Supprimer mes {count} postes", "commentedCount": "Supprimer mes {count} commentaires", "accountDescription": "Sachez que vos postes et commentaires sont importants pour notre communauté. Si vous voulez quand même les supprimer, vous devez les marquer ci-dessous.", @@ -749,4 +749,4 @@ "donate-now": "Faites un don", "amount-of-total": "{amount} de {total} € collectés" } -} \ No newline at end of file +} From cc0a33ec7d6204e73f65804ff6c0af66b34d1f1e Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 13:26:18 +0100 Subject: [PATCH 24/65] 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 65a51ea471c68eb41705c3aa88358390dee32b3a Mon Sep 17 00:00:00 2001 From: Erhardt Stiefel <49943365+erhardtstiefel@users.noreply.github.com> Date: Wed, 11 Dec 2019 13:30:06 +0100 Subject: [PATCH 25/65] Update it.json These two lines added to give the user the chance to delete his posts and/or his comments when deleting his account, too "contributionsCount": "Cancellare i miei {count} messaggi", "commentedCount": "Cancella i miei {count} commenti", --- webapp/locales/it.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/webapp/locales/it.json b/webapp/locales/it.json index 7d7dd2507..a44cc8d42 100644 --- a/webapp/locales/it.json +++ b/webapp/locales/it.json @@ -114,6 +114,8 @@ }, "deleteUserAccount": { "name": "Cancellare l'account utente", + "contributionsCount": "Cancellare i miei {count} messaggi", + "commentedCount": "Cancella i miei {count} commenti", "accountDescription": "Essere consapevoli che i tuoi post e commenti sono importanti per la nostra comunità. Se cancelli il tuo account utente, tutto scomparirà per sempre - e sarebbe un vero peccato!", "accountWarning": "Attenzione!Tu Non puoi gestire e Non puoi recuperare il tuo account, i tuoi messaggi o commenti dopo aver cancellato il tuo account!", "success": "Account eliminato con successo!", From c871ec2632d713713ab24c768bf735601038c285 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Wed, 11 Dec 2019 16:29:20 +0100 Subject: [PATCH 26/65] 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 27/65] 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 28/65] 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 29/65] 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 30/65] 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 31/65] 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 32/65] 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 33/65] 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 34/65] 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 35/65] 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 36/65] 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 986c1e8d05360d5eb41eb487e0a5d12ca65741c8 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 12 Dec 2019 04:19:03 +0000 Subject: [PATCH 37/65] build(deps): bump neo4j-graphql-js from 2.10.0 to 2.10.1 in /backend Bumps [neo4j-graphql-js](https://github.com/neo4j-graphql/neo4j-graphql-js) from 2.10.0 to 2.10.1. - [Release notes](https://github.com/neo4j-graphql/neo4j-graphql-js/releases) - [Changelog](https://github.com/neo4j-graphql/neo4j-graphql-js/blob/master/CHANGELOG.md) - [Commits](https://github.com/neo4j-graphql/neo4j-graphql-js/commits) Signed-off-by: dependabot-preview[bot] --- backend/package.json | 2 +- backend/yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/package.json b/backend/package.json index b5fec7f1a..9465f9ebd 100644 --- a/backend/package.json +++ b/backend/package.json @@ -81,7 +81,7 @@ "minimatch": "^3.0.4", "mustache": "^3.1.0", "neo4j-driver": "~1.7.6", - "neo4j-graphql-js": "^2.10.0", + "neo4j-graphql-js": "^2.10.1", "neode": "^0.3.3", "node-fetch": "~2.6.0", "nodemailer": "^6.4.1", diff --git a/backend/yarn.lock b/backend/yarn.lock index dd070679a..ac97d40d3 100644 --- a/backend/yarn.lock +++ b/backend/yarn.lock @@ -6161,10 +6161,10 @@ neo4j-driver@^1.7.3, neo4j-driver@^1.7.5, neo4j-driver@~1.7.6: text-encoding-utf-8 "^1.0.2" uri-js "^4.2.2" -neo4j-graphql-js@^2.10.0: - version "2.10.0" - resolved "https://registry.yarnpkg.com/neo4j-graphql-js/-/neo4j-graphql-js-2.10.0.tgz#4298793756d839dedb98bc3e50a2bd40a311874d" - integrity sha512-jRdIyw+DHg9gfB6pWKb1ZHMR9rXIl7qf51efjUHIRHRbVR3RCcw1cKyONkq4LE8v2bHc7QDrKwJs+GQ1SRxDug== +neo4j-graphql-js@^2.10.1: + version "2.10.1" + resolved "https://registry.yarnpkg.com/neo4j-graphql-js/-/neo4j-graphql-js-2.10.1.tgz#e470d067db681bac8f4daa755f697000110aca4b" + integrity sha512-D6Gimu39lkg+3pXKWR3qEY6yMXOv/JOdKSizsYSAE73lj9CubJAYx4hdtmNXJ0Tyy+C9LxcPZwWZEzg0P9niEw== dependencies: "@babel/runtime" "^7.5.5" "@babel/runtime-corejs2" "^7.5.5" From d9ca8127500a3b64293bbf6f923c20d6645e8faf Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 12 Dec 2019 04:19:21 +0000 Subject: [PATCH 38/65] build(deps): bump @nuxtjs/apollo in /webapp Bumps [@nuxtjs/apollo](https://github.com/nuxt-community/apollo-module) from 4.0.0-rc18 to 4.0.0-rc19. - [Release notes](https://github.com/nuxt-community/apollo-module/releases) - [Commits](https://github.com/nuxt-community/apollo-module/compare/v4.0.0-rc.18...v4.0.0-rc.19) Signed-off-by: dependabot-preview[bot] --- webapp/package.json | 2 +- webapp/yarn.lock | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/webapp/package.json b/webapp/package.json index 48606ace9..017afb19c 100644 --- a/webapp/package.json +++ b/webapp/package.json @@ -58,7 +58,7 @@ }, "dependencies": { "@human-connection/styleguide": "0.5.22", - "@nuxtjs/apollo": "^4.0.0-rc18", + "@nuxtjs/apollo": "^4.0.0-rc19", "@nuxtjs/axios": "~5.8.0", "@nuxtjs/dotenv": "~1.4.1", "@nuxtjs/pwa": "^3.0.0-beta.19", diff --git a/webapp/yarn.lock b/webapp/yarn.lock index abce6b81f..6d39b3789 100644 --- a/webapp/yarn.lock +++ b/webapp/yarn.lock @@ -1574,14 +1574,14 @@ webpack-node-externals "^1.7.2" webpackbar "^4.0.0" -"@nuxtjs/apollo@^4.0.0-rc18": - version "4.0.0-rc18" - resolved "https://registry.yarnpkg.com/@nuxtjs/apollo/-/apollo-4.0.0-rc18.tgz#0069cae64f414ed879d20de00881986dca6bb26c" - integrity sha512-DTwRw9XLJKyphZiVwtKn4hE6Vfn6BlxEDWFBMTXpKE3XUKpg5+Qcgr8GstkiKtWbOuNQi660KdZReJ48R8bxgQ== +"@nuxtjs/apollo@^4.0.0-rc19": + version "4.0.0-rc19" + resolved "https://registry.yarnpkg.com/@nuxtjs/apollo/-/apollo-4.0.0-rc19.tgz#145b50c8e0185dac83c37f48ab685861f9005850" + integrity sha512-OCUxdhz09vTA7jq4KrhdYw23PRXS4yHWST99Ohc1oSUiZUyNrmQc+VUNAz9bhSVjfHABrP1NP2FzKnBE1iEZhA== dependencies: cross-fetch "^3.0.4" universal-cookie "^4.0.2" - vue-apollo "^3.0.1" + vue-apollo "^3.0.2" vue-cli-plugin-apollo "^0.21.3" webpack-node-externals "^1.7.2" @@ -16283,10 +16283,10 @@ vscode-uri@^1.0.6: resolved "https://registry.yarnpkg.com/vscode-uri/-/vscode-uri-1.0.8.tgz#9769aaececae4026fb6e22359cb38946580ded59" integrity sha512-obtSWTlbJ+a+TFRYGaUumtVwb+InIUVI0Lu0VBUAPmj2cU5JutEXg3xUE0c2J5Tcy7h2DEKVJBFi+Y9ZSFzzPQ== -vue-apollo@^3.0.1: - version "3.0.1" - resolved "https://registry.yarnpkg.com/vue-apollo/-/vue-apollo-3.0.1.tgz#b7c24b6d6032bf707be7872e6615d59aa6621241" - integrity sha512-NM+kWbPGV5bnRMK7BmMJMxoT1NqPjVAYf+MsjPDyfQNgyVEHSIObRVqLQDIs56PYQSC6YOGa0luo6Ykjj6rrPw== +vue-apollo@^3.0.2: + version "3.0.2" + resolved "https://registry.yarnpkg.com/vue-apollo/-/vue-apollo-3.0.2.tgz#b198ecfa3765850a0b9f2b84ffaa7fbd8ec15f52" + integrity sha512-lrKyTT1L5mjDEp7nyqnTRJwD/kTpLDBIqFfZ+TGQVivjlUz6o5VA0pLYGCx5cGa1gEF/ERWc0AEdNSdKgs7Ygg== dependencies: chalk "^2.4.2" serialize-javascript "^2.1.0" From 4dd0f3ae4dfde6af62049d1d958a46699b137423 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 12 Dec 2019 04:19:39 +0000 Subject: [PATCH 39/65] build(deps-dev): bump nodemon from 2.0.1 to 2.0.2 in /backend Bumps [nodemon](https://github.com/remy/nodemon) from 2.0.1 to 2.0.2. - [Release notes](https://github.com/remy/nodemon/releases) - [Commits](https://github.com/remy/nodemon/compare/v2.0.1...v2.0.2) Signed-off-by: dependabot-preview[bot] --- backend/package.json | 2 +- backend/yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/package.json b/backend/package.json index b5fec7f1a..c61d7a430 100644 --- a/backend/package.json +++ b/backend/package.json @@ -119,7 +119,7 @@ "eslint-plugin-promise": "~4.2.1", "eslint-plugin-standard": "~4.0.1", "jest": "~24.9.0", - "nodemon": "~2.0.1", + "nodemon": "~2.0.2", "prettier": "~1.19.1", "supertest": "~4.0.2" } diff --git a/backend/yarn.lock b/backend/yarn.lock index dd070679a..98602be73 100644 --- a/backend/yarn.lock +++ b/backend/yarn.lock @@ -6275,10 +6275,10 @@ nodemailer@^6.4.1: resolved "https://registry.yarnpkg.com/nodemailer/-/nodemailer-6.4.1.tgz#f70b40355b7b08f1f80344b353970a4f8f664370" integrity sha512-mSQAzMim8XIC1DemK9TifDTIgASfoJEllG5aC1mEtZeZ+FQyrSOdGBRth6JRA1ERzHQCET3QHVSd9Kc6mh356g== -nodemon@~2.0.1: - version "2.0.1" - resolved "https://registry.yarnpkg.com/nodemon/-/nodemon-2.0.1.tgz#cec436f8153ad5d3e6c27c304849a06cabea71cc" - integrity sha512-UC6FVhNLXjbbV4UzaXA3wUdbEkUZzLGgMGzmxvWAex5nzib/jhcSHVFlQODdbuUHq8SnnZ4/EABBAbC3RplvPg== +nodemon@~2.0.2: + version "2.0.2" + resolved "https://registry.yarnpkg.com/nodemon/-/nodemon-2.0.2.tgz#9c7efeaaf9b8259295a97e5d4585ba8f0cbe50b0" + integrity sha512-GWhYPMfde2+M0FsHnggIHXTqPDHXia32HRhh6H0d75Mt9FKUoCBvumNHr7LdrpPBTKxsWmIEOjoN+P4IU6Hcaw== dependencies: chokidar "^3.2.2" debug "^3.2.6" From 40bf38085d3abadea3e042eff093f9043d58f30c Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 12 Dec 2019 09:48:29 +0000 Subject: [PATCH 40/65] build(deps): bump nodemailer from 6.4.1 to 6.4.2 in /backend Bumps [nodemailer](https://github.com/nodemailer/nodemailer) from 6.4.1 to 6.4.2. - [Release notes](https://github.com/nodemailer/nodemailer/releases) - [Changelog](https://github.com/nodemailer/nodemailer/blob/master/CHANGELOG.md) - [Commits](https://github.com/nodemailer/nodemailer/compare/v6.4.1...v6.4.2) Signed-off-by: dependabot-preview[bot] --- backend/package.json | 2 +- backend/yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/package.json b/backend/package.json index 9465f9ebd..41df77759 100644 --- a/backend/package.json +++ b/backend/package.json @@ -84,7 +84,7 @@ "neo4j-graphql-js": "^2.10.1", "neode": "^0.3.3", "node-fetch": "~2.6.0", - "nodemailer": "^6.4.1", + "nodemailer": "^6.4.2", "nodemailer-html-to-text": "^3.1.0", "npm-run-all": "~4.1.5", "request": "~2.88.0", diff --git a/backend/yarn.lock b/backend/yarn.lock index ac97d40d3..fb8f07781 100644 --- a/backend/yarn.lock +++ b/backend/yarn.lock @@ -6270,10 +6270,10 @@ nodemailer-html-to-text@^3.1.0: dependencies: html-to-text "^5.1.1" -nodemailer@^6.4.1: - version "6.4.1" - resolved "https://registry.yarnpkg.com/nodemailer/-/nodemailer-6.4.1.tgz#f70b40355b7b08f1f80344b353970a4f8f664370" - integrity sha512-mSQAzMim8XIC1DemK9TifDTIgASfoJEllG5aC1mEtZeZ+FQyrSOdGBRth6JRA1ERzHQCET3QHVSd9Kc6mh356g== +nodemailer@^6.4.2: + version "6.4.2" + resolved "https://registry.yarnpkg.com/nodemailer/-/nodemailer-6.4.2.tgz#7147550e32cdc37453380ab78d2074533966090a" + integrity sha512-g0n4nH1ONGvqYo1v72uSWvF/MRNnnq1LzmSzXb/6EPF3LFb51akOhgG3K2+aETAsJx90/Q5eFNTntu4vBCwyQQ== nodemon@~2.0.1: version "2.0.1" From 8a93e402b9c810858b0e3e36cd50bcad0127b160 Mon Sep 17 00:00:00 2001 From: mattwr18 Date: Thu, 12 Dec 2019 14:24:43 +0100 Subject: [PATCH 41/65] 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 42/65] 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 43/65] 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 @@