From 9f4f1d07ca4d5e7fba33e65ad5859bda9e26ed5b Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 5 Apr 2024 15:50:01 +0800 Subject: [PATCH 1/4] do not create label if label name case-insensitively matches existing label name --- packages/api/src/resolvers/labels/index.ts | 42 +++++++------- packages/api/test/resolvers/labels.test.ts | 67 +++++++++++----------- 2 files changed, 54 insertions(+), 55 deletions(-) diff --git a/packages/api/src/resolvers/labels/index.ts b/packages/api/src/resolvers/labels/index.ts index 2762865cb..1da2f3053 100644 --- a/packages/api/src/resolvers/labels/index.ts +++ b/packages/api/src/resolvers/labels/index.ts @@ -89,30 +89,30 @@ export const createLabelResolver = authorized< CreateLabelSuccess, CreateLabelError, MutationCreateLabelArgs ->(async (_, { input }, { authTrx, log, uid }) => { - try { - const label = await authTrx(async (tx) => { - return tx.withRepository(labelRepository).createLabel(input, uid) - }) - - analytics.capture({ - distinctId: uid, - event: 'label_created', - properties: { - ...input, - env: env.server.apiEnv, - }, - }) - +>(async (_, { input }, { authTrx, uid }) => { + const existingLabel = await labelRepository.findByName(input.name) + if (existingLabel) { return { - label, - } - } catch (error) { - log.error('createLabelResolver', error) - return { - errorCodes: [CreateLabelErrorCode.BadRequest], + errorCodes: [CreateLabelErrorCode.LabelAlreadyExists], } } + + const label = await authTrx(async (tx) => + tx.withRepository(labelRepository).createLabel(input, uid) + ) + + analytics.capture({ + distinctId: uid, + event: 'label_created', + properties: { + ...input, + env: env.server.apiEnv, + }, + }) + + return { + label, + } }) export const deleteLabelResolver = authorized< diff --git a/packages/api/test/resolvers/labels.test.ts b/packages/api/test/resolvers/labels.test.ts index 22f8cdf4c..0a7608c70 100644 --- a/packages/api/test/resolvers/labels.test.ts +++ b/packages/api/test/resolvers/labels.test.ts @@ -35,7 +35,7 @@ describe('Labels API', () => { const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) - authToken = res.body.authToken + authToken = res.body.authToken as string }) after(async () => { @@ -109,18 +109,9 @@ describe('Labels API', () => { }) describe('Create label', () => { - let query: string - let name: string - - beforeEach(() => { - query = ` - mutation { - createLabel( - input: { - color: "#ffffff" - name: "${name}" - } - ) { + const query = ` + mutation CreateLabel($input: CreateLabelInput!) { + createLabel(input: $input) { ... on CreateLabelSuccess { label { id @@ -133,12 +124,9 @@ describe('Labels API', () => { } } ` - }) context('when name not exists', () => { - before(() => { - name = 'label3' - }) + const name = 'label3' after(async () => { // clean up @@ -146,7 +134,9 @@ describe('Labels API', () => { }) it('should create label', async () => { - const res = await graphqlRequest(query, authToken).expect(200) + const res = await graphqlRequest(query, authToken, { + input: { name }, + }).expect(200) const label = await findLabelById( res.body.data.createLabel.label.id, user.id @@ -160,17 +150,30 @@ describe('Labels API', () => { before(async () => { existingLabel = await createLabel('label3', '#ffffff', user.id) - name = existingLabel.name }) after(async () => { await deleteLabels({ id: existingLabel.id }, user.id) }) - it('should return error code BAD_REQUEST', async () => { - const res = await graphqlRequest(query, authToken).expect(200) + it('should return error code LABEL_ALREADY_EXISTS', async () => { + const res = await graphqlRequest(query, authToken, { + input: { name: existingLabel.name }, + }).expect(200) - expect(res.body.data.createLabel.errorCodes).to.eql(['BAD_REQUEST']) + expect(res.body.data.createLabel.errorCodes).to.eql([ + 'LABEL_ALREADY_EXISTS', + ]) + }) + + it('returns error code LABEL_ALREADY_EXISTS if case-insensitive label name exists', async () => { + const name = existingLabel.name.toUpperCase() + const res = await graphqlRequest(query, authToken, { + input: { name }, + }).expect(200) + expect(res.body.data.createLabel.errorCodes).to.eql([ + 'LABEL_ALREADY_EXISTS', + ]) }) }) @@ -334,10 +337,7 @@ describe('Labels API', () => { after(async () => { // clean up - await deleteLabels( - { user: { id: user.id } }, - user.id - ) + await deleteLabels({ user: { id: user.id } }, user.id) await deleteLibraryItemById(item.id) }) @@ -464,7 +464,7 @@ describe('Labels API', () => { }) after(async () => { - await deleteLabels( { id: toUpdateLabel.id }, user.id) + await deleteLabels({ id: toUpdateLabel.id }, user.id) }) it('should return the updated label', async () => { @@ -538,10 +538,7 @@ describe('Labels API', () => { after(async () => { // clean up - await deleteLabels( - { user: { id: user.id } }, - user.id - ) + await deleteLabels({ user: { id: user.id } }, user.id) await deleteLibraryItemById(item.id) }) @@ -588,7 +585,9 @@ describe('Labels API', () => { it('should set labels for highlight', async () => { const res = await graphqlRequest(query, authToken).expect(200) expect( - res.body.data.setLabelsForHighlight.labels.map((l: any) => l.id) + (res.body.data.setLabelsForHighlight.labels as Label[]).map( + (l) => l.id + ) ).to.eql(labelIds) }) }) @@ -652,7 +651,7 @@ describe('Labels API', () => { ` let labelId: string let afterLabelId: string - let labels: Label[] = [] + const labels: Label[] = [] before(async () => { // create testing labels @@ -671,7 +670,7 @@ describe('Labels API', () => { }) context('when label exists', () => { - before(async () => { + before(() => { labelId = labels[1].id afterLabelId = labels[4].id }) From 6f03b214b5e04ce61dcab8855212e48f90a8bbf9 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 5 Apr 2024 15:58:39 +0800 Subject: [PATCH 2/4] allow delete internal labels --- packages/api/src/repository/label.ts | 3 +- packages/api/test/resolvers/labels.test.ts | 41 ++++++++++++++++------ 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/packages/api/src/repository/label.ts b/packages/api/src/repository/label.ts index 9105f96eb..baaac24bb 100644 --- a/packages/api/src/repository/label.ts +++ b/packages/api/src/repository/label.ts @@ -82,7 +82,8 @@ export const labelRepository = appDataSource.getRepository(Label).extend({ }, deleteById(id: string) { - return this.delete({ id, internal: false }) + // internal labels can be deleted but it will be recreated when next feed/newsletter saved + return this.delete({ id }) }, updateLabel(id: string, label: QueryDeepPartialEntity