From 7707bb8fecd85cc65a4e511f3033170a1c24fb1d Mon Sep 17 00:00:00 2001 From: Alexander Petric Date: Tue, 16 Sep 2025 16:28:38 -0400 Subject: [PATCH] feat: simplify sync vs promotion mode ui in git sync settings (#6615) * feat: simplify sync vs promotion mode ui in git sync settings * improvements --- .../components/git_sync/DetectionFlow.svelte | 81 +-- .../git_sync/GitSyncContext.svelte.ts | 171 ++++- .../git_sync/GitSyncModeDisplay.svelte | 24 + .../git_sync/GitSyncRepositoryCard.svelte | 660 +++++++++++------- .../components/git_sync/GitSyncSection.svelte | 151 +++- .../git_sync/PullWorkspaceModal.svelte | 62 +- .../git_sync/PushWorkspaceModal.svelte | 60 +- frontend/src/lib/services/JobManager.ts | 17 +- 8 files changed, 829 insertions(+), 397 deletions(-) create mode 100644 frontend/src/lib/components/git_sync/GitSyncModeDisplay.svelte diff --git a/frontend/src/lib/components/git_sync/DetectionFlow.svelte b/frontend/src/lib/components/git_sync/DetectionFlow.svelte index a1f411530d..1ee2b37d1c 100644 --- a/frontend/src/lib/components/git_sync/DetectionFlow.svelte +++ b/frontend/src/lib/components/git_sync/DetectionFlow.svelte @@ -6,13 +6,41 @@ import Toggle from '$lib/components/Toggle.svelte' import { sendUserToast } from '$lib/toast' import { workspaceStore } from '$lib/stores' + import GitSyncModeDisplay from './GitSyncModeDisplay.svelte' - let { idx } = $props<{ idx: number }>() + let { idx, mode } = $props<{ idx: number, mode?: 'sync' | 'promotion' }>() const gitSyncContext = getGitSyncContext() const repo = $derived(gitSyncContext.getRepository(idx)) + let targetBranch = $state('main') + + // Update target branch when repository changes + $effect(() => { + const abortController = new AbortController() + + if (repo?.git_repo_resource_path) { + gitSyncContext.getTargetBranch(repo).then(branch => { + if (!abortController.signal.aborted) { + targetBranch = branch + } + }).catch(error => { + if (!abortController.signal.aborted) { + console.warn('Failed to get target branch:', error) + } + }) + } + + return () => { + abortController.abort() + } + }) async function handleDetect() { + if (!repo) { + sendUserToast('Repository not found', true) + return + } + try { await gitSyncContext.detectRepository(idx) } catch (error: any) { @@ -46,24 +74,13 @@ } + {#if repo}
{#if !repo.detectionState || repo.detectionState === 'idle'} - -
- - - {#if repo.use_individual_branch} + + {#if mode === 'promotion'} +
- {/if} -
+
+ {/if}
@@ -113,19 +130,7 @@ useIndividualBranch={repo.use_individual_branch} /> - -
- {#if repo.use_individual_branch} -
Promotion: Creating branches whose promotion target is main
- {#if repo.group_by_folder} -
Grouped by folder
- {/if} - {:else} -
Sync: Syncing back to branch main
- {/if} -
+
@@ -152,19 +157,7 @@ useIndividualBranch={repo.use_individual_branch} /> - -
- {#if repo.use_individual_branch} -
Promotion: Creating branches whose promotion target is main
- {#if repo.group_by_folder} -
Grouped by folder
- {/if} - {:else} -
Sync: Syncing back to branch main
- {/if} -
+
diff --git a/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts b/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts index 5a64d3c450..ebca4e8132 100644 --- a/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts +++ b/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts @@ -1,5 +1,5 @@ import { getContext, setContext } from 'svelte' -import { JobService, WorkspaceService } from '$lib/gen' +import { JobService, WorkspaceService, ResourceService } from '$lib/gen' import type { GitRepositorySettings as BackendGitRepositorySettings, GitSyncObjectType } from '$lib/gen' import { jobManager } from '$lib/services/JobManager' import hubPaths from '$lib/hubPaths.json' @@ -20,6 +20,10 @@ export type GitSyncRepository = BackendGitRepositorySettings & { detectionJobStatus?: 'running' | 'success' | 'failure' // Internal tracking for resource path changes _trackedPath?: string + // Cached target branch from git resource + _targetBranch?: string + // Detection timestamp to avoid race conditions + _detectionTimestamp?: number } export type GitSyncTestJob = { @@ -68,8 +72,15 @@ export function createGitSyncContext(workspace: string) { if (repo.isUnsavedConnection) { const currentPath = repo.git_repo_resource_path - if (repo._trackedPath && repo._trackedPath !== currentPath && repo.detectionState && repo.detectionState !== 'idle') { - _resetRepoDetectionState(repo) + if (repo._trackedPath && repo._trackedPath !== currentPath) { + // Clear cached branch when resource path changes + repo._targetBranch = undefined + + // Reset detection state for any non-idle state (including errors) + if (repo.detectionState && repo.detectionState !== 'idle') { + // Cancel any running detection job by resetting immediately + _resetRepoDetectionState(repo) + } } repo._trackedPath = currentPath @@ -273,6 +284,9 @@ export function createGitSyncContext(workspace: string) { repo.detectionJobId = undefined repo.detectionJobStatus = undefined + // Track the detection timestamp to avoid race conditions from old jobs + const detectionTimestamp = Date.now() + try { const jobId = await JobService.runScriptByPath({ workspace, @@ -291,15 +305,21 @@ export function createGitSyncContext(workspace: string) { repo.detectionJobId = jobId repo.detectionJobStatus = 'running' + repo._detectionTimestamp = detectionTimestamp // Use JobManager for polling - result will be the actual job response await jobManager.runWithProgress( () => Promise.resolve(jobId), { workspace, - timeout: 30000, - timeoutMessage: 'Detection job timed out after 30s', + timeout: 60000, + timeoutMessage: 'Detection job timed out after 60s', onProgress: (status) => { + // Only update state if this detection is still current + if (repo._detectionTimestamp !== detectionTimestamp) { + return + } + repo.detectionJobStatus = status.status // Process successful detection result @@ -313,7 +333,7 @@ export function createGitSyncContext(workspace: string) { if (response.local) { repo.extractedSettings = response.local // Auto-apply the extracted settings - repo.settings = { + repo.settings = { ...response.local, exclude_path: response.local.exclude_path || [], extra_include_path: response.local.extra_include_path || [] @@ -328,6 +348,11 @@ export function createGitSyncContext(workspace: string) { } ) } catch (error: any) { + // Only set error if this detection is still current + if (repo._detectionTimestamp !== detectionTimestamp) { + return + } + repo.detectionState = 'error' repo.detectionError = error?.message || error?.toString() || 'Failed to detect repository' repo.detectionJobStatus = 'failure' @@ -533,6 +558,128 @@ export function createGitSyncContext(workspace: string) { } } + function getPrimarySyncRepository(): { repo: GitSyncRepository, idx: number } | null { + const idx = repositories.findIndex(r => !r.use_individual_branch) + return idx !== -1 ? { repo: repositories[idx], idx } : null + } + + function getPrimaryPromotionRepository(): { repo: GitSyncRepository, idx: number } | null { + const idx = repositories.findIndex(r => r.use_individual_branch) + return idx !== -1 ? { repo: repositories[idx], idx } : null + } + + function getSecondarySyncRepositories(): { repo: GitSyncRepository, idx: number }[] { + const result: { repo: GitSyncRepository, idx: number }[] = [] + let foundFirst = false + repositories.forEach((repo, idx) => { + if (!repo.use_individual_branch) { + if (foundFirst) { + result.push({ repo, idx }) + } else { + foundFirst = true + } + } + }) + return result + } + + function getLegacyPromotionRepositories(): { repo: GitSyncRepository, idx: number }[] { + const result: { repo: GitSyncRepository, idx: number }[] = [] + let foundFirst = false + repositories.forEach((repo, idx) => { + if (repo.use_individual_branch) { + if (foundFirst) { + result.push({ repo, idx }) + } else { + foundFirst = true + } + } + }) + return result + } + + async function removeRepositoryByPath(resourcePath: string) { + const idx = repositories.findIndex(r => r.git_repo_resource_path === resourcePath) + if (idx !== -1) { + await removeRepository(idx) + } + } + + function addSyncRepository() { + repositories.push({ + git_repo_resource_path: '', + script_path: hubPaths.gitSync, + use_individual_branch: false, + group_by_folder: false, + settings: { + include_path: ['f/**'], + exclude_path: [], + extra_include_path: [], + include_type: ['script', 'flow', 'app', 'folder'] + }, + exclude_types_override: [], + legacyImported: false, + isUnsavedConnection: true, + collapsed: false + }) + gitSyncTestJobs.push({ + jobId: '', + status: undefined + }) + } + + function addPromotionRepository() { + repositories.push({ + git_repo_resource_path: '', + script_path: hubPaths.gitSync, + use_individual_branch: true, + group_by_folder: false, + settings: { + include_path: ['f/**'], + exclude_path: [], + extra_include_path: [], + include_type: ['script', 'flow', 'app', 'folder'] + }, + exclude_types_override: [], + legacyImported: false, + isUnsavedConnection: true, + collapsed: false + }) + gitSyncTestJobs.push({ + jobId: '', + status: undefined + }) + } + + // Helper to get target branch from git resource + async function getTargetBranch(repo: GitSyncRepository): Promise { + if (!repo.git_repo_resource_path) { + return 'main' + } + + if (repo._targetBranch) { + return repo._targetBranch + } + + try { + const resource = await ResourceService.getResource({ + workspace, + path: repo.git_repo_resource_path + }) + + // Extract branch from git resource value + const resourceValue = resource.value as any + const targetBranch = resourceValue?.branch || 'main' + + // Cache the result + repo._targetBranch = targetBranch + return targetBranch + } catch (error) { + console.warn('Failed to fetch git resource for branch info:', error) + return 'main' + } + } + // Return context object return { // State (read-only access) @@ -553,7 +700,10 @@ export function createGitSyncContext(workspace: string) { // Methods addRepository, + addSyncRepository, + addPromotionRepository, removeRepository, + removeRepositoryByPath, getRepository, getValidation, revertRepository, @@ -569,6 +719,15 @@ export function createGitSyncContext(workspace: string) { closeSuccessModal, loadSettings, saveRepository, + + // Repository categorization methods + getPrimarySyncRepository, + getPrimaryPromotionRepository, + getSecondarySyncRepositories, + getLegacyPromotionRepositories, + + // Helper methods + getTargetBranch, } } diff --git a/frontend/src/lib/components/git_sync/GitSyncModeDisplay.svelte b/frontend/src/lib/components/git_sync/GitSyncModeDisplay.svelte new file mode 100644 index 0000000000..e592a970fb --- /dev/null +++ b/frontend/src/lib/components/git_sync/GitSyncModeDisplay.svelte @@ -0,0 +1,24 @@ + + +
+ {#if mode === 'promotion'} +
Promotion: Creating branches whose promotion target is {targetBranch}
+ {#if repository?.group_by_folder} +
Grouped by folder
+ {/if} + {:else} +
Sync: Syncing back to branch {targetBranch}
+ {/if} +
\ No newline at end of file diff --git a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte index 23498246f7..1283f81c0c 100644 --- a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte @@ -1,5 +1,5 @@ -{#if repo} -
- -
-
- Repository #{idx + 1} - {#if repo.legacyImported} - - Legacy Configuration - - {/if} - - {repo.git_repo_resource_path} - -
-
- {#if validation.hasChanges && validation.isValid && !repo.isUnsavedConnection} - - {#if gitSyncContext.initialRepositories[idx] && !repo.legacyImported} - - {/if} - {/if} - + {#if idx !== null && gitSyncContext.initialRepositories[idx] && !repo.legacyImported} + + {/if} + {/if} + {/if} + {#if isCollapsible} + + {/if} + {#if !confirmingDelete} +
+ +
+ {:else} +
+ + +
+ {/if} +{/snippet} + +{#snippet repositoryContent()} +
+ +
+
Resource:
+
+ +
+ {#if !emptyString(repo.git_repo_resource_path)} + + {/if} +
+ + {#if !emptyString(repo.git_repo_resource_path)} + + {#if validation?.isDuplicate} +
+ This resource is already used by another repository. +
+ {/if} + {#if gitSyncTestJob && gitSyncTestJob.status !== undefined} +
+ {#if gitSyncTestJob.status === 'running'} + + {:else if gitSyncTestJob.status === 'success'} + {:else} - - - + {/if} - - {#if !confirmingDelete} - - {:else} -
-
+ {/if} + + + {#if repo.legacyImported} + + This repository was initialized from workspace-level legacy Git-Sync settings. Review the filters and press Save to migrate. + + {/if} + + {#if repo.script_path != hubPaths.gitSync} + + The git sync version for this repository is not latest. Current: {repo.script_path}, latest: + {hubPaths.gitSync} +
+ - + Update git sync script (require save git settings to be applied) +
+
+ {/if} + + + {#if repo.isUnsavedConnection && !emptyString(repo.git_repo_resource_path) && idx !== null} + + {:else} + + {#snippet actions()} + + {/snippet} + + + {#if !repo.isUnsavedConnection} +
+ +
+ +
+ + + {#if !emptyString(repo.git_repo_resource_path) && !repo.legacyImported} +
+
Manual workspace content sync
+
+ + +
+
+ {/if} +
+ {/if} + {/if} + {:else} +
Please select a Git repository resource.
+ {/if} +
+{/snippet} + + + +{#if shouldShowEmptyState} + +
+
+
+

{displayTitle}

+ {#if displayDescription} +

{displayDescription}

{/if}
- {#if !repo.collapsed} -
-
- {#key repo} -
Resource:
- - {#if !emptyString(repo.git_repo_resource_path)} - - {/if} - {/key} -
- {#if !emptyString(repo.git_repo_resource_path)} -
- {#if validation.isDuplicate} - This resource is already used by another repository. - {/if} - {#if gitSyncTestJob && gitSyncTestJob.status !== undefined} - {#if gitSyncTestJob.status === 'running'} - - {:else if gitSyncTestJob.status === 'success'} - - {:else} - - {/if} - Git sync resource checked via Windmill job - - {gitSyncTestJob.jobId} - WARNING: Only read permissions are verified. - {/if} -
- - {#if repo.legacyImported} - - This repository was initialized from workspace-level legacy Git-Sync settings. Review the filters and press Save to migrate. - +
+
+

+ {#if mode === 'sync'} + No sync repository configured. Add one to enable direct synchronization. + {:else if mode === 'promotion'} + No promotion repository configured. Add one to enable branch-based workflows. + {:else} + No repository configured. {/if} -

- {#if repo} - {#if repo.script_path != hubPaths.gitSync} - - The git sync version for this repository is not latest. Current: {repo.script_path}, latest: - {hubPaths.gitSync} -
- -
-
- {/if} - {#if repo.isUnsavedConnection && !emptyString(repo.git_repo_resource_path)} - -
- -
- {:else} - - - {#snippet actions()} - - {/snippet} - - {/if} - {#if !repo.isUnsavedConnection} -
- -
- {#if repo.use_individual_branch} -
Promotion: Creating branches whose promotion target is main
- {#if repo.group_by_folder} -
Grouped by folder
- {/if} - {:else} -
Sync: Syncing back to branch main
- {/if} -
- - - {#if !emptyString(repo.git_repo_resource_path) && !repo.legacyImported} -
-
Manual workspace content sync
-
- - -
-
- {/if} -
- - {/if} - - {/if} -
- {:else} -
Please select a Git repository resource.
- {/if} +

- {/if} + {#if onAdd} + + {/if} +
-{/if} +{:else if repo} + {#if variant === 'primary-sync' || variant === 'primary-promotion'} + +
+
+
+

{displayTitle}

+ {#if displayDescription} +

{displayDescription}

+ {/if} +
+
+ {@render headerActions()} +
+
+ {@render repositoryContent()} +
+ {:else} + +
+
+
+ {displayTitle} + {#if repo.legacyImported} + + Legacy Configuration + + {/if} + + {repo.git_repo_resource_path} + +
+
+ {@render headerActions()} +
+
+ {#if !repo.collapsed} +
+ {@render repositoryContent()} +
+ {/if} +
+ {/if} +{/if} \ No newline at end of file diff --git a/frontend/src/lib/components/git_sync/GitSyncSection.svelte b/frontend/src/lib/components/git_sync/GitSyncSection.svelte index 2f65e8e75a..f4b685fba7 100644 --- a/frontend/src/lib/components/git_sync/GitSyncSection.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncSection.svelte @@ -1,9 +1,9 @@ {#if !gitSyncContext} @@ -72,19 +85,129 @@
- - + +
+ gitSyncContext.addSyncRepository()} + isCollapsible={false} + showEmptyState={primarySync?.repo === null} + /> - -
- + + {#if primarySync && !primarySync.repo?.isUnsavedConnection} + {#if secondarySync.length > 0 || secondarySyncExpanded} +
+ + + {#if secondarySyncExpanded} +
+ {#if secondarySync.length === 0} +
+ No secondary sync repositories configured +
+ {:else} + {#each secondarySync as { repo, idx } (repo.git_repo_resource_path)} +
+ +
+ {/each} + {/if} + + {#if !hasUnsavedSecondary} +
+ +
+ {/if} +
+ {/if} +
+ {:else} + + {#if !hasUnsavedSecondary} +
+ +
+ {/if} + {/if} + {/if} + + +
+ gitSyncContext.addPromotionRepository()} + isCollapsible={false} + showEmptyState={primaryPromotion?.repo === null} + /> +
+ + + {#if legacyPromotion.length > 0} + + Multiple promotion repositories are no longer supported. Please reduce to a single promotion repository. + Only deletion is allowed for the additional repositories below. + +
+ + + {#if legacyPromotionExpanded} +
+ {#each legacyPromotion as { repo, idx } (repo.git_repo_resource_path)} +
+ +
+ {/each} +
+ {/if} +
+ {/if}
diff --git a/frontend/src/lib/components/git_sync/PullWorkspaceModal.svelte b/frontend/src/lib/components/git_sync/PullWorkspaceModal.svelte index 1d47e8f68b..265593801d 100644 --- a/frontend/src/lib/components/git_sync/PullWorkspaceModal.svelte +++ b/frontend/src/lib/components/git_sync/PullWorkspaceModal.svelte @@ -7,7 +7,7 @@ import { workspaceStore } from '$lib/stores' import { sendUserToast } from '$lib/toast' import hubPaths from '$lib/hubPaths.json' - import { tryEvery } from '$lib/utils' + import { jobManager } from '$lib/services/JobManager' import type { SyncResponse, SettingsResponse, SettingsObject } from '$lib/git-sync' interface Props { @@ -163,50 +163,46 @@ applyJobStatus = 'running' } - let jobSuccess = false - let result: any = {} + // Use JobManager instead of tryEvery + const result = await jobManager.runWithProgress( + () => Promise.resolve(jobId), + { + workspace, + timeout: 60000, + timeoutMessage: `${isPreview ? 'Preview' : 'Apply'} job timed out after 60s`, + onProgress: (status) => { + if (isPreview) { + previewJobStatus = status.status + } else { + applyJobStatus = status.status + } - await tryEvery({ - tryCode: async () => { - const testResult = await JobService.getCompletedJob({ workspace, id: jobId }) - jobSuccess = !!testResult.success - if (jobSuccess) { - const jobResult = await JobService.getCompletedJobResult({ workspace, id: jobId }) - result = jobResult + // Handle failure status + if (status.status === 'failure') { + if (isPreview) { + previewError = status.error || 'Preview failed' + } else { + applyError = status.error || 'Pull failed' + } + } } - }, - timeoutCode: async () => { - try { - await JobService.cancelQueuedJob({ - workspace, - id: jobId, - requestBody: { reason: `${isPreview ? 'Preview' : 'Apply'} job timed out after 60s` } - }) - } catch (err) {} - }, - interval: 500, - timeout: 60000 - }) + } + ) + // Handle successful result if (isPreview) { - previewJobStatus = jobSuccess ? 'success' : 'failure' - if (jobSuccess) { - previewResult = result - } else { - previewError = 'Preview failed' + if (previewJobStatus === 'success') { + previewResult = result as SyncResponse | SettingsResponse } } else { - applyJobStatus = jobSuccess ? 'success' : 'failure' - if (jobSuccess) { - const settingsData = result?.local + if (applyJobStatus === 'success') { + const settingsData = (result as any)?.local const hasSettingsChanges = settingsData && onFilterUpdate if (hasSettingsChanges) { onFilterUpdate(settingsData) await saveUpdatedSettings() } onSuccess?.() - } else { - applyError = 'Pull failed' } } } catch (e) { diff --git a/frontend/src/lib/components/git_sync/PushWorkspaceModal.svelte b/frontend/src/lib/components/git_sync/PushWorkspaceModal.svelte index 3719982254..8b040adb3d 100644 --- a/frontend/src/lib/components/git_sync/PushWorkspaceModal.svelte +++ b/frontend/src/lib/components/git_sync/PushWorkspaceModal.svelte @@ -6,7 +6,7 @@ import { JobService } from '$lib/gen' import { workspaceStore } from '$lib/stores' import hubPaths from '$lib/hubPaths.json' - import { tryEvery } from '$lib/utils' + import { jobManager } from '$lib/services/JobManager' import type { SyncResponse, SettingsObject } from '$lib/git-sync' interface Props { @@ -105,44 +105,40 @@ applyJobStatus = 'running' } - let jobSuccess = false - let result: any = {} + // Use JobManager instead of tryEvery + const result = await jobManager.runWithProgress( + () => Promise.resolve(jobId), + { + workspace, + timeout: 60000, + timeoutMessage: `${isPreview ? 'Preview' : 'Apply'} job timed out after 60s`, + onProgress: (status) => { + if (isPreview) { + previewJobStatus = status.status + } else { + applyJobStatus = status.status + } - await tryEvery({ - tryCode: async () => { - const testResult = await JobService.getCompletedJob({ workspace, id: jobId }) - jobSuccess = !!testResult.success - if (jobSuccess) { - const jobResult = await JobService.getCompletedJobResult({ workspace, id: jobId }) - result = jobResult + // Handle failure status + if (status.status === 'failure') { + if (isPreview) { + previewError = status.error || 'Preview failed' + } else { + applyError = status.error || 'Push failed' + } + } } - }, - timeoutCode: async () => { - try { - await JobService.cancelQueuedJob({ - workspace, - id: jobId, - requestBody: { reason: `${isPreview ? 'Preview' : 'Apply'} job timed out after 60s` } - }) - } catch (err) {} - }, - interval: 500, - timeout: 60000 - }) + } + ) + // Handle successful result if (isPreview) { - previewJobStatus = jobSuccess ? 'success' : 'failure' - if (jobSuccess) { - previewResult = result - } else { - previewError = 'Preview failed' + if (previewJobStatus === 'success') { + previewResult = result as SyncResponse } } else { - applyJobStatus = jobSuccess ? 'success' : 'failure' - if (jobSuccess) { + if (applyJobStatus === 'success') { onSuccess?.() - } else { - applyError = 'Push failed' } } } catch (e) { diff --git a/frontend/src/lib/services/JobManager.ts b/frontend/src/lib/services/JobManager.ts index 09063777ca..d0d8be5d54 100644 --- a/frontend/src/lib/services/JobManager.ts +++ b/frontend/src/lib/services/JobManager.ts @@ -84,10 +84,15 @@ export class JobManager { throw new Error('Job was cancelled') } - const jobResult = await JobService.getCompletedJob({ - workspace, - id: jobId - }) + let jobResult + try { + jobResult = await JobService.getCompletedJob({ + workspace, + id: jobId + }) + } catch (error) { + throw error + } const success = !!jobResult.success const status: JobStatus = { @@ -98,10 +103,6 @@ export class JobManager { onProgress?.(status) - if (!success) { - throw new Error(status.error) - } - return jobResult.result as T }, timeoutCode: async () => {