Add tests for unpinPost, refactor

- following @roschaefer's PR review suggestions - simplify UpdatePost by using pinPost/unpinPost - did not remove filtering because attempting to have two queries caused problems as well to do with duplicate records, etc... and it's working now
2025-12-13 07:46:06 +00:00 · 2019-10-17 21:51:41 +02:00 · 2019-10-17 21:51:41 +02:00 · 973912fb87
commit 973912fb87
parent 3e04b26068
6 changed files with 243 additions and 86 deletions
--- a/backend/src/middleware/permissionsMiddleware.js
+++ b/backend/src/middleware/permissionsMiddleware.js
@ -111,11 +111,6 @@ const noEmailFilter = rule({
  return !('email' in args)
 })

-const pinnedPost = rule({
-  cache: 'no_cache',
-})(async (_, args) => {
-  return 'pinned' in args
-})
 const publicRegistration = rule()(() => !!CONFIG.PUBLIC_REGISTRATION)

 // Permissions
@ -149,7 +144,7 @@ const permissions = shield(
      CreateInvitationCode: and(isAuthenticated, or(not(invitationLimitReached), isAdmin)),
      UpdateUser: onlyYourself,
      CreatePost: isAuthenticated,
-      UpdatePost: or(and(isAuthor, not(pinnedPost)), isAdmin),
+      UpdatePost: isAuthor,
      DeletePost: isAuthor,
      report: isAuthenticated,
      CreateSocialMedia: isAuthenticated,
@ -179,6 +174,8 @@ const permissions = shield(
      markAsRead: isAuthenticated,
      AddEmailAddress: isAuthenticated,
      VerifyEmailAddress: isAuthenticated,
+      pinPost: isAdmin,
+      unpinPost: isAdmin,
    },
    User: {
      email: or(isMyOwn, isAdmin),
--- a/backend/src/schema/resolvers/posts.js
+++ b/backend/src/schema/resolvers/posts.js
@ -33,12 +33,7 @@ const maintainPinnedPosts = params => {
  if (isEmpty(params.filter)) {
    params.filter = { OR: [pinnedPostFilter, {}] }
  } else {
-    const filteredPostsArray = []
-    Object.keys(params.filter).forEach(key => {
-      filteredPostsArray.push({ [key]: params.filter[key] })
-    })
-    filteredPostsArray.push(pinnedPostFilter)
-    params.filter = { OR: filteredPostsArray }
+    params.filter = { OR: [pinnedPostFilter, { ...params.filter }] }
  }
  return params
 }
@ -128,7 +123,7 @@ export default {
      return post
    },
    UpdatePost: async (_parent, params, context, _resolveInfo) => {
-      const { categoryIds, pinned, unpinned } = params
+      const { categoryIds } = params
      const { id: userId } = context.user
      delete params.pinned
      delete params.unpinned
@ -158,32 +153,6 @@ export default {
        `
      }

-      if (unpinned) {
-        const cypherRemovePinnedStatus = `
-          MATCH ()-[previousRelations:PINNED]->(post:Post {id: $params.id})
-          DELETE previousRelations
-          RETURN post
-      `
-
-        await session.run(cypherRemovePinnedStatus, { params })
-      }
-
-      if (pinned) {
-        const cypherDeletePreviousRelations = `
-          MATCH ()-[previousRelations:PINNED]->(post:Post)
-          DELETE previousRelations
-          RETURN post
-        `
-
-        await session.run(cypherDeletePreviousRelations)
-
-        updatePostCypher += `
-          MATCH (user:User {id: $userId}) WHERE user.role = 'admin'
-          MERGE (user)-[:PINNED {createdAt: toString(datetime())}]->(post)
-          WITH post
-          `
-      }
-
      updatePostCypher += `RETURN post`
      const updatePostVariables = { categoryIds, params, userId }

@ -257,6 +226,64 @@ export default {
      })
      return emoted
    },
+    pinPost: async (_parent, params, context, _resolveInfo) => {
+      let pinnedPost
+      const { driver, user } = context
+      const session = driver.session()
+      const { id: userId } = user
+      let writeTxResultPromise = session.writeTransaction(async transaction => {
+        const deletePreviousRelationsResponse = await transaction.run(
+          `
+          MATCH ()-[previousRelations:PINNED]->(post:Post)
+          DELETE previousRelations
+          RETURN post
+        `,
+          { params },
+        )
+        return deletePreviousRelationsResponse.records.map(record => record.get('post').properties)
+      })
+      await writeTxResultPromise
+
+      writeTxResultPromise = session.writeTransaction(async transaction => {
+        const pinPostTransactionResponse = await transaction.run(
+          `
+            MATCH (user:User {id: $userId}) WHERE user.role = 'admin'
+            MATCH (post:Post {id: $params.id})
+            MERGE (user)-[:PINNED {createdAt: toString(datetime())}]->(post)
+            RETURN post
+         `,
+          { userId, params },
+        )
+        return pinPostTransactionResponse.records.map(record => record.get('post').properties)
+      })
+      try {
+        ;[pinnedPost] = await writeTxResultPromise
+      } finally {
+        session.close()
+      }
+      return pinnedPost
+    },
+    unpinPost: async (_parent, params, context, _resolveInfo) => {
+      let unpinnedPost
+      const session = context.driver.session()
+      const writeTxResultPromise = session.writeTransaction(async transaction => {
+        const unpinPostTransactionResponse = await transaction.run(
+          `
+          MATCH ()-[previousRelations:PINNED]->(post:Post {id: $params.id})
+          DELETE previousRelations
+          RETURN post
+        `,
+          { params },
+        )
+        return unpinPostTransactionResponse.records.map(record => record.get('post').properties)
+      })
+      try {
+        ;[unpinnedPost] = await writeTxResultPromise
+      } finally {
+        session.close()
+      }
+      return unpinnedPost
+    },
  },
  Post: {
    ...Resolver('Post', {
--- a/backend/src/schema/resolvers/posts.spec.js
+++ b/backend/src/schema/resolvers/posts.spec.js
@ -31,11 +31,6 @@ const createPostMutation = gql`
      slug
      disabled
      deleted
-      pinnedBy {
-        id
-        name
-        role
-      }
      language
      author {
        name
@ -373,14 +368,8 @@ describe('CreatePost', () => {
 describe('UpdatePost', () => {
  let author, newlyCreatedPost
  const updatePostMutation = gql`
-    mutation($id: ID!, $title: String!, $content: String!, $categoryIds: [ID], $pinned: Boolean) {
-      UpdatePost(
-        id: $id
-        title: $title
-        content: $content
-        categoryIds: $categoryIds
-        pinned: $pinned
-      ) {
+    mutation($id: ID!, $title: String!, $content: String!, $categoryIds: [ID]) {
+      UpdatePost(id: $id, title: $title, content: $content, categoryIds: $categoryIds) {
        id
        title
        content
@ -388,11 +377,6 @@ describe('UpdatePost', () => {
          name
          slug
        }
-        pinnedBy {
-          id
-          name
-          role
-        }
        categories {
          id
        }
@ -579,15 +563,46 @@ describe('UpdatePost', () => {
    })
  })

-  describe('pinned posts', () => {
+  describe('pin posts', () => {
+    const pinPostMutation = gql`
+      mutation($id: ID!) {
+        pinPost(id: $id) {
+          id
+          title
+          content
+          author {
+            name
+            slug
+          }
+          pinnedBy {
+            id
+            name
+            role
+          }
+          createdAt
+          updatedAt
+        }
+      }
+    `
    beforeEach(async () => {
-      variables = { ...variables, pinned: true }
+      variables = { ...variables }
    })
+
+    describe('unauthenticated', () => {
+      it('throws authorization error', async () => {
+        authenticatedUser = null
+        await expect(mutate({ mutation: pinPostMutation, variables })).resolves.toMatchObject({
+          errors: [{ message: 'Not Authorised!' }],
+          data: { pinPost: null },
+        })
+      })
+    })
+
    describe('users cannot pin posts', () => {
      it('throws authorization error', async () => {
-        await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject({
+        await expect(mutate({ mutation: pinPostMutation, variables })).resolves.toMatchObject({
          errors: [{ message: 'Not Authorised!' }],
-          data: { UpdatePost: null },
+          data: { pinPost: null },
        })
      })
    })
@ -600,9 +615,9 @@ describe('UpdatePost', () => {
      })

      it('throws authorization error', async () => {
-        await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject({
+        await expect(mutate({ mutation: pinPostMutation, variables })).resolves.toMatchObject({
          errors: [{ message: 'Not Authorised!' }],
-          data: { UpdatePost: null },
+          data: { pinPost: null },
        })
      })
    })
@ -615,11 +630,6 @@ describe('UpdatePost', () => {
          name: 'Admin',
          updatedAt: new Date().toISOString(),
        })
-        variables = {
-          ...variables,
-          title: 'pinned post',
-          content: 'this is super important for the community',
-        }
        authenticatedUser = await admin.toJson()
      })

@ -635,9 +645,8 @@ describe('UpdatePost', () => {
          variables = { ...variables, id: 'created-and-pinned-by-same-admin' }
          const expected = {
            data: {
-              UpdatePost: {
-                title: 'pinned post',
-                content: 'this is super important for the community',
+              pinPost: {
+                id: 'created-and-pinned-by-same-admin',
                author: {
                  name: 'Admin',
                },
@ -651,7 +660,7 @@ describe('UpdatePost', () => {
            errors: undefined,
          }

-          await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject(
+          await expect(mutate({ mutation: pinPostMutation, variables })).resolves.toMatchObject(
            expected,
          )
        })
@ -676,9 +685,8 @@ describe('UpdatePost', () => {
          variables = { ...variables, id: 'created-by-one-admin-pinned-by-different-one' }
          const expected = {
            data: {
-              UpdatePost: {
-                title: 'pinned post',
-                content: 'this is super important for the community',
+              pinPost: {
+                id: 'created-by-one-admin-pinned-by-different-one',
                author: {
                  name: 'otherAdmin',
                },
@ -692,7 +700,7 @@ describe('UpdatePost', () => {
            errors: undefined,
          }

-          await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject(
+          await expect(mutate({ mutation: pinPostMutation, variables })).resolves.toMatchObject(
            expected,
          )
        })
@ -702,9 +710,8 @@ describe('UpdatePost', () => {
        it('responds with the updated Post', async () => {
          const expected = {
            data: {
-              UpdatePost: {
-                title: 'pinned post',
-                content: 'this is super important for the community',
+              pinPost: {
+                id: 'p9876',
                author: {
                  slug: 'the-author',
                },
@ -718,7 +725,7 @@ describe('UpdatePost', () => {
            errors: undefined,
          }

-          await expect(mutate({ mutation: updatePostMutation, variables })).resolves.toMatchObject(
+          await expect(mutate({ mutation: pinPostMutation, variables })).resolves.toMatchObject(
            expected,
          )
        })
@ -731,9 +738,9 @@ describe('UpdatePost', () => {
            id: 'only-pinned-post',
            author: admin,
          })
-          await mutate({ mutation: updatePostMutation, variables })
+          await mutate({ mutation: pinPostMutation, variables })
          variables = { ...variables, id: 'only-pinned-post' }
-          await mutate({ mutation: updatePostMutation, variables })
+          await mutate({ mutation: pinPostMutation, variables })
          pinnedPost = await neode.cypher(
            `MATCH ()-[relationship:PINNED]->(post:Post) RETURN post, relationship`,
          )
@ -818,6 +825,98 @@ describe('UpdatePost', () => {
      })
    })
  })
+
+  describe('unpin posts', () => {
+    const unpinPostMutation = gql`
+      mutation($id: ID!) {
+        unpinPost(id: $id) {
+          id
+          title
+          content
+          author {
+            name
+            slug
+          }
+          pinnedBy {
+            id
+            name
+            role
+          }
+          createdAt
+          updatedAt
+        }
+      }
+    `
+    beforeEach(async () => {
+      variables = { ...variables }
+    })
+
+    describe('unauthenticated', () => {
+      it('throws authorization error', async () => {
+        authenticatedUser = null
+        await expect(mutate({ mutation: unpinPostMutation, variables })).resolves.toMatchObject({
+          errors: [{ message: 'Not Authorised!' }],
+          data: { unpinPost: null },
+        })
+      })
+    })
+
+    describe('users cannot unpin posts', () => {
+      it('throws authorization error', async () => {
+        await expect(mutate({ mutation: unpinPostMutation, variables })).resolves.toMatchObject({
+          errors: [{ message: 'Not Authorised!' }],
+          data: { unpinPost: null },
+        })
+      })
+    })
+
+    describe('moderator cannot unpin posts', () => {
+      let moderator
+      beforeEach(async () => {
+        moderator = await user.update({ role: 'moderator', updatedAt: new Date().toISOString() })
+        authenticatedUser = await moderator.toJson()
+      })
+
+      it('throws authorization error', async () => {
+        await expect(mutate({ mutation: unpinPostMutation, variables })).resolves.toMatchObject({
+          errors: [{ message: 'Not Authorised!' }],
+          data: { unpinPost: null },
+        })
+      })
+    })
+
+    describe('admin can unpin posts', () => {
+      let admin, pinnedPost
+      beforeEach(async () => {
+        pinnedPost = await neode.create('Post', { id: 'post-to-be-unpinned' })
+        admin = await user.update({
+          role: 'admin',
+          name: 'Admin',
+          updatedAt: new Date().toISOString(),
+        })
+        authenticatedUser = await admin.toJson()
+        await admin.relateTo(pinnedPost, 'pinned', { createdAt: new Date().toISOString() })
+      })
+
+      it('responds with the unpinned Post', async () => {
+        authenticatedUser = await admin.toJson()
+        variables = { ...variables, id: 'post-to-be-unpinned' }
+        const expected = {
+          data: {
+            unpinPost: {
+              id: 'post-to-be-unpinned',
+              pinnedBy: null,
+            },
+          },
+          errors: undefined,
+        }
+
+        await expect(mutate({ mutation: unpinPostMutation, variables })).resolves.toMatchObject(
+          expected,
+        )
+      })
+    })
+  })
 })

 describe('DeletePost', () => {
--- a/backend/src/schema/types/type/Post.gql
+++ b/backend/src/schema/types/type/Post.gql
@ -84,12 +84,12 @@ type Mutation {
    visibility: Visibility
    language: String
    categoryIds: [ID]
-    pinned: Boolean
-    unpinned: Boolean
  ): Post
  DeletePost(id: ID!): Post
  AddPostEmotions(to: _PostInput!, data: _EMOTEDInput!): EMOTED
  RemovePostEmotions(to: _PostInput!, data: _EMOTEDInput!): EMOTED
+  pinPost(id: ID!): Post
+  unpinPost(id: ID!): Post
 }

 type Query {
--- a/webapp/graphql/PostMutations.js
+++ b/webapp/graphql/PostMutations.js
@ -95,5 +95,39 @@ export default () => {
        }
      }
    `,
+    pinPost: gql`
+      mutation($id: ID!) {
+        pinPost(id: $id) {
+          id
+          title
+          slug
+          content
+          contentExcerpt
+          language
+          pinnedBy {
+            id
+            name
+            role
+          }
+        }
+      }
+    `,
+    unpinPost: gql`
+      mutation($id: ID!) {
+        unpinPost(id: $id) {
+          id
+          title
+          slug
+          content
+          contentExcerpt
+          language
+          pinnedBy {
+            id
+            name
+            role
+          }
+        }
+      }
+    `,
  }
 }
--- a/webapp/pages/index.vue
+++ b/webapp/pages/index.vue
@ -177,7 +177,7 @@ export default {
    pinPost(post) {
      this.$apollo
        .mutate({
-          mutation: PostMutations().UpdatePost,
+          mutation: PostMutations().pinPost,
          variables: { id: post.id, title: post.title, content: post.content, pinned: true },
        })
        .then(() => {
@ -190,7 +190,7 @@ export default {
    unpinPost(post) {
      this.$apollo
        .mutate({
-          mutation: PostMutations().UpdatePost,
+          mutation: PostMutations().unpinPost,
          variables: { id: post.id, title: post.title, content: post.content, unpinned: true },
        })
        .then(() => {