Set a default initial directory, handle an existing settings file wit…

…h no project directory (#3734) * Fix the project directory setting assignment from file * Fix default project directory value initialization * Add a couple tests for loading the app without project directory settings * A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest) * A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest) * A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest) * trigger CI * Object merging logic was bad, blew away other app settings if they existed * Update silly little export file size expectation numbers * Make rename timeout in test way shorter * Fix silly little test issues --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Kurt Hutten Irev-Dev <[email protected]>
KittyCAD · Sep 4, 2024 · 35f9b82 · 35f9b82
1 parent cbddb35
commit 35f9b82
Show file tree

Hide file tree

Showing 8 changed files with 121 additions and 53 deletions.
diff --git a/e2e/playwright/code-pane-and-errors.spec.ts b/e2e/playwright/code-pane-and-errors.spec.ts
@@ -271,10 +271,7 @@ test(
 
       await page.getByText('bracket').click()
 
-      await expect(page.getByTestId('loading')).toBeAttached()
-      await expect(page.getByTestId('loading')).not.toBeAttached({
-        timeout: 20_000,
-      })
+      await u.waitForPageLoad()
     })
 
     // If they're open by default, we're not actually testing anything.
@@ -302,16 +299,7 @@ test(
 
       await page.getByText('router-template-slate').click()
 
-      await expect(page.getByTestId('loading')).toBeAttached()
-      await expect(page.getByTestId('loading')).not.toBeAttached({
-        timeout: 20_000,
-      })
-
-      await expect(
-        page.getByRole('button', { name: 'Start Sketch' })
-      ).toBeEnabled({
-        timeout: 20_000,
-      })
+      await u.waitForPageLoad()
     })
 
     await test.step('All panes opened before should be visible', async () => {

diff --git a/e2e/playwright/projects.spec.ts b/e2e/playwright/projects.spec.ts
@@ -938,16 +938,7 @@ test(
 
       await page.getByText('bracket').click()
 
-      await expect(page.getByTestId('loading')).toBeAttached()
-      await expect(page.getByTestId('loading')).not.toBeAttached({
-        timeout: 20_000,
-      })
-
-      await expect(
-        page.getByRole('button', { name: 'Start Sketch' })
-      ).toBeEnabled({
-        timeout: 20_000,
-      })
+      await u.waitForPageLoad()
 
       // gray at this pixel means the stream has loaded in the most
       // user way we can verify it (pixel color)
@@ -972,16 +963,7 @@ test(
 
       await page.getByText('router-template-slate').click()
 
-      await expect(page.getByTestId('loading')).toBeAttached()
-      await expect(page.getByTestId('loading')).not.toBeAttached({
-        timeout: 20_000,
-      })
-
-      await expect(
-        page.getByRole('button', { name: 'Start Sketch' })
-      ).toBeEnabled({
-        timeout: 20_000,
-      })
+      await u.waitForPageLoad()
 
       // gray at this pixel means the stream has loaded in the most
       // user way we can verify it (pixel color)
@@ -1740,7 +1722,7 @@ test.describe('Renaming in the file tree', () => {
       })
 
       await test.step('Rename the folder', async () => {
-        await page.waitForTimeout(60000)
+        await page.waitForTimeout(2000)
         await folderToRename.click({ button: 'right' })
         await expect(renameMenuItem).toBeVisible()
         await renameMenuItem.click()

diff --git a/...ests.spec.ts-snapshots/Zoom-to-fit-on-load---solid-3d-1-Google-Chrome-win32.png b/...ests.spec.ts-snapshots/Zoom-to-fit-on-load---solid-3d-1-Google-Chrome-win32.png
diff --git a/...pshots/extrude-on-default-planes-should-be-stable--XY-1-Google-Chrome-win32.png b/...pshots/extrude-on-default-planes-should-be-stable--XY-1-Google-Chrome-win32.png
diff --git a/e2e/playwright/test-utils.ts b/e2e/playwright/test-utils.ts
@@ -852,10 +852,12 @@ export async function setupElectron({
   testInfo,
   folderSetupFn,
   cleanProjectDir = true,
+  appSettings,
 }: {
   testInfo: TestInfo
   folderSetupFn?: (projectDirName: string) => Promise<void>
   cleanProjectDir?: boolean
+  appSettings?: Partial<SaveSettingsPayload>
 }) {
   // create or otherwise clear the folder
   const projectDirName = testInfo.outputPath('electron-test-projects-dir')
@@ -889,15 +891,19 @@ export async function setupElectron({
 
   if (cleanProjectDir) {
     const tempSettingsFilePath = join(projectDirName, SETTINGS_FILE_NAME)
-    const settingsOverrides = TOML.stringify({
-      ...TEST_SETTINGS,
-      settings: {
-        app: {
-          ...TEST_SETTINGS.app,
-          projectDirectory: projectDirName,
-        },
-      },
-    })
+    const settingsOverrides = TOML.stringify(
+      appSettings
+        ? { settings: appSettings }
+        : {
+            ...TEST_SETTINGS,
+            settings: {
+              app: {
+                ...TEST_SETTINGS.app,
+                projectDirectory: projectDirName,
+              },
+            },
+          }
+    )
     await fsp.writeFile(tempSettingsFilePath, settingsOverrides)
   }
 

diff --git a/e2e/playwright/testing-settings.spec.ts b/e2e/playwright/testing-settings.spec.ts
@@ -303,6 +303,61 @@ test.describe('Testing settings', () => {
     }
   )
 
+  test(
+    `Load desktop app with no settings file`,
+    { tag: '@electron' },
+    async ({ browser: _ }, testInfo) => {
+      const { electronApp, page } = await setupElectron({
+        // This is what makes no settings file get created
+        cleanProjectDir: false,
+        testInfo,
+      })
+
+      await page.setViewportSize({ width: 1200, height: 500 })
+
+      // Selectors and constants
+      const errorHeading = page.getByRole('heading', {
+        name: 'An unextected error occurred',
+      })
+      const projectDirLink = page.getByText('Loaded from')
+
+      // If the app loads without exploding we're in the clear
+      await expect(errorHeading).not.toBeVisible()
+      await expect(projectDirLink).toBeVisible()
+
+      await electronApp.close()
+    }
+  )
+
+  test(
+    `Load desktop app with a settings file, but no project directory setting`,
+    { tag: '@electron' },
+    async ({ browser: _ }, testInfo) => {
+      const { electronApp, page } = await setupElectron({
+        testInfo,
+        appSettings: {
+          app: {
+            themeColor: '259',
+          },
+        },
+      })
+
+      await page.setViewportSize({ width: 1200, height: 500 })
+
+      // Selectors and constants
+      const errorHeading = page.getByRole('heading', {
+        name: 'An unextected error occurred',
+      })
+      const projectDirLink = page.getByText('Loaded from')
+
+      // If the app loads without exploding we're in the clear
+      await expect(errorHeading).not.toBeVisible()
+      await expect(projectDirLink).toBeVisible()
+
+      await electronApp.close()
+    }
+  )
+
   test(
     `Closing settings modal should go back to the original file being viewed`,
     { tag: '@electron' },

diff --git a/src/lib/desktop.ts b/src/lib/desktop.ts
@@ -462,29 +462,60 @@ export const readProjectSettingsFile = async (
  */
 export const readAppSettingsFile = async () => {
   let settingsPath = await getAppSettingsFilePath()
+  const initialProjectDirConfig: DeepPartial<
+    Configuration['settings']['project']
+  > = { directory: await getInitialDefaultDir() }
 
   // The file exists, read it and parse it.
   if (window.electron.exists(settingsPath)) {
     const configToml = await window.electron.readFile(settingsPath)
-    const configObj = parseAppSettings(configToml)
-    if (err(configObj)) {
-      return Promise.reject(configObj)
+    const parsedAppConfig = parseAppSettings(configToml)
+    if (err(parsedAppConfig)) {
+      return Promise.reject(parsedAppConfig)
     }
 
-    return configObj
+    const hasProjectDirectorySetting =
+      parsedAppConfig.settings?.project?.directory ||
+      parsedAppConfig.settings?.app?.project_directory
+
+    if (hasProjectDirectorySetting) {
+      return parsedAppConfig
+    } else {
+      // inject the default project directory setting
+      const mergedConfig: DeepPartial<Configuration> = {
+        ...parsedAppConfig,
+        settings: {
+          ...parsedAppConfig.settings,
+          project: Object.assign(
+            {},
+            parsedAppConfig.settings?.project,
+            initialProjectDirConfig
+          ),
+        },
+      }
+      return mergedConfig
+    }
   }
 
   // The file doesn't exist, create a new one.
-  // This defaultAppConfig is truly an empty object every time.
   const defaultAppConfig = defaultAppSettings()
   if (err(defaultAppConfig)) {
     return Promise.reject(defaultAppConfig)
   }
-  const initialDirConfig: DeepPartial<Configuration> = {
-    settings: { project: { directory: await getInitialDefaultDir() } },
+
+  // inject the default project directory setting
+  const mergedDefaultConfig: DeepPartial<Configuration> = {
+    ...defaultAppConfig,
+    settings: {
+      ...defaultAppConfig.settings,
+      project: Object.assign(
+        {},
+        defaultAppConfig.settings?.project,
+        initialProjectDirConfig
+      ),
+    },
   }
-  const config = Object.assign(defaultAppConfig, initialDirConfig)
-  return config
+  return mergedDefaultConfig
 }
 
 export const writeAppSettingsFile = async (tomlStr: string) => {

diff --git a/src/lib/settings/settingsUtils.ts b/src/lib/settings/settingsUtils.ts
@@ -14,6 +14,7 @@ import { Configuration } from 'wasm-lib/kcl/bindings/Configuration'
 import { mouseControlsToCameraSystem } from 'lib/cameraControls'
 import { appThemeToTheme } from 'lib/theme'
 import {
+  getInitialDefaultDir,
   readAppSettingsFile,
   readProjectSettingsFile,
   writeAppSettingsFile,
@@ -176,6 +177,11 @@ export async function loadAndValidateSettings(
   if (err(appSettingsPayload)) return Promise.reject(appSettingsPayload)
 
   const settings = createSettings()
+  // Because getting the default directory is async, we need to set it after
+  if (onDesktop) {
+    settings.app.projectDirectory.default = await getInitialDefaultDir()
+  }
+
   setSettingsAtLevel(
     settings,
     'user',