From 4bd7f61fccc14501729caf6e199943cba4f999de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wolfgang=20Hu=C3=9F?= Date: Thu, 20 Feb 2020 10:35:37 +0100 Subject: [PATCH] Refactor GQL schema, resolvers and moderators report list - Co-Authored-By: mattwr18 --- .../src/middleware/permissionsMiddleware.js | 10 +- backend/src/schema/resolvers/reports.js | 14 +- backend/src/schema/resolvers/reports.spec.js | 192 ++++++------------ backend/src/schema/types/type/Report.gql | 4 +- .../features/ReportList/ReportList.vue | 18 +- webapp/graphql/Moderation.js | 2 +- 6 files changed, 78 insertions(+), 162 deletions(-) diff --git a/backend/src/middleware/permissionsMiddleware.js b/backend/src/middleware/permissionsMiddleware.js index 107a45a35..10dc98845 100644 --- a/backend/src/middleware/permissionsMiddleware.js +++ b/backend/src/middleware/permissionsMiddleware.js @@ -152,15 +152,7 @@ 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), - }, + Report: isModerator, }, { debug, diff --git a/backend/src/schema/resolvers/reports.js b/backend/src/schema/resolvers/reports.js index 6a9366a89..7556b9c9e 100644 --- a/backend/src/schema/resolvers/reports.js +++ b/backend/src/schema/resolvers/reports.js @@ -57,7 +57,6 @@ export default { orderByClause = '' } - // Wolle This should be only for open reports or? switch (params.reviewed) { case true: filterClause = 'AND ((report)<-[:REVIEWED]-(:User))' @@ -69,8 +68,6 @@ export default { filterClause = '' } - // 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' @@ -86,8 +83,9 @@ export default { params.offset && typeof params.offset === 'number' ? `SKIP ${params.offset}` : '' const limit = params.first && typeof params.first === 'number' ? `LIMIT ${params.first}` : '' - const reportReadTxPromise = session.readTransaction(async transaction => { - const allReportsTransactionResponse = await transaction.run( + const reportsReadTxPromise = session.readTransaction(async transaction => { + const reportsTransactionResponse = await transaction.run( + // !!! this Cypher query returns multiple reports on the same resource! i will create an issue for refactoring (bug fixing) ` MATCH (report:Report)-[:BELONGS_TO]->(resource) WHERE (resource:User OR resource:Post OR resource:Comment) @@ -105,11 +103,11 @@ export default { ${offset} ${limit} `, ) - log(allReportsTransactionResponse) - return allReportsTransactionResponse.records.map(record => record.get('report')) + log(reportsTransactionResponse) + return reportsTransactionResponse.records.map(record => record.get('report')) }) try { - const reports = await reportReadTxPromise + const reports = await reportsReadTxPromise return reports } finally { session.close() diff --git a/backend/src/schema/resolvers/reports.spec.js b/backend/src/schema/resolvers/reports.spec.js index e3c2e659f..97b0a2ac6 100644 --- a/backend/src/schema/resolvers/reports.spec.js +++ b/backend/src/schema/resolvers/reports.spec.js @@ -10,41 +10,6 @@ const driver = getDriver() describe('file a report on a resource', () => { let authenticatedUser, currentUser, mutate, query, moderator, abusiveUser, otherReportingUser const categoryIds = ['cat9'] - // 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( @@ -61,6 +26,7 @@ describe('file a report on a resource', () => { ... on User { name # Wolle filedUnclosedReportByCurrentUser + # Wolle test followedByCurrentUser } ... on Post { title @@ -90,8 +56,6 @@ describe('file a report on a resource', () => { __typename ... on User { id - # Wolle filedUnclosedReportByCurrentUser - # Wolle test followedByCurrentUser } ... on Post { id @@ -264,92 +228,82 @@ 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, ) }) - 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( - query({ - query: reportsQuery, - }), - ).resolves.toMatchObject({ - data: { - reports: [ - { - rule: 'latestReviewUpdatedAtRules', - }, - ], - }, - errors: undefined, - }) - }) + describe('report properties are set correctly', () => { + const reportsCypherQuery = + 'MATCH (resource:User {id: $resourceId})<-[:BELONGS_TO]-(report:Report {closed: false})<-[filed:FILED]-(user:User {id: $currentUserId}) RETURN report' - describe('with overtaken disabled from resource in disable property', () => { - it('disable is false', 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( - query({ - query: reportsQuery, - }), - ).resolves.toMatchObject({ - data: { - reports: [ - { - disable: false, - }, - ], - }, - errors: undefined, + + const reportsCypherQueryResponse = await instance.cypher(reportsCypherQuery, { + resourceId: 'abusive-user-id', + currentUserId: authenticatedUser.id, }) + expect(reportsCypherQueryResponse.records).toHaveLength(1) + const [reportProperties] = reportsCypherQueryResponse.records.map( + record => record.get('report').properties, + ) + expect(reportProperties).toMatchObject({ rule: 'latestReviewUpdatedAtRules' }) }) - 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() - await mutate({ - mutation: reviewMutation, - variables: { + describe('with overtaken disabled from resource in disable property', () => { + it('disable is false', async () => { + await mutate({ + mutation: fileReportMutation, + variables: { ...variables, resourceId: 'abusive-user-id' }, + }) + + const reportsCypherQueryResponse = await instance.cypher(reportsCypherQuery, { resourceId: 'abusive-user-id', - disable: true, - closed: true, - }, + currentUserId: authenticatedUser.id, + }) + expect(reportsCypherQueryResponse.records).toHaveLength(1) + const [reportProperties] = reportsCypherQueryResponse.records.map( + record => record.get('report').properties, + ) + expect(reportProperties).toMatchObject({ disable: false }) }) - 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('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() + await mutate({ + mutation: reviewMutation, + variables: { + resourceId: 'abusive-user-id', + disable: true, + closed: true, + }, + }) + authenticatedUser = await currentUser.toJson() + // second time filling a report to see if the "disable is true" of the resource is overtaken + await mutate({ + mutation: fileReportMutation, + variables: { ...variables, resourceId: 'abusive-user-id' }, + }) + + const reportsCypherQueryResponse = await instance.cypher(reportsCypherQuery, { + resourceId: 'abusive-user-id', + currentUserId: authenticatedUser.id, + }) + expect(reportsCypherQueryResponse.records).toHaveLength(1) + const [reportProperties] = reportsCypherQueryResponse.records.map( + record => record.get('report').properties, + ) + expect(reportProperties).toMatchObject({ disable: true }) }) }) }) @@ -395,28 +349,6 @@ describe('file a report on a resource', () => { }) }) - it.skip('returns the submitter', async () => { - await expect( - mutate({ - mutation: fileReportMutation, - variables: { ...variables, resourceId: 'abusive-user-id' }, - }), - ).resolves.toMatchObject({ - data: { - fileReport: { - filed: [ - { - submitter: { - id: 'current-user-id', - }, - }, - ], - }, - }, - errors: undefined, - }) - }) - it('returns a createdAt', async () => { await expect( mutate({ diff --git a/backend/src/schema/types/type/Report.gql b/backend/src/schema/types/type/Report.gql index 092530318..9a4a48c4b 100644 --- a/backend/src/schema/types/type/Report.gql +++ b/backend/src/schema/types/type/Report.gql @@ -5,9 +5,9 @@ type Report { rule: ReportRule! disable: Boolean! closed: Boolean! - filed: [FILED] + filed: [FILED]! reviewed: [REVIEWED]! - resource: ReportedResource + resource: ReportedResource! } union ReportedResource = User | Post | Comment diff --git a/webapp/components/features/ReportList/ReportList.vue b/webapp/components/features/ReportList/ReportList.vue index 9e35fe034..865599dee 100644 --- a/webapp/components/features/ReportList/ReportList.vue +++ b/webapp/components/features/ReportList/ReportList.vue @@ -43,14 +43,13 @@ export default { computed: { 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.all'), value: { reviewed: null, closed: null } }, + { label: this.$t('moderation.reports.filterLabel.unreviewed'), - value: { reviewed: false }, + value: { reviewed: false, closed: 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 } }, + { label: this.$t('moderation.reports.filterLabel.reviewed'), value: { reviewed: true, closed: false } }, + { label: this.$t('moderation.reports.filterLabel.closed'), value: { reviewed: null, closed: true } }, ] }, modalData() { @@ -108,13 +107,8 @@ export default { filter(option) { this.selected = option.label this.offset = 0 - if (option.value.closed) { - this.closed = option.value.closed - this.reviewed = null - return - } - this.closed = null this.reviewed = option.value.reviewed + this.closed = option.value.closed }, async confirmCallback(resource) { const { disabled: disable, id: resourceId } = resource diff --git a/webapp/graphql/Moderation.js b/webapp/graphql/Moderation.js index c63df921e..f4796d080 100644 --- a/webapp/graphql/Moderation.js +++ b/webapp/graphql/Moderation.js @@ -100,7 +100,7 @@ export const reportMutation = () => { reasonCategory: $reasonCategory reasonDescription: $reasonDescription ) { - id + reportId } } `