From ee8a338ec4e927932a64d233364f06e2dd37592f Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Thu, 2 Feb 2023 23:49:35 +0100 Subject: [PATCH 1/2] fix logger on contributionLinkResolver --- .../resolver/ContributionLinkResolver.test.ts | 72 ++++++++----------- .../resolver/ContributionLinkResolver.ts | 42 +++++------ 2 files changed, 46 insertions(+), 68 deletions(-) diff --git a/backend/src/graphql/resolver/ContributionLinkResolver.test.ts b/backend/src/graphql/resolver/ContributionLinkResolver.test.ts index 0cf27bf33..46296e009 100644 --- a/backend/src/graphql/resolver/ContributionLinkResolver.test.ts +++ b/backend/src/graphql/resolver/ContributionLinkResolver.test.ts @@ -246,6 +246,7 @@ describe('Contribution Links', () => { }) it('returns an error if missing startDate', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -270,6 +271,7 @@ describe('Contribution Links', () => { }) it('returns an error if missing endDate', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -292,6 +294,7 @@ describe('Contribution Links', () => { }) it('returns an error if endDate is before startDate', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -317,6 +320,7 @@ describe('Contribution Links', () => { }) it('returns an error if name is an empty string', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -327,16 +331,17 @@ describe('Contribution Links', () => { }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('The name must be initialized!')], + errors: [new GraphQLError('The name must be initialized')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('The name must be initialized!') + expect(logger.error).toBeCalledWith('The name must be initialized') }) it('returns an error if name is shorter than 5 characters', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -347,22 +352,17 @@ describe('Contribution Links', () => { }), ).resolves.toEqual( expect.objectContaining({ - errors: [ - new GraphQLError( - `The value of 'name' with a length of 3 did not fulfill the requested bounderies min=5 and max=100`, - ), - ], + errors: [new GraphQLError('The value of name is too short')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith( - `The value of 'name' with a length of 3 did not fulfill the requested bounderies min=5 and max=100`, - ) + expect(logger.error).toBeCalledWith('The value of name is too short', 3) }) it('returns an error if name is longer than 100 characters', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -373,22 +373,17 @@ describe('Contribution Links', () => { }), ).resolves.toEqual( expect.objectContaining({ - errors: [ - new GraphQLError( - `The value of 'name' with a length of 101 did not fulfill the requested bounderies min=5 and max=100`, - ), - ], + errors: [new GraphQLError('The value of name is too long')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith( - `The value of 'name' with a length of 101 did not fulfill the requested bounderies min=5 and max=100`, - ) + expect(logger.error).toBeCalledWith('The value of name is too long', 101) }) it('returns an error if memo is an empty string', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -399,16 +394,17 @@ describe('Contribution Links', () => { }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('The memo must be initialized!')], + errors: [new GraphQLError('The memo must be initialized')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('The memo must be initialized!') + expect(logger.error).toBeCalledWith('The memo must be initialized') }) it('returns an error if memo is shorter than 5 characters', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -419,22 +415,17 @@ describe('Contribution Links', () => { }), ).resolves.toEqual( expect.objectContaining({ - errors: [ - new GraphQLError( - `The value of 'memo' with a length of 3 did not fulfill the requested bounderies min=5 and max=255`, - ), - ], + errors: [new GraphQLError('The value of memo is too short')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith( - `The value of 'memo' with a length of 3 did not fulfill the requested bounderies min=5 and max=255`, - ) + expect(logger.error).toBeCalledWith('The value of memo is too short', 3) }) it('returns an error if memo is longer than 255 characters', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -445,22 +436,17 @@ describe('Contribution Links', () => { }), ).resolves.toEqual( expect.objectContaining({ - errors: [ - new GraphQLError( - `The value of 'memo' with a length of 256 did not fulfill the requested bounderies min=5 and max=255`, - ), - ], + errors: [new GraphQLError('The value of memo is too long')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith( - `The value of 'memo' with a length of 256 did not fulfill the requested bounderies min=5 and max=255`, - ) + expect(logger.error).toBeCalledWith('The value of memo is too long', 256) }) it('returns an error if amount is not positive', async () => { + jest.clearAllMocks() await expect( mutate({ mutation: createContributionLink, @@ -471,15 +457,13 @@ describe('Contribution Links', () => { }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('The amount=0 must be initialized with a positiv value!')], + errors: [new GraphQLError('The amount must be a positiv value')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith( - 'The amount=0 must be initialized with a positiv value!', - ) + expect(logger.error).toBeCalledWith('The amount must be a positiv value', new Decimal(0)) }) }) @@ -530,14 +514,14 @@ describe('Contribution Links', () => { }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('Contribution Link not found to given id.')], + errors: [new GraphQLError('Contribution Link not found')], }), ) }) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('Contribution Link not found to given id: -1') + expect(logger.error).toBeCalledWith('Contribution Link not found', -1) }) describe('valid id', () => { @@ -601,13 +585,13 @@ describe('Contribution Links', () => { mutate({ mutation: deleteContributionLink, variables: { id: -1 } }), ).resolves.toEqual( expect.objectContaining({ - errors: [new GraphQLError('Contribution Link not found to given id.')], + errors: [new GraphQLError('Contribution Link not found')], }), ) }) it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('Contribution Link not found to given id: -1') + expect(logger.error).toBeCalledWith('Contribution Link not found', -1) }) }) diff --git a/backend/src/graphql/resolver/ContributionLinkResolver.ts b/backend/src/graphql/resolver/ContributionLinkResolver.ts index 0a6bb971c..6a7a71391 100644 --- a/backend/src/graphql/resolver/ContributionLinkResolver.ts +++ b/backend/src/graphql/resolver/ContributionLinkResolver.ts @@ -20,6 +20,7 @@ import Paginated from '@arg/Paginated' // TODO: this is a strange construct import { transactionLinkCode as contributionLinkCode } from './TransactionLinkResolver' +import LogError from '@/server/LogError' @Resolver() export class ContributionLinkResolver { @@ -39,35 +40,30 @@ export class ContributionLinkResolver { }: ContributionLinkArgs, ): Promise { isStartEndDateValid(validFrom, validTo) + // TODO: this should be enforced by the schema. if (!name) { - logger.error(`The name must be initialized!`) - throw new Error(`The name must be initialized!`) + throw new LogError('The name must be initialized') } - if ( - name.length < CONTRIBUTIONLINK_NAME_MIN_CHARS || - name.length > CONTRIBUTIONLINK_NAME_MAX_CHARS - ) { - const msg = `The value of 'name' with a length of ${name.length} did not fulfill the requested bounderies min=${CONTRIBUTIONLINK_NAME_MIN_CHARS} and max=${CONTRIBUTIONLINK_NAME_MAX_CHARS}` - logger.error(`${msg}`) - throw new Error(`${msg}`) + if (name.length < CONTRIBUTIONLINK_NAME_MIN_CHARS) { + throw new LogError('The value of name is too short', name.length) } + if (name.length > CONTRIBUTIONLINK_NAME_MAX_CHARS) { + throw new LogError('The value of name is too long', name.length) + } + // TODO: this should be enforced by the schema. if (!memo) { - logger.error(`The memo must be initialized!`) - throw new Error(`The memo must be initialized!`) + throw new LogError('The memo must be initialized') } - if (memo.length < MEMO_MIN_CHARS || memo.length > MEMO_MAX_CHARS) { - const msg = `The value of 'memo' with a length of ${memo.length} did not fulfill the requested bounderies min=${MEMO_MIN_CHARS} and max=${MEMO_MAX_CHARS}` - logger.error(`${msg}`) - throw new Error(`${msg}`) + if (memo.length < MEMO_MIN_CHARS) { + throw new LogError('The value of memo is too short', memo.length) } - if (!amount) { - logger.error(`The amount must be initialized!`) - throw new Error('The amount must be initialized!') + if (memo.length > MEMO_MAX_CHARS) { + throw new LogError('The value of memo is too long', memo.length) } if (!new Decimal(amount).isPositive()) { - logger.error(`The amount=${amount} must be initialized with a positiv value!`) - throw new Error(`The amount=${amount} must be initialized with a positiv value!`) + throw new LogError('The amount must be a positiv value', amount) } + const dbContributionLink = new DbContributionLink() dbContributionLink.amount = amount dbContributionLink.name = name @@ -107,8 +103,7 @@ export class ContributionLinkResolver { async deleteContributionLink(@Arg('id', () => Int) id: number): Promise { const contributionLink = await DbContributionLink.findOne(id) if (!contributionLink) { - logger.error(`Contribution Link not found to given id: ${id}`) - throw new Error('Contribution Link not found to given id.') + throw new LogError('Contribution Link not found', id) } await contributionLink.softRemove() logger.debug(`deleteContributionLink successful!`) @@ -134,8 +129,7 @@ export class ContributionLinkResolver { ): Promise { const dbContributionLink = await DbContributionLink.findOne(id) if (!dbContributionLink) { - logger.error(`Contribution Link not found to given id: ${id}`) - throw new Error('Contribution Link not found to given id.') + throw new LogError('Contribution Link not found', id) } dbContributionLink.amount = amount dbContributionLink.name = name From a04d4d39855b42200b201081d9fc4ffe027d1f03 Mon Sep 17 00:00:00 2001 From: Ulf Gebhardt Date: Tue, 7 Feb 2023 11:01:35 +0100 Subject: [PATCH 2/2] remove empty string check since we do a length check on memeo & name --- .../resolver/ContributionLinkResolver.test.ts | 42 ------------------- .../resolver/ContributionLinkResolver.ts | 8 ---- 2 files changed, 50 deletions(-) diff --git a/backend/src/graphql/resolver/ContributionLinkResolver.test.ts b/backend/src/graphql/resolver/ContributionLinkResolver.test.ts index 46296e009..49bca2c42 100644 --- a/backend/src/graphql/resolver/ContributionLinkResolver.test.ts +++ b/backend/src/graphql/resolver/ContributionLinkResolver.test.ts @@ -319,27 +319,6 @@ describe('Contribution Links', () => { ) }) - it('returns an error if name is an empty string', async () => { - jest.clearAllMocks() - await expect( - mutate({ - mutation: createContributionLink, - variables: { - ...variables, - name: '', - }, - }), - ).resolves.toEqual( - expect.objectContaining({ - errors: [new GraphQLError('The name must be initialized')], - }), - ) - }) - - it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('The name must be initialized') - }) - it('returns an error if name is shorter than 5 characters', async () => { jest.clearAllMocks() await expect( @@ -382,27 +361,6 @@ describe('Contribution Links', () => { expect(logger.error).toBeCalledWith('The value of name is too long', 101) }) - it('returns an error if memo is an empty string', async () => { - jest.clearAllMocks() - await expect( - mutate({ - mutation: createContributionLink, - variables: { - ...variables, - memo: '', - }, - }), - ).resolves.toEqual( - expect.objectContaining({ - errors: [new GraphQLError('The memo must be initialized')], - }), - ) - }) - - it('logs the error thrown', () => { - expect(logger.error).toBeCalledWith('The memo must be initialized') - }) - it('returns an error if memo is shorter than 5 characters', async () => { jest.clearAllMocks() await expect( diff --git a/backend/src/graphql/resolver/ContributionLinkResolver.ts b/backend/src/graphql/resolver/ContributionLinkResolver.ts index 6a7a71391..39f202848 100644 --- a/backend/src/graphql/resolver/ContributionLinkResolver.ts +++ b/backend/src/graphql/resolver/ContributionLinkResolver.ts @@ -40,20 +40,12 @@ export class ContributionLinkResolver { }: ContributionLinkArgs, ): Promise { isStartEndDateValid(validFrom, validTo) - // TODO: this should be enforced by the schema. - if (!name) { - throw new LogError('The name must be initialized') - } if (name.length < CONTRIBUTIONLINK_NAME_MIN_CHARS) { throw new LogError('The value of name is too short', name.length) } if (name.length > CONTRIBUTIONLINK_NAME_MAX_CHARS) { throw new LogError('The value of name is too long', name.length) } - // TODO: this should be enforced by the schema. - if (!memo) { - throw new LogError('The memo must be initialized') - } if (memo.length < MEMO_MIN_CHARS) { throw new LogError('The value of memo is too short', memo.length) }