From 7d82b27aaa53a422954a9e18bd7e203147fbf59a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Tue, 12 Mar 2019 12:59:45 +0100 Subject: [PATCH] Hide disabled comments and posts by default I had to trick neo4j-graphql-js to return disabled and deleted fields by default on any neo4j node. If disabled is not queried, then we don't have it in our middleware and we're not able to filter it. --- src/middleware/idMiddleware.js | 28 -------- src/middleware/includedFieldsMiddleware.js | 29 ++++++++ src/middleware/index.js | 4 +- src/middleware/softDeleteMiddleware.js | 45 +++++++++---- src/middleware/softDeleteMiddleware.spec.js | 73 +++++++++++++++++++-- src/schema.graphql | 4 +- src/seed/seed-db.js | 9 ++- 7 files changed, 136 insertions(+), 56 deletions(-) delete mode 100644 src/middleware/idMiddleware.js create mode 100644 src/middleware/includedFieldsMiddleware.js diff --git a/src/middleware/idMiddleware.js b/src/middleware/idMiddleware.js deleted file mode 100644 index 2f1854fa6..000000000 --- a/src/middleware/idMiddleware.js +++ /dev/null @@ -1,28 +0,0 @@ -import cloneDeep from 'lodash/cloneDeep' - -const includeId = async (resolve, root, args, context, resolveInfo) => { - // Keeping the graphql resolveInfo untouched ensures that we don't add the - // following attributes to the result set returned to the graphQL client. - // We only want to pass these attributes to our resolver for internal - // purposes e.g. authorization. - const copy = cloneDeep(resolveInfo) - - copy.fieldNodes[0].selectionSet.selections.unshift({ - kind: 'Field', - name: { kind: 'Name', value: 'id' } - }) - return resolve(root, args, context, copy) -} - -export default { - Query: { - User: (resolve, root, args, context, info) => { - return includeId(resolve, root, args, context, info) - } - }, - Mutation: { - CreatePost: (resolve, root, args, context, info) => { - return includeId(resolve, root, args, context, info) - } - } -} diff --git a/src/middleware/includedFieldsMiddleware.js b/src/middleware/includedFieldsMiddleware.js new file mode 100644 index 000000000..6d8ccf8f1 --- /dev/null +++ b/src/middleware/includedFieldsMiddleware.js @@ -0,0 +1,29 @@ +import cloneDeep from 'lodash/cloneDeep' + +const _includeFieldsRecursively = (selectionSet, includedFields) => { + if (!selectionSet) return + includedFields.forEach((includedField) => { + selectionSet.selections.unshift({ + kind: 'Field', + name: { kind: 'Name', value: includedField } + }) + }) + selectionSet.selections.forEach((selection) => { + _includeFieldsRecursively(selection.selectionSet, includedFields) + }) +} + +const includeFieldsRecursively = (includedFields) => { + return (resolve, root, args, context, resolveInfo) => { + const copy = cloneDeep(resolveInfo) + copy.fieldNodes.forEach((fieldNode) => { + _includeFieldsRecursively(fieldNode.selectionSet, includedFields) + }) + return resolve(root, args, context, copy) + } +} + +export default { + Query: includeFieldsRecursively(['id', 'disabled', 'deleted']), + Mutation: includeFieldsRecursively(['id', 'disabled', 'deleted']) +} diff --git a/src/middleware/index.js b/src/middleware/index.js index 5cc20f969..6f95c7451 100644 --- a/src/middleware/index.js +++ b/src/middleware/index.js @@ -7,7 +7,7 @@ import dateTimeMiddleware from './dateTimeMiddleware' import xssMiddleware from './xssMiddleware' import permissionsMiddleware from './permissionsMiddleware' import userMiddleware from './userMiddleware' -import idMiddleware from './idMiddleware' +import includedFieldsMiddleware from './includedFieldsMiddleware' export default schema => { let middleware = [ @@ -19,7 +19,7 @@ export default schema => { fixImageUrlsMiddleware, softDeleteMiddleware, userMiddleware, - idMiddleware + includedFieldsMiddleware ] // add permisions middleware at the first position (unless we're seeding) diff --git a/src/middleware/softDeleteMiddleware.js b/src/middleware/softDeleteMiddleware.js index 0c12e7a72..042219916 100644 --- a/src/middleware/softDeleteMiddleware.js +++ b/src/middleware/softDeleteMiddleware.js @@ -1,26 +1,43 @@ -const setDefaults = (args) => { +const isModerator = ({ user }) => { + return user && (user.role === 'moderator' || user.role === 'admin') +} + +const setDefaultFilters = (resolve, root, args, context, info) => { if (typeof args.deleted !== 'boolean') { args.deleted = false } - if (typeof args.disabled !== 'boolean') { + + if (!isModerator(context)) { args.disabled = false } - return args + return resolve(root, args, context, info) +} + +const hideDisabledComments = async (resolve, root, args, context, info) => { + const { comments } = root + if (!Array.isArray(comments)) return resolve(root, args, context, info) + if (!isModerator(context)) { + root.comments = comments.filter((comment) => { + return !comment.disabled + }) + } + return resolve(root, args, context, info) } export default { Query: { - Post: (resolve, root, args, context, info) => { - return resolve(root, setDefaults(args), context, info) - }, - Comment: async (resolve, root, args, context, info) => { - return resolve(root, setDefaults(args), context, info) - }, - User: async (resolve, root, args, context, info) => { - return resolve(root, setDefaults(args), context, info) - } + Post: setDefaultFilters, + Comment: setDefaultFilters, + User: setDefaultFilters }, Mutation: async (resolve, root, args, context, info) => { - return resolve(root, setDefaults(args), context, info) - } + args.disabled = false + // TODO: remove as soon as our factories don't need this anymore + if (typeof args.deleted !== 'boolean') { + args.deleted = false + } + return resolve(root, args, context, info) + }, + Post: hideDisabledComments, + User: hideDisabledComments } diff --git a/src/middleware/softDeleteMiddleware.spec.js b/src/middleware/softDeleteMiddleware.spec.js index ed132104e..f478feb7c 100644 --- a/src/middleware/softDeleteMiddleware.spec.js +++ b/src/middleware/softDeleteMiddleware.spec.js @@ -9,14 +9,14 @@ let action beforeEach(async () => { await Promise.all([ - factory.create('User', { role: 'user', email: 'user@example.org', password: '1234' }), + factory.create('User', { id: 'u1', role: 'user', email: 'user@example.org', password: '1234' }), factory.create('User', { id: 'm1', role: 'moderator', email: 'moderator@example.org', password: '1234' }) ]) await factory.authenticateAs({ email: 'user@example.org', password: '1234' }) await Promise.all([ - factory.create('Post', { title: 'Deleted post', deleted: true }), - factory.create('Post', { id: 'p2', title: 'Disabled post', deleted: false }), - factory.create('Post', { title: 'Publicly visible post', deleted: false }) + await factory.create('Post', { id: 'p1', title: 'Deleted post', deleted: true }), + await factory.create('Post', { id: 'p2', title: 'Disabled post', deleted: false }), + await factory.create('Post', { id: 'p3', title: 'Publicly visible post', deleted: false }) ]) const moderatorFactory = Factory() await moderatorFactory.authenticateAs({ email: 'moderator@example.org', password: '1234' }) @@ -62,9 +62,68 @@ describe('softDeleteMiddleware', () => { client = new GraphQLClient(host, { headers }) }) - it('hides deleted or disabled posts', async () => { - const expected = { Post: [{ title: 'Publicly visible post' }] } - await expect(action()).resolves.toEqual(expected) + it('shows disabled but hides deleted posts', async () => { + const expected = [ + { title: 'Disabled post' }, + { title: 'Publicly visible post' } + ] + const { Post } = await action() + await expect(Post).toEqual(expect.arrayContaining(expected)) + }) + }) + + describe('.comments', () => { + beforeEach(async () => { + query = '{ Post(id: "p3") { title comments { content } } }' + + const asModerator = Factory() + await asModerator.authenticateAs({ email: 'moderator@example.org', password: '1234' }) + await Promise.all([ + asModerator.create('Comment', { id: 'c1', content: 'Disabled comment' }), + asModerator.create('Comment', { id: 'c2', content: 'Enabled comment on public post' }) + ]) + await Promise.all([ + asModerator.relate('Comment', 'Author', { from: 'u1', to: 'c1' }), + asModerator.relate('Comment', 'Post', { from: 'c1', to: 'p3' }), + asModerator.relate('Comment', 'Author', { from: 'u1', to: 'c2' }), + asModerator.relate('Comment', 'Post', { from: 'c2', to: 'p3' }) + ]) + await asModerator.mutate('mutation { disable( id: "c1") }') + }) + + describe('as user', () => { + beforeEach(async () => { + const headers = await login({ email: 'user@example.org', password: '1234' }) + client = new GraphQLClient(host, { headers }) + }) + + it('hides disabled comments', async () => { + const expected = { Post: [ { + title: 'Publicly visible post', + comments: [ + { content: 'Enabled comment on public post' } + ] + } + ] } + await expect(action()).resolves.toEqual(expected) + }) + }) + + describe('as moderator', () => { + beforeEach(async () => { + const headers = await login({ email: 'moderator@example.org', password: '1234' }) + client = new GraphQLClient(host, { headers }) + }) + + it('shows disabled comments', async () => { + const expected = [ + { content: 'Enabled comment on public post' }, + { content: 'Disabled comment' } + ] + const { Post: [{ comments }] } = await action() + + await expect(comments).toEqual(expect.arrayContaining(expected)) + }) }) }) diff --git a/src/schema.graphql b/src/schema.graphql index 152301715..801eb4502 100644 --- a/src/schema.graphql +++ b/src/schema.graphql @@ -148,7 +148,7 @@ type User { ) comments: [Comment]! @relation(name: "WROTE", direction: "OUT") - commentsCount: Int! @cypher(statement: "MATCH (this)-[:WROTE]->(r:Comment) WHERE NOT r.deleted = true RETURN COUNT(r)") + commentsCount: Int! @cypher(statement: "MATCH (this)-[:WROTE]->(r:Comment) WHERE NOT r.deleted = true AND NOT r.disabled = true RETURN COUNT(r)") shouted: [Post]! @relation(name: "SHOUTED", direction: "OUT") shoutedCount: Int! @cypher(statement: "MATCH (this)-[:SHOUTED]->(r:Post) WHERE NOT r.deleted = true AND NOT r.disabled = true RETURN COUNT(DISTINCT r)") @@ -189,7 +189,7 @@ type Post { categories: [Category]! @relation(name: "CATEGORIZED", direction: "OUT") comments: [Comment]! @relation(name: "COMMENTS", direction: "IN") - commentsCount: Int! @cypher(statement: "MATCH (this)<-[:COMMENTS]-(r:Comment) RETURN COUNT(r)") + commentsCount: Int! @cypher(statement: "MATCH (this)<-[:COMMENTS]-(r:Comment) WHERE NOT r.deleted = true AND NOT r.disabled = true RETURN COUNT(r)") shoutedBy: [User]! @relation(name: "SHOUTED", direction: "IN") shoutedCount: Int! @cypher(statement: "MATCH (this)<-[:SHOUTED]-(r:User) WHERE NOT r.deleted = true AND NOT r.disabled = true RETURN COUNT(DISTINCT r)") diff --git a/src/seed/seed-db.js b/src/seed/seed-db.js index e8c757db8..d526518b9 100644 --- a/src/seed/seed-db.js +++ b/src/seed/seed-db.js @@ -107,9 +107,6 @@ import Factory from './factories' asTick.create('Post', { id: 'p15' }) ]) - const disableMutation = 'mutation { disable( id: "p11") }' - await asModerator.mutate(disableMutation) - await Promise.all([ f.relate('Post', 'Categories', { from: 'p0', to: 'cat16' }), f.relate('Post', 'Categories', { from: 'p1', to: 'cat1' }), @@ -214,6 +211,12 @@ import Factory from './factories' f.relate('Comment', 'Post', { from: 'c7', to: 'p2' }) ]) + const disableMutation = 'mutation($id: ID!) { disable(id: $id) }' + await Promise.all([ + asModerator.mutate(disableMutation, { id: 'p11' }), + asModerator.mutate(disableMutation, { id: 'c5' }) + ]) + await Promise.all([ asTick.create('Report', { description: 'I don\'t like this comment', id: 'c1' }), asTrick.create('Report', { description: 'I don\'t like this post', id: 'p1' }),