Favor transaction functions

Created by: Tirokk

Authored by mattwr18 Dec 5, 2019 Merged Dec 13, 2019

---

Pullrequest

Issues

• fixes #2412 (closed)

Todo

Middleware

• permissionsMiddleware.js
• sluggifyMiddleware.js
• validationMiddleware.js

Resolvers

• locations.js
• comments.js
• createPasswordReset.js
• existingEmailAddress.js
• notifications.js
• passwordReset.js
• posts.js
• rewards.js
• shout.js
• statistics.js
• user_management.js
• users.js

Tests

• Add tests for middleware/nodes/locations.js
• passwordReset.spec.js

Factories

• index.js

Created by: Tirokk

Authored by roschaefer Dec 13, 2019

---

Outdated (history rewrite) - original diff

---

@@ -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

Very good!

Created by: Tirokk

Authored by roschaefer Dec 13, 2019

---

Outdated (history rewrite) - original diff

---

@@ -115,12 +116,32 @@ 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!')
+  }
+}
+
+const validateUpdateUser = async (resolve, root, params, context, info) => {
+  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)

I would say just remove the if check, no? Is there a reason for it?

Created by: Tirokk

Authored by roschaefer Dec 13, 2019

---

Outdated (history rewrite) - original diff

---

@@ -172,6 +175,7 @@ describe('UpdateComment', () => {
               createdAt: expect.any(String),
             },
           },
+          errors: undefined,

Good! We should do that more often.

Created by: Mogge

Review: Approved

Looks good to me! Feel free to add the few suggestions.