From ba3cfa8e0405083834d07eca0878ca62f620eaa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolfgang=20Hu=C3=9F?= Date: Fri, 14 Feb 2020 16:52:51 +0100 Subject: [PATCH 1/9] Set sensible report properties to moderators only in the permissionsMiddleware --- backend/src/middleware/permissionsMiddleware.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/backend/src/middleware/permissionsMiddleware.js b/backend/src/middleware/permissionsMiddleware.js index 755ddabf8..107a45a35 100644 --- a/backend/src/middleware/permissionsMiddleware.js +++ b/backend/src/middleware/permissionsMiddleware.js @@ -152,6 +152,15 @@ export default shield( User: { email: or(isMyOwn, isAdmin), }, + Report: { + createdAt: or(isModerator), + updatedAt: or(isModerator), + rule: or(isModerator), + disable: or(isModerator), + closed: or(isModerator), + filed: or(isModerator), + reviewed: or(isModerator), + }, }, { debug, From f380915b2c679d42e5db136ea1d923cf00bbcf10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolfgang=20Hu=C3=9F?= Date: Mon, 17 Feb 2020 12:10:16 +0100 Subject: [PATCH 2/9] Refactor GQL and tests, first approach --- .../middleware/permissionsMiddleware.spec.js | 121 +++++++++++-- backend/src/schema/resolvers/reports.js | 28 +-- backend/src/schema/resolvers/reports.spec.js | 165 +++++++++++------- backend/src/schema/resolvers/users.js | 6 +- backend/src/schema/types/type/Report.gql | 7 +- backend/src/schema/types/type/User.gql | 11 +- 6 files changed, 242 insertions(+), 96 deletions(-) diff --git a/backend/src/middleware/permissionsMiddleware.spec.js b/backend/src/middleware/permissionsMiddleware.spec.js index 3c307348d..7e5245ef9 100644 --- a/backend/src/middleware/permissionsMiddleware.spec.js +++ b/backend/src/middleware/permissionsMiddleware.spec.js @@ -9,14 +9,6 @@ const driver = getDriver() let query, authenticatedUser, owner, anotherRegularUser, administrator, variables, moderator -const userQuery = gql` - query($name: String) { - User(name: $name) { - email - } - } -` - describe('authorization', () => { beforeAll(async () => { await cleanDatabase() @@ -30,7 +22,11 @@ describe('authorization', () => { query = createTestClient(server).query }) - describe('given two existing users', () => { + afterEach(async () => { + await cleanDatabase() + }) + + describe('given an owner, an other user, an admin, a moderator', () => { beforeEach(async () => { ;[owner, anotherRegularUser, administrator, moderator] = await Promise.all([ Factory.build( @@ -79,15 +75,20 @@ describe('authorization', () => { variables = {} }) - afterEach(async () => { - await cleanDatabase() - }) - describe('access email address', () => { + const userQuery = gql` + query($name: String) { + User(name: $name) { + email + } + } + ` + describe('unauthenticated', () => { beforeEach(() => { authenticatedUser = null }) + it("throws an error and does not expose the owner's email address", async () => { await expect( query({ query: userQuery, variables: { name: 'Owner' } }), @@ -143,7 +144,7 @@ describe('authorization', () => { }) }) - describe('administrator', () => { + describe('as an administrator', () => { beforeEach(async () => { authenticatedUser = await administrator.toJson() }) @@ -158,5 +159,97 @@ describe('authorization', () => { }) }) }) + + // Wolle describe('access reports protected propertied', () => { + // const reportsQuery = gql ` + // query { + // reports { + // id + // createdAt + // updatedAt + // rule + // disable + // closed + // filed + // reviewed + // resource + // } + // } + // ` + + // describe('unauthenticated', () => { + // beforeEach(() => { + // authenticatedUser = null + // }) + // it("throws an error and does not expose the owner's email address", async () => { + // await expect( + // query({ query: reportsQuery, variables: { name: 'Owner' } }), + // ).resolves.toMatchObject({ + // errors: [{ message: 'Not Authorised!' }], + // data: { User: [null] }, + // }) + // }) + // }) + + // describe('authenticated', () => { + // describe('as the owner', () => { + // beforeEach(async () => { + // authenticatedUser = await owner.toJson() + // }) + + // it("exposes the owner's email address", async () => { + // variables = { name: 'Owner' } + // await expect(query({ query: reportsQuery, variables })).resolves.toMatchObject({ + // data: { User: [{ email: 'owner@example.org' }] }, + // errors: undefined, + // }) + // }) + // }) + + // describe('as another regular user', () => { + // beforeEach(async () => { + // authenticatedUser = await anotherRegularUser.toJson() + // }) + + // it("throws an error and does not expose the owner's email address", async () => { + // await expect( + // query({ query: reportsQuery, variables: { name: 'Owner' } }), + // ).resolves.toMatchObject({ + // errors: [{ message: 'Not Authorised!' }], + // data: { User: [null] }, + // }) + // }) + // }) + + // describe('as a moderator', () => { + // beforeEach(async () => { + // authenticatedUser = await moderator.toJson() + // }) + + // it("throws an error and does not expose the owner's email address", async () => { + // await expect( + // query({ query: reportsQuery, variables: { name: 'Owner' } }), + // ).resolves.toMatchObject({ + // errors: [{ message: 'Not Authorised!' }], + // data: { User: [null] }, + // }) + // }) + // }) + + // describe('as an administrator', () => { + // beforeEach(async () => { + // authenticatedUser = await administrator.toJson() + // }) + + // it("exposes the owner's email address", async () => { + // variables = { name: 'Owner' } + // await expect(query({ query: reportsQuery, variables })).resolves.toMatchObject({ + // data: { User: [{ email: 'owner@example.org' }] }, + // errors: undefined, + // }) + // }) + // }) + // }) + // }) }) }) diff --git a/backend/src/schema/resolvers/reports.js b/backend/src/schema/resolvers/reports.js index 0565c4d8a..d68f60f36 100644 --- a/backend/src/schema/resolvers/reports.js +++ b/backend/src/schema/resolvers/reports.js @@ -1,14 +1,14 @@ import log from './helpers/databaseLogger' -const transformReturnType = record => { - return { - ...record.get('report').properties, - resource: { - __typename: record.get('type'), - ...record.get('resource').properties, - }, - } -} +// Wolle const transformReturnType = record => { +// return { +// ...record.get('report').properties, +// resource: { +// __typename: record.get('type'), +// ...record.get('resource').properties, +// }, +// } +// } export default { Mutation: { @@ -27,7 +27,8 @@ export default { WITH submitter, resource, report CREATE (report)<-[filed:FILED {createdAt: $createdAt, reasonCategory: $reasonCategory, reasonDescription: $reasonDescription}]-(submitter) - RETURN report, resource, labels(resource)[0] AS type + WITH report, resource {.*, __typename: labels(resource)[0]} AS finalResource + RETURN {reportId: report.id, resource: properties(finalResource)} AS filedReport `, { resourceId, @@ -38,13 +39,18 @@ export default { }, ) log(reportTransactionResponse) - return reportTransactionResponse.records.map(transformReturnType) + // Wolle return reportTransactionResponse.records.map(transformReturnType) + return reportTransactionResponse.records.map(record => + record.get('filedReport'), + ) }) try { const [createdRelationshipWithNestedAttributes] = await reportWriteTxResultPromise + console.log('createdRelationshipWithNestedAttributes: ', createdRelationshipWithNestedAttributes) if (!createdRelationshipWithNestedAttributes) return null return createdRelationshipWithNestedAttributes } finally { + console.log('session.close !!!') session.close() } }, diff --git a/backend/src/schema/resolvers/reports.spec.js b/backend/src/schema/resolvers/reports.spec.js index 0e690c19e..e411a2046 100644 --- a/backend/src/schema/resolvers/reports.spec.js +++ b/backend/src/schema/resolvers/reports.spec.js @@ -10,22 +10,54 @@ const driver = getDriver() describe('file a report on a resource', () => { let authenticatedUser, currentUser, mutate, query, moderator, abusiveUser, otherReportingUser const categoryIds = ['cat9'] - const reportMutation = gql` + // Wolle const reportMutation = gql` + // mutation($resourceId: ID!, $reasonCategory: ReasonCategory!, $reasonDescription: String!) { + // fileReport( + // resourceId: $resourceId + // reasonCategory: $reasonCategory + // reasonDescription: $reasonDescription + // ) { + // id + // createdAt + // updatedAt + // closed + // rule + // resource { + // __typename + // ... on User { + // name + // } + // ... on Post { + // title + // } + // ... on Comment { + // content + // } + // } + // filed { + // submitter { + // id + // } + // createdAt + // reasonCategory + // reasonDescription + // } + // } + // } + // ` + const fileReportMutation = gql` mutation($resourceId: ID!, $reasonCategory: ReasonCategory!, $reasonDescription: String!) { fileReport( resourceId: $resourceId reasonCategory: $reasonCategory reasonDescription: $reasonDescription ) { - id - createdAt - updatedAt - closed - rule + reportId resource { __typename ... on User { name + # Wolle filedUnclosedReportByCurrentUser } ... on Post { title @@ -34,6 +66,35 @@ describe('file a report on a resource', () => { content } } + } + } + ` + const variables = { + resourceId: 'invalid', + reasonCategory: 'other', + reasonDescription: 'Violates code of conduct !!!', + } + const reportsQuery = gql` + query { + reports(orderBy: createdAt_desc) { + id + createdAt + updatedAt + closed + resource { + __typename + ... on User { + id + # Wolle FOLLOWSfiledUnclosedReportByCurrentUser + followedByCurrentUser + } + ... on Post { + id + } + ... on Comment { + id + } + } filed { submitter { id @@ -45,11 +106,6 @@ describe('file a report on a resource', () => { } } ` - const variables = { - resourceId: 'whatever', - reasonCategory: 'other', - reasonDescription: 'Violates code of conduct !!!', - } beforeAll(async () => { await cleanDatabase() @@ -74,7 +130,7 @@ describe('file a report on a resource', () => { describe('unauthenticated', () => { it('throws authorization error', async () => { authenticatedUser = null - await expect(mutate({ mutation: reportMutation, variables })).resolves.toMatchObject({ + await expect(mutate({ mutation: fileReportMutation, variables })).resolves.toMatchObject({ data: { fileReport: null }, errors: [{ message: 'Not Authorised!' }], }) @@ -127,7 +183,7 @@ describe('file a report on a resource', () => { describe('invalid resource id', () => { it('returns null', async () => { - await expect(mutate({ mutation: reportMutation, variables })).resolves.toMatchObject({ + await expect(mutate({ mutation: fileReportMutation, variables })).resolves.toMatchObject({ data: { fileReport: null }, errors: undefined, }) @@ -136,16 +192,20 @@ describe('file a report on a resource', () => { describe('valid resource', () => { describe('creates report', () => { - it('which belongs to resource', async () => { + it.only('which belongs to resource now reported by current user', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }), ).resolves.toMatchObject({ data: { fileReport: { - id: expect.any(String), + reportId: expect.any(String), + resource: { + name: 'abusive-user', + // Wolle filedUnclosedReportByCurrentUser: true, + }, }, }, errors: undefined, @@ -154,12 +214,12 @@ describe('file a report on a resource', () => { it('creates only one report for multiple reports on the same resource', async () => { const firstReport = await mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }) authenticatedUser = await otherReportingUser.toJson() const secondReport = await mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }) expect(firstReport.data.fileReport.id).toEqual(secondReport.data.fileReport.id) @@ -168,7 +228,7 @@ describe('file a report on a resource', () => { it('returns the rule for how the report was decided', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }), ).resolves.toMatchObject({ @@ -187,7 +247,7 @@ describe('file a report on a resource', () => { it('returns __typename "User"', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }), ).resolves.toMatchObject({ @@ -205,7 +265,7 @@ describe('file a report on a resource', () => { it('returns user attribute info', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }), ).resolves.toMatchObject({ @@ -224,7 +284,7 @@ describe('file a report on a resource', () => { it('returns the submitter', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }), ).resolves.toMatchObject({ @@ -246,7 +306,7 @@ describe('file a report on a resource', () => { it('returns a date', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }), ).resolves.toMatchObject({ @@ -262,7 +322,7 @@ describe('file a report on a resource', () => { it('returns the reason category', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id', @@ -286,7 +346,7 @@ describe('file a report on a resource', () => { it('gives an error if the reason category is not in enum "ReasonCategory"', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id', @@ -307,7 +367,7 @@ describe('file a report on a resource', () => { it('returns the reason description', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id', @@ -331,7 +391,7 @@ describe('file a report on a resource', () => { it('sanitizes the reason description', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id', @@ -371,7 +431,7 @@ describe('file a report on a resource', () => { it('returns type "Post"', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'post-to-report-id', @@ -392,7 +452,7 @@ describe('file a report on a resource', () => { it('returns resource in post attribute', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'post-to-report-id', @@ -442,7 +502,7 @@ describe('file a report on a resource', () => { it('returns type "Comment"', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'comment-to-report-id', @@ -463,7 +523,7 @@ describe('file a report on a resource', () => { it('returns resource in comment attribute', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'comment-to-report-id', @@ -493,7 +553,7 @@ describe('file a report on a resource', () => { it('returns null', async () => { await expect( mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { ...variables, resourceId: 'tag-to-report-id', @@ -510,37 +570,6 @@ describe('file a report on a resource', () => { }) describe('query for reported resource', () => { - const reportsQuery = gql` - query { - reports(orderBy: createdAt_desc) { - id - createdAt - updatedAt - closed - resource { - __typename - ... on User { - id - } - ... on Post { - id - } - ... on Comment { - id - } - } - filed { - submitter { - id - } - createdAt - reasonCategory - reasonDescription - } - } - } - ` - beforeEach(async () => { authenticatedUser = null moderator = await Factory.build( @@ -632,7 +661,7 @@ describe('file a report on a resource', () => { authenticatedUser = await currentUser.toJson() await Promise.all([ mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { resourceId: 'abusive-post-1', reasonCategory: 'other', @@ -640,7 +669,7 @@ describe('file a report on a resource', () => { }, }), mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { resourceId: 'abusive-comment-1', reasonCategory: 'discrimination_etc', @@ -648,7 +677,7 @@ describe('file a report on a resource', () => { }, }), mutate({ - mutation: reportMutation, + mutation: fileReportMutation, variables: { resourceId: 'abusive-user-1', reasonCategory: 'doxing', @@ -678,7 +707,7 @@ describe('file a report on a resource', () => { }) }) - it('role "moderator" gets reports', async () => { + it.only('role "moderator" gets reports', async () => { const expected = { reports: expect.arrayContaining([ expect.objectContaining({ @@ -689,6 +718,8 @@ describe('file a report on a resource', () => { resource: { __typename: 'User', id: 'abusive-user-1', + // Wolle filedUnclosedReportByCurrentUser: false, + followedByCurrentUser: false, }, filed: expect.arrayContaining([ expect.objectContaining({ diff --git a/backend/src/schema/resolvers/users.js b/backend/src/schema/resolvers/users.js index cbdc683e8..6d8041716 100644 --- a/backend/src/schema/resolvers/users.js +++ b/backend/src/schema/resolvers/users.js @@ -251,12 +251,14 @@ export default { boolean: { followedByCurrentUser: 'MATCH (this)<-[:FOLLOWS]-(u:User {id: $cypherParams.currentUserId}) RETURN COUNT(u) >= 1', + filedUnclosedReportByCurrentUser: + 'MATCH (this)<-[:BELONGS_TO]-(:Report {closed: false})<-[:FILED]-(u:User {id: $cypherParams.currentUserId}) RETURN COUNT(u) >= 1', + isBlocked: + 'MATCH (this)<-[:BLOCKED]-(u:User {id: $cypherParams.currentUserId}) RETURN COUNT(u) >= 1', blocked: 'MATCH (this)-[:BLOCKED]-(u:User {id: $cypherParams.currentUserId}) RETURN COUNT(u) >= 1', isMuted: 'MATCH (this)<-[:MUTED]-(u:User {id: $cypherParams.currentUserId}) RETURN COUNT(u) >= 1', - isBlocked: - 'MATCH (this)<-[:BLOCKED]-(u:User {id: $cypherParams.currentUserId}) RETURN COUNT(u) >= 1', }, count: { contributionsCount: diff --git a/backend/src/schema/types/type/Report.gql b/backend/src/schema/types/type/Report.gql index ad0015d01..ae8c640fb 100644 --- a/backend/src/schema/types/type/Report.gql +++ b/backend/src/schema/types/type/Report.gql @@ -10,6 +10,11 @@ type Report { resource: ReportedResource } +type FiledReport { + reportId: ID! + resource: ReportedResource +} + union ReportedResource = User | Post | Comment enum ReportRule { @@ -17,7 +22,7 @@ enum ReportRule { } type Mutation { - fileReport(resourceId: ID!, reasonCategory: ReasonCategory!, reasonDescription: String!): Report + fileReport(resourceId: ID!, reasonCategory: ReasonCategory!, reasonDescription: String!): FiledReport } type Query { diff --git a/backend/src/schema/types/type/User.gql b/backend/src/schema/types/type/User.gql index baefc9d29..4f07a7ebc 100644 --- a/backend/src/schema/types/type/User.gql +++ b/backend/src/schema/types/type/User.gql @@ -64,10 +64,19 @@ type User { # Is the currently logged in user following that user? followedByCurrentUser: Boolean! @cypher( statement: """ - MATCH (this)<-[: FOLLOWS]-(u: User { id: $cypherParams.currentUserId}) + MATCH (this)<-[:FOLLOWS]-(u:User { id: $cypherParams.currentUserId}) RETURN COUNT(u) >= 1 """ ) + + # Has the currently logged in user reported that user? + filedUnclosedReportByCurrentUser: Boolean! @cypher( + statement: """ + MATCH (this)<-[:BELONGS_TO]-(:Report {closed: false})<-[:FILED]-(u:User { id: $cypherParams.currentUserId}) + RETURN COUNT(u) >= 1 + """ + ) + isBlocked: Boolean! @cypher( statement: """ MATCH (this)<-[:BLOCKED]-(user:User {id: $cypherParams.currentUserId}) From 5ffaac193d79e0a4c9d33dbaaf7d22230429bca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolfgang=20Hu=C3=9F?= Date: Wed, 19 Feb 2020 11:37:31 +0100 Subject: [PATCH 3/9] Refactor tests, going on --- backend/src/schema/resolvers/moderation.js | 32 ++--- backend/src/schema/resolvers/reports.js | 20 +-- backend/src/schema/resolvers/reports.spec.js | 136 ++++++++++++++++--- backend/src/schema/types/type/REVIEWED.gql | 1 - 4 files changed, 142 insertions(+), 47 deletions(-) diff --git a/backend/src/schema/resolvers/moderation.js b/backend/src/schema/resolvers/moderation.js index 4bdf82d50..08a3b6ff3 100644 --- a/backend/src/schema/resolvers/moderation.js +++ b/backend/src/schema/resolvers/moderation.js @@ -1,20 +1,10 @@ -const transformReturnType = record => { - return { - ...record.get('review').properties, - report: record.get('report').properties, - resource: { - __typename: record.get('type'), - ...record.get('resource').properties, - }, - } -} +import log from './helpers/databaseLogger' export default { Mutation: { review: async (_object, params, context, _resolveInfo) => { const { user: moderator, driver } = context - let createdRelationshipWithNestedAttributes = null // return value const session = driver.session() try { const cypher = ` @@ -25,10 +15,11 @@ export default { ON CREATE SET review.createdAt = $dateTime, review.updatedAt = review.createdAt ON MATCH SET review.updatedAt = $dateTime SET review.disable = $params.disable - SET report.updatedAt = $dateTime, report.closed = $params.closed - SET resource.disabled = review.disable + SET report.updatedAt = $dateTime, report.disable = review.disable, report.closed = $params.closed + SET resource.disabled = report.disable - RETURN review, report, resource, labels(resource)[0] AS type + WITH review, report, resource {.*, __typename: labels(resource)[0]} AS finalResource + RETURN review {.*, report: properties(report), resource: properties(finalResource)} ` const reviewWriteTxResultPromise = session.writeTransaction(async txc => { const reviewTransactionResponse = await txc.run(cypher, { @@ -36,16 +27,17 @@ export default { moderatorId: moderator.id, dateTime: new Date().toISOString(), }) - return reviewTransactionResponse.records.map(transformReturnType) + log(reviewTransactionResponse) + return reviewTransactionResponse.records.map(record => + record.get('review'), + ) }) - const txResult = await reviewWriteTxResultPromise - if (!txResult[0]) return null - createdRelationshipWithNestedAttributes = txResult[0] + const [reviewed] = await reviewWriteTxResultPromise + // Wolle console.log('reviewed: ', reviewed) + return reviewed || null } finally { session.close() } - - return createdRelationshipWithNestedAttributes }, }, } diff --git a/backend/src/schema/resolvers/reports.js b/backend/src/schema/resolvers/reports.js index d68f60f36..8e6bcf786 100644 --- a/backend/src/schema/resolvers/reports.js +++ b/backend/src/schema/resolvers/reports.js @@ -16,8 +16,8 @@ export default { const { resourceId, reasonCategory, reasonDescription } = params const { driver, user } = context const session = driver.session() - const reportWriteTxResultPromise = session.writeTransaction(async transaction => { - const reportTransactionResponse = await transaction.run( + const fileReportWriteTxResultPromise = session.writeTransaction(async transaction => { + const fileReportTransactionResponse = await transaction.run( ` MATCH (submitter:User {id: $submitterId}) MATCH (resource {id: $resourceId}) @@ -38,19 +38,19 @@ export default { reasonDescription, }, ) - log(reportTransactionResponse) - // Wolle return reportTransactionResponse.records.map(transformReturnType) - return reportTransactionResponse.records.map(record => + log(fileReportTransactionResponse) + // Wolle return fileReportTransactionResponse.records.map(transformReturnType) + return fileReportTransactionResponse.records.map(record => record.get('filedReport'), ) }) try { - const [createdRelationshipWithNestedAttributes] = await reportWriteTxResultPromise - console.log('createdRelationshipWithNestedAttributes: ', createdRelationshipWithNestedAttributes) - if (!createdRelationshipWithNestedAttributes) return null - return createdRelationshipWithNestedAttributes + const [filedReport] = await fileReportWriteTxResultPromise + console.log('filedReport: ', filedReport) + // Wolle if (!filedReport) return null + return filedReport || null } finally { - console.log('session.close !!!') + console.log('fileReport: session.close !!!') session.close() } }, diff --git a/backend/src/schema/resolvers/reports.spec.js b/backend/src/schema/resolvers/reports.spec.js index e411a2046..6a9df6639 100644 --- a/backend/src/schema/resolvers/reports.spec.js +++ b/backend/src/schema/resolvers/reports.spec.js @@ -75,18 +75,20 @@ describe('file a report on a resource', () => { reasonDescription: 'Violates code of conduct !!!', } const reportsQuery = gql` - query { - reports(orderBy: createdAt_desc) { + query($closed: Boolean) { + reports(orderBy: createdAt_desc, closed: $closed) { id createdAt updatedAt + rule + disable closed resource { __typename ... on User { id - # Wolle FOLLOWSfiledUnclosedReportByCurrentUser - followedByCurrentUser + # Wolle filedUnclosedReportByCurrentUser + # Wolle test followedByCurrentUser } ... on Post { id @@ -106,6 +108,31 @@ describe('file a report on a resource', () => { } } ` + const reviewMutation = gql` + mutation($resourceId: ID!, $disable: Boolean, $closed: Boolean) { + review(resourceId: $resourceId, disable: $disable, closed: $closed) { + createdAt + resource { + __typename + ... on User { + id + disabled + } + ... on Post { + id + disabled + } + ... on Comment { + id + disabled + } + } + report { + disable + } + } + } + ` beforeAll(async () => { await cleanDatabase() @@ -150,6 +177,17 @@ describe('file a report on a resource', () => { password: '1234', }, ) + moderator = await Factory.build( + 'user', + { + id: 'moderator-id', + role: 'moderator', + }, + { + email: 'moderator@example.org', + password: '1234', + }, + ) otherReportingUser = await Factory.build( 'user', { @@ -192,7 +230,8 @@ describe('file a report on a resource', () => { describe('valid resource', () => { describe('creates report', () => { - it.only('which belongs to resource now reported by current user', async () => { + it('which belongs to resource', async () => { + // Wolle it('which belongs to resource now reported by current user', async () => { await expect( mutate({ mutation: fileReportMutation, @@ -212,7 +251,7 @@ describe('file a report on a resource', () => { }) }) - it('creates only one report for multiple reports on the same resource', async () => { + it('only one report for multiple reports on the same resource', async () => { const firstReport = await mutate({ mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, @@ -222,24 +261,89 @@ describe('file a report on a resource', () => { mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }) - expect(firstReport.data.fileReport.id).toEqual(secondReport.data.fileReport.id) + expect(firstReport.data.fileReport.reportId).toEqual(secondReport.data.fileReport.reportId) }) - it('returns the rule for how the report was decided', async () => { + it('with the rule for how the report will be decided', async () => { + await mutate({ + mutation: fileReportMutation, + variables: { ...variables, resourceId: 'abusive-user-id' }, + }) + authenticatedUser = await moderator.toJson() await expect( - mutate({ - mutation: fileReportMutation, - variables: { ...variables, resourceId: 'abusive-user-id' }, - }), + query({ + query: reportsQuery + }) ).resolves.toMatchObject({ data: { - fileReport: { + reports: [{ rule: 'latestReviewUpdatedAtRules', - }, + }], }, errors: undefined, }) }) + + describe('with overtaken disabled from resource in disable property', () => { + it('disable is false', async () => { + await mutate({ + mutation: fileReportMutation, + variables: { ...variables, resourceId: 'abusive-user-id' }, + }) + authenticatedUser = await moderator.toJson() + await expect( + query({ + query: reportsQuery + }), + ).resolves.toMatchObject({ + data: { + reports: [{ + disable: false, + }], + }, + errors: undefined, + }) + }) + + it.only('disable is true', async () => { + // first time filling a report to enable a moderator the disable the resource + await mutate({ + mutation: fileReportMutation, + variables: { ...variables, resourceId: 'abusive-user-id' }, + }) + authenticatedUser = await moderator.toJson() + const review = await mutate({ + mutation: reviewMutation, + variables: { + resourceId: 'abusive-user-id', + disable: true, + closed: true, + }, + }) + console.log('review: ', review) + authenticatedUser = await currentUser.toJson() + // second time filling a report to see if the "disabled is true" of the resource is overtaken + await mutate({ + mutation: fileReportMutation, + variables: { ...variables, resourceId: 'abusive-user-id' }, + }) + // authenticatedUser = await moderator.toJson() + // await expect( + // query({ + // query: reportsQuery, + // variables: { closed: false }, + // }), + // ).resolves.toMatchObject({ + // data: { + // reports: [{ + // disable: true, + // }], + // }, + // errors: undefined, + // }) + }) + }) + it.todo('creates multiple filed reports') }) @@ -707,7 +811,7 @@ describe('file a report on a resource', () => { }) }) - it.only('role "moderator" gets reports', async () => { + it('role "moderator" gets reports', async () => { const expected = { reports: expect.arrayContaining([ expect.objectContaining({ @@ -719,7 +823,7 @@ describe('file a report on a resource', () => { __typename: 'User', id: 'abusive-user-1', // Wolle filedUnclosedReportByCurrentUser: false, - followedByCurrentUser: false, + // Wolle test followedByCurrentUser: false, }, filed: expect.arrayContaining([ expect.objectContaining({ diff --git a/backend/src/schema/types/type/REVIEWED.gql b/backend/src/schema/types/type/REVIEWED.gql index aea005abe..086d73815 100644 --- a/backend/src/schema/types/type/REVIEWED.gql +++ b/backend/src/schema/types/type/REVIEWED.gql @@ -4,7 +4,6 @@ type REVIEWED { disable: Boolean! closed: Boolean! report: Report - # @cypher(statement: "MATCH (report:Report)<-[this:REVIEWED]-(:User) RETURN report") moderator: User resource: ReviewedResource } From 3421afe4e00b5444033979f04346c8aa38873c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolfgang=20Hu=C3=9F?= Date: Wed, 19 Feb 2020 13:27:01 +0100 Subject: [PATCH 4/9] Refactor tests, next step --- .../validation/validationMiddleware.spec.js | 2 +- backend/src/schema/resolvers/moderation.js | 4 +- backend/src/schema/resolvers/reports.js | 36 ++++---- backend/src/schema/resolvers/reports.spec.js | 92 +++++++++---------- backend/src/schema/types/type/FILED.gql | 12 +++ backend/src/schema/types/type/Report.gql | 9 -- .../features/ReportList/ReportList.vue | 3 +- 7 files changed, 78 insertions(+), 80 deletions(-) diff --git a/backend/src/middleware/validation/validationMiddleware.spec.js b/backend/src/middleware/validation/validationMiddleware.spec.js index b2c669369..74a343eeb 100644 --- a/backend/src/middleware/validation/validationMiddleware.spec.js +++ b/backend/src/middleware/validation/validationMiddleware.spec.js @@ -58,7 +58,7 @@ const reportMutation = gql` reasonCategory: $reasonCategory reasonDescription: $reasonDescription ) { - id + reportId } } ` diff --git a/backend/src/schema/resolvers/moderation.js b/backend/src/schema/resolvers/moderation.js index 08a3b6ff3..58090c762 100644 --- a/backend/src/schema/resolvers/moderation.js +++ b/backend/src/schema/resolvers/moderation.js @@ -28,9 +28,7 @@ export default { dateTime: new Date().toISOString(), }) log(reviewTransactionResponse) - return reviewTransactionResponse.records.map(record => - record.get('review'), - ) + return reviewTransactionResponse.records.map(record => record.get('review')) }) const [reviewed] = await reviewWriteTxResultPromise // Wolle console.log('reviewed: ', reviewed) diff --git a/backend/src/schema/resolvers/reports.js b/backend/src/schema/resolvers/reports.js index 8e6bcf786..6a9366a89 100644 --- a/backend/src/schema/resolvers/reports.js +++ b/backend/src/schema/resolvers/reports.js @@ -1,15 +1,5 @@ import log from './helpers/databaseLogger' -// Wolle const transformReturnType = record => { -// return { -// ...record.get('report').properties, -// resource: { -// __typename: record.get('type'), -// ...record.get('resource').properties, -// }, -// } -// } - export default { Mutation: { fileReport: async (_parent, params, context, _resolveInfo) => { @@ -27,8 +17,8 @@ export default { WITH submitter, resource, report CREATE (report)<-[filed:FILED {createdAt: $createdAt, reasonCategory: $reasonCategory, reasonDescription: $reasonDescription}]-(submitter) - WITH report, resource {.*, __typename: labels(resource)[0]} AS finalResource - RETURN {reportId: report.id, resource: properties(finalResource)} AS filedReport + WITH filed, report, resource {.*, __typename: labels(resource)[0]} AS finalResource + RETURN filed {.*, reportId: report.id, resource: properties(finalResource)} AS filedReport `, { resourceId, @@ -40,17 +30,13 @@ export default { ) log(fileReportTransactionResponse) // Wolle return fileReportTransactionResponse.records.map(transformReturnType) - return fileReportTransactionResponse.records.map(record => - record.get('filedReport'), - ) + return fileReportTransactionResponse.records.map(record => record.get('filedReport')) }) try { const [filedReport] = await fileReportWriteTxResultPromise - console.log('filedReport: ', filedReport) - // Wolle if (!filedReport) return null + // Wolle console.log('filedReport: ', filedReport) return filedReport || null } finally { - console.log('fileReport: session.close !!!') session.close() } }, @@ -71,6 +57,7 @@ export default { orderByClause = '' } + // Wolle This should be only for open reports or? switch (params.reviewed) { case true: filterClause = 'AND ((report)<-[:REVIEWED]-(:User))' @@ -82,7 +69,18 @@ export default { filterClause = '' } - if (params.closed) filterClause = 'AND report.closed = true' + // Wolle console.log('params.closed: ', params.closed) + // Wolle if (params.closed) filterClause = 'AND report.closed = true' + switch (params.closed) { + case true: + filterClause = 'AND report.closed = true' + break + case false: + filterClause = 'AND report.closed = false' + break + default: + break + } const offset = params.offset && typeof params.offset === 'number' ? `SKIP ${params.offset}` : '' diff --git a/backend/src/schema/resolvers/reports.spec.js b/backend/src/schema/resolvers/reports.spec.js index 6a9df6639..e3c2e659f 100644 --- a/backend/src/schema/resolvers/reports.spec.js +++ b/backend/src/schema/resolvers/reports.spec.js @@ -52,6 +52,9 @@ describe('file a report on a resource', () => { reasonCategory: $reasonCategory reasonDescription: $reasonDescription ) { + createdAt + reasonCategory + reasonDescription reportId resource { __typename @@ -231,7 +234,7 @@ describe('file a report on a resource', () => { describe('valid resource', () => { describe('creates report', () => { it('which belongs to resource', async () => { - // Wolle it('which belongs to resource now reported by current user', async () => { + // Wolle it('which belongs to resource now reported by current user', async () => { await expect( mutate({ mutation: fileReportMutation, @@ -261,7 +264,9 @@ describe('file a report on a resource', () => { mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }) - expect(firstReport.data.fileReport.reportId).toEqual(secondReport.data.fileReport.reportId) + expect(firstReport.data.fileReport.reportId).toEqual( + secondReport.data.fileReport.reportId, + ) }) it('with the rule for how the report will be decided', async () => { @@ -272,13 +277,15 @@ describe('file a report on a resource', () => { authenticatedUser = await moderator.toJson() await expect( query({ - query: reportsQuery - }) + query: reportsQuery, + }), ).resolves.toMatchObject({ data: { - reports: [{ - rule: 'latestReviewUpdatedAtRules', - }], + reports: [ + { + rule: 'latestReviewUpdatedAtRules', + }, + ], }, errors: undefined, }) @@ -293,26 +300,28 @@ describe('file a report on a resource', () => { authenticatedUser = await moderator.toJson() await expect( query({ - query: reportsQuery + query: reportsQuery, }), ).resolves.toMatchObject({ data: { - reports: [{ - disable: false, - }], + reports: [ + { + disable: false, + }, + ], }, errors: undefined, }) }) - it.only('disable is true', async () => { + it('disable is true', async () => { // first time filling a report to enable a moderator the disable the resource await mutate({ mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }) authenticatedUser = await moderator.toJson() - const review = await mutate({ + await mutate({ mutation: reviewMutation, variables: { resourceId: 'abusive-user-id', @@ -320,30 +329,31 @@ describe('file a report on a resource', () => { closed: true, }, }) - console.log('review: ', review) authenticatedUser = await currentUser.toJson() - // second time filling a report to see if the "disabled is true" of the resource is overtaken + // second time filling a report to see if the "disabled is true" of the resource is overtaken await mutate({ mutation: fileReportMutation, variables: { ...variables, resourceId: 'abusive-user-id' }, }) - // authenticatedUser = await moderator.toJson() - // await expect( - // query({ - // query: reportsQuery, - // variables: { closed: false }, - // }), - // ).resolves.toMatchObject({ - // data: { - // reports: [{ - // disable: true, - // }], - // }, - // errors: undefined, - // }) + authenticatedUser = await moderator.toJson() + await expect( + query({ + query: reportsQuery, + variables: { closed: false }, + }), + ).resolves.toMatchObject({ + data: { + reports: [ + { + disable: true, + }, + ], + }, + errors: undefined, + }) }) }) - + it.todo('creates multiple filed reports') }) @@ -385,7 +395,7 @@ describe('file a report on a resource', () => { }) }) - it('returns the submitter', async () => { + it.skip('returns the submitter', async () => { await expect( mutate({ mutation: fileReportMutation, @@ -407,7 +417,7 @@ describe('file a report on a resource', () => { }) }) - it('returns a date', async () => { + it('returns a createdAt', async () => { await expect( mutate({ mutation: fileReportMutation, @@ -436,11 +446,7 @@ describe('file a report on a resource', () => { ).resolves.toMatchObject({ data: { fileReport: { - filed: [ - { - reasonCategory: 'criminal_behavior_violation_german_law', - }, - ], + reasonCategory: 'criminal_behavior_violation_german_law', }, }, errors: undefined, @@ -481,11 +487,7 @@ describe('file a report on a resource', () => { ).resolves.toMatchObject({ data: { fileReport: { - filed: [ - { - reasonDescription: 'My reason!', - }, - ], + reasonDescription: 'My reason!', }, }, errors: undefined, @@ -505,11 +507,7 @@ describe('file a report on a resource', () => { ).resolves.toMatchObject({ data: { fileReport: { - filed: [ - { - reasonDescription: 'My reason !', - }, - ], + reasonDescription: 'My reason !', }, }, errors: undefined, diff --git a/backend/src/schema/types/type/FILED.gql b/backend/src/schema/types/type/FILED.gql index cdce62116..85eca951e 100644 --- a/backend/src/schema/types/type/FILED.gql +++ b/backend/src/schema/types/type/FILED.gql @@ -16,3 +16,15 @@ enum ReasonCategory { advert_products_services_commercial criminal_behavior_violation_german_law } + +type FiledReport { + createdAt: String! + reasonCategory: ReasonCategory! + reasonDescription: String! + reportId: ID! + resource: ReportedResource! +} + +type Mutation { + fileReport(resourceId: ID!, reasonCategory: ReasonCategory!, reasonDescription: String!): FiledReport +} \ No newline at end of file diff --git a/backend/src/schema/types/type/Report.gql b/backend/src/schema/types/type/Report.gql index ae8c640fb..092530318 100644 --- a/backend/src/schema/types/type/Report.gql +++ b/backend/src/schema/types/type/Report.gql @@ -10,21 +10,12 @@ type Report { resource: ReportedResource } -type FiledReport { - reportId: ID! - resource: ReportedResource -} - union ReportedResource = User | Post | Comment enum ReportRule { latestReviewUpdatedAtRules } -type Mutation { - fileReport(resourceId: ID!, reasonCategory: ReasonCategory!, reasonDescription: String!): FiledReport -} - type Query { reports(orderBy: ReportOrdering, first: Int, offset: Int, reviewed: Boolean, closed: Boolean): [Report] } diff --git a/webapp/components/features/ReportList/ReportList.vue b/webapp/components/features/ReportList/ReportList.vue index 62a29e66b..9e35fe034 100644 --- a/webapp/components/features/ReportList/ReportList.vue +++ b/webapp/components/features/ReportList/ReportList.vue @@ -44,10 +44,11 @@ export default { filterOptions() { return [ { label: this.$t('moderation.reports.filterLabel.all'), value: { reviewed: null } }, - { + { // Wolle This should be only for open reports or? label: this.$t('moderation.reports.filterLabel.unreviewed'), value: { reviewed: false }, }, + // Wolle This should be only for open reports or? { label: this.$t('moderation.reports.filterLabel.reviewed'), value: { reviewed: true } }, { label: this.$t('moderation.reports.filterLabel.closed'), value: { closed: true } }, ] From a84d78e3ee72f285ef88f55478d3d5abac2697a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolfgang=20Hu=C3=9F?= Date: Thu, 20 Feb 2020 10:23:24 +0100 Subject: [PATCH 5/9] Fix key doubling error in ReportTable.vue - Co-Authored-By: mattwr18 --- webapp/components/features/ReportsTable/ReportsTable.vue | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webapp/components/features/ReportsTable/ReportsTable.vue b/webapp/components/features/ReportsTable/ReportsTable.vue index 512c04ea8..5736cc716 100644 --- a/webapp/components/features/ReportsTable/ReportsTable.vue +++ b/webapp/components/features/ReportsTable/ReportsTable.vue @@ -24,8 +24,9 @@