Merge pull request #3326 from omnivore-app/fix/merge-highlight-error

fix labels not saved when merging highlights

packages/api/src/resolvers/highlight/index.ts

@@ -123,7 +123,6 @@ export const mergeHighlightResolver = authorized<
       ...newHighlightInput,
       annotation:
         mergedAnnotations.length > 0 ? mergedAnnotations.join('\n') : null,
-      labels: mergedLabels,
       color,
       user: { id: uid },
       libraryItem: { id: input.articleId },
@@ -134,6 +133,7 @@ export const mergeHighlightResolver = authorized<
     const newHighlight = await mergeHighlights(
       overlapHighlightIdList,
       highlight,
+      mergedLabels,
       input.articleId,
       uid,
       pubsub

packages/api/src/services/highlights.ts

@@ -1,7 +1,9 @@
 import { diff_match_patch } from 'diff-match-patch'
 import { DeepPartial } from 'typeorm'
 import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'
+import { EntityLabel } from '../entity/entity_label'
 import { Highlight } from '../entity/highlight'
+import { Label } from '../entity/label'
 import { homePageURL } from '../env'
 import { createPubSubClient, EntityType } from '../pubsub'
 import { authTrx } from '../repository'
@@ -65,6 +67,7 @@ export const createHighlight = async (
 export const mergeHighlights = async (
   highlightsToRemove: string[],
   highlightToAdd: DeepPartial<Highlight>,
+  labels: Label[],
   libraryItemId: string,
   userId: string,
   pubsub = createPubSubClient()
@@ -75,6 +78,17 @@ export const mergeHighlights = async (
     await highlightRepo.delete(highlightsToRemove)
 
     const newHighlight = await highlightRepo.createAndSave(highlightToAdd)
+
+    if (labels.length > 0) {
+      // save new labels
+      await tx.getRepository(EntityLabel).save(
+        labels.map((l) => ({
+          labelId: l.id,
+          highlightId: newHighlight.id,
+        }))
+      )
+    }
+
     return highlightRepo.findOneOrFail({
       where: { id: newHighlight.id },
       relations: {

packages/api/test/resolvers/highlight.test.ts

@@ -6,10 +6,12 @@ import { User } from '../../src/entity/user'
 import {
   createHighlight,
   deleteHighlightById,
+  findHighlightById,
 } from '../../src/services/highlights'
+import { createLabel, saveLabelsInHighlight } from '../../src/services/labels'
 import { deleteUser } from '../../src/services/user'
 import { createTestLibraryItem, createTestUser } from '../db'
-import { generateFakeUuid, graphqlRequest, request } from '../util'
+import { generateFakeShortId, generateFakeUuid, graphqlRequest, request } from '../util'
 
 chai.use(chaiString)
 
@@ -227,14 +229,18 @@ describe('Highlights API', () => {
   context('mergeHighlightMutation', () => {
     let highlightId: string
 
-    before(async () => {
+    beforeEach(async () => {
       // create test highlight
       highlightId = generateFakeUuid()
-      const shortHighlightId = '_short_id_1'
+      const shortHighlightId = generateFakeShortId()
       const query = createHighlightQuery(itemId, highlightId, shortHighlightId)
       await graphqlRequest(query, authToken).expect(200)
     })
 
+    afterEach(async () => {
+      await deleteHighlightById(highlightId)
+    })
+
     it('should not fail', async () => {
       const newHighlightId = generateFakeUuid()
       const newShortHighlightId = '_short_id_2'
@@ -257,6 +263,35 @@ describe('Highlights API', () => {
       expect(
         res.body.data.mergeHighlight.highlight.highlightPositionAnchorIndex
       ).to.eq(highlightPositionAnchorIndex)
+
+      highlightId = newHighlightId
+    })
+
+    it('keeps the labels of the merged highlight', async () => {
+      // create label
+      const labelName = 'test label'
+      const labelColor = '#ff0000'
+      const label = await createLabel(labelName, labelColor, user.id)
+
+      await saveLabelsInHighlight([label], highlightId, user.id)
+
+      const newHighlightId = generateFakeUuid()
+      const newShortHighlightId = generateFakeShortId()
+      const query = mergeHighlightQuery(
+        itemId,
+        newHighlightId,
+        newShortHighlightId,
+        [highlightId],
+      )
+      const res = await graphqlRequest(query, authToken).expect(200)
+
+      expect(res.body.data.mergeHighlight.highlight.id).to.eq(newHighlightId)
+
+      const highlight = await findHighlightById(newHighlightId, user.id)
+      expect(highlight.labels).to.have.lengthOf(1)
+      expect(highlight.labels?.[0]?.name).to.eq(labelName)
+
+      highlightId = newHighlightId
     })
   })
 

packages/api/test/util.ts

@@ -1,3 +1,4 @@
+import { nanoid } from 'nanoid'
 import supertest from 'supertest'
 import { v4 } from 'uuid'
 import { createApp } from '../src/server'
@@ -18,7 +19,7 @@ export const stopApolloServer = async () => {
 export const graphqlRequest = (
   query: string,
   authToken: string,
-  variables?: Record<string, unknown>,
+  variables?: Record<string, unknown>
 ): supertest.Test => {
   return request
     .post(apollo.graphqlPath)
@@ -31,3 +32,7 @@ export const graphqlRequest = (
 export const generateFakeUuid = () => {
   return v4()
 }
+
+export const generateFakeShortId = () => {
+  return nanoid(8)
+}