From c5a97c7a4123771b466247bdc5601841e1e51d84 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 17 Nov 2023 13:51:33 +0800 Subject: [PATCH] change archive and trash from folder to state --- packages/api/src/resolvers/article/index.ts | 16 ++++--- packages/api/src/resolvers/links/index.ts | 21 ++++++-- packages/api/src/resolvers/update/index.ts | 4 ++ packages/api/src/services/library_item.ts | 48 +++++++++++++++---- packages/api/src/services/save_page.ts | 6 ++- packages/api/src/utils/helpers.ts | 4 +- packages/api/test/resolvers/article.test.ts | 28 ++++++----- .../0148.do.update_folder_in_library_item.sql | 9 ++++ ...148.undo.update_folder_in_library_item.sql | 10 ++++ 9 files changed, 111 insertions(+), 35 deletions(-) create mode 100755 packages/db/migrations/0148.do.update_folder_in_library_item.sql create mode 100755 packages/db/migrations/0148.undo.update_folder_in_library_item.sql diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index b9de1f73c..7ac91c69c 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -5,8 +5,9 @@ /* eslint-disable @typescript-eslint/no-floating-promises */ import { Readability } from '@omnivore/readability' import graphqlFields from 'graphql-fields' +import { IsNull } from 'typeorm' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' -import { LibraryItem } from '../../entity/library_item' +import { LibraryItem, LibraryItemState } from '../../entity/library_item' import { env } from '../../env' import { ArticleError, @@ -390,7 +391,10 @@ export const getArticleResolver = authorized< const libraryItem = await authTrx((tx) => tx.withRepository(libraryItemRepository).findOne({ select: selectColumns, - where, + where: { + ...where, + deletedAt: IsNull(), + }, relations: { labels: true, highlights: { @@ -406,7 +410,7 @@ export const getArticleResolver = authorized< }) ) - if (!libraryItem || libraryItem.folder === InFilter.TRASH) { + if (!libraryItem) { return { errorCodes: [ArticleErrorCode.NotFound] } } @@ -528,8 +532,8 @@ export const setBookmarkArticleResolver = authorized< const deletedLibraryItem = await updateLibraryItem( articleID, { - folder: InFilter.TRASH, - savedAt: new Date(), + state: LibraryItemState.Deleted, + deletedAt: new Date(), }, uid, pubsub @@ -867,7 +871,7 @@ export const setFavoriteArticleResolver = authorized< }) const getUpdateReason = (libraryItem: LibraryItem, since: Date) => { - if (libraryItem.folder === InFilter.TRASH) { + if (libraryItem.deletedAt) { return UpdateReason.Deleted } if (libraryItem.createdAt >= since) { diff --git a/packages/api/src/resolvers/links/index.ts b/packages/api/src/resolvers/links/index.ts index d16b9a155..8950222d1 100644 --- a/packages/api/src/resolvers/links/index.ts +++ b/packages/api/src/resolvers/links/index.ts @@ -1,3 +1,4 @@ +import { LibraryItemState } from '../../entity/library_item' import { env } from '../../env' import { ArchiveLinkError, @@ -8,7 +9,6 @@ import { import { updateLibraryItem } from '../../services/library_item' import { analytics } from '../../utils/analytics' import { authorized } from '../../utils/helpers' -import { InFilter } from '../../utils/search' // export const updateLinkShareInfoResolver = authorized< // UpdateLinkShareInfoSuccess, @@ -54,9 +54,20 @@ export const setLinkArchivedResolver = authorized< ArchiveLinkError, MutationSetLinkArchivedArgs >(async (_obj, args, { uid }) => { + let state = LibraryItemState.Archived + let archivedAt: Date | null = new Date() + let event = 'link_archived' + + const isUnarchive = !args.input.archived + if (isUnarchive) { + state = LibraryItemState.Succeeded + archivedAt = null + event = 'link_unarchived' + } + analytics.track({ userId: uid, - event: args.input.archived ? 'link_archived' : 'link_unarchived', + event, properties: { env: env.server.apiEnv, }, @@ -66,8 +77,8 @@ export const setLinkArchivedResolver = authorized< await updateLibraryItem( args.input.linkId, { - savedAt: new Date(), - folder: args.input.archived ? InFilter.ARCHIVE : InFilter.INBOX, + state, + archivedAt, }, uid ) @@ -80,6 +91,6 @@ export const setLinkArchivedResolver = authorized< return { linkId: args.input.linkId, - message: 'Link Archived', + message: event, } }) diff --git a/packages/api/src/resolvers/update/index.ts b/packages/api/src/resolvers/update/index.ts index 10eb68d32..255efd2b5 100644 --- a/packages/api/src/resolvers/update/index.ts +++ b/packages/api/src/resolvers/update/index.ts @@ -1,3 +1,4 @@ +import { LibraryItemState } from '../../entity/library_item' import { MutationUpdatePageArgs, UpdatePageError, @@ -20,6 +21,9 @@ export const updatePageResolver = authorized< savedAt: input.savedAt ? new Date(input.savedAt) : undefined, publishedAt: input.publishedAt ? new Date(input.publishedAt) : undefined, thumbnail: input.previewImage ?? undefined, + state: input.state + ? (input.state as unknown as LibraryItemState) + : undefined, }, uid ) diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index 02d60e1d0..9b6f9b4e1 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -105,9 +105,24 @@ const buildWhereClause = ( } if (args.inFilter !== InFilter.ALL) { - queryBuilder.andWhere('library_item.folder = :folder', { - folder: args.inFilter, - }) + switch (args.inFilter) { + case InFilter.INBOX: + queryBuilder.andWhere('library_item.archived_at IS NULL') + break + case InFilter.ARCHIVE: + queryBuilder.andWhere('library_item.archived_at IS NOT NULL') + break + case InFilter.TRASH: + // return only deleted pages within 14 days + queryBuilder.andWhere( + "library_item.deleted_at >= now() - interval '14 days'" + ) + break + default: + queryBuilder.andWhere('library_item.folder = :folder', { + folder: args.inFilter, + }) + } } if (args.readFilter !== ReadFilter.ALL) { @@ -227,7 +242,7 @@ const buildWhereClause = ( } if (!args.includeDeleted && args.inFilter !== InFilter.TRASH) { - queryBuilder.andWhere("library_item.folder <> 'trash'") + queryBuilder.andWhere("library_item.state <> 'DELETED'") } if (args.noFilters) { @@ -365,6 +380,8 @@ export const restoreLibraryItem = async ( { state: LibraryItemState.Succeeded, savedAt: new Date(), + archivedAt: null, + deletedAt: null, }, userId, pubsub @@ -382,6 +399,21 @@ export const updateLibraryItem = async ( const itemRepo = tx.withRepository(libraryItemRepository) await itemRepo.update(id, libraryItem) + // reset deletedAt and archivedAt + switch (libraryItem.state) { + case LibraryItemState.Archived: + libraryItem.archivedAt = new Date() + break + case LibraryItemState.Deleted: + libraryItem.deletedAt = new Date() + break + case LibraryItemState.Processing: + case LibraryItemState.Succeeded: + libraryItem.archivedAt = null + libraryItem.deletedAt = null + break + } + return itemRepo.findOneByOrFail({ id }) }, undefined, @@ -600,14 +632,14 @@ export const updateLibraryItems = async ( switch (action) { case BulkActionType.Archive: values = { - folder: InFilter.ARCHIVE, - savedAt: new Date(), + archivedAt: new Date(), + state: LibraryItemState.Archived, } break case BulkActionType.Delete: values = { - savedAt: new Date(), - folder: InFilter.TRASH, + state: LibraryItemState.Deleted, + deletedAt: new Date(), } break case BulkActionType.AddLabels: diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index 5ee1b2708..14347bde8 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -261,6 +261,9 @@ export const parsedContentToLibraryItem = ({ uploadFileId: uploadFileId || undefined, readingProgressTopPercent: 0, readingProgressHighestReadAnchor: 0, + state: state + ? (state as unknown as LibraryItemState) + : LibraryItemState.Succeeded, createdAt: validatedDate(saveTime), savedAt: validatedDate(saveTime), siteName: parsedContent?.siteName, @@ -269,7 +272,6 @@ export const parsedContentToLibraryItem = ({ wordCount: wordsCount(parsedContent?.textContent || ''), contentReader: contentReaderForLibraryItem(itemType, uploadFileId), subscription: rssFeedUrl, - folder: state === ArticleSavingRequestStatus.Archived ? 'archive' : 'inbox', - state: LibraryItemState.Succeeded, + folder: 'inbox', } } diff --git a/packages/api/src/utils/helpers.ts b/packages/api/src/utils/helpers.ts index 80a883be7..fedfe34f0 100644 --- a/packages/api/src/utils/helpers.ts +++ b/packages/api/src/utils/helpers.ts @@ -240,7 +240,7 @@ export const libraryItemToArticle = (item: LibraryItem): Article => ({ state: item.state as unknown as ArticleSavingRequestStatus, content: item.readableContent, hash: item.textContentHash || '', - isArchived: item.folder === InFilter.ARCHIVE, + isArchived: !!item.archivedAt, recommendations: item.recommendations?.map( recommandationDataToRecommendation ), @@ -259,7 +259,7 @@ export const libraryItemToSearchItem = (item: LibraryItem): SearchItem => ({ url: item.originalUrl, state: item.state as unknown as ArticleSavingRequestStatus, content: item.readableContent, - isArchived: item.folder === InFilter.ARCHIVE, + isArchived: !!item.archivedAt, pageType: item.itemType as unknown as PageType, readingProgressPercent: item.readingProgressBottomPercent, contentReader: item.contentReader as unknown as ContentReader, diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index 6fc8f5f88..6227f2cd9 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -410,7 +410,8 @@ describe('Article API', () => { title, user: { id: user.id }, originalUrl: url, - folder: 'archive', + archivedAt: new Date(), + state: LibraryItemState.Archived, }, user.id ) @@ -608,7 +609,7 @@ describe('Article API', () => { ).expect(200) const savedItem = await findLibraryItemByUrl(url, user.id) - expect(savedItem?.folder).to.eql('archive') + expect(savedItem?.archivedAt).to.not.be.null expect(savedItem?.labels?.map((l) => l.name)).to.eql(labels) }) }) @@ -1030,7 +1031,8 @@ describe('Article API', () => { readableContent: '

