Merge pull request #1572 from Human-Connection/set_created_at_in_cypher

Fix bug where Post.createdAt is sometimes null

backend/src/middleware/dateTimeMiddleware.js

@@ -1,18 +0,0 @@
-const setCreatedAt = (resolve, root, args, context, info) => {
-  args.createdAt = new Date().toISOString()
-  return resolve(root, args, context, info)
-}
-const setUpdatedAt = (resolve, root, args, context, info) => {
-  args.updatedAt = new Date().toISOString()
-  return resolve(root, args, context, info)
-}
-
-export default {
-  Mutation: {
-    CreatePost: setCreatedAt,
-    CreateComment: setCreatedAt,
-    UpdateUser: setUpdatedAt,
-    UpdatePost: setUpdatedAt,
-    UpdateComment: setUpdatedAt,
-  },
-}

backend/src/middleware/index.js

@@ -5,7 +5,6 @@ import activityPub from './activityPubMiddleware'
 import softDelete from './softDelete/softDeleteMiddleware'
 import sluggify from './sluggifyMiddleware'
 import excerpt from './excerptMiddleware'
-import dateTime from './dateTimeMiddleware'
 import xss from './xssMiddleware'
 import permissions from './permissionsMiddleware'
 import user from './userMiddleware'
@@ -22,7 +21,6 @@ export default schema => {
     permissions,
     sentry,
     activityPub,
-    dateTime,
     validation,
     sluggify,
     excerpt,
@@ -40,7 +38,6 @@ export default schema => {
     'sentry',
     'permissions',
     // 'activityPub', disabled temporarily
-    'dateTime',
     'validation',
     'sluggify',
     'excerpt',

backend/src/middleware/notifications/mentions/extractMentionedUsers.js

@@ -1,6 +1,6 @@
 import cheerio from 'cheerio'
 
-export default function(content) {
+export default content => {
   if (!content) return []
   const $ = cheerio.load(content)
   const userIds = $('a.mention[data-mention-id]')

backend/src/middleware/notifications/notificationsMiddleware.js

@@ -16,7 +16,6 @@ const notifyUsers = async (label, id, idsOfUsers, reason, context) => {
   }
 
   const session = context.driver.session()
-  const createdAt = new Date().toISOString()
   let cypher
   switch (reason) {
     case 'mentioned_in_post': {
@@ -27,7 +26,11 @@ const notifyUsers = async (label, id, idsOfUsers, reason, context) => {
         AND NOT (user)<-[:BLOCKED]-(author)
         MERGE (post)-[notification:NOTIFIED {reason: $reason}]->(user)
         SET notification.read = FALSE
-        SET notification.createdAt = $createdAt
+        SET (
+        CASE
+        WHEN notification.createdAt IS NULL
+        THEN notification END ).createdAt = toString(datetime())
+        SET notification.updatedAt = toString(datetime())
       `
       break
     }
@@ -40,7 +43,11 @@ const notifyUsers = async (label, id, idsOfUsers, reason, context) => {
         AND NOT (user)<-[:BLOCKED]-(postAuthor)
         MERGE (comment)-[notification:NOTIFIED {reason: $reason}]->(user)
         SET notification.read = FALSE
-        SET notification.createdAt = $createdAt
+        SET (
+        CASE
+        WHEN notification.createdAt IS NULL
+        THEN notification END ).createdAt = toString(datetime())
+        SET notification.updatedAt = toString(datetime())
       `
       break
     }
@@ -53,17 +60,19 @@ const notifyUsers = async (label, id, idsOfUsers, reason, context) => {
         AND NOT (author)<-[:BLOCKED]-(user)
         MERGE (comment)-[notification:NOTIFIED {reason: $reason}]->(user)
         SET notification.read = FALSE
-        SET notification.createdAt = $createdAt
+        SET (
+        CASE
+        WHEN notification.createdAt IS NULL
+        THEN notification END ).createdAt = toString(datetime())
+        SET notification.updatedAt = toString(datetime())
       `
       break
     }
   }
   await session.run(cypher, {
-    label,
     id,
     idsOfUsers,
     reason,
-    createdAt,
   })
   session.close()
 }
@@ -82,6 +91,7 @@ const handleContentDataOfPost = async (resolve, root, args, context, resolveInfo
 
 const handleContentDataOfComment = async (resolve, root, args, context, resolveInfo) => {
   const idsOfUsers = extractMentionedUsers(args.content)
+
   const comment = await resolve(root, args, context, resolveInfo)
 
   if (comment) {

backend/src/middleware/notifications/notificationsMiddleware.spec.js

@@ -77,7 +77,7 @@ afterEach(async () => {
 describe('notifications', () => {
   const notificationQuery = gql`
     query($read: Boolean) {
-      notifications(read: $read, orderBy: createdAt_desc) {
+      notifications(read: $read, orderBy: updatedAt_desc) {
         read
         reason
         createdAt
@@ -391,7 +391,7 @@ describe('notifications', () => {
                 expect(Date.parse(createdAtBefore)).toEqual(expect.any(Number))
                 expect(createdAtAfter).toBeTruthy()
                 expect(Date.parse(createdAtAfter)).toEqual(expect.any(Number))
-                expect(createdAtBefore).not.toEqual(createdAtAfter)
+                expect(createdAtBefore).toEqual(createdAtAfter)
               })
             })
           })

backend/src/middleware/permissionsMiddleware.js

@@ -78,7 +78,7 @@ const invitationLimitReached = rule({
 
 const isAuthor = rule({
   cache: 'no_cache',
-})(async (parent, args, { user, driver }) => {
+})(async (_parent, args, { user, driver }) => {
   if (!user) return false
   const session = driver.session()
   const { id: resourceId } = args

backend/src/schema/resolvers/comments.js

@@ -1,4 +1,4 @@
-import { neo4jgraphql } from 'neo4j-graphql-js'
+import uuid from 'uuid/v4'
 import Resolver from './helpers/Resolver'
 
 export default {
@@ -10,44 +10,44 @@ export default {
       // because we use relationships for this. So, we are deleting it from params
       // before comment creation.
       delete params.postId
+      params.id = params.id || uuid()
+
       const session = context.driver.session()
-      const commentWithoutRelationships = await neo4jgraphql(
+      const createCommentCypher = `
-        object,
+        MATCH (post:Post {id: $postId})
-        params,
+        MATCH (author:User {id: $userId})
-        context,
+        WITH post, author
-        resolveInfo,
+        CREATE (comment:Comment {params})
-        false,
+        SET comment.createdAt = toString(datetime())
-      )
+        SET comment.updatedAt = toString(datetime())
-
-      const transactionRes = await session.run(
-        `
-        MATCH (post:Post {id: $postId}), (comment:Comment {id: $commentId}), (author:User {id: $userId})
         MERGE (post)<-[:COMMENTS]-(comment)<-[:WROTE]-(author)
-        RETURN comment, author`,
+        RETURN comment
-        {
+      `
-          userId: context.user.id,
+      const transactionRes = await session.run(createCommentCypher, {
-          postId,
+        userId: context.user.id,
-          commentId: commentWithoutRelationships.id,
+        postId,
-        },
+        params,
-      )
-
-      const [commentWithAuthor] = transactionRes.records.map(record => {
-        return {
-          comment: record.get('comment'),
-          author: record.get('author'),
-        }
       })
-
-      const { comment, author } = commentWithAuthor
-
-      const commentReturnedWithAuthor = {
-        ...comment.properties,
-        author: author.properties,
-      }
       session.close()
-      return commentReturnedWithAuthor
+
+      const [comment] = transactionRes.records.map(record => record.get('comment').properties)
+
+      return comment
     },
-    DeleteComment: async (object, args, context, resolveInfo) => {
+    UpdateComment: async (_parent, params, context, _resolveInfo) => {
+      const session = context.driver.session()
+      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 })
+      session.close()
+      const [comment] = transactionRes.records.map(record => record.get('comment').properties)
+      return comment
+    },
+    DeleteComment: async (_parent, args, context, _resolveInfo) => {
       const session = context.driver.session()
       const transactionRes = await session.run(
         `

backend/src/schema/resolvers/comments.spec.js

@@ -8,10 +8,7 @@ const driver = getDriver()
 const neode = getNeode()
 const factory = Factory()
 
-let variables
+let variables, mutate, authenticatedUser, commentAuthor, newlyCreatedComment
-let mutate
-let authenticatedUser
-let commentAuthor
 
 beforeAll(() => {
   const { server } = createServer({
@@ -57,7 +54,7 @@ const setupPostAndComment = async () => {
     content: 'Post to be commented',
     categoryIds: ['cat9'],
   })
-  await factory.create('Comment', {
+  newlyCreatedComment = await factory.create('Comment', {
     id: 'c456',
     postId: 'p1',
     author: commentAuthor,
@@ -160,6 +157,8 @@ describe('UpdateComment', () => {
       UpdateComment(content: $content, id: $id) {
         id
         content
+        createdAt
+        updatedAt
       }
     }
   `
@@ -200,6 +199,33 @@ describe('UpdateComment', () => {
         )
       })
 
+      it('updates a comment, but maintains non-updated attributes', async () => {
+        const expected = {
+          data: {
+            UpdateComment: {
+              id: 'c456',
+              content: 'The comment is updated',
+              createdAt: expect.any(String),
+            },
+          },
+        }
+        await expect(mutate({ mutation: updateCommentMutation, variables })).resolves.toMatchObject(
+          expected,
+        )
+      })
+
+      it('updates the updatedAt attribute', async () => {
+        newlyCreatedComment = await newlyCreatedComment.toJson()
+        const {
+          data: { UpdateComment },
+        } = await mutate({ mutation: updateCommentMutation, variables })
+        expect(newlyCreatedComment.updatedAt).toBeTruthy()
+        expect(Date.parse(newlyCreatedComment.updatedAt)).toEqual(expect.any(Number))
+        expect(UpdateComment.updatedAt).toBeTruthy()
+        expect(Date.parse(UpdateComment.updatedAt)).toEqual(expect.any(Number))
+        expect(newlyCreatedComment.updatedAt).not.toEqual(UpdateComment.updatedAt)
+      })
+
       describe('if `content` empty', () => {
         beforeEach(() => {
           variables = { ...variables, content: '  <p> </p>' }

backend/src/schema/resolvers/notifications.js

@@ -15,7 +15,7 @@ const transformReturnType = record => {
 
 export default {
   Query: {
-    notifications: async (parent, args, context, resolveInfo) => {
+    notifications: async (_parent, args, context, _resolveInfo) => {
       const { user: currentUser } = context
       const session = context.driver.session()
       let notifications
@@ -32,11 +32,11 @@ export default {
           whereClause = ''
       }
       switch (args.orderBy) {
-        case 'createdAt_asc':
+        case 'updatedAt_asc':
-          orderByClause = 'ORDER BY notification.createdAt ASC'
+          orderByClause = 'ORDER BY notification.updatedAt ASC'
           break
-        case 'createdAt_desc':
+        case 'updatedAt_desc':
-          orderByClause = 'ORDER BY notification.createdAt DESC'
+          orderByClause = 'ORDER BY notification.updatedAt DESC'
           break
         default:
           orderByClause = ''

backend/src/schema/resolvers/notifications.spec.js

@@ -145,47 +145,46 @@ describe('given some notifications', () => {
 
       describe('no filters', () => {
         it('returns all notifications of current user', async () => {
-          const expected = {
+          const expected = [
-            data: {
+            {
-              notifications: [
+              from: {
-                {
+                __typename: 'Comment',
-                  from: {
+                content: 'You have seen this comment mentioning already',
-                    __typename: 'Comment',
+              },
-                    content: 'You have seen this comment mentioning already',
+              read: true,
-                  },
+              createdAt: '2019-08-30T15:33:48.651Z',
-                  read: true,
-                  createdAt: '2019-08-30T15:33:48.651Z',
-                },
-                {
-                  from: {
-                    __typename: 'Post',
-                    content: 'Already seen post mention',
-                  },
-                  read: true,
-                  createdAt: '2019-08-30T17:33:48.651Z',
-                },
-                {
-                  from: {
-                    __typename: 'Comment',
-                    content: 'You have been mentioned in a comment',
-                  },
-                  read: false,
-                  createdAt: '2019-08-30T19:33:48.651Z',
-                },
-                {
-                  from: {
-                    __typename: 'Post',
-                    content: 'You have been mentioned in a post',
-                  },
-                  read: false,
-                  createdAt: '2019-08-31T17:33:48.651Z',
-                },
-              ],
             },
-          }
+            {
-          await expect(query({ query: notificationQuery, variables })).resolves.toMatchObject(
+              from: {
-            expected,
+                __typename: 'Post',
-          )
+                content: 'Already seen post mention',
+              },
+              read: true,
+              createdAt: '2019-08-30T17:33:48.651Z',
+            },
+            {
+              from: {
+                __typename: 'Comment',
+                content: 'You have been mentioned in a comment',
+              },
+              read: false,
+              createdAt: '2019-08-30T19:33:48.651Z',
+            },
+            {
+              from: {
+                __typename: 'Post',
+                content: 'You have been mentioned in a post',
+              },
+              read: false,
+              createdAt: '2019-08-31T17:33:48.651Z',
+            },
+          ]
+
+          await expect(query({ query: notificationQuery, variables })).resolves.toMatchObject({
+            data: {
+              notifications: expect.arrayContaining(expected),
+            },
+          })
         })
       })
 

backend/src/schema/resolvers/posts.js

@@ -74,7 +74,7 @@ export default {
     },
   },
   Mutation: {
-    CreatePost: async (object, params, context, resolveInfo) => {
+    CreatePost: async (_parent, params, context, _resolveInfo) => {
       const { categoryIds } = params
       delete params.categoryIds
       params = await fileUpload(params, { file: 'imageUpload', url: 'image' })
@@ -82,6 +82,8 @@ export default {
       let post
 
       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)
@@ -96,9 +98,7 @@ export default {
       const session = context.driver.session()
       try {
         const transactionRes = await session.run(createPostCypher, createPostVariables)
-        const posts = transactionRes.records.map(record => {
+        const posts = transactionRes.records.map(record => record.get('post').properties)
-          return record.get('post').properties
-        })
         post = posts[0]
       } catch (e) {
         if (e.code === 'Neo.ClientError.Schema.ConstraintValidationFailed')
@@ -110,14 +110,15 @@ export default {
 
       return post
     },
-    UpdatePost: async (object, params, context, resolveInfo) => {
+    UpdatePost: async (_parent, params, context, _resolveInfo) => {
       const { categoryIds } = params
       delete params.categoryIds
       params = await fileUpload(params, { file: 'imageUpload', url: 'image' })
       const session = context.driver.session()
 
       let updatePostCypher = `MATCH (post:Post {id: $params.id})
-      SET post = $params
+      SET post += $params
+      SET post.updatedAt = toString(datetime())
       `
 
       if (categoryIds && categoryIds.length) {
@@ -129,10 +130,11 @@ export default {
 
         await session.run(cypherDeletePreviousRelations, { params })
 
-        updatePostCypher += `WITH post
+        updatePostCypher += `
-        UNWIND $categoryIds AS categoryId
+          WITH post
-        MATCH (category:Category {id: categoryId})
+          UNWIND $categoryIds AS categoryId
-        MERGE (post)-[:CATEGORIZED]->(category)
+          MATCH (category:Category {id: categoryId})
+          MERGE (post)-[:CATEGORIZED]->(category)
         `
       }
 
@@ -141,12 +143,12 @@ export default {
 
       const transactionRes = await session.run(updatePostCypher, updatePostVariables)
       const [post] = transactionRes.records.map(record => {
-        return record.get('post')
+        return record.get('post').properties
       })
 
       session.close()
 
-      return post.properties
+      return post
     },
 
     DeletePost: async (object, args, context, resolveInfo) => {

backend/src/schema/resolvers/posts.spec.js

@@ -361,7 +361,7 @@ describe('CreatePost', () => {
 })
 
 describe('UpdatePost', () => {
-  let author
+  let author, newlyCreatedPost
   const updatePostMutation = gql`
     mutation($id: ID!, $title: String!, $content: String!, $categoryIds: [ID]) {
       UpdatePost(id: $id, title: $title, content: $content, categoryIds: $categoryIds) {
@@ -370,12 +370,14 @@ describe('UpdatePost', () => {
         categories {
           id
         }
+        createdAt
+        updatedAt
       }
     }
   `
   beforeEach(async () => {
     author = await factory.create('User', { slug: 'the-author' })
-    await factory.create('Post', {
+    newlyCreatedPost = await factory.create('Post', {
       author,
       id: 'p9876',
       title: 'Old title',
@@ -421,6 +423,29 @@ describe('UpdatePost', () => {
       )
     })
 
+    it('updates a post, but maintains non-updated attributes', async () => {
+      const expected = {
+        data: {
+          UpdatePost: { id: 'p9876', content: 'New content', createdAt: expect.any(String) },
+        },
+      }
+      await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject(
+        expected,
+      )
+    })
+
+    it('updates the updatedAt attribute', async () => {
+      newlyCreatedPost = await newlyCreatedPost.toJson()
+      const {
+        data: { UpdatePost },
+      } = await mutate({ mutation: updatePostMutation, variables })
+      expect(newlyCreatedPost.updatedAt).toBeTruthy()
+      expect(Date.parse(newlyCreatedPost.updatedAt)).toEqual(expect.any(Number))
+      expect(UpdatePost.updatedAt).toBeTruthy()
+      expect(Date.parse(UpdatePost.updatedAt)).toEqual(expect.any(Number))
+      expect(newlyCreatedPost.updatedAt).not.toEqual(UpdatePost.updatedAt)
+    })
+
     describe('no new category ids provided for update', () => {
       it('resolves and keeps current categories', async () => {
         const expected = {

backend/src/schema/resolvers/users.js

@@ -100,7 +100,7 @@ export default {
       try {
         const user = await instance.find('User', args.id)
         if (!user) return null
-        await user.update(args)
+        await user.update({ ...args, updatedAt: new Date().toISOString() })
         return user.toJson()
       } catch (e) {
         throw new UserInputError(e.message)

backend/src/schema/types/type/NOTIFIED.gql

@@ -2,6 +2,7 @@ type NOTIFIED {
   from: NotificationSource
   to: User
   createdAt: String
+  updatedAt: String
   read: Boolean
   reason: NotificationReason
 }
@@ -11,6 +12,8 @@ union NotificationSource = Post | Comment
 enum NotificationOrdering {
   createdAt_asc
   createdAt_desc
+  updatedAt_asc
+  updatedAt_desc
 }
 
 enum NotificationReason {

webapp/graphql/User.js

@@ -97,7 +97,7 @@ export const notificationQuery = i18n => {
     ${postFragment(lang)}
 
     query {
-      notifications(read: false, orderBy: createdAt_desc) {
+      notifications(read: false, orderBy: updatedAt_desc) {
         read
         reason
         createdAt