diff --git a/packages/api/src/elastic/labels.ts b/packages/api/src/elastic/labels.ts index 90c930ac2..4741fd9b0 100644 --- a/packages/api/src/elastic/labels.ts +++ b/packages/api/src/elastic/labels.ts @@ -136,9 +136,8 @@ export const deleteLabelInPages = async ( refresh: ctx.refresh, }) - if (body.updated === 0) return false - - await ctx.pubsub.entityDeleted(EntityType.LABEL, label, ctx.uid) + body.updated > 0 && + (await ctx.pubsub.entityDeleted(EntityType.LABEL, label, ctx.uid)) return true } catch (e) { @@ -146,3 +145,56 @@ export const deleteLabelInPages = async ( return false } } + +export const updateLabelInPage = async ( + label: Label, + ctx: PageContext +): Promise => { + try { + const { body } = await client.updateByQuery({ + index: INDEX_ALIAS, + body: { + script: { + source: `ctx._source.labels.removeIf(l -> l.id == params.label.id); + ctx._source.labels.add(params.label)`, + lang: 'painless', + params: { + label: label, + }, + }, + query: { + bool: { + filter: [ + { + term: { + userId: ctx.uid, + }, + }, + { + nested: { + path: 'labels', + query: { + term: { + 'labels.id': label.id, + }, + }, + }, + }, + ], + }, + }, + }, + refresh: ctx.refresh, + conflicts: 'proceed', // ignore conflicts + }) + + body.updated > 0 && + (await ctx.pubsub.entityUpdated(EntityType.LABEL, label, ctx.uid)) + + return true + } catch (e) { + console.error('failed to update label in elastic', e) + + return false + } +} diff --git a/packages/api/src/resolvers/labels/index.ts b/packages/api/src/resolvers/labels/index.ts index b95d46521..54c0b3263 100644 --- a/packages/api/src/resolvers/labels/index.ts +++ b/packages/api/src/resolvers/labels/index.ts @@ -29,7 +29,11 @@ import { getRepository, setClaims } from '../../entity/utils' import { createPubSubClient } from '../../datalayer/pubsub' import { AppDataSource } from '../../server' import { getPageById } from '../../elastic/pages' -import { deleteLabelInPages, updateLabelsInPage } from '../../elastic/labels' +import { + deleteLabelInPages, + updateLabelInPage, + updateLabelsInPage, +} from '../../elastic/labels' export const labelsResolver = authorized( async (_obj, _params, { claims: { uid }, log }) => { @@ -222,7 +226,7 @@ export const setLabelsResolver = authorized< const labels = await getRepository(Label).find({ where: { id: In(labelIds), user: { id: user.id } }, - relations: ['user'], + select: ['id', 'name', 'color', 'description', 'createdAt'], }) if (labels.length !== labelIds.length) { return { @@ -266,7 +270,7 @@ export const updateLabelResolver = authorized< UpdateLabelSuccess, UpdateLabelError, MutationUpdateLabelArgs ->(async (_, { input }, { claims: { uid }, log }) => { +>(async (_, { input }, { claims: { uid }, log, pubsub }) => { log.info('updateLabelResolver') try { @@ -278,9 +282,9 @@ export const updateLabelResolver = authorized< } } - const label = await getRepository(Label).findOneBy({ - id: labelId, - user: { id: uid }, + const label = await getRepository(Label).findOne({ + where: { id: labelId, user: { id: uid } }, + select: ['id', 'name', 'color', 'description', 'createdAt'], }) if (!label) { return { @@ -308,7 +312,17 @@ export const updateLabelResolver = authorized< if (!result.affected) { log.error('failed to update') return { - errorCodes: [UpdateLabelErrorCode.NotFound], + errorCodes: [UpdateLabelErrorCode.BadRequest], + } + } + + const updated = await updateLabelInPage(label, { + pubsub, + uid, + }) + if (!updated) { + return { + errorCodes: [UpdateLabelErrorCode.BadRequest], } } diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts index 7b908dd7f..bf9577b74 100644 --- a/packages/api/src/services/labels.ts +++ b/packages/api/src/services/labels.ts @@ -54,5 +54,15 @@ export const addLabelToPage = async ( console.log('adding label to page', label.name, pageId) - return addLabelInPage(pageId, labelEntity, ctx) + return addLabelInPage( + pageId, + { + id: labelEntity.id, + name: labelEntity.name, + color: labelEntity.color, + description: labelEntity.description, + createdAt: labelEntity.createdAt, + }, + ctx + ) } diff --git a/packages/api/test/resolvers/labels.test.ts b/packages/api/test/resolvers/labels.test.ts index 83640a32b..6d7814fc7 100644 --- a/packages/api/test/resolvers/labels.test.ts +++ b/packages/api/test/resolvers/labels.test.ts @@ -9,9 +9,11 @@ import { Label } from '../../src/entity/label' import { expect } from 'chai' import 'mocha' import { User } from '../../src/entity/user' -import { Page } from '../../src/elastic/types' +import { Page, PageContext } from '../../src/elastic/types' import { getRepository } from '../../src/entity/utils' import { getPageById } from '../../src/elastic/pages' +import { addLabelInPage } from '../../src/elastic/labels' +import { createPubSubClient } from '../../src/datalayer/pubsub' describe('Labels API', () => { const username = 'fakeUser' @@ -20,6 +22,7 @@ describe('Labels API', () => { let authToken: string let page: Page let labels: Label[] + let ctx: PageContext before(async () => { // create test user and login @@ -41,7 +44,19 @@ describe('Labels API', () => { 'different_label', '#dddddd' ) - page = await createTestElasticPage(user, [existingLabelOfLink]) + page = await createTestElasticPage(user, [ + { + id: existingLabelOfLink.id, + name: existingLabelOfLink.name, + color: existingLabelOfLink.color, + }, + ]) + + ctx = { + pubsub: createPubSubClient(), + refresh: true, + uid: user.id, + } }) after(async () => { @@ -198,8 +213,10 @@ describe('Labels API', () => { }) context('when label exists', () => { + let toDeleteLabel: Label + before(async () => { - const toDeleteLabel = await createTestLabel(user, 'label4', '#ffffff') + toDeleteLabel = await createTestLabel(user, 'label4', '#ffffff') labelId = toDeleteLabel.id }) @@ -208,6 +225,19 @@ describe('Labels API', () => { const label = await getRepository(Label).findOneBy({ id: labelId }) expect(label).to.not.exist }) + + context('when a page has this label', () => { + before(async () => { + await addLabelInPage(page.id, toDeleteLabel, ctx) + }) + + it('should update page', async () => { + await graphqlRequest(query, authToken).expect(200) + const updatedPage = await getPageById(page.id) + + expect(updatedPage?.labels).not.to.include(toDeleteLabel) + }) + }) }) context('when label not exist', () => { @@ -293,7 +323,7 @@ describe('Labels API', () => { }) }) - context('when link not exist', () => { + context('when page not exist', () => { before(() => { pageId = generateFakeUuid() labelIds = [labels[0].id, labels[1].id] @@ -319,4 +349,95 @@ describe('Labels API', () => { return graphqlRequest(query, invalidAuthToken).expect(500) }) }) + + describe('Update label', () => { + let query: string + let labelId: string + let name: string + let color: string + + beforeEach(() => { + query = ` + mutation { + updateLabel( + input: { + labelId: "${labelId}", + name: "${name}", + color: "${color}" + } + ) { + ... on UpdateLabelSuccess { + label { + id + name + color + } + } + ... on UpdateLabelError { + errorCodes + } + } + } + ` + }) + + context('when labels exists', () => { + let toUpdateLabel: Label + + before(async () => { + toUpdateLabel = await createTestLabel(user, 'label5', '#ffffff') + labelId = toUpdateLabel.id + name = 'Updated label' + color = '#aabbcc' + }) + + it('should return the updated label', async () => { + const res = await graphqlRequest(query, authToken).expect(200) + expect(res.body.data.updateLabel.label).to.eql({ + id: labelId, + name, + color, + }) + }) + + it('should update the label in db', async () => { + await graphqlRequest(query, authToken).expect(200) + const updatedLabel = await getRepository(Label).findOne({ + where: { id: labelId }, + }) + + expect(updatedLabel?.name).to.eql(name) + expect(updatedLabel?.color).to.eql(color) + }) + + context('when a page has the label', () => { + before(async () => { + await addLabelInPage(page.id, toUpdateLabel, ctx) + }) + + it('should update the page with the label', async () => { + await graphqlRequest(query, authToken).expect(200) + + const updatedPage = await getPageById(page.id) + const updatedLabel = updatedPage?.labels?.filter( + (l) => l.id === labelId + )?.[0] + + expect(updatedLabel?.name).to.eql(name) + expect(updatedLabel?.color).to.eql(color) + }) + }) + }) + + context('when labels not exist', () => { + before(() => { + labelId = generateFakeUuid() + }) + + it('should return error code NOT_FOUND', async () => { + const res = await graphqlRequest(query, authToken).expect(200) + expect(res.body.data.updateLabel.errorCodes).to.eql(['NOT_FOUND']) + }) + }) + }) }) diff --git a/packages/api/test/services/save_newsletter_email.test.ts b/packages/api/test/services/save_newsletter_email.test.ts index b655d196e..df0ec6445 100644 --- a/packages/api/test/services/save_newsletter_email.test.ts +++ b/packages/api/test/services/save_newsletter_email.test.ts @@ -44,16 +44,14 @@ describe('saveNewsletterEmail', () => { ctx ) - setTimeout(async () => { - const page = await getPageByParam({ userId: user.id }) - if (!page) { - expect.fail('page not found') - } - expect(page.url).to.equal('https://example.com') - expect(page.title).to.equal('fake title') - expect(page.author).to.equal('fake author') - expect(page.content).to.contain(fakeContent) - }) + const page = await getPageByParam({ userId: user.id }) + if (!page) { + expect.fail('page not found') + } + expect(page.url).to.equal('https://example.com') + expect(page.title).to.equal('fake title') + expect(page.author).to.equal('fake author') + expect(page.content).to.contain(fakeContent) }) it('should adds a Newsletter label to that page', async () => { @@ -73,9 +71,7 @@ describe('saveNewsletterEmail', () => { ctx ) - setTimeout(async () => { - const page = await getPageByParam({ userId: user.id }) - expect(page?.labels).to.deep.include(newLabel) - }) + const page = await getPageByParam({ userId: user.id }) + expect(page?.labels?.[0]).to.deep.include(newLabel) }) }) diff --git a/packages/api/test/util.ts b/packages/api/test/util.ts index 8f1bab161..f63fca1a3 100644 --- a/packages/api/test/util.ts +++ b/packages/api/test/util.ts @@ -2,10 +2,9 @@ import { createApp } from '../src/server' import supertest from 'supertest' import { v4 } from 'uuid' import { corsConfig } from '../src/utils/corsConfig' -import { ArticleSavingRequestStatus, Page } from '../src/elastic/types' +import { ArticleSavingRequestStatus, Label, Page } from '../src/elastic/types' import { PageType } from '../src/generated/graphql' import { User } from '../src/entity/user' -import { Label } from '../src/entity/label' import { createPubSubClient } from '../src/datalayer/pubsub' import { createPage, getPageById } from '../src/elastic/pages' diff --git a/packages/db/elastic_migrations/index_settings.json b/packages/db/elastic_migrations/index_settings.json index dc51766ee..be7205a5a 100644 --- a/packages/db/elastic_migrations/index_settings.json +++ b/packages/db/elastic_migrations/index_settings.json @@ -50,9 +50,21 @@ "labels": { "type": "nested", "properties": { + "id": { + "type": "keyword" + }, "name": { "type": "keyword", "normalizer": "lowercase_normalizer" + }, + "color": { + "type": "keyword" + }, + "description": { + "type": "text" + }, + "createdAt": { + "type": "date" } } },