test 1

', slug: 'test slug 1', originalUrl: `${url}/test1`, - folder: 'archive', + archivedAt: new Date(), + state: LibraryItemState.Archived, }, { user, @@ -1038,7 +1040,8 @@ describe('Article API', () => { readableContent: '

test 2

', slug: 'test slug 2', originalUrl: `${url}/test2`, - folder: 'archive', + archivedAt: new Date(), + state: LibraryItemState.Archived, }, { user, @@ -1173,7 +1176,8 @@ describe('Article API', () => { readableContent: '

test 3

', slug: 'test slug 3', originalUrl: `${url}/test3`, - folder: 'archive', + archivedAt: new Date(), + state: LibraryItemState.Archived, }, ], user.id @@ -1263,7 +1267,7 @@ describe('Article API', () => { slug: 'test slug 1', originalUrl: `${url}/test1`, itemType: PageType.File, - folder: 'archive', + archivedAt: new Date(), }, { user, @@ -1271,7 +1275,7 @@ describe('Article API', () => { readableContent: '

test 2

', slug: 'test slug 2', originalUrl: `${url}/test2`, - folder: 'archive', + archivedAt: new Date(), readingProgressBottomPercent: 100, }, { @@ -1313,7 +1317,7 @@ describe('Article API', () => { slug: 'test slug 1', originalUrl: `${url}/test1`, subscription: 'feed', - folder: 'archive', + archivedAt: new Date(), }, { user, @@ -1329,7 +1333,7 @@ describe('Article API', () => { readableContent: '

