diff --git a/packages/api/src/resolvers/labels/index.ts b/packages/api/src/resolvers/labels/index.ts index 2e3b9c6d2..a1e17a605 100644 --- a/packages/api/src/resolvers/labels/index.ts +++ b/packages/api/src/resolvers/labels/index.ts @@ -30,6 +30,7 @@ import { } from '../../generated/graphql' import { labelRepository } from '../../repository/label' import { userRepository } from '../../repository/user' +import { findHighlightById } from '../../services/highlights' import { deleteLabelById, findOrCreateLabels, @@ -37,6 +38,7 @@ import { saveLabelsInLibraryItem, updateLabel, } from '../../services/labels' +import { findLibraryItemById } from '../../services/library_item' import { analytics } from '../../utils/analytics' import { authorized } from '../../utils/gql-utils' @@ -189,48 +191,48 @@ export const setLabelsResolver = authorized< labelSource = source } - try { - let labelsSet: Label[] = [] + let labelsSet: Label[] = [] - if (labels && labels.length > 0) { - // for new clients that send label names - // create labels if they don't exist - labelsSet = await findOrCreateLabels(labels, uid) - } else if (labelIds && labelIds.length > 0) { - // for old clients that send labelIds - labelsSet = await authTrx(async (tx) => { - return tx.withRepository(labelRepository).findLabelsById(labelIds) - }) - - if (labelsSet.length !== labelIds.length) { - return { - errorCodes: [SetLabelsErrorCode.NotFound], - } - } - } - - // save labels in the library item - await saveLabelsInLibraryItem(labelsSet, pageId, uid, labelSource, pubsub) - - analytics.capture({ - distinctId: uid, - event: 'labels_set', - properties: { - pageId, - labelIds, - env: env.server.apiEnv, - }, + if (labels && labels.length > 0) { + // for new clients that send label names + // create labels if they don't exist + labelsSet = await findOrCreateLabels(labels, uid) + } else if (labelIds && labelIds.length > 0) { + // for old clients that send labelIds + labelsSet = await authTrx(async (tx) => { + return tx.withRepository(labelRepository).findLabelsById(labelIds) }) - return { - labels: labelsSet, + if (labelsSet.length !== labelIds.length) { + return { + errorCodes: [SetLabelsErrorCode.NotFound], + } } - } catch (error) { - log.error('setLabelsResolver error', error) + } + + const libraryItem = await findLibraryItemById(pageId, uid) + if (!libraryItem) { return { - errorCodes: [SetLabelsErrorCode.BadRequest], + errorCodes: [SetLabelsErrorCode.Unauthorized], } } + + // save labels in the library item + await saveLabelsInLibraryItem(labelsSet, pageId, uid, labelSource, pubsub) + + analytics.capture({ + distinctId: uid, + event: 'labels_set', + properties: { + pageId, + labelIds, + env: env.server.apiEnv, + }, + }) + + return { + labels: labelsSet, + } } ) @@ -255,7 +257,7 @@ export const setLabelsForHighlightResolver = authorized< SetLabelsSuccess, SetLabelsError, MutationSetLabelsForHighlightArgs ->(async (_, { input }, { uid, log, pubsub, authTrx }) => { +>(async (_, { input }, { uid, log, authTrx }) => { const { highlightId, labelIds, labels } = input if (!labelIds && !labels) { @@ -265,47 +267,47 @@ export const setLabelsForHighlightResolver = authorized< } } - try { - let labelsSet: Label[] = [] + let labelsSet: Label[] = [] - if (labels && labels.length > 0) { - // for new clients that send label names - // create labels if they don't exist - labelsSet = await findOrCreateLabels(labels, uid) - } else if (labelIds && labelIds.length > 0) { - // for old clients that send labelIds - labelsSet = await authTrx(async (tx) => { - return tx.withRepository(labelRepository).findLabelsById(labelIds) - }) - if (labelsSet.length !== labelIds.length) { - return { - errorCodes: [SetLabelsErrorCode.NotFound], - } + if (labels && labels.length > 0) { + // for new clients that send label names + // create labels if they don't exist + labelsSet = await findOrCreateLabels(labels, uid) + } else if (labelIds && labelIds.length > 0) { + // for old clients that send labelIds + labelsSet = await authTrx(async (tx) => { + return tx.withRepository(labelRepository).findLabelsById(labelIds) + }) + if (labelsSet.length !== labelIds.length) { + return { + errorCodes: [SetLabelsErrorCode.NotFound], } } + } - // save labels in the highlight - await saveLabelsInHighlight(labelsSet, input.highlightId, uid, pubsub) - - analytics.capture({ - distinctId: uid, - event: 'labels_set_for_highlight', - properties: { - highlightId, - labelIds, - env: env.server.apiEnv, - }, - }) - + const highlight = await findHighlightById(highlightId, uid) + if (!highlight) { return { - labels: labelsSet, - } - } catch (error) { - log.error('setLabelsForHighlightResolver error', error) - return { - errorCodes: [SetLabelsErrorCode.BadRequest], + errorCodes: [SetLabelsErrorCode.Unauthorized], } } + + // save labels in the highlight + await saveLabelsInHighlight(labelsSet, input.highlightId) + + analytics.capture({ + distinctId: uid, + event: 'labels_set_for_highlight', + properties: { + highlightId, + labelIds, + env: env.server.apiEnv, + }, + }) + + return { + labels: labelsSet, + } }) export const moveLabelResolver = authorized< diff --git a/packages/api/src/services/highlights.ts b/packages/api/src/services/highlights.ts index 533d29eb2..861461de8 100644 --- a/packages/api/src/services/highlights.ts +++ b/packages/api/src/services/highlights.ts @@ -253,6 +253,7 @@ export const findHighlightById = async ( const highlightRepo = tx.withRepository(highlightRepository) return highlightRepo.findOneBy({ id: highlightId, + user: { id: userId }, }) }, undefined, diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts index 9e47ea65f..e31a597a5 100644 --- a/packages/api/src/services/labels.ts +++ b/packages/api/src/services/labels.ts @@ -185,15 +185,27 @@ export const addLabelsToLibraryItem = async ( ) => { await authTrx( async (tx) => { + // assign new labels if not exist to the item owner by user await tx.query( `INSERT INTO omnivore.entity_labels (label_id, library_item_id, source) - SELECT id, $1, $2 FROM omnivore.labels - WHERE id = ANY($3) - AND NOT EXISTS ( - SELECT 1 FROM omnivore.entity_labels - WHERE label_id = labels.id - AND library_item_id = $1 - )`, + SELECT + lbl.id, + $1, + $2 + FROM + omnivore.labels lbl + LEFT JOIN + omnivore.entity_labels el + ON + el.label_id = lbl.id + AND el.library_item_id = $1 + INNER JOIN + omnivore.library_item li + ON + li.id = $1 + WHERE + lbl.id = ANY($3) + AND el.label_id IS NULL;`, [libraryItemId, source, labelIds] ) }, @@ -207,9 +219,7 @@ export const addLabelsToLibraryItem = async ( export const saveLabelsInHighlight = async ( labels: Label[], - highlightId: string, - userId: string, - pubsub = createPubSubClient() + highlightId: string ) => { await authTrx(async (tx) => { const repo = tx.getRepository(EntityLabel) @@ -227,31 +237,6 @@ export const saveLabelsInHighlight = async ( })) ) }) - - // const highlight = await findHighlightById(highlightId, userId) - // if (!highlight) { - // logger.error('Highlight not found', { highlightId, userId }) - // return - // } - - // const libraryItemId = highlight.libraryItemId - // // create pubsub event - // await pubsub.entityCreated( - // EntityType.LABEL, - // { - // id: libraryItemId, - // highlights: [ - // { - // id: highlightId, - // labels: labels.map((l) => deepDelete(l, columnToDelete)), - // }, - // ], - // }, - // userId - // ) - - // // update labels in library item - // await bulkEnqueueUpdateLabels([{ libraryItemId, userId }]) } export const findLabelsByIds = async ( diff --git a/packages/api/src/services/score.ts b/packages/api/src/services/score.ts index 25c78f5a0..8752e4b4a 100644 --- a/packages/api/src/services/score.ts +++ b/packages/api/src/services/score.ts @@ -79,4 +79,4 @@ class ScoreClientImpl implements ScoreClient { } } -export const scoreClient = new StubScoreClientImpl() +export const scoreClient = new ScoreClientImpl() diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts index b51e93b37..3089ac76b 100644 --- a/packages/api/test/resolvers/highlight.test.ts +++ b/packages/api/test/resolvers/highlight.test.ts @@ -290,7 +290,7 @@ describe('Highlights API', () => { const labelColor = '#ff0000' const label = await createLabel(labelName, labelColor, user.id) - await saveLabelsInHighlight([label], highlightId, user.id) + await saveLabelsInHighlight([label], highlightId) const newHighlightId = generateFakeUuid() const newShortHighlightId = generateFakeShortId() @@ -459,11 +459,7 @@ describe('Highlights API', () => { const label1 = await createLabel(labelName1, '#ff0001', user.id) // save labels in highlights - await saveLabelsInHighlight( - [label, label1], - existingHighlights[0].id, - user.id - ) + await saveLabelsInHighlight([label, label1], existingHighlights[0].id) const res = await graphqlRequest(query, authToken, { query: `label:"${labelName}" label:"${labelName1}"`, diff --git a/packages/api/test/resolvers/labels.test.ts b/packages/api/test/resolvers/labels.test.ts index 4020e2eaa..1eef77366 100644 --- a/packages/api/test/resolvers/labels.test.ts +++ b/packages/api/test/resolvers/labels.test.ts @@ -322,7 +322,7 @@ describe('Labels API', () => { libraryItem: item, } await createHighlight(highlight, item.id, user.id) - await saveLabelsInHighlight([toDeleteLabel], highlightId, user.id) + await saveLabelsInHighlight([toDeleteLabel], highlightId) }) after(async () => { @@ -452,9 +452,9 @@ describe('Labels API', () => { labelIds = [labels[0].id, labels[1].id] }) - it('should return error code BAD_REQUEST', async () => { + it('should return error code UNAUTHORIZED', async () => { const res = await graphqlRequest(query, authToken).expect(200) - expect(res.body.data.setLabels.errorCodes).to.eql(['BAD_REQUEST']) + expect(res.body.data.setLabels.errorCodes).to.eql(['UNAUTHORIZED']) }) }) @@ -670,14 +670,14 @@ describe('Labels API', () => { context('when highlight not exist', () => { before(() => { - highlightId = 'fake_highlight_id' + highlightId = generateFakeUuid() labelIds = [labels[0].id, labels[1].id] }) - it('should return error code BAD_REQUEST', async () => { + it('should return error code UNAUTHORIZED', async () => { const res = await graphqlRequest(query, authToken).expect(200) expect(res.body.data.setLabelsForHighlight.errorCodes).to.eql([ - 'BAD_REQUEST', + 'UNAUTHORIZED', ]) }) })