-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Revert "fix(workspace url id bug): switch workspace bug " #567
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 | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -333,45 +333,23 @@ export const WorkspaceHeader = React.memo<WorkspaceHeaderProps>( | |||||||||||||||||||||||||||||||||||||||||||||||||
| }, [sessionData?.user?.id, fetchSubscriptionStatus, fetchWorkspaces]) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const switchWorkspace = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| async (workspace: Workspace) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| (workspace: Workspace) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // If already on this workspace, close dropdown and do nothing else | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (activeWorkspace?.id === workspace.id) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setWorkspaceDropdownOpen(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Close dropdown immediately for responsive feel | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setActiveWorkspace(workspace) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setWorkspaceDropdownOpen(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Update UI state optimistically | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setActiveWorkspace(workspace) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Use full workspace switch which now handles localStorage automatically | ||||||||||||||||||||||||||||||||||||||||||||||||||
| switchToWorkspace(workspace.id) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Update URL to include workspace ID | ||||||||||||||||||||||||||||||||||||||||||||||||||
| router.push(`/workspace/${workspace.id}/w`) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| activeWorkspace?.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| switchToWorkspace, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| router, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setWorkspaceDropdownOpen, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| workspaces, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| currentWorkspaceId, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [activeWorkspace?.id, switchToWorkspace, router, setWorkspaceDropdownOpen] | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleCreateWorkspace = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -394,11 +372,11 @@ export const WorkspaceHeader = React.memo<WorkspaceHeaderProps>( | |||||||||||||||||||||||||||||||||||||||||||||||||
| setWorkspaces((prev) => [...prev, newWorkspace]) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setActiveWorkspace(newWorkspace) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Switch workspace data first with the explicit workspace ID | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Use switchToWorkspace to properly load workflows for the new workspace | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // This will clear existing workflows, set loading state, and fetch workflows from DB | ||||||||||||||||||||||||||||||||||||||||||||||||||
| switchToWorkspace(newWorkspace.id) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Then update URL to include new workspace ID | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Update URL to include new workspace ID | ||||||||||||||||||||||||||||||||||||||||||||||||||
| router.push(`/workspace/${newWorkspace.id}/w`) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -487,14 +465,10 @@ export const WorkspaceHeader = React.memo<WorkspaceHeaderProps>( | |||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // If deleted workspace was active, switch to another workspace | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (activeWorkspace?.id === id && updatedWorkspaces.length > 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`) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Use the specialized method for handling workspace deletion | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const newWorkspaceId = updatedWorkspaces[0].id | ||||||||||||||||||||||||||||||||||||||||||||||||||
| useWorkflowRegistry.getState().handleWorkspaceDeletion(newWorkspaceId) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setActiveWorkspace(updatedWorkspaces[0]) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
466
to
472
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. logic: Missing router.push after workspace deletion. User may remain on deleted workspace URL.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| setWorkspaceDropdownOpen(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -504,7 +478,7 @@ export const WorkspaceHeader = React.memo<WorkspaceHeaderProps>( | |||||||||||||||||||||||||||||||||||||||||||||||||
| setIsDeleting(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [workspaces, activeWorkspace?.id, router] | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [workspaces, activeWorkspace?.id] | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const openEditModal = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,35 +7,18 @@ import { useWorkflowRegistry } from '@/stores/workflows/registry/store' | |
|
|
||
| export default function WorkflowsPage() { | ||
| const router = useRouter() | ||
| const { workflows, isLoading, loadWorkflows } = useWorkflowRegistry() | ||
| const { workflows, isLoading } = useWorkflowRegistry() | ||
|
|
||
| const params = useParams() | ||
| 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]) | ||
| const workspaceId = params.workspaceId | ||
|
|
||
| useEffect(() => { | ||
| // Wait for workflows to load | ||
| if (isLoading) return | ||
|
|
||
| const workflowIds = Object.keys(workflows) | ||
|
|
||
| // If we have workflows, redirect to the first one (database already sorted by lastModified desc) | ||
| // If we have workflows, redirect to the first one | ||
| if (workflowIds.length > 0) { | ||
| router.replace(`/workspace/${workspaceId}/w/${workflowIds[0]}`) | ||
| return | ||
|
Comment on lines
+21
to
24
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. logic: No verification that workflows belong to current workspace before redirect. Could cause incorrect routing |
||
|
|
||
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.
logic: Missing orderBy clause - workflows are no longer sorted by creation date which could affect frontend display order