From 8cfa24a847f27d330eda5e34fc9f8d3ff0353d37 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Wed, 15 May 2024 21:46:22 +0800
Subject: [PATCH 1/4] allow downloading/uploading readable content

---
 packages/api/src/jobs/save_page.ts           | 10 +++---
 packages/api/src/jobs/upload_content.ts      | 36 ++++++++++++++------
 packages/api/src/routers/content_router.ts   | 24 +++++++------
 packages/api/src/services/library_item.ts    | 18 ++++++++--
 packages/api/src/utils/createTask.ts         |  3 ++
 packages/api/src/utils/highlightGenerator.ts |  2 +-
 packages/api/src/utils/uploads.ts            | 26 +++++++++++---
 7 files changed, 84 insertions(+), 35 deletions(-)

diff --git a/packages/api/src/jobs/save_page.ts b/packages/api/src/jobs/save_page.ts
index bf338a167..332196e00 100644
--- a/packages/api/src/jobs/save_page.ts
+++ b/packages/api/src/jobs/save_page.ts
@@ -186,12 +186,12 @@ export const savePageJob = async (data: Data, attemptsMade: number) => {
     }
 
     // download the original content
-    const filePath = contentFilePath(
+    const filePath = contentFilePath({
       userId,
-      articleSavingRequestId,
-      new Date(savedAt).getTime(),
-      'original'
-    )
+      libraryItemId: articleSavingRequestId,
+      format: 'original',
+      savedAt: new Date(savedAt),
+    })
     const exists = await isFileExists(filePath)
     if (!exists) {
       logger.error('Original content file does not exist', {
diff --git a/packages/api/src/jobs/upload_content.ts b/packages/api/src/jobs/upload_content.ts
index 7159510b2..fbddd79ee 100644
--- a/packages/api/src/jobs/upload_content.ts
+++ b/packages/api/src/jobs/upload_content.ts
@@ -2,11 +2,15 @@ import { Highlight } from '../entity/highlight'
 import { findLibraryItemById } from '../services/library_item'
 import { logger } from '../utils/logger'
 import { htmlToHighlightedMarkdown, htmlToMarkdown } from '../utils/parser'
-import { uploadToBucket } from '../utils/uploads'
+import { isFileExists, uploadToBucket } from '../utils/uploads'
 
 export const UPLOAD_CONTENT_JOB = 'UPLOAD_CONTENT_JOB'
 
-export type ContentFormat = 'markdown' | 'highlightedMarkdown' | 'original'
+export type ContentFormat =
+  | 'markdown'
+  | 'highlightedMarkdown'
+  | 'original'
+  | 'readable'
 
 export interface UploadContentJobData {
   libraryItemId: string
@@ -26,6 +30,7 @@ const convertContent = (
     case 'highlightedMarkdown':
       return htmlToHighlightedMarkdown(content, highlights)
     case 'original':
+    case 'readable':
       return content
     default:
       throw new Error('Unsupported format')
@@ -36,6 +41,7 @@ const CONTENT_TYPES = {
   markdown: 'text/markdown',
   highlightedMarkdown: 'text/markdown',
   original: 'text/html',
+  readable: 'text/html',
 }
 
 const getSelectOptions = (
@@ -43,6 +49,7 @@ const getSelectOptions = (
 ): { column: 'readableContent' | 'originalContent'; highlights?: boolean } => {
   switch (format) {
     case 'markdown':
+    case 'readable':
       return {
         column: 'readableContent',
       }
@@ -73,31 +80,40 @@ export const uploadContentJob = async (data: UploadContentJobData) => {
     },
   })
   if (!libraryItem) {
-    logger.error('Library item not found', data)
+    logger.error(`Library item not found: ${libraryItemId}`)
     throw new Error('Library item not found')
   }
 
   const content = libraryItem[column]
 
   if (!content) {
-    logger.error(`${column} not found`, data)
+    logger.error(`${column} not found`)
     throw new Error('Content not found')
   }
 
-  logger.info('Converting content', data)
+  logger.info('Converting content')
   const convertedContent = convertContent(
     content,
     format,
     libraryItem.highlights
   )
 
-  console.time('uploadToBucket')
-  logger.info('Uploading content', data)
+  const exists = await isFileExists(filePath)
+  if (exists) {
+    logger.info(`File already exists: ${filePath}`)
+    return
+  }
+
+  logger.info(`Uploading content: ${filePath}`)
+  logger.profile('Uploader')
+
   await uploadToBucket(filePath, Buffer.from(convertedContent), {
     contentType: CONTENT_TYPES[format],
-    timeout: 60000, // 1 minute
+    timeout: 10_000, // 10 seconds
   })
-  console.timeEnd('uploadToBucket')
 
-  logger.info('Content uploaded', data)
+  logger.profile('Uploader', {
+    level: 'info',
+    message: 'Content uploaded',
+  })
 }
diff --git a/packages/api/src/routers/content_router.ts b/packages/api/src/routers/content_router.ts
index b3110d354..14643b89a 100644
--- a/packages/api/src/routers/content_router.ts
+++ b/packages/api/src/routers/content_router.ts
@@ -72,14 +72,13 @@ export function contentRouter() {
     // generate signed url for each library item
     const data = await Promise.all(
       libraryItems.map(async (libraryItem) => {
-        const date =
-          format === 'original' ? libraryItem.savedAt : libraryItem.updatedAt
-        const filePath = contentFilePath(
+        const filePath = contentFilePath({
           userId,
-          libraryItem.id,
-          date.getTime(),
-          format
-        )
+          libraryItemId: libraryItem.id,
+          format,
+          savedAt: libraryItem.savedAt,
+          updatedAt: libraryItem.updatedAt,
+        })
 
         try {
           const downloadUrl = await generateDownloadSignedUrl(filePath, {
@@ -89,7 +88,7 @@ export function contentRouter() {
           // check if file is already uploaded
           const exists = await isFileExists(filePath)
           if (exists) {
-            logger.info('File already exists', filePath)
+            logger.info(`File already exists: ${filePath}`)
           }
 
           return {
@@ -109,7 +108,10 @@ export function contentRouter() {
         }
       })
     )
-    logger.info('Signed urls generated', data)
+    logger.info(
+      'Signed urls generated',
+      data.map((d) => d.downloadUrl)
+    )
 
     // skip uploading if there is an error or file already exists
     const uploadData = data.filter(
@@ -117,8 +119,8 @@ export function contentRouter() {
     ) as UploadContentJobData[]
 
     if (uploadData.length > 0) {
-      await enqueueBulkUploadContentJob(uploadData)
-      logger.info('Bulk upload content job enqueued', uploadData)
+      const jobs = await enqueueBulkUploadContentJob(uploadData)
+      logger.info('Bulk upload content job enqueued', jobs)
     }
 
     res.send({
diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts
index f62d9d2a4..9ad7f8471 100644
--- a/packages/api/src/services/library_item.ts
+++ b/packages/api/src/services/library_item.ts
@@ -1695,14 +1695,21 @@ export const uploadOriginalContent = async (
   userId: string,
   libraryItemId: string,
   savedAt: Date,
-  originalContent: string
+  originalContent: string,
+  timeout = 10_000 // 10 seconds
 ) => {
   await uploadToBucket(
-    contentFilePath(userId, libraryItemId, savedAt.getTime(), 'original'),
+    contentFilePath({
+      userId,
+      libraryItemId,
+      savedAt,
+      format: 'original',
+    }),
     Buffer.from(originalContent),
     {
       public: false,
       contentType: 'text/html',
+      timeout,
     }
   )
 }
@@ -1713,6 +1720,11 @@ export const downloadOriginalContent = async (
   savedAt: Date
 ) => {
   return downloadFromBucket(
-    contentFilePath(userId, libraryItemId, savedAt.getTime(), 'original')
+    contentFilePath({
+      userId,
+      libraryItemId,
+      savedAt,
+      format: 'original',
+    })
   )
 }
diff --git a/packages/api/src/utils/createTask.ts b/packages/api/src/utils/createTask.ts
index 546b69d11..e06c1bb99 100644
--- a/packages/api/src/utils/createTask.ts
+++ b/packages/api/src/utils/createTask.ts
@@ -970,6 +970,9 @@ export const enqueueBulkUploadContentJob = async (
     name: UPLOAD_CONTENT_JOB,
     data: d,
     opts: {
+      jobId: `${UPLOAD_CONTENT_JOB}_${d.filePath}_${JOB_VERSION}`, // dedupe by job id
+      removeOnComplete: true,
+      removeOnFail: true,
       attempts: 3,
       priority: getJobPriority(UPLOAD_CONTENT_JOB),
     },
diff --git a/packages/api/src/utils/highlightGenerator.ts b/packages/api/src/utils/highlightGenerator.ts
index b3f4448ba..8f9da4e1f 100644
--- a/packages/api/src/utils/highlightGenerator.ts
+++ b/packages/api/src/utils/highlightGenerator.ts
@@ -49,7 +49,7 @@ type FillNodeResponse = {
 }
 
 function getTextNodesBetween(rootNode: Node, startNode: Node, endNode: Node) {
-  const maxTime = 1000 * 60 // 60 seconds
+  const maxTime = 10_000 // 10 seconds
   const start = Date.now()
   let textNodeStartingPoint = 0
   let articleText = ''
diff --git a/packages/api/src/utils/uploads.ts b/packages/api/src/utils/uploads.ts
index 62acc9ce4..801f4f623 100644
--- a/packages/api/src/utils/uploads.ts
+++ b/packages/api/src/utils/uploads.ts
@@ -163,9 +163,25 @@ export const downloadFromBucket = async (filePath: string): Promise<Buffer> => {
   return data
 }
 
-export const contentFilePath = (
-  userId: string,
-  libraryItemId: string,
-  timestamp: number,
+export const contentFilePath = ({
+  userId,
+  libraryItemId,
+  format,
+  savedAt,
+  updatedAt,
+}: {
+  userId: string
+  libraryItemId: string
   format: ContentFormat
-) => `content/${userId}/${libraryItemId}.${timestamp}.${format}`
+  savedAt?: Date
+  updatedAt?: Date
+}) => {
+  // Use updatedAt for highlightedMarkdown format because highlights are saved
+  const date = format === 'highlightedMarkdown' ? updatedAt : savedAt
+
+  if (!date) {
+    throw new Error('Date not found')
+  }
+
+  return `content/${userId}/${libraryItemId}.${date.getTime()}.${format}`
+}

From 9769eab5dc967f0c8af04bfde24bad2f04b38c63 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Thu, 16 May 2024 11:23:36 +0800
Subject: [PATCH 2/4] fix youtube transcript

---
 .../api/src/jobs/process-youtube-video.ts     | 482 +++++++-----------
 1 file changed, 183 insertions(+), 299 deletions(-)

diff --git a/packages/api/src/jobs/process-youtube-video.ts b/packages/api/src/jobs/process-youtube-video.ts
index 427444959..916f7fdb0 100644
--- a/packages/api/src/jobs/process-youtube-video.ts
+++ b/packages/api/src/jobs/process-youtube-video.ts
@@ -1,21 +1,25 @@
-import { Storage } from '@google-cloud/storage'
 import { PromptTemplate } from '@langchain/core/prompts'
 import { OpenAI } from '@langchain/openai'
 import { parseHTML } from 'linkedom'
 import showdown from 'showdown'
-import * as stream from 'stream'
 import { Chapter, Client as YouTubeClient } from 'youtubei'
 import { LibraryItem, LibraryItemState } from '../entity/library_item'
-import { env } from '../env'
-import { authTrx } from '../repository'
-import { libraryItemRepository } from '../repository/library_item'
 import { FeatureName, findGrantedFeatureByName } from '../services/features'
+import {
+  findLibraryItemById,
+  updateLibraryItem,
+} from '../services/library_item'
+import { OPENAI_MODEL } from '../utils/ai'
 import { enqueueProcessYouTubeTranscript } from '../utils/createTask'
 import { stringToHash } from '../utils/helpers'
 import { logger } from '../utils/logger'
 import { parsePreparedContent } from '../utils/parser'
+import {
+  downloadFromBucket,
+  isFileExists,
+  uploadToBucket,
+} from '../utils/uploads'
 import { videoIdFromYouTubeUrl } from '../utils/youtube'
-import { OPENAI_MODEL } from '../utils/ai'
 
 export interface ProcessYouTubeVideoJobData {
   userId: string
@@ -143,132 +147,44 @@ export const addTranscriptToReadableContent = async (
   originalHTML: string,
   transcriptHTML: string
 ): Promise<string | undefined> => {
-  const html = parseHTML(originalHTML)
+  const document = parseHTML(originalHTML).document
 
-  const transcriptNode = html.document.querySelector(
-    '#_omnivore_youtube_transcript'
-  )
+  const rootElement = document.querySelector('#readability-page-1')
+  if (!rootElement) {
+    logger.warning('no readability-page-1 element found')
+    return undefined
+  }
+
+  const transcriptNode =
+    rootElement.querySelector('#_omnivore_youtube_transcript') ||
+    rootElement.querySelector('._omnivore_youtube_transcript')
 
   if (transcriptNode) {
     transcriptNode.innerHTML = transcriptHTML
   } else {
-    const div = html.document.createElement('div')
+    const div = document.createElement('div')
     div.innerHTML = transcriptHTML
-    html.document.body.appendChild(div)
-  }
+    div.className = '_omnivore_youtube_transcript'
 
-  const preparedDocument = {
-    document: html.document.toString(),
-    pageInfo: {},
-  }
-  const updatedContent = await parsePreparedContent(
-    originalUrl,
-    preparedDocument,
-    true
-  )
-  return updatedContent.parsedContent?.content
-}
-
-export const addTranscriptPlaceholdReadableContent = async (
-  originalUrl: string,
-  originalHTML: string
-): Promise<string | undefined> => {
-  const html = parseHTML(originalHTML)
-
-  const transcriptNode = html.document.querySelector(
-    '#_omnivore_youtube_transcript'
-  )
-
-  if (transcriptNode) {
-    transcriptNode.innerHTML = TRANSCRIPT_PLACEHOLDER_TEXT
-  } else {
-    const div = html.document.createElement('div')
-    div.innerHTML = TRANSCRIPT_PLACEHOLDER_TEXT
-    html.document.body.appendChild(div)
-  }
-
-  const preparedDocument = {
-    document: html.document.toString(),
-    pageInfo: {},
-  }
-  const updatedContent = await parsePreparedContent(
-    originalUrl,
-    preparedDocument,
-    true
-  )
-  return updatedContent.parsedContent?.content
-}
-
-async function readStringFromStorage(
-  bucketName: string,
-  fileName: string
-): Promise<string | undefined> {
-  try {
-    const storage = env.fileUpload?.gcsUploadSAKeyFilePath
-      ? new Storage({ keyFilename: env.fileUpload.gcsUploadSAKeyFilePath })
-      : new Storage()
-
-    const existsResponse = await storage
-      .bucket(bucketName)
-      .file(fileName)
-      .exists()
-    const exists = existsResponse[0]
-
-    if (!exists) {
-      throw new Error(
-        `File '${fileName}' does not exist in bucket '${bucketName}'.`
-      )
+    const videoElement = rootElement.querySelector('#_omnivore_youtube')
+    if (!videoElement) {
+      logger.warning('no video element found')
+      return undefined
     }
 
-    // Download the file contents as a string
-    const fileContentResponse = await storage
-      .bucket(bucketName)
-      .file(fileName)
-      .download()
-    const fileContent = fileContentResponse[0].toString()
-    return fileContent
-  } catch (error) {
-    // This isn't a catastrophic error it just means the file doesn't exist
-    logger.info('Error downloading file:', error)
-    return undefined
+    videoElement.appendChild(div)
   }
-}
 
-const writeStringToStorage = async (
-  bucketName: string,
-  fileName: string,
-  content: string
-): Promise<void> => {
-  try {
-    const storage = env.fileUpload?.gcsUploadSAKeyFilePath
-      ? new Storage({ keyFilename: env.fileUpload.gcsUploadSAKeyFilePath })
-      : new Storage()
-
-    const writableStream = storage
-      .bucket(bucketName)
-      .file(fileName)
-      .createWriteStream()
-
-    // Convert the string content to a readable stream
-    const readableStream = new stream.Readable()
-    readableStream.push(content)
-    readableStream.push(null) // Signal the end of the stream
-
-    // Pipe the readable stream to the writable stream to upload the file content
-    await new Promise((resolve, reject) => {
-      readableStream
-        .pipe(writableStream)
-        .on('finish', resolve)
-        .on('error', reject)
-    })
-
-    logger.info(
-      `File '${fileName}' uploaded successfully to bucket '${bucketName}'.`
-    )
-  } catch (error) {
-    logger.error('Error uploading file:', error)
-    throw error
+  const preparedDocument = {
+    document: `<html><body>${rootElement.innerHTML}</body></html>`,
+    pageInfo: {},
   }
+  const updatedContent = await parsePreparedContent(
+    originalUrl,
+    preparedDocument,
+    true
+  )
+  return updatedContent.parsedContent?.content
 }
 
 const fetchCachedYouTubeTranscript = async (
@@ -276,13 +192,16 @@ const fetchCachedYouTubeTranscript = async (
   transcriptHash: string,
   promptHash: string
 ): Promise<string | undefined> => {
-  const bucketName = env.fileUpload.gcsUploadBucket
-
   try {
-    return await readStringFromStorage(
-      bucketName,
-      `youtube-transcripts/${videoId}/${transcriptHash}.${promptHash}.html`
-    )
+    const filePath = `youtube-transcripts/${videoId}/${transcriptHash}.${promptHash}.html`
+    const exists = await isFileExists(filePath)
+    if (!exists) {
+      logger.info(`cached transcript not found: ${filePath}`)
+      return undefined
+    }
+
+    const buffer = await downloadFromBucket(filePath)
+    return buffer.toString()
   } catch (err) {
     logger.info(`unable to fetch cached transcript`, { error: err })
   }
@@ -296,124 +215,105 @@ const cacheYouTubeTranscript = async (
   promptHash: string,
   transcript: string
 ): Promise<void> => {
-  const bucketName = env.fileUpload.gcsUploadBucket
-
-  try {
-    await writeStringToStorage(
-      bucketName,
-      `youtube-transcripts/${videoId}/${transcriptHash}.${promptHash}.html`,
-      transcript
-    )
-  } catch (err) {
-    logger.info(`unable to cache transcript`, { error: err })
-  }
+  await uploadToBucket(
+    `youtube-transcripts/${videoId}/${transcriptHash}.${promptHash}.html`,
+    Buffer.from(transcript)
+  )
 }
 
 export const processYouTubeVideo = async (
   jobData: ProcessYouTubeVideoJobData
 ) => {
-  let videoURL: URL | undefined
-  try {
-    const libraryItem = await authTrx(
-      async (tx) =>
-        tx
-          .withRepository(libraryItemRepository)
-          .findById(jobData.libraryItemId),
-      undefined,
+  const libraryItem = await findLibraryItemById(
+    jobData.libraryItemId,
+    jobData.userId,
+    {
+      select: [
+        'id',
+        'originalUrl',
+        'description',
+        'wordCount',
+        'publishedAt',
+        'state',
+        'readableContent',
+      ],
+    }
+  )
+  if (!libraryItem || libraryItem.state !== LibraryItemState.Succeeded) {
+    logger.info(
+      `Not ready to get YouTube metadata job state: ${
+        libraryItem?.state ?? 'null'
+      }`
+    )
+    return
+  }
+
+  const videoURL = new URL(libraryItem.originalUrl)
+  const videoId = videoIdFromYouTubeUrl(videoURL.href)
+
+  if (!videoId) {
+    logger.warning('no video id for supplied youtube url', {
+      url: libraryItem.originalUrl,
+    })
+    return
+  }
+
+  const updatedLibraryItem: Partial<LibraryItem> = {}
+  const youtube = new YouTubeClient()
+  const video = await youtube.getVideo(videoId)
+  if (!video) {
+    logger.warning('no video found for youtube url', {
+      url: libraryItem.originalUrl,
+    })
+    return
+  }
+
+  if (video.description && libraryItem.description !== video.description) {
+    updatedLibraryItem.description = video.description
+  }
+
+  let duration = -1
+  if ('duration' in video && video.duration > 0) {
+    updatedLibraryItem.wordCount = calculateWordCount(video.duration)
+    duration = video.duration
+  }
+
+  if (video.uploadDate && !Number.isNaN(Date.parse(video.uploadDate))) {
+    updatedLibraryItem.publishedAt = new Date(video.uploadDate)
+  }
+
+  if (
+    await findGrantedFeatureByName(
+      FeatureName.YouTubeTranscripts,
       jobData.userId
     )
-    if (!libraryItem || libraryItem.state !== LibraryItemState.Succeeded) {
-      logger.info(
-        `Not ready to get YouTube metadata job state: ${
-          libraryItem?.state ?? 'null'
-        }`
+  ) {
+    if ('getTranscript' in video && duration > 0 && duration < 1801) {
+      // If the video has a transcript available, put a placehold in and
+      // enqueue a job to process the full transcript
+      const updatedContent = await addTranscriptToReadableContent(
+        libraryItem.originalUrl,
+        libraryItem.readableContent,
+        TRANSCRIPT_PLACEHOLDER_TEXT
       )
-      return
-    }
 
-    videoURL = new URL(libraryItem.originalUrl)
-    const videoId = videoIdFromYouTubeUrl(libraryItem.originalUrl)
-
-    if (!videoId) {
-      logger.warning('no video id for supplied youtube url', {
-        url: libraryItem.originalUrl,
-      })
-      return
-    }
-
-    let needsUpdate = false
-    const youtube = new YouTubeClient()
-    const video = await youtube.getVideo(videoId)
-    if (!video) {
-      logger.warning('no video found for youtube url', {
-        url: libraryItem.originalUrl,
-      })
-      return
-    }
-
-    if (video.description && libraryItem.description !== video.description) {
-      needsUpdate = true
-      libraryItem.description = video.description
-    }
-
-    let duration = -1
-    if ('duration' in video && video.duration > 0) {
-      needsUpdate = true
-      libraryItem.wordCount = calculateWordCount(video.duration)
-      duration = video.duration
-    }
-
-    if (video.uploadDate && !Number.isNaN(Date.parse(video.uploadDate))) {
-      needsUpdate = true
-      libraryItem.publishedAt = new Date(video.uploadDate)
-    }
-
-    if (
-      await findGrantedFeatureByName(
-        FeatureName.YouTubeTranscripts,
-        jobData.userId
-      )
-    ) {
-      if ('getTranscript' in video && duration > 0 && duration < 1801) {
-        // If the video has a transcript available, put a placehold in and
-        // enqueue a job to process the full transcript
-        const updatedContent = await addTranscriptPlaceholdReadableContent(
-          libraryItem.originalUrl,
-          libraryItem.readableContent
-        )
-
-        if (updatedContent) {
-          needsUpdate = true
-          libraryItem.readableContent = updatedContent
-        }
-
-        await enqueueProcessYouTubeTranscript({
-          videoId,
-          ...jobData,
-        })
+      if (updatedContent) {
+        updatedLibraryItem.readableContent = updatedContent
       }
-    }
 
-    if (needsUpdate) {
-      const updated = await authTrx(
-        async (t) => {
-          return t
-            .getRepository(LibraryItem)
-            .update(jobData.libraryItemId, libraryItem)
-        },
-        undefined,
-        jobData.userId
-      )
-      if (!updated) {
-        logger.warning('could not updated library item')
-      }
+      await enqueueProcessYouTubeTranscript({
+        videoId,
+        ...jobData,
+      })
     }
-  } catch (err) {
-    logger.warning('error getting youtube metadata: ', {
-      err,
-      jobData,
-      videoURL,
-    })
+  }
+
+  if (updatedLibraryItem !== {}) {
+    await updateLibraryItem(
+      jobData.libraryItemId,
+      updatedLibraryItem,
+      jobData.userId
+    )
   }
 }
 
@@ -426,79 +326,63 @@ export interface ProcessYouTubeTranscriptJobData {
 export const processYouTubeTranscript = async (
   jobData: ProcessYouTubeTranscriptJobData
 ) => {
-  try {
-    const libraryItem = await authTrx(
-      async (tx) =>
-        tx
-          .withRepository(libraryItemRepository)
-          .findById(jobData.libraryItemId),
-      undefined,
-      jobData.userId
+  const libraryItem = await findLibraryItemById(
+    jobData.libraryItemId,
+    jobData.userId,
+    {
+      select: ['id', 'originalUrl', 'readableContent', 'state'],
+    }
+  )
+  if (!libraryItem || libraryItem.state !== LibraryItemState.Succeeded) {
+    logger.info(
+      `Not ready to get YouTube metadata job state: ${
+        libraryItem?.state ?? 'null'
+      }`
     )
-    if (!libraryItem || libraryItem.state !== LibraryItemState.Succeeded) {
-      logger.info(
-        `Not ready to get YouTube metadata job state: ${
-          libraryItem?.state ?? 'null'
-        }`
-      )
-      return
+    return
+  }
+
+  const youtube = new YouTubeClient()
+  const video = await youtube.getVideo(jobData.videoId)
+  if (!video) {
+    logger.warning('no video found for youtube url', {
+      url: libraryItem.originalUrl,
+    })
+    return
+  }
+
+  let chapters: Chapter[] = []
+  if ('chapters' in video) {
+    chapters = video.chapters
+  }
+
+  let transcript: TranscriptProperties[] | undefined = undefined
+  if ('getTranscript' in video) {
+    transcript = await video.getTranscript()
+  }
+
+  if (transcript) {
+    if (chapters) {
+      transcript = addTranscriptChapters(chapters, transcript)
     }
+    const transcriptHTML = await createTranscriptHTML(
+      jobData.videoId,
+      transcript
+    )
+    const updatedContent = await addTranscriptToReadableContent(
+      libraryItem.originalUrl,
+      libraryItem.readableContent,
+      transcriptHTML
+    )
 
-    let needsUpdate = false
-    const youtube = new YouTubeClient()
-    const video = await youtube.getVideo(jobData.videoId)
-    if (!video) {
-      logger.warning('no video found for youtube url', {
-        url: libraryItem.originalUrl,
-      })
-      return
-    }
-
-    let chapters: Chapter[] = []
-    if ('chapters' in video) {
-      chapters = video.chapters
-    }
-
-    let transcript: TranscriptProperties[] | undefined = undefined
-    if ('getTranscript' in video) {
-      transcript = await video.getTranscript()
-    }
-
-    if (transcript) {
-      if (chapters) {
-        transcript = addTranscriptChapters(chapters, transcript)
-      }
-      const transcriptHTML = await createTranscriptHTML(
-        jobData.videoId,
-        transcript
-      )
-      const updatedContent = await addTranscriptToReadableContent(
-        libraryItem.originalUrl,
-        libraryItem.readableContent,
-        transcriptHTML
-      )
-
-      if (updatedContent) {
-        needsUpdate = true
-        libraryItem.readableContent = updatedContent
-      }
-    }
-
-    if (needsUpdate) {
-      const updated = await authTrx(
-        async (t) => {
-          return t
-            .getRepository(LibraryItem)
-            .update(jobData.libraryItemId, libraryItem)
+    if (updatedContent) {
+      await updateLibraryItem(
+        jobData.libraryItemId,
+        {
+          readableContent: updatedContent,
         },
-        undefined,
         jobData.userId
       )
-      if (!updated) {
-        logger.warning('could not updated library item')
-      }
     }
-  } catch (err) {
-    logger.warning('error getting youtube transcript: ', { err, jobData })
   }
 }

From f92c6ef5a19894c2369c11dfe679c25878aae804 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Thu, 16 May 2024 12:03:41 +0800
Subject: [PATCH 3/4] lint test file

---
 packages/api/test/resolvers/article.test.ts | 107 +++++++++++---------
 1 file changed, 59 insertions(+), 48 deletions(-)

diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts
index a9081b6cd..c81677a08 100644
--- a/packages/api/test/resolvers/article.test.ts
+++ b/packages/api/test/resolvers/article.test.ts
@@ -58,7 +58,7 @@ const archiveLink = async (authToken: string, linkId: string) => {
     setLinkArchived(
       input: {
         linkId: "${linkId}",
-        archived: ${true}
+        archived: true
       }
     ) {
       ... on ArchiveLinkSuccess {
@@ -239,11 +239,13 @@ const saveUrlQuery = (
           url: "${url}",
           source: "test",
           clientRequestId: "${generateFakeUuid()}",
-          state: ${state}
+          state: ${state ?? 'null'},
           labels: ${
             labels
-              ? '[' + labels.map((label) => `{ name: "${label}" }`) + ']'
-              : null
+              ? '[' +
+                labels.map((label) => `{ name: "${label}" }`).join(',') +
+                ']'
+              : 'null'
           }
         }
       ) {
@@ -264,7 +266,7 @@ const setBookmarkQuery = (articleId: string, bookmark: boolean) => {
       setBookmarkArticle(
         input: {
           articleID: "${articleId}",
-          bookmark: ${bookmark}
+          bookmark: ${String(bookmark)}
         }
       ) {
         ... on SetBookmarkArticleSuccess {
@@ -293,8 +295,8 @@ const saveArticleReadingProgressQuery = (
           id: "${articleId}",
           readingProgressPercent: ${progress},
           readingProgressAnchorIndex: 0,
-          readingProgressTopPercent: ${topPercent},
-          force: ${force}
+          readingProgressTopPercent: ${topPercent ?? 'null'},
+          force: ${String(force) ?? 'null'}
         }
       ) {
         ... on SaveArticleReadingProgressSuccess {
@@ -344,7 +346,7 @@ describe('Article API', () => {
       .post('/local/debug/fake-user-login')
       .send({ fakeEmail: user.email })
 
-    authToken = res.body.authToken
+    authToken = res.body.authToken as string
   })
 
   after(async () => {
@@ -360,7 +362,7 @@ describe('Article API', () => {
     let title = ''
     let itemId = ''
 
-    beforeEach(async () => {
+    beforeEach(() => {
       query = createArticleQuery(url, source, document, title)
     })
 
@@ -380,7 +382,7 @@ describe('Article API', () => {
         const res = await graphqlRequest(query, authToken).expect(200)
 
         expect(res.body.data.createArticle.createdArticle.title).to.eql(title)
-        itemId = res.body.data.createArticle.createdArticle.id
+        itemId = res.body.data.createArticle.createdArticle.id as string
       })
     })
 
@@ -462,7 +464,7 @@ describe('Article API', () => {
       await deleteLibraryItemById(itemId, user.id)
     })
 
-    beforeEach(async () => {
+    beforeEach(() => {
       query = getArticleQuery(slug)
     })
 
@@ -587,7 +589,7 @@ describe('Article API', () => {
           searchQuery('in:inbox'),
           authToken
         ).expect(200)
-        const justSavedId = allLinks.body.data.search.edges[0].node.id
+        const justSavedId = allLinks.body.data.search.edges[0].node.id as string
         await archiveLink(authToken, justSavedId)
 
         // test the negative case, ensuring the archive link wasn't returned
@@ -888,7 +890,7 @@ describe('Article API', () => {
     })
 
     context('when the file is not uploaded', () => {
-      before(async () => {
+      before(() => {
         url = 'fake url'
         uploadFileId = generateFakeUuid()
       })
@@ -963,7 +965,7 @@ describe('Article API', () => {
       }
     })
 
-    beforeEach(async () => {
+    beforeEach(() => {
       query = searchQuery(keyword)
     })
 
@@ -1039,7 +1041,7 @@ describe('Article API', () => {
     })
 
     context('when no:label is in the query', () => {
-      before(async () => {
+      before(() => {
         keyword = `'${searchedKeyword}' no:label`
       })
 
@@ -1051,7 +1053,7 @@ describe('Article API', () => {
     })
 
     context('when no:highlight is in the query', () => {
-      before(async () => {
+      before(() => {
         keyword = `'${searchedKeyword}' no:highlight`
       })
 
@@ -1063,7 +1065,7 @@ describe('Article API', () => {
     })
 
     context('when site:${site_name} is in the query', () => {
-      before(async () => {
+      before(() => {
         keyword = `'${searchedKeyword}' site:example`
       })
 
@@ -1268,7 +1270,7 @@ describe('Article API', () => {
     })
 
     context('when wildcard search for labels', () => {
-      let items: LibraryItem[] = []
+      const items: LibraryItem[] = []
       let labelIds: string[]
 
       before(async () => {
@@ -1278,32 +1280,41 @@ describe('Article API', () => {
         const label2 = await createLabel('test/two', '', user.id)
         labelIds = [label1.id, label2.id]
 
-        items = await createLibraryItems(
-          [
-            {
-              user,
-              title: 'test title wildcard',
-              readableContent: '<p>test wildcard</p>',
-              slug: 'test slug wildcard',
-              originalUrl: `${url}/wildcard`,
-            },
-            {
-              user,
-              title: 'test title wildcard 1',
-              readableContent: '<p>test wildcard</p>',
-              slug: 'test slug wildcard 1',
-              originalUrl: `${url}/wildcard_1`,
-            },
-            {
-              user,
-              title: 'test title wildcard 2',
-              readableContent: '<p>test wildcard</p>',
-              slug: 'test slug wildcard 2',
-              originalUrl: `${url}/wildcard_2`,
-            },
-          ],
-          user.id
-        )
+        const itemsToSave = [
+          {
+            user,
+            title: 'test title wildcard',
+            readableContent: '<p>test wildcard</p>',
+            slug: 'test slug wildcard',
+            originalUrl: `${url}/wildcard`,
+          },
+          {
+            user,
+            title: 'test title wildcard 1',
+            readableContent: '<p>test wildcard</p>',
+            slug: 'test slug wildcard 1',
+            originalUrl: `${url}/wildcard_1`,
+          },
+          {
+            user,
+            title: 'test title wildcard 2',
+            readableContent: '<p>test wildcard</p>',
+            slug: 'test slug wildcard 2',
+            originalUrl: `${url}/wildcard_2`,
+          },
+        ]
+
+        for (const item of itemsToSave) {
+          const savedItem = await createOrUpdateLibraryItem(
+            item,
+            user.id,
+            undefined,
+            true,
+            true
+          )
+          items.push(savedItem)
+        }
+
         await saveLabelsInLibraryItem([label1], items[0].id, user.id)
         await saveLabelsInLibraryItem([label2], items[1].id, user.id)
       })
@@ -1317,8 +1328,8 @@ describe('Article API', () => {
         const res = await graphqlRequest(query, authToken).expect(200)
 
         expect(res.body.data.search.pageInfo.totalCount).to.eq(2)
-        expect(res.body.data.search.edges[0].node.id).to.eq(items[0].id)
-        expect(res.body.data.search.edges[1].node.id).to.eq(items[1].id)
+        expect(res.body.data.search.edges[0].node.id).to.eq(items[1].id)
+        expect(res.body.data.search.edges[1].node.id).to.eq(items[0].id)
       })
     })
 
@@ -1999,7 +2010,7 @@ describe('Article API', () => {
       }
     })
 
-    beforeEach(async () => {
+    beforeEach(() => {
       query = typeaheadSearchQuery(keyword)
     })
 
@@ -2095,7 +2106,7 @@ describe('Article API', () => {
       ).expect(200)
 
       expect(
-        res.body.data.updatesSince.edges.filter(
+        (res.body.data.updatesSince.edges as SyncUpdatedItemEdge[]).filter(
           (e: SyncUpdatedItemEdge) => e.updateReason === UpdateReason.Deleted
         ).length
       ).to.eql(3)

From 293ed87100ffc63ff10f8a2b09fd09512ed3ee92 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Thu, 16 May 2024 12:21:11 +0800
Subject: [PATCH 4/4] remove redundant response from return value

---
 packages/puppeteer-parse/src/index.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/puppeteer-parse/src/index.ts b/packages/puppeteer-parse/src/index.ts
index b622cfd36..5772a7660 100644
--- a/packages/puppeteer-parse/src/index.ts
+++ b/packages/puppeteer-parse/src/index.ts
@@ -460,7 +460,7 @@ async function retrievePage(
     logRecord.finalUrl = finalUrl
     logRecord.contentType = contentType
 
-    return { context, page, response, finalUrl, contentType }
+    return { context, page, finalUrl, contentType }
   } catch (error) {
     if (lastPdfUrl) {
       return {