test 3

', slug: 'test slug 3', originalUrl: `${url}/test3`, - folder: 'archive', + archivedAt: new Date(), }, ], user.id @@ -1362,7 +1366,7 @@ describe('Article API', () => { readableContent: '

test 1

', slug: 'test slug 1', originalUrl: `${url}/test1`, - folder: 'trash', + deletedAt: new Date(), }, { user, @@ -1371,7 +1375,7 @@ describe('Article API', () => { slug: 'test slug 2', originalUrl: `${url}/test2`, readingProgressBottomPercent: 100, - folder: 'trash', + deletedAt: new Date(), }, { user, @@ -1733,7 +1737,7 @@ describe('Article API', () => { for (let i = 0; i < 3; i++) { await updateLibraryItem( items[i].id, - { folder: 'trash', savedAt: new Date() }, + { state: LibraryItemState.Deleted, deletedAt: new Date() }, user.id ) deletedItems.push(items[i]) diff --git a/packages/db/migrations/0148.do.update_folder_in_library_item.sql b/packages/db/migrations/0148.do.update_folder_in_library_item.sql new file mode 100755 index 000000000..33d0ca9db --- /dev/null +++ b/packages/db/migrations/0148.do.update_folder_in_library_item.sql @@ -0,0 +1,9 @@ +-- Type: DO +-- Name: update_folder_in_library_item +-- Description: Update folder column in library_item table + +BEGIN; + +UPDATE omnivore.library_item SET folder = 'inbox' WHERE folder = 'archive' OR folder = 'trash'; + +COMMIT; diff --git a/packages/db/migrations/0148.undo.update_folder_in_library_item.sql b/packages/db/migrations/0148.undo.update_folder_in_library_item.sql new file mode 100755 index 000000000..3b3eea8c2 --- /dev/null +++ b/packages/db/migrations/0148.undo.update_folder_in_library_item.sql @@ -0,0 +1,10 @@ +-- Type: UNDO +-- Name: update_folder_in_library_item +-- Description: Update folder column in library_item table + +BEGIN; + +UPDATE omnivore.library_item SET folder = 'archive' WHERE archived_at IS NOT NULL; +UPDATE omnivore.library_item SET folder = 'trash' WHERE deleted_at IS NOT NULL; + +COMMIT;