-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(deployState): deploy not persisting bug #518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -428,6 +428,11 @@ export async function POST(req: NextRequest) { | |
| color: clientWorkflow.color, | ||
| state: stateToSave, | ||
| marketplaceData: clientWorkflow.marketplaceData || null, | ||
| // Include deployment status from client state | ||
| isDeployed: clientWorkflow.state.isDeployed || false, | ||
| deployedAt: clientWorkflow.state.deployedAt || null, | ||
| // deployedState is null for new workflows - only set during deployment | ||
| deployedState: null, | ||
| lastSynced: now, | ||
| createdAt: now, | ||
| updatedAt: now, | ||
|
|
@@ -485,6 +490,32 @@ export async function POST(req: NextRequest) { | |
| needsUpdate = true | ||
| } | ||
|
|
||
| // Check deployment status changes - ensure dual-write consistency | ||
| const clientIsDeployed = clientWorkflow.state.isDeployed || false | ||
| const clientDeployedAt = clientWorkflow.state.deployedAt || null | ||
|
|
||
| if (dbWorkflow.isDeployed !== clientIsDeployed) { | ||
| updateData.isDeployed = clientIsDeployed | ||
| needsUpdate = true | ||
| } | ||
|
|
||
| // Handle deployedAt comparison (convert to comparable format) | ||
| const dbDeployedAtTime = dbWorkflow.deployedAt | ||
| ? new Date(dbWorkflow.deployedAt).getTime() | ||
| : null | ||
| const clientDeployedAtTime = clientDeployedAt | ||
| ? new Date(clientDeployedAt).getTime() | ||
| : null | ||
|
|
||
| if (dbDeployedAtTime !== clientDeployedAtTime) { | ||
| updateData.deployedAt = clientDeployedAt | ||
| needsUpdate = true | ||
| } | ||
|
|
||
| // CRITICAL: Always preserve deployedState - it should only be modified by deploy/undeploy operations | ||
| // The sync operation should never touch deployedState to prevent data loss | ||
| // Note: We don't add deployedState to updateData because we want to preserve the existing value | ||
|
|
||
|
Comment on lines
+493
to
+518
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider extracting deployment status comparison logic into a separate function for better maintainability and reusability |
||
| if (needsUpdate) { | ||
| updateData.lastSynced = now | ||
| updateData.updatedAt = now | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -630,6 +630,21 @@ export const useWorkflowRegistry = create<WorkflowRegistry>()( | |
| // Set the workflow state in the store | ||
| useWorkflowStore.setState(workflowState) | ||
|
|
||
| // CRITICAL: Set deployment status in registry when switching to workflow | ||
| if (workflowData?.isDeployed || workflowData?.deployedAt) { | ||
| set((state) => ({ | ||
| deploymentStatuses: { | ||
| ...state.deploymentStatuses, | ||
| [id]: { | ||
| isDeployed: workflowData.isDeployed || false, | ||
| deployedAt: workflowData.deployedAt ? new Date(workflowData.deployedAt) : undefined, | ||
| apiKey: workflowData.apiKey || undefined, | ||
| needsRedeployment: false, // Default to false when loading from DB | ||
| }, | ||
| }, | ||
| })) | ||
| } | ||
|
Comment on lines
+634
to
+646
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider moving this deployment status sync logic into a separate function like |
||
|
|
||
| // Update the active workflow ID | ||
| set({ activeWorkflowId: id, error: null }) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -198,6 +198,7 @@ export async function fetchWorkflowsFromDB(): Promise<void> { | |
|
|
||
| // Process workflows | ||
| const registryWorkflows: Record<string, WorkflowMetadata> = {} | ||
| const deploymentStatuses: Record<string, any> = {} | ||
|
|
||
| data.forEach((workflow) => { | ||
| const { | ||
|
|
@@ -210,6 +211,9 @@ export async function fetchWorkflowsFromDB(): Promise<void> { | |
| marketplaceData, | ||
| workspaceId, | ||
| folderId, | ||
| isDeployed, | ||
| deployedAt, | ||
| apiKey, | ||
| } = workflow | ||
|
|
||
| // Skip if workflow doesn't belong to active workspace | ||
|
|
@@ -229,6 +233,16 @@ export async function fetchWorkflowsFromDB(): Promise<void> { | |
| folderId: folderId || null, | ||
| } | ||
|
|
||
| // CRITICAL: Extract deployment status from database and add to registry | ||
| if (isDeployed || deployedAt) { | ||
| deploymentStatuses[id] = { | ||
| isDeployed: isDeployed || false, | ||
| deployedAt: deployedAt ? new Date(deployedAt) : undefined, | ||
| apiKey: apiKey || undefined, | ||
| needsRedeployment: false, // Default to false when loading from DB | ||
| } | ||
| } | ||
|
Comment on lines
+238
to
+244
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider using a TypeScript interface for deploymentStatuses instead of 'any' to ensure type safety |
||
|
|
||
| // Initialize subblock values | ||
| const subblockValues: Record<string, Record<string, any>> = {} | ||
| if (state?.blocks) { | ||
|
|
@@ -251,9 +265,10 @@ export async function fetchWorkflowsFromDB(): Promise<void> { | |
| })) | ||
| }) | ||
|
|
||
| // Update registry with loaded workflows | ||
| // Update registry with loaded workflows and deployment statuses | ||
| useWorkflowRegistry.setState({ | ||
| workflows: registryWorkflows, | ||
| deploymentStatuses: deploymentStatuses, | ||
| isLoading: false, | ||
| error: null, | ||
| }) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Use a utility function for date comparison to handle null cases and standardize timestamp conversion