Repository: dvanoni/notero (ts)

Issue:
Changing database ID does not work
<!--
Have you checked the [FAQ section](https://github.com/dvanoni/notero#frequently-asked-questions) in the README? There may already be an answer for you there. If there isn't, please provide the info below!
-->

**Describe the issue**
Changing Database ID to a new one does not work. Notero still tries to sync using the old Notion Database ID.

**To Reproduce**
Steps to reproduce the behavior:

1. Create/copy Notion database.
2. Paste new Database ID into Notero preferences.
3. Sync any enabled collection.
4. See changes in the old Notion database.

**Expected behavior**
Changes synced to a new Notion database.

**Screenshots**
N/A

**Additional context**
N/A


Pull request:
fix(sync): Support changing destination Notion database
Fixes #241 

Previously, changing the selected destination Notion database did not have the desired effect for items that had been synced to the previous database because Notero would continue to sync to the page in the previous database. To remedy this, we now do the following during the sync process:

- Recreate Notion pages when found in a different database
- Reset synced notes when page ID changes

This PR also includes a number of refactors. See the commits for details.

Patch:
diff --git a/.zed/tasks.json b/.zed/tasks.json
index aee119f..f3a666d 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/item-data.ts b/src/content/data/item-data.ts
index 3c411ca..ff349df 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?: SyncedNotes,
 ) {
   let note = `
 <h2 style="background-color: #ff666680;">Do not modify or delete!</h2>
diff --git a/src/content/errors/ItemSyncError.ts b/src/content/errors/ItemSyncError.ts
new file mode 100644
index 0000000..01e77b6
--- /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 f57b9ba..f1bf879 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 8ad6f72..3ea4c74 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<MenuItem>((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/notion-utils/index.ts b/src/content/sync/notion-utils/index.ts
index ea99018..2648a68 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 0000000..88c4660
--- /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 b912d04..08347c7 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<SyncJobParams, 'items' | 'progressWindow'> & {
-  window: Window;
-}): Promise<SyncJob> {
+async function prepareSyncJob(window: Window): Promise<SyncJobParams> {
   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<CreatePageResponse & UpdatePageResponse> {
-    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 062111c..6702211 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<void> {
   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 0000000..84085d5
--- /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<void> {
+  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<CreatePageResponse> {
+  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<CreatePageResponse> {
+  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<CreatePageResponse> {
+  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<CreatePageResponse> {
+  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);
+}


Test patch:
diff --git a/src/content/data/__tests__/item-data.spec.ts b/src/content/data/__tests__/item-data.spec.ts
index 49b6b1f..9f1d686 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 =
+      '<pre id="notero-synced-notes">{"existing":"notes"}</pre>';
+    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 =
+      '<pre id="notero-synced-notes">{"existing":"notes"}</pre>';
+    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('<pre id="notero-synced-notes">{}</pre>'),
+    );
+  });
+});
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 70f12b6..3967f88 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 0000000..a2df910
--- /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<Client>({
+    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,
+    );
+  });
+});