From b0f0dba53d2c32cbb562fd9972dd05c05ed6a22a Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 12 Oct 2023 10:39:35 +0800 Subject: [PATCH 01/19] drop position trigger on labels table and sort labels by name returned by labels api --- packages/api/src/resolvers/labels/index.ts | 2 +- .../0133.do.drop_position_trigger_ob_labels.sql | 10 ++++++++++ ...133.undo.drop_position_trigger_ob_labels.sql | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100755 packages/db/migrations/0133.do.drop_position_trigger_ob_labels.sql create mode 100755 packages/db/migrations/0133.undo.drop_position_trigger_ob_labels.sql diff --git a/packages/api/src/resolvers/labels/index.ts b/packages/api/src/resolvers/labels/index.ts index ef0687c04..27cd0ab25 100644 --- a/packages/api/src/resolvers/labels/index.ts +++ b/packages/api/src/resolvers/labels/index.ts @@ -54,7 +54,7 @@ export const labelsResolver = authorized( user: { id: uid }, }, order: { - position: 'ASC', + name: 'ASC', }, }) }) diff --git a/packages/db/migrations/0133.do.drop_position_trigger_ob_labels.sql b/packages/db/migrations/0133.do.drop_position_trigger_ob_labels.sql new file mode 100755 index 000000000..78d422202 --- /dev/null +++ b/packages/db/migrations/0133.do.drop_position_trigger_ob_labels.sql @@ -0,0 +1,10 @@ +-- Type: DO +-- Name: drop_position_trigger_ob_labels +-- Description: Drop increment_label_position and decrement_label_position trigger on omnivore.labels table + +BEGIN; + +DROP TRIGGER IF EXISTS increment_label_position ON omnivore.labels; +DROP TRIGGER IF EXISTS decrement_label_position ON omnivore.labels; + +COMMIT; diff --git a/packages/db/migrations/0133.undo.drop_position_trigger_ob_labels.sql b/packages/db/migrations/0133.undo.drop_position_trigger_ob_labels.sql new file mode 100755 index 000000000..6224a7284 --- /dev/null +++ b/packages/db/migrations/0133.undo.drop_position_trigger_ob_labels.sql @@ -0,0 +1,17 @@ +-- Type: UNDO +-- Name: drop_position_trigger_ob_labels +-- Description: Drop increment_label_position and decrement_label_position trigger on omnivore.labels table + +BEGIN; + +CREATE TRIGGER decrement_label_position + AFTER DELETE ON omnivore.labels + FOR EACH ROW +EXECUTE FUNCTION update_label_position(); + +CREATE TRIGGER increment_label_position + BEFORE INSERT ON omnivore.labels + FOR EACH ROW + EXECUTE FUNCTION update_label_position(); + +COMMIT; From f1c4bc6a745c5110b16aff92cfa5851302df2c5d Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 12 Oct 2023 10:46:12 +0800 Subject: [PATCH 02/19] fix email attachment not being saved as item --- packages/api/src/routers/svc/email_attachment.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/api/src/routers/svc/email_attachment.ts b/packages/api/src/routers/svc/email_attachment.ts index 361a5b16f..d73f1b52c 100644 --- a/packages/api/src/routers/svc/email_attachment.ts +++ b/packages/api/src/routers/svc/email_attachment.ts @@ -61,10 +61,10 @@ export function emailAttachmentRouter() { (tx) => tx.getRepository(UploadFile).save({ url: '', - userId: user.id, - fileName: fileName, + fileName, status: UploadFileStatus.Initialized, contentType: contentType, + user: { id: user.id }, }), undefined, user.id From ebb45f1973b52edf0666ad7241b8f2c4e9995861 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 12 Oct 2023 10:47:20 +0800 Subject: [PATCH 03/19] fix email attachment not being saved as item --- packages/api/src/routers/svc/email_attachment.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/api/src/routers/svc/email_attachment.ts b/packages/api/src/routers/svc/email_attachment.ts index d73f1b52c..b6c79b33d 100644 --- a/packages/api/src/routers/svc/email_attachment.ts +++ b/packages/api/src/routers/svc/email_attachment.ts @@ -150,7 +150,7 @@ export function emailAttachmentRouter() { ? PageType.File : PageType.Book const title = subject || uploadFileData.fileName - const articleToSave: DeepPartial = { + const itemToCreate: DeepPartial = { originalUrl: uploadFileUrlOverride, itemType, textContentHash: uploadFileHash, @@ -159,9 +159,10 @@ export function emailAttachmentRouter() { readableContent: '', slug: generateSlug(title), state: LibraryItemState.Succeeded, + user: { id: user.id }, } - const pageId = await createLibraryItem(articleToSave, user.id) + const pageId = await createLibraryItem(itemToCreate, user.id) // update received email type await updateReceivedEmail(receivedEmailId, 'article', user.id) From 1bc9912271834e43cd03c32685c83dd18cc160a9 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 12 Oct 2023 11:27:46 +0800 Subject: [PATCH 04/19] fix failure of saving highlight when position is null --- packages/api/src/entity/highlight.ts | 8 +++--- packages/api/src/resolvers/highlight/index.ts | 6 +++- packages/api/test/resolvers/highlight.test.ts | 28 ++++++++++++++++--- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/api/src/entity/highlight.ts b/packages/api/src/entity/highlight.ts index 811876b80..356c8bddc 100644 --- a/packages/api/src/entity/highlight.ts +++ b/packages/api/src/entity/highlight.ts @@ -59,11 +59,11 @@ export class Highlight { @Column('timestamp') sharedAt?: Date - @Column('real') - highlightPositionPercent?: number | null + @Column('real', { default: 0 }) + highlightPositionPercent!: number - @Column('integer') - highlightPositionAnchorIndex?: number | null + @Column('integer', { default: 0 }) + highlightPositionAnchorIndex!: number @Column('enum', { enum: HighlightType, diff --git a/packages/api/src/resolvers/highlight/index.ts b/packages/api/src/resolvers/highlight/index.ts index 0a0ca526e..4f54f7aa4 100644 --- a/packages/api/src/resolvers/highlight/index.ts +++ b/packages/api/src/resolvers/highlight/index.ts @@ -47,7 +47,9 @@ export const createHighlightResolver = authorized< ...input, user: { id: uid }, libraryItem: { id: input.articleId }, - highlightType: input.type as HighlightType, + highlightType: input.type || HighlightType.Highlight, + highlightPositionAnchorIndex: input.highlightPositionAnchorIndex || 0, + highlightPositionPercent: input.highlightPositionPercent || 0, }, input.articleId, uid, @@ -125,6 +127,8 @@ export const mergeHighlightResolver = authorized< color, user: { id: uid }, libraryItem: { id: input.articleId }, + highlightPositionAnchorIndex: input.highlightPositionAnchorIndex || 0, + highlightPositionPercent: input.highlightPositionPercent || 0, } const newHighlight = await mergeHighlights( diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts index 5ea84002e..6383f14d2 100644 --- a/packages/api/test/resolvers/highlight.test.ts +++ b/packages/api/test/resolvers/highlight.test.ts @@ -3,8 +3,10 @@ import { expect } from 'chai' import chaiString from 'chai-string' import 'mocha' import { User } from '../../src/entity/user' -import { createHighlight } from '../../src/services/highlights' -import { updateLibraryItem } from '../../src/services/library_item' +import { + createHighlight, + deleteHighlightById, +} from '../../src/services/highlights' import { deleteUser } from '../../src/services/user' import { createTestLibraryItem, createTestUser } from '../db' import { generateFakeUuid, graphqlRequest, request } from '../util' @@ -15,8 +17,8 @@ const createHighlightQuery = ( linkId: string, highlightId: string, shortHighlightId: string, - highlightPositionPercent = 0.0, - highlightPositionAnchorIndex = 0, + highlightPositionPercent: number | null = null, + highlightPositionAnchorIndex: number | null = null, annotation = '_annotation', html: string | null = null, prefix = '_prefix', @@ -182,6 +184,24 @@ describe('Highlights API', () => { expect(res.body.data.createHighlight.highlight.html).to.eq(html) }) + context('when highlight position is null', () => { + it('sets highlight position = 0', async () => { + const newHighlightId = generateFakeUuid() + const newShortHighlightId = '_short_id_5' + const query = createHighlightQuery( + itemId, + newHighlightId, + newShortHighlightId + ) + const res = await graphqlRequest(query, authToken).expect(200) + expect( + res.body.data.createHighlight.highlight.highlightPositionPercent + ).to.eq(0) + + await deleteHighlightById(newHighlightId) + }) + }) + context('when the annotation has HTML reserved characters', () => { it('unescapes the annotation and creates', async () => { const newHighlightId = generateFakeUuid() From 039632556b1b7f25901ead685d04d60696fdbba9 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 12 Oct 2023 11:56:54 +0800 Subject: [PATCH 05/19] add more method in mock cloud storage and update test cases for email-attachment rest api --- .../api/src/routers/svc/email_attachment.ts | 6 +-- packages/api/test/mock_storage.ts | 16 +++++++ ...ents.test.ts => email_attachments.test.ts} | 46 +++++++++++++------ 3 files changed, 51 insertions(+), 17 deletions(-) rename packages/api/test/routers/{pdf_attachments.test.ts => email_attachments.test.ts} (54%) diff --git a/packages/api/src/routers/svc/email_attachment.ts b/packages/api/src/routers/svc/email_attachment.ts index b6c79b33d..fde4be36d 100644 --- a/packages/api/src/routers/svc/email_attachment.ts +++ b/packages/api/src/routers/svc/email_attachment.ts @@ -63,7 +63,7 @@ export function emailAttachmentRouter() { url: '', fileName, status: UploadFileStatus.Initialized, - contentType: contentType, + contentType, user: { id: user.id }, }), undefined, @@ -162,12 +162,12 @@ export function emailAttachmentRouter() { user: { id: user.id }, } - const pageId = await createLibraryItem(itemToCreate, user.id) + const item = await createLibraryItem(itemToCreate, user.id) // update received email type await updateReceivedEmail(receivedEmailId, 'article', user.id) - res.send({ id: pageId }) + res.send({ id: item.id }) } catch (err) { logger.info(err) res.status(500).send(err) diff --git a/packages/api/test/mock_storage.ts b/packages/api/test/mock_storage.ts index 00415ab6b..57d0aaf2b 100644 --- a/packages/api/test/mock_storage.ts +++ b/packages/api/test/mock_storage.ts @@ -38,6 +38,22 @@ class MockFile { createWriteStream() { return new MockWriteStream(this) } + + getSignedUrl() { + return ['https://signed-url.upload.omnivore.app'] + } + + getMetadata() { + return [{ md5Hash: 'md5Hash' }] + } + + publicUrl() { + return 'https://public-url.upload.omnivore.app' + } + + makePublic() { + return + } } class MockWriteStream extends Writable { diff --git a/packages/api/test/routers/pdf_attachments.test.ts b/packages/api/test/routers/email_attachments.test.ts similarity index 54% rename from packages/api/test/routers/pdf_attachments.test.ts rename to packages/api/test/routers/email_attachments.test.ts index 80a92be04..f699b6a55 100644 --- a/packages/api/test/routers/pdf_attachments.test.ts +++ b/packages/api/test/routers/email_attachments.test.ts @@ -1,15 +1,19 @@ +import { Storage } from '@google-cloud/storage' import { expect } from 'chai' import * as jwt from 'jsonwebtoken' import 'mocha' +import sinon from 'sinon' +import { NewsletterEmail } from '../../src/entity/newsletter_email' import { User } from '../../src/entity/user' +import { getRepository } from '../../src/repository' import { findLibraryItemById } from '../../src/services/library_item' -import { createNewsletterEmail } from '../../src/services/newsletters' import { deleteUser } from '../../src/services/user' import { createTestUser } from '../db' +import { MockBucket } from '../mock_storage' import { request } from '../util' -describe('PDF attachments Router', () => { - const newsletterEmail = 'fakeEmail@omnivore.app' +describe('Email attachments Router', () => { + const newsletterEmailAddress = 'fakeEmail@omnivore.app' let user: User let authToken: string @@ -18,25 +22,38 @@ describe('PDF attachments Router', () => { // create test user and login user = await createTestUser('fakeUser') - await createNewsletterEmail(user.id, newsletterEmail) - authToken = jwt.sign(newsletterEmail, process.env.JWT_SECRET || '') + await getRepository(NewsletterEmail).save({ + address: newsletterEmailAddress, + user: { id: user.id }, + }) + authToken = jwt.sign(newsletterEmailAddress, process.env.JWT_SECRET || '') + + // mock cloud storage + const mockBucket = new MockBucket('test') + sinon.replace( + Storage.prototype, + 'bucket', + sinon.fake.returns(mockBucket as never) + ) }) after(async () => { // clean up await deleteUser(user.id) + sinon.restore() }) describe('upload', () => { - xit('create upload file request and return id and url', async () => { + it('create upload file request and return id and url', async () => { const testFile = 'testFile.pdf' const res = await request - .post('/svc/pdf-attachments/upload') + .post('/svc/email-attachment/upload') .set('Authorization', `${authToken}`) .send({ - email: newsletterEmail, + email: newsletterEmailAddress, fileName: testFile, + contentType: 'application/pdf', }) .expect(200) @@ -52,22 +69,23 @@ describe('PDF attachments Router', () => { // upload file first const testFile = 'testFile.pdf' const res = await request - .post('/svc/pdf-attachments/upload') + .post('/svc/email-attachment/upload') .set('Authorization', `${authToken}`) .send({ - email: newsletterEmail, + email: newsletterEmailAddress, fileName: testFile, + contentType: 'application/pdf', }) uploadFileId = res.body.id }) - xit('create article with uploaded file id and url', async () => { + it('create article with uploaded file id and url', async () => { // create article const res2 = await request - .post('/svc/pdf-attachments/create-article') + .post('/svc/email-attachment/create-article') .send({ - email: newsletterEmail, - uploadFileId: uploadFileId, + email: newsletterEmailAddress, + uploadFileId, }) .set('Authorization', `${authToken}`) .expect(200) From 4d64231abf02f1fca25af7aefcc963c4f7395025 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 12 Oct 2023 12:59:36 +0800 Subject: [PATCH 06/19] save content reader in the item --- packages/api/src/routers/svc/email_attachment.ts | 10 +++++++++- packages/api/test/routers/email_attachments.test.ts | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/api/src/routers/svc/email_attachment.ts b/packages/api/src/routers/svc/email_attachment.ts index fde4be36d..b9bf3c77c 100644 --- a/packages/api/src/routers/svc/email_attachment.ts +++ b/packages/api/src/routers/svc/email_attachment.ts @@ -1,6 +1,10 @@ import express from 'express' import { DeepPartial } from 'typeorm' -import { LibraryItem, LibraryItemState } from '../../entity/library_item' +import { + ContentReaderType, + LibraryItem, + LibraryItemState, +} from '../../entity/library_item' import { UploadFile } from '../../entity/upload_file' import { env } from '../../env' import { PageType, UploadFileStatus } from '../../generated/graphql' @@ -160,6 +164,10 @@ export function emailAttachmentRouter() { slug: generateSlug(title), state: LibraryItemState.Succeeded, user: { id: user.id }, + contentReader: + itemType === PageType.File + ? ContentReaderType.PDF + : ContentReaderType.EPUB, } const item = await createLibraryItem(itemToCreate, user.id) diff --git a/packages/api/test/routers/email_attachments.test.ts b/packages/api/test/routers/email_attachments.test.ts index f699b6a55..79c0f6fd0 100644 --- a/packages/api/test/routers/email_attachments.test.ts +++ b/packages/api/test/routers/email_attachments.test.ts @@ -94,6 +94,7 @@ describe('Email attachments Router', () => { const item = await findLibraryItemById(res2.body.id, user.id) expect(item).to.exist + expect(item?.contentReader).to.eq('PDF') }) }) }) From 5aff626c84c08a80cf33dbcfee5976100684c824 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Thu, 12 Oct 2023 15:11:07 +0800 Subject: [PATCH 07/19] Send highlight positions on Android --- android/Omnivore/app/build.gradle | 4 ++-- .../src/main/graphql/ArticleContent.graphql | 2 ++ .../dataService/HighlightActionHandlers.kt | 10 +++++--- .../omnivore/dataService/LibrarySync.kt | 5 ++-- .../omnivore/networking/HighlightMutations.kt | 14 +++++++---- .../omnivore/networking/SavedItemQuery.kt | 4 +++- .../omnivore/networking/SearchQuery.kt | 6 ++--- .../omnivore/persistence/AppDatabase.kt | 2 +- .../persistence/entities/Highlight.kt | 5 ++-- .../omnivore/omnivore/ui/reader/WebReader.kt | 24 +++++++++---------- 10 files changed, 46 insertions(+), 30 deletions(-) diff --git a/android/Omnivore/app/build.gradle b/android/Omnivore/app/build.gradle index 0b03709cc..e525d3c63 100644 --- a/android/Omnivore/app/build.gradle +++ b/android/Omnivore/app/build.gradle @@ -17,8 +17,8 @@ android { applicationId "app.omnivore.omnivore" minSdk 26 targetSdk 33 - versionCode 102 - versionName "0.0.102" + versionCode 110 + versionName "0.0.110" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" vectorDrawables { diff --git a/android/Omnivore/app/src/main/graphql/ArticleContent.graphql b/android/Omnivore/app/src/main/graphql/ArticleContent.graphql index 83ed8da7b..3070ab101 100644 --- a/android/Omnivore/app/src/main/graphql/ArticleContent.graphql +++ b/android/Omnivore/app/src/main/graphql/ArticleContent.graphql @@ -57,6 +57,8 @@ fragment HighlightFields on Highlight { updatedAt sharedAt color + highlightPositionPercent + highlightPositionAnchorIndex } fragment LabelFields on Label { diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/dataService/HighlightActionHandlers.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/dataService/HighlightActionHandlers.kt index d92975f89..6e9492aea 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/dataService/HighlightActionHandlers.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/dataService/HighlightActionHandlers.kt @@ -1,12 +1,10 @@ package app.omnivore.omnivore.dataService -import app.omnivore.omnivore.graphql.generated.type.CreateHighlightInput import app.omnivore.omnivore.graphql.generated.type.HighlightType import app.omnivore.omnivore.models.ServerSyncStatus import app.omnivore.omnivore.networking.* import app.omnivore.omnivore.persistence.entities.Highlight import app.omnivore.omnivore.persistence.entities.SavedItemAndHighlightCrossRef -import com.apollographql.apollo3.api.Optional import com.google.gson.Gson import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -29,6 +27,8 @@ suspend fun DataService.createWebHighlight(jsonString: String, colorName: String updatedAt = null, createdByMe = false, color = colorName ?: createHighlightInput.color.getOrNull(), + highlightPositionPercent = createHighlightInput.highlightPositionPercent.getOrNull() ?: 0.0, + highlightPositionAnchorIndex = createHighlightInput.highlightPositionAnchorIndex.getOrNull() ?: 0 ) highlight.serverSyncStatus = ServerSyncStatus.NEEDS_CREATION.rawValue @@ -66,7 +66,9 @@ suspend fun DataService.createNoteHighlight(savedItemId: String, note: String): createdAt = null, updatedAt = null, createdByMe = true, - color = null + color = null, + highlightPositionAnchorIndex = 0, + highlightPositionPercent = 0.0 ) highlight.serverSyncStatus = ServerSyncStatus.NEEDS_CREATION.rawValue @@ -87,6 +89,8 @@ suspend fun DataService.createNoteHighlight(savedItemId: String, note: String): quote = null, patch = null, annotation = note, + highlightPositionAnchorIndex = 0, + highlightPositionPercent = 0.0 ).asCreateHighlightInput()) newHighlight?.let { diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/dataService/LibrarySync.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/dataService/LibrarySync.kt index f914190ba..44a0dbaf4 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/dataService/LibrarySync.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/dataService/LibrarySync.kt @@ -1,7 +1,6 @@ package app.omnivore.omnivore.dataService import android.util.Log -import androidx.room.PrimaryKey import app.omnivore.omnivore.models.ServerSyncStatus import app.omnivore.omnivore.networking.* import app.omnivore.omnivore.persistence.entities.* @@ -85,7 +84,9 @@ suspend fun DataService.sync(since: String, cursor: String?, limit: Int = 20): S suffix = highlight.highlightFields.suffix, createdAt = null, updatedAt = highlight.highlightFields.updatedAt as String?, - color = highlight.highlightFields.color + color = highlight.highlightFields.color, + highlightPositionPercent = highlight.highlightFields.highlightPositionPercent, + highlightPositionAnchorIndex = highlight.highlightFields.highlightPositionAnchorIndex, ) } ?: listOf() SavedItemWithLabelsAndHighlights( diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/HighlightMutations.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/HighlightMutations.kt index b90f1708c..2bc7033d4 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/HighlightMutations.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/HighlightMutations.kt @@ -20,7 +20,9 @@ data class CreateHighlightParams( val quote: String?, val patch: String?, val articleId: String?, - val `annotation`: String? + val `annotation`: String?, + val highlightPositionAnchorIndex: Int, + val highlightPositionPercent: Double ) { fun asCreateHighlightInput() = CreateHighlightInput( type = Optional.presentIfNotNull(type), @@ -29,7 +31,9 @@ data class CreateHighlightParams( id = id ?: "", patch = Optional.presentIfNotNull(patch), quote = Optional.presentIfNotNull(quote), - shortId = shortId ?: "" + shortId = shortId ?: "", + highlightPositionAnchorIndex = Optional.presentIfNotNull(highlightPositionAnchorIndex), + highlightPositionPercent = Optional.presentIfNotNull(highlightPositionPercent) ) } @@ -151,8 +155,10 @@ suspend fun Networker.createHighlight(input: CreateHighlightInput): Highlight? { createdAt = createdHighlight.highlightFields.createdAt.toString(), updatedAt = createdHighlight.highlightFields.updatedAt.toString(), createdByMe = createdHighlight.highlightFields.createdByMe, - color = createdHighlight.highlightFields.color - ) + color = createdHighlight.highlightFields.color, + highlightPositionPercent = createdHighlight.highlightFields.highlightPositionPercent, + highlightPositionAnchorIndex = createdHighlight.highlightFields.highlightPositionAnchorIndex + ) } else { return null } diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SavedItemQuery.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SavedItemQuery.kt index 30016cfcb..2fd8e9298 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SavedItemQuery.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SavedItemQuery.kt @@ -54,7 +54,9 @@ suspend fun Networker.savedItem(slug: String): SavedItemQueryResponse { createdAt = it.highlightFields.createdAt as String?, updatedAt = it.highlightFields.updatedAt as String?, createdByMe = it.highlightFields.createdByMe, - color = it.highlightFields.color + color = it.highlightFields.color, + highlightPositionPercent = it.highlightFields.highlightPositionPercent, + highlightPositionAnchorIndex = it.highlightFields.highlightPositionAnchorIndex ) } diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SearchQuery.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SearchQuery.kt index 52cfbd19a..018dce283 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SearchQuery.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/networking/SearchQuery.kt @@ -1,8 +1,6 @@ package app.omnivore.omnivore.networking -import androidx.room.PrimaryKey import app.omnivore.omnivore.graphql.generated.SearchQuery -import app.omnivore.omnivore.graphql.generated.TypeaheadSearchQuery import app.omnivore.omnivore.models.ServerSyncStatus import app.omnivore.omnivore.persistence.entities.* import com.apollographql.apollo3.api.Optional @@ -82,7 +80,9 @@ suspend fun Networker.search( suffix = highlight.highlightFields.suffix, updatedAt = highlight.highlightFields.updatedAt as String?, createdAt = highlight.highlightFields.createdAt as String?, - color = highlight.highlightFields.color + color = highlight.highlightFields.color, + highlightPositionPercent = highlight.highlightFields.highlightPositionPercent, + highlightPositionAnchorIndex = highlight.highlightFields.highlightPositionAnchorIndex ) } ) diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/AppDatabase.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/AppDatabase.kt index ac39dbbc4..07202993f 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/AppDatabase.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/AppDatabase.kt @@ -13,7 +13,7 @@ import app.omnivore.omnivore.persistence.entities.* SavedItemAndSavedItemLabelCrossRef::class, SavedItemAndHighlightCrossRef::class ], - version = 9 + version = 11 ) abstract class AppDatabase : RoomDatabase() { abstract fun viewerDao(): ViewerDao diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/entities/Highlight.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/entities/Highlight.kt index c26d5224b..8d133ad89 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/entities/Highlight.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/persistence/entities/Highlight.kt @@ -1,6 +1,5 @@ package app.omnivore.omnivore.persistence.entities -import androidx.lifecycle.LiveData import androidx.room.* import app.omnivore.omnivore.models.ServerSyncStatus import com.google.gson.annotations.SerializedName @@ -22,7 +21,9 @@ data class Highlight( var shortId: String, val suffix: String?, val updatedAt: String?, - val color: String? + val color: String?, + val highlightPositionPercent: Double?, + val highlightPositionAnchorIndex: Int? ) @Entity( diff --git a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/reader/WebReader.kt b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/reader/WebReader.kt index 8a1372802..1387f57bb 100644 --- a/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/reader/WebReader.kt +++ b/android/Omnivore/app/src/main/java/app/omnivore/omnivore/ui/reader/WebReader.kt @@ -152,18 +152,18 @@ fun WebReader( webReaderViewModel.resetJavascriptDispatchQueue() } }) - if (showHighlightColorPalette.value == true) { - HighlightColorPalette( - mode = if (isDarkMode) HighlightColorPaletteMode.Dark else HighlightColorPaletteMode.Light, - selectedColorName = highlightColor.value?.name ?: "yellow", - onColorSelected = { - webReaderViewModel.setHighlightColor(it) - }, - modifier = Modifier - .align(Alignment.BottomCenter) - .padding(12.dp, 12.dp, 12.dp, 36.dp) - ) - } +// if (showHighlightColorPalette.value == true) { +// HighlightColorPalette( +// mode = if (isDarkMode) HighlightColorPaletteMode.Dark else HighlightColorPaletteMode.Light, +// selectedColorName = highlightColor.value?.name ?: "yellow", +// onColorSelected = { +// webReaderViewModel.setHighlightColor(it) +// }, +// modifier = Modifier +// .align(Alignment.BottomCenter) +// .padding(12.dp, 12.dp, 12.dp, 36.dp) +// ) +// } } } From a6c5f4e8623ff0f5f0a2d287bceb13d1e29db60a Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 13 Oct 2023 13:45:09 +0800 Subject: [PATCH 08/19] fix bulk action slow query --- packages/api/src/resolvers/article/index.ts | 12 ++-- packages/api/src/services/library_item.ts | 62 +++++++++------------ packages/api/test/resolvers/article.test.ts | 16 ++++++ 3 files changed, 48 insertions(+), 42 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 1c191d0ec..af551b85e 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -5,7 +5,6 @@ /* eslint-disable @typescript-eslint/no-floating-promises */ import { Readability } from '@omnivore/readability' import graphqlFields from 'graphql-fields' -import { Not } from 'typeorm' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { LibraryItem, LibraryItemState } from '../../entity/library_item' import { env } from '../../env' @@ -792,6 +791,12 @@ export const bulkActionResolver = authorized< }, }) + // parse query + const searchQuery = parseSearchQuery(query) + if (searchQuery.ids.length > 100) { + return { errorCodes: [BulkActionErrorCode.BadRequest] } + } + // get labels if needed let labels = undefined if (action === BulkActionType.AddLabels) { @@ -802,10 +807,7 @@ export const bulkActionResolver = authorized< labels = await findLabelsByIds(labelIds, uid) } - // parse query - const searchQuery = parseSearchQuery(query) - - await updateLibraryItems(action, searchQuery, labels) + await updateLibraryItems(action, searchQuery, uid, labels) return { success: true } } catch (error) { diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index 49a363acf..628873cfb 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -1,13 +1,4 @@ -import { - Between, - DeepPartial, - In, - IsNull, - LessThan, - MoreThan, - Not, - SelectQueryBuilder, -} from 'typeorm' +import { DeepPartial, SelectQueryBuilder } from 'typeorm' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { EntityLabel } from '../entity/entity_label' import { Highlight } from '../entity/highlight' @@ -115,14 +106,10 @@ const buildWhereClause = ( if (args.inFilter !== InFilter.ALL) { switch (args.inFilter) { case InFilter.INBOX: - queryBuilder.andWhere({ - archivedAt: IsNull(), - }) + queryBuilder.andWhere('library_item.archived_at IS NULL') break case InFilter.ARCHIVE: - queryBuilder.andWhere({ - archivedAt: Not(IsNull()), - }) + queryBuilder.andWhere('library_item.archived_at IS NOT NULL') break case InFilter.TRASH: // return only deleted pages within 14 days @@ -133,19 +120,15 @@ const buildWhereClause = ( case InFilter.SUBSCRIPTION: queryBuilder .andWhere("NOT ('library' ILIKE ANY (library_item.label_names))") - .andWhere({ - subscription: Not(IsNull()), - archivedAt: IsNull(), - }) + .andWhere('library_item.archived_at IS NULL') + .andWhere('library_item.subscription IS NOT NULL') break case InFilter.LIBRARY: queryBuilder .andWhere( "(library_item.subscription IS NULL OR 'library' ILIKE ANY (library_item.label_names))" ) - .andWhere({ - archivedAt: IsNull(), - }) + .andWhere('library_item.archived_at IS NULL') break } } @@ -153,17 +136,19 @@ const buildWhereClause = ( if (args.readFilter !== ReadFilter.ALL) { switch (args.readFilter) { case ReadFilter.READ: - queryBuilder.andWhere({ - readingProgressBottomPercent: MoreThan(98), - }) + queryBuilder.andWhere( + 'library_item.reading_progress_bottom_percent > 98' + ) break case ReadFilter.READING: - queryBuilder.andWhere({ readingProgressBottomPercent: Between(2, 98) }) + queryBuilder.andWhere( + 'library_item.reading_progress_bottom_percent BETWEEN 2 AND 98' + ) break case ReadFilter.UNREAD: - queryBuilder.andWhere({ - readingProgressBottomPercent: LessThan(2), - }) + queryBuilder.andWhere( + 'library_item.reading_progress_bottom_percent < 2' + ) break } } @@ -247,20 +232,20 @@ const buildWhereClause = ( } if (args.ids && args.ids.length > 0) { - queryBuilder.andWhere({ - id: In(args.ids), + queryBuilder.andWhere('library_item.id = ANY(:ids)', { + ids: args.ids, }) } if (!args.includePending) { - queryBuilder.andWhere({ - state: Not(LibraryItemState.Processing), + queryBuilder.andWhere('library_item.state <> :state', { + state: LibraryItemState.Processing, }) } if (!args.includeDeleted && args.inFilter !== InFilter.TRASH) { - queryBuilder.andWhere({ - state: Not(LibraryItemState.Deleted), + queryBuilder.andWhere('library_item.state <> :state', { + state: LibraryItemState.Deleted, }) } @@ -528,6 +513,7 @@ export const countByCreatedAt = async ( export const updateLibraryItems = async ( action: BulkActionType, args: SearchArgs, + userId: string, labels?: Label[] ) => { // build the script @@ -561,7 +547,9 @@ export const updateLibraryItems = async ( } await authTrx(async (tx) => { - const queryBuilder = tx.createQueryBuilder(LibraryItem, 'library_item') + const queryBuilder = tx + .createQueryBuilder(LibraryItem, 'library_item') + .where('library_item.user_id = :userId', { userId }) // build the where clause buildWhereClause(queryBuilder, args) diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index d8c2545d9..aeb76604a 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -1545,6 +1545,22 @@ describe('Article API', () => { await deleteLibraryItemsByUserId(user.id) }) + context('when action is MarkAsRead and query is in:unread', () => { + it('marks unread items as read', async () => { + const res = await graphqlRequest( + bulkActionQuery(BulkActionType.MarkAsRead, 'is:unread'), + authToken + ).expect(200) + expect(res.body.data.bulkAction.success).to.be.true + + const items = await graphqlRequest( + searchQuery('is:unread'), + authToken + ).expect(200) + expect(items.body.data.search.pageInfo.totalCount).to.eql(0) + }) + }) + context('when action is Archive', () => { it('archives all items', async () => { const res = await graphqlRequest( From f960379783d3720bd7a72e0695649a1d2cb0b898 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Fri, 13 Oct 2023 17:50:38 +0800 Subject: [PATCH 09/19] Allow navigating to next/previous items on the web with keyboard --- .../templates/article/ArticleActionsMenu.tsx | 2 +- .../templates/homeFeed/HomeFeedContainer.tsx | 9 +++ .../web/pages/[username]/[slug]/index.tsx | 62 +++++++++++++++++-- 3 files changed, 66 insertions(+), 7 deletions(-) diff --git a/packages/web/components/templates/article/ArticleActionsMenu.tsx b/packages/web/components/templates/article/ArticleActionsMenu.tsx index c71c65700..201dd9d4d 100644 --- a/packages/web/components/templates/article/ArticleActionsMenu.tsx +++ b/packages/web/components/templates/article/ArticleActionsMenu.tsx @@ -180,7 +180,7 @@ export function ArticleActionsMenu( ) : (