Merge pull request #1905 from omnivore-app/annotation-api

Add type to the highlight in the elasticsearch and three types are HIGHLIGHTS, REDACTION and NOTE

packages/api/src/elastic/highlights.ts

@@ -29,7 +29,7 @@ export const addHighlightToPage = async (
                   ctx._source.updatedAt = params.highlight.updatedAt`,
           lang: 'painless',
           params: {
-            highlight: highlight,
+            highlight,
           },
         },
       },
@@ -136,9 +136,12 @@ export const deleteHighlight = async (
       refresh: ctx.refresh,
     })
 
-    if (body.updated === 0) return false
-
-    await ctx.pubsub.entityDeleted(EntityType.HIGHLIGHT, highlightId, ctx.uid)
+    body.updated > 0 &&
+      (await ctx.pubsub.entityDeleted(
+        EntityType.HIGHLIGHT,
+        highlightId,
+        ctx.uid
+      ))
 
     return true
   } catch (e) {
@@ -254,7 +257,7 @@ export const updateHighlight = async (
   ctx: PageContext
 ): Promise<boolean> => {
   try {
-    await client.updateByQuery({
+    const { body } = await client.updateByQuery({
       index: INDEX_ALIAS,
       body: {
         script: {
@@ -292,11 +295,12 @@ export const updateHighlight = async (
       conflicts: 'proceed',
     })
 
-    await ctx.pubsub.entityUpdated<Highlight>(
-      EntityType.HIGHLIGHT,
-      highlight,
-      ctx.uid
-    )
+    body.updated > 0 &&
+      (await ctx.pubsub.entityUpdated<Highlight>(
+        EntityType.HIGHLIGHT,
+        highlight,
+        ctx.uid
+      ))
 
     return true
   } catch (e) {

packages/api/src/elastic/labels.ts

@@ -282,7 +282,7 @@ export const setLabelsForHighlight = async (
           lang: 'painless',
           params: {
             highlightId,
-            labels: labels,
+            labels,
             updatedAt: new Date(),
           },
         },

packages/api/src/elastic/types.ts

@@ -71,6 +71,12 @@ export enum ArticleSavingRequestStatus {
   Deleted = 'DELETED',
 }
 
+export enum HighlightType {
+  Highlight = 'HIGHLIGHT',
+  Redaction = 'REDACTION', // allowing people to remove text from the page
+  Note = 'NOTE', // allowing people to add a note at the document level
+}
+
 export interface Label {
   id: string
   name: string
@@ -82,8 +88,8 @@ export interface Label {
 export interface Highlight {
   id: string
   shortId: string
-  patch: string
-  quote: string
+  patch?: string | null
+  quote?: string | null
   userId: string
   createdAt: Date
   prefix?: string | null
@@ -94,6 +100,7 @@ export interface Highlight {
   labels?: Label[]
   highlightPositionPercent?: number | null
   highlightPositionAnchorIndex?: number | null
+  type: HighlightType
   html?: string | null
 }
 

packages/api/src/generated/graphql.ts

@@ -348,12 +348,13 @@ export type CreateHighlightInput = {
   highlightPositionPercent?: InputMaybe<Scalars['Float']>;
   html?: InputMaybe<Scalars['String']>;
   id: Scalars['ID'];
-  patch: Scalars['String'];
+  patch?: InputMaybe<Scalars['String']>;
   prefix?: InputMaybe<Scalars['String']>;
-  quote: Scalars['String'];
+  quote?: InputMaybe<Scalars['String']>;
   sharedAt?: InputMaybe<Scalars['Date']>;
   shortId: Scalars['String'];
   suffix?: InputMaybe<Scalars['String']>;
+  type?: InputMaybe<HighlightType>;
 };
 
 export type CreateHighlightReplyError = {
@@ -905,14 +906,15 @@ export type Highlight = {
   html?: Maybe<Scalars['String']>;
   id: Scalars['ID'];
   labels?: Maybe<Array<Label>>;
-  patch: Scalars['String'];
+  patch?: Maybe<Scalars['String']>;
   prefix?: Maybe<Scalars['String']>;
-  quote: Scalars['String'];
+  quote?: Maybe<Scalars['String']>;
   reactions: Array<Reaction>;
   replies: Array<HighlightReply>;
   sharedAt?: Maybe<Scalars['Date']>;
   shortId: Scalars['String'];
   suffix?: Maybe<Scalars['String']>;
+  type: HighlightType;
   updatedAt: Scalars['Date'];
   user: User;
 };
@@ -932,6 +934,12 @@ export type HighlightStats = {
   highlightCount: Scalars['Int'];
 };
 
+export enum HighlightType {
+  Highlight = 'HIGHLIGHT',
+  Note = 'NOTE',
+  Redaction = 'REDACTION'
+}
+
 export type Integration = {
   __typename?: 'Integration';
   createdAt: Scalars['Date'];
@@ -3384,6 +3392,7 @@ export type ResolversTypes = {
   Highlight: ResolverTypeWrapper<Highlight>;
   HighlightReply: ResolverTypeWrapper<HighlightReply>;
   HighlightStats: ResolverTypeWrapper<HighlightStats>;
+  HighlightType: HighlightType;
   ID: ResolverTypeWrapper<Scalars['ID']>;
   Int: ResolverTypeWrapper<Scalars['Int']>;
   Integration: ResolverTypeWrapper<Integration>;
@@ -4722,14 +4731,15 @@ export type HighlightResolvers<ContextType = ResolverContext, ParentType extends
   html?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
   id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
   labels?: Resolver<Maybe<Array<ResolversTypes['Label']>>, ParentType, ContextType>;
-  patch?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
+  patch?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
   prefix?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
-  quote?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
+  quote?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
   reactions?: Resolver<Array<ResolversTypes['Reaction']>, ParentType, ContextType>;
   replies?: Resolver<Array<ResolversTypes['HighlightReply']>, ParentType, ContextType>;
   sharedAt?: Resolver<Maybe<ResolversTypes['Date']>, ParentType, ContextType>;
   shortId?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
   suffix?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
+  type?: Resolver<ResolversTypes['HighlightType'], ParentType, ContextType>;
   updatedAt?: Resolver<ResolversTypes['Date'], ParentType, ContextType>;
   user?: Resolver<ResolversTypes['User'], ParentType, ContextType>;
   __isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;

packages/api/src/generated/schema.graphql

@@ -300,12 +300,13 @@ input CreateHighlightInput {
   highlightPositionPercent: Float
   html: String
   id: ID!
-  patch: String!
+  patch: String
   prefix: String
-  quote: String!
+  quote: String
   sharedAt: Date
   shortId: String!
   suffix: String
+  type: HighlightType
 }
 
 type CreateHighlightReplyError {
@@ -802,14 +803,15 @@ type Highlight {
   html: String
   id: ID!
   labels: [Label!]
-  patch: String!
+  patch: String
   prefix: String
-  quote: String!
+  quote: String
   reactions: [Reaction!]!
   replies: [HighlightReply!]!
   sharedAt: Date
   shortId: String!
   suffix: String
+  type: HighlightType!
   updatedAt: Date!
   user: User!
 }
@@ -827,6 +829,12 @@ type HighlightStats {
   highlightCount: Int!
 }
 
+enum HighlightType {
+  HIGHLIGHT
+  NOTE
+  REDACTION
+}
+
 type Integration {
   createdAt: Date!
   enabled: Boolean!

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

@@ -8,7 +8,11 @@ import {
   updateHighlight,
 } from '../../elastic/highlights'
 import { getPageById, updatePage } from '../../elastic/pages'
-import { Highlight as HighlightData } from '../../elastic/types'
+import {
+  Highlight as HighlightData,
+  HighlightType,
+  Label,
+} from '../../elastic/types'
 import { env } from '../../env'
 import {
   CreateHighlightError,
@@ -81,6 +85,7 @@ export const createHighlightResolver = authorized<
       createdAt: new Date(),
       userId: claims.uid,
       annotation,
+      type: input.type || HighlightType.Highlight,
     }
 
     if (
@@ -139,38 +144,48 @@ export const mergeHighlightResolver = authorized<
       errorCodes: [MergeHighlightErrorCode.Unauthorized],
     }
   }
-  const articleHighlights = page.highlights
-
   /* Compute merged annotation form the order of highlights appearing on page */
-  const overlapAnnotations: { [id: string]: string } = {}
-  articleHighlights.forEach((highlight, index) => {
-    if (overlapHighlightIdList.includes(highlight.id)) {
-      articleHighlights.splice(index, 1)
-
+  const mergedAnnotations: string[] = []
+  const mergedLabels: Label[] = []
+  const pageHighlights = page.highlights.filter((highlight) => {
+    // filter out highlights that are in the overlap list
+    // and are of type highlight (not annotation or note)
+    if (
+      overlapHighlightIdList.includes(highlight.id) &&
+      highlight.type === HighlightType.Highlight
+    ) {
       if (highlight.annotation) {
-        overlapAnnotations[highlight.id] = highlight.annotation
+        mergedAnnotations.push(highlight.annotation)
       }
+      if (highlight.labels) {
+        // remove duplicates from labels by checking id
+        highlight.labels.forEach((label) => {
+          if (
+            !mergedLabels.find((mergedLabel) => mergedLabel.id === label.id)
+          ) {
+            mergedLabels.push(label)
+          }
+        })
+      }
+      return false
     }
+    return true
   })
-  const mergedAnnotation: string[] = []
-  overlapHighlightIdList.forEach((highlightId) => {
-    if (overlapAnnotations[highlightId]) {
-      mergedAnnotation.push(overlapAnnotations[highlightId])
-    }
-  })
-
   try {
     const highlight: HighlightData = {
       ...newHighlightInput,
       updatedAt: new Date(),
       createdAt: new Date(),
       userId: claims.uid,
-      annotation: mergedAnnotation ? mergedAnnotation.join('\n') : null,
+      annotation:
+        mergedAnnotations.length > 0 ? mergedAnnotations.join('\n') : null,
+      type: HighlightType.Highlight,
+      labels: mergedLabels,
     }
 
     const merged = await updatePage(
       pageId,
-      { highlights: articleHighlights.concat(highlight) },
+      { highlights: pageHighlights.concat(highlight) },
       { pubsub, uid: claims.uid }
     )
     if (!merged) {

packages/api/src/schema.ts

@@ -660,18 +660,24 @@ const schema = gql`
     reactions: [Reaction!]!
   }
 
