From 00496442241dc98feb502bc7fd2b912f5c18e011 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 26 Jun 2025 21:49:43 -0700 Subject: [PATCH] fix(workspace url id bug): switch workspace bug (#564) * fix switch workspace bug" * fix(workspaces): resolve workflow selection ordering issue, eliminate race conditions during workspace transitions --------- Co-authored-by: Vikhyath Mondreti Co-authored-by: Waleed Latif --- apps/sim/app/api/workflows/sync/route.ts | 18 +++++-- .../workspace-header/workspace-header.tsx | 54 ++++++++++++++----- .../w/components/sidebar/sidebar.tsx | 51 +++++++----------- .../app/workspace/[workspaceId]/w/page.tsx | 23 ++++++-- apps/sim/contexts/socket-context.tsx | 1 - apps/sim/stores/workflows/registry/store.ts | 1 + 6 files changed, 93 insertions(+), 55 deletions(-) diff --git a/apps/sim/app/api/workflows/sync/route.ts b/apps/sim/app/api/workflows/sync/route.ts index 3c7a25af0..0484b715a 100644 --- a/apps/sim/app/api/workflows/sync/route.ts +++ b/apps/sim/app/api/workflows/sync/route.ts @@ -1,5 +1,5 @@ import crypto from 'crypto' -import { and, eq, isNull } from 'drizzle-orm' +import { and, desc, eq, isNull } from 'drizzle-orm' import { NextResponse } from 'next/server' import { getSession } from '@/lib/auth' import { createLogger } from '@/lib/logs/console-logger' @@ -155,14 +155,22 @@ export async function GET(request: Request) { if (workspaceId) { // Filter by workspace ID only, not user ID // This allows sharing workflows across workspace members - workflows = await db.select().from(workflow).where(eq(workflow.workspaceId, workspaceId)) + // Order by createdAt desc to match frontend sorting by lastModified + workflows = await db + .select() + .from(workflow) + .where(eq(workflow.workspaceId, workspaceId)) + .orderBy(desc(workflow.createdAt)) } else { // Filter by user ID only, including workflows without workspace IDs - workflows = await db.select().from(workflow).where(eq(workflow.userId, userId)) + // Order by createdAt desc to match frontend sorting by lastModified + workflows = await db + .select() + .from(workflow) + .where(eq(workflow.userId, userId)) + .orderBy(desc(workflow.createdAt)) } - const elapsed = Date.now() - startTime - // Return the workflows return NextResponse.json({ data: workflows }, { status: 200 }) } catch (error: any) { diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx index 4ffc0ba6d..d6d26a8bc 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx @@ -333,23 +333,45 @@ export const WorkspaceHeader = React.memo( }, [sessionData?.user?.id, fetchSubscriptionStatus, fetchWorkspaces]) const switchWorkspace = useCallback( - (workspace: Workspace) => { + async (workspace: Workspace) => { // If already on this workspace, close dropdown and do nothing else if (activeWorkspace?.id === workspace.id) { setWorkspaceDropdownOpen(false) return } - setActiveWorkspace(workspace) + // Close dropdown immediately for responsive feel setWorkspaceDropdownOpen(false) - // Use full workspace switch which now handles localStorage automatically - switchToWorkspace(workspace.id) + try { + // Update UI state optimistically + setActiveWorkspace(workspace) - // Update URL to include workspace ID - router.push(`/workspace/${workspace.id}/w`) + // Switch workspace data first with the explicit workspace ID + // This ensures the data switch happens with the correct ID regardless of URL timing + await switchToWorkspace(workspace.id) + + // Then update URL - this will trigger useParams updates in other components + router.push(`/workspace/${workspace.id}/w`) + } catch (error) { + // If workspace switch fails, revert the optimistic UI update + logger.error('Failed to switch workspace:', error) + // Revert to previous workspace if we can identify it + const currentWorkspaces = workspaces + const fallbackWorkspace = currentWorkspaces.find((w) => w.id === currentWorkspaceId) + if (fallbackWorkspace) { + setActiveWorkspace(fallbackWorkspace) + } + } }, - [activeWorkspace?.id, switchToWorkspace, router, setWorkspaceDropdownOpen] + [ + activeWorkspace?.id, + switchToWorkspace, + router, + setWorkspaceDropdownOpen, + workspaces, + currentWorkspaceId, + ] ) const handleCreateWorkspace = useCallback( @@ -372,11 +394,11 @@ export const WorkspaceHeader = React.memo( setWorkspaces((prev) => [...prev, newWorkspace]) setActiveWorkspace(newWorkspace) - // Use switchToWorkspace to properly load workflows for the new workspace + // Switch workspace data first with the explicit workspace ID // This will clear existing workflows, set loading state, and fetch workflows from DB switchToWorkspace(newWorkspace.id) - // Update URL to include new workspace ID + // Then update URL to include new workspace ID router.push(`/workspace/${newWorkspace.id}/w`) } } catch (err) { @@ -465,10 +487,14 @@ export const WorkspaceHeader = React.memo( // If deleted workspace was active, switch to another workspace if (activeWorkspace?.id === id && updatedWorkspaces.length > 0) { - // Use the specialized method for handling workspace deletion - const newWorkspaceId = updatedWorkspaces[0].id - useWorkflowRegistry.getState().handleWorkspaceDeletion(newWorkspaceId) - setActiveWorkspace(updatedWorkspaces[0]) + const newWorkspace = updatedWorkspaces[0] + setActiveWorkspace(newWorkspace) + + // Use the specialized method for handling workspace deletion with explicit workspace ID + useWorkflowRegistry.getState().handleWorkspaceDeletion(newWorkspace.id) + + // Update URL to the new workspace + router.push(`/workspace/${newWorkspace.id}/w`) } setWorkspaceDropdownOpen(false) @@ -478,7 +504,7 @@ export const WorkspaceHeader = React.memo( setIsDeleting(false) } }, - [workspaces, activeWorkspace?.id] + [workspaces, activeWorkspace?.id, router] ) const openEditModal = useCallback( diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx index ace5b0b0c..30e05d745 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx @@ -13,7 +13,7 @@ import { useGlobalShortcuts, } from '@/app/workspace/[workspaceId]/w/hooks/use-keyboard-shortcuts' import { useSidebarStore } from '@/stores/sidebar/store' -import { useWorkflowRegistry } from '@/stores/workflows/registry/store' +import { isWorkspaceInTransition, useWorkflowRegistry } from '@/stores/workflows/registry/store' import type { WorkflowMetadata } from '@/stores/workflows/registry/types' import { useUserPermissionsContext } from '../providers/workspace-permissions-provider' import { CreateMenu } from './components/create-menu/create-menu' @@ -32,13 +32,16 @@ const IS_DEV = process.env.NODE_ENV === 'development' export function Sidebar() { useGlobalShortcuts() - const { workflows, createWorkflow, isLoading: workflowsLoading } = useWorkflowRegistry() - const { isPending: sessionLoading } = useSession() - const userPermissions = useUserPermissionsContext() - const isLoading = workflowsLoading || sessionLoading const router = useRouter() const params = useParams() const workspaceId = params.workspaceId as string + + const { workflows, createWorkflow, isLoading: workflowsLoading } = useWorkflowRegistry() + const { isPending: sessionLoading } = useSession() + const userPermissions = useUserPermissionsContext() + + // Simple loading logic: wait for workflows and valid workspaceId, plus workspace transitions + const isLoading = workflowsLoading || sessionLoading || !workspaceId || isWorkspaceInTransition() const pathname = usePathname() const [showSettings, setShowSettings] = useState(false) @@ -57,41 +60,25 @@ export function Sidebar() { } }, [showSettings, showHelp, showInviteMembers, setAnyModalOpen]) - // Separate regular workflows from temporary marketplace workflows + // Filter workflows for current workspace (database already sorted) const { regularWorkflows, tempWorkflows } = useMemo(() => { + if (isLoading) return { regularWorkflows: [], tempWorkflows: [] } + const regular: WorkflowMetadata[] = [] const temp: WorkflowMetadata[] = [] - if (!isLoading) { - Object.values(workflows).forEach((workflow) => { - if (workflow.workspaceId === workspaceId || !workflow.workspaceId) { - if (workflow.marketplaceData?.status === 'temp') { - temp.push(workflow) - } else { - regular.push(workflow) - } + Object.values(workflows).forEach((workflow) => { + if (workflow.workspaceId === workspaceId || !workflow.workspaceId) { + if (workflow.marketplaceData?.status === 'temp') { + temp.push(workflow) + } else { + regular.push(workflow) } - }) - - // Sort by last modified date (newest first) - const sortByLastModified = (a: WorkflowMetadata, b: WorkflowMetadata) => { - const dateA = - a.lastModified instanceof Date - ? a.lastModified.getTime() - : new Date(a.lastModified).getTime() - const dateB = - b.lastModified instanceof Date - ? b.lastModified.getTime() - : new Date(b.lastModified).getTime() - return dateB - dateA } - - regular.sort(sortByLastModified) - temp.sort(sortByLastModified) - } + }) return { regularWorkflows: regular, tempWorkflows: temp } - }, [workflows, isLoading, workspaceId]) + }, [workflows, workspaceId, isLoading]) // Create workflow handler const handleCreateWorkflow = async (folderId?: string) => { diff --git a/apps/sim/app/workspace/[workspaceId]/w/page.tsx b/apps/sim/app/workspace/[workspaceId]/w/page.tsx index 8792827f0..f7ccd6f47 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/page.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/page.tsx @@ -7,10 +7,27 @@ import { useWorkflowRegistry } from '@/stores/workflows/registry/store' export default function WorkflowsPage() { const router = useRouter() - const { workflows, isLoading } = useWorkflowRegistry() + const { workflows, isLoading, loadWorkflows } = useWorkflowRegistry() const params = useParams() - const workspaceId = params.workspaceId + const workspaceId = params.workspaceId as string + + // Load workflows for this specific workspace when component mounts or workspaceId changes + // Only load if we don't already have workflows for this workspace (to prevent duplicate calls during workspace switches) + useEffect(() => { + if (workspaceId) { + // Check if we already have workflows for this workspace + const workflowIds = Object.keys(workflows) + const hasWorkflowsForWorkspace = + workflowIds.length > 0 && + Object.values(workflows).some((w) => w.workspaceId === workspaceId) + + // Only load if we don't have workflows for this workspace and we're not loading + if (!hasWorkflowsForWorkspace && !isLoading) { + loadWorkflows(workspaceId) + } + } + }, [workspaceId, loadWorkflows, isLoading, workflows]) useEffect(() => { // Wait for workflows to load @@ -18,7 +35,7 @@ export default function WorkflowsPage() { const workflowIds = Object.keys(workflows) - // If we have workflows, redirect to the first one + // If we have workflows, redirect to the first one (database already sorted by lastModified desc) if (workflowIds.length > 0) { router.replace(`/workspace/${workspaceId}/w/${workflowIds[0]}`) return diff --git a/apps/sim/contexts/socket-context.tsx b/apps/sim/contexts/socket-context.tsx index ce4327465..a8045e196 100644 --- a/apps/sim/contexts/socket-context.tsx +++ b/apps/sim/contexts/socket-context.tsx @@ -270,7 +270,6 @@ export function SocketProvider({ children, user }: SocketProviderProps) { }) socketInstance.on('workflow-state', (state) => { - logger.info('Received workflow state from server:', state) // This will be used to sync initial state when joining a workflow }) diff --git a/apps/sim/stores/workflows/registry/store.ts b/apps/sim/stores/workflows/registry/store.ts index 4968c2dac..3ea224732 100644 --- a/apps/sim/stores/workflows/registry/store.ts +++ b/apps/sim/stores/workflows/registry/store.ts @@ -151,6 +151,7 @@ async function fetchWorkflowsFromDB(workspaceId?: string): Promise { }) // Only set first workflow as active if no active workflow is set and we have workflows + // Database already returns workflows sorted by lastModified desc const currentState = useWorkflowRegistry.getState() if (!currentState.activeWorkflowId && Object.keys(registryWorkflows).length > 0) { const firstWorkflowId = Object.keys(registryWorkflows)[0]