From 9867bb6a6f7397d6fc8ce9ec78176076d18d5cf0 Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 5 Jul 2022 09:21:29 +0200 Subject: [PATCH 01/14] Add RIGHT for DELETE_CONTRIBUTION. --- backend/src/auth/RIGHTS.ts | 1 + backend/src/auth/ROLES.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/backend/src/auth/RIGHTS.ts b/backend/src/auth/RIGHTS.ts index c10fc96de..abe600c99 100644 --- a/backend/src/auth/RIGHTS.ts +++ b/backend/src/auth/RIGHTS.ts @@ -26,6 +26,7 @@ export enum RIGHTS { LIST_TRANSACTION_LINKS = 'LIST_TRANSACTION_LINKS', GDT_BALANCE = 'GDT_BALANCE', CREATE_CONTRIBUTION = 'CREATE_CONTRIBUTION', + DELETE_CONTRIBUTION = 'DELETE_CONTRIBUTION', // Admin SEARCH_USERS = 'SEARCH_USERS', SET_USER_ROLE = 'SET_USER_ROLE', diff --git a/backend/src/auth/ROLES.ts b/backend/src/auth/ROLES.ts index 2d9ac2deb..539158986 100644 --- a/backend/src/auth/ROLES.ts +++ b/backend/src/auth/ROLES.ts @@ -24,6 +24,7 @@ export const ROLE_USER = new Role('user', [ RIGHTS.LIST_TRANSACTION_LINKS, RIGHTS.GDT_BALANCE, RIGHTS.CREATE_CONTRIBUTION, + RIGHTS.DELETE_CONTRIBUTION, ]) export const ROLE_ADMIN = new Role('admin', Object.values(RIGHTS)) // all rights From 62e7abe76efd5974f7840fbf8f881e28f0fa73ea Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 5 Jul 2022 09:22:32 +0200 Subject: [PATCH 02/14] Add mutation to soft delete a contribution. --- .../src/graphql/resolver/ContributionResolver.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index 924108f87..e75471c08 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -2,7 +2,7 @@ import { RIGHTS } from '@/auth/RIGHTS' import { Context, getUser } from '@/server/context' import { backendLogger as logger } from '@/server/logger' import { Contribution } from '@entity/Contribution' -import { Args, Authorized, Ctx, Mutation, Resolver } from 'type-graphql' +import { Arg, Args, Authorized, Ctx, Int, Mutation, Resolver } from 'type-graphql' import ContributionArgs from '../arg/ContributionArgs' import { UnconfirmedContribution } from '../model/UnconfirmedContribution' import { isContributionValid, getUserCreation } from './util/isContributionValid' @@ -32,4 +32,15 @@ export class ContributionResolver { await Contribution.save(contribution) return new UnconfirmedContribution(contribution, user, creations) } + + @Authorized([RIGHTS.DELETE_CONTRIBUTION]) + @Mutation(() => Boolean) + async adminDeleteContribution(@Arg('id', () => Int) id: number): Promise { + const contribution = await Contribution.findOne(id) + if (!contribution) { + throw new Error('Contribution not found for given id.') + } + const res = await contribution.softRemove() + return !!res + } } From 9bd57109333274928ed6056316fbe2cd0d9588ff Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 5 Jul 2022 09:37:41 +0200 Subject: [PATCH 03/14] Add check if user is owner of the contribution before deleting. --- backend/src/graphql/resolver/ContributionResolver.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index e75471c08..bdd8e74a4 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -35,11 +35,18 @@ export class ContributionResolver { @Authorized([RIGHTS.DELETE_CONTRIBUTION]) @Mutation(() => Boolean) - async adminDeleteContribution(@Arg('id', () => Int) id: number): Promise { + async adminDeleteContribution( + @Arg('id', () => Int) id: number, + @Ctx() context: Context, + ): Promise { + const user = getUser(context) const contribution = await Contribution.findOne(id) if (!contribution) { throw new Error('Contribution not found for given id.') } + if (contribution.userId !== user.id) { + throw new Error('Can not delete contribution of another user') + } const res = await contribution.softRemove() return !!res } From 93c2d6814ad1952105d1fbdc05f94ef174712c10 Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 11:27:44 +0200 Subject: [PATCH 04/14] Change the listContribution query to give back ContributionListResult object with linkCount. --- backend/src/graphql/resolver/ContributionResolver.ts | 12 ++++++++---- backend/src/seeds/graphql/queries.ts | 9 ++++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index d71ecac59..c834904d6 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -2,6 +2,7 @@ import { RIGHTS } from '@/auth/RIGHTS' import { Context, getUser } from '@/server/context' import { backendLogger as logger } from '@/server/logger' import { Contribution as dbContribution } from '@entity/Contribution' +import { User as dbUser } from '@entity/User' import { Arg, Args, Authorized, Ctx, Int, Mutation, Query, Resolver } from 'type-graphql' import { FindOperator, IsNull } from '@dbTools/typeorm' import ContributionArgs from '@arg/ContributionArgs' @@ -39,21 +40,21 @@ export class ContributionResolver { } @Authorized([RIGHTS.LIST_CONTRIBUTIONS]) - @Query(() => [Contribution]) + @Query(() => ContributionListResult) async listContributions( @Args() { currentPage = 1, pageSize = 5, order = Order.DESC }: Paginated, @Arg('filterConfirmed', () => Boolean) filterConfirmed: boolean | null, @Ctx() context: Context, - ): Promise { + ): Promise { const user = getUser(context) const where: { userId: number confirmedBy?: FindOperator | null } = { userId: user.id } if (filterConfirmed) where.confirmedBy = IsNull() - const contributions = await dbContribution.find({ + const [contributions, count] = await dbContribution.findAndCount({ where, order: { createdAt: order, @@ -62,7 +63,10 @@ export class ContributionResolver { skip: (currentPage - 1) * pageSize, take: pageSize, }) - return contributions.map((contribution) => new Contribution(contribution, new User(user))) + return new ContributionListResult( + count, + contributions.map((contribution) => new Contribution(contribution, new User(user))), + ) } @Authorized([RIGHTS.LIST_ALL_CONTRIBUTIONS]) diff --git a/backend/src/seeds/graphql/queries.ts b/backend/src/seeds/graphql/queries.ts index deae5f97b..e18d6b303 100644 --- a/backend/src/seeds/graphql/queries.ts +++ b/backend/src/seeds/graphql/queries.ts @@ -185,9 +185,12 @@ export const listContributions = gql` order: $order filterConfirmed: $filterConfirmed ) { - id - amount - memo + linkCount + linkList { + id + amount + memo + } } } ` From 495fbfd05ce3b16b3d081ed128854d0c32e014eb Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 11:31:04 +0200 Subject: [PATCH 05/14] Change linkCount to contributionCount and linkList to contributionList. --- backend/src/graphql/model/Contribution.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/src/graphql/model/Contribution.ts b/backend/src/graphql/model/Contribution.ts index 34bffd6d7..13c2d40d7 100644 --- a/backend/src/graphql/model/Contribution.ts +++ b/backend/src/graphql/model/Contribution.ts @@ -48,13 +48,13 @@ export class Contribution { @ObjectType() export class ContributionListResult { constructor(count: number, list: Contribution[]) { - this.linkCount = count - this.linkList = list + this.contributionCount = count + this.contributionList = list } @Field(() => Int) - linkCount: number + contributionCount: number @Field(() => [Contribution]) - linkList: Contribution[] + contributionList: Contribution[] } From 3a8fb6b353454452e344ae9eef76cfeea51ed8d4 Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 11:31:39 +0200 Subject: [PATCH 06/14] Change query of listContribution to get contributionCount and contributionList instead of link... --- backend/src/seeds/graphql/queries.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/seeds/graphql/queries.ts b/backend/src/seeds/graphql/queries.ts index e18d6b303..0d72c165d 100644 --- a/backend/src/seeds/graphql/queries.ts +++ b/backend/src/seeds/graphql/queries.ts @@ -185,8 +185,8 @@ export const listContributions = gql` order: $order filterConfirmed: $filterConfirmed ) { - linkCount - linkList { + contributionCount + contributionList { id amount memo From 11dacd40be4c9fec9000253d1898ee536a3eb0c1 Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 11:34:30 +0200 Subject: [PATCH 07/14] Change query listAllContributions now gets contributionCount and contributionList instead of linkCount and linkList. --- backend/src/seeds/graphql/queries.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/seeds/graphql/queries.ts b/backend/src/seeds/graphql/queries.ts index 0d72c165d..9f7a02e70 100644 --- a/backend/src/seeds/graphql/queries.ts +++ b/backend/src/seeds/graphql/queries.ts @@ -198,8 +198,8 @@ export const listContributions = gql` export const listAllContributions = ` query ($currentPage: Int = 1, $pageSize: Int = 5, $order: Order = DESC) { listAllContributions(currentPage: $currentPage, pageSize: $pageSize, order: $order) { - linkCount - linkList { + contributionCount + contributionList { id firstName lastName From 42b8d625da34a908c5c3dd79254141f7bc5ee52f Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 11:38:58 +0200 Subject: [PATCH 08/14] Change query fields to new names. --- .../resolver/ContributionResolver.test.ts | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.test.ts b/backend/src/graphql/resolver/ContributionResolver.test.ts index e6478ffc2..37ff45f25 100644 --- a/backend/src/graphql/resolver/ContributionResolver.test.ts +++ b/backend/src/graphql/resolver/ContributionResolver.test.ts @@ -193,18 +193,21 @@ describe('ContributionResolver', () => { ).resolves.toEqual( expect.objectContaining({ data: { - listContributions: expect.arrayContaining([ - expect.objectContaining({ - id: expect.any(Number), - memo: 'Herzlich Willkommen bei Gradido!', - amount: '1000', - }), - expect.objectContaining({ - id: expect.any(Number), - memo: 'Test env contribution', - amount: '100', - }), - ]), + listContributions: { + contributionCount: 2, + contributionList: expect.arrayContaining([ + expect.objectContaining({ + id: expect.any(Number), + memo: 'Herzlich Willkommen bei Gradido!', + amount: '1000', + }), + expect.objectContaining({ + id: expect.any(Number), + memo: 'Test env contribution', + amount: '100', + }), + ]), + }, }, }), ) @@ -226,13 +229,16 @@ describe('ContributionResolver', () => { ).resolves.toEqual( expect.objectContaining({ data: { - listContributions: expect.arrayContaining([ - expect.objectContaining({ - id: expect.any(Number), - memo: 'Test env contribution', - amount: '100', - }), - ]), + listContributions: { + contributionCount: 1, + contributionList: expect.arrayContaining([ + expect.objectContaining({ + id: expect.any(Number), + memo: 'Test env contribution', + amount: '100', + }), + ]), + }, }, }), ) @@ -496,8 +502,8 @@ describe('ContributionResolver', () => { expect.objectContaining({ data: { listAllContributions: { - linkCount: 2, - linkList: expect.arrayContaining([ + contributionCount: 2, + contributionList: expect.arrayContaining([ expect.objectContaining({ id: expect.any(Number), memo: 'Herzlich Willkommen bei Gradido!', From ae04024dd38b56ed5e702fc7a9e37d183cf76066 Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 11:39:17 +0200 Subject: [PATCH 09/14] Remove unused import. --- backend/src/graphql/resolver/ContributionResolver.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index c834904d6..f21c65068 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -2,7 +2,6 @@ import { RIGHTS } from '@/auth/RIGHTS' import { Context, getUser } from '@/server/context' import { backendLogger as logger } from '@/server/logger' import { Contribution as dbContribution } from '@entity/Contribution' -import { User as dbUser } from '@entity/User' import { Arg, Args, Authorized, Ctx, Int, Mutation, Query, Resolver } from 'type-graphql' import { FindOperator, IsNull } from '@dbTools/typeorm' import ContributionArgs from '@arg/ContributionArgs' From da3574edb81aa93a362aa61a69ff5cbd459fe5ba Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 12:35:42 +0200 Subject: [PATCH 10/14] Fix merge conflict and reviewed name changes. --- .../graphql/resolver/ContributionResolver.ts | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index 20eeaa199..90ec6b5a6 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -1,13 +1,6 @@ import { RIGHTS } from '@/auth/RIGHTS' import { Context, getUser } from '@/server/context' import { backendLogger as logger } from '@/server/logger' -<<<<<<< HEAD -import { Contribution } from '@entity/Contribution' -import { Arg, Args, Authorized, Ctx, Int, Mutation, Resolver } from 'type-graphql' -import ContributionArgs from '../arg/ContributionArgs' -import { UnconfirmedContribution } from '../model/UnconfirmedContribution' -import { validateContribution, getUserCreation } from './util/creations' -======= import { Contribution as dbContribution } from '@entity/Contribution' import { Arg, Args, Authorized, Ctx, Int, Mutation, Query, Resolver } from 'type-graphql' import { FindOperator, IsNull } from '@dbTools/typeorm' @@ -18,7 +11,6 @@ import { Contribution, ContributionListResult } from '@model/Contribution' import { UnconfirmedContribution } from '@model/UnconfirmedContribution' import { User } from '@model/User' import { validateContribution, getUserCreation, updateCreations } from './util/creations' ->>>>>>> master @Resolver() export class ContributionResolver { @@ -46,15 +38,14 @@ export class ContributionResolver { return new UnconfirmedContribution(contribution, user, creations) } -<<<<<<< HEAD @Authorized([RIGHTS.DELETE_CONTRIBUTION]) @Mutation(() => Boolean) - async adminDeleteContribution( + async deleteContribution( @Arg('id', () => Int) id: number, @Ctx() context: Context, ): Promise { const user = getUser(context) - const contribution = await Contribution.findOne(id) + const contribution = await dbContribution.findOne(id) if (!contribution) { throw new Error('Contribution not found for given id.') } @@ -63,7 +54,8 @@ export class ContributionResolver { } const res = await contribution.softRemove() return !!res -======= + } + @Authorized([RIGHTS.LIST_CONTRIBUTIONS]) @Query(() => [Contribution]) async listContributions( @@ -147,6 +139,5 @@ export class ContributionResolver { dbContribution.save(contributionToUpdate) return new UnconfirmedContribution(contributionToUpdate, user, creations) ->>>>>>> master } } From 4c33d3784b4dfcbe13ad0d2054c0e3d3863f904d Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 13:07:44 +0200 Subject: [PATCH 11/14] Added mutation for deleteContribution --- backend/src/seeds/graphql/mutations.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/backend/src/seeds/graphql/mutations.ts b/backend/src/seeds/graphql/mutations.ts index 4e7fa8a90..bf898bd7d 100644 --- a/backend/src/seeds/graphql/mutations.ts +++ b/backend/src/seeds/graphql/mutations.ts @@ -255,3 +255,9 @@ export const updateContribution = gql` } } ` + +export const deleteContribution = gql` + mutation ($id: Int!) { + deleteContribution(id: $id) + } +` From e84061c412a9ba5ecb3b7f0c35dff2ba8702c35d Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 13:08:27 +0200 Subject: [PATCH 12/14] Added tests for deleteContribution --- .../resolver/ContributionResolver.test.ts | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/backend/src/graphql/resolver/ContributionResolver.test.ts b/backend/src/graphql/resolver/ContributionResolver.test.ts index e6478ffc2..37cb2c645 100644 --- a/backend/src/graphql/resolver/ContributionResolver.test.ts +++ b/backend/src/graphql/resolver/ContributionResolver.test.ts @@ -5,6 +5,7 @@ import { bibiBloxberg } from '@/seeds/users/bibi-bloxberg' import { adminUpdateContribution, createContribution, + deleteContribution, updateContribution, } from '@/seeds/graphql/mutations' import { listAllContributions, listContributions, login } from '@/seeds/graphql/queries' @@ -481,6 +482,11 @@ describe('ContributionResolver', () => { }) }) + afterAll(async () => { + await cleanDB() + resetToken() + }) + it('returns allCreation', async () => { await expect( query({ @@ -516,4 +522,98 @@ describe('ContributionResolver', () => { }) }) }) + + describe('deleteContribution', () => { + describe('unauthenticated', () => { + it('returns an error', async () => { + await expect( + query({ + query: deleteContribution, + variables: { + id: -1, + }, + }), + ).resolves.toEqual( + expect.objectContaining({ + errors: [new GraphQLError('401 Unauthorized')], + }), + ) + }) + }) + + describe('authenticated', () => { + beforeAll(async () => { + await userFactory(testEnv, bibiBloxberg) + await userFactory(testEnv, peterLustig) + await query({ + query: login, + variables: { email: 'bibi@bloxberg.de', password: 'Aa12345_' }, + }) + result = await mutate({ + mutation: createContribution, + variables: { + amount: 100.0, + memo: 'Test env contribution', + creationDate: new Date().toString(), + }, + }) + }) + + afterAll(async () => { + await cleanDB() + resetToken() + }) + + describe('wrong contribution id', () => { + it('returns an error', async () => { + await expect( + query({ + query: deleteContribution, + variables: { + id: -1, + }, + }), + ).resolves.toEqual( + expect.objectContaining({ + errors: [new GraphQLError('Contribution not found for given id.')], + }), + ) + }) + }) + + describe('other user sends a deleteContribtuion', () => { + it('returns an error', async () => { + await query({ + query: login, + variables: { email: 'peter@lustig.de', password: 'Aa12345_' }, + }) + await expect( + query({ + query: deleteContribution, + variables: { + id: result.data.createContribution.id, + }, + }), + ).resolves.toEqual( + expect.objectContaining({ + errors: [new GraphQLError('Can not delete contribution of another user')], + }), + ) + }) + }) + + describe('User deletes own contribution', () => { + it('deletes successfully', async () => { + await expect( + query({ + query: deleteContribution, + variables: { + id: result.data.createContribution.id, + }, + }), + ).resolves.toBeTruthy() + }) + }) + }) + }) }) From b4a4a9e12699ce49ba77bafe927a0f460b874d09 Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 19 Jul 2022 13:09:03 +0200 Subject: [PATCH 13/14] Throw error if confirmedAt is present. --- backend/src/graphql/resolver/ContributionResolver.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index 8ee994989..ef4467a71 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -52,6 +52,9 @@ export class ContributionResolver { if (contribution.userId !== user.id) { throw new Error('Can not delete contribution of another user') } + if (contribution.confirmedAt) { + throw new Error('A confirmed contribution can not be deleted') + } const res = await contribution.softRemove() return !!res } From ccc16b4083acb5c2607af5e20b7dd58c92d27bd7 Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 19 Jul 2022 13:09:44 +0200 Subject: [PATCH 14/14] Test that an already confirmed contribution can not be deleted. --- .../resolver/ContributionResolver.test.ts | 44 ++++++++++++++++--- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.test.ts b/backend/src/graphql/resolver/ContributionResolver.test.ts index 0fcd0f5af..b584624c2 100644 --- a/backend/src/graphql/resolver/ContributionResolver.test.ts +++ b/backend/src/graphql/resolver/ContributionResolver.test.ts @@ -4,6 +4,7 @@ import { bibiBloxberg } from '@/seeds/users/bibi-bloxberg' import { adminUpdateContribution, + confirmContribution, createContribution, deleteContribution, updateContribution, @@ -573,8 +574,8 @@ describe('ContributionResolver', () => { describe('wrong contribution id', () => { it('returns an error', async () => { await expect( - query({ - query: deleteContribution, + mutate({ + mutation: deleteContribution, variables: { id: -1, }, @@ -594,8 +595,8 @@ describe('ContributionResolver', () => { variables: { email: 'peter@lustig.de', password: 'Aa12345_' }, }) await expect( - query({ - query: deleteContribution, + mutate({ + mutation: deleteContribution, variables: { id: result.data.createContribution.id, }, @@ -611,8 +612,8 @@ describe('ContributionResolver', () => { describe('User deletes own contribution', () => { it('deletes successfully', async () => { await expect( - query({ - query: deleteContribution, + mutate({ + mutation: deleteContribution, variables: { id: result.data.createContribution.id, }, @@ -620,6 +621,37 @@ describe('ContributionResolver', () => { ).resolves.toBeTruthy() }) }) + + describe('User deletes already confirmed contribution', () => { + it('throws an error', async () => { + await query({ + query: login, + variables: { email: 'peter@lustig.de', password: 'Aa12345_' }, + }) + await mutate({ + mutation: confirmContribution, + variables: { + id: result.data.createContribution.id, + }, + }) + await query({ + query: login, + variables: { email: 'bibi@bloxberg.de', password: 'Aa12345_' }, + }) + await expect( + mutate({ + mutation: deleteContribution, + variables: { + id: result.data.createContribution.id, + }, + }), + ).resolves.toEqual( + expect.objectContaining({ + errors: [new GraphQLError('A confirmed contribution can not be deleted')], + }), + ) + }) + }) }) }) })