+  enum HighlightType {
+    HIGHLIGHT
+    REDACTION
+    NOTE
+  }
+
   # Highlight
   type Highlight {
     id: ID!
     # used for simplified url format
     shortId: String!
     user: User!
-    quote: String!
+    quote: String
     # piece of content before the quote
     prefix: String
     # piece of content after the quote
     suffix: String
-    patch: String!
+    patch: String
     annotation: String
     replies: [HighlightReply!]!
     sharedAt: Date
@@ -682,6 +688,7 @@ const schema = gql`
     highlightPositionPercent: Float
     highlightPositionAnchorIndex: Int
     labels: [Label!]
+    type: HighlightType!
     html: String
   }
 
@@ -689,14 +696,15 @@ const schema = gql`
     id: ID!
     shortId: String!
     articleId: ID!
-    patch: String!
-    quote: String! @sanitize(maxLength: 6000, minLength: 1)
+    patch: String
+    quote: String @sanitize(maxLength: 6000, minLength: 1)
     prefix: String @sanitize
     suffix: String @sanitize
     annotation: String @sanitize(maxLength: 4000)
     sharedAt: Date
     highlightPositionPercent: Float
     highlightPositionAnchorIndex: Int
+    type: HighlightType
     html: String
   }
 

packages/api/src/services/integrations.ts

