From abd42f70648c0931d543c79d72ca6f53283ad8bc Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Mon, 6 Mar 2023 14:34:50 +0800
Subject: [PATCH] Add labels and state to saveUrl API

---
 packages/api/src/generated/graphql.ts         |  5 +-
 packages/api/src/generated/schema.graphql     |  5 +-
 packages/api/src/resolvers/article/index.ts   |  4 +-
 .../resolvers/article_saving_request/index.ts |  8 +++-
 packages/api/src/routers/article_router.ts    |  3 +-
 packages/api/src/routers/svc/links.ts         |  9 ++--
 packages/api/src/schema.ts                    |  5 +-
 .../src/services/create_page_save_request.ts  | 37 +++++++++++----
 packages/api/src/services/labels.ts           | 25 ++++------
 packages/api/src/services/save_file.ts        | 10 +++-
 packages/api/src/services/save_page.ts        | 34 +++++++-------
 packages/api/src/services/save_url.ts         | 25 +++++++---
 packages/api/src/utils/helpers.ts             |  3 +-
 packages/api/test/resolvers/article.test.ts   | 47 +++++++++++++++++--
 14 files changed, 147 insertions(+), 73 deletions(-)

diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts
index 0b98217dd..da0ef3fba 100644
--- a/packages/api/src/generated/graphql.ts
+++ b/packages/api/src/generated/graphql.ts
@@ -2187,14 +2187,15 @@ export type SaveError = {
 
 export enum SaveErrorCode {
   EmbeddedHighlightFailed = 'EMBEDDED_HIGHLIGHT_FAILED',
-  EmbeddedLabelFailed = 'EMBEDDED_LABEL_FAILED',
   Unauthorized = 'UNAUTHORIZED',
   Unknown = 'UNKNOWN'
 }
 
 export type SaveFileInput = {
   clientRequestId: Scalars['ID'];
+  labels?: InputMaybe<Array<CreateLabelInput>>;
   source: Scalars['String'];
+  state?: InputMaybe<ArticleSavingRequestStatus>;
   uploadFileId: Scalars['ID'];
   url: Scalars['String'];
 };
@@ -2245,7 +2246,9 @@ export type SaveSuccess = {
 
 export type SaveUrlInput = {
   clientRequestId: Scalars['ID'];
+  labels?: InputMaybe<Array<CreateLabelInput>>;
   source: Scalars['String'];
+  state?: InputMaybe<ArticleSavingRequestStatus>;
   url: Scalars['String'];
 };
 
diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql
index 4d95ef831..1ae8efa8d 100644
--- a/packages/api/src/generated/schema.graphql
+++ b/packages/api/src/generated/schema.graphql
@@ -1584,14 +1584,15 @@ type SaveError {
 
 enum SaveErrorCode {
   EMBEDDED_HIGHLIGHT_FAILED
-  EMBEDDED_LABEL_FAILED
   UNAUTHORIZED
   UNKNOWN
 }
 
 input SaveFileInput {
   clientRequestId: ID!
+  labels: [CreateLabelInput!]
   source: String!
+  state: ArticleSavingRequestStatus
   uploadFileId: ID!
   url: String!
 }
@@ -1639,7 +1640,9 @@ type SaveSuccess {
 
 input SaveUrlInput {
   clientRequestId: ID!
+  labels: [CreateLabelInput!]
   source: String!
+  state: ArticleSavingRequestStatus
   url: String!
 }
 
diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts
index abe36814b..c7e703c10 100644
--- a/packages/api/src/resolvers/article/index.ts
+++ b/packages/api/src/resolvers/article/index.ts
@@ -248,7 +248,7 @@ export const createArticleResolver = authorized<
         source !== 'puppeteer-parse' &&
         FORCE_PUPPETEER_URLS.some((regex) => regex.test(url))
       ) {
-        await createPageSaveRequest(uid, url, models)
+        await createPageSaveRequest({ userId: uid, url })
         return DUMMY_RESPONSE
       } else if (!skipParsing && preparedDocument?.document) {
         const parseResults = await traceAs<Promise<ParsedContentPuppeteer>>(
@@ -264,7 +264,7 @@ export const createArticleResolver = authorized<
       } else if (!preparedDocument?.document) {
         // We have a URL but no document, so we try to send this to puppeteer
         // and return a dummy response.
-        await createPageSaveRequest(uid, url, models)
+        await createPageSaveRequest({ userId: uid, url })
         return DUMMY_RESPONSE
       }
 
diff --git a/packages/api/src/resolvers/article_saving_request/index.ts b/packages/api/src/resolvers/article_saving_request/index.ts
index a6ce2a8ba..2c431dfb0 100644
--- a/packages/api/src/resolvers/article_saving_request/index.ts
+++ b/packages/api/src/resolvers/article_saving_request/index.ts
@@ -25,7 +25,7 @@ export const createArticleSavingRequestResolver = authorized<
   CreateArticleSavingRequestSuccess,
   CreateArticleSavingRequestError,
   MutationCreateArticleSavingRequestArgs
->(async (_, { input: { url } }, { models, claims, pubsub }) => {
+>(async (_, { input: { url } }, { claims, pubsub }) => {
   analytics.track({
     userId: claims.uid,
     event: 'link_saved',
@@ -37,7 +37,11 @@ export const createArticleSavingRequestResolver = authorized<
   })
 
   try {
-    const request = await createPageSaveRequest(claims.uid, url, models, pubsub)
+    const request = await createPageSaveRequest({
+      userId: claims.uid,
+      url,
+      pubsub,
+    })
     return {
       articleSavingRequest: request,
     }
diff --git a/packages/api/src/routers/article_router.ts b/packages/api/src/routers/article_router.ts
index 988a94a2b..dc28281c1 100644
--- a/packages/api/src/routers/article_router.ts
+++ b/packages/api/src/routers/article_router.ts
@@ -59,8 +59,7 @@ export function articleRouter() {
       return res.status(400).send({ errorCode: 'BAD_DATA' })
     }
 
-    const models = initModels(kx, false)
-    const result = await createPageSaveRequest(uid, url, models)
+    const result = await createPageSaveRequest({ userId: uid, url })
 
     if (isSiteBlockedForParse(url)) {
       return res
diff --git a/packages/api/src/routers/svc/links.ts b/packages/api/src/routers/svc/links.ts
index b656203c0..4e6c23ed0 100644
--- a/packages/api/src/routers/svc/links.ts
+++ b/packages/api/src/routers/svc/links.ts
@@ -3,9 +3,7 @@
 /* eslint-disable @typescript-eslint/explicit-module-boundary-types */
 import express from 'express'
 import { readPushSubscription } from '../../datalayer/pubsub'
-import { kx } from '../../datalayer/knex_config'
 import { createPageSaveRequest } from '../../services/create_page_save_request'
-import { initModels } from '../../server'
 
 interface CreateLinkRequestMessage {
   url: string
@@ -39,10 +37,11 @@ export function linkServiceRouter() {
     }
     const msg = data as CreateLinkRequestMessage
 
-    const models = initModels(kx, false)
-
     try {
-      const request = await createPageSaveRequest(msg.userId, msg.url, models)
+      const request = await createPageSaveRequest({
+        userId: msg.userId,
+        url: msg.url,
+      })
       console.log('create link request', request)
 
       res.status(200).send(request)
diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts
index 2bfee3e6e..a09570776 100755
--- a/packages/api/src/schema.ts
+++ b/packages/api/src/schema.ts
@@ -513,7 +513,6 @@ const schema = gql`
     UNKNOWN
     UNAUTHORIZED
     EMBEDDED_HIGHLIGHT_FAILED
-    EMBEDDED_LABEL_FAILED
   }
 
   type SaveError {
@@ -531,6 +530,8 @@ const schema = gql`
     source: String!
     clientRequestId: ID!
     uploadFileId: ID!
+    state: ArticleSavingRequestStatus
+    labels: [CreateLabelInput!]
   }
 
   input ParseResult {
@@ -563,6 +564,8 @@ const schema = gql`
     url: String!
     source: String!
     clientRequestId: ID!
+    state: ArticleSavingRequestStatus
+    labels: [CreateLabelInput!]
   }
 
   union SaveResult = SaveSuccess | SaveError
diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts
index f515d1e8c..77c3748ee 100644
--- a/packages/api/src/services/create_page_save_request.ts
+++ b/packages/api/src/services/create_page_save_request.ts
@@ -8,7 +8,7 @@ import {
   getPageByParam,
   updatePage,
 } from '../elastic/pages'
-import { ArticleSavingRequestStatus, PageType } from '../elastic/types'
+import { ArticleSavingRequestStatus, Label, PageType } from '../elastic/types'
 import {
   ArticleSavingRequest,
   CreateArticleSavingRequestErrorCode,
@@ -17,6 +17,19 @@ import {
 import { DataModels } from '../resolvers/types'
 import { enqueueParseRequest } from '../utils/createTask'
 import { generateSlug, pageToArticleSavingRequest } from '../utils/helpers'
+import * as privateIpLib from 'private-ip'
+import { getRepository } from '../entity/utils'
+import { User } from '../entity/user'
+
+interface PageSaveRequest {
+  userId: string
+  url: string
+  pubsub?: PubsubClient
+  articleSavingRequestId?: string
+  archivedAt?: Date | null
+  labels?: Label[]
+  priority?: 'low' | 'high'
+}
 
 const SAVING_CONTENT = 'Your link is being saved...'
 
@@ -58,15 +71,15 @@ export const validateUrl = (url: string): URL => {
   return u
 }
 
-export const createPageSaveRequest = async (
-  userId: string,
-  url: string,
-  models: DataModels,
-  pubsub: PubsubClient = createPubSubClient(),
+export const createPageSaveRequest = async ({
+  userId,
+  url,
+  pubsub = createPubSubClient(),
   articleSavingRequestId = uuidv4(),
-  archivedAt?: Date | null,
-  priority?: 'low' | 'high'
-): Promise<ArticleSavingRequest> => {
+  archivedAt,
+  priority,
+  labels,
+}: PageSaveRequest): Promise<ArticleSavingRequest> => {
   try {
     validateUrl(url)
   } catch (error) {
@@ -76,7 +89,10 @@ export const createPageSaveRequest = async (
     })
   }
 
-  const user = await models.user.get(userId)
+  const user = await getRepository(User).findOne({
+    where: { id: userId },
+    relations: ['profile'],
+  })
   if (!user) {
     console.log('User not found', userId)
     return Promise.reject({
@@ -118,6 +134,7 @@ export const createPageSaveRequest = async (
       createdAt: new Date(),
       savedAt: new Date(),
       archivedAt,
+      labels,
     }
 
     // create processing page
diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts
index dd7ffa31d..fee84bc46 100644
--- a/packages/api/src/services/labels.ts
+++ b/packages/api/src/services/labels.ts
@@ -2,11 +2,12 @@ import { Label } from '../entity/label'
 import { ILike, In } from 'typeorm'
 import { PageContext } from '../elastic/types'
 import { User } from '../entity/user'
-import { addLabelInPage, updateLabelsInPage } from '../elastic/labels'
+import { addLabelInPage } from '../elastic/labels'
 import { getRepository } from '../entity/utils'
 import { Link } from '../entity/link'
 import DataLoader from 'dataloader'
 import { generateRandomColor } from '../utils/helpers'
+import { CreateLabelInput } from '../generated/graphql'
 
 const batchGetLabelsFromLinkIds = async (
   linkIds: readonly string[]
@@ -104,21 +105,16 @@ export const createLabel = async (
   })
 }
 
-export const addLabelsToNewPage = async (
+export const createLabels = async (
   ctx: PageContext,
-  pageId: string,
-  labels: {
-    name: string
-    color?: string | null
-    description?: string | null
-  }[]
-): Promise<boolean> => {
+  labels: CreateLabelInput[]
+): Promise<Label[]> => {
   const user = await getRepository(User).findOneBy({
     id: ctx.uid,
   })
   if (!user) {
-    console.log('user not found')
-    return false
+    console.error('user not found')
+    return []
   }
 
   const labelEntities = await getRepository(Label).findBy({
@@ -136,10 +132,5 @@ export const addLabelsToNewPage = async (
       user,
     }))
   )
-  // add all labels to page
-  return updateLabelsInPage(
-    pageId,
-    [...newLabelEntities, ...labelEntities],
-    ctx
-  )
+  return [...labelEntities, ...newLabelEntities]
 }
diff --git a/packages/api/src/services/save_file.ts b/packages/api/src/services/save_file.ts
index 3664d71ad..7e12f7150 100644
--- a/packages/api/src/services/save_file.ts
+++ b/packages/api/src/services/save_file.ts
@@ -36,10 +36,18 @@ export const saveFile = async (
 
   await getStorageFileDetails(input.uploadFileId, uploadFile.fileName)
 
-  await ctx.authTrx(async (tx) => {
+  const uploadFileData = await ctx.authTrx(async (tx) => {
     return ctx.models.uploadFile.setFileUploadComplete(input.uploadFileId, tx)
   })
 
+  if (!uploadFileData) {
+    return {
+      errorCodes: [SaveErrorCode.Unknown],
+    }
+  }
+
+  // TODO: save labels and archive state
+
   return {
     clientRequestId: input.clientRequestId,
     url: `${homePageURL()}/${saver.profile.username}/links/${
diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts
index ebca00339..78edd5c09 100644
--- a/packages/api/src/services/save_page.ts
+++ b/packages/api/src/services/save_page.ts
@@ -22,7 +22,7 @@ import {
 } from '../utils/helpers'
 import { parsePreparedContent } from '../utils/parser'
 import { createPageSaveRequest } from './create_page_save_request'
-import { addLabelsToNewPage } from './labels'
+import { createLabels } from './labels'
 
 type SaveContext = {
   pubsub: PubsubClient
@@ -108,8 +108,13 @@ export const savePage = async (
     userId: saver.userId,
     url: articleToSave.url,
   })
+  // save state
   const archivedAt =
     input.state === ArticleSavingRequestStatus.Archived ? new Date() : null
+  // add labels to page
+  const labels = input.labels
+    ? await createLabels(ctx, input.labels)
+    : undefined
 
   if (existingPage) {
     pageId = existingPage.id
@@ -124,6 +129,7 @@ export const savePage = async (
           id: pageId, // we don't want to update the id
           slug, // we don't want to update the slug
           createdAt: existingPage.createdAt, // we don't want to update the createdAt
+          labels,
         },
         ctx
       ))
@@ -135,14 +141,14 @@ export const savePage = async (
     }
   } else if (shouldParseInBackend(input)) {
     try {
-      await createPageSaveRequest(
-        saver.userId,
-        articleToSave.url,
-        ctx.models,
-        ctx.pubsub,
-        input.clientRequestId,
-        archivedAt
-      )
+      await createPageSaveRequest({
+        userId: saver.userId,
+        url: articleToSave.url,
+        pubsub: ctx.pubsub,
+        articleSavingRequestId: input.clientRequestId,
+        archivedAt,
+        labels,
+      })
     } catch (e) {
       return {
         errorCodes: [SaveErrorCode.Unknown],
@@ -154,6 +160,7 @@ export const savePage = async (
       {
         ...articleToSave,
         archivedAt,
+        labels,
       },
       ctx
     )
@@ -183,15 +190,6 @@ export const savePage = async (
       }
     }
   }
-  // add labels to page
-  if (pageId && input.labels) {
-    if (!(await addLabelsToNewPage(ctx, pageId, input.labels))) {
-      return {
-        errorCodes: [SaveErrorCode.EmbeddedLabelFailed],
-        message: 'Failed to save labels',
-      }
-    }
-  }
 
   return {
     clientRequestId: pageId,
diff --git a/packages/api/src/services/save_url.ts b/packages/api/src/services/save_url.ts
index ed26080dc..7b5695285 100644
--- a/packages/api/src/services/save_url.ts
+++ b/packages/api/src/services/save_url.ts
@@ -4,6 +4,8 @@ import { homePageURL } from '../env'
 import { SaveErrorCode, SaveResult, SaveUrlInput } from '../generated/graphql'
 import { DataModels } from '../resolvers/types'
 import { createPageSaveRequest } from './create_page_save_request'
+import { ArticleSavingRequestStatus } from '../elastic/types'
+import { createLabels } from './labels'
 
 type SaveContext = {
   pubsub: PubsubClient
@@ -16,13 +18,22 @@ export const saveUrl = async (
   input: SaveUrlInput
 ): Promise<SaveResult> => {
   try {
-    const pageSaveRequest = await createPageSaveRequest(
-      saver.id,
-      input.url,
-      ctx.models,
-      ctx.pubsub,
-      input.clientRequestId
-    )
+    // save state
+    const archivedAt =
+      input.state === ArticleSavingRequestStatus.Archived ? new Date() : null
+    // add labels to page
+    const labels = input.labels
+      ? await createLabels({ ...ctx, uid: saver.id }, input.labels)
+      : undefined
+
+    const pageSaveRequest = await createPageSaveRequest({
+      userId: saver.id,
+      url: input.url,
+      pubsub: ctx.pubsub,
+      articleSavingRequestId: input.clientRequestId,
+      archivedAt,
+      labels,
+    })
 
     return {
       clientRequestId: pageSaveRequest.id,
diff --git a/packages/api/src/utils/helpers.ts b/packages/api/src/utils/helpers.ts
index 76ad7776c..a51e120b1 100644
--- a/packages/api/src/utils/helpers.ts
+++ b/packages/api/src/utils/helpers.ts
@@ -18,6 +18,7 @@ import path from 'path'
 import normalizeUrl from 'normalize-url'
 import wordsCounter from 'word-counting'
 import _ from 'underscore'
+import { User } from '../entity/user'
 
 interface InputObject {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -187,7 +188,7 @@ export const pageError = async (
 }
 
 export const pageToArticleSavingRequest = (
-  user: UserData,
+  user: User,
   page: Page
 ): ArticleSavingRequest => ({
   ...page,
diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts
index ceebb1f15..cbcbaddae 100644
--- a/packages/api/test/resolvers/article.test.ts
+++ b/packages/api/test/resolvers/article.test.ts
@@ -37,6 +37,8 @@ import {
   graphqlRequest,
   request,
 } from '../util'
+import sinon from 'sinon'
+import * as createTask from '../../src/utils/createTask'
 
 chai.use(chaiString)
 
@@ -266,7 +268,11 @@ const saveFileQuery = (url: string, uploadFileId: string) => {
     `
 }
 
-const saveUrlQuery = (url: string) => {
+const saveUrlQuery = (
+  url: string,
+  state: ArticleSavingRequestStatus | null = null,
+  labels: string[] | null = null
+) => {
   return `
     mutation {
       saveUrl(
@@ -274,6 +280,12 @@ const saveUrlQuery = (url: string) => {
           url: "${url}",
           source: "test",
           clientRequestId: "${generateFakeUuid()}",
+          state: ${state}
+          labels: ${
+            labels
+              ? '[' + labels.map((label) => `{ name: "${label}" }`) + ']'
+              : null
+          }
         }
       ) {
         ... on SaveSuccess {
@@ -650,15 +662,23 @@ describe('Article API', () => {
     let query = ''
     let url = 'https://blog.omnivore.app/new-url-1'
 
+    before(() => {
+      sinon.replace(createTask, 'enqueueParseRequest', sinon.fake.resolves(''))
+    })
+
     beforeEach(() => {
       query = saveUrlQuery(url)
     })
 
-    context('when we save a new url', () => {
-      after(async () => {
-        await deletePagesByParam({ url }, ctx)
-      })
+    after(() => {
+      sinon.restore()
+    })
 
+    afterEach(async () => {
+      await deletePagesByParam({ url }, ctx)
+    })
+
+    context('when we save a new url', () => {
       it('should return a slugged url', async () => {
         const res = await graphqlRequest(query, authToken).expect(200)
         expect(res.body.data.saveUrl.url).to.startsWith(
@@ -666,6 +686,23 @@ describe('Article API', () => {
         )
       })
     })
+
+    context('when we save labels', () => {
+      it('saves the labels and archives the page', async () => {
+        url = 'https://blog.omnivore.app/new-url-2'
+        const state = ArticleSavingRequestStatus.Archived
+        const labels = ['test name', 'test name 2']
+        await graphqlRequest(
+          saveUrlQuery(url, state, labels),
+          authToken
+        ).expect(200)
+        await refreshIndex()
+
+        const savedPage = await getPageByParam({ url })
+        expect(savedPage?.archivedAt).to.not.be.null
+        expect(savedPage?.labels?.map((l) => l.name)).to.eql(labels)
+      })
+    })
   })
 
   describe('setBookmarkArticle', () => {