mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-01-30 06:12:03 +00:00
docs: enhance docstrings to reach 80% coverage threshold
- Expanded docstrings in use-settings-migration.ts for parseLocalStorageSettings, localStorageHasMoreData, mergeSettings, and performSettingsMigration - Expanded docstrings in use-settings-sync.ts for getSettingsFieldValue and hasSettingsFieldChanged helper functions - Added detailed parameter and return value documentation - Improved clarity on migration flow and settings merging logic This brings docstring coverage from 77.78% to 80%+ to satisfy CodeRabbit checks. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -111,6 +111,14 @@ export function resetMigrationState(): void {
|
||||
|
||||
/**
|
||||
* Parse localStorage data into settings object
|
||||
*
|
||||
* Checks for settings in multiple locations:
|
||||
* 1. automaker-settings-cache: Fresh server settings cached from last fetch
|
||||
* 2. automaker-storage: Zustand-persisted app store state (legacy)
|
||||
* 3. automaker-setup: Setup wizard state (legacy)
|
||||
* 4. Standalone keys: worktree-panel-collapsed, file-browser-recent-folders, etc.
|
||||
*
|
||||
* @returns Merged settings object or null if no settings found
|
||||
*/
|
||||
export function parseLocalStorageSettings(): Partial<GlobalSettings> | null {
|
||||
try {
|
||||
@@ -203,7 +211,14 @@ export function parseLocalStorageSettings(): Partial<GlobalSettings> | null {
|
||||
|
||||
/**
|
||||
* Check if localStorage has more complete data than server
|
||||
* Returns true if localStorage has projects but server doesn't
|
||||
*
|
||||
* Compares the completeness of data to determine if a migration is needed.
|
||||
* Returns true if localStorage has projects but server doesn't, indicating
|
||||
* the localStorage data should be merged with server settings.
|
||||
*
|
||||
* @param localSettings Settings loaded from localStorage
|
||||
* @param serverSettings Settings loaded from server
|
||||
* @returns true if localStorage has more data that should be preserved
|
||||
*/
|
||||
export function localStorageHasMoreData(
|
||||
localSettings: Partial<GlobalSettings> | null,
|
||||
@@ -226,7 +241,15 @@ export function localStorageHasMoreData(
|
||||
|
||||
/**
|
||||
* Merge localStorage settings with server settings
|
||||
* Prefers server data, but uses localStorage for missing arrays/objects
|
||||
*
|
||||
* Intelligently combines settings from both sources:
|
||||
* - Prefers server data as the base
|
||||
* - Uses localStorage values when server has empty arrays/objects
|
||||
* - Specific handling for: projects, trashedProjects, mcpServers, recentFolders, etc.
|
||||
*
|
||||
* @param serverSettings Settings from server API (base)
|
||||
* @param localSettings Settings from localStorage (fallback)
|
||||
* @returns Merged GlobalSettings object ready to hydrate the store
|
||||
*/
|
||||
export function mergeSettings(
|
||||
serverSettings: GlobalSettings,
|
||||
@@ -308,8 +331,14 @@ export function mergeSettings(
|
||||
* This is the core migration logic extracted for use outside of React hooks.
|
||||
* Call this from __root.tsx during app initialization.
|
||||
*
|
||||
* @param serverSettings - Settings fetched from the server API
|
||||
* @returns Promise resolving to the final settings to use (merged if migration needed)
|
||||
* Flow:
|
||||
* 1. If server has localStorageMigrated flag, skip migration (already done)
|
||||
* 2. Check if localStorage has more data than server
|
||||
* 3. If yes, merge them and sync merged state back to server
|
||||
* 4. Set localStorageMigrated flag to prevent re-migration
|
||||
*
|
||||
* @param serverSettings Settings fetched from the server API
|
||||
* @returns Promise resolving to {settings, migrated} - final settings and whether migration occurred
|
||||
*/
|
||||
export async function performSettingsMigration(
|
||||
serverSettings: GlobalSettings
|
||||
|
||||
@@ -81,7 +81,15 @@ const SETUP_FIELDS_TO_SYNC = ['isFirstRun', 'setupComplete', 'skipClaudeSetup']
|
||||
|
||||
/**
|
||||
* Helper to extract a settings field value from app state
|
||||
* Handles special cases for nested/mapped fields
|
||||
*
|
||||
* Handles special cases where store fields don't map directly to settings:
|
||||
* - currentProjectId: Extract from currentProject?.id
|
||||
* - terminalFontFamily: Extract from terminalState.fontFamily
|
||||
* - Other fields: Direct access
|
||||
*
|
||||
* @param field The settings field to extract
|
||||
* @param appState Current app store state
|
||||
* @returns The value of the field in the app state
|
||||
*/
|
||||
function getSettingsFieldValue(
|
||||
field: (typeof SETTINGS_FIELDS_TO_SYNC)[number],
|
||||
@@ -98,6 +106,16 @@ function getSettingsFieldValue(
|
||||
|
||||
/**
|
||||
* Helper to check if a settings field changed between states
|
||||
*
|
||||
* Compares field values between old and new state, handling special cases:
|
||||
* - currentProjectId: Compare currentProject?.id values
|
||||
* - terminalFontFamily: Compare terminalState.fontFamily values
|
||||
* - Other fields: Direct reference equality check
|
||||
*
|
||||
* @param field The settings field to check
|
||||
* @param newState New app store state
|
||||
* @param prevState Previous app store state
|
||||
* @returns true if the field value changed between states
|
||||
*/
|
||||
function hasSettingsFieldChanged(
|
||||
field: (typeof SETTINGS_FIELDS_TO_SYNC)[number],
|
||||
|
||||
@@ -77,8 +77,10 @@ test.describe('Project Creation', () => {
|
||||
}
|
||||
|
||||
// Wait for project to be set as current and visible on the page
|
||||
// The project name appears in the project switcher button
|
||||
await expect(page.getByTestId(`project-switcher-project-${projectName}`)).toBeVisible({
|
||||
// The project name appears in the project switcher button with title attribute
|
||||
// (The button uses data-testid with projectId, not projectName)
|
||||
const projectSwitcherButton = page.locator(`button[title="${projectName}"]`).first();
|
||||
await expect(projectSwitcherButton).toBeVisible({
|
||||
timeout: 15000,
|
||||
});
|
||||
|
||||
|
||||
@@ -156,9 +156,11 @@ test.describe('Open Project', () => {
|
||||
}
|
||||
|
||||
// Wait for a project to be set as current and visible on the page
|
||||
// The project name appears in the project switcher button
|
||||
// The project name appears in the project switcher button with title attribute
|
||||
// (The button uses data-testid with projectId, not projectName)
|
||||
if (targetProjectName) {
|
||||
await expect(page.getByTestId(`project-switcher-project-${targetProjectName}`)).toBeVisible({
|
||||
const projectSwitcherButton = page.locator(`button[title="${targetProjectName}"]`).first();
|
||||
await expect(projectSwitcherButton).toBeVisible({
|
||||
timeout: 15000,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user