@@ -2,7 +2,7 @@ import { IntegrationType } from '../generated/graphql'
 import { env } from '../env'
 import axios from 'axios'
 import { wait } from '../utils/helpers'
-import { Page } from '../elastic/types'
+import { HighlightType, Page } from '../elastic/types'
 import { getHighlightUrl } from './highlights'
 import { Integration } from '../entity/integration'
 import { getRepository } from '../entity/utils'
@@ -68,11 +68,14 @@ const pageToReadwiseHighlight = (page: Page): ReadwiseHighlight[] => {
   const category = page.siteName === 'Twitter' ? 'tweets' : 'articles'
   return (
     page.highlights
-      // filter out highlights with no quote
-      .filter((highlight) => highlight.quote.length > 0)
+      // filter out highlights with no quote and are not of type Highlight
+      .filter(
+        (highlight) =>
+          highlight.type === HighlightType.Highlight && highlight.quote
+      )
       .map((highlight) => {
         return {
-          text: highlight.quote,
+          text: highlight.quote!,
           title: page.title,
           author: page.author || undefined,
           highlight_url: getHighlightUrl(page.slug, highlight.id),

packages/api/src/services/save_page.ts

@@ -6,6 +6,7 @@ import { createPage, getPageByParam, updatePage } from '../elastic/pages'
 import { ArticleSavingRequestStatus, Page, PageType } from '../elastic/types'
 import { homePageURL } from '../env'
 import {
+  HighlightType,
   Maybe,
   PreparedDocumentInput,
   SaveErrorCode,
@@ -161,6 +162,7 @@ export const savePage = async (
       userId: ctx.uid,
       elasticPageId: pageId,
       ...parseResult.highlightData,
+      type: HighlightType.Highlight,
     }
 
     if (

packages/api/test/elastic/highlights.test.ts

@@ -1,6 +1,11 @@
 import 'mocha'
 import { expect } from 'chai'
-import { Highlight, Page, PageContext } from '../../src/elastic/types'
+import {
+  Highlight,
+  HighlightType,
+  Page,
+  PageContext,
+} from '../../src/elastic/types'
 import { createPubSubClient } from '../../src/datalayer/pubsub'
 import { deletePage } from '../../src/elastic/pages'
 import {
@@ -32,6 +37,7 @@ describe('highlights in elastic', () => {
         userId: page.userId,
         createdAt: new Date(),
         updatedAt: new Date(),
+        type: HighlightType.Highlight,
       }
 
       await addHighlightToPage(page.id, highlightData, ctx)

packages/api/test/elastic/labels.test.ts

@@ -3,6 +3,7 @@ import { expect } from 'chai'
 import {
   ArticleSavingRequestStatus,
   Highlight,
+  HighlightType,
   Label,
   Page,
   PageContext,
@@ -123,6 +124,7 @@ describe('labels in elastic', () => {
         userId: page.userId,
         createdAt: new Date(),
         updatedAt: new Date(),
+        type: HighlightType.Highlight,
       }
 
       await addHighlightToPage(page.id, highlightData, ctx)

packages/api/test/resolvers/article.test.ts

@@ -15,6 +15,7 @@ import {
 import {
   ArticleSavingRequestStatus,
   Highlight,
+  HighlightType,
   Page,
   PageContext,
   PageType,
@@ -476,6 +477,7 @@ describe('Article API', () => {
             quote: 'test quote',
             updatedAt: new Date(),
             userId: user.id,
+            type: HighlightType.Highlight,
           },
         ],
       }
@@ -861,6 +863,7 @@ describe('Article API', () => {
           quote: '<p>search highlight</p>',
           createdAt: new Date(),
           updatedAt: new Date(),
+          type: HighlightType.Highlight,
         }
         await addHighlightToPage(page.id, highlight, ctx)
         highlights.push(highlight)

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

@@ -1,11 +1,3 @@
-import * as chai from 'chai'
-import { expect } from 'chai'
-import chaiString from 'chai-string'
-import 'mocha'
-import { createPubSubClient } from '../../src/datalayer/pubsub'
-import { deletePage, updatePage } from '../../src/elastic/pages'
-import { PageContext } from '../../src/elastic/types'
-import { User } from '../../src/entity/user'
 import { createTestUser, deleteTestUser } from '../db'
 import {
   createTestElasticPage,
@@ -13,6 +5,14 @@ import {
   graphqlRequest,
   request,
 } from '../util'
+import * as chai from 'chai'
+import { expect } from 'chai'
+import 'mocha'
+import { User } from '../../src/entity/user'
+import chaiString from 'chai-string'
+import { createPubSubClient } from '../../src/datalayer/pubsub'
+import { HighlightType, PageContext } from '../../src/elastic/types'
+import { deletePage, updatePage } from '../../src/elastic/pages'
 
 chai.use(chaiString)
 
@@ -269,6 +269,7 @@ describe('Highlights API', () => {
               quote: '',
               createdAt: new Date(),
               updatedAt: new Date(),
+              type: HighlightType.Highlight,
             },
           ],
         },

packages/api/test/resolvers/labels.test.ts

@@ -14,7 +14,12 @@ import { Label } from '../../src/entity/label'
 import { expect } from 'chai'
 import 'mocha'
 import { User } from '../../src/entity/user'
-import { Highlight, Page, PageContext } from '../../src/elastic/types'
+import {
+  Highlight,
+  HighlightType,
+  Page,
+  PageContext,
+} from '../../src/elastic/types'
 import { getRepository } from '../../src/entity/utils'
 import { deletePage, getPageById } from '../../src/elastic/pages'
 import { createPubSubClient } from '../../src/datalayer/pubsub'
@@ -287,6 +292,7 @@ describe('Labels API', () => {
             createdAt: new Date(),
             labels: [toDeleteLabel],
             updatedAt: new Date(),
+            type: HighlightType.Highlight,
           }
           await addHighlightToPage(page.id, highlight, ctx)
         })
@@ -596,6 +602,7 @@ describe('Labels API', () => {
           shortId: 'test shortId',
           userId: user.id,
           updatedAt: new Date(),
+          type: HighlightType.Highlight,
         }
         await addHighlightToPage(page.id, highlight, ctx)
         labelIds = [labels[0].id, labels[1].id]
@@ -620,6 +627,7 @@ describe('Labels API', () => {
           shortId: 'test shortId',
           userId: user.id,
           updatedAt: new Date(),
+          type: HighlightType.Highlight,
         }
         await addHighlightToPage(page.id, highlight, ctx)
         labelIds = [generateFakeUuid(), generateFakeUuid()]

packages/api/test/routers/integrations.test.ts

@@ -10,7 +10,12 @@ import { User } from '../../src/entity/user'
 import { createTestUser, deleteTestIntegrations, deleteTestUser } from '../db'
 import { Integration, IntegrationType } from '../../src/entity/integration'
 import { getRepository } from '../../src/entity/utils'
-import { Highlight, Page, PageContext } from '../../src/elastic/types'
+import {
+  Highlight,
+  HighlightType,
+  Page,
+  PageContext,
+} from '../../src/elastic/types'
 import nock from 'nock'
 import { READWISE_API_URL } from '../../src/services/integrations'
 import { addHighlightToPage } from '../../src/elastic/highlights'
@@ -142,6 +147,7 @@ describe('Integrations routers', () => {
               updatedAt: new Date(),
               userId: user.id,
               highlightPositionPercent,
+              type: HighlightType.Highlight,
             }
             await addHighlightToPage(page.id, highlight, ctx)
             // create highlights data for integration request

packages/db/elastic_migrations/index_settings.json

@@ -84,6 +84,10 @@
           "userId": {
             "type": "keyword"
           },
+          "type": {
+            "type": "keyword",
+            "null_value": "HIGHLIGHT"
+          },
           "quote": {
             "type": "text",
             "analyzer": "strip_html_analyzer"

packages/db/migrate.ts

@@ -148,41 +148,8 @@ const elasticMigration = esClient.indices
       log('Elastic index mappings updated.')
     })
   })
-  .then(() => {
-    log('Adding default state to pages in elastic...')
-    return esClient
-      .update_by_query({
-        index: INDEX_ALIAS,
-        requests_per_second: 250,
-        scroll_size: 500,
-        timeout: '30m',
-        body: {
-          script: {
-            source: 'ctx._source.state = params.state',
-            lang: 'painless',
-            params: {
-              state: 'SUCCEEDED',
-            },
-          },
-          query: {
-            bool: {
-              must_not: [
-                {
-                  exists: {
-                    field: 'state',
-                  },
-                },
-              ],
-            },
-          },
-        },
-      })
-      .then(() => log('Default state added.'))
-  })
   .catch((error) => {
     log(`${chalk.red('Elastic migration failed: ')}${error.message}`, chalk.red)
-    const { appliedMigrations } = error
-    logAppliedMigrations(appliedMigrations)
     process.exit(1)
   })