Code review feedback

- Apply default settinigs to disk
- make sync serial rather than parallel
- Move cancel to before the early return
- Added on blur to settings to make it more clear
- added more debugging statements

manifest.json

@@ -2,7 +2,7 @@
   "id": "things-logbook",
   "name": "Things Logbook",
   "description": "Sync your Things.app Logbook with Daily Notes",
-  "version": "0.1.0",
+  "version": "0.1.2",
   "author": "Liam Cain",
   "authorUrl": "https://github.com/liamcain/",
   "isDesktopOnly": false,

package.json

@@ -1,6 +1,6 @@
 {
   "name": "obsidian-things-logbook-plugin",
-  "version": "0.1.0",
+  "version": "0.1.2",
   "description": "Sync Things.app Logbook with Obsidian",
   "author": "liamcain",
   "main": "main.js",

src/constants.ts

@@ -1,2 +1,3 @@
+// Documented here: https://culturedcode.com/things/support/articles/2982272/
 export const THINGS_DB_PATH =
   "~/Library/Group Containers/JLMPQHK86H.com.culturedcode.ThingsMac/Things Database.thingsdatabase/main.sqlite";

src/index.ts

@@ -8,9 +8,7 @@ import {
 
 import { createConfirmationDialog } from "./modal";
 import {
-  defaultSettings,
-  DEFAULT_SECTION_HEADING,
-  DEFAULT_TAG_PREFIX,
+  DEFAULT_SETTINGS,
   ISettings,
   ThingsLogbookSettingsTab,
 } from "./settings";
@@ -116,33 +114,30 @@ export default class ThingsLogbookPlugin extends Plugin {
       (task) => window.moment.unix(task.stopDate).startOf("day").format()
     );
 
-    const jobPromises: Promise<void>[] = Object.entries(daysToTasks).map(
-      async ([dateStr, tasks]) => {
-        const date = window.moment(dateStr);
+    Object.entries(daysToTasks).map(async ([dateStr, tasks]) => {
+      const date = window.moment(dateStr);
 
-        let dailyNote = getDailyNote(date, dailyNotes);
-        if (!dailyNote) {
-          dailyNote = await createDailyNote(date);
-        }
-        return updateSection(dailyNote, "## Logbook", this.renderTasks(tasks));
+      let dailyNote = getDailyNote(date, dailyNotes);
+      if (!dailyNote) {
+        dailyNote = await createDailyNote(date);
       }
-    );
 
-    Promise.all(jobPromises).then(() => {
-      new Notice("Things Logbook sync complete");
-      this.writeOptions(() => ({
-        hasAcceptedDisclaimer: true,
-        latestSyncTime: window.moment().unix(),
-      }));
-      this.scheduleNextSync();
+      await updateSection(dailyNote, "## Logbook", this.renderTasks(tasks));
     });
+
+    new Notice("Things Logbook sync complete");
+    this.writeOptions(() => ({
+      hasAcceptedDisclaimer: true,
+      latestSyncTime: window.moment().unix(),
+    }));
+    this.scheduleNextSync();
   }
 
   renderTask(task: ITask): string {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const vault = this.app.vault as any;
     const tab = getTab(vault.getConfig("useTab"), vault.getConfig("tabSize"));
-    const prefix = this.options.tagPrefix ?? DEFAULT_TAG_PREFIX;
+    const prefix = this.options.tagPrefix;
 
     const tags = task.tags
       .filter((tag) => !!tag)
@@ -167,7 +162,7 @@ export default class ThingsLogbookPlugin extends Plugin {
   }
 
   renderTasks(tasks: ITask[]): string {
-    const { sectionHeading = DEFAULT_SECTION_HEADING } = this.options;
+    const { sectionHeading } = this.options;
     const areas = groupBy<ITask>(tasks, (task) => task.area || "");
     const headingLevel = getHeadingLevel(sectionHeading);
 
@@ -190,27 +185,29 @@ export default class ThingsLogbookPlugin extends Plugin {
 
   scheduleNextSync(): void {
     const now = window.moment().unix();
-    const options = { ...defaultSettings, ...this.options };
-    const { isSyncEnabled, latestSyncTime, syncInterval } = options;
 
-    if (!isSyncEnabled) {
+    this.cancelScheduledSync();
+    if (!this.options.isSyncEnabled || !this.options.syncInterval) {
       return;
     }
 
+    const { latestSyncTime, syncInterval } = this.options;
     const syncIntervalMs = syncInterval * 1000;
     const nextSync = Math.max(latestSyncTime + syncIntervalMs - now, 20);
+
+    console.debug(`[Things Logbook] next sync scheduled in ${nextSync}ms`);
     this.syncTimeoutId = window.setTimeout(this.tryToSyncLogbook, nextSync);
   }
 
   async loadOptions(): Promise<void> {
-    this.options = (await this.loadData()) || {};
+    this.options = Object.assign({}, DEFAULT_SETTINGS, await this.loadData());
   }
 
   async writeOptions(
     changeOpts: (settings: ISettings) => Partial<ISettings>
   ): Promise<void> {
     const diff = changeOpts(this.options);
-    this.options = { ...this.options, ...diff };
+    this.options = Object.assign(this.options, diff);
 
     // reschedule if interval changed
     if (diff.syncInterval !== undefined && this.options.isSyncEnabled) {

src/settings.ts

@@ -16,7 +16,7 @@ export interface ISettings {
   tagPrefix: string;
 }
 
-export const defaultSettings = Object.freeze({
+export const DEFAULT_SETTINGS = Object.freeze({
   hasAcceptedDisclaimer: false,
   latestSyncTime: 0,
 
@@ -57,11 +57,10 @@ export class ThingsLogbookSettingsTab extends PluginSettingTab {
         "Markdown heading to use when adding the logbook to a daily note"
       )
       .addText((textfield) => {
-        textfield.setPlaceholder(String(DEFAULT_SECTION_HEADING));
         textfield.setValue(this.plugin.options.sectionHeading);
         textfield.onChange(async (value) => {
           this.plugin.writeOptions(() => ({
-            sectionHeading: value !== "" ? value : undefined,
+            sectionHeading: value,
           }));
         });
       });
@@ -86,12 +85,14 @@ export class ThingsLogbookSettingsTab extends PluginSettingTab {
       .setName("Sync Frequency")
       .setDesc("Number of seconds the plugin will wait before syncing again")
       .addText((textfield) => {
-        textfield.setPlaceholder(String(DEFAULT_SYNC_FREQUENCY_SECONDS));
         textfield.inputEl.type = "number";
+        textfield.inputEl.onblur = () => {
+          textfield.setValue(String(this.plugin.options.syncInterval));
+        };
         textfield.setValue(String(this.plugin.options.syncInterval));
         textfield.onChange(async (value) => {
           this.plugin.writeOptions(() => ({
-            syncInterval: value !== "" ? Number(value) : undefined,
+            syncInterval: Number(value) || 0,
           }));
         });
       });
@@ -104,11 +105,10 @@ export class ThingsLogbookSettingsTab extends PluginSettingTab {
         "Prefix added to Things tags when imported into Obsidian (e.g. #logbook/work)"
       )
       .addText((textfield) => {
-        textfield.setPlaceholder(String(DEFAULT_TAG_PREFIX));
         textfield.setValue(this.plugin.options.tagPrefix);
         textfield.onChange(async (value) => {
           this.plugin.writeOptions(() => ({
-            tagPrefix: value !== "" ? value : undefined,
+            tagPrefix: value,
           }));
         });
       });

src/things.ts

@@ -146,6 +146,9 @@ export async function getTasksFromThingsLogbook(
       stopTime = batch.filter((t) => t.stopDate).last()?.stopDate;
 
       taskRecords.push(...batch);
+      console.debug(
+        `[Things Logbook] fetched ${batch.length} tasks from sqlite db`
+      );
     }
   } catch (err) {
     console.error("[Things Logbook] Failed to query the Things SQLite DB", err);
@@ -174,6 +177,9 @@ export async function getChecklistItemsFromThingsLogbook(
       stopTime = batch.filter((t) => t.stopDate).last()?.stopDate;
 
       checklistItems.push(...batch);
+      console.debug(
+        `[Things Logbook] fetched ${batch.length} checklist items from sqlite db`
+      );
     }
   } catch (err) {
     console.error("[Things Logbook] Failed to query the Things SQLite DB", err);