diff --git a/.zed/tasks.json b/.zed/tasks.json index aee119fe..f3a666d7 100644 --- a/.zed/tasks.json +++ b/.zed/tasks.json @@ -2,9 +2,9 @@ [ { "label": "Build", - "command": "npm", + "command": "npm run build", // rest of the parameters are optional - "args": ["run", "build"], + // "args": [], // Env overrides for the command, will be appended to the terminal's environment from the settings. // "env": { "foo": "bar" }, // Current working directory to spawn the command into, defaults to current project root. @@ -20,38 +20,45 @@ }, { "label": "Build with source maps", - "command": "npm", - "args": ["run", "build", "--", "--sourcemap"], + "command": "npm run build -- --sourcemap", "reveal": "never" }, { "label": "Generate Fluent types", - "command": "npm", - "args": ["run", "generate-fluent-types"], + "command": "npm run generate-fluent-types", "reveal": "never" }, { "label": "Start Zotero", - "command": "npm", - "args": ["run", "start"], + "command": "npm run start", "reveal": "never" }, { "label": "Start Zotero Beta", - "command": "npm", - "args": ["run", "start:beta"], + "command": "npm run start:beta", "reveal": "never" }, { - "label": "Test", - "command": "npm", - "args": ["run", "test"], + "label": "Test all (once)", + "command": "npm run test", "reveal": "always" }, { - "label": "Test watch", - "command": "npm", - "args": ["run", "test:watch"], + "label": "Test all (watch)", + "command": "npm run test:watch", "reveal": "always" + }, + { + "label": "Test $ZED_STEM", + "command": "npx vitest related", + "args": ["\"$ZED_RELATIVE_FILE\""], + "reveal": "always" + }, + { + "label": "Test $ZED_SYMBOL", + "command": "npx vitest", + "args": ["\"$ZED_RELATIVE_FILE\"", "--testNamePattern", "\"$ZED_SYMBOL\""], + "reveal": "always", + "tags": ["ts-test", "tsx-test"] } ] diff --git a/src/content/data/__tests__/item-data.spec.ts b/src/content/data/__tests__/item-data.spec.ts index 49b6b1fa..9f1d6867 100644 --- a/src/content/data/__tests__/item-data.spec.ts +++ b/src/content/data/__tests__/item-data.spec.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from 'vitest'; -import { createZoteroItemMock } from '../../../../test/utils'; -import { getSyncedNotesFromAttachment } from '../item-data'; +import { createZoteroItemMock, mockZoteroPrefs } from '../../../../test/utils'; +import { NoteroPref, setNoteroPref } from '../../prefs/notero-pref'; +import { + getSyncedNotesFromAttachment, + saveNotionLinkAttachment, +} from '../item-data'; describe('getSyncedNotesFromAttachment', () => { it('loads expected data when synced notes are saved in original format', () => { @@ -50,3 +54,49 @@ describe('getSyncedNotesFromAttachment', () => { }); }); }); + +describe('saveNotionLinkAttachment', () => { + it('preserves synced notes when page ID does not change', async () => { + mockZoteroPrefs(); + setNoteroPref(NoteroPref.syncNotes, true); + const pageURL = + 'notion://www.notion.so/page-00000000000000000000000000000000'; + const syncedNotes = + '
{"existing":"notes"}
'; + const item = createZoteroItemMock(); + const attachment = createZoteroItemMock(); + item.getAttachments.mockReturnValue([attachment.id]); + attachment.getField.calledWith('url').mockReturnValue(pageURL); + attachment.getNote.mockReturnValue(syncedNotes); + + await saveNotionLinkAttachment(item, pageURL); + + // eslint-disable-next-line @typescript-eslint/unbound-method + expect(attachment.setNote).toHaveBeenCalledWith( + expect.stringContaining(syncedNotes), + ); + }); + + it('resets synced notes when page ID changes', async () => { + mockZoteroPrefs(); + setNoteroPref(NoteroPref.syncNotes, true); + const oldPageURL = + 'notion://www.notion.so/old-page-00000000000000000000000000000000'; + const newPageURL = + 'notion://www.notion.so/new-page-77777777777777777777777777777777'; + const syncedNotes = + '
{"existing":"notes"}
'; + const item = createZoteroItemMock(); + const attachment = createZoteroItemMock(); + item.getAttachments.mockReturnValue([attachment.id]); + attachment.getField.calledWith('url').mockReturnValue(oldPageURL); + attachment.getNote.mockReturnValue(syncedNotes); + + await saveNotionLinkAttachment(item, newPageURL); + + // eslint-disable-next-line @typescript-eslint/unbound-method + expect(attachment.setNote).toHaveBeenCalledWith( + expect.stringContaining('
{}
'), + ); + }); +}); diff --git a/src/content/data/item-data.ts b/src/content/data/item-data.ts index 3c411ca5..ff349df5 100644 --- a/src/content/data/item-data.ts +++ b/src/content/data/item-data.ts @@ -49,9 +49,13 @@ export async function saveNotionLinkAttachment( await Zotero.Items.erase(attachmentIDs); } - let attachment = attachments.length ? attachments[0] : null; + let attachment = attachments[0]; + let pageIDChanged = false; if (attachment) { + const currentURL = attachment.getField('url'); + pageIDChanged = + !currentURL || getPageIDFromURL(currentURL) !== getPageIDFromURL(appURL); attachment.setField('url', appURL); } else { attachment = await Zotero.Attachments.linkFromURL({ @@ -64,7 +68,8 @@ export async function saveNotionLinkAttachment( }); } - updateNotionLinkAttachmentNote(attachment); + const syncedNotes = pageIDChanged ? {} : undefined; + updateNotionLinkAttachmentNote(attachment, syncedNotes); await attachment.saveTx(); } @@ -157,7 +162,7 @@ export async function saveSyncedNote( function updateNotionLinkAttachmentNote( attachment: Zotero.Item, - syncedNotes?: Required, + syncedNotes?: SyncedNotes, ) { let note = `

Do not modify or delete!

diff --git a/src/content/errors/ItemSyncError.ts b/src/content/errors/ItemSyncError.ts new file mode 100644 index 00000000..01e77b6a --- /dev/null +++ b/src/content/errors/ItemSyncError.ts @@ -0,0 +1,11 @@ +export class ItemSyncError extends Error { + public readonly item: Zotero.Item; + public readonly name = 'ItemSyncError'; + + public constructor(cause: unknown, item: Zotero.Item) { + super(`Failed to sync item with ID ${item.id} due to ${String(cause)}`, { + cause, + }); + this.item = item; + } +} diff --git a/src/content/errors/index.ts b/src/content/errors/index.ts index f57b9ba8..f1bf879a 100644 --- a/src/content/errors/index.ts +++ b/src/content/errors/index.ts @@ -1,2 +1,3 @@ +export { ItemSyncError } from './ItemSyncError'; export { LocalizableError } from './LocalizableError'; export { MissingPrefError } from './MissingPrefError'; diff --git a/src/content/prefs/preferences.tsx b/src/content/prefs/preferences.tsx index 8ad6f72d..3ea4c74c 100644 --- a/src/content/prefs/preferences.tsx +++ b/src/content/prefs/preferences.tsx @@ -7,6 +7,7 @@ import type { createRoot } from 'react-dom/client'; import { FluentMessageId } from '../../locale/fluent-types'; import { LocalizableError } from '../errors'; import { getNotionClient } from '../sync/notion-client'; +import { normalizeID } from '../sync/notion-utils'; import { createXULElement, getLocalizedErrorMessage, @@ -157,14 +158,13 @@ class Preferences { const databases = await this.retrieveNotionDatabases(); menuItems = databases.map((database) => { - const idWithoutDashes = database.id.replace(/-/g, ''); const title = database.title.map((t) => t.plain_text).join(''); const icon = database.icon?.type === 'emoji' ? database.icon.emoji : null; return { label: icon ? `${icon} ${title}` : title, - value: idWithoutDashes, + value: normalizeID(database.id), }; }); diff --git a/src/content/sync/__tests__/sync-note.spec.ts b/src/content/sync/__tests__/sync-note-item.spec.ts similarity index 90% rename from src/content/sync/__tests__/sync-note.spec.ts rename to src/content/sync/__tests__/sync-note-item.spec.ts index 70f12b64..3967f88b 100644 --- a/src/content/sync/__tests__/sync-note.spec.ts +++ b/src/content/sync/__tests__/sync-note-item.spec.ts @@ -1,7 +1,7 @@ import { APIErrorCode, APIResponseError, type Client } from '@notionhq/client'; -import { +import type { + AppendBlockChildrenResponse, PartialBlockObjectResponse, - type AppendBlockChildrenResponse, } from '@notionhq/client/build/src/api-endpoints'; import { describe, expect, it, vi } from 'vitest'; import { mockDeep, objectContainsValue } from 'vitest-mock-extended'; @@ -13,7 +13,7 @@ import { getSyncedNotes, saveSyncedNote, } from '../../data/item-data'; -import { syncNote } from '../sync-note'; +import { syncNoteItem } from '../sync-note-item'; vi.mock('../../data/item-data'); @@ -74,13 +74,13 @@ function setup({ syncedNotes }: { syncedNotes: SyncedNotes }) { return { noteItem, notion, regularItem }; } -describe('syncNote', () => { +describe('syncNoteItem', () => { it('throws an error when note has no parent', async () => { const { noteItem, notion } = setup({ syncedNotes: {} }); noteItem.isTopLevelItem.mockReturnValue(true); noteItem.topLevelItem = noteItem; - await expect(syncNote(notion, noteItem)).rejects.toThrow( + await expect(() => syncNoteItem(noteItem, notion)).rejects.toThrow( 'Cannot sync note without a parent item', ); }); @@ -89,7 +89,7 @@ describe('syncNote', () => { const { noteItem, notion } = setup({ syncedNotes: {} }); vi.mocked(getNotionPageID).mockReturnValue(undefined); - await expect(syncNote(notion, noteItem)).rejects.toThrow( + await expect(() => syncNoteItem(noteItem, notion)).rejects.toThrow( 'Cannot sync note because its parent item is not synced', ); }); @@ -97,7 +97,7 @@ describe('syncNote', () => { it('creates a container block when item does not already have one', async () => { const { noteItem, notion } = setup({ syncedNotes: {} }); - await syncNote(notion, noteItem); + await syncNoteItem(noteItem, notion); expect(notion.blocks.children.append).toHaveBeenCalledWith({ block_id: fakePageID, @@ -108,7 +108,7 @@ describe('syncNote', () => { it('saves the containerBlockID to the regular item', async () => { const { noteItem, notion, regularItem } = setup({ syncedNotes: {} }); - await syncNote(notion, noteItem); + await syncNoteItem(noteItem, notion); expect(saveSyncedNote).toHaveBeenCalledWith( regularItem, @@ -124,7 +124,7 @@ describe('syncNote', () => { syncedNotes: { containerBlockID: fakeContainerID }, }); - await syncNote(notion, noteItem); + await syncNoteItem(noteItem, notion); expect(notion.blocks.children.append).not.toHaveBeenCalledWith({ block_id: fakePageID, @@ -142,7 +142,7 @@ describe('syncNote', () => { .mockRejectedValueOnce(objectNotFoundError) .mockResolvedValueOnce(createResponseMock({ id: fakeNoteBlockID })); - await syncNote(notion, noteItem); + await syncNoteItem(noteItem, notion); expect(notion.blocks.children.append).toHaveBeenCalledWith({ block_id: fakePageID, @@ -160,7 +160,7 @@ describe('syncNote', () => { .calledWith(objectContainsValue(fakeContainerID)) .mockRejectedValue(new Error('Failed to append children')); - await expect(() => syncNote(notion, noteItem)).rejects.toThrow(); + await expect(() => syncNoteItem(noteItem, notion)).rejects.toThrow(); expect(saveSyncedNote).toHaveBeenCalledWith( regularItem, @@ -179,7 +179,7 @@ describe('syncNote', () => { .calledWith(objectContainsValue(fakeNoteBlockID)) .mockRejectedValue(new Error('Failed to append children')); - await expect(() => syncNote(notion, noteItem)).rejects.toThrow(); + await expect(() => syncNoteItem(noteItem, notion)).rejects.toThrow(); expect(saveSyncedNote).toHaveBeenCalledWith( regularItem, diff --git a/src/content/sync/__tests__/sync-regular-item.spec.ts b/src/content/sync/__tests__/sync-regular-item.spec.ts new file mode 100644 index 00000000..a2df9102 --- /dev/null +++ b/src/content/sync/__tests__/sync-regular-item.spec.ts @@ -0,0 +1,186 @@ +import { APIErrorCode, APIResponseError, type Client } from '@notionhq/client'; +import type { PageObjectResponse } from '@notionhq/client/build/src/api-endpoints'; +import { describe, expect, it, vi } from 'vitest'; +import { mockDeep } from 'vitest-mock-extended'; + +import { createZoteroItemMock } from '../../../../test/utils'; +import { getNotionPageID } from '../../data/item-data'; +import { PageTitleFormat } from '../../prefs/notero-pref'; +import type { DatabaseRequestProperties } from '../notion-types'; +import { buildProperties } from '../property-builder'; +import type { SyncJobParams } from '../sync-job'; +import { syncRegularItem } from '../sync-regular-item'; + +vi.mock('../../data/item-data'); +vi.mock('../property-builder'); + +const objectNotFoundError = new APIResponseError({ + code: APIErrorCode.ObjectNotFound, + status: 404, + message: 'Not found', + headers: {}, + rawBodyText: 'Not found', +}); + +const validationError = new APIResponseError({ + code: APIErrorCode.ValidationError, + status: 400, + message: 'Validation error', + headers: {}, + rawBodyText: 'Validation error', +}); + +const fakeCitationFormat = 'fake-style'; +const fakeDatabaseID = 'fake-database-id'; +const fakeDatabaseProperties = {}; +const fakePageID = 'fake-page-id'; +const fakePageProperties: DatabaseRequestProperties = { title: { title: [] } }; +const fakePageTitleFormat = PageTitleFormat.itemAuthorDateCitation; +const fakePageResponse: PageObjectResponse = { + archived: false, + cover: null, + created_by: { id: '', object: 'user' }, + created_time: '', + icon: null, + id: fakePageID, + in_trash: false, + last_edited_by: { id: '', object: 'user' }, + last_edited_time: '', + object: 'page', + parent: { database_id: fakeDatabaseID, type: 'database_id' }, + properties: {}, + public_url: null, + url: 'fake-url', +}; + +function setup({ pageID }: { pageID?: string }) { + const regularItem = createZoteroItemMock(); + const notion = mockDeep({ + fallbackMockImplementation: () => { + throw new Error('NOT MOCKED'); + }, + }); + + vi.mocked(getNotionPageID).mockReturnValue(pageID); + vi.mocked(buildProperties).mockResolvedValue(fakePageProperties); + + notion.pages.create.mockResolvedValue(fakePageResponse); + notion.pages.update.mockResolvedValue(fakePageResponse); + notion.pages.retrieve.mockResolvedValue(fakePageResponse); + + const params: SyncJobParams = { + citationFormat: fakeCitationFormat, + databaseID: fakeDatabaseID, + databaseProperties: fakeDatabaseProperties, + notion, + pageTitleFormat: fakePageTitleFormat, + }; + + return { notion, params, regularItem }; +} + +describe('syncRegularItem', () => { + it('creates new page when page ID is not set', async () => { + const { notion, params, regularItem } = setup({ pageID: undefined }); + + await syncRegularItem(regularItem, params); + + expect(notion.pages.create).toHaveBeenCalledWith({ + parent: { database_id: fakeDatabaseID }, + properties: fakePageProperties, + }); + expect(notion.pages.update).not.toHaveBeenCalled(); + }); + + it('updates existing page when page ID is set', async () => { + const { notion, params, regularItem } = setup({ pageID: fakePageID }); + + await syncRegularItem(regularItem, params); + + expect(notion.pages.update).toHaveBeenCalledWith({ + page_id: fakePageID, + properties: fakePageProperties, + }); + expect(notion.pages.create).not.toHaveBeenCalled(); + }); + + it('creates new page when existing page is not found', async () => { + const { notion, params, regularItem } = setup({ pageID: fakePageID }); + notion.pages.update.mockRejectedValue(objectNotFoundError); + + await syncRegularItem(regularItem, params); + + expect(notion.pages.update).toHaveBeenCalledWith({ + page_id: fakePageID, + properties: fakePageProperties, + }); + expect(notion.pages.create).toHaveBeenCalledWith({ + parent: { database_id: fakeDatabaseID }, + properties: fakePageProperties, + }); + }); + + it('creates new page when existing page belongs to different database', async () => { + const { notion, params, regularItem } = setup({ pageID: fakePageID }); + notion.pages.update.mockResolvedValue({ + ...fakePageResponse, + parent: { database_id: 'different-database-id', type: 'database_id' }, + }); + + await syncRegularItem(regularItem, params); + + expect(notion.pages.update).toHaveBeenCalledWith({ + page_id: fakePageID, + properties: fakePageProperties, + }); + expect(notion.pages.create).toHaveBeenCalledWith({ + parent: { database_id: fakeDatabaseID }, + properties: fakePageProperties, + }); + }); + + it('creates new page when validation error is caused by differing database', async () => { + const { notion, params, regularItem } = setup({ pageID: fakePageID }); + notion.pages.update.mockRejectedValue(validationError); + notion.pages.retrieve.mockResolvedValue({ + ...fakePageResponse, + parent: { database_id: 'different-database-id', type: 'database_id' }, + }); + + await syncRegularItem(regularItem, params); + + expect(notion.pages.update).toHaveBeenCalledWith({ + page_id: fakePageID, + properties: fakePageProperties, + }); + expect(notion.pages.create).toHaveBeenCalledWith({ + parent: { database_id: fakeDatabaseID }, + properties: fakePageProperties, + }); + }); + + it('throws error when validation error is not caused by differing database', async () => { + const { notion, params, regularItem } = setup({ pageID: fakePageID }); + notion.pages.update.mockRejectedValue(validationError); + + await expect(() => syncRegularItem(regularItem, params)).rejects.toThrow( + validationError, + ); + }); + + it('throws error when error is unexpected type', async () => { + const { notion, params, regularItem } = setup({ pageID: fakePageID }); + const unexpectedError = new APIResponseError({ + code: APIErrorCode.InternalServerError, + status: 500, + message: 'Internal server error', + headers: {}, + rawBodyText: 'Internal server error', + }); + notion.pages.update.mockRejectedValue(unexpectedError); + + await expect(() => syncRegularItem(regularItem, params)).rejects.toThrow( + unexpectedError, + ); + }); +}); diff --git a/src/content/sync/notion-utils/index.ts b/src/content/sync/notion-utils/index.ts index ea990180..2648a681 100644 --- a/src/content/sync/notion-utils/index.ts +++ b/src/content/sync/notion-utils/index.ts @@ -1,4 +1,5 @@ export { buildDate } from './build-date'; export { buildRichText } from './build-rich-text'; export { isArchivedOrNotFoundError, isNotionErrorWithCode } from './error'; +export { normalizeID } from './normalize-id'; export { convertWebURLToAppURL, getPageIDFromURL, isNotionURL } from './url'; diff --git a/src/content/sync/notion-utils/normalize-id.ts b/src/content/sync/notion-utils/normalize-id.ts new file mode 100644 index 00000000..88c4660d --- /dev/null +++ b/src/content/sync/notion-utils/normalize-id.ts @@ -0,0 +1,3 @@ +export function normalizeID(id: string): string { + return id.replace(/-/g, ''); +} diff --git a/src/content/sync/sync-job.ts b/src/content/sync/sync-job.ts index b912d04a..08347c7d 100644 --- a/src/content/sync/sync-job.ts +++ b/src/content/sync/sync-job.ts @@ -1,16 +1,7 @@ -import { isFullPage, type Client, APIErrorCode } from '@notionhq/client'; -import type { - CreatePageResponse, - UpdatePageResponse, -} from '@notionhq/client/build/src/api-endpoints'; +import { type Client } from '@notionhq/client'; import { APA_STYLE } from '../constants'; -import { - getNotionPageID, - saveNotionLinkAttachment, - saveNotionTag, -} from '../data/item-data'; -import { LocalizableError } from '../errors'; +import { ItemSyncError } from '../errors'; import { NoteroPref, PageTitleFormat, @@ -21,19 +12,16 @@ import { getLocalizedErrorMessage, logger } from '../utils'; import { getNotionClient } from './notion-client'; import type { DatabaseProperties } from './notion-types'; -import { convertWebURLToAppURL, isNotionErrorWithCode } from './notion-utils'; import { ProgressWindow } from './progress-window'; -import { buildProperties } from './property-builder'; -import { syncNote } from './sync-note'; +import { syncNoteItem } from './sync-note-item'; +import { syncRegularItem } from './sync-regular-item'; -type SyncJobParams = { +export type SyncJobParams = { citationFormat: string; databaseID: string; databaseProperties: DatabaseProperties; - items: Zotero.Item[]; notion: Client; pageTitleFormat: PageTitleFormat; - progressWindow: ProgressWindow; }; export async function performSyncJob( @@ -47,38 +35,14 @@ export async function performSyncJob( await progressWindow.show(); try { - const syncJob = await prepareSyncJob({ items, progressWindow, window }); - - await syncJob.perform(); - - progressWindow.complete(); + const params = await prepareSyncJob(window); + await syncItems(items, progressWindow, params); } catch (error) { - let cause = error; - let failedItem: Zotero.Item | undefined; - - if (error instanceof ItemSyncError) { - cause = error.cause; - failedItem = error.item; - } - - const errorMessage = await getLocalizedErrorMessage( - cause, - window.document.l10n, - ); - - logger.error(error, failedItem?.getDisplayTitle()); - - progressWindow.fail(errorMessage, failedItem); + await handleError(error, progressWindow, window); } } -async function prepareSyncJob({ - items, - progressWindow, - window, -}: Pick & { - window: Window; -}): Promise { +async function prepareSyncJob(window: Window): Promise { const notion = getNotionClient(window); const databaseID = getRequiredNoteroPref(NoteroPref.notionDatabaseID); const databaseProperties = await retrieveDatabaseProperties( @@ -88,15 +52,13 @@ async function prepareSyncJob({ const citationFormat = getCitationFormat(); const pageTitleFormat = getPageTitleFormat(); - return new SyncJob({ + return { citationFormat, databaseID, databaseProperties, - items, notion, pageTitleFormat, - progressWindow, - }); + }; } function getCitationFormat(): string { @@ -122,111 +84,58 @@ async function retrieveDatabaseProperties( return database.properties; } -class ItemSyncError extends Error { - public readonly item: Zotero.Item; - public readonly name = 'ItemSyncError'; - - public constructor(cause: unknown, item: Zotero.Item) { - super(`Failed to sync item with ID ${item.id} due to ${String(cause)}`, { - cause, - }); - this.item = item; - } -} +async function syncItems( + items: Zotero.Item[], + progressWindow: ProgressWindow, + params: SyncJobParams, +) { + for (const [index, item] of items.entries()) { + const step = index + 1; + logger.groupCollapsed( + `Syncing item ${step} of ${items.length} with ID`, + item.id, + ); + logger.debug(item.getDisplayTitle()); -class SyncJob { - private readonly citationFormat: string; - private readonly databaseID: string; - private readonly databaseProperties: DatabaseProperties; - private readonly items: Zotero.Item[]; - private readonly notion: Client; - private readonly pageTitleFormat: PageTitleFormat; - private readonly progressWindow: ProgressWindow; - - public constructor(params: SyncJobParams) { - this.citationFormat = params.citationFormat; - this.databaseID = params.databaseID; - this.databaseProperties = params.databaseProperties; - this.items = params.items; - this.notion = params.notion; - this.pageTitleFormat = params.pageTitleFormat; - this.progressWindow = params.progressWindow; - } + await progressWindow.updateText(step); - public async perform() { - for (const [index, item] of this.items.entries()) { - const step = index + 1; - logger.groupCollapsed( - `Syncing item ${step} of ${this.items.length} with ID`, - item.id, - ); - logger.debug(item.getDisplayTitle()); - - await this.progressWindow.updateText(step); - - try { - if (item.isNote()) { - await this.syncNoteItem(item); - } else { - await this.syncRegularItem(item); - } - } catch (error) { - throw new ItemSyncError(error, item); - } finally { - logger.groupEnd(); + try { + if (item.isNote()) { + await syncNoteItem(item, params.notion); + } else { + await syncRegularItem(item, params); } - - this.progressWindow.updateProgress(step); + } catch (error) { + throw new ItemSyncError(error, item); + } finally { + logger.groupEnd(); } + + progressWindow.updateProgress(step); } - private async syncRegularItem(item: Zotero.Item) { - const response = await this.saveItemToDatabase(item); + progressWindow.complete(); +} - await saveNotionTag(item); +async function handleError( + error: unknown, + progressWindow: ProgressWindow, + window: Window, +) { + let cause = error; + let failedItem: Zotero.Item | undefined; - if (isFullPage(response)) { - const appURL = convertWebURLToAppURL(response.url); - await saveNotionLinkAttachment(item, appURL); - } else { - throw new LocalizableError( - 'Failed to create Notion link attachment', - 'notero-error-notion-link-attachment', - ); - } + if (error instanceof ItemSyncError) { + cause = error.cause; + failedItem = error.item; } - private async saveItemToDatabase( - item: Zotero.Item, - ): Promise { - const pageID = getNotionPageID(item); - - const properties = await buildProperties({ - citationFormat: this.citationFormat, - databaseProperties: this.databaseProperties, - item, - pageTitleFormat: this.pageTitleFormat, - }); - - if (pageID) { - try { - logger.debug('Update page', pageID, properties); - return await this.notion.pages.update({ page_id: pageID, properties }); - } catch (error) { - if (!isNotionErrorWithCode(error, APIErrorCode.ObjectNotFound)) { - throw error; - } - } - } + const errorMessage = await getLocalizedErrorMessage( + cause, + window.document.l10n, + ); - logger.debug('Create page', properties); - return await this.notion.pages.create({ - parent: { database_id: this.databaseID }, - properties, - }); - } + logger.error(error, failedItem?.getDisplayTitle()); - private async syncNoteItem(item: Zotero.Item) { - await syncNote(this.notion, item); - } + progressWindow.fail(errorMessage, failedItem); } diff --git a/src/content/sync/sync-note.ts b/src/content/sync/sync-note-item.ts similarity index 97% rename from src/content/sync/sync-note.ts rename to src/content/sync/sync-note-item.ts index 062111ca..6702211e 100644 --- a/src/content/sync/sync-note.ts +++ b/src/content/sync/sync-note-item.ts @@ -1,5 +1,5 @@ -import { Client, isFullBlock } from '@notionhq/client'; -import { BlockObjectRequest } from '@notionhq/client/build/src/api-endpoints'; +import { type Client, isFullBlock } from '@notionhq/client'; +import type { BlockObjectRequest } from '@notionhq/client/build/src/api-endpoints'; import { getNotionPageID, @@ -35,12 +35,12 @@ import { isArchivedOrNotFoundError } from './notion-utils'; * supports notes within synced blocks as the synced block is used as the * container rather than the top-level container. * - * @param notion an initialized Notion `Client` instance * @param noteItem the Zotero note item to sync to Notion + * @param notion an initialized Notion `Client` instance */ -export async function syncNote( - notion: Client, +export async function syncNoteItem( noteItem: Zotero.Item, + notion: Client, ): Promise { if (noteItem.isTopLevelItem()) { throw new LocalizableError( diff --git a/src/content/sync/sync-regular-item.ts b/src/content/sync/sync-regular-item.ts new file mode 100644 index 00000000..84085d58 --- /dev/null +++ b/src/content/sync/sync-regular-item.ts @@ -0,0 +1,126 @@ +import { APIErrorCode, type Client, isFullPage } from '@notionhq/client'; +import type { CreatePageResponse } from '@notionhq/client/build/src/api-endpoints'; + +import { + getNotionPageID, + saveNotionLinkAttachment, + saveNotionTag, +} from '../data/item-data'; +import { LocalizableError } from '../errors'; +import { logger } from '../utils'; + +import type { DatabaseRequestProperties } from './notion-types'; +import { + convertWebURLToAppURL, + isArchivedOrNotFoundError, + isNotionErrorWithCode, + normalizeID, +} from './notion-utils'; +import { buildProperties } from './property-builder'; +import type { SyncJobParams } from './sync-job'; + +export async function syncRegularItem( + item: Zotero.Item, + params: SyncJobParams, +): Promise { + const response = await saveItemToDatabase(item, params); + + await saveNotionTag(item); + + if (isFullPage(response)) { + const appURL = convertWebURLToAppURL(response.url); + await saveNotionLinkAttachment(item, appURL); + } else { + throw new LocalizableError( + 'Failed to create Notion link attachment', + 'notero-error-notion-link-attachment', + ); + } +} + +async function saveItemToDatabase( + item: Zotero.Item, + { databaseID, notion, ...params }: SyncJobParams, +): Promise { + const pageID = getNotionPageID(item); + + const properties = await buildProperties({ item, ...params }); + + if (pageID) { + return updatePage(notion, databaseID, pageID, properties); + } + + return createPage(notion, databaseID, properties); +} + +function createPage( + notion: Client, + databaseID: string, + properties: DatabaseRequestProperties, +): Promise { + logger.debug('Creating page in database', databaseID, properties); + return notion.pages.create({ + parent: { database_id: databaseID }, + properties, + }); +} + +async function updatePage( + notion: Client, + databaseID: string, + pageID: string, + properties: DatabaseRequestProperties, +): Promise { + logger.debug('Updating page', pageID, 'in database', databaseID, properties); + try { + const response = await notion.pages.update({ page_id: pageID, properties }); + return await recreatePageIfDatabaseDiffers( + notion, + databaseID, + properties, + response, + ); + } catch (error) { + if (isArchivedOrNotFoundError(error)) { + logger.debug('Recreating page that was not found'); + return createPage(notion, databaseID, properties); + } + if (!isNotionErrorWithCode(error, APIErrorCode.ValidationError)) { + throw error; + } + const retrieveResponse = await notion.pages.retrieve({ page_id: pageID }); + const createResponse = await recreatePageIfDatabaseDiffers( + notion, + databaseID, + properties, + retrieveResponse, + ); + // Throw the original error if the page was not recreated + if (createResponse === retrieveResponse) { + throw error; + } + return createResponse; + } +} + +async function recreatePageIfDatabaseDiffers( + notion: Client, + desiredDatabaseID: string, + properties: DatabaseRequestProperties, + response: CreatePageResponse, +): Promise { + if (!isFullPage(response) || response.parent.type !== 'database_id') { + return response; + } + + const currentDatabaseID = normalizeID(response.parent.database_id); + if (currentDatabaseID === normalizeID(desiredDatabaseID)) { + return response; + } + + logger.debug( + 'Recreating page found in different database', + currentDatabaseID, + ); + return createPage(notion, desiredDatabaseID, properties); +}